Skip to content

fix: forward Claude guided login verification code - #1678

Closed
simple-agent-manager[bot] wants to merge 22 commits into
mainfrom
sam/fix-claude-code-guided-gh57hs
Closed

fix: forward Claude guided login verification code#1678
simple-agent-manager[bot] wants to merge 22 commits into
mainfrom
sam/fix-claude-code-guided-gh57hs

Conversation

@simple-agent-manager

@simple-agent-manager simple-agent-manager Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Rebased the Claude guided-login verification-code flow onto current main and reconciled 107 commits of drift. The work is not superseded: main still lacked the verification-code submission route, exchanging state, separate Enter delivery, and server-side completion path.
  • Forward the browser-displayed code#state into the still-running Claude CLI, then capture and save the resulting OAuth token entirely server-side. Claude CLI v2.1.220 receives the pasted code and Enter as separate stdin writes after a configurable 1000 ms settle period so Ink's paste prompt does not absorb Enter.
  • Preserve the newer code_incomplete, code_rejected, and exchange_network_error classification introduced by b26de657f, while tolerating Ink redraw character drops and safely reconstructing PTY-wrapped tokens.
  • Reconcile current-main migration numbering and active-session indexing, make guided-login timing/size limits configurable, prevent stale UI polls from overwriting a submitted state, and update public docs plus two-viewport Playwright coverage.

Alarm candidate volume remains one session per Durable Object. Each tick adds one sandbox state-file read before the existing credential-file check; worst-case work remains bounded by one configured capture poll per active session and TTL teardown.

Human review approval has now been recorded. Raphaël stated on 2026-08-16: "Pretty sure I tested the Claude login and it worked. Merge it if possible." This satisfies the prior paid Claude login verification gate. The PR remains open and unmerged because the PR-backlog parent will handle batch integration and the final merge.

Validation

  • pnpm lint (0 errors; repository baseline warnings only)
  • pnpm typecheck
  • pnpm test (full workspace suite)
  • pnpm build (full workspace production build)
  • Focused API, Worker/Miniflare, web component, migration ordering/safety, and Playwright tests
  • Candidate volume and worst-case per-candidate cost are stated above

Focused evidence includes 79 guided-login API tests, 6 Worker route tests, 11 modal tests, and 4 Playwright cases across iPhone SE 375x667 and desktop 1280x800. The route-to-Durable-Object vertical test exercises POST submission, ownership, the real setup DO, sandbox writes, alarm capture, credential save, teardown, and completed status.

Staging Verification (REQUIRED for all code changes — merge-blocking)

  • Staging deployment green — intentionally not run for this refresh
  • Live app verified via Playwright — intentionally not run against staging for this refresh
  • Existing workflows confirmed working — covered locally; not re-claimed for staging
  • Human manual test recorded — Raphaël stated on 2026-08-16: "Pretty sure I tested the Claude login and it worked. Merge it if possible."
  • Infrastructure verification completed — N/A: no cloud-init, VM-agent, DNS, TLS, or deployment-script changes
  • Mobile and desktop verification notes added for UI changes

Staging Verification Evidence

No staging deployment or mutation was performed for this refresh, at Raphaël's explicit direction. Historical staging evidence from the original PR revision is not claimed as current verification after the rebase. Local component-backed Playwright audits passed at 375x667 and 1280x800, including keyboard submission and visible code_incomplete failure feedback.

  • Human manual test (2026-08-16): Raphaël stated on 2026-08-16: "Pretty sure I tested the Claude login and it worked. Merge it if possible." This is the manual real-Claude login verification that the prior needs-human-review label was waiting for.

UI Compliance Checklist (Required for UI changes)

  • Mobile-first layout verified
  • Accessibility checks completed
  • Shared UI components used
  • Playwright visual audit passed locally at 375x667 and 1280x800 with no horizontal overflow; normal submission and error-feedback scenarios were exercised

End-to-End Verification (Required for multi-component changes)

  • Data flow traced from user input to final outcome with code-path citations
  • Capability coverage exercises the route, service, real Durable Object, sandbox, capture, save, and teardown boundaries
  • Existing behavior and current-main guided-login work verified against code
  • The remaining real-provider gap and manual verification steps are documented below

