Summary
agent/tests/test_post_hooks.py (TestReconcileAgentBranch._make_repo, lines 287–288) seeds its throwaway git repo with a minimal identity:
self._git(repo, "config", "user.email", "t@t")
self._git(repo, "config", "user.name", "t")
The test itself is correctly isolated — _git pins cwd=repo (a tmp_path dir), so under pytest these writes land in a temp repo that is deleted on teardown. Running the suite never touches a real config.
The hazard is out-of-band transcription: when a human or a review agent reproduces the fixture by running the bare commands (git config user.email t@t) in a real checkout instead of via the test runner, the unscoped git config defaults to --local and writes to $GIT_DIR/config. The values t / t@t then land verbatim in the real repo's .git/config.
Observed impact
A /review_pr session corrupted a maintainer's repo-local identity to exactly t / t@t. Notable specifics that make this worse than it looks:
- A linked worktree is NOT an isolation boundary for
git config. All worktrees share the common $GIT_DIR/config; an unscoped git config inside a worktree writes through to the config the primary checkout uses. (Verified experimentally.)
- A global identity is not a backstop.
--local always shadows --global, so a global fallback does not prevent mis-attributed commits once the local value is clobbered.
- The short, non-obvious values (
t, t@t) give no signal that they are disposable, so a transcribed write is easy to make and easy to miss in review.
This is the same failure class as #622 (there: git config --global clobbering ~/.gitconfig; here: the --local variant). agent/src/pipeline.py:1037-1047 already avoids it in the runtime by setting identity via GIT_AUTHOR_*/GIT_COMMITTER_* env vars rather than on-disk config.
Proposed change
- Replace the fixture identity with clearly-reserved, self-documenting values so any stray transcription is obvious and harmless to trace:
self._git(repo, "config", "user.email", "fixture@test.invalid") # RFC 2606 reserved TLD
self._git(repo, "config", "user.name", "ABCA Test Fixture")
.invalid / .test are RFC 2606 reserved and can never be a real address.)
- Audit other test fixtures for the same short/ambiguous identity pattern and align them.
- Add a one-line note near the fixture (and/or in
agent/AGENTS.md test guidance) that git identity in fixtures must use reserved values because agents/humans sometimes transcribe fixture commands out of the diff.
Out of scope (tracked separately if desired)
The stronger, behavioral guard — instructing review agents to reproduce tests via the test runner and to run reviews in a throwaway clone or with GIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEM sandboxed, never a linked worktree — is process/skill guidance (.abca/commands/review_pr.md) rather than a code change. Note here for traceability; split into its own issue if preferred.
Acceptance criteria
Summary
agent/tests/test_post_hooks.py(TestReconcileAgentBranch._make_repo, lines 287–288) seeds its throwaway git repo with a minimal identity:The test itself is correctly isolated —
_gitpinscwd=repo(atmp_pathdir), so underpytestthese writes land in a temp repo that is deleted on teardown. Running the suite never touches a real config.The hazard is out-of-band transcription: when a human or a review agent reproduces the fixture by running the bare commands (
git config user.email t@t) in a real checkout instead of via the test runner, the unscopedgit configdefaults to--localand writes to$GIT_DIR/config. The valuest/t@tthen land verbatim in the real repo's.git/config.Observed impact
A
/review_prsession corrupted a maintainer's repo-local identity to exactlyt/t@t. Notable specifics that make this worse than it looks:git config. All worktrees share the common$GIT_DIR/config; an unscopedgit configinside a worktree writes through to the config the primary checkout uses. (Verified experimentally.)--localalways shadows--global, so a global fallback does not prevent mis-attributed commits once the local value is clobbered.t,t@t) give no signal that they are disposable, so a transcribed write is easy to make and easy to miss in review.This is the same failure class as #622 (there:
git config --globalclobbering~/.gitconfig; here: the--localvariant).agent/src/pipeline.py:1037-1047already avoids it in the runtime by setting identity viaGIT_AUTHOR_*/GIT_COMMITTER_*env vars rather than on-disk config.Proposed change
.invalid/.testare RFC 2606 reserved and can never be a real address.)agent/AGENTS.mdtest guidance) that git identity in fixtures must use reserved values because agents/humans sometimes transcribe fixture commands out of the diff.Out of scope (tracked separately if desired)
The stronger, behavioral guard — instructing review agents to reproduce tests via the test runner and to run reviews in a throwaway clone or with
GIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEMsandboxed, never a linked worktree — is process/skill guidance (.abca/commands/review_pr.md) rather than a code change. Note here for traceability; split into its own issue if preferred.Acceptance criteria
test_post_hooks.pyfixtures use RFC-2606 reserved identity values.t@t, baret, etc.).mise //agent:test(oruv run pytest agent/tests/test_post_hooks.py) passes.