johno/beads
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(sync): use sync-branch worktree for --full --no-pull (#1183)

Browse Source 
* fix(sync): use sync-branch worktree for --full --no-pull (#1173)

Bug 1: PullFromSyncBranch was copying uncommitted worktree changes to
main repo when remoteAhead==0. This corrupted the 3-way merge because
local changes appeared as remote changes. Fixed by copying only the
committed state from HEAD instead of the working directory.

Bug 2: doExportOnlySync was checking main repo for changes via
gitHasBeadsChanges, but when sync-branch is configured, changes go to
the worktree, not main. Fixed by detecting sync-branch config and using
CommitToSyncBranch which operates on the worktree.

Fixes #1173

* refactor(sync): consolidate sync-branch detection and commit/push logic

Extract repeated patterns into reusable helpers:

- SyncBranchContext struct: holds branch name and repo root
- getSyncBranchContext(): detects sync-branch config from store
- commitAndPushBeads(): handles both sync-branch and regular git workflows

This eliminates duplicated sync-branch detection code (was in 3 places)
and the duplicated commit/push conditional logic (was in 2 places).

Net reduction of ~20 lines while improving maintainability.

* fix: remove unused bool return from commitAndPushBeads
This commit is contained in:
John Zila
2026-01-20 16:06:22 -06:00
committed by GitHub
parent d929c8f974
commit f336e669e9
2 changed files with 127 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files

173
cmd/bd/sync.go
View File

@@ -18,6 +18,81 @@ import (
"github.com/steveyegge/beads/internal/syncbranch"
)
// SyncBranchContext holds sync-branch configuration detected from the store.
// This consolidates the repeated pattern of checking for sync-branch config.
type SyncBranchContext struct {
Branch string // Sync branch name, empty if not configured
RepoRoot string // Git repository root path
}
// IsConfigured returns true if a sync branch is configured.
func (s *SyncBranchContext) IsConfigured() bool {
return s.Branch != ""
}
// getSyncBranchContext detects sync-branch configuration from the store.
// Returns a context with empty Branch if not configured or on error.
func getSyncBranchContext(ctx context.Context) *SyncBranchContext {
sbc := &SyncBranchContext{}
if err := ensureStoreActive(); err != nil || store == nil {
return sbc
}
if sb, _ := syncbranch.Get(ctx, store); sb != "" {
sbc.Branch = sb
if rc, err := beads.GetRepoContext(); err == nil {
sbc.RepoRoot = rc.RepoRoot
}
}
return sbc
}
// commitAndPushBeads commits and pushes .beads changes using the appropriate method.
// When sync-branch is configured, uses worktree-based commit/push.
// Otherwise, uses standard git commit/push on the current branch.
func commitAndPushBeads(ctx context.Context, sbc *SyncBranchContext, jsonlPath string, noPush bool, message string) error {
if sbc.IsConfigured() {
fmt.Printf("→ Committing to sync branch '%s'...\n", sbc.Branch)
commitResult, err := syncbranch.CommitToSyncBranch(ctx, sbc.RepoRoot, sbc.Branch, jsonlPath, !noPush)
if err != nil {
return fmt.Errorf("committing to sync branch: %w", err)
}
if commitResult.Committed {
fmt.Printf(" Committed: %s\n", commitResult.Message)
if commitResult.Pushed {
fmt.Println(" Pushed to remote")
}
} else {
fmt.Println("→ No changes to commit")
}
return nil
}
// Standard git workflow
hasChanges, err := gitHasBeadsChanges(ctx)
if err != nil {
return fmt.Errorf("checking git status: %w", err)
}
if hasChanges {
fmt.Println("→ Committing changes...")
if err := gitCommitBeadsDir(ctx, message); err != nil {
return fmt.Errorf("committing: %w", err)
}
} else {
fmt.Println("→ No changes to commit")
}
// Push to remote
if !noPush && hasChanges {
fmt.Println("→ Pushing to remote...")
if err := gitPush(ctx, ""); err != nil {
return fmt.Errorf("pushing: %w", err)
}
}
return nil
}
var syncCmd = &cobra.Command{
Use: "sync",
GroupID: "sync",
@@ -264,16 +339,7 @@ The --full flag provides the legacy full sync behavior for backwards compatibili
// GH#638: Check sync.branch BEFORE upstream check
// When sync.branch is configured, we should use worktree-based sync even if
// the current branch has no upstream (e.g., detached HEAD in jj, git worktrees)
var syncBranchName, syncBranchRepoRoot string
if err := ensureStoreActive(); err == nil && store != nil {
if sb, _ := syncbranch.Get(ctx, store); sb != "" {
syncBranchName = sb
if rc, err := beads.GetRepoContext(); err == nil {
syncBranchRepoRoot = rc.RepoRoot
}
}
}
hasSyncBranchConfig := syncBranchName != ""
sbc := getSyncBranchContext(ctx)
// GH#1166: Block sync if currently on the sync branch
// This must happen BEFORE worktree operations - after entering a worktree,
@@ -294,7 +360,7 @@ The --full flag provides the legacy full sync behavior for backwards compatibili
// the beads files are in a different git repo than the current working directory.
redirectInfo := beads.GetRedirectInfo()
if redirectInfo.IsRedirected {
if hasSyncBranchConfig {
if sbc.IsConfigured() {
fmt.Printf("⚠️ Redirect active (-> %s), skipping sync-branch operations\n", redirectInfo.TargetDir)
fmt.Println(" Hint: Redirected clones should not have sync-branch configured")
fmt.Println(" The owner of the target .beads directory handles sync-branch")
@@ -319,7 +385,7 @@ The --full flag provides the legacy full sync behavior for backwards compatibili
// Preflight: check for upstream tracking
// If no upstream, automatically switch to --from-main mode (gt-ick9: ephemeral branch support)
// GH#638: Skip this fallback if sync.branch is explicitly configured
if !noPull && !gitHasUpstream() && !hasSyncBranchConfig {
if !noPull && !gitHasUpstream() && !sbc.IsConfigured() {
if hasGitRemote(ctx) {
// Remote exists but no upstream - use from-main mode
fmt.Println("→ No upstream configured, using --from-main mode")
@@ -335,7 +401,7 @@ The --full flag provides the legacy full sync behavior for backwards compatibili
// Pull-first sync: Pull → Merge → Export → Commit → Push
// This eliminates the export-before-pull data loss pattern (#911) by
// seeing remote changes before exporting local state.
if err := doPullFirstSync(ctx, jsonlPath, renameOnImport, noGitHistory, dryRun, noPush, noPull, message, acceptRebase, syncBranchName, syncBranchRepoRoot); err != nil {
if err := doPullFirstSync(ctx, jsonlPath, renameOnImport, noGitHistory, dryRun, noPush, noPull, message, acceptRebase, sbc); err != nil {
FatalError("%v", err)
}
},
@@ -354,7 +420,7 @@ The --full flag provides the legacy full sync behavior for backwards compatibili
//
// When noPull is true, skips the pull/merge steps and just does:
// Export → Commit → Push
func doPullFirstSync(ctx context.Context, jsonlPath string, renameOnImport, noGitHistory, dryRun, noPush, noPull bool, message string, acceptRebase bool, syncBranch, syncBranchRepoRoot string) error {
func doPullFirstSync(ctx context.Context, jsonlPath string, renameOnImport, noGitHistory, dryRun, noPush, noPull bool, message string, acceptRebase bool, sbc *SyncBranchContext) error {
beadsDir := filepath.Dir(jsonlPath)
_ = acceptRebase // Reserved for future sync branch force-push detection
@@ -388,9 +454,6 @@ func doPullFirstSync(ctx context.Context, jsonlPath string, renameOnImport, noGi
return fmt.Errorf("activating store: %w", err)
}
// Derive sync-branch config from parameters (detected at caller)
hasSyncBranchConfig := syncBranch != ""
localIssues, err := store.SearchIssues(ctx, "", beads.IssueFilter{IncludeTombstones: true})
if err != nil {
return fmt.Errorf("loading local issues: %w", err)
@@ -452,9 +515,9 @@ func doPullFirstSync(ctx context.Context, jsonlPath string, renameOnImport, noGi
// Git-based pull (for git-portable, belt-and-suspenders, or when Dolt not available)
if ShouldExportJSONL(ctx, store) {
if hasSyncBranchConfig {
fmt.Printf("→ Pulling from sync branch '%s'...\n", syncBranch)
pullResult, err := syncbranch.PullFromSyncBranch(ctx, syncBranchRepoRoot, syncBranch, jsonlPath, false)
if sbc.IsConfigured() {
fmt.Printf("→ Pulling from sync branch '%s'...\n", sbc.Branch)
pullResult, err := syncbranch.PullFromSyncBranch(ctx, sbc.RepoRoot, sbc.Branch, jsonlPath, false)
if err != nil {
return fmt.Errorf("pulling from sync branch: %w", err)
}
@@ -527,46 +590,9 @@ func doPullFirstSync(ctx context.Context, jsonlPath string, renameOnImport, noGi
return fmt.Errorf("exporting: %w", err)
}
// Step 8: Check for changes and commit
// Step 9: Push to remote
// When sync.branch is configured, use worktree-based commit/push to sync branch
// Otherwise, use normal git commit/push on the current branch
if hasSyncBranchConfig {
fmt.Printf("→ Committing to sync branch '%s'...\n", syncBranch)
commitResult, err := syncbranch.CommitToSyncBranch(ctx, syncBranchRepoRoot, syncBranch, jsonlPath, !noPush)
if err != nil {
return fmt.Errorf("committing to sync branch: %w", err)
}
if commitResult.Committed {
fmt.Printf(" Committed: %s\n", commitResult.Message)
if commitResult.Pushed {
fmt.Println(" Pushed to remote")
}
} else {
fmt.Println("→ No changes to commit")
}
} else {
hasChanges, err := gitHasBeadsChanges(ctx)
if err != nil {
return fmt.Errorf("checking git status: %w", err)
}
if hasChanges {
fmt.Println("→ Committing changes...")
if err := gitCommitBeadsDir(ctx, message); err != nil {
return fmt.Errorf("committing: %w", err)
}
} else {
fmt.Println("→ No changes to commit")
}
// Push to remote
if !noPush && hasChanges {
fmt.Println("→ Pushing to remote...")
if err := gitPush(ctx, ""); err != nil {
return fmt.Errorf("pushing: %w", err)
}
}
// Step 8 & 9: Commit and push changes
if err := commitAndPushBeads(ctx, sbc, jsonlPath, noPush, message); err != nil {
return err
}
// Step 10: Update base state for next sync (after successful push)
@@ -627,32 +653,17 @@ func doExportOnlySync(ctx context.Context, jsonlPath string, noPush bool, messag
return err
}
// GH#1173: Detect sync-branch configuration and use appropriate commit method
sbc := getSyncBranchContext(ctx)
fmt.Println("→ Exporting pending changes to JSONL...")
if err := exportToJSONL(ctx, jsonlPath); err != nil {
return fmt.Errorf("exporting: %w", err)
}
// Check for changes and commit
hasChanges, err := gitHasBeadsChanges(ctx)
if err != nil {
return fmt.Errorf("checking git status: %w", err)
}
if hasChanges {
fmt.Println("→ Committing changes...")
if err := gitCommitBeadsDir(ctx, message); err != nil {
return fmt.Errorf("committing: %w", err)
}
} else {
fmt.Println("→ No changes to commit")
}
// Push to remote
if !noPush && hasChanges {
fmt.Println("→ Pushing to remote...")
if err := gitPush(ctx, ""); err != nil {
return fmt.Errorf("pushing: %w", err)
}
// Commit and push using the appropriate method (sync-branch worktree or regular git)
if err := commitAndPushBeads(ctx, sbc, jsonlPath, noPush, message); err != nil {
return err
}
// Clear sync state on successful sync

37
internal/syncbranch/worktree.go
View File

@@ -272,8 +272,12 @@ func PullFromSyncBranch(ctx context.Context, repoRoot, syncBranch, jsonlPath str
// Case 1: Already up to date (remote has nothing new)
if remoteAhead == 0 {
result.Pulled = true
// Still copy JSONL in case worktree has uncommitted changes
if err := copyJSONLToMainRepo(worktreePath, jsonlRelPath, jsonlPath); err != nil {
// GH#1173: Do NOT copy uncommitted worktree changes to main repo.
// The worktree may have uncommitted changes from previous exports that
// haven't been committed yet. Copying those to main would make local
// data appear as "remote" data, corrupting the 3-way merge.
// Instead, copy only the COMMITTED state from the worktree.
if err := copyCommittedJSONLToMainRepo(ctx, worktreePath, jsonlRelPath, jsonlPath); err != nil {
return nil, err
}
return result, nil
@@ -655,6 +659,35 @@ func extractJSONLFromCommit(ctx context.Context, worktreePath, commit, filePath
return output, nil
}
// copyCommittedJSONLToMainRepo copies the COMMITTED JSONL from worktree to main repo.
// GH#1173: This extracts the file from HEAD rather than the working directory,
// ensuring uncommitted local changes don't corrupt the 3-way merge.
func copyCommittedJSONLToMainRepo(ctx context.Context, worktreePath, jsonlRelPath, jsonlPath string) error {
// GH#785: Handle bare repo worktrees
normalizedRelPath := normalizeBeadsRelPath(jsonlRelPath)
// Extract the committed JSONL from HEAD
data, err := extractJSONLFromCommit(ctx, worktreePath, "HEAD", normalizedRelPath)
if err != nil {
// File might not exist in HEAD yet (first sync), nothing to copy
return nil
}
if err := os.WriteFile(jsonlPath, data, 0600); err != nil {
return fmt.Errorf("failed to write main JSONL: %w", err)
}
// Also copy committed metadata.json if it exists
beadsDir := filepath.Dir(jsonlPath)
metadataRelPath := filepath.Join(filepath.Dir(normalizedRelPath), "metadata.json")
if metaData, err := extractJSONLFromCommit(ctx, worktreePath, "HEAD", metadataRelPath); err == nil {
dstPath := filepath.Join(beadsDir, "metadata.json")
_ = os.WriteFile(dstPath, metaData, 0600) // Best effort
}
return nil
}
// copyJSONLToMainRepo copies JSONL and related files from worktree to main repo.
func copyJSONLToMainRepo(worktreePath, jsonlRelPath, jsonlPath string) error {
// GH#785: Handle bare repo worktrees where jsonlRelPath might include the