refactor(desktop): move root diagnostics below AppShell

[chihumyum]

Summary

Refs #4582 — M5, diagnostics and startup notices.

AppShell reached Desktop diagnostics directly in three places: the error toast's report action (diagnostics.copyReport) and the previous-shutdown notice (takePreviousMainProcessInterruption / copyPreviousMainProcessInterruption), whose ref and effect lived in AppShellContent. Move both into a features/diagnostics slice:

• DiagnosticReportToastProvider is the renderer's toast layer and builds the report action from the injected copyToastReport service. AppShell supplies only the three labels, which stay in the shared shell catalog beside the Error Boundary and command-palette copy that use the same words.
• PreviousMainProcessInterruptionNotice owns the read, the shown-once guard and the toast. It is mounted beside the always-mounted ConversationLifecycle and still waits for appearance hydration. Its copy moves out of shell-remaining-copy.ts unchanged.
• platform/desktop/create-diagnostics-services.ts is the only adapter, and feature-services composition is its only production importer. It forwards only the fields the toast carried, as AppShell did.

AppShell loses its three diagnostics bridge paths (9 → 6), one useEffect (hook gate 28 hooks / 43 → 42 call sites), one useRef and 262 non-trivia tokens. The renderer architecture ratchet rejects those bridge paths if they come back to app-shell.tsx.

Not in this PR: the Error Boundary's crash report, the command palette's manual report (app-shell-command-actions.ts, which #5712 is rewriting) and About's manual report still call the bridge from legacy files. None of them runs in AppShell's render body.

Verification

• Desktop: 3,103/3,103 (Node 24, build:test + test:dist), including 8 new tests in diagnostics-owner.test.ts: the notice reads nothing before ready, shows once and copies its report; shows nothing after a clean shutdown; a locale change while the read is pending shows one notice, in the current locale; error toasts offer the report and copy it through the injected service; a failed copy shows the supplied labels; the adapter forwards only the fields present and delegates the previous-run calls; each owner is mounted once and the adapter is the only Desktop path.
• Mutations: removing the shown-once guard, the cancellation check, the ready gate or the adapter's empty-field filtering each fails one of those tests; the restored implementation passes.
• Desktop typecheck (preload, main, renderer, storybook), lint, format:check, Knip (desktop, ui), check:asf-headers, check:locale-hygiene --base, windows:inventory, check:app-shell-hooks, check:renderer-architecture --base c838e1fa7 --strict-base and the Astryx surface inventory (regenerated for the three new files) pass.
• Electron: rename-focus.spec.ts and zh-tw-locale.spec.ts pass (2/2) on a local build, so the app boots with the new provider tree and survives a runtime locale switch. No Electron test exercises the report action or the notice; this PR adds none, per e2e/AGENTS.md.
• Not run: a packaged build or a manual interrupted-shutdown reproduction. The Desktop side of the notice is covered by the existing main-process-diagnostics.test.ts.

AI use

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

Tool(s) and scope: Claude Opus 5.5 (in Claude Code) implemented this ownership move, its tests, documentation and validation under human direction. The commit carries Generated-by: Claude Opus 5.5.

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 at b46d8454 against main. No P0–P3 found.

The move is scoped to two things — the error-toast "Copy report" action and the previous-main-process "recovered" notice — so the ErrorBoundary, crash/renderer-failure reporting and startup errors are untouched. DiagnosticReportToastProvider takes the exact slot ToastProvider held, so provider order is unchanged (Locale > AstryxLocale > toast > ErrorBoundary > AppUpdate), and the services provider mounts in main.tsx above the only AppShell render. The report payload matches field-for-field; the recovered notice keeps its once-per-renderer, settings-ready gate, 10 s duration and unmount cancellation, and is always rendered. Copy strings are identical across locales, and there are no subscriptions to leak.

Hook gate (AppShell useEffect 3→2), renderer architecture check (three diagnostics bridge paths removed from AppShell's allowance), renderer typecheck, and 93 focused tests pass locally. Minor gap only: no test renders AppShell through the real provider tree, which is low risk since the services lookup throws if the provider is missing.

Not verified: the real toast after an unclean shutdown, and the Storybook typecheck.

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

[hqhq1025]

Reviewed commit b46d84547225fd07fe4c4bfab8a75e89237710e4. The change moves the error-toast diagnostic action into DiagnosticReportToastProvider (apps/desktop/src/renderer/features/diagnostics/ui/diagnostic-report-toast-provider.tsx:36-54) and the previous-main-process interruption notice into its own always-mounted component (apps/desktop/src/renderer/features/diagnostics/ui/previous-main-process-interruption-notice.tsx:38-65). AppShell keeps the same toast-provider position and the appearance-hydrated gate (apps/desktop/src/renderer/app-shell.tsx:189-228, :1594); the adapter preserves the toast report fields and previous-run bridge calls (apps/desktop/src/renderer/platform/desktop/create-diagnostics-services.ts:26-39). I found no substantiated P0-P3 issue in the inspected paths.

Node 24 full build:test, 28 focused Desktop tests, Desktop typecheck, renderer architecture check (121 fixtures plus ledger), AppShell hook check, and Astryx inventory check (324 files plus 23 inventory tests) passed locally. Current-head hosted test and label are green; fetched main c838e1fa merges cleanly and git diff --check passed. I did not exercise a packaged Electron unclean-shutdown recovery notice, a real clipboard denial, or the full local test suite.

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

[hqhq1025]

Reviewed ae174d73840fd1224f015295ddf56dc8b003eb81 against fetched main 1e80e3b885d963b799f7e2e9a70083ceda99943d. I found no substantiated P0–P3 issue in the inspected paths.

The 17-file change extracts the error-toast report action and previous-main-process interruption notice from AppShell. The toast provider keeps its existing position (apps/desktop/src/renderer/app-shell.tsx:192); the notice remains gated by appearance hydration (apps/desktop/src/renderer/app-shell.tsx:1588). The service adapter preserves the toast report fields (apps/desktop/src/renderer/platform/desktop/create-diagnostics-services.ts:31), and singleton service construction remains above AppShell (apps/desktop/src/renderer/composition/desktop-feature-services.tsx:82). I checked all changed production files, locale strings, feature/inventory boundaries, and added tests. The effect cancellation and shown flag preserve current-locale, at-most-once delivery within the mounted notice (apps/desktop/src/renderer/features/diagnostics/ui/previous-main-process-interruption-notice.tsx:39), with the unchanged preload promise cache supplying a stable previous-run result (apps/desktop/src/preload/preload.ts:3966). No storage schema or wire-protocol change is introduced.

Node 24.18.1 full build:test, 28 focused diagnostics tests, Desktop typecheck, renderer architecture checks (121 tests and ledger), AppShell hook check, and Astryx inventory checks (325 files and 23 tests) passed locally. The current-head hosted test is successful; git diff --check and merge-tree against fetched main are clean. Both prior reviews bind b46d8454, not this head. GitHub still reports the PR as blocked, so this comment is not merge authorization.

Not verified: packaged Electron recovery after an actual unclean shutdown, native clipboard denial, or the full local test suite.

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

[Astro-Han]

Reviewed cccb676102684e7fddd5e20f6926e5f4489b19bf (17 files, +765/−75; three commits — one refactor and two merges of main).

No P0–P3 findings.

What the change does. Root diagnostics move out of app-shell.tsx into a features/diagnostics/ owner: the shell keeps a single import * as Diagnostics and loses the ToastProvider error-action object, the previous-interruption effect and the shell-remaining-copy import (−50 lines), while the feature gains a port type, a services context, a Desktop adapter, two UI owners, its own locale catalog and a README.

The extraction is faithful, which is the thing to check in a move like this.

• The interruption notice keeps the old semantics exactly: the ready gate, the cancelled guard, and the once-per-renderer shownRef, with the same toast payload (variant, duration 10 s, and the copy-diagnostics action) and the same failure log. It renders null and only owns the read, as its comment states.
• DiagnosticReportToastProvider rebuilds the ToastErrorAction on the labels and documents why the memo matters — the injected services are created once at composition, so toast consumers do not see a new action on unrelated renders.
• The Desktop adapter maps the report field for field with the same conditional spreading the old inline action used: surface, title, description, diagnosticDetails → details, diagnosticTarget → target. This is where a field would usually get dropped in such a move, and nothing is.
• The port type derives the report shape from the UI contract itself (Parameters<ToastErrorAction['onClick']>[0]), so the port cannot drift from what the toast actually hands over — a better arrangement than the shell-local copy it replaces.
• The allow-list script is tightened rather than loosened: scripts/check-app-shell-hooks.mjs lowers app-shell's useEffect allowance from 3 to 2, matching the hook that left, and the architecture manifest drops the three window.maka.diagnostics.* bridge paths from app-shell along with the useRef count.

Tests cover the semantics that a copy-paste relocation would quietly break, including "reads nothing until ready, then shows the notice once and copies its report", a locale change during the read still showing one notice in the current locale, and "sends a toast report with only the fields the toast carried".

On the delta since the previous review. The branch's own commits stop at the refactor; everything after it is merges of main. I checked that the merges leave the branch's own work untouched — the only file they touch from this change is the architecture manifest, where both sides' edits land together — so the refactor still reads as reviewed at this head.

Gate on this head: test is green.

What I could not judge

• No Electron run: the interruption notice and the toast action are verified by reading the extraction plus their tests, not by exercising a real previous-session interruption.
• I did not run the suites myself.

I did not approve, request changes, or merge.

---

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.

[Astro-Han]

Thanks @chihumyum — a faithful extraction with tests that pin exactly the semantics a move like this could break. Merging now.

[Astro-Han]

Approved at @Astro-Han's explicit request: the automated reviews of this PR found no P0–P3 issues at this head, CI is green and the branch has no conflicts.