fix(git): resolve the agent repo root with git rev-parse, not a content probe (#2075)

[AndriiPasternak31]

Closes #

Symptom

_detect_git_dir decided an agent's repository root by probing whether
/home/developer/workspace had any content. That tests content, not
legacy-ness: an agent whose repository is rooted at /home/developer but which
also keeps a populated, non-git workspace/ data directory was classified as
workspace-rooted. Every consumer that treats the result as a filesystem path then
read and wrote a subdirectory's files as if they were the repository root's — the
compatibility collector snapshotted the wrong root (producing a false
F-001 template.yaml exists, which cascades into 24 no_template skips), the
per-Push .gitignore migration's [ -d <dir>/.git ] guard failed and the whole
migration silently no-opped, and POST /{agent}/compatibility/fix wrote its
correction into a .gitignore that governs only that subdirectory — then verified
its own write and reported success. A security-relevant check could therefore pass
on the strength of a rule that does not apply to the files it names.

Fix

Ask git. git rev-parse --show-toplevel, run from /home/developer/workspace
when it exists and from /home/developer otherwise, returns the root of the
nearest enclosing repository — --show-toplevel walks up, so a genuinely
workspace-rooted legacy agent still resolves to workspace/ while a standard
agent resolves to /home/developer even with a populated workspace/.

GIT_DISCOVERY_ACROSS_FILESYSTEM=1 is set because workspace/ is a plausible
mount point and git otherwise stops discovery at a filesystem boundary, which
would silently reintroduce the exact misclassification this fixes. That is safe
only because _parse_git_root accepts an answer only when it is
/home/developer or below — /, /home and look-alikes such as
/home/developer2 are rejected, each covered by a test.

Caller audit (all 7)

Git discovers the enclosing repo by walking up from the cwd, so a caller that
merely cds into the returned directory and runs a plain git command was already
operating on the right repository. Only callers that treat the result as a
path were broken.

#	Caller	Uses result as	Before	After
1	update_remote_pat:211	cwd	accidentally correct via upward discovery	same repo, directly
2	rebind_origin_and_push:325	cwd	correct via upward discovery	unchanged; stage="detect" path preserved
3	inspect_container_git:453	cwd	correct via upward discovery	unchanged
4	_migrate_workspace_gitignore:1453	path	.git guard fails → silent no-op	migration finally runs on the real .gitignore
5	initialize_git_in_container:1533	path	chooses where git init lands	byte-compatible for fresh agents (see below)
6	compatibility/collector.py:176	path	snapshots a data directory	snapshots the real repo root
7	compatibility/fixes.py:172	path	writes a subdirectory .gitignore	writes the root .gitignore

Why not just reuse check_git_initialized?

Worth preempting, because check_git_initialized:1698-1727 already resolves
affected agents correctly ([ -d workspace/.git ] then
[ -d /home/developer/.git ]). The honest framing is that this change makes the
two helpers agree rather than adding a third opinion — the file previously
held three different answers to one question (_detect_git_dir's heuristic,
check_git_initialized, and _append_agent_gitignore:709's hardcoded
/home/developer). This matters beyond tidiness: check_git_initialized gates
the 409 Git sync already configured at routers/git.py:388-397, so the two
helpers disagreeing was caller-visible.

It is not reused directly because it answers a different question — is there
a .git here?, returning Optional[str] with None when absent — whereas
callers of _detect_git_dir need a directory even when no repository exists
yet: initialize_git_in_container uses it to choose where to create one.
Collapsing them would either break init placement or force every caller to handle
None. Consolidating the two is a reasonable follow-up, out of scope here.

The no-repo fallback is retained verbatim

The old content heuristic is kept byte-identical as the no-repository
fallback, including its "1" in output substring test. This is deliberate and is
the safety argument for caller #5: initialize_git_in_container uses this value
to choose where git init runs, so fresh-agent placement must not move. The
"1" in output looseness is not where the bug lives — the bug is using a content
probe to answer a repo-topology question — and tightening it would be an
unrequested behaviour change on the exact path whose byte-compatibility is the
safety argument. Two tests pin this (populated → workspace/, empty → home).

Cost: 1 exec in the common repo-present case, 2 in the no-repo case (previously
always 1), plus one warning log line when no repository resolves.

Exceptions are not swallowed. execute_command_in_container already converts
a dead container into a non-zero exit rather than a raise, so a genuine raise is
a real fault — swallowing it would destroy caller #2's stage="detect" message
and caller #3's unknown-never-agreement invariant (ent#109 AC #5). A test pins
propagation.

Operator notes — behaviour changes on already-affected agents

1. The next Push contains deletions. Once the migration stops no-op'ing, the
   next Push appends the fleet-wide _GITIGNORE_PATTERNS to the real root
   .gitignore and git rm --cacheds every tracked file that now matches a
   rule. Working-tree files are untouched and history is not rewritten —
   anything already pushed stays in history, so any credential rotation is
   separate ops follow-up, not part of this PR.
2. Re-init has a larger scope. A post-fix initialize_git_in_container on an
   affected agent runs at /home/developer rather than workspace/, so the
   .gitignore merge, git init, git add . and (on the empty-remote path)
   git push -u origin main --force cover the whole home directory — a much
   larger add surface. The ordering is safe by construction: the ignore merge
   (:1545-1549) happens before git init (:1566) and git add ., so the
   fleet-wide rules are in force at the real root before anything is staged.
   Relatedly, git init at /home/developer where a repo already exists is a
   harmless re-init, whereas the pre-fix behaviour could create a nested repo
   inside workspace/.

Known limitation — agents this does not repair

An agent already re-initialised while misdetected now has a real
workspace/.git, i.e. a genuine nested repository, and is indistinguishable
from a legitimate legacy agent by any probe — git's own answer for it is
workspace/. This fix keeps resolving it to workspace/: correct by the new
rule, still wrong by intent. Repairing those needs a per-agent operator decision,
not an algorithm.

Exposure is bounded: routers/git.py:388-397 refuses re-initialisation with a
409 whenever a git-config row exists and check_git_initialized finds a
.git — and that is one of the correct helpers. Only an agent with an
orphaned config row (row present, no .git) reaches the re-init path.

Read-only ops detection, per agent:

docker exec agent-<name> bash -c \
  '[ -d /home/developer/.git ] && echo home-repo; \
   [ -d /home/developer/workspace/.git ] && echo workspace-repo; \
   git -C /home/developer rev-parse --show-toplevel 2>/dev/null'

Both markers present ⇒ nested repo ⇒ needs a human decision.

Follow-up filed separately (not widened into this diff)

_detect_git_dir and check_git_initialized can still diverge when .git is a
file rather than a directory — the pointer form used by git worktree and by
submodule checkouts. git rev-parse resolves it; [ -d …/.git ] does not. The
result would be _detect_git_dir reporting a root while check_git_initialized
reports None (so the re-init 409 fails to fire), and
_migrate_workspace_gitignore's own [ -d <dir>/.git ] guard still bailing at a
root the probe just resolved. Pre-existing in kind and not triggered by any
current agent layout.

Tests

TDD, two commits: the failing test first, then the minimal fix.

RED — new test file against unfixed code:

$ cd tests && pytest unit/test_<N>_git_root_detection.py -q -p no:randomly -p no:cacheprovider
4 failed, 8 passed, 15 warnings in 0.17s
FAILED ...::test_populated_workspace_with_home_rooted_repo_returns_home
FAILED ...::test_noisy_probe_output_is_parsed
FAILED ...::test_content_probe_skipped_when_repo_found
FAILED ...::test_migrate_gitignore_targets_the_real_repo_root

The migration case shows the defect directly — the guard is issued against the
wrong directory and no further command is ever sent:

E  AssertionError: the migration never ran at the real repo root — the .git guard was issued
   against the wrong directory and the migration no-opped; commands issued:
   ['bash -c "[ -d /home/developer/workspace ] && find /home/developer/workspace -mindepth 1
   -maxdepth 1 | head -1 | wc -l"', 'bash -c "[ -d /home/developer/workspace/.git ]"']

The 8 that passed pre-fix are the invariants that must not move: the legacy
workspace-rooted layout, both no-repository fallback branches, look-alike root
rejection, and exception propagation.

GREEN — after the fix:

$ cd tests && pytest unit/test_<N>_git_root_detection.py -q -p no:randomly -p no:cacheprovider
12 passed, 15 warnings in 0.15s

Neighbours (post-fix):

$ pytest unit/test_github_init_gitignore.py unit/test_github_init_push.py \
    unit/test_compatibility_checks.py unit/test_2036_claude_settings_leak.py \
    unit/test_ent123_tokenless_clone.py unit/test_1264_per_agent_pat_propagation.py -q -p no:randomly
142 passed, 17 warnings in 4.03s

Full unit suite, identical pinned flags before and after
(-m "not slow" -p no:randomly -p no:cacheprovider; pytest-randomly is pinned
off so the two runs are comparable):

Run	passed	failed	skipped
Before (branch point)	8912	1	18
After	8924	1	18

Delta: +12 passed, +0 failed. The +12 is exactly the new test file.

The single failure is pre-existing and unrelated — it reproduces on untouched
dev before any change in this PR, and is not addressed here:

FAILED unit/test_1898_sys_modules_isolation.py::TestTheOutcome::test_a_victim_passes_when_collected_after_the_offender[test_ent125_resilient_system_deploy.py]

🤖 Generated with Claude Code

[github-actions]

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

[dolho]

Recommend closing this as superseded by #2077 — not fixing its CI

I picked this up to fix the red lint (sys.modules pollution check), and found the check is stale and the PR is a duplicate.

#2075 was fixed twice, independently

6a30fa87 on dev — "fix(git): resolve an agent's repo root from git, not a workspace/ content probe (#2075) (#2077)" — already ships a fix for this issue, and dev already contains tests/unit/test_2075_detect_git_dir.py. Rebasing this branch produces an add/add conflict on that file, which is what surfaced the duplication.

Two different implementations of the same idea:

	dev (merged, #2077)	this branch (#2076)
probe	_git_toplevel → git -c safe.directory='*' -C "$start" rev-parse --show-toplevel	_GIT_ROOT_PROBE → cd workspace || cd home; GIT_DISCOVERY_ACROSS_FILESYSTEM=1 git rev-parse --show-toplevel
start point	workspace/ if it exists, else home	same
answer validated inside /home/developer	yes	yes (_parse_git_root)
foreign volume ownership (safe.directory)	handled	not handled
filesystem-boundary discovery	not set	GIT_DISCOVERY_ACROSS_FILESYSTEM=1

I checked the one thing this branch does that dev does not

Its comment argues GIT_DISCOVERY_ACROSS_FILESYSTEM=1 is needed because "workspace/ is a plausible mount point and git's default is to stop discovery at a filesystem boundary". That would be a real residual gap — so I verified it rather than assuming:

• By construction — crud.py:1766 mounts the agent volume at /home/developer; the only other bind under it is /home/developer/shared-out (:1816), which is not on the path from workspace/ up to home. No mount targets workspace/.
• Live — on all four running agents, /home/developer is the single mount and stat reports one device; none even has a workspace/ directory.

So workspace/ is a plain subdirectory of one volume, no boundary is crossed, and the flag is inapplicable to Trinity's container layout. This branch therefore adds nothing dev lacks, while dev additionally handles the foreign-ownership case this one misses.

On the red CI

The failing lint (sys.modules pollution check) is from 2026-08-09 and names tests/unit/test_2075_git_root_detection.py: 0 → 1. It is stale: this PR is DIRTY, and a conflicted PR does not re-run checks, so the last red result simply persists and reads as current. Running the linter against the branch today gives OK: 194 violation(s); baseline allows 240 — no new violations.

Recommendation

Close #2076 as superseded by #2077. Nothing needs porting. If the maintainer would rather keep this implementation, that is a revert-and-replace on dev, not a merge — the two cannot both land.

[AndriiPasternak31]

Bump from the field — this is blocking honest compatibility panels on a live OSS instance.

Three agents there (polymarket-andrii, polymarket-vybe, osint-agent) are the mismatched shape
this PR fixes: git root at /home/developer, plus a populated non-legacy workspace/ directory.
Current effects on that box:

• polymarket-andrii reports 7 hard compatibility failures, which look like the false F-001
  plus the no_template skip cascade rather than real defects — but nobody can tell which until the
  detector is right.
• _migrate_workspace_gitignore still silently no-ops on all three, so the canonical
  _GITIGNORE_PATTERNS migration has never touched their real root .gitignore.

Happy to verify on that instance once it merges and the box upgrades — I can report the before/after
per-agent hard/soft counts, and confirm the ship-time behaviour described in the PR's operator notes
(next backend Push runs the migration for the first time, appends canonical patterns, and fires
git rm --cached on newly-ignored tracked files).

Anything I can add here to help it land?

[AndriiPasternak31]

Concrete before-evidence from the live instance, for whoever reviews this.

polymarket-andrii — git rev-parse --show-toplevel → /home/developer, and it also keeps a
populated workspace/ data directory, so it is exactly the mismatched shape. The compatibility
report gives it 7 hard failures. Every one is false:

Check says	Reality at /home/developer	In workspace/
template.yaml is missing	EXISTS	absent
CLAUDE.md is missing	EXISTS	absent
.gitignore is missing	EXISTS	absent
no skill or command files found	15 skills under .claude/skills/	—
.env is not excluded in .gitignore	.gitignore:25:.env — excluded	—
.mcp.json is not excluded in .gitignore	.gitignore:26:.mcp.json — excluded	—

Verified with docker exec … test -f per path and git check-ignore -v. The only genuine finding
in the whole report is README.md is missing, which is absent at both paths.

The two .gitignore rows are the ones worth highlighting: the panel tells an operator that this
agent's .env and .mcp.json are unprotected and may be committed, when both are ignored and the
.env is not tracked. That is a security check reporting the opposite of the truth, and it sits
alongside two other agents on this box where the same class of file genuinely is tracked — so the
false positives here actively camouflage the real ones elsewhere.

Happy to re-run and post the after-table once this merges and the box upgrades.