feat(git): bind an agent to a GitHub repo you own — post-creation ownership retrofit (ent#109)

[AndriiPasternak31]

Fixes Abilityai/trinity-enterprise#109
Supersedes Abilityai/trinity-enterprise#230 (its three ACs are folded in and verified here).

Stacked on #1913 (AndriiPasternak31/git-env-seam-fix). This branch is cut from that PR's head, not from dev, so until #1913 merges the diff below also shows its 9 commits. Once #1913 lands they become dev ancestors and this diff cleans itself up.

The stack is functional, not textual. PR 2's step 7 calls recreate_container_with_updated_config() to re-bake GITHUB_REPO — and that re-bake is what #1913 introduces. Before it, that path replayed git env from the old container and never read the DB. A file-overlap check says these two PRs are independent and is wrong: branched off plain dev, this PR's "rebind survives a restart" acceptance criterion verifies green against a mechanism that isn't in the tree.

---

What this does

A tokenless public-template agent — the default Cornelius — accumulates a knowledge base it cannot push anywhere. Until now the only documented escape was create a new agent with fork-to-own and import your data, which throws away the agent's identity, its container, its 180-day name reservation and its history in order to keep a directory.

POST /api/agents/{name}/git/bind-to-own-repo creates a repo the user owns from the agent's current workspace volume (not from its template — that is exactly why it cannot reuse ent#93's copy step), pushes the committed history, repoints origin in place, persists the per-agent PAT and re-bakes the container env.

It is framed as a rebind, not a fork verb. An already-writable agent is an ordinary rebind rather than a refusal — which is what makes AC #3 ("works for any agent") literally true, and is less code than gating that state out.

Owner-only and human-only: reject_agent_principal on top of OwnedAgentByName, because an agent-scoped key resolves to its owner carrying the owner's role, so a role gate alone is satisfied by any agent's injected key on a default admin-owned install.

Design decisions worth reviewing

• Classification partitions on source_mode, the column idx_git_config_repo_branch_unique actually keys on — not on write-credential state, which is an orthogonal column. A credential-less source_mode = 0 row would pass a credentials gate and be rebound inside the unique index. Everything unsupported is refused by name (BIND_NO_GIT_CONFIG, BIND_WORKING_BRANCH_MODE_UNSUPPORTED, BIND_STATE_UNCLASSIFIED).
• The lock is destination-scoped, agent:bind_dest:{sha256(lower(dest))}. The real race is two different agents targeting one repo, which no per-agent lock serializes. It fails CLOSED (503 + Retry-After), unlike agent:data_op: — a lost lock here means two repo creates and two concurrent recreates of one container.
• The commit point is a single CAS (rebind_git_config, predicate stated in its docstring). The post-write loser is restored from captured previous values — never delete_git_config. ent#93 may delete because its row was INSERTed microseconds earlier; on a pre-existing row that strips a live agent's binding, and the next recreate brings it back with no repo (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 class).
• The PAT is persisted LAST, but strictly before the recreate. Earlier makes _agent_has_write_credentials report the agent already-writable on a retry, and lets a mid-window manual Push hit the OLD repo with the NEW token. Later bakes a repo-bound container with no token, because the config-drift recreate resolves the PAT with pat_gate="per_agent_only" and startup.sh's configure_push_remote would then blackhole its push remote.
• The recreate is mandatory, and is not a re-provision. startup.sh's restart branch rewrites origin unconditionally from baked GITHUB_REPO, so a DB-only rebind is silently reverted by the next plain restart. Same volumes are reused via the volume_base_name pin (bug: #1581 orphan-volume sweep can destroy a live renamed agent's home volume (rename keeps old volume name+label) #1664), so ent#230's AC Feature/gemini runtime support #2 (the S4 persistent-state allowlist survives) holds by construction, not by copying.
• Resumption is an explicit branch, not assumed idempotence. See "What review caught" below.
• Decision Client/Viewer User Role (AUTH-002) #17's drift predicate was cut. The only drift-proof way to write check_github_repo_env_matches is to call _apply_git_env_from_db itself — which turns fix(lifecycle): re-derive git env from the DB on config-drift recreate (ent#109) #1913's AST writer-set guard red. An independent re-implementation is precisely the writer/checker feedback loop lifecycle.py documents against. Idempotent retry supplies the convergence instead.

What review caught (and how)

/review C1 — critical, empirically proven, fixed. Every post-commit failure message promised an idempotent retry, and all four were refused. The CAS is the commit point, so after it the DB row names the destination while the container's origin still names the old repo — and both pre-flight gates read that skew as a refusal: _classify (origin ≠ row → BIND_STATE_UNCLASSIFIED), and the destination policy (branches present → BIND_DESTINATION_EXISTS, where those branches are the agent's own pushed history). The tell was that _classify(agent_name, destination_repo) never used destination_repo — the carve-out had been designed and not written.

Fixed by treating "row already names this destination" as a resumption that relaxes both gates. That is safe because origin never selects what is pushed (the push is by explicit URL; origin is written afterwards) and the push carries no --force / + refspec, so git rejects unrelated history non-fast-forward — the branches gate was UX, not integrity.

