[lilyshen0722]

/cli/authorize is where commonly login sends you to approve a device code. It was the last auth screen still drawn as its own card. It now sits on the shared auth card, like sign-in, sign-up, recovery and the invite page, and matches the d215 s-cli-authorize frames (added in this PR).

What changes

• Layout: the brand row, title, one line of prose, the code as the field, and one key per state. The old "C" tile and "CLI SIGN-IN" eyebrow are gone.
• Signed out: the code field stays visible but read-only, and "Sign in to continue" goes to /v2/login?next=… with the code kept. It's now a button that navigates rather than a link, so it takes the family's key style (the shared rule styles button.v2-login__submit only), the same trade the invite page made. It no longer opens in a new tab on middle-click.
• Confirm: the facts (client, instance, requested) are a bordered list, the caution is the shared notice row, and Deny and Authorize share one row. The host name doesn't break at its hyphen.
• Done, denied, expired, used, error: a title plus the shared status or alert row.
• Behaviour is unchanged: the same two POSTs, code normalising, ?code= prefill and ?next= return.

The App.css leak, fixed at its gate. This page mounts outside V2App, so <body> lacked v2-canvas, and App.css's legacy input, button and form rules (all gated on body.modern-ui:not(.v2-canvas)) reached the card. The old page out-ranked them with restated overrides. The page now claims v2-canvas while mounted, the same way V2App does, and index.tsx stamps it at boot for this path (with or without a trailing slash) so there's no dark flash. A render test checks the class on mount and its removal on unmount. No override rules remain.

Checks

• frame-diff against the frames, local build, signed in, enter-code state: 1440 layout 0.018 / structure 0, 390 layout 0.053 / structure 0.167, light and dark, 0% repaint.
• Confirm and done compared by eye against their frames, with the device POST answered locally in the browser. Nothing was sent to the API.
• src/v2: 185 suites pass (V2AgentSettingsPage times out only under the full parallel run, and passes alone; untouched here). tsc is clean.
• The TASK-682 invariants (tokens only, .v2-root wrapper, short-screen scroll) are kept and now assert the canvas claim instead of the overrides.

🤖 Generated with Claude Code

https://claude.ai/code/session_015AKgpMD6Qh13f7fj6mbGT3

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Team-Commonly/commonly
• Branch: launch/cli-authorize
• File: .coderabbit.yaml
• Commit: cdae9b7
• Layers: Repository YAML
• Review profile: QUIET
• Plan: Advanced
• Run ID: e0f59608-a993-4170-b3bf-cc6865eafa07

Commits

Reviewing files that changed from the base of the PR and between a0f31be and cdae9b7.

Files selected for processing: 7

• docs/design/artboards/d215/index.html
• frontend/src/index.tsx
• frontend/src/v2/__tests__/V2CliAuthorize.test.tsx
• frontend/src/v2/__tests__/signalAccent.test.ts
• frontend/src/v2/__tests__/v2-layout-invariants.test.ts
• frontend/src/v2/components/V2CliAuthorize.css
• frontend/src/v2/components/V2CliAuthorize.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

Walkthrough

The CLI authorization page now uses shared V2 login components and styles across its loading, sign-in, code-entry, confirmation, and outcome states. The authorization route applies the V2 canvas class, and tests and design mockups cover the updated page.

Changes: CLI authorization page

Layer / File(s)	Summary
Authorization rendering and navigation frontend/src/index.tsx, frontend/src/v2/components/V2CliAuthorize.tsx, frontend/src/v2/__tests__/V2CliAuthorize.test.tsx	The boot-path condition now includes /cli/authorize, with or without a trailing slash. The page uses shared V2 login cards and a navigation button for sign-in. Tests cover canvas-class lifecycle, navigation, loading, and authorization outcomes.
Authorization card styling and layout frontend/src/v2/components/V2CliAuthorize.css, docs/design/artboards/d215/index.html, frontend/src/v2/__tests__/v2-layout-invariants.test.ts, frontend/src/v2/__tests__/signalAccent.test.ts	The page adds styling for the code field, hostname, request facts, actions, and notice. The design mockups show code entry, approval, and success states. Layout and style tests check shared login rules and CSS constraints.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Possibly related issues

• Team-Commonly/commonly#1296 — This change implements the browser-based CLI authorization step, but does not implement token auto-refresh.

Possibly related PRs

