From fa3ca6206e40cab98469634bd046a01f42bd5646 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 5 Apr 2026 01:05:03 -0400 Subject: [PATCH] fix(git): use git add -u in symlink .gsd fallback to prevent hang (#3299) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(git): use git add -u in symlink .gsd fallback to prevent hang on large repos (#1977) When .gsd is a symlink, nativeAddAllWithExclusions falls back from pathspec exclusions (which git rejects with "beyond a symbolic link"). The previous fallback used nativeAddAll (git add -A), which traverses the entire working tree — hanging indefinitely on repos with large untracked data directories (444GB+ observed). Change the fallback to git add -u, which only stages changes to already-tracked files. This is O(tracked) instead of O(filesystem), making it safe for repos with massive untracked trees. The tradeoff — new untracked files are not staged in the symlink fallback path — is acceptable because auto-commit primarily captures changes the agent made to existing tracked files. Regression of #1712 which introduced the symlink fallback but chose git add -A. That fix assumed .gitignore covered all large directories, but scientific/ML repos often have large untracked data dirs not in .gitignore. Fixes #1977 Co-Authored-By: Claude Opus 4.6 (1M context) * chore: retrigger CI * fix(test): update symlink autoCommit test to use tracked file modification The PR changed the symlink fallback from git add -A to git add -u. git add -u only stages changes to tracked files; untracked files are skipped. The existing test created a new untracked file which git add -u ignores, causing autoCommit to return null (nothing to commit). Pre-commit the source file before the scenario and modify it so git add -u can stage the tracked change. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Opus 4.6 (1M context) Co-authored-by: trek-e --- .../extensions/gsd/native-git-bridge.ts | 8 +++-- .../gsd/tests/integration/git-service.test.ts | 34 +++++++++++++++---- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/resources/extensions/gsd/native-git-bridge.ts b/src/resources/extensions/gsd/native-git-bridge.ts index 76c140251..ad126332a 100644 --- a/src/resources/extensions/gsd/native-git-bridge.ts +++ b/src/resources/extensions/gsd/native-git-bridge.ts @@ -724,10 +724,12 @@ export function nativeAddAllWithExclusions(basePath: string, exclusions: readonl return; } // When .gsd is a symlink, git rejects `:!.gsd/...` pathspecs with - // "beyond a symbolic link". Fall back to plain `git add -A` which - // respects .gitignore (where .gsd/ is listed by default). + // "beyond a symbolic link". Fall back to `git add -u` which only + // stages changes to already-tracked files — O(tracked) not O(filesystem). + // Using `git add -A` here would traverse the entire working tree, + // hanging indefinitely on repos with large untracked data dirs. (#1977) if (stderr.includes("beyond a symbolic link")) { - nativeAddAll(basePath); + gitFileExec(basePath, ["add", "-u"]); return; } throw new GSDError(GSD_GIT_ERROR, `git add -A with exclusions failed in ${basePath}: ${getErrorMessage(err)}`); diff --git a/src/resources/extensions/gsd/tests/integration/git-service.test.ts b/src/resources/extensions/gsd/tests/integration/git-service.test.ts index 2c7d23fc8..f88901431 100644 --- a/src/resources/extensions/gsd/tests/integration/git-service.test.ts +++ b/src/resources/extensions/gsd/tests/integration/git-service.test.ts @@ -1246,7 +1246,7 @@ describe('git-service', async () => { test('nativeAddAllWithExclusions: symlinked .gsd fallback', () => { // When .gsd is a symlink, git rejects `:!.gsd/...` pathspecs with // "fatal: pathspec '...' is beyond a symbolic link". The fix falls - // back to plain `git add -A`, which respects .gitignore. + // back to `git add -u` (tracked files only), NOT `git add -A`. const repo = initTempRepo(); // Create the real .gsd directory outside the repo, then symlink it @@ -1258,11 +1258,18 @@ describe('git-service', async () => { // Symlink .gsd -> external directory symlinkSync(externalGsd, join(repo, ".gsd")); - // Add .gitignore so git add -A fallback skips .gsd/ + // Add .gitignore so .gsd/ is ignored writeFileSync(join(repo, ".gitignore"), ".gsd\n"); - // Create a real file that should be staged + // Create a tracked file and commit it, then modify it createFile(repo, "src/app.ts", "export const x = 1;"); + run("git add -A", repo); + run('git commit -m "add app"', repo); + writeFileSync(join(repo, "src/app.ts"), "export const x = 2;"); + + // Create an untracked file simulating large data (NOT in .gitignore) + // This is the key scenario: large untracked dirs that git add -A would traverse + createFile(repo, "data/large-model.bin", "pretend this is 10GB"); // nativeAddAllWithExclusions should NOT throw despite .gsd being a symlink let threw = false; @@ -1274,9 +1281,15 @@ describe('git-service', async () => { } assert.ok(!threw, "nativeAddAllWithExclusions does not throw with symlinked .gsd"); - // Verify the real file was staged + // Verify the tracked modified file was staged const staged = run("git diff --cached --name-only", repo); - assert.ok(staged.includes("src/app.ts"), "real file staged despite symlinked .gsd"); + assert.ok(staged.includes("src/app.ts"), "modified tracked file staged despite symlinked .gsd"); + + // CRITICAL: untracked files must NOT be staged — the symlink fallback + // should use `git add -u` (tracked only), not `git add -A` (all files). + // Using `git add -A` on a repo with large untracked data dirs hangs. (#1977) + assert.ok(!staged.includes("data/large-model.bin"), + "symlink fallback must not stage untracked files (would hang on large repos)"); assert.ok(!staged.includes(".gsd"), ".gsd content not staged"); rmSync(repo, { recursive: true, force: true }); @@ -1435,13 +1448,20 @@ describe('git-service', async () => { run('git add .gitignore', repo); run('git commit -m "add gitignore"', repo); + // Pre-commit a tracked source file so git add -u can stage modifications. + // The symlink fallback uses git add -u (tracked files only), so the file + // must be tracked before the autoCommit scenario runs. + createFile(repo, "src/feature.ts", "export const feature = true;"); + run('git add src/feature.ts', repo); + run('git commit -m "add feature"', repo); + // Simulate new milestone artifacts created during execution writeFileSync(join(externalGsd, "milestones", "M009", "M009-SUMMARY.md"), "# M009 Summary"); writeFileSync(join(externalGsd, "milestones", "M009", "S01-SUMMARY.md"), "# S01 Summary"); writeFileSync(join(externalGsd, "milestones", "M009", "T01-VERIFY.json"), '{"passed":true}'); - // Also create a normal source file change - createFile(repo, "src/feature.ts", "export const feature = true;"); + // Modify the tracked source file — git add -u will stage this change + writeFileSync(join(repo, "src/feature.ts"), "export const feature = false; // updated"); const svc = new GitServiceImpl(repo); const msg = svc.autoCommit("complete-milestone", "M009");