[WRONG BRANCH] merge dev into preview for the v2.33.0-preview.20260825 release

devlog/_plan/260825_oauth_login_ux/000_baseline_and_scope.md

@@ -0,0 +1,112 @@
+# 000 — OAuth login UX: baseline, pain points, and work-phase map
+
+Unit opened 2026-08-25. Session `01a036cb-4f1c-7b81-8c8d-277f92c914a7`.
+Goalplan slug `fix-oauth-login-remote-headless-ux-in-opencodex`.
+
+## Baseline
+
+| Ref | SHA |
+|-----|-----|
+| `origin/dev` | `bb89eafbe` |
+| `origin/main` | `71c57ea64` |
+
+## The pain points, as reported
+
+These are the operator's own words, recorded before any code was read. Every
+work-phase in this unit traces back to one of them.
+
+> 지금 oauth 로그인 + 원격 지원이 너무 불편하다
+>
+> 1. 이미 프로바이더가 추가된 상태에서는 링크를 복사할 수 있지만 → 첫 추가 때 못함
+>    → 링크 복사 못함, 크롬 다른 프로필 못함
+> 2. 프로바이더 추가도 링크는 보이지만 device 로그인을 하기가 좀 불편함. GUI에서
+>    코드 붙여넣기가 있는 그록이나 claude 쪽도 약간 불편함
+
+In English, for the issue tracker:
+
+1. **The first add is the worst experience.** Once a provider exists, its
+   workspace panel shows the authorization URL with a copy button. During the
+   very first login — the one where the operator has no other way in — that
+   affordance is not there. No copyable link means no way to open the URL in a
+   *different* Chrome profile, and no way to finish the login from another
+   machine.
+2. **Device login is awkward in the GUI**, and the paste-a-code providers
+   (xAI Grok, Anthropic Claude) are awkward too.
+
+## What the code says
+
+The report is accurate, and the cause is a split that nobody planned. Two
+login surfaces exist and each one has exactly the half the other is missing.
+
+| Affordance | Workspace panel (existing provider) | Add-provider modal (first add) |
+|------------|-------------------------------------|--------------------------------|
+| Authorization URL + copy | yes | **no** |
+| Device / user code + copy | yes | **no** |
+| Paste redirect URL or code | **no** | yes |
+| Cancel | yes | no |
+
+- `gui/src/components/provider-workspace/ProviderAuthPanel.tsx` renders the
+  URL and the device code, and has no paste input anywhere in its 568 lines.
+- `gui/src/components/add-provider-oauth-pane.tsx` renders the paste input,
+  and its `LoginUrlBlock` is fed by a hook that never reads `deviceCode`.
+- `gui/src/components/use-add-provider-oauth.ts` parses the login response as
+  `{ url, instructions, error }`. The server returns `deviceCode` as well
+  (`src/server/management/oauth-account-routes.ts`); the modal discards it.
+- The Accounts tab of the add-provider modal
+  (`gui/src/components/provider-catalog/ProviderCatalog.tsx`) starts a login
+  through `Providers.tsx`, which stores the hint in `loginInfo` — read only
+  by `ProviderAuthPanel`. During a first add the modal is on screen and the
+  panel is not, so the hint is computed, stored, and never displayed.
+
+That is the whole of pain point 1: not a missing feature, a hint with no
+renderer.
+
+## The remote half
+
+`POST /api/oauth/login` calls `openUrl(authUrl)` unconditionally whenever a
+browser flow returns a URL. `src/lib/open-url.ts` shells out to `open` /
+`xdg-open` / `rundll32`, which means the **OS default browser profile** —
+the operator cannot send it to a second Chrome profile, and on a headless or
+SSH host the spawn is simply lost. There is no opt-out today: not a request
+field, not a config key, not an environment variable.
+
+## Work-phase map
+
+| Phase | Doc | Deliverable |
+|-------|-----|-------------|
+| WP1 | this unit | Docs-only roadmap at diff-level precision |
+| WP2 | `010` | One login-hint component: URL + device code + paste, on all three surfaces |
+| WP3 | `020` | First-add parity: the hint renders inside the add-provider modal |
+| WP4 | `030` | Operator control over server-side browser auto-open |
+| WP5 | `040` | Paste normalization and survivable failures |
+
+One work-phase is one full PABCD cycle, one decade doc, one issue, one PR
+against `dev`.
+
+## Scope boundary
+
+Out of scope, stated once:
+
+- Token storage format, credential refresh, and the account store schema.
+- New providers or adapters, account-pool routing, quota surfaces.
+- `src/lab/` — the core-lab boundary test exists for a reason.
+- Publishing, releasing, or merging anything.
+
+## Security invariants this unit must not break
+
+These are already load-bearing in the code and a UX change is not permitted to
+soften them:
+
+- `src/oauth/github-copilot.ts` constructs its verification URL locally and
+  refuses a non-allowlisted one; a server-supplied `verification_uri_complete`
+  is never handed to `openUrl`.
+- `src/oauth/callback-server.ts` enforces state on `url`/`query`-shaped
+  pastes and exempts only a syntactically raw in-session code.
+- `src/oauth/index.ts` bounds a pasted payload at 4 KiB and validates it
+  synchronously before it reaches the flow.
+- No token, authorization code, or request body may be logged.
+
+## Evidence rule
+
+A remembered pass is not evidence. Every completion claim carries exact command
+output, the issue and PR numbers, the head SHA, and the CI conclusion on it.

