Repository navigation
[WRONG BRANCH] merge dev into main for the v2.33.0 release #2553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
88 commits
Select commit
Hold shift + click to select a range
a0f5897
release: v2.17.1-preview.20260814
lidge-jun fef111e
Merge main into preview: v2.19.0 release (preview keeps its prereleas…
lidge-jun 255c25d
release: v2.19.0-preview.20260815
lidge-jun fd84ae1
Merge dev into preview: v2.20.0 line
lidge-jun 906286c
Merge pull request #1768 from lidge-jun/promote/260815-preview
lidge-jun 8e154f7
Merge main into preview: v2.22.0 line
lidge-jun 0bc04f0
release: v2.23.0-preview.20260816
lidge-jun 86039da
Merge dev into preview: v2.23.0 release
lidge-jun d240148
Merge dev into preview: v2.24.0 release
lidge-jun f32a2f5
Merge dev into preview: v2.24.1 release
lidge-jun 6a23ea9
Merge dev into preview: Windows suite green
lidge-jun e25ec09
Merge pull request #1907 from lidge-jun/promote-preview
lidge-jun 76e87cd
Merge dev into preview: ignore leftover test temp files
lidge-jun 844081d
Merge pull request #1911 from lidge-jun/promote-preview2
lidge-jun 56f2ed9
Merge main into preview: v2.24.2 release
lidge-jun 506bcab
Merge pull request #1918 from lidge-jun/sync-preview
lidge-jun a43150c
Merge pull request #1962 from lidge-jun/codex/promote-preview-w5
lidge-jun e6709a1
Merge pull request #1967 from lidge-jun/codex/promote-preview-w5b
lidge-jun 379a355
Merge pull request #1970 from lidge-jun/codex/promote-preview-w5c
lidge-jun bed4ca8
Merge pull request #1975 from lidge-jun/codex/promote-preview-w5d
lidge-jun c0961d8
Merge pull request #1979 from lidge-jun/codex/promote-preview-w5e
lidge-jun 82c0576
Merge pull request #1982 from lidge-jun/codex/promote-preview-w5f
lidge-jun 6d2eae7
Merge pull request #1985 from lidge-jun/codex/promote-preview-w5g
lidge-jun 952a832
Merge dev into preview: v2.25.0 release train
lidge-jun 70d7ba5
Merge pull request #2000 from lidge-jun/codex/promote-preview-2250
lidge-jun 46858ef
release: v2.25.0-preview.20260818
lidge-jun 11f6f4c
Merge pull request #2002 from lidge-jun/release-2.25.0-preview.20260818
lidge-jun f4cc168
Merge pull request #2088 from lidge-jun/dev
lidge-jun 60f273d
release: v2.26.0-preview.20260819
lidge-jun 6d6a1b4
Merge pull request #2090 from lidge-jun/release-2.26.0-preview.20260819
lidge-jun 17c8a3a
merge preview into dev for the 2.27.0 preview sync
lidge-jun a055461
Merge pull request #2161 from lidge-jun/codex/sync-preview-2.27.0
lidge-jun a3c33bb
Merge pull request #2187 from lidge-jun/codex/sync-preview-2.28.0
lidge-jun 19fbc93
release: v2.28.0-preview.20260820
lidge-jun d2c700c
Merge pull request #2191 from lidge-jun/codex/release-2.28.0-preview
lidge-jun 190a457
merge dev into preview for the 2.29.0 preview sync
lidge-jun b2609a5
Merge pull request #2285 from lidge-jun/codex/promote-preview-2.29.0
lidge-jun aef1bf0
merge dev into preview for the 2.29.0 preview release
lidge-jun 639dc73
merge main into preview for the 2.29.0-preview release
lidge-jun 298e0f6
release: v2.29.0-preview.20260821
lidge-jun 4cfdc26
merge dev into preview for the 2.30.0-preview release
lidge-jun e9f8840
release: v2.30.0-preview.20260821
lidge-jun c1d2915
merge dev into preview for the 2.31.0-preview.20260822 release
lidge-jun 22541fa
release: v2.31.0-preview.20260822
lidge-jun 91f8606
Merge pull request #2478 from lidge-jun/dev
lidge-jun 09a28e2
release: v2.32.0-preview.20260824
lidge-jun f4cb9f8
merge dev into preview for the v2.32.1-preview.20260825 release
lidge-jun 3be3e55
fix(management): validate the sidecar pair against the submitted back…
lidge-jun b06cb1b
fix(cli): report Codex routing in ocx status and name an unused proxy…
lidge-jun 47a31d7
fix(codex): report a destroyed shim instead of bailing silently (#2519)
lidge-jun fff8611
fix(xai): stop the undeclared-tool guard from killing hosted x_search…
olddonkey c09f040
fix(gui): guard quota reset date formatting (#2405)
luvs01 312b3e7
fix(gui): ignore stale Startup secondary responses (#2416)
luvs01 d402272
fix(responses): retry pre-output EOFs affecting Ox Alpha (#2486)
kremnyi 0f30b39
fix(claude): do not let a non-registering row veto a bare context key…
ntdatt812 0906201
fix(kiro): prioritize tool search results within catalog budget (#2475)
mchlkim d659c54
feat(codex): add per-model ChatGPT compaction budgets (#1905)
luvs01 b2f95e1
fix(anthropic): apply provider default reasoning effort when caller o…
L-Y-J b694268
fix(anthropic): do not let the __omit__ sentinel become an effort val…
lidge-jun 224f23d
fix: normalize legacy exec_command/shell_command tool calls to declar…
L-Y-J 121c1fb
test(bridge): pin legacy shell-name normalization on the SSE path (#2…
lidge-jun 64bc085
fix(catalog): do not carry a retained compact limit onto a corrected …
lidge-jun 8c21b69
devlog: operator visibility train roadmap unit (260825) (#2520)
lidge-jun 98ed186
fix(gui): inset the Claude account-pool warning and threshold field (…
Yoonkeee b12658e
devlog: OAuth login UX roadmap unit (260825)
lidge-jun 2e03252
devlog: OAuth login UX delivery map (issues + PR stack)
lidge-jun eae9fb4
fix(gui): show the device code and authorization link on every login …
lidge-jun 4edef75
devlog: record the as-landed WP2 design and its test split
lidge-jun da6cbfc
fix(gui): render the login hint during a first-time provider add
lidge-jun d7d708f
Merge pull request #2530 from lidge-jun/codex/oauth-login-ux
lidge-jun 315f5bf
Merge pull request #2534 from lidge-jun/codex/oauth-first-add-hint
lidge-jun 1c49c38
feat(oauth): let the operator decline a proxy-side browser open
lidge-jun c6c98e9
fix(gui): read the server browser-open default as a resource, not pos…
lidge-jun 90c4aa2
fix(gui): stop the browser-open toggle from issuing its own settings …
lidge-jun 86bc623
fix(oauth): read code and state from a redirect URL fragment
lidge-jun 34ef539
test(update): stop the launcher-recovery wait from failing a slow CI …
lidge-jun e65d6d3
Merge pull request #2537 from lidge-jun/codex/oauth-open-browser-choice
lidge-jun 858352a
Merge pull request #2540 from lidge-jun/codex/oauth-paste-fragment
lidge-jun 693b6e4
devlog: close out the OAuth login UX merge train
lidge-jun aeea04c
fix(oauth): never pair a code and state from different URL components
lidge-jun 5170fc8
Merge pull request #2543 from lidge-jun/codex/oauth-mixed-source-fix
lidge-jun 9af029c
Merge pull request #2544 from lidge-jun/dev
lidge-jun 89295c6
fix(release): run the preflight suite in the same groups CI does
lidge-jun c0c9544
Merge pull request #2546 from lidge-jun/codex/release-gate-isolation-…
lidge-jun 5559f85
Merge pull request #2547 from lidge-jun/dev
lidge-jun 121ecbc
test(usage): stop asserting the overlay version against a moving oracle
lidge-jun e1fb675
Merge pull request #2549 from lidge-jun/codex/api-usage-overlay-versi…
lidge-jun d560ac6
merge dev into main for the v2.33.0 release
lidge-jun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
112 changes: 112 additions & 0 deletions
112
devlog/_plan/260825_oauth_login_ux/000_baseline_and_scope.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. |
147 changes: 147 additions & 0 deletions
147
devlog/_plan/260825_oauth_login_ux/001_current_state_inventory.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language identifier to both fenced flow diagrams.
markdownlintreports MD040 for both fences. Change each opening fence to```text.devlog/_plan/260825_oauth_login_ux/002_plan_audit.md#L72-L77: Label the dependency-order diagram as text.devlog/_plan/260825_oauth_login_ux/020_wp3_first_add_parity.md#L81-L89: Label the login-flow diagram as text.🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 72-72: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
📍 Affects 2 files
devlog/_plan/260825_oauth_login_ux/002_plan_audit.md#L72-L77(this comment)devlog/_plan/260825_oauth_login_ux/020_wp3_first_add_parity.md#L81-L89🤖 Prompt for AI Agents
Source: Linters/SAST tools