From def4cf4efad36b334d8ca6bf7cf868fd2819bd8c Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Tue, 30 Dec 2025 17:06:01 -0800 Subject: [PATCH] Add sync branch integrity guards for force-push detection (bd-hlsw.4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This implements sync branch integrity guards that detect when the remote sync branch has been force-pushed since the last sync, preventing silent data corruption. Changes: - Add internal/syncbranch/integrity.go with: - CheckForcePush() - detects force-push via stored remote SHA comparison - UpdateStoredRemoteSHA() - stores current remote SHA after successful sync - ClearStoredRemoteSHA() - clears stored SHA when resetting - GetStoredRemoteSHA() - retrieves stored SHA for inspection - Update cmd/bd/sync.go to: - Add --accept-rebase flag for non-interactive reset to remote - Add force-push detection before sync branch pull operations - Prompt user for confirmation when force-push detected - Update stored remote SHA after successful sync The implementation: 1. Tracks the remote sync branch commit SHA in config after each sync 2. On subsequent syncs, checks if stored SHA is ancestor of current remote 3. If not (force-push detected), warns user with details and prompts 4. User can accept reset or abort to investigate manually 5. --accept-rebase flag allows scripted/non-interactive recovery šŸ¤– Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- cmd/bd/sync.go | 90 +++++++++++++ internal/syncbranch/integrity.go | 179 ++++++++++++++++++++++++++ internal/syncbranch/integrity_test.go | 132 +++++++++++++++++++ 3 files changed, 401 insertions(+) create mode 100644 internal/syncbranch/integrity.go create mode 100644 internal/syncbranch/integrity_test.go diff --git a/cmd/bd/sync.go b/cmd/bd/sync.go index 7ec9966a..5d568747 100644 --- a/cmd/bd/sync.go +++ b/cmd/bd/sync.go @@ -50,6 +50,7 @@ Use --merge to merge the sync branch back to main branch.`, noGitHistory, _ := cmd.Flags().GetBool("no-git-history") squash, _ := cmd.Flags().GetBool("squash") checkIntegrity, _ := cmd.Flags().GetBool("check") + acceptRebase, _ := cmd.Flags().GetBool("accept-rebase") // If --no-push not explicitly set, check no-push config if !cmd.Flags().Changed("no-push") { @@ -393,6 +394,86 @@ Use --merge to merge the sync branch back to main branch.`, } } + // Force-push detection for sync branch (bd-hlsw.4) + // Check if the remote sync branch was force-pushed since last sync + if useSyncBranch && !noPull && !dryRun { + forcePushStatus, err := syncbranch.CheckForcePush(ctx, store, repoRoot, syncBranchName) + if err != nil { + fmt.Fprintf(os.Stderr, "Warning: could not check for force-push: %v\n", err) + } else if forcePushStatus.Detected { + fmt.Fprintf(os.Stderr, "\nāš ļø %s\n\n", forcePushStatus.Message) + + if acceptRebase { + // User explicitly accepted the rebase via --accept-rebase flag + fmt.Println("ā†’ --accept-rebase specified, resetting to remote state...") + if err := syncbranch.ResetToRemote(ctx, repoRoot, syncBranchName, jsonlPath); err != nil { + FatalError("failed to reset to remote: %v", err) + } + // Clear the stored SHA since we're resetting + if err := syncbranch.ClearStoredRemoteSHA(ctx, store); err != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to clear stored remote SHA: %v\n", err) + } + fmt.Println("āœ“ Reset to remote sync branch state") + fmt.Println("ā†’ Re-importing JSONL after reset...") + if err := importFromJSONL(ctx, jsonlPath, renameOnImport, noGitHistory); err != nil { + FatalError("importing after reset: %v", err) + } + fmt.Println("āœ“ Import complete after reset") + + // Update stored SHA to current remote + if err := syncbranch.UpdateStoredRemoteSHA(ctx, store, repoRoot, syncBranchName); err != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to update stored remote SHA: %v\n", err) + } + + fmt.Println("\nāœ“ Sync complete (reset to remote after force-push)") + return + } + + // Prompt for confirmation + fmt.Fprintln(os.Stderr, "Options:") + fmt.Fprintln(os.Stderr, " 1. Reset to remote (discard local sync branch changes)") + fmt.Fprintln(os.Stderr, " 2. Abort sync (investigate manually)") + fmt.Fprintln(os.Stderr, "") + fmt.Fprintln(os.Stderr, "To reset automatically, run: bd sync --accept-rebase") + fmt.Fprintln(os.Stderr, "") + fmt.Fprint(os.Stderr, "Reset to remote state? [y/N]: ") + + var response string + reader := bufio.NewReader(os.Stdin) + response, _ = reader.ReadString('\n') + response = strings.TrimSpace(strings.ToLower(response)) + + if response == "y" || response == "yes" { + fmt.Println("ā†’ Resetting to remote state...") + if err := syncbranch.ResetToRemote(ctx, repoRoot, syncBranchName, jsonlPath); err != nil { + FatalError("failed to reset to remote: %v", err) + } + // Clear the stored SHA since we're resetting + if err := syncbranch.ClearStoredRemoteSHA(ctx, store); err != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to clear stored remote SHA: %v\n", err) + } + fmt.Println("āœ“ Reset to remote sync branch state") + fmt.Println("ā†’ Re-importing JSONL after reset...") + if err := importFromJSONL(ctx, jsonlPath, renameOnImport, noGitHistory); err != nil { + FatalError("importing after reset: %v", err) + } + fmt.Println("āœ“ Import complete after reset") + + // Update stored SHA to current remote + if err := syncbranch.UpdateStoredRemoteSHA(ctx, store, repoRoot, syncBranchName); err != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to update stored remote SHA: %v\n", err) + } + + fmt.Println("\nāœ“ Sync complete (reset to remote after force-push)") + return + } + + // User chose to abort + FatalErrorWithHint("sync aborted due to force-push detection", + "investigate the sync branch history, then run 'bd sync --accept-rebase' to reset to remote") + } + } + // Step 2: Check if there are changes to commit (check entire .beads/ directory) hasChanges, err := gitHasBeadsChanges(ctx) if err != nil { @@ -716,6 +797,14 @@ Use --merge to merge the sync branch back to main branch.`, _ = ClearSyncState(bd) } + // Update stored remote SHA after successful sync (bd-hlsw.4) + // This enables force-push detection on subsequent syncs + if useSyncBranch && !noPush { + if err := syncbranch.UpdateStoredRemoteSHA(ctx, store, repoRoot, syncBranchName); err != nil { + debug.Logf("sync: failed to update stored remote SHA: %v", err) + } + } + fmt.Println("\nāœ“ Sync complete") } }, @@ -736,6 +825,7 @@ func init() { syncCmd.Flags().Bool("no-git-history", false, "Skip git history backfill for deletions (use during JSONL filename migrations)") syncCmd.Flags().BoolVar(&jsonOutput, "json", false, "Output sync statistics in JSON format") syncCmd.Flags().Bool("check", false, "Pre-sync integrity check: detect forced pushes, prefix mismatches, and orphaned issues") + syncCmd.Flags().Bool("accept-rebase", false, "Accept remote sync branch history (use when force-push detected)") rootCmd.AddCommand(syncCmd) } diff --git a/internal/syncbranch/integrity.go b/internal/syncbranch/integrity.go new file mode 100644 index 00000000..1aa06273 --- /dev/null +++ b/internal/syncbranch/integrity.go @@ -0,0 +1,179 @@ +// Package syncbranch provides sync branch configuration and integrity checking. +package syncbranch + +import ( + "context" + "fmt" + "os/exec" + "strings" + + "github.com/steveyegge/beads/internal/storage" +) + +// Config keys for sync branch integrity tracking +const ( + // RemoteSHAConfigKey stores the last known remote sync branch commit SHA. + // This is used to detect force pushes on the remote sync branch. + RemoteSHAConfigKey = "sync.remote_sha" +) + +// ForcePushStatus represents the result of a force-push detection check. +type ForcePushStatus struct { + // Detected is true if a force-push was detected on the remote sync branch. + Detected bool + + // StoredSHA is the SHA we stored after the last successful sync. + StoredSHA string + + // CurrentRemoteSHA is the current SHA of the remote sync branch. + CurrentRemoteSHA string + + // Message provides a human-readable description of the status. + Message string + + // Branch is the sync branch name. + Branch string + + // Remote is the remote name (e.g., "origin"). + Remote string +} + +// CheckForcePush detects if the remote sync branch has been force-pushed since the last sync. +// +// A force-push is detected when: +// 1. We have a stored remote SHA from a previous sync +// 2. The stored SHA is NOT an ancestor of the current remote SHA +// +// This means the remote history was rewritten (e.g., via force-push, rebase). +// +// Parameters: +// - ctx: Context for cancellation +// - store: Storage interface for reading config +// - repoRoot: Path to the git repository root +// - syncBranch: Name of the sync branch (e.g., "beads-sync") +// +// Returns ForcePushStatus with details about the check. +func CheckForcePush(ctx context.Context, store storage.Storage, repoRoot, syncBranch string) (*ForcePushStatus, error) { + status := &ForcePushStatus{ + Detected: false, + Branch: syncBranch, + } + + // Get stored remote SHA from last sync + storedSHA, err := store.GetConfig(ctx, RemoteSHAConfigKey) + if err != nil { + return nil, fmt.Errorf("failed to get stored remote SHA: %w", err) + } + + status.StoredSHA = storedSHA + + if storedSHA == "" { + status.Message = "No previous sync recorded (first sync)" + return status, nil + } + + // Get worktree path for git operations + worktreePath := getBeadsWorktreePath(ctx, repoRoot, syncBranch) + + // Get remote name + status.Remote = getRemoteForBranch(ctx, worktreePath, syncBranch) + + // Fetch from remote to get latest state + fetchCmd := exec.CommandContext(ctx, "git", "-C", repoRoot, "fetch", status.Remote, syncBranch) + fetchOutput, err := fetchCmd.CombinedOutput() + if err != nil { + // Check if remote branch doesn't exist + if strings.Contains(string(fetchOutput), "couldn't find remote ref") { + status.Message = "Remote sync branch does not exist" + return status, nil + } + return nil, fmt.Errorf("failed to fetch remote: %w", err) + } + + // Get current remote SHA + remoteRef := fmt.Sprintf("%s/%s", status.Remote, syncBranch) + revParseCmd := exec.CommandContext(ctx, "git", "-C", repoRoot, "rev-parse", remoteRef) + revParseOutput, err := revParseCmd.Output() + if err != nil { + return nil, fmt.Errorf("failed to get remote SHA: %w", err) + } + status.CurrentRemoteSHA = strings.TrimSpace(string(revParseOutput)) + + // If SHA matches, no change at all + if storedSHA == status.CurrentRemoteSHA { + status.Message = "Remote sync branch unchanged since last sync" + return status, nil + } + + // Check if stored SHA is an ancestor of current remote SHA + // This means remote was updated normally (fast-forward) + isAncestorCmd := exec.CommandContext(ctx, "git", "-C", repoRoot, "merge-base", "--is-ancestor", storedSHA, status.CurrentRemoteSHA) + if isAncestorCmd.Run() == nil { + // Stored SHA is ancestor - normal update, no force-push + status.Message = "Remote sync branch updated normally (fast-forward)" + return status, nil + } + + // Stored SHA is NOT an ancestor - this indicates a force-push or rebase + status.Detected = true + status.Message = fmt.Sprintf( + "FORCE-PUSH DETECTED: Remote sync branch history was rewritten.\n"+ + " Previous known commit: %s\n"+ + " Current remote commit: %s\n"+ + " The remote history no longer contains your previously synced commit.\n"+ + " This typically happens when someone force-pushed or rebased the sync branch.", + storedSHA[:8], status.CurrentRemoteSHA[:8]) + + return status, nil +} + +// UpdateStoredRemoteSHA stores the current remote sync branch SHA in the database. +// Call this after a successful sync to track the remote state. +// +// Parameters: +// - ctx: Context for cancellation +// - store: Storage interface for writing config +// - repoRoot: Path to the git repository root +// - syncBranch: Name of the sync branch (e.g., "beads-sync") +// +// Returns error if the update fails. +func UpdateStoredRemoteSHA(ctx context.Context, store storage.Storage, repoRoot, syncBranch string) error { + // Get worktree path for git operations + worktreePath := getBeadsWorktreePath(ctx, repoRoot, syncBranch) + + // Get remote name + remote := getRemoteForBranch(ctx, worktreePath, syncBranch) + + // Get current remote SHA + remoteRef := fmt.Sprintf("%s/%s", remote, syncBranch) + revParseCmd := exec.CommandContext(ctx, "git", "-C", repoRoot, "rev-parse", remoteRef) + revParseOutput, err := revParseCmd.Output() + if err != nil { + // Remote branch might not exist yet (first push) + // Try local branch instead + revParseCmd = exec.CommandContext(ctx, "git", "-C", repoRoot, "rev-parse", syncBranch) + revParseOutput, err = revParseCmd.Output() + if err != nil { + return fmt.Errorf("failed to get sync branch SHA: %w", err) + } + } + currentSHA := strings.TrimSpace(string(revParseOutput)) + + // Store the SHA + if err := store.SetConfig(ctx, RemoteSHAConfigKey, currentSHA); err != nil { + return fmt.Errorf("failed to store remote SHA: %w", err) + } + + return nil +} + +// ClearStoredRemoteSHA removes the stored remote SHA. +// Use this when resetting the sync state (e.g., after accepting a rebase). +func ClearStoredRemoteSHA(ctx context.Context, store storage.Storage) error { + return store.DeleteConfig(ctx, RemoteSHAConfigKey) +} + +// GetStoredRemoteSHA returns the stored remote sync branch SHA. +func GetStoredRemoteSHA(ctx context.Context, store storage.Storage) (string, error) { + return store.GetConfig(ctx, RemoteSHAConfigKey) +} diff --git a/internal/syncbranch/integrity_test.go b/internal/syncbranch/integrity_test.go new file mode 100644 index 00000000..05c9db97 --- /dev/null +++ b/internal/syncbranch/integrity_test.go @@ -0,0 +1,132 @@ +package syncbranch + +import ( + "context" + "testing" + + "github.com/steveyegge/beads/internal/storage/sqlite" +) + +func TestGetStoredRemoteSHA(t *testing.T) { + ctx := context.Background() + store := newTestStore(t) + defer store.Close() + + // Test getting SHA when not set + sha, err := GetStoredRemoteSHA(ctx, store) + if err != nil { + t.Fatalf("GetStoredRemoteSHA() error = %v", err) + } + if sha != "" { + t.Errorf("GetStoredRemoteSHA() = %q, want empty string", sha) + } + + // Set a SHA + testSHA := "abc123def456" + if err := store.SetConfig(ctx, RemoteSHAConfigKey, testSHA); err != nil { + t.Fatalf("SetConfig() error = %v", err) + } + + // Test getting SHA when set + sha, err = GetStoredRemoteSHA(ctx, store) + if err != nil { + t.Fatalf("GetStoredRemoteSHA() error = %v", err) + } + if sha != testSHA { + t.Errorf("GetStoredRemoteSHA() = %q, want %q", sha, testSHA) + } +} + +func TestClearStoredRemoteSHA(t *testing.T) { + ctx := context.Background() + store := newTestStore(t) + defer store.Close() + + // Set a SHA first + testSHA := "abc123def456" + if err := store.SetConfig(ctx, RemoteSHAConfigKey, testSHA); err != nil { + t.Fatalf("SetConfig() error = %v", err) + } + + // Clear it + if err := ClearStoredRemoteSHA(ctx, store); err != nil { + t.Fatalf("ClearStoredRemoteSHA() error = %v", err) + } + + // Verify it's gone + sha, err := GetStoredRemoteSHA(ctx, store) + if err != nil { + t.Fatalf("GetStoredRemoteSHA() error = %v", err) + } + if sha != "" { + t.Errorf("SHA should be empty after clear, got %q", sha) + } +} + +func TestForcePushStatus(t *testing.T) { + // Test ForcePushStatus struct + status := &ForcePushStatus{ + Detected: true, + StoredSHA: "abc123", + CurrentRemoteSHA: "def456", + Message: "Force push detected", + Branch: "beads-sync", + Remote: "origin", + } + + if !status.Detected { + t.Error("Expected Detected to be true") + } + if status.StoredSHA != "abc123" { + t.Errorf("StoredSHA = %q, want 'abc123'", status.StoredSHA) + } + if status.CurrentRemoteSHA != "def456" { + t.Errorf("CurrentRemoteSHA = %q, want 'def456'", status.CurrentRemoteSHA) + } +} + +// newTestStoreIntegrity creates a test store for integrity tests +// Note: This is a duplicate of newTestStore from syncbranch_test.go +// but we need it here since tests are in the same package +func newTestStoreIntegrity(t *testing.T) *sqlite.SQLiteStorage { + t.Helper() + store, err := sqlite.New(context.Background(), "file::memory:?mode=memory&cache=private") + if err != nil { + t.Fatalf("Failed to create test database: %v", err) + } + ctx := context.Background() + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + _ = store.Close() + t.Fatalf("Failed to set issue_prefix: %v", err) + } + return store +} + +func TestCheckForcePush_NoStoredSHA(t *testing.T) { + ctx := context.Background() + store := newTestStoreIntegrity(t) + defer store.Close() + + // When no stored SHA exists, CheckForcePush should return "first sync" status + // Note: We can't fully test this without a git repo, but we can test the early return + status, err := CheckForcePush(ctx, store, "/nonexistent", "beads-sync") + if err != nil { + t.Fatalf("CheckForcePush() error = %v", err) + } + if status.Detected { + t.Error("Expected Detected to be false when no stored SHA") + } + if status.StoredSHA != "" { + t.Errorf("StoredSHA = %q, want empty", status.StoredSHA) + } + if status.Message != "No previous sync recorded (first sync)" { + t.Errorf("Message = %q, want 'No previous sync recorded (first sync)'", status.Message) + } +} + +func TestRemoteSHAConfigKey(t *testing.T) { + // Verify the config key is what we expect + if RemoteSHAConfigKey != "sync.remote_sha" { + t.Errorf("RemoteSHAConfigKey = %q, want 'sync.remote_sha'", RemoteSHAConfigKey) + } +}