Why it shipped green the first time: the regression test written for exactly this derived the container's origin from the DB row (origin_repo=fake_db.config.github_repo), so the two stores could never disagree — and disagreement was the entire subject. A hand-set dest_state = "empty" between the two calls stepped around the other gate.

/cso S1 — HIGH, verified end-to-end, fixed. A GitHub PAT is sent as Authorization: Bearer <pat>, and h11 rejects an illegal header value by echoing it — confirmed against real httpx: LocalProtocolError: Illegal header value b'Bearer ghp_REAL\r'. The validator only checked non-emptiness and returned the value unstripped, so a token with a trailing \r/\n — a routine paste artifact, not an attack — reached the 500 body and the Vector-captured platform log.

The obvious fix is a lateral move on its own: Pydantic v2 puts the rejected value in errors()["input"] and FastAPI returns exc.errors() verbatim, so a model-level rejection merely relocates the secret into the 422. Both halves landed together, proven against a real TestClient before either shipped: models._validate_pat_secret (strip, then printable-ASCII only, on both BindAgentRepoRequest and ForkToOwnRequest) plus error_handlers.validation_error_without_input, which strips input from every 422 entry.

Also fixed in the same pass: the bind is recreate_container_with_updated_config's second production call site and was skipping clear_agent_breakers (#1560 — both breakers are name-keyed with no TTL, so the replacement container inherits its predecessor's verdict); audit rows on both idempotency-replay exits; the status endpoint wired into the client-timeout branch.

Shared with ent#93 (AC #4)

fork_to_own.inspect_or_create_destination_repo() returns created | empty | branches and never decides. Reuse/refuse policy stays in each caller, because the create path's reuse branch is its template-tip SHA comparison and the rebind has no template to compare against. validate_destination_pat is a sibling rather than folded in, preserving the create path's validate-before-resolve-template ordering.

Both no_write_credentials workaround strings retired

The backend refusal (git_service.NO_WRITE_CREDENTIALS_MESSAGE) and the MCP hint (tools/git.ts) no longer teach create a new agent and import your data — they point at Bind to your own repo on the agent's own Git tab. Kept in sync per Invariant #13 by tests/unit/test_ent109_no_write_credentials_message.py. test_ent123_tokenless_clone.py was re-anchored onto the constant rather than its literal copy, so the copy has exactly one owner.

Verification

All figures below were observed from real runs, not inferred.

