redesign(desktop): rebuild the subagent settings page on the settings route idiom by Astro-Han · Pull Request #2008 · apache/maka

Astro-Han · 2026-08-03T12:05:50Z

Summary

#1999 shipped 设置 · 子 Agent as dense rows over a 560px Dialog holding eight fields behind an inner scrollbar, plus three hand-written .subagentPreset* CSS rules. A modal exists to interrupt the current task for something short and immediately decidable; naming a capability, writing the guidance the main agent selects on, and picking a connection/model/thinking route is none of those. Astryx says the same — "if the content grows beyond what fits, consider a full page instead" — and the providers panel next door already answers this exact shape with a route level, so this page follows it instead of inventing a second answer.

Structure — the editor becomes the page's second level (list ── editor), reusing settingsRouteLevel and the back-header Toolbar. RouteHeader, private to ProvidersPanel, moves to settings-route-header.tsx as the one way back from any settings sub-level, so the shape is not written twice.

Zero page CSS — all three rules are gone, each a restatement of something the kit or Astryx already draws: the flex-wrap action cluster is the row end slot, the oklch-tinted callout is Banner status="warning", the flex-end button row is SettingsActions.

Fewer elements

A row carries the name, the guidance, and a badge only when the preset cannot be selected — a list where every row says 可用 says nothing. Route, id, and capability boundary are the editor's answer, one level in.
The list group loses its title and lede, which restated the page heading verbatim. The 64-preset ceiling speaks only once it is hit.
立即启用 leaves the editor: whether the main agent may select a preset is the list row's switch, which was already answering it.
A new preset's subagent_id starts empty (typing the name fills it) instead of pre-deriving subagent; a saved one reads as a settled row value, not a disabled input.
Delete moves out of the row into its own trailing section, so nothing quiet sits beside the destructive action.

Presentation only: availability, validation, id immutability, and the settings.update({ subagents }) contract are unchanged.

Refs #1999. This is the first of three planned steps; the delegation runtime surface (workbar Subagents tab, sidebar convergence) is #1457 and lands separately.

Verification

typecheck · lint · format:check · check-dead-css · check-a11y/check-copy/check-console · 1379 desktop unit tests · Storybook smoke (67 manifest checks, 75 catalog renders) · settings.spec.ts + providers.spec.ts e2e (5 passed).

The e2e journey now walks the route level instead of a dialog. Stories gain SubagentEditorExisting, covering the three states the create form cannot show: the settled read-only subagent_id, the implementation capability warning, and the delete section.

…e what the rebuild dropped Three-way review of #2008 (codex + two agents) against the providers panel it copies. The rebuild took that panel's route-level shell without its route controller, and traded away two capabilities to look simpler. Route controller, at the owner: - focus follows the level. The level carried `tabIndex={-1}` and a comment claiming it takes focus on landing, but nothing ever called `focus()`: clicking the chevron unmounted the button that had it, so a keyboard user restarted from `document.body` on every move. The Dialog this replaced got this for free. Entering focuses the level (not the back button — an IconButton pops its tooltip on focus), returning goes back to the row the user left from, falling back to the add button when that row is gone, and first render does not pull focus off the settings nav at all. - an edit route whose preset vanished used to render as a BLANK CREATE FORM that appended a second preset on save, because "no presetId" and "presetId that no longer resolves" were the same `null`. The route is now a discriminated union and an unsatisfiable edit derives back to the list, as ProvidersPanel:194 already spells out. Restored: - a preset can again be created disabled. Dropping the editor switch left a real window where a just-saved preset is selectable before the user can reach the list row, and the reviewers were right that two controls over one state are two entrances, not two authorities. The switch is back on create only; for an existing preset the header states the disabled fact via the badge slot providers already uses for its default connection. - `SubagentEditor` story. It was deleted under a rule written in this same PR, worded widely enough to cover a whole second route level — the wrong direction. `play` does run in the browser, so that story was the editor's only automated render; without it the entire `preset === null` branch and every degraded-route state rendered nowhere. Corrections to what this PR asserted: - FIDELITY.md claimed the smoke script renders every story at three viewports. It renders the seven manifest surfaces that way and every other story once, at 1280 light (`catalogJobs`). The rule is rewritten around what actually renders, and the "play does not execute" section is corrected to what is true: it runs, nothing collects its assertions. - the id read-only row hand-rolled `<code class="settingsReadOnlyValue">`, missing `data-mono`, so a 128-char id rendered as body type in a 320px right-anchored box. It uses the kit's `SettingRow mono` path. - the list section header lost its title, leaving an empty header with a divider under it and dropping the only count under a 64-preset ceiling. Title and count are back, and the ceiling reads from MAX_SUBAGENT_PRESETS rather than a hardcoded 64 in copy. - the empty state showed two identical add buttons; the section action now yields to the EmptyState CTA. - three imports left dead in ProvidersPanel by the RouteHeader extraction (biome has no noUnusedImports, tsc no noUnusedLocals — CI cannot see them). - `settingsSubagentsPage` was a class with no rule, on a page whose header says it owns no CSS; the save/cancel row was a title-less SettingsSection rendering neither header nor divider around one HStack; the danger section repeated the confirm dialog's sentence verbatim. Tests, for the regressions that shipped green: - e2e seeds the preset DISABLED, so saving an unrelated field proves `enabled` survives — the single line holding up the switch removal. - a create journey: id derives from the name, create-disabled persists as false, and the list row's switch enables it. - focus assertions on both level changes and on arrival. - `nextSubagentDraftForName` extracted and unit-tested, including the `idWasEdited` takeover the UI wiring had no coverage for. - availability now asserts `tone` with `kind`; a broken route quietly turning green was a pure-data regression with no other home. - the story fixture gains an enabled preset whose connection is gone, so the destructive badge renders somewhere (the disabled one short-circuits first). Verified: typecheck, lint, format:check, check-dead-css, check-story- annotations, check-a11y/copy/console, 1382 unit tests, Storybook smoke, and settings + providers e2e (6 passed).

… route idiom #1999 shipped 设置 · 子 Agent as a list of dense rows over a 560px Dialog holding eight fields behind an inner scrollbar, plus three hand-written `.subagentPreset*` CSS rules. A modal exists to interrupt the current task for something short and immediately decidable; naming a capability, writing the guidance the main agent selects on, and picking a connection/model/ thinking route is none of those — Astryx says the same ("if the content grows beyond what fits, consider a full page instead"), and the providers panel next door already answers this exact shape with a route level. - the editor becomes the page's second level (list ── editor), reusing `settingsRouteLevel` + the back-header Toolbar rather than a Dialog; - `RouteHeader`, private to ProvidersPanel, moves to `settings-route-header` as the one way back from a settings sub-level, so the shape is not written twice; - the page drops all three CSS rules: the flex-wrap action cluster is the row end slot, the oklch-tinted callout is `Banner status="warning"`, and the flex-end button row is a plain action row; - rows carry the name, the guidance, and a badge ONLY when the preset cannot be selected — a list where every row says 可用 says nothing. The route, the id, and the capability boundary are the editor's answer; - the list group loses its title and lede, which restated the page heading verbatim; the preset ceiling speaks only once it is hit; - 立即启用 leaves the editor: whether the main agent may select a preset is the list row's switch, which was already answering it; - a new preset's subagent_id starts empty (the name fills it) instead of pre-deriving `subagent`, and a saved one reads as a settled row value rather than a disabled input; - delete moves out of the row into its own trailing section, so nothing quiet sits beside the destructive action. Presentation only: availability, validation, id immutability, and the `settings.update({ subagents })` contract are unchanged. Verified: typecheck, lint, format:check, check-dead-css, check-a11y/copy/ console, 1379 desktop unit tests, Storybook smoke (75 renders), and settings + providers e2e. Story gains SubagentEditorExisting for the read-only id, the implementation warning, and the delete section.