devlog/_plan/260825_oauth_login_ux/001_current_state_inventory.md

@@ -0,0 +1,147 @@
+# 001 — Current-state inventory of every login surface
+
+Read at `bb89eafbe`. Every claim below is a file:line read, not a memory.
+
+## Three surfaces, three renderers
+
+There are three places a human can start an OAuth login in the GUI, and they
+do not share a renderer.
+
+### A. Workspace auth panel — an already-added provider
+
+`gui/src/components/provider-workspace/ProviderAuthPanel.tsx`
+
+- `:242` picks the hint for this row: `loginHint?.provider === item.name`.
+- `:392-401` renders the device code with a copy button.
+- `:402` renders `<LoginUrlBlock url={hintForThis.url ?? ""} />`.
+- `:403-408` renders Cancel.
+- A search for `paste`, `manual`, or `submitManual` across all 568 lines
+  returns nothing. **This surface cannot accept a pasted code.**
+
+### B. Add-provider modal, OAuth pane — a first add via a catalog preset
+
+`gui/src/components/add-provider-oauth-pane.tsx`
+
+- `:59` renders `<LoginUrlBlock url={oauthUrl} />`.
+- `:60-99` renders the paste input and submit button.
+- No device code is rendered anywhere. The prop does not exist.
+- When a device flow returns no `url`, `LoginUrlBlock` returns `null`
+  (`login-url-block.tsx:16`), so the pane shows a spinner label and an empty
+  paste box with nothing to act on.
+
+The hook behind it, `gui/src/components/use-add-provider-oauth.ts:53`:
+
+```ts
+const data = await res.json() as { url?: string; instructions?: string; error?: string };
+```
+
+`deviceCode` is not in the type and is never read, although
+`src/server/management/oauth-account-routes.ts:176` returns it:
+
+```ts
+return jsonResponse({ url: authUrl, instructions, deviceCode });
+```
+
+### C. Add-provider modal, Accounts tab — a first add via an account row
+
+`gui/src/components/provider-catalog/ProviderCatalog.tsx:148-212`
+
+The account rows call `onLogin(row.id)`, which is `Providers.tsx`'s
+`requestLoginOAuth`. That path stores the hint:
+
+- `gui/src/pages/use-providers-oauth.ts:93-96` reads `url`,
+  `instructions` **and** `deviceCode`, then calls `setLoginInfo`.
+- `gui/src/pages/Providers.tsx:365` passes `loginInfo` to
+  `ProviderDetails` → `ProviderAuthPanel`.
+
+`ProviderAuthPanel` is the workspace surface for an **existing** provider.
+During a first add the modal is open and no panel is mounted for that
+provider, so the hint has no renderer. The row renders
+`t("prov.waitingBrowser")` and a Cancel button, and that is all the operator
+gets: no URL, no code, no paste box.
+
+**This is pain point 1 exactly.** The data arrives; nothing draws it.
+
+### D. Codex account modal — a fourth, near-duplicate surface
+
+`gui/src/components/add-codex-account-waiting-step.tsx:38-69` renders
+`LoginUrlBlock` plus its own paste input against
+`/api/codex-auth/login/code`. `src/codex/auth-api.ts:2077` returns only
+`{ flowId, url, instructions }` — no device code on this path either.
+
+## What each provider actually returns
+
+`startLoginFlow` (`src/oauth/index.ts:1420-1504`) resolves
+`{ url, instructions?, deviceCode? }` from the provider's `onAuth` call.
+
+| Provider | Shape | Produced at |
+|----------|-------|-------------|
+| xAI, Anthropic, ChatGPT, Cursor, Antigravity, Kiro | browser redirect + loopback callback | `OAuthCallbackFlow.login()`, `callback-server.ts:116` |
+| Kimi | device flow | `kimi.ts:212` — `instructions: "Enter code: …"`, **no `deviceCode` field** |
+| Nous | device flow | `nous.ts:660-663` — sets `deviceCode: device.userCode` |
+| GitHub Copilot | device flow | `github-copilot.ts:393-402` — locally constructed verify URL, `deviceCode: device.userCode` |
+| local-token import | no browser | `index.ts:1473` resolves `{ url: "" }` with an explanatory string |
+
+Two observations that matter for WP2:
+
+1. Kimi puts the user code inside a free-text `instructions` string instead of
+   the structured `deviceCode` field, so no surface can render it as a code.
+2. `github-copilot.ts:170` refuses to trust `verification_uri_complete` and
+   builds the URL itself. That invariant is not negotiable in WP4.
+
+## The manual-paste path, end to end
+
+1. `POST /api/oauth/login/code` — `oauth-account-routes.ts:202-213`.
+   Caps input at 4096 chars, calls `submitManualLoginCode`, returns 409 on
+   failure.
+2. `submitManualLoginCode` — `index.ts:1329-1360`. Rejects empty, rejects
+   >4 KiB, rejects when no login is in progress, then `parseCallbackInput`.
+   Rejects when no `code` is found; for `url`/`query` shapes enforces
+   state once the flow has registered `expectedState`.
+3. `parseCallbackInput` — `callback-server.ts:273-300`. Three shapes:
+   a parseable URL, a string containing `code=`, or a raw code with optional
+   `#state`.
+4. `OAuthCallbackFlow.#waitForCallback` — `callback-server.ts:238-261`
+   re-parses and loops on a bad paste.
+
+The flow **does** survive a rejected paste — the loop re-prompts. What it does
+not do is tell the operator anything useful: the GUI shows
+`t("prov.pasteFail", { error })`, and only surface B has a paste box at all.
+
+Accepted today: `https://…/callback?code=X&state=Y`, `?code=X&state=Y`,
+`code=X&state=Y`, `X`, `X#Y`. Whitespace is trimmed at three separate
+layers. A URL missing `state` is rejected with a specific message.
+
+## Server-side browser opening
+
+`oauth-account-routes.ts:170-175`:
+
+```ts
+if (authUrl && !deviceCode) {
+  const { openUrl } = await import("../../lib/open-url");
+  openUrl(authUrl);
+}
+```
+
+Unconditional for browser flows. `src/codex/auth-api.ts:1844` does the same
+on the Codex path. `src/lib/open-url.ts:11-24` spawns the platform opener,
+which resolves the **default** browser and therefore the default profile. It
+swallows spawn errors deliberately (a headless host emits ENOENT
+asynchronously), so a failed open is indistinguishable from a successful one
+from the GUI's perspective.
+
+There is no opt-out: no request field, no config key, no environment variable.
+Grepping `openUrl` finds callers in `login-cli.ts:85`, `dispatch.ts:305`,
+`auth-api.ts:1844`, and `oauth-account-routes.ts:173`; none is conditional.
+
+## Existing tests
+
+`tests/oauth-manual-code.test.ts` covers the paste path;
+`tests/oauth-callback-server.test.ts` and `oauth-callback-binds.test.ts`
+cover parsing and binding; `tests/github-copilot-oauth.test.ts`,
+`nous-oauth.test.ts`, `kimi-oauth-identity.test.ts` cover device flows;
+`tests/oauth-public-surface.test.ts` and `oauth-status-privacy.test.ts`
+cover the management surface and its redaction.
+
+Not covered anywhere: what the GUI *renders* during a login. Every gap in this
+unit lives in that hole.

