Conversation
…tHub token (#3164) Fork-to-own and the post-creation repo binding required a GitHub token in the form even when the user had a personal token saved in Settings; the create path already resolved it (resolve_github_pat, tier per_user) but only used it to read the template. - ForkToOwnRequest.github_pat / BindAgentRepoRequest.github_pat are optional; a supplied token still wins and is still charset-validated. - Omitted: fork-to-own uses the resolver's per_user token; binding reads the caller's own saved token by user id (never the agent's current per-agent token). Persisted as the agent's per-agent PAT, tier `fork`, as before. - The platform token is never a fork or bind identity: with no personal token the request is a named 400 FORK_PAT_REQUIRED pointing at Settings (ent#162 Decision 2 — the global PAT must not land in a per-agent row). - A token refusal raised while using the saved token says so (token_source "saved", message naming Settings). - UI: SavedGithubTokenField shows "Using your saved GitHub token" with an explicit override, presence read from GET /api/users/me/github-pat; the field is required only when nothing is saved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
merge-train (2026-10-05, evening run): not on this train. It rides the next one once fixed. CI is green at Blocking
Needs a decision before it lands
Smaller
What held up: precedence (form, then saved, never platform), the field names on both sides, the raw-colour baseline (both files shrank), and the frontend never receives the token value. |
AndriiPasternak31
left a comment
There was a problem hiding this comment.
Independent /review + /validate-pr pass at 512381f. Same head as vybe's merge-train comment, so his four items are still open. I confirmed each one and have two additions.
Confirmed, with evidence
- git.py:1101 crash (vybe #1). Reproduced with your own
bindfixture:bind.saved = SAVED, then_bindraisesRuntimeError("header value b'Bearer ghp_saved_personal' is illegal"). The result isAttributeError: 'NoneType' object has no attribute 'get_secret_value'at git.py:1101. The form-token control passes.
Addition: the audit row and the idempotency release are both skipped, and there's a second problem. main.py has no catch-all exception handler, so the AttributeError's chained traceback ("During handling of the above exception…") logs the original exception text unscrubbed. That's theBearer <PAT>echo the comment above line 1101 says this branch exists to keep out of the Vector log.
Fix:scrub_secret_and_urls(str(e), user_pat), plus a saved-token test for that branch. The assert I used:
assert e.value.status_code == 500 and SAVED not in str(e.value.detail) - Override + empty field sends the saved token (vybe #2). BindRepoPanel.vue:311 and CreateAgentModal.vue:549 can't see
overriding(SavedGithubTokenField.vue:63). Emitting the override state and guarding on!token && (!hasSaved || overriding)closes it. One mount spec per form for override+empty, please. - Admin bind (D1) and agent-key fork (D2). Both reachable as described: OwnedAgentByName admits admin (dependencies.py:1755), and POST /api/agents is require_role("creator") with no agent_name check on fork_to_own. For D1, note that resolve_github_pat's own docstring says resolution "keys on ownership only, never on a calling/sharing user" (settings_service.py:1019). Keying the bind fallback on owner == caller would match that.
New
- docs/memory/architecture/agent-lifecycle.md:191 still says the user PAT is header-validated at the model for both request types. The saved token is a plain str from db.get_user_github_pat and never passes
_validate_pat_secret. It's validated at save time instead (strip + GitHub round-trip, users.py:225), and the doc should say that. The ent#109 bind section there also doesn't mention the fallback yet. SAVED_TOKEN_TIERSincludesper_agent, but both create-path resolver calls pass onlyowner_id(crud.py:706,724), so that tier can't occur there. Either drop it or add a comment saying why it's safe if a future caller passesagent_name.
What holds up: reverting the crud and router hunks turns 7 of the 14 new tests red. Adding "global" to SAVED_TOKEN_TIERS turns the global-tier test red. 380 related fork/bind/PAT unit tests pass locally.
The two CANCELLED checks are benign. The first frontend-e2e run was superseded under cancel-in-progress when the ui label and the PR open fired two seconds apart. The superseding run on the same SHA is green, and report only runs on schedule.
Summary
Fork-to-own and the post-creation repo binding always asked for a GitHub token in the form, even when the user had a personal token saved in Settings. The create path already resolved that token (
resolve_github_pat, tierper_user), but it only used it to read the template.ForkToOwnRequest.github_patandBindAgentRepoRequest.github_patare now optional. A supplied token still wins and is still charset-validated; a blank token is still rejected.per_usertoken (crud._apply_fork_to_own).db.get_user_github_pat). It never uses the agent's current per-agent token, which is the agent's identity, not the binding person's.fork, exactly as before.FORK_PAT_REQUIRED, which points at Settings. Otherwise the destination would belong to the platform account, and the per-agent row would hold the global PAT (ent#162 Decision 2).token_source: "saved"and the message says to update it in Settings.SavedGithubTokenField.vueis used in the create form and the bind panel. It reads presence only from the existingGET /api/users/me/github-pat(no new endpoint, never the value). With a saved token it shows "Using your saved GitHub token" and an explicit "Use a different token" override, and the request carries no token. Without one, the field is required as before.Changes
models.py,services/agent_service/{crud,fork_to_own}.py,routers/git.py: the optional token, the fallback,FORK_PAT_REQUIREDand the saved-token wordingstores/auth.js(fetchGithubPatStatus),SavedGithubTokenField.vue(new),CreateAgentModal.vue,BindRepoPanel.vueraw-color-baseline.json: two entries lowered by hand (BindRepoPanel 46 → 41, CreateAgentModal 135 → 126) with a_3164_note. Not regenerated, because that would drop therefrozenblock.feature-flows/github-import-intents.md,requirements/github.mdTest Plan
tests/unit/test_3164_fork_uses_saved_pat.py: 14 pass. It covers the AC cases (a) saved token + no form token forks with the saved one, (b) global-only gives 400FORK_PAT_REQUIREDand nothing reaches GitHub, (c) a form token beats a saved one, and (d) the same three for binding. It also covers the saved-token error wording and the models.-p randomly --randomly-seed=12345.savedGithubToken.mount.spec.js: 7 mounted tests. The 5 payload tests fail without the wiring.npm run test:unit: all passed, including the ratchets.verify-localstack, fake tokens seeded directly):FORK_PAT_INVALID,token_source: saved, saved-token wording, and the token is not in the response;FORK_PAT_REQUIRED;configuredonly;Fixes #3164
🤖 Generated with Claude Code