…er subagent level Two follow-ups from reviewing the rebuilt page in Storybook. The settings story harness wrapped `SettingsSurface` in `height: '100%'`. `SettingsSurface` is a `Layout height="fill"`, which needs a bounded ancestor before it hands its content pane a scroll box; under Storybook's fullscreen body a percentage height resolves against an auto-height parent, so every page taller than the frame stretched the whole surface instead of scrolling inside it. 权限与能力 reached 1942px in a 720px frame with no way down, and 记忆 and the subagent editor were stuck the same way — a pre-existing hole that only shows on a page long enough to need it. `100dvh` restores the scroll box: the editor's content pane now measures 582/1005 with `overflow-y: auto`, and the delete section is reachable. The subagent page also arrived with four stories where the settings convention is one per page plus a variant for a state the page cannot otherwise show. The narrow variant had nothing to say now that the row's trailing cluster is a switch and a chevron, and the create form is the edit form minus three elements, so both are gone: 子 Agent keeps its list story and one editor story, on the preset that exercises the most of it. Verified: typecheck, lint, Storybook smoke (67 manifest checks, 73 catalog renders across three viewports), and a scroll probe on the editor level.

…l in e2e The editor story was the list story plus one click — same bridge, same render, one `play`. A reviewer reaches the same screen by clicking the row, so 子 Agent keeps one story, as the settings convention has it. What the story was covering, the e2e now states outright: the level carries the settled subagent_id and the delete action, the two things the list row deliberately does not show. That path already ran in a real Electron window; it just was not asserting the level's shape. Verified: typecheck, lint, Storybook smoke (72 catalog renders), settings e2e (3 passed).

The subagent page arrived with four stories for one surface. FIDELITY.md is where the Product/* story conventions live, so the rule goes there rather than into a second document beside it: a surface gets one story, and earns another only for a state that story cannot reach.

…e what the rebuild dropped Three-way review of #2008 (codex + two agents) against the providers panel it copies. The rebuild took that panel's route-level shell without its route controller, and traded away two capabilities to look simpler. Route controller, at the owner: - focus follows the level. The level carried `tabIndex={-1}` and a comment claiming it takes focus on landing, but nothing ever called `focus()`: clicking the chevron unmounted the button that had it, so a keyboard user restarted from `document.body` on every move. The Dialog this replaced got this for free. Entering focuses the level (not the back button — an IconButton pops its tooltip on focus), returning goes back to the row the user left from, falling back to the add button when that row is gone, and first render does not pull focus off the settings nav at all. - an edit route whose preset vanished used to render as a BLANK CREATE FORM that appended a second preset on save, because "no presetId" and "presetId that no longer resolves" were the same `null`. The route is now a discriminated union and an unsatisfiable edit derives back to the list, as ProvidersPanel:194 already spells out. Restored: - a preset can again be created disabled. Dropping the editor switch left a real window where a just-saved preset is selectable before the user can reach the list row, and the reviewers were right that two controls over one state are two entrances, not two authorities. The switch is back on create only; for an existing preset the header states the disabled fact via the badge slot providers already uses for its default connection. - `SubagentEditor` story. It was deleted under a rule written in this same PR, worded widely enough to cover a whole second route level — the wrong direction. `play` does run in the browser, so that story was the editor's only automated render; without it the entire `preset === null` branch and every degraded-route state rendered nowhere. Corrections to what this PR asserted: - FIDELITY.md claimed the smoke script renders every story at three viewports. It renders the seven manifest surfaces that way and every other story once, at 1280 light (`catalogJobs`). The rule is rewritten around what actually renders, and the "play does not execute" section is corrected to what is true: it runs, nothing collects its assertions. - the id read-only row hand-rolled `<code class="settingsReadOnlyValue">`, missing `data-mono`, so a 128-char id rendered as body type in a 320px right-anchored box. It uses the kit's `SettingRow mono` path. - the list section header lost its title, leaving an empty header with a divider under it and dropping the only count under a 64-preset ceiling. Title and count are back, and the ceiling reads from MAX_SUBAGENT_PRESETS rather than a hardcoded 64 in copy. - the empty state showed two identical add buttons; the section action now yields to the EmptyState CTA. - three imports left dead in ProvidersPanel by the RouteHeader extraction (biome has no noUnusedImports, tsc no noUnusedLocals — CI cannot see them). - `settingsSubagentsPage` was a class with no rule, on a page whose header says it owns no CSS; the save/cancel row was a title-less SettingsSection rendering neither header nor divider around one HStack; the danger section repeated the confirm dialog's sentence verbatim. Tests, for the regressions that shipped green: - e2e seeds the preset DISABLED, so saving an unrelated field proves `enabled` survives — the single line holding up the switch removal. - a create journey: id derives from the name, create-disabled persists as false, and the list row's switch enables it. - focus assertions on both level changes and on arrival. - `nextSubagentDraftForName` extracted and unit-tested, including the `idWasEdited` takeover the UI wiring had no coverage for. - availability now asserts `tone` with `kind`; a broken route quietly turning green was a pure-data regression with no other home. - the story fixture gains an enabled preset whose connection is gone, so the destructive badge renders somewhere (the disabled one short-circuits first). Verified: typecheck, lint, format:check, check-dead-css, check-story- annotations, check-a11y/copy/console, 1382 unit tests, Storybook smoke, and settings + providers e2e (6 passed).

…s to Second review round on this PR found the page disagreeing with three owners at once, so this fixes them at those owners rather than at the symptoms. The store: `normalizeSubagentSettings` DROPS a preset it dislikes instead of rejecting the write. A name past 128 characters therefore resolved the promise, returned the user to the list, and deleted the row they had just edited — with no error anywhere. The limits are now exported from core and enforced where the fields change, and `persist` reads the result back instead of trusting a resolved promise, which also covers a preset created into a list that filled up elsewhere. Conversely the store makes `description` optional and the list has a fallback line for it, so requiring it here made a legal preset uneditable until the user wrote prose; that requirement is gone. The id: an existing preset's id now comes from the preset at submit time, not from the draft. `idWasEdited` was the only thing standing between a rename and a silently re-keyed preset, and the branch it guarded does not even render the field — the flag disarmed a trap that no longer exists. The route controller: `useSettingsRouteFocus` is now shared with ProvidersPanel, which had the same 48 lines of arrive-vs-navigate, rAF, return-focus and fallback logic written out separately. A focus rule is the one duplication that cannot be seen on screen when the two copies drift. The unsatisfiable-edit-route rule moves out with it into `resolveSubagentRoute`, where it can be tested — its regression (a vanished preset rendering the create branch, so saving appended a second preset) had no coverage at all. Also: `enabled` had three authorities (a list switch, a create-only switch, and a read-only badge on the level that owns deletion) — the editor now carries it in both branches and the badge is gone; the route's two choices get two errors instead of one aimed at the connection; `contract` was a required prop with zero consumers; `.settingsRouteLevel` moves out of the models stylesheet to the shared one, since it now has two owners. Tests: the rename that could re-key a preset, the delete flow in both directions (cancel keeps it; confirm returns to the list and lands focus on the fallback), and arrival focus asserted positively on the settings nav item.

FIDELITY.md said a story whose `play` throws "still ships green" and "never could" fail. It is the opposite: `storybook-visual-smoke.mjs` subscribes to `playFunctionThrewException` and `unhandledErrorsWhilePlaying` and collects the console error too, so a throwing play function fails the smoke run and CI — verified by adding a probe story that throws unconditionally and watching the run exit 1. The script's own header says the same thing from the other side: that pass exists BECAUSE a build alone would let such a story through. What `play` genuinely cannot do is report: with no test addon a failure names the story, not the assertion, the state, or the expectation — and reaching it costs a Storybook build. That is the reason to put contracts elsewhere, and it is what the section now says. The viewport claim was overstated in the same paragraph: manifest surfaces declare their own viewports and colour schemes, with a written opt-out for each one skipped, rather than all rendering at three widths across light and dark. The subagent editor story's annotation claimed it "exercises" four states it only renders; the e2e journeys are what pin them.

…y drifts Third review round. The hook landed with eight options, three of which the subagent page degenerated (a `focusSelectors` that ignores its argument, no `routeKey`, no `isReady`) — and the total came out longer than the duplication it removed. It now shares only the mechanism that drifts invisibly: skip the first render, one animation frame, `preventScroll`, cancel on the way out. Which element a level focuses goes back to the page, because a wrong target puts the ring somewhere you can see. Its `resolveTarget` is also read through a ref written in an effect rather than during render. Deleting the effect that committed the derived route back into state: it was the same rule as `resolveSubagentRoute` said twice, it contradicted ProvidersPanel — which answers the identical vanished-target case by deriving alone — and it had a window where a click landing between commit and effect would be overwritten. The three claims the diff made that the code did not support: - Three e2e comments said the editor no longer carries an enable switch for an existing preset. It carries one in both branches; that was this PR's own last round. The tests are fine, the reasons written on them were not. - "An IconButton opens its tooltip on focus, so focusing one on arrival would pop a tooltip at every mouse user" is false in both files that said it: Astryx's `handleFocusIn` returns early unless `:focus-visible` matches, and says so in a comment. Focusing the region is still right; the reason is that a named region is what gets announced. - The unit test explained itself with a bug that only existed in this PR's own intermediate commit, which no reader after merge can see. It now states the present-tense structure: `preset: null` IS the create branch. Coverage: breaking any of the three providers focus branches left CI green, so the catalog landing, the detail region landing, and the return-to-row are now asserted there — each verified to fail when its branch is broken. The create journey also takes the id over before renaming, which is the half of the derivation contract the component never exercised. Also: an existing preset's id is no longer derived at the call site either, not just ignored at submit; the back affordance is disabled mid-save, where leaving discarded a draft the failed write could not give back; a thinking level whose row is off screen is no longer submitted; the id limit is interpolated from `SUBAGENT_PRESET_ID_MAX_CHARS` instead of spelled out in two locales; name and description truncate the way the store measures them, after trimming; and the providers detail is a named region too, since it now shares this header.

Astro-Han · 2026-08-03T12:05:50Z

Summary

#1999 shipped 设置 · 子 Agent as dense rows over a 560px Dialog holding eight fields behind an inner scrollbar, plus three hand-written .subagentPreset* CSS rules. A modal exists to interrupt the current task for something short and immediately decidable; naming a capability, writing the guidance the main agent selects on, and picking a connection/model/thinking route is none of those. Astryx says the same — "if the content grows beyond what fits, consider a full page instead" — and the providers panel next door already answers this exact shape with a route level, so this page follows it instead of inventing a second answer.

Structure — the editor becomes the page's second level (list ── editor), reusing settingsRouteLevel and the back-header Toolbar. RouteHeader, private to ProvidersPanel, moves to settings-route-header.tsx as the one way back from any settings sub-level, so the shape is not written twice.

Zero page CSS — all three rules are gone, each a restatement of something the kit or Astryx already draws: the flex-wrap action cluster is the row end slot, the oklch-tinted callout is Banner status="warning", the flex-end button row is SettingsActions.

Fewer elements

A row carries the name, the guidance, and a badge only when the preset cannot be selected — a list where every row says 可用 says nothing. Route, id, and capability boundary are the editor's answer, one level in.
The list group loses its title and lede, which restated the page heading verbatim. The 64-preset ceiling speaks only once it is hit.
立即启用 leaves the editor: whether the main agent may select a preset is the list row's switch, which was already answering it.
A new preset's subagent_id starts empty (typing the name fills it) instead of pre-deriving subagent; a saved one reads as a settled row value, not a disabled input.
Delete moves out of the row into its own trailing section, so nothing quiet sits beside the destructive action.