devlog/_plan/260825_oauth_login_ux/002_plan_audit.md

@@ -0,0 +1,93 @@
+# 002 — Plan audit: what ships, what does not, and how it splits
+
+## The one-sentence diagnosis
+
+Nothing here is missing infrastructure. The server already computes the
+authorization URL, the device code, and the manual-paste channel; four GUI
+surfaces each render a different subset of it, and one of them renders none of
+it at the exact moment the operator has no other option.
+
+## Candidate list, and the cut
+
+Eleven changes were on the table after reading the tree. Four ship in this
+unit. The rest are recorded here so a later cycle does not rediscover them.
+
+### Shipping
+
+| WP | Change | Why it is in |
+|----|--------|--------------|
+| WP2 | One login-hint component: URL + device code + paste on all surfaces | Directly answers pain point 2; every other GUI fix depends on it existing |
+| WP3 | The hint renders during a first add | Directly answers pain point 1 |
+| WP4 | Operator control over server-side auto-open | The "다른 Chrome 프로필" half of pain point 1, and the whole remote story |
+| WP5 | Paste normalization: hash-fragment redirects | A real paste that looks valid and is silently rejected today |
+
+### Deferred, with reasons
+
+- **Expiry countdown / poll interval in the GUI.** The polling math is already
+  correct in each provider; showing it is additive UI over a DTO that does not
+  carry `expiresAt` yet. Real, but not a pain point that was reported.
+- **Kimi allowlisting of `verification_uri_complete`.** Kimi and Nous pass the
+  provider-supplied complete URI into `onAuth`. Copilot refuses to
+  (`github-copilot.ts:393`). Tightening Kimi/Nous is a **security** change,
+  not a UX change, and it does not belong in a PR whose title says "GUI". It
+  gets its own issue.
+- **`openUrl` returning success/failure.** Attractive, but the spawn is
+  detached and a browser that opens then fails is indistinguishable from one
+  that never launched. A truthful signal needs more than an exit code.
+- **A browser/profile picker in the GUI.** WP4 gives the operator the
+  *ability* to not have their default profile hijacked. A full picker is a
+  product surface, and the operator asked for control, not a picker.
+- **`ocx login` re-prompt loop.** The CLI's one-shot readline is a smaller
+  version of the same bug, on a surface nobody reported. Own issue.
+
+## The default-preservation rule
+
+WP4 is the only phase that can change what already happens, so it carries the
+strictest constraint in this unit: **auto-open stays the default.** An
+operator who upgrades and does nothing must see byte-identical behavior. The
+new capability is an explicit choice, never an inferred one.
+
+That rules out the tempting version of this feature — sniffing
+`SSH_CONNECTION` or an absent `DISPLAY` and silently declining to open. A
+false positive there (X11 forwarding, WSLg, a desktop session that does not
+advertise itself) breaks a login that works today, and breaks it silently.
+Explicit opt-out first; inference is a separate decision with its own
+evidence.
+
+## Security invariants, restated as gates
+
+A PR in this unit fails review if it:
+
+1. Hands a provider-supplied `verification_uri_complete` to `openUrl`.
+2. Weakens state enforcement on `url`/`query`-shaped pastes
+   (`callback-server.ts:252`, `index.ts:1347-1350`).
+3. Accepts `access_token` from a URL fragment — hash parsing in WP5 reads
+   `code` and `state` only, never a token.
+4. Raises the 4 KiB paste bound or the 4096-char route cap.
+5. Logs a URL, a code, a token, or a request body.
+6. Passes a shell string where an argv array is required.
+
+## Dependency order and the PR stack
+
+```
+WP2 (shared hint component)
+ └── WP3 (first-add parity — renders the WP2 component)
+WP4 (auto-open control)      ← independent
+WP5 (paste normalization)    ← independent
+```
+
+WP3 stacks on WP2 because it mounts the component WP2 creates. WP4 and WP5
+touch disjoint files and target `dev` directly.
+
+| WP | Issue template | PR base | GUI screenshot |
+|----|----------------|---------|----------------|
+| WP2 | feature_request | `dev` | required |
+| WP3 | bug_report | WP2 head | required |
+| WP4 | feature_request | `dev` | required if GUI toggle lands |
+| WP5 | bug_report | `dev` | not required |
+
+## Verdict
+
+PASS. Four phases, dependency-ordered, each with a falsifiable test and a
+bounded diff. The audit's one binding instruction to later phases: WP4 must
+ship the explicit choice and must not ship inference.