Data Flow Trace

CodexConnectModal.handleSubmitVerificationCodesubmitAgentCredentialSetupVerificationCode → owned POST /:id/verification-codesubmitSetupSessionVerificationCodeCredentialSetupSession.submitVerificationCode → sandbox verification-code.txtrunClaudeSetupToken PTY stdin (code write, configurable settle, separate Enter write) → CLI OAuth exchange → server-only claude-oauth-token.txtattemptCapturesaveAgentCredentialForUser → teardown.

Untested Gaps

The previous real-provider gap is closed by the human manual test recorded above: Raphaël stated on 2026-08-16: "Pretty sure I tested the Claude login and it worked. Merge it if possible." This refresh still intentionally skips staging deployment by explicit instruction; the parent orchestrator handles batch staging validation/integration.

Post-Mortem (Required for bug fix PRs)

What broke

Guided Claude login displayed the authorization URL but could not complete because the running CLI never received the browser verification code; the flow eventually expired.

Root cause

The original guided flow launched claude setup-token without a working code-return path. A first follow-up modeled a final-token prompt even though Claude's browser provides code#state, and Claude CLI v2.1.220's Ink paste handling absorbs Enter when it is appended to the same large stdin write.

Class of bug

Mock-hidden cross-boundary protocol failure involving browser output, PTY paste semantics, terminal redraw artifacts, asynchronous polling, and server-only token capture.

Why it wasn't caught

Coverage initially stopped at URL display and did not exercise exact stdin sequencing, redraw-corrupted failures, a real route-to-DO boundary, or the full sandbox state-file exchange.

Process fix included in this PR

Exact-write driver regressions, redraw-tolerant classification cases, real route-to-Durable-Object vertical coverage, stale-poll UI coverage, Worker route coverage, and mobile/desktop Playwright keyboard/error scenarios now protect the full contract.

Post-mortem file

tasks/archive/2026-07-26-claude-guided-verification-code.md

Specialist Review Evidence (Required for agent-authored PRs)

  • All local reviewers completed and findings addressed before merge
  • If any reviewer did NOT complete: needs-human-review label added and merge deferred to human
Reviewer Status Outcome
cloudflare-specialist PASS Migration/index semantics, Durable Object lifecycle, alarms, and Worker coverage pass after fixes
security-auditor PASS Ownership, sandbox paths, input limits, secret handling, token parsing, save, and teardown pass after fixes
env-validator PASS guided-login timing/size knobs are declared, defaulted, forwarded, and documented consistently
ui-ux-specialist PASS Mobile/desktop hierarchy, keyboard flow, state transitions, errors, and accessibility verified
doc-sync-validator PASS Public guided-login and configuration docs match the rebased implementation
test-engineer PASS Focused, vertical, Worker, component, and Playwright coverage pass
constitution-validator PASS No-hardcoded-values and configuration-boundary findings addressed
task-completion-validator PASS Acceptance criteria and implementation evidence validated across sections A–F

All local reviews passed. Human review approval is now recorded above, and the needs-human-review label has been removed so Specialist Review Evidence can pass. Final merge remains delegated to the PR-backlog parent/batch integration.

Exceptions (If any)

  • Scope: Staging deployment and current live-staging verification for this refresh
  • Rationale: Raphaël explicitly instructed this refresh not to deploy to or mutate staging
  • Expiration: A future human-authorized merge cycle may perform staging verification; this refresh stops after CI

Agent Preflight (Required)

  • Preflight completed before code changes

Classification

  • external-api-change
  • cross-component-change
  • business-logic-change
  • public-surface-change
  • docs-sync-change
  • security-sensitive-change
  • ui-change
  • infra-change

External References

Reviewed the official documentation for Claude Code authentication for the subscription-login contract, then verified the exact behavior against the current Claude CLI v2.1.220 PTY interaction and existing project tests.

Codebase Impact Analysis

Affected paths include apps/api/scripts/claude-setup-token.mjs; the apps/api setup route, service, Durable Object, credential validation/save boundaries, environment configuration, migration, and tests; apps/web modal behavior and unit/Playwright tests; and apps/www guided-login/configuration documentation. The flow crosses browser UI, Worker route/service, Durable Object state, sandbox PTY/state files, encrypted credential save, and teardown.

Documentation & Specs

Updated apps/www/src/content/docs/docs/guides/agents.md, apps/www/src/content/docs/docs/reference/configuration.md, apps/api/.env.example, and .claude/skills/env-reference/SKILL.md. The archived incident/task remains tasks/archive/2026-07-26-claude-guided-verification-code.md.

Constitution & Risk Check

Checked Principle XI/no hardcoded operational values, credential-boundary rules, ownership enforcement, input and output bounds, migration safety, Durable Object lifecycle behavior, and public-copy consistency. The short-lived browser code is bounded and written only under the per-session setup home; the OAuth token remains server-side and is saved through the existing encrypted credential path. Configurability adds operational flexibility without exposing secrets or weakening teardown.

@simple-agent-manager simple-agent-manager Bot added needs-human-review Agent could not complete all review gates — human must approve before merge and removed needs-human-review Agent could not complete all review gates — human must approve before merge labels Jul 26, 2026
@codspeed-hq

codspeed-hq Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 6 untouched benchmarks


Comparing sam/fix-claude-code-guided-gh57hs (99100f2) with main (cb6c485)

Open in CodSpeed

@simple-agent-manager

Copy link
Copy Markdown
Contributor Author

Post-verification fix: real codes never submitted inside the CLI (commit adc7c4f)

Raphaël's real-subscription E2E failed: the modal sat on "Completing sign-in…" until the 15-min TTL (staging session 01KYESZPQZ41TFTBSSB88KHNWG, expired 09:07 UTC).

Root cause (reproduced locally against claude v2.1.220, the same version as the staging sandbox image): the Claude Code prompt treats one large stdin chunk as a paste and absorbs an inline trailing \r — the code is typed into the field (masked asterisks visible in PTY capture) but never submitted. Realistic code#state blobs are ~100+ chars and always hit this; the 21-char garbage#rejected-code used for the original staging verification stays below the paste threshold and submits fine, which is why verification passed while every real login hung.

Changes:

  • Driver writes the code, then sends Enter as a separate write after a settle delay (CLAUDE_SETUP_ENTER_DELAY_MS, default 1000ms) — verified against the real CLI with 21/99/210-char codes.
  • Broadened terminal-failure detection: any post-forward OAuth error: screen is terminal. The old pattern required a "status code 4xx" wording that the CLI's 401 ("Authentication failed: Invalid authorization code"), state-mismatch, and network failures never print — those would still have hung forever.
  • Bounded exchange deadline (CLAUDE_SETUP_EXCHANGE_TIMEOUT_MS, default 120s): any unrecognized outcome now fails visibly as exchange_timeout (mapped distinctly by the DO) instead of stalling until TTL.
  • Tests: rewrote the single-write stdin contract (rule 42), added a discriminating paste-absorption regression (fails on the pre-fix driver — verified: 5 tests red pre-fix), 401-wording rejection, exchange-timeout, and DO timeout-mapping tests. 39/39 pass.

Staging verification (run 30197296704, deployed): session 01KYEYE9KBR79YRWPMZG6HTXYJ reached waiting_for_user in 12s; a 99-char garbage code transitioned to exchanging and produced a visible code_rejected failure in 5s (pre-fix: infinite hang — this exact scenario). No active setup-session rows remain.

Follow-ups noted (not blocking): DO-side watchdog for exchanging if the driver process dies (container sleep/OOM); modal UX — Cancel affordance during exchanging, hide the sign-in button once the code is submitted, immediate poll on visibilitychange for backgrounded mobile tabs; disable the manual credential Save button when the field is empty ("Credential cannot be empty").

Still do not merge — awaiting Raphaël's real-subscription code-paste E2E on staging.

@simple-agent-manager

Copy link
Copy Markdown
Contributor Author

