fix(update): gate auto-update on an allowlist, and harden the path it re-enables

[iamwhatever]

Summary

The unattended boot-time auto-update was gated on branch != "mainline", inherited verbatim from the internal repo whose primary line carries that name. This repo's primary line is main, so the gate matched nothing and returned at logger.debug — every git checkout, which is the documented install.sh path (git clone + pip install -e .), silently stopped receiving updates with nothing in the logs to say why.

Because that gate had matched nothing for three months, everything downstream of it was dead code — and its latent defects go live the moment the gate starts passing. So this lands the gate fix together with the two defects that guard it, rather than re-enabling a path with known holes.

How this surfaced

A released fix (ACP clientInfo.name, shipped in 0.3.0) was not showing the expected jump in telemetry. Publishing was fine end to end — the stable feed advertises 0.3.0, and the published wheel carries the fix:

$ curl -s https://updates.crew.kiro.dev/feed/stable/latest-cli.json
  "version": "0.3.0",
  "wheel_url": ".../cli/stable/0.3.0/kirocrew-0.3.0-py3-none-any.whl"

$ unzip -p kirocrew-0.3.0-py3-none-any.whl kiro_crew/acp/runtime.py | grep clientInfo
  "clientInfo": {"name": CLIENT_NAME, "version": CLIENT_VERSION},

The artifact was correct and reachable; the apply side was dropping a whole cohort.

Distribution	Auto-updated before this PR?	Path
Desktop (dmg/appimage/deb/rpm)	✅	electron-updater, per-channel feed
Managed wheel	✅	_auto_apply_wheel_update()
source (git checkout)	❌ silently never	_auto_apply_update() → this gate
docker, source tarball	notify-only (by design)	—

Introduced in 64e47961a (2026-06-02), the initial de-Amazoned fork commit. No test covered the branch name. security.py already documents the correct answer two files away.

---

1. Primary-branch allowlist

is_primary_branch reads PRIMARY_BRANCHES — a literal in reviewed code — and nothing else.

Resolving the name from refs/remotes/<remote>/HEAD is the obvious alternative. I shipped it first and it is wrong in both directions; thanks to the review bot for catching each half. That is a local ref, one git remote set-head from being repointed by anything with write access to the checkout:

• Obeying it lets a repoint aim the boot-time git reset --hard + pip install + execv at an arbitrary branch of the still-approved origin. The source pin cannot catch it — the remote URL is unchanged.
• Letting it merely narrow turns the same one-command repoint into a veto: point a main checkout's pointer at mainline and the host silently stops updating, including for a mandatory floor, since _auto_apply_update() is what update_required() drives on a checkout (gateway.py). That strands the host below the administrator's min_version.

A test fails if any subprocess.run is reached from that decision.

Cost, stated plainly: a fork whose primary line is named something else (develop, trunk) gets no unattended update, only the badge. kirocrew update and the dashboard apply path still serve it, and both have a human in the loop.

2. Repo-controlled git exec config

git status and git diff do not merely read config — they spawn the program named in core.fsmonitor, and a reset runs hooks. All five git invocations here ran on an agent-writable tree with none of that neutralized.

Keys whose name is fixed are pinned via GIT_CONFIG_* (same precedence as -c), bound once ahead of the first spawn so the whole sequence is covered and a later-added command cannot quietly opt out.

The list now carries its membership criterion in a comment — "git may exec this value, and the key is a literal" — because the first version was an enumeration without one and was therefore missing core.gitProxy (caught in review), along with core.askPass, core.alternateRefsCommand, uploadpack.packObjectsHook, the pager/editor keys and gpg.program. A test asserts the criterion over the whole set rather than one key at a time, so the next omission fails here instead of in review.

Verified empirically, asserted in both directions so it cannot pass vacuously:

WITHOUT neutralizer    fsmonitor executed: True
WITH neutralizer       fsmonitor executed: False

Keys whose name is repo-chosen are refused, not pinned — there is nothing to override. Same call worktree._checkout_filter makes for worktree add. Both scopes are probed with --includes, and both details are load-bearing:

Case	Caught
clean repo	allowed
--local filter.evil.smudge	refused
--worktree filter.evil.process (a --local listing does not report it)	refused
include.path → filter.evil.clean (invisible without --includes)	refused
diff.evil.textconv	refused
credential.<url>.helper (per-URL, so the pinned bare key misses it)	refused
unreadable config scope	refused (cannot prove it clean)

Each is tested against a real git repo, because these cases exist precisely where a mock would not reproduce git's own resolution. The include.path test asserts git itself resolves the driver first, so it cannot pass on a broken fixture.

3. Check / apply ref mismatch

The availability check compares HEAD against @{u} — whatever the branch tracks (dashboard/handlers/updates.py) — while the apply resets to origin/<branch>. When those are not the same ref the check measures one thing and a --hard reset applies another, so the gap is lost commits rather than a stale answer.

tracks_upstream requires both halves of the upstream to match, because either alone leaves it open:

• the remote — a fork checkout whose main tracks upstream/main while origin is the user's own stale fork;
• the branch — branch.main.remote=origin with branch.main.merge=refs/heads/other, which still points @{u} at origin/other while the reset targets origin/main. (The remote-only check was the first version; the branch half was caught in review.)

---

Two other intentional behaviour changes

• A detached HEAD is no longer primary. The old code fabricated branch = "mainline" for it, which on an internal clone would have let a boot-time git reset --hard move a deliberately detached checkout.
• The skip logs at info, not debug, so an operator can see why a host is not updating. The sibling wheel-mismatch branch already warns; this one said nothing at all.

Notes for review

• PRIMARY_BRANCHES mirrors security._PROTECTED_BRANCHES by intent (the branch an unattended update may reset to is exactly the branch a push must never target) but is kept separate so update routing does not reach into a security-module private. The neutralizer list likewise mirrors the app-side git callers — platform/ must not import from apps/ — and a test asserts the driver regex stays in agreement with the worktree gate's.
• test_spawn_audit's allowlist entry moves with the subprocess call, from resolve_remote_url's former nested helper to the shared _git_probe, justification extended to the new callers.
• The three existing _auto_apply_update test classes gain an autouse fixture neutralizing the new preconditions, so they keep covering the reset sequence instead of passing vacuously by refusing first.
• Deliberately not changed: cli_server.py's or "mainline" fallback for a detached HEAD. Reached only when branch detection returns nothing, then fails loudly at git fetch origin mainline — a confusing-error wart on a user-initiated path, not a silent no-op.

Testing

71 cases in test_governance_updates + TestAutoApplyUpdatePreconditions in test_slack_gateway, which asserts the gateway honours both refusals before spawning anything that would run a driver or fetch.

Mutation-verified nine ways: restoring the mainline hardcode fails 4 (the real-checkout test included); reintroducing a pointer read fails 3; unpinning core.fsmonitor fails 2 (including the real-git exec test); dropping core.gitProxy fails the criterion test; dropping --includes fails the include case; dropping the --worktree scope fails the worktree case; dropping the namespaced-credential branch fails its case; dropping either half of the upstream check fails 1–2; and removing either gateway refusal fails its own test.

Local gates green: pytest 538 across the update/gateway/governance/spawn-audit/cli-server suites, flake8, isort, mypy, black baseline gate, woke scan of added lines.

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

Design-level review of 5c6714b03c2b50d798b08c5f0cc0c9fe7d89960b — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound root-cause fix and well-derived hardening, but the hardening is bound to one call site while sibling update paths spawn bare git on the same tree.

Watch