• 134 ent#109 tests pass across 6 modules (5 new + the fix(lifecycle): re-derive git env from the DB on config-drift recreate (ent#109) #1913 seam module); all 5 new modules registered in tests/registry.json.
• Full backend unit suite: 6373 passed, 1 failed, 16 skipped (6m09s). The single failure is test_1069_voip_call_path_param::test_flat_path_params_are_agent_name_not_name → ImportError: cannot import name 'get_flat_dependant' from 'fastapi.dependencies.utils', a fastapi-pin drift in the local venv with no relation to routers/git.py. Reproduced byte-identically by a second independent run.
• /verify-local FULL mode — PASS (exit 0), agent stage included. preflight ✔ · agent-precheck ✔ · backend build + import main ✔ · agent base build + import agent_server ✔ · boot+health ✔ · agent-exercise ✔ (real container verify-smoke-… reached /health → {"status":"healthy","runtime":"claude-code","clone_status":"ok"}, correct network attachment) · integration ✔ (70 passed, 13 skipped, 2 deselected). Unit stage skipped deliberately, since it had already been run directly (above) and its known false-fail fail-fasts the pipeline before the build/boot/agent stages.
• configure_push_remote proved separately — and this matters. verify-local's agent stage boots a local:test-echo agent with no GITHUB_REPO/GITHUB_PAT, so GIT_SYNC_ENABLED is never true and every startup.sh branch behind that gate is structurally unreachable. A passing agent stage therefore does not prove this PR's git-env premise. The function was extracted from /app/startup.sh inside the built image and both branches exercised against a throwaway repo: tokenless → push blackholed; GITHUB_PAT baked → blackhole cleared; idempotent on a second start. The extraction anchor was asserted non-empty (an empty body would make every assertion pass vacuously), and the image was tied to the branch by sha (a4788b71… host == image).
• No agent-side change at all: git diff origin/dev...HEAD -- docker/ returns 0 files, and the agent base-image build was a cache hit.
• Design-system contract (Design system v2: shared UI primitives, raw-color ratchet, and written style guide #1430): scan-raw-colors.mjs exits 0 — no ratchet regression. BindRepoPanel.vue at raw_nongray 0; GitPanel.vue unchanged at its baseline 24/146/0. raw-color-baseline.json is not touched by this PR, so the ratchet is honest rather than rebaselined.
• Merges clean against dev @ c4c83f4a.

Notes for the reviewer

• The linked issue is on the private tracker, so GitHub's auto-close and the status-in-dev promotion (issue-status-on-merge.yml) are same-repo only — ent#109 and ent#230 both need a manual status bump / close at release.
• stores/agents.js uses raw axios rather than the shared api.js instance, matching the surrounding idiom in that file (no store in the codebase imports api.js for git calls). The timeout override is mandatory either way — api.js hard-codes 30s, well below this call's budget, and would abort after the commit point straight into the 504 recovery path.
• No VERSION bump and no CHANGELOG entry, per the release process.

🤖 Generated with Claude Code

[AndriiPasternak31]

⚠️ Reviewer: I dismissed two CodeQL alerts on this PR — please sanity-check that call

Flagging this explicitly rather than letting a green check speak for itself, because the alerts land on the highest-risk lines in a PR whose entire subject is not leaking a PAT. If you disagree, re-open in one call (command at the bottom).

Dismissed as false positive:

Alert	Location	Rule
#274	src/backend/routers/git.py:989	py/clear-text-logging-sensitive-data
#275	src/backend/services/agent_service/repo_binding.py:435	py/clear-text-logging-sensitive-data

Why they fire. Both flagged expressions are the output of the scrubber, not the secret:

# routers/git.py:986-990
safe = scrub_secret_and_urls(str(e), body.github_pat.get_secret_value())
logger.error("repo-bind: unexpected failure for %s (%s): %s", agent_name, type(e).__name__, safe)

The PAT reaches these lines only as the argument naming what to strip. CodeQL tracks taint into the call and has no sanitizer model for a custom scrubber, so adding the guard is what created the alert.

Evidence the scrub actually works — scrub_secret_and_urls = redact_url_userinfo(scrub_secret(text, secret)), run against four real leak shapes:

Input shape	Output
h11 header echo (the /cso S1 finding)	Illegal header value b'Bearer ***\r'
git remote URL userinfo	https://***@github.com/o/r.git/
stale baked token (not the request PAT)	https://***@github.com
bare token in text	denied for token ***

No leak in any. The stale-token case is why both passes exist — scrub_secret alone would miss a credential that isn't the caller's. Covered by test_ent109_bind_endpoint.py::test_stale_baked_token_is_redacted_not_just_the_request_pat.

Why not a code barrier instead. py/path-injection has a modeled SafeAccessCheck a real guard can satisfy (how #1455 was fixed). This rule has no equivalent. The only ways to silence it are to stop passing the secret to the scrubber (breaking the scrubbing) or rename the variable out of the sensitive-name heuristic (gaming the linter in security code). Both are worse than the alert.

To re-open if you disagree:

gh api -X PATCH repos/Abilityai/trinity/code-scanning/alerts/274 -f state=open
gh api -X PATCH repos/Abilityai/trinity/code-scanning/alerts/275 -f state=open

[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 ✅ · 32 files (post-rebase, under the 50 threshold) ✅ · Fixes Abilityai/trinity-enterprise#109 resolves ✅
• Security scan clean. ghp_THE_USERS_REAL_TOKEN is a leak-assertion fixture, not a credential — the tests assert it does NOT appear in responses/logs ✅
• MCP third surface updated (src/mcp-server/src/tools/git.ts) — Invariant #13 ✅
• Docs: requirements §11.12 + architecture + 8 feature flows ✅ · 9 test files ✅

error_handlers.py is packaged. docker/backend/Dockerfile:83 is COPY ../../src/backend/*.py /app/ — a glob, changed to exactly this after #1033 — so a new top-level module is picked up, and chmod -R 644 /app/*.py covers it. No packaging gap.

Three things I verified rather than took on trust:

1. The 422 leak is real and this closes it, not relocates it. Pydantic v2 puts the rejected value in errors()[i]["input"] and FastAPI returns exc.errors() verbatim, so the PAT charset guard alone would have moved the leak from a 500 into a 422. Dropping input/ctx for every field rather than name-matching is the right shape — an allowlist is the new-producer-missing-from-the-list class, and the caller already has the value they sent.
2. The bind endpoint is owner-only AND human-only. OwnedAgentByName (uniform 404, Invariant #8) plus reject_agent_principal — necessary because an agent-scoped key resolves to its owner carrying the owner's role, which on a default admin-owned install is the whole fleet (trinity-ops-agent#232).
3. The bind locks fail CLOSED, and the destination lock is the one that matters. A per-agent lock never serializes two different agents binding one destination repo; keying on sha256(lower(dest)) also handles GitHub's case-insensitive slugs. Diverging from _agent_data_op_lock's fail-open is correct — a lost lock here means two repo creations, two CAS writes and two concurrent container recreates.

Conflict resolution (mine). #1913 was squash-merged, so this branch's 9 shared commits stopped being ancestors and the stack surfaced as 5 conflicts. Resolved additively and pushed as e6e82a34: lifecycle.py docstring → this branch's superset (it names repo_binding's bind path as the second caller); architecture.md → both; feature-flows.md → both rows, newest-first (ent#109 08-02 above #1932 08-01); learnings.md → both (dev added nothing in the hunk); registry.json → this branch's 5 entries, dev's 104 retained (109 total). Verified zero markers tree-wide, registry parses, lifecycle.py compiles, and git diff origin/dev shows no deletions in any of the four docs. Diff went 36 files/+6001 → 32/+5034, which is #1913's content correctly dropping out.

⚠️ Cross-tracker: ent#109 needs status-in-dev by hand.