Skip to content

fix(git-sync): fork-to-own and repo binding use the saved personal GitHub token (#3164) - #3235

Open
dolho wants to merge 1 commit into
devfrom
fix/3164-fork-uses-saved-pat
Open

dolho wants to merge 1 commit into
devfrom
fix/3164-fork-uses-saved-pat

Conversation

@dolho

@dolho dolho commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

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, tier per_user), but it only used it to read the template.

  • Optional token. ForkToOwnRequest.github_pat and BindAgentRepoRequest.github_pat are now optional. A supplied token still wins and is still charset-validated; a blank token is still rejected.
  • Fallback to the saved personal token:
    • Fork-to-own uses the resolver's per_user token (crud._apply_fork_to_own).
    • Binding reads the caller's own saved token by user id (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.
    • Either way the token is persisted as the agent's per-agent PAT with tier fork, exactly as before.
  • The platform token is never a fork or bind identity. With no personal token, the request gets a named 400, 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).
  • Saved-token failures are named. If the saved token is refused (invalid, can't create the repo, can't push), the response carries token_source: "saved" and the message says to update it in Settings.
  • UI. The new SavedGithubTokenField.vue is used in the create form and the bind panel. It reads presence only from the existing GET /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.
  • MCP: unchanged (fork stays UI-only, ent#15).

Changes

  • models.py, services/agent_service/{crud,fork_to_own}.py, routers/git.py: the optional token, the fallback, FORK_PAT_REQUIRED and the saved-token wording
  • stores/auth.js (fetchGithubPatStatus), SavedGithubTokenField.vue (new), CreateAgentModal.vue, BindRepoPanel.vue
  • raw-color-baseline.json: two entries lowered by hand (BindRepoPanel 46 → 41, CreateAgentModal 135 → 126) with a _3164_note. Not regenerated, because that would drop the refrozen block.
  • Docs: feature-flows/github-import-intents.md, requirements/github.md

Test 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 400 FORK_PAT_REQUIRED and 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.
  • 31 related backend files (fork, bind, create): 879 passed, also under -p randomly --randomly-seed=12345.
  • savedGithubToken.mount.spec.js: 7 mounted tests. The 5 payload tests fail without the wiring.
  • Full npm run test:unit: all passed, including the ratchets.
  • Against a running stack built from this branch (an isolated verify-local stack, fake tokens seeded directly):
    • saved token + no form token: 400 FORK_PAT_INVALID, token_source: saved, saved-token wording, and the token is not in the response;
    • platform token only: 400 FORK_PAT_REQUIRED;
    • the presence route returns configured only;
    • no agents were created.
    • A genuinely token-less run wasn't exercised, because the stack's env still supplied a platform token.
  • Manual, needs a real token (creates a repo): save a personal token, fork a template without typing one, and check the repo is created under your account and the agent pushes.

Fixes #3164

🤖 Generated with Claude Code

…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>
@dolho dolho added the ui PR touches the frontend UI — triggers Playwright e2e tests label Oct 5, 2026
@vybe

vybe commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

merge-train (2026-10-05, evening run): not on this train. It rides the next one once fixed. CI is green at 512381fc; these were found by tracing the saved-token path, which no test drives through an error or an empty override.

Blocking

  1. The unexpected-error handler crashes on the saved-token path. src/backend/routers/git.py:1101 still calls body.github_pat.get_secret_value(), and body.github_pat is None when the saved token is used. Any unexpected bind error then raises AttributeError before the scrub runs, so the BIND_UNEXPECTED_ERROR audit row and idempotency_service.fail(idem) are both skipped and the key stays in flight. Pass user_pat, and add a saved-token case for that branch.
  2. "Use a different token" with an empty field sends the saved token. overriding lives only inside SavedGithubTokenField.vue (:63-64); BindRepoPanel.vue:311 and CreateAgentModal.vue:549 guard on !token && !hasSavedGithubPat alone. Choosing the override and submitting it empty binds the saved token. The retry case is the sharp one: type a narrow token, the first submit fails, the panel clears pat by design, the field still shows override mode, and the resubmit persists the broad saved token as the agent's credential. No spec covers override-and-empty. How the parent learns about the override is your call, which is why this was not fixed on the branch.

Needs a decision before it lands

  • Admin binding another user's agent. OwnedAgentByName admits an admin, and git.py:994 reads db.get_user_github_pat(current_user.id), so the admin's personal token becomes that agent's PAT by default. Before this PR that needed a typed token. Either restrict the fallback to caller-is-owner or document the behaviour.
  • Fork-to-own from an agent key. POST /api/agents is require_role("creator"), which agent keys satisfy as their owner, so an agent can now have a repo created in the owner's GitHub account with the saved token (crud.py:905). Bind is human-only for this reason. If fork should be too, the saved branch needs a not current_user.agent_name check.

Smaller

  • SavedGithubTokenField.vue:36-47 hand-rolls its password input; the design-system contract asks for BaseInput.
  • Stale docs: architecture/agent-lifecycle.md:191, feature-flows/agent-repo-binding.md:68, feature-flows/template-processing.md:890 still describe the token as always present or model-validated.
  • The presence check is "column not null" while the read returns None when the value cannot be decrypted, so the UI can say "using saved" and then get FORK_PAT_REQUIRED.

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.

@vybe vybe added the status-needs-fix PR has an unaddressed review/validation finding; cleared by the author's next push (#2815) label Oct 5, 2026

@AndriiPasternak31 AndriiPasternak31 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 bind fixture: bind.saved = SAVED, then _bind raises RuntimeError("header value b'Bearer ghp_saved_personal' is illegal"). The result is AttributeError: '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 the Bearer <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_TIERS includes per_agent, but both create-path resolver calls pass only owner_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 passes agent_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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status-needs-fix PR has an unaddressed review/validation finding; cleared by the author's next push (#2815) ui PR touches the frontend UI — triggers Playwright e2e tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants