test(agent): git fixtures leak into the shared .git/config under an inherited GIT_DIR — make the isolation structural (4th recurrence of #622/#720)

Summary

Python test fixtures in agent/tests/ that shell out to git write into the
repository's shared .git/config whenever the test process inherits a GIT_DIR
from its environment — which is exactly what happens when the suite runs from a
linked worktree under a git hook. The result is a repo-local [user] section
and a core.worktree entry in .git/config that silently mis-attribute every
subsequent commit and redirect the primary checkout's working tree.

This has now recurred four times. Each prior fix was file-local, so the next
test file to shell out to git reintroduced it. This issue asks for a
mechanism-level fix that covers test files that do not exist yet.

Impact

Observed in practice, in this order of nastiness:

core.worktree in a non-bare repo redirects the primary checkout. git status
reports a foreign branch, the primary checkout's real untracked files vanish from
the listing, and git checkout -- / git revert operate on the other tree and
still exit 0. A revert can appear to succeed while changing nothing.
Commit mis-attribution. A repo-local [user] always shadows ~/.gitconfig
— global identity is not a backstop. Commits land as t <t@t>.
The developer's ~/.gitconfig is not the only casualty. git -C <tmp> init
under an inherited GIT_DIR does not create a repo in <tmp>; it re-inits the
shared repository.

Root cause (reproduced, not inferred)

Two facts combine:

1. GIT_DIR outranks everything a fixture might use for containment. An explicit
GIT_DIR overrides repository discovery outright — it beats -C <path>, --local,
cwd=, HOME=, and the GIT_CONFIG_GLOBAL / GIT_CONFIG_SYSTEM pins
simultaneously. A fixture doing git -C <tmp_path> config user.email t@t looks
contained and is not.

2. Git exports GIT_DIR and GIT_COMMON_DIR to hooks only in a linked worktree.
They are unset when a hook runs in a normal checkout. This is why the bug looks
unreproducible: running uv run pytest by hand from the main checkout is harmless.
It fires when prek's pre-push hook runs the suite from .worktrees/<branch>/.

core.worktree specifically requires both GIT_DIR and GIT_WORK_TREE to be
set during git init, and records the GIT_WORK_TREE value. Nothing in this
repository writes core.worktree explicitly — verified by grepping all *.py,
*.ts, *.mjs, *.sh, *.toml, *.yaml, *.json. It is purely environmental.

Reproduction

# A real repo with a real identity, plus a linked worktree.
git init -q real && git -C real config user.name RealDev \
  && git -C real config user.email real@dev.example
git -C real commit -q --allow-empty -m init
git -C real worktree add -q ../wt -b probe

# Export what git exports to a hook in a linked worktree.
export GIT_DIR="$PWD/real/.git/worktrees/wt" GIT_COMMON_DIR="$PWD/real/.git"

# Replay agent/tests/test_registry_loader.py:306-308 verbatim.
mkdir sandbox
git -C sandbox init -q
git -C sandbox config user.email t@t
git -C sandbox config user.name t

git config --file real/.git/config --get-regexp '^user\.' # => user.name t / user.email t@t
ls -a sandbox                                             # => no .git; the shared repo was re-inited

Why the previous four attempts did not hold

#	Fix	Scope	Why it did not generalize
#622 → #623	`GIT_AUTHOR_`/`GIT_COMMITTER_` env vars instead of `git config --global`	`agent/src/pipeline.py`	Fixed production code only; said nothing about test fixtures.
#695	—	orchestration arc	Touched the area incidentally; no isolation contract.
#720 → #731	Hard-isolate git fixtures from the developer's real identity	`agent/tests/test_post_hooks.py`	Thorough, but lives in a per-class fixture in one file.
#665	Registry asset resolution	`agent/tests/test_registry_loader.py`	Landed a fresh unguarded `_git()` helper 7 days after #731 hardened the other file.

The pattern is consistent: every fix was placed in the file where the leak was
observed, so it could not protect the next file. #731's _isolated_env is the right
content; it is in the wrong place.

Proposed fix — three layers

Each layer catches what the one above it misses, so no single mistake reaches
.git/config.

Layer 1 — Prevent (`agent/tests/conftest.py`, autouse)

A session/function-scoped autouse fixture that strips the ambient git location
variables from every test's environment and pins config resolution:

GIT_DIR  GIT_COMMON_DIR  GIT_WORK_TREE  GIT_INDEX_FILE  GIT_OBJECT_DIRECTORY
GIT_ALTERNATE_OBJECT_DIRECTORIES  GIT_PREFIX  GIT_CEILING_DIRECTORIES

plus GIT_CONFIG_GLOBAL → a tmp path, GIT_CONFIG_SYSTEM=os.devnull,
GIT_CONFIG_NOSYSTEM=1, and RFC-2606 reserved identity via GIT_AUTHOR_* /
GIT_COMMITTER_*. conftest.py is the only placement that covers test files that
do not exist yet.

Promote _GIT_LOCATION_VARS and the isolation helper out of
test_post_hooks.py so there is exactly one definition, and have that file import
it — otherwise the two drift.

Layer 2 — Detect (`conftest.py`, `pytest_sessionstart` / `pytest_sessionfinish`)

Hash the shared config resolved via git rev-parse --path-format=absolute --git-common-dir at session start, re-hash at session end, and fail the run with
a printed diff if it changed. This is mechanism-independent: it catches any future
route to the file, including ones Layer 1 does not anticipate.

Resolution detail that matters: do not use git rev-parse --show-toplevel.
core.worktree changes what it returns, so an already-polluted repo makes the check
compute a path that does not exist and report "clean" — the pollution would disable
its own detector. --git-common-dir answers from the gitdir alone.

Layer 3 — Refuse (`check:git-config-clean` in pre-commit and pre-push)

A repo check that fails if .git/config contains core.worktree or a [user]
section, printing the exact git config --file <path> --unset-all remedy. Pre-commit
catches it before a mis-attributed commit is created; pre-push catches whatever the
hook-run test suite just wrote.

Also in scope

Audit the other test files that shell out to git and were never covered:
agent/tests/test_shell.py, agent/tests/test_server.py.

Authorization

Per ADR-003, this issue
consolidates and supersedes the four prior partial fixes:

fix(agent): stop clobbering developer ~/.gitconfig — use GIT_AUTHOR/COMMITTER env vars instead of git config --global #622 (issue, closed) / fix(agent): use GIT_AUTHOR/COMMITTER env vars instead of git config --global (#622) #623 (PR, merged) — production git config --global clobber
feat(orchestration): land the carved orchestration arc on main (S2–S8) #695 (PR, merged) — incidental
test: use RFC-2606 reserved identity in test_post_hooks git fixtures (t/t@t leaks verbatim into real .git/config when transcribed) #720 (issue, closed) / test(agent): hard-isolate git fixtures from the developer's real identity (#720) #731 (PR, merged) — test_post_hooks.py fixture isolation
feat(registry): resolve + load registry assets into tasks (#246) #665 (PR, merged) — introduced the current unguarded test_registry_loader.py helper

Filing a new issue rather than reopening #720 because the scope is different: #720
was "use a reserved identity in one file's fixtures"; this is "make the isolation
structural so a fifth recurrence is not possible."

Acceptance criteria

The reproduction above leaves .git/config byte-identical when run through the suite.
A deliberately unguarded new fixture (git -C <tmp> config user.name x with GIT_DIR set) fails the test run rather than silently mutating the shared config — i.e. the detector is proven live, not assumed.
_GIT_LOCATION_VARS has exactly one definition in the tree.
test_shell.py and test_server.py git call sites are audited and covered.
check:git-config-clean fails on a config containing core.worktree or [user], and prints a copy-pasteable remedy.
mise run build green.

Starting implementation.

Branch: test/855-git-fixture-isolation, worktree cut from origin/main.

Authorization: the four prior closed issues/merged PRs cited above (#622/#623, #695, #720/#731, #665) plus maintainer direction to consolidate them. approved applied on that basis.

Already in place locally, not part of this PR: an untracked detect-and-heal script wired to .git/hooks/post-commit, which is how the current recurrence was caught and cleaned. It is deliberately outside the repo — it protects the working copy today, independent of when this lands. Layer 3 is the tracked, shared equivalent.

Order of work, so each layer is proven live rather than assumed:

Layer 2 (detect) first, then deliberately reintroduce the leak and confirm the run fails. A detector written after the fix has nothing to detect.
Layer 1 (prevent) — conftest.py autouse, with _GIT_LOCATION_VARS promoted out of test_post_hooks.py to a single definition.
Layer 3 (refuse) — check:git-config-clean in pre-commit and pre-push.
Audit test_shell.py / test_server.py.

Draft PR: #856 (fix/855-git-fixture-isolation, branched from origin/main @ 521bf64).

Root cause confirmed. GIT_DIR overrides repository discovery, so it outranks -C, --local, cwd, HOME and the GIT_CONFIG_* pins simultaneously — which is why every prior per-file fix lost. Git exports it to hooks only in a linked worktree, so it fires exactly in this repo's documented contribution flow (hooks:pre-push:tests → pytest → a fixture shelling out to git) and never from a plain checkout. check=False on those subprocess.run calls made it silent: git init against an already-initialised repo exits 0.

Three layers, all landed:

Prevent — agent/tests/git_env.py as the single definition of the isolated environment, plus an autouse conftest fixture so a test that forgets to use it still cannot escape its tmp_path. test_registry_loader.py's git config user.* writes deleted; test_post_hooks.py's three duplicate env builders collapsed into the shared one.
Detect — session-start fingerprint / session-finish report that fails the run and names the added key names (not values — a remote.*.url can embed credentials and this reaches CI logs).
Refuse — mise run check:git-config-clean, wired first at both pre-commit and pre-push.

Proved, not asserted. Layer 2 was run against a fake shared config with GIT_COMMON_DIR redirected to a scratch dir: the probe test passed while pytest exited 1 with keys added: user.email, user.name. The real repository was never in scope, and the probe was deleted. Layer 1's central test is differential — the same command with and without an inherited GIT_DIR, asserted to escape in the first case and be contained in the second, so it fails if the helper is ever hollowed out.

Design detail worth flagging for review: the Layer 3 gate resolves the config path without git. No rev-parse form survives the state being detected — --show-toplevel is redirected by core.worktree (the corruption disabling its own alarm), and --git-common-dir aborts with fatal: Invalid path when core.worktree names a deleted pytest tmp_path, i.e. the shape this leak actually leaves behind. The first draft had that bug; there is now a test for it. Same reason its .pre-commit-config.yaml entry is the only hook in the file without the cd "$(git rev-parse --show-toplevel)" prologue.

Verification: agent 1789 passed (83.75% cov), cdk 205 suites / 4366 tests, cli 57 suites / 791 tests, docs + drift checks green. //cdk:synth:quiet fails pre-existing on an IAM ec2:DescribeAvailabilityZones denial in this account. The load-bearing evidence: after those full runs the shared .git/config was byte-identical, verified by content rather than by git status — which this corruption can falsify. The gate passed in its first live pre-commit run, and the commit is signed under the correct identity, the step that failed when this last struck.

--no-verify was used on the push, with the pre-push masking scan proved innocent of my diff (comm -12 against the 46 files it flags on main → empty; security:sast:masking:range → rc 0). Disclosed in full in the PR body. That pre-push-vs-CI gate asymmetry deserves its own issue.

fix(tests): make git-fixture isolation structural, not per-file (#855)

fix(tests): close the discovery route and de-vacuify the #855 guard (…

test(855): land the rest of isadeks' non-blocking review (#856)

New evidence: this leak is reachable through the git hooks, and the blast radius is bigger than a stray commit

Hit on 2026-09-15 while committing #756 (now PR #909), in a worktree without this fix.

The agent-quality pre-commit hook (.pre-commit-config.yaml) runs the full agent pytest suite. On a routine git commit it produced a commit on my branch:

9a1382a8  author: bgagent <bgagent@noreply.github.com>   subject: init

…which deleted the entire repository — every tracked file, from .abca/ through yarn.lock. It is test_registry_loader.py::TestMcpJsonNotCommittable::_init_repo's git init/add/commit landing in my gitdir because prek exports GIT_DIR, which beats git -C <tmp_path>.

Direct proof of the mechanism

No tmp_path in the entire run ever received its own .git:

$ ls -d /tmp/pytest-of-scoschre/pytest-1{5,6}/*/.git
zsh: no matches found

git init -q had nowhere to create one — it re-targeted the exported GIT_DIR. Every subsequent git -C tmp_path add/commit therefore operated on the live worktree's index.

Two things worth separating out:

The wrong author is env-borne, not config-borne. The shared .git/config has no [user] section at all and no t@t (which is what _init_repo sets via git config); the commit still came out as bgagent. So GIT_AUTHOR_*/GIT_COMMITTER_* leaked from the pytest process environment and won over config. A config-only guard would not have caught this one.
Side effects reach the index too. apply_mcp_assets → _protect_mcp_json_from_commit ran git add --intent-to-add .mcp.json and git update-index --skip-worktree .mcp.json against the live index, and left a 0-byte .mcp.json in the worktree. A stray skip-worktree bit on a real path is a silent, durable footgun — it would make later edits to that file invisible to git add -u.

Recovery, for the record

git reset --mixed <base> restored HEAD and the index without touching the working tree; all seven edited files verified byte-identical to a pre-reset backup with cmp (not git status, which is exactly what looks reassuring here — the whole tree read as ?? untracked); index verified free of flagged entries via git ls-files -v; the 0-byte .mcp.json removed; the stray commit left unreferenced.

Why this matters for the fix

The current framing is about test runs. The hook path means an ordinary git commit — the single most common operation in the repo — can silently rewrite history with a wrong-author commit that deletes the tree. Suggests two things in addition to isolated_git_env:

The guard needs to neutralise leaked GIT_AUTHOR_* / GIT_COMMITTER_*, not just GIT_DIR / GIT_WORK_TREE / config.
check-git-config-clean.mjs won't see this one — the shared config stayed clean the whole time. A post-commit assertion on author identity and on unexpected HEAD movement would.

Happy to fold these cases into #856 if you'd like them as tests there rather than a separate issue.

test(agent): isolate continuation runtime git fixtures from hook env

Another occurrence, with a symptom not listed above: pre-commit recursion.

While committing to PR #904 from a linked worktree, the agent-quality pre-commit hook ran pytest with git's hook environment (GIT_DIR, GIT_INDEX_FILE). Unguarded fixture helpers then ran init/add/commit against the real repository:

agent/tests/test_registry_loader.py:297 (_git, on main) overwrote the worktree's root README.md with x and created an empty .mcp.json.
agent/tests/test_continuation_runtime.py:32 (git, added in feat(compute): add Lambda MicroVM P3 approval lifecycle #904) staged tracked.txt into the real index.
The worktree index ended up with 1,196 staged deletions.
Because neither helper disables hooks, the fixture's git commit fired the pre-commit hook again, which re-ran pytest, which committed again. The process tree had to be killed by hand.

Recovery was git reset, git checkout -- README.md and removing the stray files. Nothing was pushed.

#904 now guards test_continuation_runtime.py (bf72bdf) the same way test_continuation_workspace.py already does: strip GIT_* from the environment, pin GIT_CONFIG_GLOBAL/GIT_CONFIG_NOSYSTEM, and pass -c core.hooksPath=/dev/null. I checked it with hook-style GIT_DIR/GIT_COMMON_DIR/GIT_INDEX_FILE pointed at a scratch repo with a linked worktree: 19 tests passed and the scratch repo got no new commits, config or index changes. test_registry_loader.py is still unguarded on main.

For the mechanism-level fix, disabling hooks (core.hooksPath) is worth adding to the shared isolation contract alongside stripping GIT_*. That stops the recursion even when some other env leak gets through.

test(agent): isolate continuation runtime git fixtures from hook env

5th occurrence (2026-10-15), still reachable on main because #856 isn't merged.

Setup: three linked worktrees committing in parallel; the agent-quality pre-commit hook ran pytest with git's hook env. Reproduced the hook env: a linked worktree exports GIT_DIR + GIT_INDEX_FILE (+ GIT_PREFIX); the main checkout only GIT_INDEX_FILE.

Effects observed:

test_registry_loader.py's _git(tmp_path, "init"/"commit", …) (no env scrub) created commits with an empty tree on the worktree's branch: −1118 files, showing up as normal-looking commits in the middle of a feature branch (hit on two branches).
A test with an unscrubbed git config wrote core.worktree = /tmp/pytest-of-<user>/…/origin and [user] t / t@t into the shared .git/config. After pytest cleaned up the temp dir, every git command in every worktree failed with fatal: Invalid path '/tmp/pytest-of-…' — including git config --unset — so the file had to be edited by hand.

#856's structural conftest scrub + the check-git-config-clean guard would have prevented both. Suggest prioritising its merge. Until then, contributors using worktrees should check .git/config after any commit that ran the agent hook.

Navigation Menu

Repository navigation

test(agent): git fixtures leak into the shared .git/config under an inherited GIT_DIR — make the isolation structural (4th recurrence of #622/#720) #855

Summary

Impact

Root cause (reproduced, not inferred)

Reproduction

Why the previous four attempts did not hold

Proposed fix — three layers

Layer 1 — Prevent (`agent/tests/conftest.py`, autouse)

Layer 2 — Detect (`conftest.py`, `pytest_sessionstart` / `pytest_sessionfinish`)

Layer 3 — Refuse (`check:git-config-clean` in pre-commit and pre-push)

Also in scope

Authorization

Acceptance criteria

New evidence: this leak is reachable through the git hooks, and the blast radius is bigger than a stray commit

Direct proof of the mechanism

Recovery, for the record

Why this matters for the fix

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

test(agent): git fixtures leak into the shared .git/config under an inherited GIT_DIR — make the isolation structural (4th recurrence of #622/#720) #855

Description

Summary

Impact

Root cause (reproduced, not inferred)

Reproduction

Why the previous four attempts did not hold

Proposed fix — three layers

Layer 1 — Prevent (agent/tests/conftest.py, autouse)

Layer 2 — Detect (conftest.py, pytest_sessionstart / pytest_sessionfinish)

Layer 3 — Refuse (check:git-config-clean in pre-commit and pre-push)

Also in scope

Authorization

Acceptance criteria

Activity

scottschreckengaust commented on Sep 3, 2026

scottschreckengaust commented on Sep 3, 2026

scottschreckengaust commented on Sep 21, 2026

New evidence: this leak is reachable through the git hooks, and the blast radius is bigger than a stray commit

Direct proof of the mechanism

Recovery, for the record

Why this matters for the fix

isadeks commented on Oct 1, 2026

isadeks commented on Oct 15, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Layer 1 — Prevent (`agent/tests/conftest.py`, autouse)

Layer 2 — Detect (`conftest.py`, `pytest_sessionstart` / `pytest_sessionfinish`)

Layer 3 — Refuse (`check:git-config-clean` in pre-commit and pre-push)