fix(github-pat): make a global PAT rotation actually reach running agents (#1967)

[dolho]

Problem

Rotating the global GitHub PAT in Settings reported success while updating nothing. Agents kept authenticating with the revoked token — git fetch/push and the gh CLI all failing — for as long as their containers stayed up. On the reporting fleet, 11–13 days.

Two independent gaps, each of which fully defeated the rotation on its own. Both existed for the same reason: the global path and the per-agent path (#1264) had drifted apart.

1. The eligibility gate was .env-shaped.

status = await _apply_pat_to_env(..., add_if_missing=False)   # global path

Returns skipped_no_pat when /home/developer/.env is absent. That is every agent provisioned from a GitHub template, since those ship .env.example and no .env. On such a fleet 100% of agents skipped — and the endpoint still answered success: true.

2. .env is not where git authenticates from.

Clones are created as https://oauth2:<PAT>@github.com/<org>/<repo>.git, and that URL is persisted in .git/config on the workspace volume. Rewriting .env changes nothing for the running git process. Only re-templating the remote restores fetch/push before a restart — which the per-agent path already did and this one did not.

So even with gap 1 fixed, a rotation still would not restore git access.

Fix

Both paths now route through one _apply_pat_to_agent body (.env write plus live remote rewrite), so the next divergence has to be deliberate rather than accidental. That drift is the bug; deduplicating it is the actual fix, not a tidy-up.

The part I did differently from the issue, and why

The issue suggests add_if_missing=True unconditionally. I did not do that. It would inject the global token into every running container, including agents that never touched GitHub — and this module's own docstring says the original gate existed to prevent exactly that. Trading a silent-skip bug for a credential-spraying one is not an improvement.

Eligibility moved to "does Trinity manage a repo for this agent" (the issue's own alternative), which targets precisely the population that was being skipped:

Agent	Before	After
git config, no .env (template-provisioned)	skipped_no_pat ❌	updated + remote re-templated
git config, has .env	updated (.env only)	updated + remote re-templated
no git config, has GITHUB_PAT line	updated	updated (unchanged)
no git config, no GITHUB_PAT line	skipped	skipped (unchanged — not sprayed)

The two rules are a union, not a replacement: an agent carrying GITHUB_PAT for the gh CLI with no Trinity-managed repo is still rotated exactly as before. That row has its own no-regression test, because "fix the gate" is the obvious place to accidentally drop a working case.

remotes_updated is reported separately from updated

updated alone overstates the fix: an agent whose .env was rewritten but whose remote was not is still broken for git until it restarts. Collapsing the two would repeat this issue's own mistake — reporting success for a partial effect.

Both degraded shapes now also WARN in the platform log (zero-reach, and updated-without-remote). The failure was silent for weeks; a Settings panel nobody is watching during an incident is not sufficient.

Gap 3 is already fixed on dev

The issue says the propagation result "is never surfaced". It is — Settings.vue renders the full updated/failed/skipped breakdown with per-agent reasons. The issue was filed against 9f766d11.

One residual did remain, and is fixed here: 0 of N applied rendered success-green whenever nothing outright failed, so a rotation that reached no agent looked identical to one that worked. It now renders as an error, and the remote-vs-env gap gets its own warning line.

Not addressed (deliberate)

The issue's "secondary observation" — an agent whose per-agent PAT holds a stale copy of the old global token is skipped forever and re-injected on every recreate. Detecting that needs a validity probe against the GitHub API per agent, which is a different feature (network calls in a rotation path, rate limits, a new failure mode) rather than part of this fix. Worth its own issue; flagging rather than silently expanding scope.

Verification

tests/unit/test_1967_global_pat_propagation.py — 13 checks, 7 of which fail against the pre-fix tree. The 6 that pass either way are the no-regression guards, which is the correct result for them.

$ git stash push -- src/ && pytest unit/test_1967_global_pat_propagation.py -q
7 failed, 6 passed
$ git stash pop && pytest unit/test_1967_global_pat_propagation.py -q
13 passed

Also green: test_1264_per_agent_pat_propagation, test_github_pat_fallback, test_github_pat_recreation, test_ent162_per_user_github_pat, test_models_centralized — 63 passed. The #1264 tests passing unchanged is the useful signal: the shared-body refactor did not alter the per-agent path's behaviour.

Two structural guards are included because the drift is the root cause: one asserts both callers reach _apply_pat_to_agent, one asserts add_if_missing=False is not reintroduced on the global path.

Acceptance criteria

• Global rotation reaches agents with a git config but no .env
• Global rotation re-templates the live git remote, so git works without a restart
• The result distinguishes ".env written" from "remote fixed", and a zero-reach rotation is loud

Related to #1967 · follow-up to #211, #1264, #1574

🤖 Generated with Claude Code

Fixes #1967

[vybe]

Validated via /validate-pr — well-scoped P1 fix with 7/13 regression tests failing pre-fix, per-agent/per-user PAT architecture preserved (skipped_per_agent_pat gate has its own test), no credential-spraying over-correction. I added the missing 'Fixes #1967' keyword, resolved the registry.json conflict (rebuilt as dev entries + this PR's entry, deduping a PRE-EXISTING duplicate test_181 entry that's been on dev since before today), and full CI re-ran green (initial run was cancelled by a transient runner issue; rerun all-green). Follow-ups filed: #1995 (stale per-agent PAT copy + github-sync.md drift).