• Team-Commonly/commonly#3063 — Changes the same authorization stylesheet and layout and accent tests.
• Team-Commonly/commonly#3297 — Changes shared .v2-login layout rules that the updated layout tests assert.
• Team-Commonly/commonly#3292 — Changes shared .v2-login card and mobile layout rules that the updated layout tests assert.

Merge Risk: Minimal · up to cdae9

No merge-blocking issue was identified in the authorization page changes.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly identifies the main change: moving the CLI authorization page onto the shared authentication card.
Description check		The description directly explains the /cli/authorize layout, navigation, state changes, styling, and test coverage described by the changeset.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

[lilyshen0722]

Code gate, PR #3311 at 47264e6: /cli/authorize moved onto the shared v2 auth card

Findings

Blocking

None.

Non-blocking

1. "Sign in to continue" is now a button that calls navigate(), where it used to be a <Link>. See frontend/src/v2/components/V2CliAuthorize.tsx:142-148. The destination URL is the same, and the test proves it (V2CliAuthorize.test.tsx, signed-out test). The cost is that the control is no longer a link: it has no href, middle-click and open-in-new-tab stop working, and a screen reader announces "button" for what is a navigation. The cause is the shared selector .v2-root button.v2-login__submit (v2.css:2125), which only styles a <button>. Two fixes would work: let the shared rule accept :where(a, button), as the OAuth buttons already do, or keep the button. The test now asserts that no link exists (queryByRole('link') … toBeNull()). That reverses an old assertion. The PR body cites the d215 frames for the visual change, but it doesn't say the link became a button.
2. The boot stamp matches the exact path only. frontend/src/index.tsx:23 checks bootPath === '/cli/authorize'. nginx (frontend/nginx.conf:59, cli/authorize/?$) and the router also serve /cli/authorize/, and that variant flashes dark until the component's effect runs. Low impact: the backend verifyUrl (backend/routes/auth.ts:267) has no trailing slash.
3. The body-class behaviour is checked against the source text, not by a test that renders it. signalAccent.test.ts:125-128 regex-matches classList.add/remove and the index.tsx condition. No test renders the page and checks that document.body has v2-canvas while mounted and loses it after unmount (or keeps it across a hand-off to a sibling that also claims it). It's worth adding; the class is what keeps App.css's legacy input and button rules off this page.
4. Live regions are nested. The card is aria-live="polite" (V2CliAuthorize.tsx:115), and it now contains role="status" / role="alert" rows (V2AuthNotice) plus a role="status" loading line (:130). Some screen readers will read the outcome twice. The old page had the same polite wrapper, so this isn't a regression, but now is a cheap time to drop the wrapper's aria-live, since each outcome row carries its own role.
5. The artboard repeats a rule. docs/design/artboards/d215/index.html:717 re-adds .authcard .sub {…}, which is identical to line 708. Harmless, but delete it.
6. The loading state has no h1 (:130). That's unchanged from main, and the new test pins it on purpose. Noted only because the brief asked for one h1 per state; every other state has exactly one.

What I verified

1. The device flow behaves as it does on main. I compared origin/main's V2CliAuthorize.tsx with the head line by line.

• Both POSTs are byte-identical: /api/auth/device/authorize with { userCode: code } for the lookup and { userCode: code, decision } for the decision (:70-72, :95-98).
• normalizeCode, errorScreen (410 → expired, anything else → error), requestAge and currentInstance are unchanged.
• The ?code= prefill (useMemo plus useEffect(() => setCode(queryCode))) is unchanged.
• The submit guard (code.replace('-', '').length !== 8) is unchanged.
• next is built the same way. The signed-out path goes to /v2/login?next=${encodeURIComponent(next)}, and the test asserts the router actually lands on /v2/login?next=%2Fcli%2Fauthorize%3Fcode%3DABCD-EFGH.
• The status → screen mapping is unchanged:
  • pending with a request → confirm
  • denied → denied
  • consumed or authorized → used
  • anything else → expired
  • decision: authorized → done, anything else → denied
• The render order is the same as the old JSX conditionals: loading, then signed out, then the screens. Confirm without a request renders the empty card, as before.
• Because card() returns the same main > div > div shape in every state, React keeps the same DOM nodes across state changes.

2. The v2-canvas lifecycle.

