fix(lifecycle): re-derive git env from the DB on config-drift recreate (ent#109)

[AndriiPasternak31]

Blast radius first

This changes what every agent container recreate produces, fleet-wide — it is not a refactor.

recreate_container_with_updated_config seeds env_vars from the old container and, before this PR, re-derived only GITHUB_PAT. Everything else in the GitHub-sync block — GITHUB_REPO, GIT_SYNC_ENABLED, GIT_SOURCE_MODE, GIT_SOURCE_BRANCH, GIT_SYNC_AUTO — was replayed from whatever that container happened to be carrying. The actual derivation lived only on the other rebuild path (_apply_persisted_auth_env → recreate_missing_container).

That path has exactly one production caller: start_agent_internal. It fires on nine config-drift predicates and on base-image drift at cold start (#1809) — so a base-image rebuild makes every cold start a recreate, and arms the replay for the entire fleet on the next release. A stale repo binding, a source_mode flip, or a deleted agent_git_config row survived indefinitely.

_apply_git_env_from_db(agent_name, env_vars, *, pat_gate) is now the single owner of that block on both rebuild paths.

Rebased onto #1912 (fix: fleet restart adopts rebuilt base images through the canonical lifecycle path, #1860), which landed on dev mid-review — and which widens this bug's reach. That PR adds restart_agent_internal (explicit stop → start_agent_internal) so a fleet "Restart All" reliably performs a cold start. Cold start is exactly the path that runs the #1809 image-drift predicate and therefore recreate_container_with_updated_config. So after #1912, the git-env replay this PR removes is reachable from an ordinary operator "Restart All", not just from a base-image rebuild. The two changes are additive (no shared writer), and this branch keeps #1912's helper, its registry entry, and its docs rows intact; test_1860_fleet_restart_adoption.py, test_1809_image_drift_recreate.py and both test_1816_* suites pass alongside the ent#109 suite (154 tests).

The two regressions this deliberately prevents

The naive fix — lift the block verbatim from the rebuild-from-nothing path into the config-drift path — ships two bugs. Both are guarded here.

1. Global-PAT injection into tokenless containers (the ent#162 class).
The two paths gate the PAT differently on purpose. _apply_persisted_auth_env uses the 2-tier get_github_pat_for_agent (per-agent → global platform PAT); the config-drift recreate deliberately keeps #211's narrower per-agent-only gate. A verbatim lift would share the broad gate and bake the platform PAT into every previously-tokenless agent on its next drift recreate. configure_push_remote then clears the push blackhole — and a tokenless ent#123 agent (Cornelius) could push a user's private knowledge base to the shared public upstream.

So pat_gate is a required keyword parameter, never a default: per_agent_only for config drift, effective for rebuild-from-nothing. An AST guard pins the writer set — {recreate_container_with_updated_config: per_agent_only, _apply_persisted_auth_env: effective} exactly — so CI fails on a flipped gate and on a third writer appearing on any container-seeded path. That count is load-bearing: ent#109's bug was two writers with one of them wrong. (A source-grep version of this guard stayed green through a planted third effective writer; verified by mutation.)

2. GIT_SYNC_AUTO stripped where the DB flag and baked env disagree.
crud.py's two creation writers genuinely disagree — the DB opt-in carries and not config.ephemeral inside a swallowing try/except while _apply_github_env does not, and the column defaults to 0. So env=true / DB=0 is reachable from one transient DB hiccup at creation and permanently for ghosts. Deriving from the column alone would silently stop auto-push for that slice of the fleet — no error, just a stale agent_sync_state. Hence DB auto_sync_enabled OR baked env.

The OR is derive-only — the column is never written back. A "convergence backfill" was implemented and then removed, because PUT /{agent}/git/auto-sync {enabled:false} writes the row and nothing else while the agent gates on container env — so baked true / DB 0 is also exactly what an owner's explicit disable looks like, and no backfill can tell them apart. It would erase that intent and make the toggle unable to ever stick. It was also a privilege escalation: PUT .../auto-sync is OwnedAgentByName, but POST /api/agents/{name}/start — the recreate's trigger — is AuthorizedAgentByName. The disagreement is logged, not resolved.

Also in scope

• Set-or-clear over _GIT_ENV_KEYS, because the config-drift path writes into a carried-forward dict. A deleted agent_git_config row pops the whole set (including an orphaned GITHUB_PAT — the per-agent token is a column on that row, so no row ⇒ no credential and no repo); a source_mode flip clears the mode/branch pair. GITHUB_PAT stays set-only while a repo is bound, so an unrelated recreate never revokes a live agent's push.
• Correct, never introduce (config-drift path only). The repo half is repo-gated (ent#123) while the PAT half keeps feat: Auto-propagate GitHub PAT to all agents on update #211's per-agent gate, and those two disagree for one real row shape: an agent bound post-creation via POST /{agent}/git/initialize on the global PAT, whose only credential lives in the container's .git/config origin URL. Handing startup.sh GIT_SYNC_ENABLED=true with no GITHUB_PAT is precisely the input it reads as "deliberately tokenless" — the restart branch rewrites origin to the credential-less CLONE_URL, destroying that token, and blackholes the push remote. So this path only ever corrects env the container already carries. effective is exempt: with no old container, not introducing the block is the bug: GET /api/templates omits local templates from config/agent-templates/ #843/bug: GitHub-template agent deploy — race intermittently leaves a silent empty agent (clone into non-empty /home/developer, exit 128, not surfaced) #1439 silently-empty-agent bug.
• Not owned here, deliberately: GIT_WORKING_BRANCH (read only by startup.sh's clone branch, which a recreate never reaches) and GIT_UPSTREAM_REPO (no DB column). Making the Sync health observability (S1) #389 toggle authoritative is a tracked follow-up.

Tests

tests/unit/test_ent109_git_env_seam.py — 22 tests, registered in tests/registry.json. Every behaviour was proved to have teeth by mutation: un-gate the PAT, flip the call site to effective, plant a third writer, derive GIT_SYNC_AUTO DB-only, drop the clear sweep, drop the source-mode clear, restore the backfill, remove the introduce-guard — all go red.

test_github_pat_fallback.py's #735 call-site guard is re-anchored onto _apply_git_env_from_db (its str.find miss previously returned -1, silently slicing from the end of the file and asserting against an empty string).

Verification: full unit suite, deterministic order — 5872 passed, 0 failed (baseline origin/dev: 5850 passed; delta is exactly the 22 new tests). Ruff on the changed files is byte-identical to baseline. tests/lint_sys_modules.py: no new violations.

Pre-existing, not from this PR: test_1081_physical_meter.py ↔ test_telegram_webhook_backfill.py are order-dependent under pytest-randomly. Reproduced identically on pristine origin/dev; my new file provably does not pollute them.

Issue references

Refs Abilityai/trinity-enterprise#109 — context only. This PR is PR 1 of 3 (the env seam); it does not implement the ent#109 feature and must not close it.

No public bug issue exists for this defect — I searched the public tracker by title and body (GITHUB_REPO, GIT_SYNC_AUTO, _apply_persisted_auth_env, recreate_container_with_updated_config) and found only adjacent base-image-drift bugs (#1809, #1816, #1860). Rather than invent a closing reference, this PR carries none; happy to file one and re-point it if you'd prefer the fix tracked publicly.

Not included

No VERSION bump, no changelog. No schema change, no migration, no new config, no endpoint change.

🤖 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.

[vybe]

/validate-pr: PASS.

• Base dev ✅ · 10 files ✅ · security scan clean ✅
• No new top-level backend module. TRINITY_GIT_BASE_URL is pre-existing on dev (already in .env.example, already read by crud.py/git_service.py/startup.sh) — not a new config var, so no #1056-class gap introduced ✅
• Docs: architecture + 4 feature flows ✅
• Test adequacy: test_ent109_git_env_seam.py pins the writer SET with an AST guard rather than a call-site grep — that survives refactors, which a string match would not ✅

The core call is right: recreate_container_with_updated_config re-derived only GITHUB_PAT and replayed whatever GITHUB_REPO/GIT_SYNC_* the old container carried, and since #1809 made base-image drift trigger a cold-start recreate, that replay is fleet-wide. Consolidating both rebuild paths onto one owner is the correct shape.

Two things I checked rather than took on trust:

1. pat_gate is a required keyword, never defaulted. The two paths gate the PAT differently and a shared default would inject a global-only platform PAT into a previously-tokenless container — per_agent_only reproduces #211's guard verbatim.
2. The correct-never-introduce guard is load-bearing, not defensive padding. POST /{agent}/git/initialize writes a git-config row whose only credential lives in the container's .git/config origin URL. Introducing GIT_SYNC_ENABLED=true with no GITHUB_PAT is exactly what startup.sh reads as deliberately-tokenless — it rewrites origin to the credential-less CLONE_URL and blackholes the push remote. Gating the config-drift path on an already-carried repo avoids that; exempting the rebuild-from-nothing path is correct because there NOT introducing the block is the #843/#1439 silently-empty-agent bug.

⚠️ Non-blocking, pre-existing and NOT introduced here: TRINITY_GIT_BASE_URL is in .env.example but in neither compose file's backend.environment:, and the backend service uses an explicit env list with no env_file. So the self-hosted-git override is inert on a compose deploy today. Worth a separate issue; this PR's set-or-clear behaviour is coherent either way.

⚠️ Cross-tracker: ent#109 needs status-in-dev set by hand — issue-status-on-merge.yml is same-repo only.