From 3b91515f8fc50a6f75a5417a12d42129145141df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Manso?= Date: Sat, 1 Aug 2026 22:02:03 +0100 Subject: [PATCH] perf(agentctl): collapse the git poll tick into a single spawn (#2133) --- .../server/process/workspace_git_poll.go | 409 +++++++++++------- .../server/process/workspace_overlap_test.go | 104 ++++- 2 files changed, 349 insertions(+), 164 deletions(-) diff --git a/apps/backend/internal/agentctl/server/process/workspace_git_poll.go b/apps/backend/internal/agentctl/server/process/workspace_git_poll.go index 5e7e43e9ad..14dca6fdc4 100644 --- a/apps/backend/internal/agentctl/server/process/workspace_git_poll.go +++ b/apps/backend/internal/agentctl/server/process/workspace_git_poll.go @@ -36,18 +36,21 @@ func (wt *WorkspaceTracker) pollGitChanges(ctx context.Context) { } defer timer.Stop() - // Initialize cached HEAD SHA, branch name, and index hash + // Prime cached HEAD SHA, branch name, and index hash. A read failure here + // is non-fatal: the caches stay empty and the first successful tick fills + // them, which is what the previous per-value getters did on error too. + snap, _ := wt.readGitPollSnapshot(ctx) wt.gitStateMu.Lock() - wt.cachedHeadSHA = wt.getHeadSHA(ctx) - wt.cachedBranchName = wt.getCurrentBranchName(ctx) - wt.cachedIndexHash = wt.getGitStatusHash(ctx) + wt.cachedHeadSHA = snap.headSHA + wt.cachedBranchName = snap.branch + wt.cachedIndexHash = snap.indexHash wt.gitStateMu.Unlock() wt.logger.Info("git polling started", zap.Duration("fast_interval", wt.gitPollInterval), zap.String("initial_mode", string(wt.GetPollMode())), - zap.String("initial_head", wt.cachedHeadSHA), - zap.String("initial_branch", wt.cachedBranchName)) + zap.String("initial_head", snap.headSHA), + zap.String("initial_branch", snap.branch)) var consecutiveFailures int @@ -116,196 +119,292 @@ func (wt *WorkspaceTracker) gitPollTick(ctx context.Context, consecutiveFailures return true } - // Quick git health check before running full change detection. - // If git is broken (e.g., worktree .git reference points to deleted gitdir), - // stop after maxConsecutiveGitFailures to avoid wasting CPU. - probeCmd := exec.CommandContext(ctx, "git", "rev-parse", "--git-dir") - probeCmd.Dir = wt.workDir - if err := subproc.RunGit(ctx, probeCmd); err != nil { - *consecutiveFailures++ - if *consecutiveFailures >= maxConsecutiveGitFailures { - wt.logger.Error("git not functional, stopping git polling", - zap.String("workDir", wt.workDir), - zap.Int("consecutiveFailures", *consecutiveFailures), - zap.Error(err)) - return true - } - return false + snap, err := wt.readGitPollSnapshot(ctx) + if err != nil { + return wt.handleGitPollFailure(ctx, consecutiveFailures, err) } *consecutiveFailures = 0 - wt.checkGitChanges(ctx) + wt.checkGitChanges(ctx, snap) return false } -// getHeadSHA returns the current HEAD commit SHA -func (wt *WorkspaceTracker) getHeadSHA(ctx context.Context) string { - cmd := exec.CommandContext(ctx, "git", "rev-parse", "HEAD") - cmd.Dir = wt.workDir - out, err := subproc.RunGitOutput(ctx, cmd) - if err != nil { - return "" +// handleGitPollFailure decides whether a failed snapshot read means git itself +// is broken (e.g. a worktree .git reference pointing at a deleted gitdir) or +// just a transient error worth retrying. The `rev-parse --git-dir` health probe +// used to run on every tick; it now runs only here, because on the happy path +// its exit code is already implied by the snapshot read succeeding. +// +// Returns true if pollGitChanges should exit. +func (wt *WorkspaceTracker) handleGitPollFailure(ctx context.Context, consecutiveFailures *int, cause error) bool { + probeCmd := exec.CommandContext(ctx, "git", "rev-parse", "--git-dir") + probeCmd.Dir = wt.workDir + if probeErr := subproc.RunGit(ctx, probeCmd); probeErr == nil { + // The repository is fine, so the snapshot read failed for some other + // reason (per-command timeout under load, a concurrent index.lock + // holder). Same as before: only probe failures count against the + // give-up budget. + *consecutiveFailures = 0 + wt.logger.Debug("git poll snapshot failed but repository is healthy", + zap.String("workDir", wt.workDir), + zap.Error(cause)) + return false } - return strings.TrimSpace(string(out)) + + // If git is broken, stop after maxConsecutiveGitFailures to avoid wasting CPU. + *consecutiveFailures++ + if *consecutiveFailures >= maxConsecutiveGitFailures { + wt.logger.Error("git not functional, stopping git polling", + zap.String("workDir", wt.workDir), + zap.Int("consecutiveFailures", *consecutiveFailures), + zap.Error(cause)) + return true + } + return false } -// getCurrentBranchName returns the current branch name. -// Returns empty string if in detached HEAD state (e.g., during rebase or when a commit/tag is checked out). -func (wt *WorkspaceTracker) getCurrentBranchName(ctx context.Context) string { - cmd := exec.CommandContext(ctx, "git", "symbolic-ref", "--short", "-q", "HEAD") - cmd.Dir = wt.workDir - out, err := subproc.RunGitOutput(ctx, cmd) - if err != nil { - return "" - } - return strings.TrimSpace(string(out)) +// gitPollSnapshot is one tick's worth of git state: HEAD, branch, and a hash +// standing in for the index contents. +type gitPollSnapshot struct { + headSHA string + branch string + indexHash string } -// getGitStatusHash returns a hash of the git status porcelain output. -// This is used to detect changes to the git index (staging/unstaging) that don't -// change HEAD. Uses --untracked-files=no because untracked file monitoring is -// already handled by monitorLoop via git ls-files. This avoids the expensive -// directory traversal that --untracked-files=all performs on large repos. -func (wt *WorkspaceTracker) getGitStatusHash(ctx context.Context) string { - cmd := wt.pollingGitCommand(ctx, "status", "--porcelain", "--untracked-files=no") - out, err := subproc.RunGitOutput(ctx, cmd) +// Porcelain v2 header prefixes, and the sentinel values git prints when there +// is no commit yet (branch.oid) or HEAD is not on a branch (branch.head). Both +// sentinels map to the empty string, matching what the rev-parse and +// symbolic-ref probes this read replaces returned in those same states. +const ( + gitStatusHeaderPrefix = "# " + gitStatusBranchOIDPrefix = "# branch.oid " + gitStatusBranchHeadPrefix = "# branch.head " + gitStatusInitialOID = "(initial)" + gitStatusDetachedHead = "(detached)" +) + +// readGitPollSnapshot reads HEAD, branch name and index state in a single git +// invocation. `--porcelain=v2 --branch` emits the `branch.oid` and `branch.head` +// headers alongside the entry lines, which is everything the poll loop +// previously gathered with three separate rev-parse / symbolic-ref / status +// spawns. +// +// Collapsing them matters far more than it looks: on Windows a git spawn pays +// for CreateProcess, the Git-for-Windows shim exec'ing the real binary, and AV +// inspection, all regardless of how little work the command does. Per-tick cost +// therefore tracks the number of spawns, not the work. Measured on a Windows +// dev machine (n=25, medians): 582ms for the four-spawn tick against 176ms for +// this one, with a 146ms floor for a bare `git --version` — i.e. the old tick +// was almost entirely spawn overhead. On macOS and Linux a spawn is 3-5ms and +// none of this is visible, which is why the cost went unnoticed for so long. +// +// Two flags keep --branch from paying for work the poller throws away: +// +// - --no-ahead-behind, because --branch otherwise walks the revision graph to +// count how far the branch is from its upstream, and parseGitPollSnapshot +// discards branch.ab. That walk is not free on a diverged branch: measured +// at 1799 commits behind, the tick took 334ms with the flag against 646ms +// without — worse than the four-spawn version this replaces. git prints +// `# branch.ab +? -?` when it skips the count, which the parser ignores as +// it ignores every other header. +// - --untracked-files=no, preserved from the old status call: untracked files +// are already monitored by monitorLoop via git ls-files, and =all would pay +// for a full directory traversal on every tick. +func (wt *WorkspaceTracker) readGitPollSnapshot(ctx context.Context) (gitPollSnapshot, error) { + out, err := wt.runPollingGitOutput(ctx, "status", "--porcelain=v2", "--branch", "--no-ahead-behind", "--untracked-files=no") if err != nil { - return "" + return gitPollSnapshot{}, err } - hash := sha256.Sum256(out) - return hex.EncodeToString(hash[:]) + return parseGitPollSnapshot(out), nil } -// checkGitChanges checks if HEAD or git index has changed and processes changes -func (wt *WorkspaceTracker) checkGitChanges(ctx context.Context) { - currentHead := wt.getHeadSHA(ctx) - currentBranch := wt.getCurrentBranchName(ctx) - currentIndexHash := wt.getGitStatusHash(ctx) +// parseGitPollSnapshot splits porcelain v2 output into the branch headers and +// the entry lines. Only the entries feed the hash — headers carry branch.oid +// and branch.ab, which move on every commit and every fetch and would make the +// index hash report changes that the HEAD and branch comparisons already cover. +func parseGitPollSnapshot(out []byte) gitPollSnapshot { + var snap gitPollSnapshot + entries := make([]byte, 0, len(out)) + for _, line := range strings.Split(string(out), "\n") { + line = strings.TrimSuffix(line, "\r") + switch { + case strings.HasPrefix(line, gitStatusBranchOIDPrefix): + if oid := strings.TrimPrefix(line, gitStatusBranchOIDPrefix); oid != gitStatusInitialOID { + snap.headSHA = oid + } + case strings.HasPrefix(line, gitStatusBranchHeadPrefix): + if head := strings.TrimPrefix(line, gitStatusBranchHeadPrefix); head != gitStatusDetachedHead { + snap.branch = head + } + case line == "" || strings.HasPrefix(line, gitStatusHeaderPrefix): + // Other headers (branch.upstream, branch.ab) are not poll state. + default: + entries = append(entries, line...) + entries = append(entries, '\n') + } + } + + hash := sha256.Sum256(entries) + snap.indexHash = hex.EncodeToString(hash[:]) + return snap +} + +// gitPollDelta is a snapshot compared against the cached state: what git says +// now, what it said last tick, and which of the three tracked values moved. +type gitPollDelta struct { + snap gitPollSnapshot + previousHead string + previousBranch string + headChanged bool + branchChanged bool + indexChanged bool +} + +// compareToCachedState reads the cached HEAD, branch and index hash once and +// diffs the snapshot against them. +func (wt *WorkspaceTracker) compareToCachedState(snap gitPollSnapshot) gitPollDelta { wt.gitStateMu.RLock() previousHead := wt.cachedHeadSHA previousBranch := wt.cachedBranchName previousIndexHash := wt.cachedIndexHash wt.gitStateMu.RUnlock() - headChanged := currentHead != "" && currentHead != previousHead - branchChanged := currentBranch != previousBranch // Track all transitions including to/from detached HEAD - indexChanged := currentIndexHash != "" && currentIndexHash != previousIndexHash - - // If nothing changed, nothing to do - if !headChanged && !branchChanged && !indexChanged { - return + return gitPollDelta{ + snap: snap, + previousHead: previousHead, + previousBranch: previousBranch, + headChanged: snap.headSHA != "" && snap.headSHA != previousHead, + // Track all transitions including to/from detached HEAD. + branchChanged: snap.branch != previousBranch, + indexChanged: snap.indexHash != "" && snap.indexHash != previousIndexHash, } +} - // Update cached index hash (HEAD will be updated below if it changed) - if indexChanged && !headChanged && !branchChanged { - wt.gitStateMu.Lock() - wt.cachedIndexHash = currentIndexHash - wt.gitStateMu.Unlock() +// checkGitChanges checks if HEAD or git index has changed and processes changes. +// It only routes: each case below owns its own cache update and notifications. +func (wt *WorkspaceTracker) checkGitChanges(ctx context.Context, snap gitPollSnapshot) { + delta := wt.compareToCachedState(snap) - wt.logger.Debug("git index changed (staging/unstaging detected)") - wt.tryUpdateGitStatus(ctx) - return + switch { + case !delta.headChanged && !delta.branchChanged && !delta.indexChanged: + // Nothing moved. + case delta.indexChanged && !delta.headChanged && !delta.branchChanged: + wt.handleIndexOnlyChange(ctx, delta) + case delta.branchChanged && !delta.headChanged: + wt.handleBranchChangeWithoutHead(ctx, delta) + case delta.headChanged: + wt.handleHeadChange(ctx, delta) } +} - // Branch changed without HEAD change - this can happen when: - // 1. Switching between branches that point to the same commit - // 2. Transitioning to/from detached HEAD state (git switch --detach, rebase end) - if branchChanged && !headChanged { - wt.gitStateMu.Lock() - wt.cachedBranchName = currentBranch - wt.cachedIndexHash = currentIndexHash - wt.gitStateMu.Unlock() - - // Only handle branch switch if we're not in detached HEAD state - // Suppress notification when currentBranch is empty (detached HEAD) - if currentBranch != "" { - wt.handleBranchSwitch(ctx, previousBranch, currentBranch, currentHead) - return - } - - // In detached HEAD state, just update git status - wt.tryUpdateGitStatus(ctx) - return - } - - // HEAD changed - handle normally - if !headChanged { - return - } - - wt.logger.Info("git HEAD changed, syncing", - zap.String("previous", previousHead), - zap.String("current", currentHead)) - - // Update cached HEAD, branch, and index hash +// handleIndexOnlyChange records staging/unstaging that left HEAD and the branch +// alone. +func (wt *WorkspaceTracker) handleIndexOnlyChange(ctx context.Context, delta gitPollDelta) { wt.gitStateMu.Lock() - wt.cachedHeadSHA = currentHead - wt.cachedBranchName = currentBranch - wt.cachedIndexHash = currentIndexHash + wt.cachedIndexHash = delta.snap.indexHash wt.gitStateMu.Unlock() - // Check if this is a branch switch (branch name changed) - // Branch switches need special handling to update the base commit - // Only handle if we're not in detached HEAD state (currentBranch != "") - if branchChanged && currentBranch != "" { - wt.handleBranchSwitch(ctx, previousBranch, currentBranch, currentHead) + wt.logger.Debug("git index changed (staging/unstaging detected)") + wt.tryUpdateGitStatus(ctx) +} + +// handleBranchChangeWithoutHead handles a branch move that left HEAD where it +// was. Two ways that happens: switching between branches pointing at the same +// commit, and entering or leaving detached HEAD (git switch --detach, end of a +// rebase). +func (wt *WorkspaceTracker) handleBranchChangeWithoutHead(ctx context.Context, delta gitPollDelta) { + wt.gitStateMu.Lock() + wt.cachedBranchName = delta.snap.branch + wt.cachedIndexHash = delta.snap.indexHash + wt.gitStateMu.Unlock() + + // In detached HEAD state there is no branch to report a switch to, so just + // refresh status. + if delta.snap.branch == "" { + wt.tryUpdateGitStatus(ctx) return } - // Check if history was rewritten (reset, rebase, amend, etc.) - // There are three cases: - // 1. HEAD moved backward: currentHead is an ancestor of previousHead (e.g., git reset HEAD~1) - // 2. History rewritten: previousHead is NOT reachable from currentHead (e.g., git rebase -i, git commit --amend) - // 3. HEAD moved forward: previousHead IS an ancestor of currentHead (normal commits) - if previousHead != "" { - switch { - case wt.isAncestor(ctx, currentHead, previousHead): - // Case 1: HEAD moved backward - emit reset notification - wt.logger.Info("detected git reset (HEAD moved backward)", - zap.String("previous", previousHead), - zap.String("current", currentHead)) - wt.notifyWorkspaceStreamGitReset(&types.GitResetNotification{ - Timestamp: time.Now(), - RepositoryName: wt.repositoryName, - PreviousHead: previousHead, - CurrentHead: currentHead, - }) - case !wt.isAncestor(ctx, previousHead, currentHead): - // Case 2: History was rewritten - previousHead is not reachable from currentHead - // This happens with interactive rebase, commit amend, etc. - wt.logger.Info("detected git history rewrite (previous HEAD not reachable)", - zap.String("previous", previousHead), - zap.String("current", currentHead)) - wt.notifyWorkspaceStreamGitReset(&types.GitResetNotification{ - Timestamp: time.Now(), - RepositoryName: wt.repositoryName, - PreviousHead: previousHead, - CurrentHead: currentHead, - }) - default: - // Case 3: HEAD moved forward normally - get new commits - commits := wt.getCommitsSince(ctx, previousHead) + wt.handleBranchSwitch(ctx, delta.previousBranch, delta.snap.branch, delta.snap.headSHA) +} - // Filter out commits that are already on remote branches. - // This prevents recording upstream commits as session commits when - // the user pulls/rebases onto a remote branch (e.g., git reset --hard main). - localCommits := wt.filterLocalCommits(ctx, commits) +// handleHeadChange handles a HEAD move, which is either a branch switch or a +// change in history that needs classifying. +func (wt *WorkspaceTracker) handleHeadChange(ctx context.Context, delta gitPollDelta) { + wt.logger.Info("git HEAD changed, syncing", + zap.String("previous", delta.previousHead), + zap.String("current", delta.snap.headSHA)) - for _, commit := range localCommits { - wt.notifyWorkspaceStreamGitCommit(commit) - } - if len(localCommits) > 0 { - wt.logger.Info("detected new commits via polling", - zap.Int("count", len(localCommits))) - } - } + wt.gitStateMu.Lock() + wt.cachedHeadSHA = delta.snap.headSHA + wt.cachedBranchName = delta.snap.branch + wt.cachedIndexHash = delta.snap.indexHash + wt.gitStateMu.Unlock() + + // Branch switches need special handling to update the base commit. Skipped + // in detached HEAD state, where there is no branch name. + if delta.branchChanged && delta.snap.branch != "" { + wt.handleBranchSwitch(ctx, delta.previousBranch, delta.snap.branch, delta.snap.headSHA) + return + } + + if delta.previousHead != "" { + wt.classifyHeadMovement(ctx, delta) } // Update and broadcast git status wt.tryUpdateGitStatus(ctx) } +// classifyHeadMovement works out what a HEAD move meant and emits the matching +// notification. There are three cases: +// 1. HEAD moved backward: the new HEAD is an ancestor of the old one (git reset HEAD~1) +// 2. History rewritten: the old HEAD is NOT reachable from the new one (git rebase -i, git commit --amend) +// 3. HEAD moved forward: the old HEAD IS an ancestor of the new one (normal commits) +func (wt *WorkspaceTracker) classifyHeadMovement(ctx context.Context, delta gitPollDelta) { + switch { + case wt.isAncestor(ctx, delta.snap.headSHA, delta.previousHead): + wt.logger.Info("detected git reset (HEAD moved backward)", + zap.String("previous", delta.previousHead), + zap.String("current", delta.snap.headSHA)) + wt.notifyWorkspaceStreamGitReset(&types.GitResetNotification{ + Timestamp: time.Now(), + RepositoryName: wt.repositoryName, + PreviousHead: delta.previousHead, + CurrentHead: delta.snap.headSHA, + }) + case !wt.isAncestor(ctx, delta.previousHead, delta.snap.headSHA): + wt.logger.Info("detected git history rewrite (previous HEAD not reachable)", + zap.String("previous", delta.previousHead), + zap.String("current", delta.snap.headSHA)) + wt.notifyWorkspaceStreamGitReset(&types.GitResetNotification{ + Timestamp: time.Now(), + RepositoryName: wt.repositoryName, + PreviousHead: delta.previousHead, + CurrentHead: delta.snap.headSHA, + }) + default: + wt.reportNewCommits(ctx, delta.previousHead) + } +} + +// reportNewCommits notifies about commits made since previousHead, minus the +// ones already on a remote branch — pulling or rebasing onto a remote branch +// would otherwise record upstream commits as session commits. +func (wt *WorkspaceTracker) reportNewCommits(ctx context.Context, previousHead string) { + commits := wt.getCommitsSince(ctx, previousHead) + localCommits := wt.filterLocalCommits(ctx, commits) + + for _, commit := range localCommits { + wt.notifyWorkspaceStreamGitCommit(commit) + } + if len(localCommits) > 0 { + wt.logger.Info("detected new commits via polling", + zap.Int("count", len(localCommits))) + } +} + // handleBranchSwitch handles a branch switch event by calculating the new base commit // and notifying subscribers. This allows the session to update its base commit to reflect // the new branch's merge-base with the target branch (e.g., main). diff --git a/apps/backend/internal/agentctl/server/process/workspace_overlap_test.go b/apps/backend/internal/agentctl/server/process/workspace_overlap_test.go index 44d81561c7..229f131e0d 100644 --- a/apps/backend/internal/agentctl/server/process/workspace_overlap_test.go +++ b/apps/backend/internal/agentctl/server/process/workspace_overlap_test.go @@ -228,10 +228,21 @@ func TestUpdateMu_PreventsConcurrentUpdates(t *testing.T) { t.Logf("TryLock acquired by %d/%d goroutines (others correctly skipped)", total, goroutines) } -// TestGetGitStatusHash_ExcludesUntrackedFiles verifies that getGitStatusHash uses +// snapshotIndexHash reads a poll snapshot and returns its index hash, failing +// the test if the underlying git read errored. +func snapshotIndexHash(t *testing.T, wt *WorkspaceTracker, ctx context.Context) string { + t.Helper() + snap, err := wt.readGitPollSnapshot(ctx) + if err != nil { + t.Fatalf("readGitPollSnapshot failed: %v", err) + } + return snap.indexHash +} + +// TestGitPollSnapshot_ExcludesUntrackedFiles verifies that the poll snapshot uses // --untracked-files=no, so untracked files don't appear in the hash. This prevents // the expensive directory traversal that --untracked-files=all performs. -func TestGetGitStatusHash_ExcludesUntrackedFiles(t *testing.T) { +func TestGitPollSnapshot_ExcludesUntrackedFiles(t *testing.T) { isolateTestGitEnv(t) repoDir, cleanup := setupTestRepo(t) @@ -242,29 +253,29 @@ func TestGetGitStatusHash_ExcludesUntrackedFiles(t *testing.T) { ctx := context.Background() // Get baseline hash with no changes - baseHash := wt.getGitStatusHash(ctx) + baseHash := snapshotIndexHash(t, wt, ctx) if baseHash == "" { t.Fatal("expected non-empty hash for clean repo") } // Create an untracked file — hash should NOT change (--untracked-files=no) writeFile(t, repoDir, "untracked.txt", "untracked content") - untrackedHash := wt.getGitStatusHash(ctx) + untrackedHash := snapshotIndexHash(t, wt, ctx) if untrackedHash != baseHash { t.Errorf("hash changed after adding untracked file; expected --untracked-files=no to exclude it\n before: %s\n after: %s", baseHash, untrackedHash) } // Stage the file — hash SHOULD change (staged files show as A, not ??) runGit(t, repoDir, "add", "untracked.txt") - stagedHash := wt.getGitStatusHash(ctx) + stagedHash := snapshotIndexHash(t, wt, ctx) if stagedHash == baseHash { t.Error("hash did not change after staging a file; expected staging to be detected") } } -// TestGetGitStatusHash_DetectsModifiedFiles verifies that getGitStatusHash still +// TestGitPollSnapshot_DetectsModifiedFiles verifies that the poll snapshot still // detects tracked file modifications even with --untracked-files=no. -func TestGetGitStatusHash_DetectsModifiedFiles(t *testing.T) { +func TestGitPollSnapshot_DetectsModifiedFiles(t *testing.T) { isolateTestGitEnv(t) repoDir, cleanup := setupTestRepo(t) @@ -274,16 +285,91 @@ func TestGetGitStatusHash_DetectsModifiedFiles(t *testing.T) { wt := NewWorkspaceTracker(repoDir, log) ctx := context.Background() - baseHash := wt.getGitStatusHash(ctx) + baseHash := snapshotIndexHash(t, wt, ctx) // Modify a tracked file — hash should change writeFile(t, repoDir, "README.md", "# Modified") - modifiedHash := wt.getGitStatusHash(ctx) + modifiedHash := snapshotIndexHash(t, wt, ctx) if modifiedHash == baseHash { t.Error("hash did not change after modifying tracked file") } } +// TestGitPollSnapshot_ReadsHeadAndBranch verifies the two values that used to +// come from separate rev-parse / symbolic-ref spawns are now parsed out of the +// porcelain v2 branch headers, including the detached-HEAD sentinel. +func TestGitPollSnapshot_ReadsHeadAndBranch(t *testing.T) { + isolateTestGitEnv(t) + + repoDir, cleanup := setupTestRepo(t) + defer cleanup() + + log := newTestLogger(t) + wt := NewWorkspaceTracker(repoDir, log) + ctx := context.Background() + + snap, err := wt.readGitPollSnapshot(ctx) + if err != nil { + t.Fatalf("readGitPollSnapshot failed: %v", err) + } + if len(snap.headSHA) != 40 { + t.Errorf("expected a 40-char HEAD SHA, got %q", snap.headSHA) + } + if snap.branch == "" { + t.Error("expected a branch name on a freshly initialized repo") + } + + // Detaching HEAD must clear the branch, not report "(detached)". + runGit(t, repoDir, "checkout", "--detach") + detached, err := wt.readGitPollSnapshot(ctx) + if err != nil { + t.Fatalf("readGitPollSnapshot failed after detach: %v", err) + } + if detached.branch != "" { + t.Errorf("expected empty branch in detached HEAD state, got %q", detached.branch) + } + if detached.headSHA != snap.headSHA { + t.Errorf("HEAD changed across detach: %q -> %q", snap.headSHA, detached.headSHA) + } +} + +// TestParseGitPollSnapshot_Sentinels covers the header parsing without needing a +// repository in each of the states git can report. +func TestParseGitPollSnapshot_Sentinels(t *testing.T) { + unborn := parseGitPollSnapshot([]byte("# branch.oid (initial)\n# branch.head main\n")) + if unborn.headSHA != "" { + t.Errorf("expected empty HEAD for an unborn branch, got %q", unborn.headSHA) + } + if unborn.branch != "main" { + t.Errorf("expected branch %q, got %q", "main", unborn.branch) + } + + // branch.ab moves on fetch without the index changing; it must not leak + // into the hash. + withAB := parseGitPollSnapshot([]byte("# branch.oid abc\n# branch.head main\n# branch.ab +0 -0\n")) + withoutAB := parseGitPollSnapshot([]byte("# branch.oid abc\n# branch.head main\n# branch.ab +3 -1\n")) + if withAB.indexHash != withoutAB.indexHash { + t.Error("index hash changed with branch.ab; headers must be excluded from the hash") + } + + // --no-ahead-behind makes git print "+? -?" instead of real counts. It must + // parse like any other header and leave the hash alone, so that dropping the + // flag (and paying for the revision walk again) stays a deliberate act. + unknownAB := parseGitPollSnapshot([]byte("# branch.oid abc\n# branch.head main\n# branch.ab +? -?\n")) + if unknownAB.indexHash != withAB.indexHash { + t.Error("index hash changed with an unknown branch.ab; the header must be ignored") + } + if unknownAB.headSHA != "abc" || unknownAB.branch != "main" { + t.Errorf("unknown branch.ab disturbed the other headers: head=%q branch=%q", unknownAB.headSHA, unknownAB.branch) + } + + // Entry lines must drive the hash. + entry := parseGitPollSnapshot([]byte("# branch.oid abc\n# branch.head main\n1 .M N... 100644 100644 100644 aaa bbb file.txt\n")) + if entry.indexHash == withAB.indexHash { + t.Error("index hash did not change when an entry line was present") + } +} + // TestOverlapGuard_ResetsAfterNormalCycle verifies that the atomic overlap guard // is properly reset after a normal poll cycle completes, allowing the next tick to run. func TestOverlapGuard_ResetsAfterNormalCycle(t *testing.T) {