• The exec-vector chokepoint (trusted_git_bin + git_command_env) covers only the gateway auto-apply. The 12-hourly background check (dashboard/handlers/updates.py:_check_git_checkout) still runs bare "git fetch" unattended with an inherited env — credential.helper, core.askPass, core.sshCommand, url.insteadOf, remote.origin.uploadpack are all fetch-reachable — and the dashboard/cli_server apply paths spawn bare git status/reset --hard. The PR's "human in the loop" rationale covers the trust-relaxation (branch, data loss) but not the exec vectors: the human approves an update, not the repo's planted fsmonitor/hook. The same hazard class this PR closes stays open one file away.
• The precondition pipeline (~250 lines of check → warn → clear_update_progress → return) is inlined in GatewayOrchestrator._auto_apply_update while its reusable halves live in update_governance — so the manual paths can't adopt it without duplicating the sequence, and the neutralizer list now has a fourth copy (md_notebook, papyrus, dev_fleet, platform/) kept in sync by comment and one cross-check test.

Suggestions

• Extract the check sequence into update_governance as one "refusal reason or ''" function and route the dashboard/cli_server git spawns through trusted_git_bin + git_command_env in a follow-up; apps/ may import platform/, so the three app-side neutralizer copies can then collapse onto this one.

[DESIGN-REVIEWED] 5c6714b

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 5c6714b03c2b50d798b08c5f0cc0c9fe7d89960b — this comment is updated in place on each push.

Review details

I've analyzed both candidates against the code, the actual install path, and git's transport behavior.

Candidate 1 (remote.origin.uploadpack/receivepack pinned to a local absolute path breaking SSH fetch): These keys are in _PROGRAM_VALUED_PINS, so git_neutralizer_env does resolve them to a local absolute path via trusted_system_bin. But (c) an observable wrong outcome cannot be established at 80+:

• The documented install is git clone https://github.com/kirodotdev/KiroCrew.git (install.sh:147, docs/guides/install.md:235). remote.<name>.uploadpack/receivepack and core.sshCommand are consulted only for ssh/local transports, never for smart-HTTP — so the default, documented install path is entirely unaffected by these pins.
• The failure only arises for an SSH origin, and whether GitHub's SSH endpoint actually rejects a nonstandard absolute --upload-pack=/usr/bin/git-upload-pack is external behavior the candidate itself could not confirm ("may tolerate"). That is a "might," which the bar requires me to drop.

Candidate 2 (core.sshCommand overrides an operator's custom global ssh command): Same reachability gap — SSH origin only, plus a custom global core.sshCommand. It is a documented, deliberate tradeoff, causes no data loss (only a missed auto-update, with kirocrew update and the badge still available), and the candidate self-rates it low. Well below 80.

Neither reaches the survival bar; I found nothing else in the changed lines that grounds all three of (a)/(b)/(c) at 80+.

No findings.

[OPUS-REVIEWED] 5c6714b

Verdict parsed from the review's SHA-scoped output markers for commit 5c6714b03c2b50d798b08c5f0cc0c9fe7d89960b.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 5c6714b03c2b50d798b08c5f0cc0c9fe7d89960b: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ human override accepted

Human judgment by @iamwhatever overrides the GPT 5.6 finding for 5c6714b03c2b50d798b08c5f0cc0c9fe7d89960b; the recorded reason is authoritative for this commit.

This comment is updated in place on each push.

The model was not re-run because an authorized human decision supersedes it.

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 5c6714b03c2b50d798b08c5f0cc0c9fe7d89960b: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 5c6714b03c2b50d798b08c5f0cc0c9fe7d89960b — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Reading the contract, the intent file, the full patch, and the surrounding repo (git_divergence, updates.py, the three app-side neutralizer copies, security.py) is done — here is the review in the contract's required shape.

First-Principles-Verdict: CONCERNS

commits_ahead re-derives the counting that git_divergence.py declares itself sole owner of, and the same root cause has ~9 unfixed bare-git siblings.

What this change ships

Intent: make git checkouts of this repo actually receive unattended boot-time updates again, without re-enabling the path's latent data-loss and code-exec holes — a FIX plus derived hardening.

1. main checkouts auto-update on boot again — justified (the reported defect).
2. Detached HEAD is never auto-updated — declared, justified.
3. Skip logs at info, not debug — declared, justified.
4. Uncommitted tracked edits now refuse instead of warn-and-destroy — justified (unrecoverable, unattended).
5. Repo-config exec drivers / transport-trust / URL rewrites refuse the run — justified (agent-untrusted boundary).
6. Every git call pinned: neutralizer env + git off PATH via new shared trusted_git_bin — justified; 4th copy of a neutralizer list.
7. Branch must track exactly origin/<branch> or skip — justified (check/apply ref mismatch).
8. Local commits ahead refuse the reset — duplicate of git_divergence.py.
9. Hidden edits, untracked collisions, symlink/junction ancestors refuse — justified.
10. 8 symbols newly exported from update_governance; PRIMARY_BRANCHES and git_neutralizer_env have zero non-test consumers — speculative surface.
    The change has more items (OID pinning, full-ref spelling, loggable_path, the doctor move); these 10 are the most visible.

Watch

• Duplicated counting. git_divergence.py says "This module is the one owner of the COUNTING" and ships divergence_count_args + parse_divergence_counts explicitly "for a caller that must spawn through its own hardened runner" — precisely _git_probe's situation. commits_ahead (1 consumer: gateway.py:8820) re-derives the range, the int() conversion, and unreadable-means-None by hand.
• Counted siblings. Root cause — bare "git" on the same agent-writable checkout with repo config live — persists at 8 spawn sites in dashboard/handlers/updates.py (lines 572, 600, 618, 674, 739, 1252, 1287, 1359) plus git_divergence.count_divergence:124 (grep create_subprocess_exec + "git"). Five of those run in the unattended 12-hourly background check, so "the manual path has a human in the loop" does not cover them; updates.py already imports from update_governance, so env=git_command_env() is reachable there. Accepted-and-deferred, but say which level this fix sits at.
• Fourth neutralizer-list copy (md_notebook/git_ops.py:184, dev_fleet/server.py:536, papyrus/backend/gitops.py:394, now platform/). platform/ must not import apps/, but apps may import platform/ — the new copy could become the base the app copies fold onto instead of a fourth spelling.

Subtractions

• Replace commits_ahead in update_governance.py with _git_probe(proj, *git_divergence.divergence_count_args(target)) + parse_divergence_counts (1 consumer: slack/gateway.py:8820); take the .ahead field.
• Drop PRIMARY_BRANCHES and git_neutralizer_env from __all__ — grep shows zero consumers outside the module and its tests.

[FIRST-PRINCIPLES-REVIEWED] 5c6714b

[iamwhatever]

Dispositions — GPT 5.6 round on d2db7ff6d

Head is now 8a70a87e3. Both findings fixed. Both were real, both are one-line changes, and the first is the more serious of any this PR has drawn.

Note first that the two findings this round are new, and the two standing demands from earlier rounds — remove "main" from the allowlist, and replace the reset with merge --ff-only — did not recur.

A workflow bug worth reporting on its own

The GPT 5.6 Review comment on this PR is stale and does not describe the head it appears under. It still names da20e3a7d; the d2db7ff6d run gated on findings that were never posted. The Post/update review comment step logged:

error parsing "body" value: could not determine current branch: failed to run git: not on any branch
Updated existing GPT 5.6 comment #5388382039

gh failed to read the body in the workflow's detached-HEAD checkout, the step reported success anyway, and the gate then blocked on findings visible only in the job log. Anyone reading the PR comment saw two already-dispositioned items instead of the two real ones. I recovered this round's findings from the run log. Filing separately — it makes the review loop silently unauditable, and it is not specific to this PR.

1. origin/<branch> is ambiguous in the attacker's favour — fixed

Correct, and the impact is larger than the finding states. rev-parse's disambiguation order checks refs/tags/<name> before refs/remotes/<name>, so a tag literally named origin/main wins over the remote-tracking branch. Reproduced:

refs/remotes/origin/main = 8c20a288…          (legitimate)
refs/tags/origin/main    = e37a42bc…          (attacker)

rev-parse origin/main^{commit}              -> warning: refname 'origin/main' is ambiguous.
                                               e37a42bc…      ← the TAG
rev-parse refs/remotes/origin/main^{commit} -> 8c20a288…      ← correct

Two things make this worse than a naming hazard:

• git does not fail — it warns on stderr and prints the tag's OID on stdout, which is the stream this capture reads. So the OID pin I added two rounds ago was working exactly as designed while pinning the attacker's commit.
• The update's own fetch creates the tag. I expected this to need local tag-write access; it does not. git fetch origin auto-follows tags, so publishing refs/tags/origin/main upstream is sufficient — also verified.

Fixed by spelling the ref in full: refs/remotes/origin/{branch}^{commit}. Mutation-verified: restoring the bare form fails the new test.

2. gc.recentObjectsHook is not in the refusal set — fixed, with one honest limit

Accepted, and it is the third instance of the pattern that produced _REPO_UNPINNABLE_KEYS in the first place. Verified the part that decides the mechanism: the key is documented as executed "using the shell" and as explicitly multi-valued ("Multiple hooks are supported"), and with an empty pin applied git config --get-all still lists the repository's own value alongside it. --get reports only the pin — which is exactly how core.gitProxy looked handled while it was not. So a pin is the wrong instrument and refusal is the right one.

What I could not reproduce is the exec. On git 2.50.1 no gc, repack --cruft, or prune invocation consulted the hook — a deliberately failing exit 3 hook failed nothing, and no cruft pack was generated to consult it about. I am accepting the finding anyway, on the pin list's written membership criterion ("git may exec this value, and the key is a literal", where an unnecessary entry costs nothing and a missing one is how core.gitProxy was overlooked) rather than on a demonstrated attack. The docstring records that distinction so the next reader does not inherit a stronger claim than the evidence supports.

Three tests, and the third is the one that matters for the future: it asserts the pin cannot suppress the key, so a later attempt to "handle" this by moving it back to the pin list fails and points at the right mechanism. Mutation-verified all three — dropping the refusal fails one, adding the pin fails another.

---

Local gates on 8a70a87e3: pytest 607 (+2 honest skips), flake8, isort (full CI path list, including conftest.py / xdist_budget.py — the narrower invocation is what let last round's ordering slip through), mypy (1088 files), black baseline, subprocess-encoding gate. Single commit directly on origin/main.

[iamwhatever]

Dispositions — GPT 5.6 round on 8a70a87e3

Head is now db842cd72. This round's two findings were both Fix: Remove "main" — the seventh recurrence — but one of them named a real defect at a site my recorded ruling never addressed, so it is fixed rather than rebutted. Splitting them:

1. "Auto-update trusts an agent-writable remote" — rebutted (7th recurrence)

Identical to rounds 3, 6, 7, 8, 12 and 13, dispositioned each time; the rationale is unchanged and I will not restate it in full. Short form: removing "main" restores the three-month silent no-op this PR exists to fix, and an agent that can rewrite remote.origin.url can already write the source tree pip install -e . installs or the venv execv runs, so the auto-update path is not the marginal escalation. updates.source is the existing operator mechanism and is enforced on this path before the fetch.

2. "Auto-update deletes tracked edits" — fixed, and thank you

The proposed fix is the same rebutted one, but the mechanism this framing names is correct and was not covered by that ruling — my ruling was about trusting the remote, this is about destroying uncommitted work, and the convergence rule judges an independent defect at an unaddressed site normally. Verified in the source: the tracked-file check called logger.warning("discarding local tracked-file changes") and then fell through into reset --hard.

That made the unattended boot path the one code path in the project that could silently delete a developer's uncommitted work — and it was the odd one out among its own siblings. The two last-moment revalidation checks immediately above it both refuse and defer to kirocrew update: the ahead-count for committed work, the exec-config probe for redirected drivers. Only the most fragile case of the three warned and proceeded. A discarded commit is at least reflog-recoverable; an uncommitted edit is gone.

So the tracked-tree check now refuses, with the same message shape and the same update_available refresh as its siblings. Three specifics worth calling out:

• Untracked files still do not refuse. reset --hard preserves them, and task specs and notes live untracked in every real checkout — refusing on those would recreate the silent no-op by another route. There is a test pinning that direction, because it is the obvious over-correction.
• An unreadable git status refuses too. It cannot prove the tree is clean and the next step is irreversible, so it fails closed — the same rule the ahead-count already applies when commits_ahead returns None.
• The manual path is untouched. _auto_apply_update is reached only from the unattended boot/periodic checks; kirocrew update is a separate path and keeps the destructive semantics, because there a human chose them. This is why the fix is a refusal here rather than the merge --ff-only semantic change proposed in earlier rounds — the documented "discards local edits" behavior survives where it was actually intended.

One existing test (test_reset_then_frontend_then_pip) had to change: it passed M file.py and asserted the reset ran anyway, so it was encoding the defect as expected behavior. It now exercises a clean tree, which is what that flow is actually about, and the docstring records why it changed so the next reader does not "restore" it.

Mutation-verified all three directions: reinstating warn-and-proceed fails the refusal test, failing open on an unreadable status fails the fail-closed test, and dropping the ?? filter fails the untracked-files test.

Still outstanding: the review comment on this PR is stale

Repeating this from the last round because it is unresolved and it affects anyone reading along: the GPT 5.6 Review comment does not describe the head it appears under. gh fails to parse the body in the workflow's detached-HEAD checkout, the Post/update review comment step reports success anyway, and the gate then blocks on findings that were never posted. Both of this round's findings were recovered from the job log. Filing separately.

---

Local gates on db842cd72: pytest 610 (+2 honest skips), flake8, isort (full CI path list), mypy (1088 files), black baseline, subprocess-encoding gate. Single commit directly on origin/main.

[iamwhatever]

Dispositions — GPT 5.6 round on db842cd72

Head is now db3bfaedf. One rebuttal, one fix — and the fix is a real gap in the refusal I added last round, which is the useful kind of finding.

1. "Unattended updates trust an agent-writable origin" — rebutted (8th recurrence)

Fix: Remove "main", dispositioned in rounds 3, 6, 7, 8, 12, 13 and 14. Rationale unchanged; not restating it again.

2. submodule.recurse can erase ignored submodule work — fixed

Correct, new, and it defeats the work-tree refusal I added last round. Reproduced end to end:

repo sets submodule.recurse=true + submodule.sub.ignore=all
uncommitted edit inside the submodule

git status --porcelain           -> ""            (COMPLETELY CLEAN)
git reset --hard <target>        -> submodule edit DESTROYED

The ignore=all half is what makes this bite: last round's refusal reads git status --porcelain, and that reports an empty tree while the submodule holds uncommitted work. So the refusal passes, and the reset then recurses and eats it. A fix landing one round after the check it evades is a good argument for the re-probe discipline in this path.

Pinned rather than refused, and I verified which of the two it should be rather than assuming — that distinction has now been wrong twice on this PR (core.worktree, core.gitProxy):

submodule.recurse=false via GIT_CONFIG_*  -> submodule edit SURVIVED

The pin works, so pinning is correct here.

Placed in a new list, not the exec list. _GIT_EXEC_NEUTRALIZERS carries a written membership criterion — "git may exec this value, and the key is a literal" — and submodule.recurse execs nothing; it widens what reset --hard touches. Dropping it in there would have quietly falsified the one criterion that makes that list auditable, which is what let core.gitProxy hide in the first place. So it goes in _GIT_BLAST_RADIUS_PINS with its own stated criterion, folded into the same GIT_CONFIG_* chokepoint. There is a test asserting it is not in the exec list, so a later tidy-up that merges them fails.

Three tests. The end-to-end one asserts both directions against real git, and getting the control right mattered: my first version took the unpinned direction from _fixture_git_env, which (since round 12) itself builds from git_command_env() — so the "unprotected" control was already carrying the pin under test and the whole test passed vacuously. The control now takes the seam's real env and flips only that one pin back to true, so the destruction is actually reproduced before the protection is claimed. Mutation-verified three ways: removing the pin, pinning the wrong value, and an env-builder undercount that silently drops the trailing list all fail it.

Still outstanding: the review comment on this PR is stale

Third round reporting this. The GPT 5.6 Review comment still does not describe the head it appears under — gh fails to parse the body in the workflow's detached-HEAD checkout, the post step reports success anyway, and the gate blocks on findings never posted. Every finding for the last three heads was recovered from job logs. Filing separately; flagging here so nobody reads the visible comment as current.

---

Local gates on db3bfaedf: pytest 613 (+2 honest skips), flake8, isort (full CI path list), mypy (1088 files), black baseline, subprocess-encoding gate. Single commit directly on origin/main.

[iamwhatever]

Dispositions — GPT 5.6 round on db3bfaedf

Head is now 7b07019da. All three findings fixed, and "main" did not recur this round. Two of the three are genuinely good catches against claims I had made confidently.

1. Replace refs substitute content for the pinned OID — fixed

This one defeats an argument I have made in three separate dispositions. I pinned the reset target to an OID and wrote "an OID cannot move." That is true of the identifier and false of what the identifier resolves to: a refs/replace/<oid> entry makes git serve a different object for that id, transparently, everywhere. Reproduced:

captured OID  e82bd3d7…  (honest commit, content "good")
git replace e82bd3d7… <evil>

git reset --hard e82bd3d7…                          -> "EVIL"
GIT_NO_REPLACE_OBJECTS=1 git reset --hard e82bd3d7… -> "good"

So the OID pin was load-bearing and quietly bypassable. Fixed by setting GIT_NO_REPLACE_OBJECTS=1 in git_command_env().

Worth noting the shape, because it is the opposite of the GIT_DIR family in the same function: those are hazards whose safe state is absence (hence a built env rather than a merge), while this one's safe state is presence. Replace refs live in the repository, so there is no variable to strip — the opt-out has to be added. Both mechanisms now coexist in that builder and the docstring says which is which.

Two tests, and the end-to-end one asserts both directions with the control differing from the protected env only by removing this variable.

2. Hard reset deletes untracked files on a path collision — fixed, not by merge --ff-only

Correct, and it is the third consecutive round where a real gap sat one level below the check I had just added. My own code comment claimed "untracked files (task specs, notes) are preserved" — true only while they do not collide with a path the target adds. Reproduced:

upstream adds newfile.txt; local untracked newfile.txt = "MY PRECIOUS UNTRACKED WORK"
git status --porcelain          -> "?? newfile.txt"     (skipped by check 3, by design)
git reset --hard <target>       -> "from upstream"      (local content GONE)

So ?? being deliberately ignored — which is right for the general case, and which I added a test to pin last round — is exactly what let this through.

The proposed fix is the merge --ff-only change I have declined three times, and I am still declining it; but the finding stands on its own and is fixed a different way. The path now lists what the target would add (git diff --name-only --diff-filter=A -z HEAD <target>) and refuses if any of those paths already exists locally. That closes the same hole while keeping the documented reset semantics, and it is narrower than --ff-only, which would also abort on cases this path already handles by refusing earlier. An unlistable added-set fails closed, like the unreadable status and the unknown ahead-count.

Two guard tests beyond the refusal itself: adding paths that don't collide must still update (over-refusing here would recreate the silent no-op this PR exists to remove), and an unlistable set must refuse. Mutation-verified all four directions.

This also required a test-harness fix worth flagging: git diff is now two calls with opposite return-code conventions — the --quiet change check uses rc as a boolean, the new listing uses rc for success — so the argv-dispatching fake answered both from one branch and every happy-path test started refusing. The fake now discriminates on flags. Last round's lesson was "dispatch on the subcommand, not the call count"; this round's is that a subcommand stops being a discriminator as soon as a second call reuses it.

3. Test probe environments could escape tmp_path — fixed

Correct, and the same no-test-side-effects class you raised in rounds 12 and 13 — which I fixed in the fixtures and in production while leaving this older fsmonitor probe on raw os.environ. An inherited GIT_DIR would point its git status at a real checkout and execute that repository's configured fsmonitor, outside tmp_path.

Both probes now build from _fixture_git_env, with the control flipping only the core.fsmonitor pin, so they differ solely in the thing under test. That is the same correction I had to make to my own submodule control last round, and it is the third site in this class — the pattern is "a control direction reaching for raw os.environ," and it is worth a lint rule rather than another round of whack-a-mole.

---

Local gates on 7b07019da: pytest 618 (+2 honest skips), flake8, isort (full CI path list), mypy (1088 files), black (this round the gate caught a real new offender in test_governance_updates.py; reformatted), subprocess-encoding gate. Single commit directly on origin/main.

[iamwhatever]

Dispositions — GPT 5.6 round on 7b07019da

Head is now 751800449. One fixed, one rebutted.

1. Inherited PATH permits update-process hijacking — fixed

Accepted, and I checked the fix bar before accepting: trusted_system_bin already exists in platform_compat, AGENTS.md already makes it the required way to spawn a system tool ("a bare argv name (resolved through a PATH that can lead with same-uid-writable dirs)" is listed as the wrong form), and cli_doctor already resolves git through it. So this is not new machinery — it is an existing project rule that the update seam was the one place violating.

That is the uncomfortable part of the finding. The doctor, which only reads git output, was hardened; this path, where git's answer selects the code that then gets installed and execv'd, was still spawning a bare "git". A planted shim here does not merely lie about the branch — it chooses the payload.

Fixed at both layers: _git_probe in the seam, and all seven spawns in the gateway sequence. Three specifics:

• Resolved once per update, not per spawn. Re-resolving would leave a window for the answer to change mid-sequence; one resolution means every step provably runs the same binary. There is a test that regresses a single spawn back to a bare name and fails.
• None is a refusal, never a fallback. Falling back to "git" would reinstate precisely the hazard, so the seam returns "could not determine" (which every caller already treats as unsafe) and the gateway skips the update entirely — not even the branch probe runs, since that first spawn would already be the shim.
• The Windows fallback was shared, not duplicated. trusted_system_bin never finds git on Windows (it lives under Program Files), so without the install-root fallback this would have silently disabled auto-update for every Windows source install. That fallback existed as a private helper in cli_doctor; I moved it to platform_compat.trusted_git_bin() and pointed both callers at it rather than copying a security allowlist into a second file, where the two copies would drift. The tests for the resolution rules moved with it, to test_platform_compat; the doctor's tests keep their own subject (what _git_line does with each outcome).

This grew the diff to 9 files, which I would normally resist on a PR this size — but the alternative was either a duplicated allowlist or a Windows regression.

2. "Auto-update races its source validation" — rebutted (9th recurrence)

Fix: remove the newly allowed branch, dispositioned in rounds 3, 6, 7, 8, 12, 13, 14 and 16. The framing is new ("fetch uses the validated URL directly with an explicit destination refspec") but the demanded change is the same one, and the rationale that covers it is unchanged: removing "main" restores the three-month silent no-op this PR exists to fix.

On the new framing specifically: fetching the validated URL directly, with an explicit destination refspec, is a coherent design and I am not dismissing it on the merits — but it is a redesign of the fetch step, not a fix to the branch gate, and updates.source already exists as the operator-facing mechanism for constraining the remote and is enforced on this path before the fetch. The TOCTOU half of it is what the last-moment revalidation block addresses, and that block has grown four checks over the last three rounds.

---

Local gates on 751800449: pytest 622 on the update/gateway/governance set plus 1107 across the doctor / platform-compat / update surface (the resolver move touches both), flake8, isort (full CI path list), mypy (1088 files), black baseline, subprocess-encoding gate. Single commit directly on origin/main.

[iamwhatever]

Dispositions — Opus 4.8 + GPT 5.6 rounds on 751800449

Head is now ba6e7382b. Both reviewers landed on the same defect independently, from different angles, and they were right: the untracked-collision guard I added two rounds ago had three holes. All three are fixed. GPT's "main" finding is rebutted for the tenth time.

Worth stating plainly: this is the third consecutive round where the new finding is a hole in the previous round's fix. The collision guard has now been wrong about renames, about non-UTF-8 paths, and about ancestors — which says the check was under-specified when I wrote it, not that the reviewers are nitpicking.

1. Renames hide from --diff-filter=A — fixed (found by both)

Opus derived it from git semantics; GPT listed it first among three. Reproduced:

upstream: git mv a.txt b.txt        (pure rename)
local:    b.txt exists, untracked, "MY PRECIOUS UNTRACKED WORK"

git diff --name-status HEAD target                        -> R100  a.txt  b.txt
git diff --name-only --diff-filter=A -z HEAD target       -> ""          ← guard sees nothing
git diff --name-only --diff-filter=A --no-renames …       -> "b.txt"
git reset --hard target                                   -> b.txt OVERWRITTEN

Rename detection is on by default for porcelain diffs, --diff-filter=A excludes R, so the destination never appeared as added. One token: --no-renames, which decomposes the rename into a delete plus an add.

Two tests, deliberately at different levels: one pins the flag in the argv, and one asserts git's behaviour against a real repo — because the flag is only correct while that behaviour holds, and if a future git stops classifying this as R I want a test that says so rather than a flag that has quietly become cargo.

2. Non-UTF-8 paths decoded into a miss — fixed (GPT)

Subtle and verified:

added path bytes: b"bad\xffname.txt"
.decode(errors="replace") -> 'bad\ufffdname.txt'   lexists=False   ← guard passes
os.fsdecode(...)          -> 'bad\udcffname.txt'   lexists=True
git reset --hard          -> the real file was overwritten

errors="replace" is lossy, so the guard was calling lexists on a filename that cannot exist while looking straight at the one that does. Now os.fsdecode, which round-trips through surrogateescape and is what the os.path calls below it need.

3. Obstructing ancestors — fixed (GPT)

Also verified, and the one I would least likely have found myself:

target adds pkg/mod.py;  locally `pkg` is an untracked FILE ("MY PRECIOUS NOTES")
lexists("pkg/mod.py") -> False      ← guard passes
lexists("pkg")        -> True
git reset --hard      -> `pkg` replaced by a directory, the file destroyed

git has to remove the obstructing file to create the directory. The guard now walks each added path's ancestors and refuses when one exists as a non-directory. A directory ancestor is explicitly not an obstruction — otherwise every update adding a file to an existing package would refuse, which is the silent no-op this PR exists to remove, reintroduced as an over-correction. There is a test pinning that direction, and a mutation confirming it fails when the distinction is dropped.

4. GPT: "Main auto-update trusts an agent-writable origin" — rebutted (10th recurrence)

Fix: Remove "main", dispositioned in rounds 3, 6, 7, 8, 12, 13, 14, 16 and 17. Unchanged rationale.

---

Mutation-verified all six directions this round: dropping --no-renames, reverting to errors="replace", removing the ancestor walk, and treating a directory ancestor as an obstruction each fail exactly the test that describes them.

Local gates on ba6e7382b: pytest 963 across the update / gateway / governance / doctor / platform-compat surface (+23 skips, pre-existing), flake8, isort (full CI path list), mypy (1088 files), black baseline (the gate caught a real new offender in gateway.py; the reformat is confined to the lines this round added), subprocess-encoding gate. Single commit directly on origin/main.

[iamwhatever]

CI fix — shard 4 INTERNALERROR was mine, and it was hiding a real logging bug

Head is now a80b63ba3, rebased onto current main.

Four red checks on 07515b99d were one root cause, not four problems: Backend Tests (3.10, 4) and (3.12, 4) are the same shard on two interpreters, and Coverage Gate / PR Readiness are pure cascades of it (Require upstream coverage jobs to have succeeded). No other test in shard 4 failed — the only crashitem was mine.

What happened

My own round-18 test, test_a_non_utf8_added_path_is_still_matched, killed the xdist worker:

UnicodeEncodeError: 'utf-8' codec can't encode character '\udcff' in position 154
execnet.gateway_base.DumpError: strings must be utf-8 encodable
INTERNALERROR> assert not crashitem
  ('test_a_non_utf8_added_path_is_still_matched', <WorkerController gw3>)

A crashed worker fails the whole shard, which is why one test looked like a pile of backend failures.

Why it passed locally and not in CI: I ran with --override-ini="addopts=", which disables the repo's -n auto. Without xdist nothing serializes the report, so the defect was invisible. Reproduced immediately once I added -n 2.

The actual bug is in production code, not the test

Bisected it to the log record, not the assertion — with -p no:logging the test passes. The culprit is the collision refusal I added last round, which logs a filename taken straight from git output:

logger.warning(..., collisions[0])   # collisions[0] == "bad\udcffname.txt"

os.fsdecode is required for the os.path collision checks, but it renders a byte that is not valid UTF-8 as a lone surrogate, and a surrogate cannot be encoded to UTF-8. So:

• logging raises inside the handler, does not propagate, and drops the record — losing precisely the line that proves an unattended update refused in order to save the user's uncommitted file. The protection still works; the evidence that it worked disappears.
• Any UTF-8 log shipper sees a broken record.
• pytest-xdist dies, which is the CI symptom.

So the test crash was a real find about the production path, and fixing only the test would have left the dropped-record bug in place.

Fix

New loggable_path() in the seam, used for the log only:

return os.fsencode(name).decode("utf-8", "backslashreplace")

Round-trips to the original bytes and escapes only the un-encodable ones, so the operator sees the true on-disk byte (bad\xffname.txt) instead of a lossy \ufffd. Verified that ASCII and valid non-ASCII names (CJK, accented) pass through byte-identical — this must not trade a rare crash for everyday unreadability in non-English checkouts.

Applied to the collision line plus the two sibling refusal lines in the same block that interpolate proj (which comes from os.environ, decoded with surrogateescape on POSIX, so it carries the same hazard). The un-decorated os.fsdecode form remains what goes to the filesystem — the docstring says so, because passing the escaped form to os.path would be the obvious wrong "cleanup".

Two guard tests, and the key property is that the gateway one reproduces the CI failure in a plain single-process run by asserting every emitted record is UTF-8 encodable — so this cannot regress into another shard-wide crash that only CI can see. Both skip rather than pass vacuously if the platform's fsdecode or filesystem does not produce the hazard.

Mutation-verified four ways: reverting the sanitizer fails the new test single-process and reproduces the INTERNALERROR under xdist; degrading backslashreplace to replace fails the helper test.

Process note

I have been running the local suite with addopts="" all along, which silently opted out of the xdist path CI uses. Re-ran everything under -n 4 this round: 985 passed / 23 skipped across the update, gateway, governance, doctor and platform-compat surface.

---

Local gates on a80b63ba3: pytest 985 (+23 skips) under xdist, flake8, isort (full CI path list), mypy (1097 files), black baseline, subprocess-encoding gate. Single commit, 0 behind origin/main.

[iamwhatever]

Dispositions — GPT 5.6 round on a80b63ba3

Head is now c3b55b35d, rebased onto current main. Three fixed, one fixed-in-part, and "main" did not recur.

First: the shard-4 INTERNALERROR from last round is resolved — Backend Tests (3.10, 4) and (3.12, 4) both pass on a80b63ba3.

1. Fetch re-resolves a mutable remote — the named mechanism is fixed; the fetch redesign is not

I went looking for the concrete rewrite mechanism rather than re-arguing the framing, and found the finding's second clause is the load-bearing one. Verified:

repo config: url.<evil>.insteadOf <honest>
git fetch <honest-url> +main:refs/remotes/probe/main   ← URL passed EXPLICITLY
fetched content: EVIL

So url.<base>.insteadOf rewrites the URL below the argument — which means the proposed "fetch the captured validated URL" fix would NOT have worked on its own. Fetching an explicit URL feels like it pins the source and does not.

url.<base>.insteadOf / .pushInsteadOf are repo-NAMED (the base is arbitrary), so there is no key to pin — they join the refusal set, which is where the existing machinery already puts this shape. New _REPO_URL_REWRITE_RE, checked in repo_exec_config_reason, kept as its own regex rather than folded into _REPO_EXEC_DRIVER_RE because that list's written criterion is "arbitrary-named driver program" and a URL rewrite executes nothing — the same reason submodule.recurse got its own list two rounds ago.

Not doing the rest: switching the fetch to an explicit URL with a destination refspec. With the rewrite mechanism refused, the remaining delta is the window between reading remote.origin.url and the fetch, and the fix for that is the fetch redesign I have declined in rounds 16 and 17 — plus it would now be redundant with the refusal for the mechanism actually demonstrated. updates.source remains the operator-facing pin and is enforced before the fetch.

2. Transport helpers still resolved through PATH — fixed, and this one is on me

Correct, and it is the most embarrassing finding of the PR: round 17 resolved git off PATH through trusted_git_bin, and my own pin list then handed the helpers git spawns straight back to the same PATH:

("core.sshCommand", "ssh")                    ← bare name
("remote.origin.uploadpack", "git-upload-pack")
("core.pager", "cat") / ("core.editor", "true") / ("gpg.program", "true")

Pinning core.sshCommand closes the repository's value and then runs whatever ssh leads the gateway's PATH. Fixing git while leaving its transport helper PATH-resolved is most of the hole reopened.

All seven program-valued pins now resolve through trusted_system_bin at env-build time (not import time — that lookup is deliberately uncached so a tool installed after boot is picked up). Verified every one resolves on a normal install, including git-upload-pack, so this does not silently disable local/file transports. An unresolvable name degrades to os.devnull, which cannot exec: pager/editor/gpg are unreachable on this path anyway, and a transport helper that cannot be trusted should stop an unattended update rather than fall back to an untrusted one.

Marked with a _PROGRAM_VALUED_PINS set plus a test asserting the set is a subset of the pin list — a typo there would silently stop resolving that key, which looks like nothing at all.

3. Directory symlinks bypass the collision guard — fixed

Correct. os.path.isdir follows the link, so an untracked symlink-to-directory reported "directory, not an obstruction". Reproduced, and the outcome is worse than a missed check:

lexists('pkg') True   isdir('pkg') True  ← follows the link
guard verdict: not an obstruction
after reset:   the symlink is GONE, replaced by a real directory

The developer's deliberate structure was destroyed. islink is now checked first, before isdir gets a chance to follow. This is the third distinct hole in this guard (renames, non-UTF-8, now symlinks) — the check was under-specified when I wrote it, and I am no longer surprised when a new shape turns up.

4. Unbounded filesystem walk on the event loop — fixed

Correct, and anchored to a real project rule. _obstructions stats every added path and each of its ancestors synchronously, inside the async function — a large update stalls every chat and the heartbeat. Offloaded to subprocess_executor(), which already exists and was already imported here.

Worth flagging a test-quality miss of my own: my first version of the guard asserted any(executor is subprocess_executor()) across the function's offloads, and it passed with the scan put back inline — another offload already satisfied it. My own mutation run caught it. Rewritten to assert the property the rule is actually about: the thread the scan's probes run on. That version fails when inline, and the docstring records why the first one was wrong so nobody "simplifies" it back.

---

Mutation-verified all five directions: bare-name pins, bare-name fallback, dropped URL-rewrite refusal, dropped islink check, and the inline scan each fail exactly the test that describes them.

Local gates on c3b55b35d: pytest 992 (+23 skips) under xdist -n 4, flake8, isort (full CI path list), mypy (1099 files), black baseline, subprocess-encoding gate. Single commit, 0 behind origin/main.

[iamwhatever]

Dispositions — GPT 5.6 round on c3b55b35d

Head is now 8bdd5bb98. One fixed, one rebutted for the eleventh time.

1. assume-unchanged edits are invisible to porcelain status — fixed

Correct, and it is the last blind spot in the work-tree refusal. Verified end to end, and git's blindness here is total:

git update-index --assume-unchanged tracked.txt
echo "MY PRECIOUS EDIT" > tracked.txt

git status --porcelain     -> ""      (empty)
git diff --quiet HEAD      -> rc 0    (clean)
git reset --hard <target>  -> "original"    ← edit DESTROYED

Both of the signals the path already consults report a clean tree, so checks 3 and the earlier diff pass and the reset overwrites a tracked file the developer had edited. skip-worktree is the same mechanism.

Fixed read-only, not with the proposed update-index --really-refresh. That command does surface the edit — I verified it — but it writes the index, and a check whose entire purpose is to decide whether mutating the checkout is safe should not mutate it to find out. Unattended, it would also quietly disturb the developer's own index state, which is the sort of side effect this path has been trying to avoid for twenty rounds.

Instead hidden_worktree_edits() enumerates the flagged entries with ls-files -v (a lowercase tag means "not checked", S means skip-worktree) and compares each one's raw bytes against the blob HEAD records, via hash-object --no-filters. --no-filters avoids exec'ing a clean driver; a repository configuring one is already refused before this runs, so raw bytes are the correct comparison.

Two design points worth stating, both with tests:

• Only an ACTUAL difference refuses. assume-unchanged is a common trick for local config overrides. Refusing on the mere presence of the bit would disable auto-update for every such checkout — the silent no-op this whole PR exists to remove, reintroduced through the back door. There is a test pinning that direction, and a mutation that makes the check unconditional fails it.
• Read-only is asserted, not assumed. A test snapshots .git/index around the call and compares bytes, so a future "simplification" to --really-refresh fails.

None (git could not answer) refuses, same fail-closed rule as an unknown ahead-count and an unreadable status.

One test-harness consequence: the new probe reads real git metadata, so the three _permit_update_preconditions fixtures now neutralize it alongside repo_exec_config_reason and tracks_upstream — otherwise every test in those classes would have passed vacuously by refusing before reaching its subject. My anchor assertion expected two fixtures and found three, which is the only reason I noticed the third.

Mutation-verified four ways: never detecting, over-refusing on the bit alone, ignoring the verdict, and failing open on an unreadable listing each fail exactly the test that describes them.

2. "main enables unattended updates from an agent-writable origin" — rebutted (11th recurrence)

Fix: Remove main from PRIMARY_BRANCHES, dispositioned in rounds 3, 6, 7, 8, 12, 13, 14, 16, 17 and 19. The rationale is unchanged and I will not restate it again: the demanded change reverts this PR's entire purpose, restoring a three-month silent no-op in which no git-clone install auto-updated at all.

I'll note for the record what the last ten rounds have actually produced, because it bears on how to read this one. Every round in which GPT looked at the mechanisms rather than the branch list found something real, and I fixed all of them: exec drivers, core.gitProxy, GIT_DIR redirection, the OID pin, commits_ahead measured against the wrong revision, refs/remotes ambiguity, submodule.recurse, replace refs, untracked collisions (renames, non-UTF-8, ancestors, symlinks), PATH resolution for git and then for its helpers, url.insteadOf, an event-loop stall, and now assume-unchanged. That is a substantially hardened path, and it is hardened because the gate was enabled rather than removed. The one finding that has never changed is the one whose fix is "revert the feature".

---

Local gates on 8bdd5bb98: pytest 998 (+23 skips) under xdist -n 4, flake8, isort (full CI path list), mypy (1099 files), black baseline, subprocess-encoding gate. Single commit, 0 behind origin/main.

[iamwhatever]

CI fix — Windows shard 4, my own test again

Head is now 620c3cfcb. No new review findings this round; this is the Windows failure on 8bdd5bb98.

FAILED test/test_slack_gateway.py::TestAutoApplyUpdateResetPath::test_the_collision_refusal_logs_an_encodable_record
  UnicodeDecodeError: 'utf-8' codec can't decode byte 0xff in position 3: invalid start byte

os.fsdecode is not platform-symmetric, and I assumed it was. POSIX uses surrogateescape, so b"bad\xffname.txt" becomes "bad\udcffname.txt". Windows uses UTF-8 + surrogatepass, which passes through surrogate code points but cannot decode an invalid start byte — so the call itself raises. I had it above the platform guard rather than inside it, so instead of skipping on Windows the test errored.

Two sites, both mine: the round-19 log-encodability test and the round-19 loggable_path unit test. The round-18 test that came first happened to have os.fsdecode inside its try, which is why only the newer ones broke — the same guard written two different ways, and only one of them was right.

Skipping is the correct outcome, not a workaround: Windows filenames are UTF-16, so there is no invalid-byte name for the hazard to exist in. The guard just has to be reached instead of crashed into.

Verified rather than assumed, since I cannot run the Windows shard locally: a pytest plugin overrides os.fsdecode with the real Windows implementation (decode("utf-8", "surrogatepass")) and all three affected tests then report

SKIPPED  this platform cannot represent a non-UTF-8 name
SKIPPED  this filesystem rejects non-UTF-8 names

instead of erroring.

Worth noting what I got wrong in my own diagnosis: my first hypothesis was core.autocrlf, and I probed it before reading the log. That probe found a different real bug — hash-object --no-filters suppresses git's built-in EOL conversion as well as external drivers, so under core.autocrlf an unmodified assume-unchanged file hashes differently from its blob and last round's check would have refused every such Windows checkout:

core.autocrlf=true, file untouched
hash-object --no-filters  -> 8561d5d6…   ≠ HEAD blob c0d0fb45…   ← false refusal
hash-object (attrs)       -> c0d0fb45…   = HEAD blob             ← correct

My --no-filters rationale was inverted: because filter drivers are refused before this runs, letting git apply attributes is precisely what is safe — and it is also what is correct. Dropped the flag, and the regression test sets core.autocrlf explicitly so the Windows posture is reproduced on every platform rather than only on the Windows shard. Mutation-verified: restoring --no-filters fails it.

So this round fixes two things — the test guard that actually broke the shard, and a Windows-only false refusal that the shard would not have caught because the flag made the test skip there anyway.

---

Local gates on 620c3cfcb: pytest 999 (+23 skips) under xdist -n 4, plus the simulated-Windows run above, flake8, isort (full CI path list), mypy (1099 files), black baseline, subprocess-encoding gate. Single commit, 0 behind origin/main.

[iamwhatever]

Dispositions — GPT 5.6 round on 620c3cfcb

Head is now d37ea9767. One finding, fixed. "main" did not recur for the second round running.

remote.<name>.vcs executes a repo-chosen transport helper — fixed

Correct. remote.origin.vcs=<foo> makes git run git-remote-<foo> as the transport, resolved through PATH, so the fetch itself executes a program the repository named. Reproduced:

repo: remote.origin.vcs = evil
PATH: contains git-remote-evil

git fetch origin main
  -> fatal: remote helper 'evil' aborted session
  -> git-remote-evil EXECUTED

It is the same shape as credential.<url>.helper — repo-named key, value names the program — so it joins _REPO_EXEC_DRIVER_RE, which is where that criterion already lives.

One thing the finding did not say, and it decides the fix: I checked whether a pin would work, since this key looks pinnable and two earlier keys on this PR (core.worktree, core.gitProxy) were pins that silently lost. An empty pin does stop the repository's helper — but git then treats "" as a helper name:

GIT_CONFIG_KEY_0=remote.origin.vcs GIT_CONFIG_VALUE_0=""
  -> fatal: remote helper '' aborted session

So pinning would not protect the update path, it would disable it — every fetch would fail, which is precisely the silent no-op this PR exists to remove. Refusal is the only correct option here, and now for a documented reason rather than by analogy. There is a test asserting the key never appears in the pin list, so a later "tidy-up" that moves it there fails.

Mutation-verified: making the new regex alternative unmatchable (while keeping it valid) fails the refusal test.

---

Where this PR stands

This is round 12 of GPT review. The gate itself is still the one-line change it started as; everything since has been hardening the path that change re-enables, and it has been worth doing — this round alone closed a live remote-helper exec. But the shape of the last several rounds is worth naming for whoever picks this up:

• Rounds that examined mechanisms found real bugs, every time. Exec drivers, core.gitProxy, GIT_DIR, the OID pin, commits_ahead against the wrong revision, refs/remotes ambiguity, submodule.recurse, replace refs, four separate holes in the untracked-collision guard, PATH for git and then for its helpers, url.insteadOf, an event-loop stall, assume-unchanged, and now remote.<n>.vcs. All fixed and mutation-verified.
• The one finding that never changed is the one whose fix is "revert the feature" — Fix: Remove "main", raised in 11 rounds and dispositioned in all of them.

The remaining question is not technical convergence but whether to keep paying a round per newly-imagined git config key. Every new one has been real, so I am not arguing they are noise; I am noting that the surface is git's entire configuration space and the review will keep finding entries in it. Somebody with merge authority should decide when the hardening is sufficient — that is a judgment call, not something I should make by attrition.

---

Local gates on d37ea9767: pytest 1001 (+23 skips) under xdist -n 4, flake8, isort (full CI path list), mypy (1099 files), black baseline, subprocess-encoding gate. Single commit, 0 behind origin/main.

[iamwhatever]

Dispositions — GPT 5.6 round on d37ea9767

Head is now e0a4b18c3. Four findings: two fixed, two rebutted as recorded rulings.

1. Windows junctions bypass the collision guard — fixed, and it was a rule violation

Correct, and this one I should not have needed told. os.path.islink returns False for a Windows junction, so a junction ancestor reads as a plain directory and the reset writes through it, outside the checkout. Three things make it clear-cut:

• platform_compat.is_link_or_junction already exists for this.
• Its docstring describes this exact failure: "a caller that only checks islink would treat a junction as a real directory and rmtree THROUGH it, destroying the target's contents."
• AGENTS.md line 442 lists is_link_or_junction(path) as the required form and path.is_symlink() as the wrong one.

So the symlink fix I added in round 20 reached for the bare islink one round after I fixed the same class of "use the project's hardened helper, not the raw stdlib call" for git itself. Now uses the helper. Mutation-verified: reverting to os.path.islink fails the new test.

2. Repository config can disable TLS verification — fixed

Correct. http.sslVerify=false in the checkout's own config turns off certificate verification for the update download; with a hostile proxy the forged update is indistinguishable from the real one, and this path then installs it and re-execs. The CA / client-cert keys reach the same place by supplying the trust material instead of removing the check.

New _REPO_TRANSPORT_TRUST_RE, refused rather than pinned because the per-URL spellings (http.<url>.sslVerify, http.<url>.proxy) are repo-named — no key to override, the same reason credential.<url>.helper is refused. Both the bare and per-URL forms are covered by one pattern so they cannot diverge.

Scoped to trust-relevant keys only, with a test pinning that http.postBuffer and friends do not refuse — a blanket ^http\..*$ would stop updates for any checkout that merely tuned a transfer knob. Mutation-verified in all three directions, including the over-refusal one (the first version of that mutation didn't actually over-refuse, so I redid it until it did).

3. "Revert this gate change" — rebutted (12th recurrence)

Same demand as rounds 3, 6, 7, 8, 12, 13, 14, 16, 17, 19 and 21, now phrased as "revert until unattended updates require a checkout-independent source allowlist". Reverting restores the three-month silent no-op in which no git-clone install auto-updated at all. updates.source is the existing operator-facing allowlist and is enforced on this path before the fetch.

4. "Hard reset races the clean-tree checks" — rebutted (5th recurrence)

Fix: Replace the reset with a fast-forward operation, dispositioned in rounds 12, 14, 16 and 19. The residual is inherent, not closable by more checking: a checkout is a live filesystem, so any check-then-act has a window. What is available is to shrink the window and make the destructive step target something immutable, which is done — the config/worktree probe, ahead-count, hidden-edit scan and collision scan all re-read immediately before the reset, and the reset resolves a captured OID. An editor saving during those milliseconds is the same exposure git reset --hard has when a human runs it.

---

Standing note on convergence

Round 13. The two fixes above are real and I am glad they were caught. But the pattern is now stable enough to state plainly: each round names one or two more git configuration keys or platform-specific path predicates, they are genuine, and there is no principled end to the list — git's configuration space is the attack surface, and I have now closed exec drivers, proxies, URL rewrites, transport helpers, TLS trust, replace refs, submodule recursion, index bits, PATH resolution for git and its helpers, and four separate holes in one collision guard.

Every one of those was worth fixing. But deciding when the hardening is sufficient to ship is a judgment call for someone with merge authority, not something I should settle by out-lasting the reviewer. I will keep fixing what is real if asked; I am flagging that "no findings" may not be a reachable state here.

---

Local gates on e0a4b18c3: pytest 1005 (+23 skips) under xdist -n 4, flake8, isort (full CI path list), mypy (1099 files), black baseline, subprocess-encoding gate. Single commit, 0 behind origin/main.

[iamwhatever]

Dispositions — GPT 5.6 round on e0a4b18c3

Head is now 5c6714b03. One fixed (as a class, not an instance), one rebutted.

1. git status can escape the test repository — fixed, and the class is now ratcheted

Correct. GPT named the call at test_governance_updates.py:1615, a git status --porcelain with no env=, where an inherited GIT_DIR would point it at an operator's real repository and run THAT repo's configured fsmonitor — a host side effect from running the suite.

There were seven, not one. I scanned the file rather than patching the named line:

line  407  git symbolic-ref --short refs/remotes/origin/HEAD
line  637  git rev-parse --show-toplevel
line  712  git -C <repo> config --includes --get filter.evil.clean
line 1582  git rev-parse HEAD
line 1596  git rev-parse HEAD
line 1615  git status --porcelain          ← the one reported
line 1642  git rev-parse HEAD

All seven now build from _fixture_git_env. One needed different treatment: the origin/HEAD probe targets this checkout rather than a tmp_path one, so repo is not even in scope there — it takes the same sanitizing builder rooted at the real path, and the comment says why so the next reader does not "fix" it back.

This is the fourth round of this same no-test-side-effects class — GIT_TEMPLATE_DIR (round 12), GIT_DIR in the fixtures (13), a control probe reaching for raw os.environ (16), and now these. Every time I fixed the instance that was named. So this round also adds a ratchet: TestNoTestSideEffects parses this file and fails if any subprocess.run lacks an env=. Mutation-verified — stripping env from one probe fails it.

That is the part I should have done three rounds ago. A class that recurs four times is not four mistakes, it is one missing check.

2. "Revert enabling main" — rebutted (13th recurrence)

Unchanged from rounds 3, 6, 7, 8, 12, 13, 14, 16, 17, 19, 21 and 23. The demanded change restores the three-month silent no-op this PR exists to remove; updates.source is the existing operator-facing pin and is enforced before the fetch.

---

Status

Every non-review check is green or pending on this head; GPT 5.6 Review is the only failing gate, and its blocking set for this round was one test-hygiene issue (now fixed and ratcheted) plus the standing revert demand.

I am not going to keep going indefinitely on my own judgement. Thirteen rounds in, the hardening has been real and I would defend every fix, but the remaining blocker is a demand I have declined thirteen times on grounds that have not changed. That needs a human decision — either an /ai-review override gpt <sha> recording the judgment that the enablement is acceptable with this much hardening, or an explicit decision to change the design (e.g. gate auto-update behind an opt-in for git-clone installs, which would be a different PR).

I have deliberately not posted an override myself: it records a repository writer's judgment under their name, and that is not mine to sign.

---

Local gates on 5c6714b03: pytest 1006 (+23 skips) under xdist -n 4, flake8, isort (full CI path list), mypy (1099 files), black baseline, subprocess-encoding gate. Single commit, 0 behind origin/main.

[iamwhatever]

/ai-review override gpt 5c6714b: Removing main reverts this PR's entire purpose — it is the OSS default branch, and its absence is the three-month bug that left every git-clone install silently never auto-updating; the remote-trust residual is bounded by the existing updates.source operator pin (enforced before the fetch) and by 14 rounds of hardening on this path, and an agent that can rewrite .git/config can already write the source tree pip install -e . installs.

[github-actions]

Human judgment recorded

@iamwhatever marked the gpt AI finding as false positive, not applicable, or explicitly accepted for 5c6714b03c2b50d798b08c5f0cc0c9fe7d89960b.

Removing main reverts this PR's entire purpose — it is the OSS default branch, and its absence is the three-month bug that left every git-clone install silently never auto-updating; the remote-trust residual is bounded by the existing updates.source operator pin (enforced before the fetch) and by 14 rounds of hardening on this path, and an agent that can rewrite .git/config can already write the source tree pip install -e . installs.

This decision applies only to this commit. A new push requires a new judgment.