Follow-up: real-code failure diagnosed — error detail is no longer discarded (b26de657f, b2ca1ebed)

Raphaël's first real-code attempt after the paste fix (session 01KYGZBWGAF1Y7Q4HREJS22XPB, 2026-07-27 05:04 UTC) failed with "Claude rejected the verification code" despite a correctly copied code.

What the investigation established (live evidence)

  1. The paste/submit fix works — exchanges now reach a terminal outcome in seconds.

  2. Sandbox egress is healthy. A Node-fetch token exchange from inside a staging sandbox received a genuine 400 invalid_grant verdict from platform.claude.com/v1/oauth/token. (curl-shaped probes get 429 rate_limit_error from the same egress — TLS-fingerprint bot scoring; a red herring. The Bun-based CLI is unaffected.)

  3. The real bug: the driver flattened every OAuth error: render into "code rejected". Live against claude v2.1.220 the CLI distinguishes:

    • paste missing the #state half → instant local Invalid code. Please make sure the full code was copied (no network!)
    • full-format bad code/state/PKCE → server Request failed with status code 400
    • network failure → connect ECONNREFUSED …

    The most likely cause of the real-code failure is an incomplete mobile copy (missing # half) or a code issued against a different PKCE challenge — but the driver discarded the only signal that could tell.

The fix

  • Driver: extracts the last OAuth-error line after a settle window (CLAUDE_SETUP_REJECTION_SETTLE_MS, default 400 ms), tolerates Ink overwrite-mangling, redacts sk-ant*, and classifies into code_incomplete / code_rejected / exchange_network_error + bounded detail.
  • DO: maps each class to accurate guidance and appends [CLI: …] (printable-ASCII sanitized, capped 160; the driver's free-form error is still never surfaced).
  • Modal: blocks a claude-code paste with no # before burning the setup session.

Verification

  • 50 unit tests green across driver/DO/modal suites; new tests use the live-captured mangled renders.
  • Real-CLI E2E (patched driver + claude 2.1.220): no-# paste → code_incomplete in 2 s; full-format fake → code_rejected + status code 400 detail.
  • Staging (deploy 30240838919, success incl. smoke tests), through the real product flow:
    • Probe A full-format fake code → code_rejected, message now ends [CLI: Requstfailed withstatus code 400] — proving the sandbox exchange gets genuine server verdicts.
    • Probe B code missing #code_incomplete: "The pasted code was incomplete. Copy the entire code Claude shows — it has a # in the middle — then start again. [CLI: Invalidcode. Please makesure the fullcde wascopied]"
    • Zero active setup sessions / orphan sandboxes after both probes.

PR stays unmerged — the remaining gate is Raphaël's real-subscription retest; whatever happens, the failure message will now carry the true reason.

@simple-agent-manager
simple-agent-manager Bot marked this pull request as draft August 5, 2026 22:18
@simple-agent-manager

Copy link
Copy Markdown
Contributor Author

Converting to draft during tonight's backlog cleanup (SAM task 01KZ9YVKD8FD6JT15FCWRD7TY4) rather than closing or merging.

State: the paste/submit fix (adc7c4f) and the error-classification work (b26de65, b2ca1eb) are on the branch and were staging-validated with garbage codes (code_incomplete / code_rejected / exchange-timeout paths all reach terminal outcomes in seconds). What's missing is the one thing an agent cannot do: a real Claude-subscription verification code end-to-end run. Your last real-code attempt (2026-07-27) failed with 'code rejected' and the branch now captures the CLI's actual error line to tell you exactly why.

Next step is yours: rebase/deploy to staging when convenient and retry with a real code — the classified [CLI: …] detail will show whether it was an incomplete copy, a PKCE/state mismatch, or a genuine exchange failure. Merge stays blocked on that per the missing-credentials policy (end-user-owned account credential).

@simple-agent-manager simple-agent-manager Bot added the needs-human-review Agent could not complete all review gates — human must approve before merge label Aug 6, 2026
@simple-agent-manager

Copy link
Copy Markdown
Contributor Author

PR shepherd run #50 (2026-08-06) — parked: needs-human-review

Reason: credential blocker an agent cannot resolve — the only remaining gate is an end-to-end run with a real Claude-subscription verification code, which requires Raphaël's own Claude Max/Pro account. Everything else is green: all 24 CI checks pass, staging deploy 30193502104 succeeded, and the failure paths (code_incomplete / code_rejected / exchange-timeout) were staging-validated with garbage codes.

No agent action remains. Next step is a real-code retry on staging; the branch now surfaces the CLI's actual error line as a classified [CLI: …] detail so the failure mode will be self-evident.

Triaged under the missing-credentials policy (end-user-owned account credential). Left as draft.

raphaeltm and others added 6 commits August 10, 2026 00:16
The Claude Code prompt treats one large stdin chunk as a paste and absorbs an
inline trailing carriage return: realistic ~100-char codes were typed into the
field but never submitted, so the guided login sat in 'exchanging' until the
session TTL (reproduced against claude v2.1.220; 21-char test codes submit,
which is why staging verification passed). Write the code, then send Enter as
a separate write after a settle delay.

Also fail visibly instead of stalling: any post-forward 'OAuth error:' screen
is terminal (the previous pattern required a 'status code 4xx' wording that
401/state-mismatch/network failures never print), and a bounded exchange
deadline reports 'exchange_timeout' distinctly through the DO when the CLI
produces no recognizable outcome. Both knobs are env-configurable
(CLAUDE_SETUP_ENTER_DELAY_MS, CLAUDE_SETUP_EXCHANGE_TIMEOUT_MS).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…etail

The driver flattened every post-forward 'OAuth error:' render into 'Claude
rejected the verification code', discarding the CLI line that distinguishes
an incomplete paste (missing #state half — instant local failure), a server
rejection (status-code wording), and a sandbox network failure. Reproduced
each wording live against claude v2.1.220; renders arrive with characters
dropped by Ink redraws, so classification tolerates that mangling.

- driver: extract the last OAuth error line after a settle window
  (CLAUDE_SETUP_REJECTION_SETTLE_MS, default 400ms), redact token-like runs,
  classify into code_incomplete / code_rejected / exchange_network_error,
  and publish a bounded detail field alongside the failure state
- DO: map the new failure classes to accurate guidance and append the
  sanitized detail as '[CLI: ...]' (printable ASCII, capped, sk-ant
  redacted); the driver's free-form error field is still never surfaced
- modal: block a claude-code paste missing its '#' half before burning the
  setup session, with copy-the-entire-code guidance

Verified end-to-end against the real CLI: no-# paste -> code_incomplete
with the CLI's own 'full code was copied' advice; full-format fake code ->
code_rejected with 'status code 400' detail.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…low-up

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@simple-agent-manager
simple-agent-manager Bot force-pushed the sam/fix-claude-code-guided-gh57hs branch from b2ca1eb to 17dc056 Compare August 10, 2026 01:02
@simple-agent-manager

Copy link
Copy Markdown
Contributor Author

Human approval recorded for the prior needs-human-review gate: Raphaël stated on 2026-08-16, "Pretty sure I tested the Claude login and it worked. Merge it if possible." I updated the PR body staging-verification evidence with this manual test. Final merge remains delegated to the PR-backlog parent/batch integration.

@simple-agent-manager simple-agent-manager Bot removed the needs-human-review Agent could not complete all review gates — human must approve before merge label Aug 16, 2026
@sonarqubecloud

Copy link
Copy Markdown

@simple-agent-manager

Copy link
Copy Markdown
Contributor Author

Closing as shipped: this PR's content merged to main via batch-2 integration PR #1839 (merge commit 6f347e9) at pinned head bac2702, with Raphaël's manual-test approval recorded and full staging verification of the guided-login flow surface (validator evidence on #1839). The branch's two post-pin commits (merges of #1835/#1836 + branch-side migration reorder) are maintenance the batch integration already accounted for — batch CI (incl. migration-safety) and the staging deploy validated the integrated migration sequence. Production deploy of the batch is being monitored.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant