fix(desktop): close three CI E2E flake mechanisms

[Astro-Han]

Summary

Three intermittent Desktop E2E failures on CI, each traced to a distinct mechanism rather than masked with timeouts or retries — plus a follow-on pass that moves the renderer-owned assertions those specs carried down to their lowest honest tier.

skill-draft-lifecycle — real product race (regressed by #5532). After session-state publication moved to per-row reads, the revision child can land in the renderer catalog before reviseBeforeTurn resolves. setActiveId then takes the deferred-handoff branch, leaving activeIdRef on the source session until the React transcript pipeline commits. prepareRevisionSend settles the child tail through a direct IPC read that wins that race, so its passive activeIdRef check treated the lagging handoff as a stale selection and rolled the send back silently — no send, no skill-resolution rejection toast, draft restored. Upstream #5494 has since removed that settlement gate entirely — the send no longer waits on React materialization, which resolves the same race by decoupling — so this branch carries no product change here; the spec's renderer-owned contracts still migrate to action-seam tests below.

side-chat-followups — test-side race, not a product regression. The spec pressed Enter on the third queued follow-up and clicked edit on the first within ~100ms. beginEdit captures the queue revision at click time, so when the third entry's Host admission landed inside that window the update carried a stale expectedQueueRevision and was correctly rejected with operation_conflict. The failed commit leaves the edit textarea open, which replaces the row's .maka-composer-queue-text span — the queue never reordered; the first entry was hidden behind its own edit box (confirmed by the failure screenshot showing the error banner plus the open textarea). The spec waits for Host-owned queued state before touching a row.

workhub-layout — native menu lifecycle. Two CI runs died mid-assertion after closePopup() with the renderer CDP session closed. popupNativeMenu never retained the Menu it built, so a wrapper collected while its popup is open can crash the native close path (electron#20737 family); each open menu is now held until its popup callback reports it closed. The spec also closed the popup unconditionally — on Linux a popup can auto-dismiss after window resizes (this spec resizes three times first), and closing a dead popup hits the same teardown crash. Since aria-expanded tracks the popup IPC resolution, closePopup is now called only while the menu still reports open. All user-real dismissal paths were verified unusable in the fixture (the window launches unfocused, and synthetic Escape/click do not reach the native menu tracking loop), so closePopup stays as the only programmatic path. The E2E fixture now logs every page crash — including the restarted window's — into the error context so a recurrence ships its stack.

E2E tier pass

The three specs carried assertions that never needed Electron; each moved to the lowest tier that can expose the defect.

Transferred down:

• skill-draft-lifecycle.spec.ts deleted (2 tests). Its assertions were renderer-owned: a refused send keeps the draft, a retry reuses the already-prepared child rather than forking a second revision, and cancel restores the pre-edit draft text (Skill token included). They now run in app-shell-revision-actions.test.ts against createAppShellRevisionActions with a stubbed bridge — deterministic, no Host or window. The blocked-Skill toast is asserted through createAppShellChatActions (app-shell-busy-race-settlement.test.ts). The success-path draft cleanup stays inline in sendWithAttachments — no production change on this path at all.
• side-chat-followups.spec.ts trimmed to its Electron boundaries. Row-level edit semantics — revision captured at beginEdit, rejection keeping the editor open instead of silently reading as reordered, action enablement gating on Host admission, and drag reorder passing the Host-owned id list — moved to ComposerMessageQueue component tests (packages/ui).

Kept in Electron:

• side-chat-followups: the real dragTo must pass the main-window drop guard (a capture listener previously swallowed drops — not reproducible in a fake DOM), the queue must drain in Host-observed order across real turn handoffs, and the transport-close → external-client-release → observation-reseed journey crosses renderer/preload/main/Host.
• workhub-layout unchanged: frameless-window geometry, BrowserWindow/WebContentsView coordination and the native menu lifecycle are the Electron surface under test.

Lost protection, honestly accounted: the setEnabled IPC → skills-catalog → chip re-render plumbing is generic preload wiring, not a revision mechanism; /skill: token ↔ chip serialization is covered by chat-input-behavior tests; the sendWithAttachments glue (unchanged-text guard, slash-command rejection, and the success-path draft cleanup) remains covered by behavior, not line; queue delete/promote argument wiring is no longer asserted at any tier. No fixture hooks were orphaned — invocableSkillsWindow still serves slash-command-compact.

Verification

• app-shell-revision-actions.test.ts + app-shell-busy-race-settlement.test.ts: 19/19 — upstream's settlement tests plus the new retry / cancel lifecycle tests. Targeted mutations fail their tests: removing the retry early-return (would fork a second revision) and restoring the wrong draft text.
• app-shell-busy-race-settlement.test.ts: refusal returns false and fires the blocked-Skill toast.
• composer-message-queue.test.tsx: 4/4 — mutations confirmed: ignoring the update result closes the editor (the original misread), and passing live queueRevision instead of the captured one defeats staleness detection.
• side-chat-followups 1/1 and workhub-layout 3/3 pass locally; check:e2e-budget 35 tests / 20 files; --strict-base architecture check passes.
• npm run format, npm run lint clean.
• Not run: the Linux+Xvfb parallel lane — the native-menu trigger is CI-platform-specific; if it recurs, the new crash logging captures the stack.

AI use

Select exactly one:

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

Tool(s) and scope: Devin — failure diagnosis from CI traces/artifacts, all code changes, the regression and migrated tests, and the tier migration.

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

[jackwener]

I reviewed dc3be31f6590e37228761d4fa5d198dfe6958dbc.

No P0–P2.

The only production change is retaining each native Menu until its popup callback runs, so the close path cannot collect it. The WorkHub spec now calls closePopup only while aria-expanded is still true. Side-chat waits for Host-admitted queue rows before dragging. Skill-draft E2E is gone; retry/cancel and the blocked-Skill toast now sit on createAppShellRevisionActions / createAppShellChatActions, and queue edit/conflict/reorder sit on ComposerMessageQueue tests. Those replacements fail on the old misreads.

Required check test is green on this head. I did not re-run the Linux+Xvfb native-menu lane.

简体中文

我审查了 dc3be31f6590e37228761d4fa5d198dfe6958dbc。没有 P0–P2。

唯一的产品改动是 native Menu 在 popup 回调前一直被持有。WorkHub 规格只在 aria-expanded 仍为 true 时 closePopup。Side-chat 等到 Host 接纳后再拖。Skill-draft E2E 删掉了，断言落到 action / component 测试。本 head 上 test 绿。Linux+Xvfb native-menu 那条线我没复跑。

---

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.