Repository navigation
Conversation
Settings (UX spec 3.10, row P9) becomes two cards in a 720 px column: Signed-in account (initials avatar, name, email) and Session with a secondary Sign out and its help line. The 'No configurable settings' line and the Back button go; Back's previousCollection history state had no other reader, so it goes too. The session read carries no sign-in method or expiry, so those rows are omitted.
Its text stays true (no editable settings), and leaving it avoids a new conflict with the open Installation configuration PR on that line.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
Visual review nits: a 96 px label column at phone width so long emails keep room, the list stretches to the card, and 24 px initials in the 64 px avatar.
clawsweeper
Bot
added
the
clawsweeper:intake:feature
clawsweeper
Bot
added
P3
|
Codex review: needs maintainer review before merge. What this changesThis PR gives Settings separate account and session cards, adds Sign out, and removes the Back button and its unused navigation state. Example: Demo Operator opens Settings at a 390-pixel viewport.
Review scores
ProductKind: Feature · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review Keep open for maintainer triage. The previous Storybook finding is fixed, and the reviewed diff introduces no concrete correctness or security defect. Priority: P3 Before mergeNone. FindingsNone. Agent review detailsHow this fits togetherThe controller-served console reads the admitted session to display account details and sends Settings sign-out requests through the existing API client and server authentication boundary. flowchart LR
User[Signed-in user] --> Settings[Settings page]
Settings --> Session[Admitted session facts]
Settings --> Logout[Existing logout handler]
Logout --> Auth[Controller authentication endpoint]
Auth --> Result[Session revoked or retry screen]
Technical reviewBest possible solution: Keep the focused Settings presentation and reuse the existing logout owner without expanding the session API. Do we have a high-confidence way to reproduce the issue? This is an enhancement; current-main source confirms the previous Settings panel, and the PR reports that its browser case fails against the base presentation and passes with the change. Is this the best way to solve the issue? The implementation uses existing session facts, shared avatar initials, and the established logout handler, while removing navigation state made unnecessary by the deliberate Back-button removal. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 6c5bbd6ff8ce. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceWhat I checked:
Likely related people:
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (1 earlier review cycle)
Reviewed October 20, 2026, 12:03 AM ET / 04:03 UTC (Revision 2). |
…Storybook The Settings story describes the account and Session cards with steps, and a new Settings 'Sign-out unconfirmed' story reuses the sign-out 503 fixture through the card's button (review: Storybook must cover the new control).
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
clawsweeper
Bot
added
clawsweeper:intake:feature
1 participant
Problem
Settings (UX spec 3.10, row P9 / S2) was one panel with name and email, a "No configurable settings in this release." line and a Back button. Sign out lived only in the account menu, and the page did not look like the rest of the warm-quiet console.
Change
.cards in a 720 px column, matchingsettings-1440.png/settings-390.png:initials()as the account row, now exported fromshell.mjs) beside adlwith Name and Email.POST /api/auth/sign-out, the account menu's path), described by "Signing out ends this session on this device only." At phone width the button spans the card and is 44 px tall.previousCollectionhistory state, so that bookkeeping innavigation.mjs/console.mjsis removed too (history entries now carrynullstate).GET /api/auth/session→{authenticated, sessionKey, user}) has no sign-in method, sign-in time or expiry, and its schema allows no extra fields. So there is no "Sign-in method" row and no "Signed in" / "Expires" rows. Adding those fields is a separate server change that needs owner sign-off.docs/reference/console.md: describes Settings (word count goes down by 2).Verification
tests/browser/console-shell.test.mjs› "Settings shows the account and session cards, and its Sign out ends the session" runs at 1440 and 390. It checks the scope line, that there is no Namespace selector, a 64 px avatar with "CA", Name/Email only, Sign out's accessible description, secondary styling (surface fill, border-strong edge), no "No configurable settings" or Back, a 720 px column, a full-width 44 px button at 390, a successfulPOST /api/auth/sign-out, the Sign in form, andGET /api/auth/sessionreturningnull.origin/main'sconsole.mjs/console.css/shell.mjs/navigation.mjs) it fails: TimeoutError waiting for the Signed-in account card's.account-avatar.console-shell.test.mjspassed 20/20 at 2 CPUs (8/8 tests each run).console-retained-revalidation.test.mjs: the Settings return now uses the sidebar Agents link instead of the removed Back button, with the same held-frame assertions. The case passed 20/20 at 2 CPUs and the full file passed 67/67.dlat phone width so long emails keep room, and 24 px initials in the 64 px avatar. Not changed: the desktop Sign out is 39 px tall, from the shared base button rule (out of scope here). After the nits,console-shell.test.mjspassed 20/20 again at 2 CPUs.console.test.mjs48/48,console-page-header.test.mjs5/5,console-shell.test.mjs8/8. Prettier and eslint are clean on the touched files.settingsframe, measured the wayshots.mjsmeasures):#maindlrowsBefore / after / mockup (Storybook
settingsstory; mockup is the alignment target. The Sign-in method, Signed in and Expires rows are omitted, see Change):Walkthrough (Storybook
settingsstory, simulated UI: choose Sign out in the Session card, "Signing out…", then the sign-in page): 1440 video · 390 video. The real sign-out path (Fastify auth,POST /api/auth/sign-out, session readnullafterwards) is covered by the browser test above.Sign-out unconfirmed from Settings (new story, sign-out returns 503 while the session stays active): private content is hidden until revocation is confirmed, with Retry.
Storybook: Pages/Settings › Account (description and walkthrough steps updated) and Pages/Settings › Sign-out unconfirmed (new; reuses the sign-out 503 fixture through the card's button).
docs/contributing/console-storybook.mdlists the new coverage.Open-PR conflict map (
git merge-treeof each open PR against main, then against this head):console.mjsand in the Settings scenario's description inscenarios.mjs, because both PRs rewrite the Settings block. The fix is mechanical: keepsettingsView()and append feat: share discoverable Driver configuration definitions #2980'srenderInstallationConfigurations(...)after it, and merge the two descriptions.console.mjs/shell.mjs/console.css): it also appends a story topages-settings.stories.mjsafter the same line, so that file conflicts too. Keep both exports.Risks
Low. This is a client-only change to one page. Sign out reuses
logout()unchanged. Dropping thepreviousCollectionhistory state only affects the removed Back button, which was its only reader (history.statehas no other reader in the console). The sign-in method and expiry rows wait for a server change.