From 12858f51468f06540db37595cd9404a17f5f071a Mon Sep 17 00:00:00 2001 From: Peter Chanthamynavong Date: Tue, 6 Jan 2026 19:22:51 -0800 Subject: [PATCH] fix(doctor): recognize lowercase 's' skip-worktree flag (#931) * fix(doctor): handle 's' status in combined git flags Problem: - Git status detection failed when 's' was combined with other flags - Branch synchronization checks produced incorrect results due to missing flag parsing Solution: - Update detection logic to correctly identify the 's' status within combined flag strings Impact: - Ensures branch synchronization state is accurately reported during doctor checks * test(doctor): add unit tests for git flag parsing - Extract git flag parsing logic into parseGitLsFilesFlag helper - Add unit tests for git flag parsing logic Coverage: Git flag parsing in sync_branch.go --- cmd/bd/doctor/fix/sync_branch.go | 26 +++++++++++---- cmd/bd/doctor/fix/sync_branch_test.go | 46 +++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 7 deletions(-) create mode 100644 cmd/bd/doctor/fix/sync_branch_test.go diff --git a/cmd/bd/doctor/fix/sync_branch.go b/cmd/bd/doctor/fix/sync_branch.go index ec9269bd..4ba8d391 100644 --- a/cmd/bd/doctor/fix/sync_branch.go +++ b/cmd/bd/doctor/fix/sync_branch.go @@ -272,6 +272,20 @@ func ClearSyncBranchGitignore(path string) error { return nil } +// parseGitLsFilesFlag interprets the flag character from git ls-files -v output. +// Returns (hasAnyFlag, hasSkipWorktree) based on the first character of the line. +// +// Git ls-files -v output flags: +// 'H' = tracked normally (no flags) +// 'h' = assume-unchanged only +// 'S' = skip-worktree only +// 's' = both skip-worktree + assume-unchanged (lowercase due to assume-unchanged) +func parseGitLsFilesFlag(flag byte) (hasAnyFlag bool, hasSkipWorktree bool) { + hasAnyFlag = flag == 'h' || flag == 'S' || flag == 's' + hasSkipWorktree = flag == 'S' || flag == 's' + return hasAnyFlag, hasSkipWorktree +} + // HasSyncBranchGitignoreFlags checks if git index flags are set on .beads/issues.jsonl. // Returns (hasAnyFlag, hasSkipWorktree, error). // Note: When both assume-unchanged and skip-worktree are set, git shows 'S' (skip-worktree @@ -285,8 +299,10 @@ func HasSyncBranchGitignoreFlags(path string) (bool, bool, error) { } // Get file status from git ls-files -v - // 'H' = tracked normally, 'h' = assume-unchanged, 'S' = skip-worktree - // When both flags are set, 'S' is shown (skip-worktree takes precedence) + // 'H' = tracked normally + // 'h' = assume-unchanged only + // 'S' = skip-worktree only + // 's' = skip-worktree + assume-unchanged (lowercase due to assume-unchanged) cmd := exec.Command("git", "ls-files", "-v", jsonlPath) cmd.Dir = path output, err := cmd.Output() @@ -299,11 +315,7 @@ func HasSyncBranchGitignoreFlags(path string) (bool, bool, error) { return false, false, nil } - firstChar := line[0] - // 'h' = assume-unchanged only, 'S' = skip-worktree (possibly with assume-unchanged too) - hasAnyFlag := firstChar == 'h' || firstChar == 'S' - hasSkipWorktree := firstChar == 'S' - + hasAnyFlag, hasSkipWorktree := parseGitLsFilesFlag(line[0]) return hasAnyFlag, hasSkipWorktree, nil } diff --git a/cmd/bd/doctor/fix/sync_branch_test.go b/cmd/bd/doctor/fix/sync_branch_test.go new file mode 100644 index 00000000..a1b513c7 --- /dev/null +++ b/cmd/bd/doctor/fix/sync_branch_test.go @@ -0,0 +1,46 @@ +package fix + +import "testing" + +func TestParseGitLsFilesFlag(t *testing.T) { + tests := map[string]struct { + flag byte + wantHasAnyFlag bool + wantHasSkipWorktree bool + }{ + "normal tracked file (H)": { + flag: 'H', + wantHasAnyFlag: false, + wantHasSkipWorktree: false, + }, + "assume-unchanged only (h)": { + flag: 'h', + wantHasAnyFlag: true, + wantHasSkipWorktree: false, + }, + "skip-worktree only (S)": { + flag: 'S', + wantHasAnyFlag: true, + wantHasSkipWorktree: true, + }, + "both flags set (s)": { + flag: 's', + wantHasAnyFlag: true, + wantHasSkipWorktree: true, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + gotHasAnyFlag, gotHasSkipWorktree := parseGitLsFilesFlag(tt.flag) + + if gotHasAnyFlag != tt.wantHasAnyFlag { + t.Errorf("hasAnyFlag = %v, want %v", gotHasAnyFlag, tt.wantHasAnyFlag) + } + + if gotHasSkipWorktree != tt.wantHasSkipWorktree { + t.Errorf("hasSkipWorktree = %v, want %v", gotHasSkipWorktree, tt.wantHasSkipWorktree) + } + }) + } +}