• App.css gates its legacy img, form, input and button rules on body.modern-ui:not(.v2-canvas) (App.css:20, 129, 147, 159, 165-182). V2App adds the class on mount and removes it on unmount (V2App.tsx:213-215). V2CliAuthorize now does the same (:58-63).
• /cli/authorize → /v2/login, through the only navigation the page offers. Both are lazy route elements under one <Routes>. In React 18, the passive-effect flush runs every passive unmount, deletions included, before any passive mount in the same commit (commitPassiveUnmountEffects before commitPassiveMountEffects). So V2CliAuthorize's remove runs first and V2App's add runs after it, and the class ends up present.
• If V2App's chunk isn't loaded yet, a non-transition update (react-router 6, no v7_startTransition) re-suspends a boundary that's already visible. React then hides the old tree rather than deleting it, so the remove waits for the same commit that mounts V2App. In the worst case there's a fallback-only gap, which is what main already had: main never set the class on this page at all.
• /v2/login → /cli/authorize, the post-sign-in next return, works the same way in reverse.
• StrictMode's double effect (add, remove, add) ends with the class present.
• Navigating to a legacy route correctly strips the class. The page has no other links: V2AuthBrand isn't a link.
• The interaction with /'s boot-only stamp is the existing V2App limitation (documented in V2OAuthComplete.tsx:53). This PR doesn't introduce it.
• v2.css is imported globally in App.tsx:23, so the .v2-login* rules load on a cold /cli/authorize.

3. CSS.

• V2CliAuthorize.css has no hex, rgb or named colour and no var(--x, fallback). Every token it uses (--v2-font-mono, --v2-r-10, --v2-border-soft, --v2-text-primary, --v2-text-secondary) is defined in v2.css.
• The shared classes are used the way v2.css defines them: .v2-login scroller (flex:1, min-height:0, overflow-y:auto) → .v2-login__card.v2-login__card--stack (grid, a direct child form set to display: contents) → __title, __subtitle, __field/__label/__input, __submit and __submit--ghost, __notice.
• The short-viewport scroll holds: .v2-root is 100vh with overflow hidden and is a flex column, .v2-login scrolls, and the card centres with margin: auto.
• Phone width: the 480px media rule sets .v2-login__input { font-size: 16px } at specificity (0,1,0). The code input's rule .v2-cli-authorize .v2-login__input.v2-cli-authorize__code is (0,3,0) and sets font: 600 18px/1 var(--v2-font-mono), so mono 18px holds at every width.

4. Accessibility.

• Each state renders exactly one h1 (loading has none, as on main).
• There is a polite live region, and outcomes get role=status/role=alert through V2AuthNotice (status rows mount empty and fill a frame later, so they get announced).
• The input is wrapped in a <label> with visible text and also has aria-label="Device code".
• Continue, Deny, Authorize and Sign in to continue are all <button>s with explicit type.
• The caution is role="note" with an info glyph, so it doesn't rely on colour.

5. Tests and the rest of the repo.

• CI=true npx jest V2CliAuthorize.test.tsx signalAccent.test.ts v2-layout-invariants.test.ts (Node 22): 3 suites and 377 tests pass. The only console output is react-router's future-flag warnings.
• npx tsc --noEmit -p . (backend lines filtered out): clean.
• The component tests assert rendered output: headings by role and name, input value and disabled state, the dt order of the facts, the dd text, the note and status/alert text including the <code>commonly login</code>, button order and parent, and the real router landing on /v2/login?next=…. They also cover the classes. New coverage: denied, error (alert) and loading.
• git grep for v2-cli-authorize__card|__primary|__secondary|__field|__mark|__eyebrow|__warning|__actions, "CLI SIGN-IN", "Authorize Commonly CLI" and "Sign in to continue" across frontend, e2e/, docs, cli and backend finds no stale consumers. The only hits are the new artboard and an unrelated i18n string. e2e/ has no /cli/authorize spec. The cli and backend tests reference only the URL, which is unchanged.
• The removed signalAccent assertions (warning-ink token, __primary overrides, accent fill) and the reversed "CLI SIGN-IN" assertion are covered by the d215 frames cited in the PR body.

GATE: PASS

[lilyshen0722]

Code gate, PR #3311: /cli/authorize moved onto the shared v2 auth card. First reviewed at 47264e6, re-stamped at cdae9b7 (see "Re-stamp" at the end).

Findings

Blocking

None.

Non-blocking

1. "Sign in to continue" is now a button that calls navigate(), where it used to be a <Link>. See frontend/src/v2/components/V2CliAuthorize.tsx:142-148. The destination URL is the same, and the test proves it (V2CliAuthorize.test.tsx, signed-out test). The cost is that the control is no longer a link: it has no href, middle-click and open-in-new-tab stop working, and a screen reader announces "button" for what is a navigation. The cause is the shared selector .v2-root button.v2-login__submit (v2.css:2125), which only styles a <button>. Two fixes would work: let the shared rule accept :where(a, button), as the OAuth buttons already do, or keep the button. The test now asserts that no link exists (queryByRole('link') … toBeNull()). That reverses an old assertion. The PR body cites the d215 frames for the visual change, but it doesn't say the link became a button.
2. The boot stamp matches the exact path only. frontend/src/index.tsx:23 checks bootPath === '/cli/authorize'. nginx (frontend/nginx.conf:59, cli/authorize/?$) and the router also serve /cli/authorize/, and that variant flashes dark until the component's effect runs. Low impact: the backend verifyUrl (backend/routes/auth.ts:267) has no trailing slash.
3. The body-class behaviour is checked against the source text, not by a test that renders it. signalAccent.test.ts:125-128 regex-matches classList.add/remove and the index.tsx condition. No test renders the page and checks that document.body has v2-canvas while mounted and loses it after unmount (or keeps it across a hand-off to a sibling that also claims it). It's worth adding; the class is what keeps App.css's legacy input and button rules off this page.
4. Live regions are nested. The card is aria-live="polite" (V2CliAuthorize.tsx:115), and it now contains role="status" / role="alert" rows (V2AuthNotice) plus a role="status" loading line (:130). Some screen readers will read the outcome twice. The old page had the same polite wrapper, so this isn't a regression, but now is a cheap time to drop the wrapper's aria-live, since each outcome row carries its own role.
5. The artboard repeats a rule. docs/design/artboards/d215/index.html:717 re-adds .authcard .sub {…}, which is identical to line 708. Harmless, but delete it.
6. The loading state has no h1 (:130). That's unchanged from main, and the new test pins it on purpose. Noted only because the brief asked for one h1 per state; every other state has exactly one.

What I verified

1. The device flow behaves as it does on main. I compared origin/main's V2CliAuthorize.tsx with the head line by line.

• Both POSTs are byte-identical: /api/auth/device/authorize with { userCode: code } for the lookup and { userCode: code, decision } for the decision (:70-72, :95-98).
• normalizeCode, errorScreen (410 → expired, anything else → error), requestAge and currentInstance are unchanged.
• The ?code= prefill (useMemo plus useEffect(() => setCode(queryCode))) is unchanged.
• The submit guard (code.replace('-', '').length !== 8) is unchanged.
• next is built the same way. The signed-out path goes to /v2/login?next=${encodeURIComponent(next)}, and the test asserts the router actually lands on /v2/login?next=%2Fcli%2Fauthorize%3Fcode%3DABCD-EFGH.
• The status → screen mapping is unchanged:
  • pending with a request → confirm
  • denied → denied
  • consumed or authorized → used
  • anything else → expired
  • decision: authorized → done, anything else → denied
• The render order is the same as the old JSX conditionals: loading, then signed out, then the screens. Confirm without a request renders the empty card, as before.
• Because card() returns the same main > div > div shape in every state, React keeps the same DOM nodes across state changes.

2. The v2-canvas lifecycle.

• App.css gates its legacy img, form, input and button rules on body.modern-ui:not(.v2-canvas) (App.css:20, 129, 147, 159, 165-182). V2App adds the class on mount and removes it on unmount (V2App.tsx:213-215). V2CliAuthorize now does the same (:58-63).
• /cli/authorize → /v2/login, through the only navigation the page offers. Both are lazy route elements under one <Routes>. In React 18, the passive-effect flush runs every passive unmount, deletions included, before any passive mount in the same commit (commitPassiveUnmountEffects before commitPassiveMountEffects). So V2CliAuthorize's remove runs first and V2App's add runs after it, and the class ends up present.
• If V2App's chunk isn't loaded yet, a non-transition update (react-router 6, no v7_startTransition) re-suspends a boundary that's already visible. React then hides the old tree rather than deleting it, so the remove waits for the same commit that mounts V2App. In the worst case there's a fallback-only gap, which is what main already had: main never set the class on this page at all.
• /v2/login → /cli/authorize, the post-sign-in next return, works the same way in reverse.
• StrictMode's double effect (add, remove, add) ends with the class present.
• Navigating to a legacy route correctly strips the class. The page has no other links: V2AuthBrand isn't a link.
• The interaction with /'s boot-only stamp is the existing V2App limitation (documented in V2OAuthComplete.tsx:53). This PR doesn't introduce it.
• v2.css is imported globally in App.tsx:23, so the .v2-login* rules load on a cold /cli/authorize.

3. CSS.

• V2CliAuthorize.css has no hex, rgb or named colour and no var(--x, fallback). Every token it uses (--v2-font-mono, --v2-r-10, --v2-border-soft, --v2-text-primary, --v2-text-secondary) is defined in v2.css.
• The shared classes are used the way v2.css defines them: .v2-login scroller (flex:1, min-height:0, overflow-y:auto) → .v2-login__card.v2-login__card--stack (grid, a direct child form set to display: contents) → __title, __subtitle, __field/__label/__input, __submit and __submit--ghost, __notice.
• The short-viewport scroll holds: .v2-root is 100vh with overflow hidden and is a flex column, .v2-login scrolls, and the card centres with margin: auto.
• Phone width: the 480px media rule sets .v2-login__input { font-size: 16px } at specificity (0,1,0). The code input's rule .v2-cli-authorize .v2-login__input.v2-cli-authorize__code is (0,3,0) and sets font: 600 18px/1 var(--v2-font-mono), so mono 18px holds at every width.

4. Accessibility.

• Each state renders exactly one h1 (loading has none, as on main).
• There is a polite live region, and outcomes get role=status/role=alert through V2AuthNotice (status rows mount empty and fill a frame later, so they get announced).
• The input is wrapped in a <label> with visible text and also has aria-label="Device code".
• Continue, Deny, Authorize and Sign in to continue are all <button>s with explicit type.
• The caution is role="note" with an info glyph, so it doesn't rely on colour.

5. Tests and the rest of the repo.

• CI=true npx jest V2CliAuthorize.test.tsx signalAccent.test.ts v2-layout-invariants.test.ts (Node 22): 3 suites and 377 tests pass. The only console output is react-router's future-flag warnings.
• npx tsc --noEmit -p . (backend lines filtered out): clean.
• The component tests assert rendered output: headings by role and name, input value and disabled state, the dt order of the facts, the dd text, the note and status/alert text including the <code>commonly login</code>, button order and parent, and the real router landing on /v2/login?next=…. They also cover the classes. New coverage: denied, error (alert) and loading.
• git grep for v2-cli-authorize__card|__primary|__secondary|__field|__mark|__eyebrow|__warning|__actions, "CLI SIGN-IN", "Authorize Commonly CLI" and "Sign in to continue" across frontend, e2e/, docs, cli and backend finds no stale consumers. The only hits are the new artboard and an unrelated i18n string. e2e/ has no /cli/authorize spec. The cli and backend tests reference only the URL, which is unchanged.
• The removed signalAccent assertions (warning-ink token, __primary overrides, accent fill) and the reversed "CLI SIGN-IN" assertion are covered by the d215 frames cited in the PR body.

Re-stamp at cdae9b7

Delta reviewed: git diff 47264e66 cdae9b7f, 4 files, +12/-3.

• Note 2 resolved. index.tsx:23 now tests /^\/cli\/authorize\/?$/. Checked by hand: it matches /cli/authorize and /cli/authorize/, and does not match /cli/authorizex or /cli/authorize/x.
• Note 3 resolved. The new test in V2CliAuthorize.test.tsx:61-69 first clears v2-canvas, then renders the page and checks the body has the class. After unmount() it checks the class is gone. The test file doesn't import index.tsx, so only the component's own effect can satisfy the first check, and only its cleanup can satisfy the second. The test therefore exercises the behaviour it names, which matches the coordinator's report that it fails with the add removed (I didn't re-mutate a read-only worktree).
• The signalAccent.test.ts:128 source check now follows the regex. It stays a source-text check; the behaviour is covered by the new render test.
• Note 5 resolved. Compared with origin/main, the artboard now has exactly one .authcard .sub rule, and the PR's net artboard diff no longer touches it.
• Note 1: the coordinator reports the PR body now states the button-not-link change as a deliberate trade. Accepted. Note 4 is unchanged and was already there before this PR.
• Re-run at this head: CI=true npx jest V2CliAuthorize.test.tsx signalAccent.test.ts v2-layout-invariants.test.ts: 3 suites, 378 tests pass, one more than before. tsc --noEmit (backend lines filtered out) is clean.
• The delta changes nothing in V2CliAuthorize.tsx or .css, so the device-flow, canvas-lifecycle, CSS and accessibility checks above still hold.

GATE: PASS