[Smilewithoutfalling]

Summary

session-sync-pull cleaned its temp tree with a bare fs.rmSync(tmp, { recursive: true, force: true }). That call failed in a CI run, and every other git-tree cleanup in this repository already guards against exactly this failure.

FAIL src/__tests__/session-sync-pull.test.ts > SyncManager.gitPull
     > finishes a merge an earlier pull left stopped on the index
Error: ENOTEMPTY: directory not empty,
  rmdir '/tmp/teamai-test-tmp-F9XRIJ/teamai-session-pull-JQsoY4/b/.git/objects/pack'

The test body passed in 153 ms; the error came from the afterEach hook, which vitest reports under the test's name — so it reads like a failed assertion for a merge that actually worked.

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (or a fix that changes existing behavior)
• Documentation only
• Refactor / internal cleanup

Tests-only, per AGENTS.md: no real-CLI record needed.

Test Plan

The diff is one fs.rmSync call and the comment above it, in src/__tests__/session-sync-pull.test.ts:

-  fs.rmSync(tmp, { recursive: true, force: true });
+  // A commit or push can leave a detached `git gc --auto` writing objects/pack
+  // after the test returns, so a single rmdir races it (ENOTEMPTY).
+  fs.rmSync(tmp, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });

• node --check --experimental-strip-types src/__tests__/session-sync-pull.test.ts — parses clean
• Byte-level check that nothing else moved: 21378 → 21559 bytes, CR=0, LF 510 → 512, trailing newline kept
• npx vitest run — full suite left to CI (this file is included in all six Lint & Test jobs)

Notes for Reviewers

This is not a new fix, it is the missing instance of an existing one. git-kind-learnings.test.ts already states the mechanism in a comment:

A push into the bare origin can leave a detached git gc --auto writing to objects/pack after the test returns, so a single rmdir races it (ENOTEMPTY).

It passes maxRetries: 5, retryDelay: 100. Three unit-test files already do the same on their temp tree — git-kind-learnings, learnings-import-remnants, reports-reader-lock — and three of the four e2e files do too (deleted-worktree-scope-810, instruction-targets, slash-skill-paths). This file still removed a tree holding a clone and a bare origin on a single attempt.

Why I believe it is a timing race and not a broken assertion. The same commit produced six Lint & Test jobs and five passed: Node 20/22/24 on macOS, Node 22/24 on ubuntu. Two of those are ubuntu and run the same file on a newer Node. A logic error would not be that selective.

What I am not claiming. I did not identify which detached git process was writing in that particular run — the failing path is b/.git/objects/pack while the comment above names a push into the bare origin, and this fixture both commits into b (archive()) and pushes from it. So this PR does not assert a new root cause; it applies the retry this repository already relies on to the one cleanup that lacked it, and cites the observed failure as the reason.

Scope. I deliberately left helpers/isolate-home.ts alone. Its afterAll also calls a bare fs.rmSync, but it removes the outer test directory after every per-test afterEach has already emptied it, and the failure above is attributed to the per-test hook. Changing a setup file shared by 555 test files is a different decision from this one.

[github-actions]

No findings in the specified diff.

• The change is limited to test cleanup, adding bounded retries to the existing recursive removal.
• The PR description documents a syntax check and explicitly defers Vitest to CI. This tests-only change requires no real-CLI/e2e record under the trusted review rules.

Reviewed only the diff; did not run, build, or install PR code.

[SaulMoro]

This was generated by AI during triage.

@Smilewithoutfalling Approving at 9a807ca. The failure this targets is real and matches: #1403's Node 20 ubuntu job failed with ENOTEMPTY: directory not empty, rmdir '.../b/.git/objects/pack', thrown from this file's afterEach, while the test body itself had passed. Bounded retries are what the other git-tree cleanups in the suite already use (maxRetries: 5, retryDelay: 100, e.g. deleted-worktree-scope-810, instruction-targets, slash-skill-paths). Tests-only, so no real-CLI record is needed.

The CI review reports no findings. We agree. All CI jobs pass; the GitHub-provider e2e job is skipped by the fork policy.