test(desktop): move P2 coverage below Electron

[Astro-Han]

Summary

• Move the P2 layout, focus, sidebar, composer, onboarding, and quote-selection contracts from Electron into Storybook play stories or focused UI unit tests.
• Keep only the two boundaries that still require Electron: physical pointer capture after leaving the renderer viewport, and sidebar grouping persistence across a renderer reload.
• Retire the warm transcript worker controls and scenario data that no remaining Electron test consumes. settings-permissions stays: the alignment audit renders it, and its snapshot fixture is what puts a multi-button row on the page at all. The E2E tier is now 35 tests in 17 files (rebased onto main with test(desktop): reduce P1 Electron coverage at owning boundaries #4803 landed).

The former minimum-window onboarding test used page.setViewportSize({ width: 480, height: 320 }) and asserted renderer scroll geometry; it did not exercise native BrowserWindow minimum-size enforcement. Its replacement therefore uses the same fixed viewport in Storybook.

Refs #4761

Verification

• npm run build:with-deps -w @maka/desktop — passed
• npm run build-storybook -w @maka/desktop — passed
• npm run smoke:storybook -w @maka/desktop — passed, 304 stories / 331 theme renders
• npm run typecheck -w @maka/desktop — passed
• npm run typecheck -w @maka/ui — passed
• npm run check:architecture -w @maka/desktop — passed, 101 tests
• node --test scripts/storybook-visual-smoke.test.mjs — passed, 8 tests
• node --test packages/ui/dist/session-setting-intent.test.js — passed, 9 tests
• npm run format / npm run lint — passed
• npm run astryx:surface-inventory — passed; npm run windows:inventory — passed
• node scripts/asf-license-headers.mjs check — passed
• npx playwright test --config e2e/playwright.config.ts e2e/quote-window-boundary.spec.ts e2e/sidebar-project-reload.spec.ts — 2 passed (macOS, local)
• node scripts/audit-alignment.mjs — all 12 fixtures clean
• Mutation checks proved the moved contracts fail for: premature plus-menu close, stale catalog reuse after a context switch, regressed sidebar nesting, dropped latest intent, uncleared queued intent, and uncaught catalog-refresh rejection.

The forced-colors focus-ring story needs the smoke runner to emulate forced-colors: active, so catalogJobs now carries an explicit forcedColors per job — explicit rather than optional, because the runner reuses one page across jobs and an unset value would leak the forced palette into the next render. storybook-visual-smoke.test.mjs pins both the default and the forced case.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented the test migrations, removed retired fixture paths, ran mutation checks, and prepared this PR. Claude Code rebased it onto main, resolved the conflicts against #4803 and #4840, restored the settings-permissions fixture the alignment audit still needs, and reran the verification above.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

Reviewed current head a48cbdf8fbab0868d08a4b95d042872ab13f972b (OPEN, MERGEABLE/BLOCKED awaiting human review). NO-GO — one P2 below, a cross-layer regression coverage gap. Checks: exact-head test SUCCESS; git diff --check and Storybook smoke-script tests 8/8 pass.

P2 — the sunk Electron coverage is replaced by a story that never touches the real boundary

The diff (43 files, +1140/-231) sinks multiple Electron/Playwright layout and interaction specs into Storybook and deletes the corresponding fixtures/IPC. The gap: the Plan/Skills story (app-shell.stories.tsx:1479-1494,1504-1543) uses only local state and a fixed setTimeout(150) — no Plan writes, no sessions.list, no newTasks.listInvocableSkills, no Electron bridge. Meanwhile the deleted composer-plus-menu-stability.spec.ts:245-406 covered real-bridge context switching, rapid toggling of last intent, post-refresh-rejection committed state and latest intent, and session-archive cleanup. The shared-worker unit test (session-setting-intent.test.ts:235-314) verifies only the abstract queue; the actual Desktop adapter (use-session-setting-intent.ts:115-120,175-176) and App-shell wiring (app-shell.tsx:1022-1029,3026-3032) are not mounted by the new story. The PR description's one-off mutation checks were not committed as a durable CI contract and cannot substitute for cross-layer regression. Fix: add a renderer integration harness mounting the real Desktop adapter/App-shell boundary with a deferred bridge (refresh rejection, last intent, context/archive cleanup, current Skills context) — or keep one minimal Electron smoke test. No new P0/P1/P3 correctness issues found.

Scope and limits

Base 8336c40f, latest origin/main eca7778b, merge-tree clean. Full build/typecheck/real Electron smoke not independently re-run (missing tsc/Vite/Storybook/Playwright/babel-parser in this environment); architecture tests could not start.

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

简体中文

评审结论来自自动化审查流程；发布者没有读这份 diff，核的是当前 head 有没有漂移、以及 exact-head 的门禁状态。当前 head 是 a48cbdf，未关闭。一条 P2：下沉的 Storybook 故事没挂真实边界，被删的 Electron 用例覆盖的跨层路径丢了。修好再合。

[jackwener]

Approving at exact head a48cbdf8. No P0 or P1: the six mutation claims in the description all reproduce, and fifteen of the moved contracts land where the description says they land. Two [P2]s below, both coverage-class. One review lane is still running and I will follow up separately rather than hold the approval for it.

What was checked rather than accepted

The six mutations reproduce, 6/6. Each was applied to production code only — no story, fixture, mock or test was touched, and each was reverted before the next, ending with no diff against the head:

Mutation	Result
composer.tsx:1985 forces hasCloseOnSelect=true on a loading Skill item	PlusMenuDuringSkillRefresh reddens at expect(menu).toBeVisible()
composer-mentions.tsx:152-154 keeps the old catalog across a context switch	ContextSwitchStartsWithALoadingCatalog reddens on the missing aria-busy=true
sidebar.css:255 nest back to 24px	ProjectGroups measures inset 24, the <16 assert reddens
drop existing.desired = value at session-setting-intent.ts:214	latest-intent test reddens, writes plan without agent
drop intents.delete(sessionId) in clear()	queued agent replays after settle, reddens
drop refreshCatalog().catch(...)	rejection test reddens on an unhandled catalog unavailable

"35 tests in 17 files" holds — 17 remaining e2e/*.spec.ts, 35 test( calls, no skips.

Fifteen destinations hold, using the strict criterion that the original expect exists and actually runs, not that a nearby story tests something related: agent-graph layout in ManyOperators, composer 1–2 in PlusMenuDuringSkillRefresh, composer 3 in ContextSwitchStartsWithALoadingCatalog, onboarding in NeedsConnection + NarrowWindow at the same 480×320, PartialHistoryNotice, PermissionCenterDiagnosticsExpanded, RelayConnectionDetail, the three focus-ring stories, Appearance, OverflowingSidebar, the Project Groups play, DefaultLayout + UpdateDownloadedCollapsed, and WideAssistantProse with width > 680 and a right gap ≤ 1. settings.spec.ts is not deleted; four Electron tests remain in it.

[P2] Four original assertions have no live destination

1. Gutter wheel. The old test moved the pointer onto the blank area right of .settingsMainPane and called mouse.wheel. GeneralWideShort sets pane.scrollTop = 600. The geometry survives — blankRight > 40, and the pane owns overflow-y: auto — but the assertion that the blank gutter is itself a scroll target does not.
2. Link colour across schemes. BotChatNeedsAttention keeps link === solid !== accent within the current scheme. The old test toggled light and dark in one run and asserted dark.link !== light.link. The smoke re-renders the story in dark, but never compares the two values, so a change that moved both schemes together would pass.
3. Composer Plan races 4–7. Races 1–3 are covered. Races 4–7 — last Plan click wins, a failed refresh keeps the box checked, Host collaborationMode follows the last ask, archive-while-pending drops the queue — moved to session-setting-intent.test.ts, which asserts overlayByChannel on the hook. It does not touch the ＋ row, window.maka.sessions.list()[0].collaborationMode, or archive-to-zero rows.
4. Sidebar pointer menu. Keyboard ordering, hover cards and the reload journey all hold. Gone: taskRow.hover() then clicking 任务操作, asserting row.matches(':hover') === false while a menu item is hovered, and mouse.click(4, 4) to dismiss.

Holes 1 and 4 are worth separating from the other two, because they are not geometry — they are pointer physics and a real hover state machine, the same category as the two boundaries this PR deliberately keeps in Electron. Whether Storybook can carry them at all is a fair question to answer explicitly rather than by omission.

[P2] The context-switch story does not pin the fence it claims

useComposerMentions has two independent defences: liveCatalog at composer-mentions.tsx:132-134 projects loading/empty synchronously on the render that carries the new contextKey, and the passive effect at :141-155 then clears state. The new story's own comment says the old Session catalog must be invalidated in the same render.

Deleting only the first defence — liveCatalog returning the stale catalog — leaves ContextSwitchStartsWithALoadingCatalog fully green. The story clicks to switch context and only then opens the ＋ menu, by which point the effect has already run, so it can only ever observe the second defence.

This does not overturn the broader mutation claim: mutating the effect's context-switch branch to { ...previous, contextKey } does redden the story. What is missing is discriminating power over the synchronous branch specifically. A focused hook or provider test — read the provider value after the key changes but before the passive effect flushes, and assert loading=true, settled=undefined, empty skills — would close it without restoring anything to Electron.

The shape both P2s share

Taken together, several composer and context-switch contracts are now guarded one level below where the user-visible behaviour lives — on the hook's state rather than on the row, the menu, or the persisted collaborationMode. Each individual move is defensible and the hook tests are real. It is the accumulation that is worth a maintainer's eye, because a regression in the wiring between hook and surface would now pass everything.

Neither P2 blocks. A third lane covering what the retired warm transcript worker controls owned, and the forcedColors cross-job claim, is still running; if it produces anything material I will post it as a follow-up rather than amend this.

简体中文

在 exact head a48cbdf8 上批准。无 P0/P1:描述里六条 mutation 主张全部复现,被搬走的契约有十五处确实落在描述所说的位置。 两条 [P2] 在下面,都属覆盖类。还有一条车道仍在跑,我不为它压住批准,有结果会另发。

被核过而不是被采信的部分

六条 mutation 复现 6/6。 每一条都只改生产代码 —— story、fixture、mock、测试一律未动,逐条改回,最终相对 head 无 diff:

变异	结果
composer.tsx:1985 强制 loading Skill item hasCloseOnSelect=true	PlusMenuDuringSkillRefresh 在 expect(menu).toBeVisible() 变红
composer-mentions.tsx:152-154 切上下文时保留旧 catalog	ContextSwitchStartsWithALoadingCatalog 因缺 aria-busy=true 变红
sidebar.css:255 嵌套改回 24px	ProjectGroups 实测 inset=24,<16 断言变红
删 session-setting-intent.ts:214 的 existing.desired = value	latest-intent 测试变红,只写 plan 缺 agent
删 clear() 里的 intents.delete(sessionId)	排队的 agent 在 settle 后被重放,变红
删 refreshCatalog().catch(...)	rejection 测试因未处理的 catalog unavailable 变红

「35 tests in 17 files」成立 —— 17 个 e2e/*.spec.ts、35 个 test(、无 skip。

十五处去处成立,判据从严:原来那条 expect 存在且真的会跑,而不是「附近有个 story 测了相关的东西」。settings.spec.ts 并未被删,里面还留着四条 Electron 测试。

[P2] 四条原断言没有真正的去处

1. 空白 gutter 的滚轮。 原测试把指针移到 .settingsMainPane 右侧空白处调 mouse.wheel;GeneralWideShort 则是设 pane.scrollTop = 600。几何仍在(blankRight > 40、pane 拥有 overflow-y: auto),但「空白 gutter 本身就是滚动目标」这条断言没了。
2. 链接色跨主题。 BotChatNeedsAttention 守的是当前主题内的 link === solid !== accent。原测试在一次运行里切换明暗并断言 dark.link !== light.link。smoke 会在暗色下重渲染该 story,但从不比较这两个值 —— 于是一个让明暗一起变动的改动会通过。
3. Composer Plan 竞赛 4–7。 1–3 已覆盖。4–7(最后一次 Plan 点击胜出、刷新失败仍保持勾选、Host collaborationMode 跟随最后一次请求、pending 期间归档丢弃队列)搬到了 session-setting-intent.test.ts,它断言的是 hook 上的 overlayByChannel,不碰 ＋ 行、不碰 window.maka.sessions.list()[0].collaborationMode、也不碰归档清零。
4. 侧栏指针菜单。 键盘顺序、hover 卡片与重载 journey 都还在。没了的是:taskRow.hover() 后点「任务操作」、菜单项被 hover 时断言 row.matches(':hover') === false、以及 mouse.click(4, 4) 关闭菜单。

第 1 和第 4 条值得与另两条分开看,因为它们不是几何,而是指针物理与真实 hover 状态机 —— 与本 PR 刻意保留在 Electron 的那两个边界属于同一类。Storybook 究竟能不能承载它们,值得明确回答,而不是默认略过。

[P2] context-switch 的 story 并没有钉住它自己声称的那道 fence

useComposerMentions 有两层独立防线:composer-mentions.tsx:132-134 的 liveCatalog 在携带新 contextKey 的那一次 render 上同步投影为 loading/empty;:141-155 的 passive effect 随后再清 state。新 story 自己的注释写明旧 Session catalog 必须在同一次 render 内失效。

只删掉第一层 —— 让 liveCatalog 返回陈旧的 catalog —— ContextSwitchStartsWithALoadingCatalog 仍然全绿。原因是 story 先点击切换上下文、之后才打开 ＋ 菜单,那时 effect 已经跑完,所以它永远只能观察到第二层。

这不推翻更宽的那条 mutation 主张:把 effect 的 context-switch 分支改成 { ...previous, contextKey } 确实会让该 story 变红。缺的是对同步分支本身的区分力。 一条聚焦的 hook/provider 测试即可补上 —— 在 key 变更之后、passive effect flush 之前读 provider 值,断言 loading=true、settled=undefined、skills 为空 —— 不需要把任何东西搬回 Electron。

两条 P2 共有的形状

合起来看,composer 与 context-switch 的若干契约现在被守在比用户可见行为更低的一层 —— 守在 hook 的 state 上,而不是守在行、菜单或已持久化的 collaborationMode 上。单看每一次搬移都站得住,hook 测试也是真的;值得维护者留意的是累积效应,因为 hook 与界面之间接线的回归,如今会全部通过。

两条 P2 都不阻塞。另有一条车道仍在跑(退役的 warm transcript worker 控件原本拥有什么,以及 forcedColors 的跨 job 主张);若它产出实质内容,我会另发一条,而不是修改本条。

---

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

[Astro-Han]

Rejected. Two of the claims are factually wrong.

Context switching is covered: ContextSwitchStartsWithALoadingCatalog in composer-slash-menu.stories.tsx mounts the real ComposerMentionsProvider and defers the projection load on a promise, asserting what composer-plus-menu-stability.spec.ts:245-275 did. The review only read app-shell.stories.tsx.

session-setting-intent.test.ts is not an abstract queue: it mounts the real useSessionSettingIntent under React, which is the sole authority for ordering, latest-intent draining and per-session clear. On the Plan path the Desktop adapter has no logic — setPlanMode is intent.request('planMode', …) and the channel is write: input.planMode.write.

What is no longer asserted end to end is two unconditional wiring calls (app-shell.tsx:970, onPlanModeChange). P3, not merge-blocking, and a new App-shell integration harness is the apparatus #4761 exists to retire — the renderer architecture ratchet refuses that seam anyway.

No change to the diff.

[Astro-Han]

Thanks — the mutation table and the destination audit are the useful parts, and the liveCatalog finding is right. Measured against what each assertion actually owns, I'm not adding tests for the four holes.

Gutter wheel. Already covered. GeneralWideShort asserts the gutter lies inside .settingsMainPane's box (paneRect.right - contentRect.right > 40), that the pane owns overflow-y: auto while .astryx-layout-content does not, and that scrollHeight > clientHeight. A wheel over the gutter scrolling the pane follows from those four by CSS semantics; asserting it again measures the same facts through a pointer.

Link colour across schemes. Dropped deliberately. dark.link !== light.link only rejects a total collapse of the pair — a palette that moved to the wrong colour still passes — and the pair is generated by build-astryx-theme.mjs from the upstream palette rather than written here. Each scheme's real claim, link === solid !== accent, is already asserted in both renders. Protection dropped: a --link that stops being scheme-aware.

Plan races 4–7. What remains unasserted is that onPlanModeChange reaches intent.request and clearSessionRendererState reaches intent.clear — two unconditional calls. That is a wiring check, not a contract; the contract (ordering, latest-intent draining, per-session clear) is asserted against the real hook. Persisted collaborationMode and archive-to-zero rows need Electron, which is the budget this PR is spending.

Sidebar pointer menu. Dropped deliberately. row.matches(':hover') === false once the pointer sits on a portaled menu is Chromium's semantics, and mouse.click(4, 4) dismissal is Astryx's; the keyboard path you confirmed is the part we own. Protection dropped: the row's hover affordance and outside-click dismissal.

liveCatalog's synchronous fence. Correct, and I'm recording it on #4761 rather than papering over it here. A hook-level test is the right seam and none exists: apps/desktop's test:dist covers dist/main/** only, and the renderer ships as a Vite bundle with no per-file output. Opening that seam is its own change.

No change to the diff.