fix(sync): atomic export and force-push detection (bd-3bhl, bd-4hh5)

bd-3bhl: Add sync rollback on git commit failure
- Use exportToJSONLDeferred() instead of exportToJSONL() for atomic sync
- Call finalizeExport() only after git commit succeeds
- Rollback JSONL from git HEAD on commit failure
- Add rollbackJSONLFromGit() helper function
- Coverage: regular commit, sync branch, external beads repo paths

bd-4hh5: Fix false-positive force-push detection
- Use explicit refspec in CheckForcePush fetch
- +refs/heads/beads-sync:refs/remotes/origin/beads-sync
- Ensures tracking ref is always created/updated
- Fixes stale ref comparison causing false positives

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
emma
2026-01-04 23:13:56 -08:00
committed by Steve Yegge
parent d7221f6858
commit 9b84ef73dd
3 changed files with 88 additions and 5 deletions

View File

@@ -168,6 +168,11 @@ Use --merge to merge the sync branch back to main branch.`,
// leaving the JSONL in an inconsistent state across worktrees.
// Track if we already exported during pre-flight to avoid redundant export later.
alreadyExported := false
// GH#885/bd-3bhl: Track pending export for atomic sync.
// When using deferred export, we store the result here and finalize
// only after git commit succeeds. If commit fails, we rollback JSONL.
var pendingExportResult *ExportResult
if hasUncommitted, err := gitHasUncommittedBeadsChanges(ctx); err != nil {
fmt.Fprintf(os.Stderr, "Warning: failed to check for uncommitted changes: %v\n", err)
} else if hasUncommitted {
@@ -311,10 +316,16 @@ Use --merge to merge the sync branch back to main branch.`,
FatalError("%v", err)
}
// GH#885/bd-3bhl: Use deferred export for atomic sync.
// Metadata updates are deferred until after git commit succeeds.
// This prevents SQLite from "lying" about sync state if commit fails.
fmt.Println("→ Exporting pending changes to JSONL...")
if err := exportToJSONL(ctx, jsonlPath); err != nil {
exportResult, err := exportToJSONLDeferred(ctx, jsonlPath)
if err != nil {
FatalError("exporting: %v", err)
}
// Store result for finalization after commit
pendingExportResult = exportResult
}
// Capture left snapshot (pre-pull state) for 3-way merge
@@ -356,9 +367,21 @@ Use --merge to merge the sync branch back to main branch.`,
} else {
committed, err := commitToExternalBeadsRepo(ctx, beadsDir, message, !noPush)
if err != nil {
FatalError("%v", err)
// GH#885/bd-3bhl: Rollback JSONL on commit failure
fmt.Fprintf(os.Stderr, "\n⚠ Git commit failed - rolling back JSONL to previous state...\n")
if rollbackErr := rollbackJSONLFromGit(ctx, jsonlPath); rollbackErr != nil {
fmt.Fprintf(os.Stderr, "Warning: failed to rollback JSONL: %v\n", rollbackErr)
fmt.Fprintf(os.Stderr, " Manual recovery: git checkout HEAD -- %s\n", jsonlPath)
} else {
fmt.Fprintf(os.Stderr, "✓ JSONL rolled back to last committed state\n")
}
FatalErrorWithHint(fmt.Sprintf("%v", err),
"fix the git issue and run 'bd sync' again")
}
if committed {
// GH#885/bd-3bhl: Finalize export after successful commit
finalizeExport(ctx, pendingExportResult)
pendingExportResult = nil
if !noPush {
fmt.Println("✓ Committed and pushed to external beads repo")
} else {
@@ -368,6 +391,9 @@ Use --merge to merge the sync branch back to main branch.`,
}
} else {
fmt.Println("→ No changes to commit in external beads repo")
// GH#885/bd-3bhl: No commit needed, but still finalize export metadata
finalizeExport(ctx, pendingExportResult)
pendingExportResult = nil
}
if !noPull {
@@ -549,10 +575,22 @@ Use --merge to merge the sync branch back to main branch.`,
fmt.Printf("→ Committing changes to sync branch '%s'...\n", syncBranchName)
result, err := syncbranch.CommitToSyncBranch(ctx, repoRoot, syncBranchName, jsonlPath, !noPush)
if err != nil {
FatalError("committing to sync branch: %v", err)
// GH#885/bd-3bhl: Rollback JSONL on commit failure
fmt.Fprintf(os.Stderr, "\n⚠ Git commit failed - rolling back JSONL to previous state...\n")
if rollbackErr := rollbackJSONLFromGit(ctx, jsonlPath); rollbackErr != nil {
fmt.Fprintf(os.Stderr, "Warning: failed to rollback JSONL: %v\n", rollbackErr)
fmt.Fprintf(os.Stderr, " Manual recovery: git checkout HEAD -- %s\n", jsonlPath)
} else {
fmt.Fprintf(os.Stderr, "✓ JSONL rolled back to last committed state\n")
}
FatalErrorWithHint(fmt.Sprintf("committing to sync branch: %v", err),
"fix the git issue and run 'bd sync' again")
}
if result.Committed {
fmt.Printf("✓ Committed to %s\n", syncBranchName)
// GH#885/bd-3bhl: Finalize export after successful commit
finalizeExport(ctx, pendingExportResult)
pendingExportResult = nil
if result.Pushed {
fmt.Printf("✓ Pushed %s to remote\n", syncBranchName)
pushedViaSyncBranch = true
@@ -561,6 +599,9 @@ Use --merge to merge the sync branch back to main branch.`,
// GH#812: When useSyncBranch is true, we always attempt commit
// (bypassing gitHasBeadsChanges). Report when worktree has no changes.
fmt.Println("→ No changes to commit")
// No commit needed, but still finalize export metadata
finalizeExport(ctx, pendingExportResult)
pendingExportResult = nil
}
} else {
// Regular commit to current branch
@@ -571,11 +612,26 @@ Use --merge to merge the sync branch back to main branch.`,
fmt.Println("→ Committing changes to git...")
}
if err := gitCommitBeadsDir(ctx, message); err != nil {
FatalError("committing: %v", err)
// GH#885/bd-3bhl: Rollback JSONL on commit failure
fmt.Fprintf(os.Stderr, "\n⚠ Git commit failed - rolling back JSONL to previous state...\n")
if rollbackErr := rollbackJSONLFromGit(ctx, jsonlPath); rollbackErr != nil {
fmt.Fprintf(os.Stderr, "Warning: failed to rollback JSONL: %v\n", rollbackErr)
fmt.Fprintf(os.Stderr, " Manual recovery: git checkout HEAD -- %s\n", jsonlPath)
} else {
fmt.Fprintf(os.Stderr, "✓ JSONL rolled back to last committed state\n")
}
FatalErrorWithHint(fmt.Sprintf("committing: %v", err),
"fix the git issue and run 'bd sync' again")
}
// GH#885/bd-3bhl: Finalize export after successful commit
finalizeExport(ctx, pendingExportResult)
pendingExportResult = nil
}
} else {
fmt.Println("→ No changes to commit")
// GH#885/bd-3bhl: No commit needed, but still finalize export metadata
finalizeExport(ctx, pendingExportResult)
pendingExportResult = nil
}
// Step 3: Pull from remote

View File

@@ -492,6 +492,27 @@ func parseGitStatusForBeadsChanges(statusOutput string) bool {
return false
}
// rollbackJSONLFromGit restores the JSONL file from git HEAD after a failed commit.
// This is part of the sync atomicity fix (GH#885/bd-3bhl): when git commit fails
// after export, we restore the JSONL to its previous state so the working
// directory stays consistent with the last successful sync.
func rollbackJSONLFromGit(ctx context.Context, jsonlPath string) error {
// Check if the file is tracked by git
cmd := exec.CommandContext(ctx, "git", "ls-files", "--error-unmatch", jsonlPath)
if err := cmd.Run(); err != nil {
// File not tracked - nothing to restore
return nil
}
// Restore from HEAD
restoreCmd := exec.CommandContext(ctx, "git", "checkout", "HEAD", "--", jsonlPath) //nolint:gosec // G204: jsonlPath from internal beads.FindBeadsDir()
output, err := restoreCmd.CombinedOutput()
if err != nil {
return fmt.Errorf("git checkout failed: %w\n%s", err, output)
}
return nil
}
// getDefaultBranch returns the default branch name (main or master) for origin remote
// Checks remote HEAD first, then falls back to checking if main/master exist
func getDefaultBranch(ctx context.Context) string {

View File

@@ -79,7 +79,13 @@ func CheckForcePush(ctx context.Context, store storage.Storage, repoRoot, syncBr
status.Remote = getRemoteForBranch(ctx, worktreePath, syncBranch)
// Fetch from remote to get latest state
fetchCmd := exec.CommandContext(ctx, "git", "-C", repoRoot, "fetch", status.Remote, syncBranch) // #nosec G204 - repoRoot/syncBranch are validated git inputs
// bd-4hh5: Use explicit refspec to ensure the remote-tracking ref is always updated.
// Without an explicit refspec, `git fetch origin beads-sync` only updates
// refs/remotes/origin/beads-sync if it already exists. On fresh clones or
// after ref cleanup, this can leave the tracking ref stale, causing
// false-positive force-push detection when comparing against wrong commits.
refspec := fmt.Sprintf("+refs/heads/%s:refs/remotes/%s/%s", syncBranch, status.Remote, syncBranch)
fetchCmd := exec.CommandContext(ctx, "git", "-C", repoRoot, "fetch", status.Remote, refspec) // #nosec G204 - repoRoot/syncBranch are validated git inputs
fetchOutput, err := fetchCmd.CombinedOutput()
if err != nil {
// Check if remote branch doesn't exist