From 135f7b4f8d5c0fdd512d6923c2d03b7bee05f79d Mon Sep 17 00:00:00 2001 From: chihumyum Date: Fri, 2 Oct 2026 17:30:29 +0800 Subject: [PATCH 1/4] refactor(desktop): route legacy diagnostics reports through the diagnostics feature The Error Boundary, About and the command palette still called window.maka.diagnostics.copyReport from legacy renderer files after #5892 moved the root's diagnostics into features/diagnostics. Each now receives one fixed-surface command from the feature's services instead: - the Error Boundary takes copyRendererCrashReport from an optional RendererCrashReportConsumer; without Desktop composition it still renders its fallback and copies its own browser report; - About takes copyManualReport from ManualDiagnosticReportConsumer; - AppShell takes the same command through that consumer and passes it to the palette in its command options. The Desktop adapter fixes the manual and renderer_crash surfaces and forwards the same fields as before. DiagnosticReportToastProvider now reads its action's words from the shared shell catalog itself, so AppShell no longer reads shell copy to hand them over. No renderer file outside platform/desktop references the diagnostics bridge any more; the ledger drops six bridge paths, and re-adding any of them is new bridgePaths debt. Refs #4582 Generated-by: Claude Code --- apps/desktop/renderer-architecture.json | 15 +- .../__tests__/about-settings-page.test.ts | 92 ++++++- .../main/__tests__/diagnostics-owner.test.ts | 235 +++++++++++++++++- .../src/renderer/app-shell-command-actions.ts | 8 +- apps/desktop/src/renderer/app-shell.tsx | 22 +- .../contracts/feature-services.tsx | 10 +- apps/desktop/src/renderer/error-boundary.tsx | 28 ++- .../renderer/features/diagnostics/README.md | 31 +-- .../renderer/features/diagnostics/index.ts | 6 + .../renderer/features/diagnostics/ports.ts | 18 +- .../features/diagnostics/services-context.tsx | 7 +- .../renderer/features/diagnostics/testing.ts | 9 +- .../ui/diagnostic-report-toast-provider.tsx | 28 +-- .../diagnostics/ui/report-consumers.tsx | 48 ++++ .../desktop/create-diagnostics-services.ts | 9 + .../renderer/settings/about-settings-page.tsx | 26 +- .../settings/settings-pages.stories.tsx | 19 +- docs/astryx-surface-file-inventory.md | 3 +- docs/astryx-surface-file-inventory.paths | 1 + 19 files changed, 523 insertions(+), 92 deletions(-) create mode 100644 apps/desktop/src/renderer/features/diagnostics/ui/report-consumers.tsx diff --git a/apps/desktop/renderer-architecture.json b/apps/desktop/renderer-architecture.json index 1c79a7b7e3..10e9701bb0 100644 --- a/apps/desktop/renderer-architecture.json +++ b/apps/desktop/renderer-architecture.json @@ -378,7 +378,6 @@ "bridgePaths": { "window.maka.connections.setDefault": 1, "window.maka.connections.test": 1, - "window.maka.diagnostics.copyReport": 1, "window.maka.memory.openFile": 1, "window.maka.sessions.saveConversationToFile": 1, "window.maka.settings.testNetworkProxy": 1 @@ -403,7 +402,7 @@ "react": 1 }, "importSpecifiers": 8, - "nonTriviaTokens": 2188 + "nonTriviaTokens": 2179 }, "src/renderer/app-shell-copy.ts": { "importDeclarations": 1, @@ -729,7 +728,7 @@ "react": 1 }, "importSpecifiers": 74, - "nonTriviaTokens": 9080 + "nonTriviaTokens": 9075 }, "src/renderer/use-app-shell-session-list.ts": { "importDeclarations": 4, @@ -1095,10 +1094,7 @@ } }, "src/renderer/error-boundary.tsx": { - "bridgePaths": { - "window.maka.diagnostics": 3, - "window.maka.diagnostics.copyReport": 1 - }, + "bridgePaths": {}, "environmentCapabilities": { "navigator": 1, "navigator.clipboard.writeText": 1, @@ -1119,6 +1115,7 @@ "unresolvedDependencies": 0, "actionFactories": [], "dependencyPaths": { + "./features/diagnostics/index.js": 1, "./locales/shell-copy.js": 1, "@maka/core/diagnostic-log": 1, "@maka/ui": 1, @@ -1657,8 +1654,7 @@ }, "src/renderer/settings/about-settings-page.tsx": { "bridgePaths": { - "window.maka.app.info": 1, - "window.maka.diagnostics.copyReport": 1 + "window.maka.app.info": 1 }, "environmentCapabilities": {}, "hookCalls": { @@ -1674,6 +1670,7 @@ "actionFactories": [], "dependencyPaths": { "../features/app-update/index.js": 1, + "../features/diagnostics/index.js": 1, "../locales/settings-preferences-copy.js": 1, "../platform/desktop/default-runtime-host-operation.js": 1, "./about-update-status.js": 1, diff --git a/apps/desktop/src/main/__tests__/about-settings-page.test.ts b/apps/desktop/src/main/__tests__/about-settings-page.test.ts index 5bfe81621f..635f0fa462 100644 --- a/apps/desktop/src/main/__tests__/about-settings-page.test.ts +++ b/apps/desktop/src/main/__tests__/about-settings-page.test.ts @@ -18,22 +18,100 @@ */ import assert from 'node:assert/strict'; -import { test } from 'node:test'; -import { createElement } from 'react'; +import { afterEach, test } from 'node:test'; +import { act, createElement } from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { AstryxLocaleProvider, LocaleProvider, ToastProvider } from '@maka/ui'; +import { + DiagnosticsServicesProvider, + createFakeDiagnosticsServices, + type DiagnosticsServices, + type ManualDiagnosticTarget, +} from '../../renderer/features/diagnostics/testing.js'; +import { getSettingsPreferencesCopy } from '../../renderer/locales/settings-preferences-copy.js'; import { AboutSettingsPage } from '../../renderer/settings/about-settings-page.js'; +import { cleanupFakeDom, installReactRenderer } from './fake-dom.js'; -test('keeps manual diagnostics available while About metadata is pending', () => { +type TreeNode = { readonly childNodes?: readonly TreeNode[]; readonly tagName?: string; readonly textContent: string }; + +afterEach(() => { + cleanupFakeDom(); +}); + +function aboutPage(services: DiagnosticsServices) { const page = createElement(AboutSettingsPage, {}); const withToasts = createElement(ToastProvider, { children: page }); - const withAstryxLocale = createElement(AstryxLocaleProvider, { children: withToasts }); - const markup = renderToStaticMarkup( - createElement(LocaleProvider, { locale: 'en', children: withAstryxLocale }), - ); + const withDiagnostics = createElement(DiagnosticsServicesProvider, { services, children: withToasts }); + const withAstryxLocale = createElement(AstryxLocaleProvider, { children: withDiagnostics }); + return createElement(LocaleProvider, { locale: 'en', children: withAstryxLocale }); +} + +async function clickCopyDiagnostics(root: TreeNode, label: string): Promise { + const buttons: TreeNode[] = []; + const visit = (node: TreeNode) => { + if (node.tagName === 'BUTTON') buttons.push(node); + for (const child of node.childNodes ?? []) visit(child); + }; + visit(root); + const key = (button: TreeNode) => Object.keys(button).find((candidate) => candidate.startsWith('__reactProps$')); + const props = (button: TreeNode) => + (button as unknown as Record)[key(button) ?? '']; + const button = buttons.find((candidate) => props(candidate)?.['aria-label'] === label); + assert.ok(button, `missing button ${label}`); + await act(async () => props(button)?.onClick?.({ preventDefault() {}, stopPropagation() {} })); + await act(async () => {}); +} + +function mountWithPendingInfo() { + const mounted = installReactRenderer(); + // About's metadata never arrives here: the copy row must not depend on it. + (globalThis.window as unknown as { maka: unknown }).maka = { + runtimeHostProfiles: { getDefaultHost: () => new Promise(() => {}) }, + }; + return mounted; +} + +test('keeps manual diagnostics available while About metadata is pending', () => { + const markup = renderToStaticMarkup(aboutPage(createFakeDiagnosticsServices())); // The row LABEL also reads "Copy diagnostics", so match the control itself: // its accessible name is the aria-label, not the verb on its face. assert.match(markup, /]*aria-label="Copy diagnostics"/); assert.match(markup, /role="status"[^>]*aria-busy="true"/); }); + +test('copies the manual report through the diagnostics feature, without a target', async () => { + const { root, container } = mountWithPendingInfo(); + const calls: Array[] = []; + const services = createFakeDiagnosticsServices({ + copyManualReport: async (...args) => { + calls.push(args); + }, + }); + const copy = getSettingsPreferencesCopy('en').about; + await act(async () => root.render(aboutPage(services))); + + await clickCopyDiagnostics(container, copy.copyDiagnostics); + + assert.deepEqual(calls, [[]]); + assert.ok(container.textContent.includes(copy.copied)); +}); + +test('reports a failed manual copy with About\'s own words', async () => { + const { root, container } = mountWithPendingInfo(); + let attempts = 0; + const services = createFakeDiagnosticsServices({ + copyManualReport: async () => { + attempts += 1; + throw new Error('denied'); + }, + }); + const copy = getSettingsPreferencesCopy('en').about; + await act(async () => root.render(aboutPage(services))); + + await clickCopyDiagnostics(container, copy.copyDiagnostics); + + assert.equal(attempts, 1); + assert.ok(container.textContent.includes(copy.copyFailed)); + assert.ok(container.textContent.includes(copy.clipboardUnavailable)); +}); diff --git a/apps/desktop/src/main/__tests__/diagnostics-owner.test.ts b/apps/desktop/src/main/__tests__/diagnostics-owner.test.ts index e8e23fdeda..850550abb9 100644 --- a/apps/desktop/src/main/__tests__/diagnostics-owner.test.ts +++ b/apps/desktop/src/main/__tests__/diagnostics-owner.test.ts @@ -23,6 +23,7 @@ import { join, relative, resolve } from 'node:path'; import { afterEach, describe, test } from 'node:test'; import { fileURLToPath } from 'node:url'; import { act, createElement, useEffect, type ReactNode } from 'react'; +import type { SessionCatalogController } from '../../renderer/application/contracts/session-catalog/session-catalog-state.js'; import type { UiLocale } from '@maka/core/ui-locale'; import { AstryxLocaleProvider, @@ -32,6 +33,12 @@ import { type ToastDiagnosticTarget, } from '@maka/ui'; import type { DesktopDiagnosticInput } from '../../preload/diagnostics-contract.js'; +import { + buildAppShellCommandList, + type AppShellCommandListOptions, +} from '../../renderer/app-shell-command-actions.js'; +import { ErrorBoundary } from '../../renderer/error-boundary.js'; +import { getShellCopy } from '../../renderer/locales/shell-copy.js'; import { createDesktopDiagnosticsServices, type DesktopDiagnosticsBridge, @@ -43,6 +50,8 @@ import { createFakeDiagnosticsServices, getDiagnosticsCopy, type DiagnosticsServices, + type ManualDiagnosticTarget, + type RendererCrashDiagnosticReport, type ToastDiagnosticReport, } from '../../renderer/features/diagnostics/testing.js'; import { cleanupFakeDom, installReactRenderer } from './fake-dom.js'; @@ -202,10 +211,11 @@ describe('PreviousMainProcessInterruptionNotice', () => { }); describe('DiagnosticReportToastProvider', () => { + const shellCopy = getShellCopy('en'); const labels = { - label: 'Copy report', - failureTitle: 'Copy failed', - failureDescription: 'Clipboard unavailable', + label: shellCopy.errorBoundary.copyReport, + failureTitle: shellCopy.commandActions.copyFailedTitle, + failureDescription: shellCopy.commandActions.clipboardDenied, }; const target: ToastDiagnosticTarget = { sessionId: 'session-1', turnId: 'turn-1', eventId: 'event-1' }; @@ -229,7 +239,7 @@ describe('DiagnosticReportToastProvider', () => { await act(async () => root.render(localized( 'en', services, - createElement(DiagnosticReportToastProvider, { labels, children: createElement(ErrorProbe) }), + createElement(DiagnosticReportToastProvider, { children: createElement(ErrorProbe) }), ))); await clickButton(container, labels.label); @@ -240,7 +250,21 @@ describe('DiagnosticReportToastProvider', () => { assert.deepEqual(reports[0]?.diagnosticTarget, target); }); - test('reports a failed copy with the supplied labels', async () => { + test('names the report action in the current locale', async () => { + const { root, container } = installReactRenderer(); + const services = createFakeDiagnosticsServices(); + await act(async () => root.render(localized( + 'zh-CN', + services, + createElement(DiagnosticReportToastProvider, { children: createElement(ErrorProbe) }), + ))); + + const label = getShellCopy('zh-CN').errorBoundary.copyReport; + assert.ok(elements(container, 'BUTTON').some((button) => button.textContent === label)); + assert.equal(occurrences(container, labels.label), 0); + }); + + test('reports a failed copy with the shell catalog\'s words', async () => { const { root, container } = installReactRenderer(); const services = createFakeDiagnosticsServices({ copyToastReport: async () => { @@ -250,7 +274,7 @@ describe('DiagnosticReportToastProvider', () => { await act(async () => root.render(localized( 'en', services, - createElement(DiagnosticReportToastProvider, { labels, children: createElement(ErrorProbe) }), + createElement(DiagnosticReportToastProvider, { children: createElement(ErrorProbe) }), ))); await clickButton(container, labels.label); @@ -305,6 +329,32 @@ describe('Desktop diagnostics adapter', () => { ]); }); + test('sends a manual report with the target only when the caller had one', async () => { + const { bridge, inputs } = recordingBridge(); + const services = createDesktopDiagnosticsServices(bridge); + + await services.copyManualReport(); + await services.copyManualReport({ sessionId: 'session-1' }); + await services.copyManualReport({ profileId: 'profile-1' }); + + assert.deepEqual(inputs, [ + { surface: 'manual' }, + { surface: 'manual', target: { sessionId: 'session-1' } }, + { surface: 'manual', target: { profileId: 'profile-1' } }, + ]); + }); + + test('sends a renderer crash report with its title and details', async () => { + const { bridge, inputs } = recordingBridge(); + const services = createDesktopDiagnosticsServices(bridge); + + await services.copyRendererCrashReport({ title: 'TypeError: boom', details: 'TypeError: boom\n\nStack:\nframe' }); + + assert.deepEqual(inputs, [ + { surface: 'renderer_crash', title: 'TypeError: boom', details: 'TypeError: boom\n\nStack:\nframe' }, + ]); + }); + test('delegates the previous-run read and report', async () => { const { bridge, calls } = recordingBridge(); const services = createDesktopDiagnosticsServices(bridge); @@ -314,6 +364,172 @@ describe('Desktop diagnostics adapter', () => { }); }); +describe('ErrorBoundary crash report', () => { + function Crash(): ReactNode { + const error = new TypeError('boom'); + error.stack = 'TypeError: boom\n at Crash'; + throw error; + } + + const boundary = createElement(ErrorBoundary, { locale: 'en', children: createElement(Crash) }); + const copy = getShellCopy('en').errorBoundary; + + function hasButton(root: TreeNode, label: string): boolean { + return elements(root, 'BUTTON').some((button) => button.textContent === label); + } + + test('copies the crash through the diagnostics feature', async (t) => { + t.mock.method(console, 'error', () => {}); + const { root, container } = installReactRenderer(); + const reports: RendererCrashDiagnosticReport[] = []; + const services = createFakeDiagnosticsServices({ + copyRendererCrashReport: async (report) => { + reports.push(report); + }, + }); + await act(async () => root.render(localized('en', services, boundary))); + assert.equal(occurrences(container, copy.title), 1); + + await clickButton(container, copy.copyReport); + await act(async () => {}); + + assert.equal(reports.length, 1); + assert.equal(reports[0]?.title, 'TypeError: boom'); + assert.match(reports[0]?.details ?? '', /^TypeError: boom\n\nStack:\nTypeError: boom\n {4}at Crash/); + assert.ok(hasButton(container, copy.copied)); + }); + + test('shows the failure when the diagnostics feature cannot copy', async (t) => { + t.mock.method(console, 'error', () => {}); + const { root, container } = installReactRenderer(); + let attempts = 0; + const services = createFakeDiagnosticsServices({ + copyRendererCrashReport: async () => { + attempts += 1; + throw new Error('denied'); + }, + }); + await act(async () => root.render(localized('en', services, boundary))); + + await clickButton(container, copy.copyReport); + await act(async () => {}); + + assert.equal(attempts, 1); + assert.ok(hasButton(container, copy.copyFailed)); + assert.equal(occurrences(container, copy.clipboardFailure), 1); + }); + + test('without Desktop composition, still renders the fallback and copies the browser report', async (t) => { + t.mock.method(console, 'error', () => {}); + const writes: string[] = []; + Object.defineProperty(navigator, 'clipboard', { + configurable: true, + value: { + writeText: async (text: string) => { + writes.push(text); + }, + }, + }); + restoreAfterEach.push(() => { + Reflect.deleteProperty(navigator, 'clipboard'); + }); + const { root, container } = installReactRenderer(); + await act(async () => root.render(createElement(LocaleProvider, { + locale: 'en', + children: createElement(AstryxLocaleProvider, { children: boundary }), + }))); + assert.equal(occurrences(container, copy.title), 1); + + await clickButton(container, copy.copyReport); + await act(async () => {}); + + assert.equal(writes.length, 1); + assert.match(writes[0] ?? '', /^Maka renderer error report\n/); + assert.match(writes[0] ?? '', /TypeError: boom/); + assert.ok(hasButton(container, copy.copied)); + }); +}); + +describe('Command palette manual report', () => { + const copy = getShellCopy('en').commandActions; + + function paletteCommands( + copyManualDiagnosticReport: AppShellCommandListOptions['copyManualDiagnosticReport'], + toasts: string[], + ) { + const options: AppShellCommandListOptions = { + uiLocale: 'en', + activeId: 'session-1', + activePermissionMode: undefined, + canSetPermissionMode: false, + clientPathsAccessible: false, + connections: [], + defaultConnection: null, + readMessages: () => [], + newTaskProfileId: 'new-task-profile', + settingsOpen: false, + settingsProfileId: undefined, + sessionCatalog: {} as SessionCatalogController, + themePref: 'auto', + hiddenSessionIds: new Set(), + captureComposerImportOwner: () => ({ sessionId: 'session-1', navSection: 'sessions' }), + copyManualDiagnosticReport, + createSession() {}, + openSideConversation() {}, + openHelp() {}, + openScheduledTaskCreate() {}, + openProjectFolder: async () => {}, + openSessionInChat() {}, + openSettings() {}, + openSettingsSection() {}, + openWorkspaceFolder: async () => {}, + refreshConnections: async () => {}, + copyTodayDailyReview: async () => {}, + pasteTodayDailyReview: async () => {}, + saveTodayDailyReview: async () => {}, + setNavSelection() {}, + setPermissionMode: async () => true, + setThemePref() {}, + toastApi: { + success: (title) => toasts.push(`success:${title}`), + info() {}, + error: (title, _description, _details, target) => toasts.push(`error:${title}:${JSON.stringify(target)}`), + }, + }; + return buildAppShellCommandList({ current: options }); + } + + async function runCopyDiagnostics(commands: ReturnType): Promise { + const command = commands.find(({ id }) => id === 'diag:copy-diagnostics'); + assert.ok(command, 'missing diag:copy-diagnostics'); + await command.run(); + } + + test('copies through the injected command with the current target, then confirms', async () => { + const targets: Array = []; + const toasts: string[] = []; + await runCopyDiagnostics(paletteCommands(async (target) => { + targets.push(target); + }, toasts)); + + assert.deepEqual(targets, [{ sessionId: 'session-1' }]); + assert.deepEqual(toasts, [`success:${copy.diagnosticsCopiedTitle}`]); + }); + + test('reports a failed copy against the same target', async (t) => { + t.mock.method(console, 'error', () => {}); + const toasts: string[] = []; + let attempts = 0; + await runCopyDiagnostics(paletteCommands(async () => { + attempts += 1; + throw new Error('denied'); + }, toasts)); + + assert.equal(attempts, 1); + assert.deepEqual(toasts, [`error:${copy.copyFailedTitle}:${JSON.stringify({ sessionId: 'session-1' })}`]); + }); +}); + describe('Diagnostics ownership', () => { const rendererRoot = resolve(fileURLToPath(new URL('../../../src/renderer/', import.meta.url))); @@ -335,7 +551,14 @@ describe('Diagnostics ownership', () => { test('mounts each owner once and reaches Desktop diagnostics through one adapter', () => { assert.deepEqual(sourcesMatching(/<(?:Diagnostics\.)?PreviousMainProcessInterruptionNotice\b/), ['app-shell.tsx']); assert.deepEqual(sourcesMatching(/<(?:Diagnostics\.)?DiagnosticReportToastProvider\b/), ['app-shell.tsx']); + assert.deepEqual( + sourcesMatching(/<(?:Diagnostics\.)?ManualDiagnosticReportConsumer\b/), + ['app-shell.tsx', 'settings/about-settings-page.tsx'], + ); + assert.deepEqual(sourcesMatching(/<(?:Diagnostics\.)?RendererCrashReportConsumer\b/), ['error-boundary.tsx']); assert.deepEqual(sourcesMatching(/create-diagnostics-services/), ['composition/desktop-feature-services.tsx']); + assert.deepEqual(sourcesMatching(/\.\s*copyReport\(/), ['platform/desktop/create-diagnostics-services.ts']); + assert.deepEqual(sourcesMatching(/\bmaka\s*\??\.\s*diagnostics\b/), []); assert.deepEqual( sourcesMatching(/\.\s*(?:takePreviousMainProcessInterruption|copyPreviousMainProcessInterruption)\(/), ['features/diagnostics/ui/previous-main-process-interruption-notice.tsx', 'platform/desktop/create-diagnostics-services.ts'], diff --git a/apps/desktop/src/renderer/app-shell-command-actions.ts b/apps/desktop/src/renderer/app-shell-command-actions.ts index 0acf9bccb0..1dc9aee184 100644 --- a/apps/desktop/src/renderer/app-shell-command-actions.ts +++ b/apps/desktop/src/renderer/app-shell-command-actions.ts @@ -30,6 +30,7 @@ import { runOnDefaultRuntimeHost, } from './platform/desktop/default-runtime-host-operation.js'; import { buildCommandList } from "./command-palette-commands.js"; +import type { CopyManualDiagnosticReport } from './features/diagnostics/index.js'; import type { Command } from './features/overlays/index.js'; import type { SessionCatalogController } from './application/contracts/session-catalog/session-catalog-state.js'; import { renderConversationMarkdown } from "./conversation-markdown.js"; @@ -77,6 +78,7 @@ export interface AppShellCommandListOptions { /** Sessions the rail hides (mounted side-chat forks) — the palette skips them too. */ hiddenSessionIds: ReadonlySet; captureComposerImportOwner: () => ComposerImportOwner; + copyManualDiagnosticReport: CopyManualDiagnosticReport; createSession: () => void; openSideConversation: () => void; openHelp: () => void; @@ -319,6 +321,7 @@ export function buildAppShellCommandList( onCopyDiagnostics: async () => { const { captureComposerImportOwner, + copyManualDiagnosticReport, newTaskProfileId, settingsOpen, settingsProfileId, @@ -332,10 +335,7 @@ export function buildAppShellCommandList( settingsProfileId, ); try { - await window.maka.diagnostics.copyReport({ - surface: "manual", - ...(target ? { target } : {}), - }); + await copyManualDiagnosticReport(target); toastApi.success(copy.diagnosticsCopiedTitle, copy.diagnosticsCopiedDescription); } catch (err) { toastApi.error( diff --git a/apps/desktop/src/renderer/app-shell.tsx b/apps/desktop/src/renderer/app-shell.tsx index 3612887cbd..29715198e0 100644 --- a/apps/desktop/src/renderer/app-shell.tsx +++ b/apps/desktop/src/renderer/app-shell.tsx @@ -175,7 +175,6 @@ export function AppShell() { const [uiLocaleOverride, setUiLocaleOverride] = useState(null); const systemUiLocale = useSystemUiLocale(); const uiLocale = resolveUiLocale(uiLocalePreference, systemUiLocale, uiLocaleOverride); - const copy = getShellCopy(uiLocale); return ( @@ -184,13 +183,7 @@ export function AppShell() { `useUiLocale()` throws before anything renders. Still above every Astryx subtree. */} - + @@ -204,9 +197,13 @@ export function AppShell() { {(workbar) => ( - + + {(copyManualDiagnosticReport) => ( + + )} + )} @@ -240,6 +237,7 @@ function AppShellContent({ overlays, sharedSessionDialog, workbar: { bridge, commands, selectors, LiveContextUsageProbe }, + copyManualDiagnosticReport, uiLocale, uiLocaleOverride, setUiLocaleOverride, @@ -249,6 +247,7 @@ function AppShellContent({ overlays: OverlaysShellProjection; sharedSessionDialog: SessionCollaborationDialogProjection; workbar: WorkbarShellProjection; + copyManualDiagnosticReport: Diagnostics.CopyManualDiagnosticReport; uiLocale: UiLocale; uiLocaleOverride: UiLocale | null; setUiLocaleOverride: Dispatch>; @@ -1414,6 +1413,7 @@ function AppShellContent({ themePref, hiddenSessionIds: selectors.hiddenSessionIds, captureComposerImportOwner, + copyManualDiagnosticReport, createSession, openHelp, openScheduledTaskCreate: () => { diff --git a/apps/desktop/src/renderer/application/contracts/feature-services.tsx b/apps/desktop/src/renderer/application/contracts/feature-services.tsx index 387661f1f0..421276f427 100644 --- a/apps/desktop/src/renderer/application/contracts/feature-services.tsx +++ b/apps/desktop/src/renderer/application/contracts/feature-services.tsx @@ -27,6 +27,11 @@ export interface ServicesContext { }) => ReactElement; /** Reads the mounted services; throws when the Provider is missing. */ readonly useServices: () => S; + /** + * Reads the mounted services, or undefined when the Provider is missing. For + * a reader that has to keep working outside Desktop composition. + */ + readonly useOptionalServices: () => S | undefined; } /** @@ -52,5 +57,8 @@ export function createServicesContext(providerName: string): ServicesContext< if (!services) throw new Error(`${providerName} is missing`); return services; } - return { Provider, useServices }; + function useOptionalServices(): S | undefined { + return useContext(Context) ?? undefined; + } + return { Provider, useServices, useOptionalServices }; } diff --git a/apps/desktop/src/renderer/error-boundary.tsx b/apps/desktop/src/renderer/error-boundary.tsx index 28852891f1..8f9bc5cdb3 100644 --- a/apps/desktop/src/renderer/error-boundary.tsx +++ b/apps/desktop/src/renderer/error-boundary.tsx @@ -22,6 +22,7 @@ import { truncateUtf8 } from '@maka/core/diagnostic-log'; import type { UiLocale } from '@maka/core/ui-locale'; import { ICON_SIZE, AlertTriangle, Check, Clipboard, RotateCw } from '@maka/ui/icons'; import { Button as UiButton, Card, redactSecrets } from '@maka/ui'; +import * as Diagnostics from './features/diagnostics/index.js'; import { getShellCopy } from './locales/shell-copy.js'; export type ErrorBoundaryCopyState = 'idle' | 'pending' | 'copied' | 'failed'; @@ -64,7 +65,25 @@ export function formatRendererErrorReport(error: Error, info?: ErrorInfo | null) ); } -export class ErrorBoundary extends Component<{ children: ReactNode; locale: UiLocale }, State> { +type ErrorBoundaryProps = { children: ReactNode; locale: UiLocale }; + +/** + * The renderer's crash surface. Desktop composition supplies the crash report + * through the diagnostics feature; without it (Storybook, renderer tests) the + * boundary still renders its fallback and copies its own browser report. + */ +export function ErrorBoundary(props: ErrorBoundaryProps): ReactNode { + return ( + + {(copyCrashReport) => } + + ); +} + +class RendererErrorBoundary extends Component< + ErrorBoundaryProps & { copyCrashReport: Diagnostics.CopyRendererCrashReport | undefined }, + State +> { state: State = { error: null, errorInfo: null, copyState: 'idle' }; private mounted = false; private copyRequestSeq = 0; @@ -110,10 +129,9 @@ export class ErrorBoundary extends Component<{ children: ReactNode; locale: UiLo const copyRequestId = ++this.copyRequestSeq; this.setState({ copyState: 'pending' }); try { - const diagnostics = window.maka?.diagnostics; - if (diagnostics) { - await diagnostics.copyReport({ - surface: 'renderer_crash', + const { copyCrashReport } = this.props; + if (copyCrashReport) { + await copyCrashReport({ title: `${error.name}: ${error.message}`, details: formatRendererErrorDetails(error, errorInfo), }); diff --git a/apps/desktop/src/renderer/features/diagnostics/README.md b/apps/desktop/src/renderer/features/diagnostics/README.md index 7db52d4b7b..5d56262efd 100644 --- a/apps/desktop/src/renderer/features/diagnostics/README.md +++ b/apps/desktop/src/renderer/features/diagnostics/README.md @@ -19,31 +19,32 @@ # Diagnostics feature -This slice owns the renderer root's use of Desktop diagnostics. +This slice owns the renderer's use of Desktop diagnostics. ## Ownership - `DiagnosticReportToastProvider` is the renderer's toast layer. It offers the Desktop diagnostic report on error toasts and copies it through the injected - `copyToastReport` service. AppShell supplies only the action's labels, which - stay in the shared shell catalog beside the Error Boundary and command - palette copy that use the same words. + `copyToastReport` service. It reads the action's words from the shared shell + catalog, where they stay beside the Error Boundary and command palette copy + that use the same ones. - `PreviousMainProcessInterruptionNotice` alone reads whether the previous main process ended without finishing its shutdown, once the shell's appearance has hydrated, and shows that notice at most once per renderer. A read that resolves after its effect was replaced (for example by a locale change) is dropped; the replacement read shows the notice in the current locale. +- `ManualDiagnosticReportConsumer` hands the manual report command to its two + callers: About, and AppShell, which passes it to the command palette in its + command options. Each caller keeps its own target, toasts and pending state; + the command takes only the optional task or Host profile target. +- `RendererCrashReportConsumer` hands the crash report command to the Error + Boundary, or nothing outside Desktop composition (Storybook, renderer tests). + The boundary then copies its own bounded browser report, so the crash surface + never depends on a provider being mounted. - `platform/desktop/create-diagnostics-services.ts` is the only adapter from the Desktop bridge into this feature, and Desktop feature-services composition is - its only production importer. The adapter forwards only the fields an error - toast carried, as the `toast` report surface. + its only production importer. Each service fixes its report surface (`toast`, + `manual`, `renderer_crash`) and forwards only the fields its caller supplied. -AppShell calls no diagnostics bridge method and holds no notice state. - -## Not in this slice - -The Error Boundary's crash report, the command palette's manual report -(`app-shell-command-actions.ts`) and About's manual report still call the -Desktop bridge from legacy renderer files. Each can move onto this feature's -services when its owner is next changed; none of them runs in AppShell's render -body. +No renderer file outside that adapter calls the diagnostics bridge. AppShell +holds no notice state and receives only the manual report command. diff --git a/apps/desktop/src/renderer/features/diagnostics/index.ts b/apps/desktop/src/renderer/features/diagnostics/index.ts index 7dd6412c49..c070c690c7 100644 --- a/apps/desktop/src/renderer/features/diagnostics/index.ts +++ b/apps/desktop/src/renderer/features/diagnostics/index.ts @@ -20,4 +20,10 @@ export { DiagnosticsServicesProvider } from './services-context.js'; export { DiagnosticReportToastProvider } from './ui/diagnostic-report-toast-provider.js'; export { PreviousMainProcessInterruptionNotice } from './ui/previous-main-process-interruption-notice.js'; +export { + ManualDiagnosticReportConsumer, + RendererCrashReportConsumer, + type CopyManualDiagnosticReport, + type CopyRendererCrashReport, +} from './ui/report-consumers.js'; export type { DiagnosticsServices } from './ports.js'; diff --git a/apps/desktop/src/renderer/features/diagnostics/ports.ts b/apps/desktop/src/renderer/features/diagnostics/ports.ts index 85f696bdf9..ea0ac2b634 100644 --- a/apps/desktop/src/renderer/features/diagnostics/ports.ts +++ b/apps/desktop/src/renderer/features/diagnostics/ports.ts @@ -22,10 +22,26 @@ import type { ToastErrorAction } from '@maka/ui'; /** What an error toast hands to its report action. */ export type ToastDiagnosticReport = Parameters[0]; -/** The Desktop diagnostics capabilities the renderer root used to reach directly. */ +/** The task or Host profile a manual report is about, when the user is looking at one. */ +export type ManualDiagnosticTarget = + | { readonly sessionId: string; readonly profileId?: never } + | { readonly profileId: string; readonly sessionId?: never }; + +/** What the Error Boundary hands to the report of a renderer crash. */ +export interface RendererCrashDiagnosticReport { + readonly title: string; + /** The error and its stacks, already redacted. */ + readonly details: string; +} + +/** The Desktop diagnostics capabilities the renderer used to reach directly. */ export interface DiagnosticsServices { /** Copies a diagnostic report for an error toast the user chose to report. */ copyToastReport(report: ToastDiagnosticReport): Promise; + /** Copies the report the user asked for from About or the command palette. */ + copyManualReport(target?: ManualDiagnosticTarget): Promise; + /** Copies the report for a renderer crash the Error Boundary caught. */ + copyRendererCrashReport(report: RendererCrashDiagnosticReport): Promise; /** * Reads whether the previous main process ended without finishing its * shutdown. Desktop reads it once per renderer; later calls return the same diff --git a/apps/desktop/src/renderer/features/diagnostics/services-context.tsx b/apps/desktop/src/renderer/features/diagnostics/services-context.tsx index 5613584a84..4cd45dc525 100644 --- a/apps/desktop/src/renderer/features/diagnostics/services-context.tsx +++ b/apps/desktop/src/renderer/features/diagnostics/services-context.tsx @@ -20,10 +20,15 @@ import { createServicesContext } from '../../application/contracts/feature-services.js'; import type { DiagnosticsServices } from './ports.js'; -const { Provider, useServices } = createServicesContext('DiagnosticsServicesProvider'); +const { Provider, useServices, useOptionalServices } = + createServicesContext('DiagnosticsServicesProvider'); export const DiagnosticsServicesProvider = Provider; export function useDiagnosticsServices(): DiagnosticsServices { return useServices(); } + +export function useOptionalDiagnosticsServices(): DiagnosticsServices | undefined { + return useOptionalServices(); +} diff --git a/apps/desktop/src/renderer/features/diagnostics/testing.ts b/apps/desktop/src/renderer/features/diagnostics/testing.ts index 2b9b3a0888..21ef2a37a5 100644 --- a/apps/desktop/src/renderer/features/diagnostics/testing.ts +++ b/apps/desktop/src/renderer/features/diagnostics/testing.ts @@ -23,13 +23,20 @@ export { DiagnosticsServicesProvider } from './services-context.js'; export { DiagnosticReportToastProvider } from './ui/diagnostic-report-toast-provider.js'; export { PreviousMainProcessInterruptionNotice } from './ui/previous-main-process-interruption-notice.js'; export { getDiagnosticsCopy } from './locales/diagnostics-copy.js'; -export type { DiagnosticsServices, ToastDiagnosticReport } from './ports.js'; +export type { + DiagnosticsServices, + ManualDiagnosticTarget, + RendererCrashDiagnosticReport, + ToastDiagnosticReport, +} from './ports.js'; export function createFakeDiagnosticsServices( overrides: Partial = {}, ): DiagnosticsServices { return { copyToastReport: async () => undefined, + copyManualReport: async () => undefined, + copyRendererCrashReport: async () => undefined, takePreviousMainProcessInterruption: async () => false, copyPreviousMainProcessInterruption: async () => undefined, ...overrides, diff --git a/apps/desktop/src/renderer/features/diagnostics/ui/diagnostic-report-toast-provider.tsx b/apps/desktop/src/renderer/features/diagnostics/ui/diagnostic-report-toast-provider.tsx index 338dc16697..baa89995b7 100644 --- a/apps/desktop/src/renderer/features/diagnostics/ui/diagnostic-report-toast-provider.tsx +++ b/apps/desktop/src/renderer/features/diagnostics/ui/diagnostic-report-toast-provider.tsx @@ -18,30 +18,26 @@ */ import { useMemo, type ReactNode } from 'react'; -import { ToastProvider, type ToastErrorAction } from '@maka/ui'; +import { ToastProvider, useUiLocale, type ToastErrorAction } from '@maka/ui'; +import { getShellCopy } from '../../../locales/shell-copy.js'; import { useDiagnosticsServices } from '../services-context.js'; -/** The report action's copy. AppShell supplies it from the shared shell catalog. */ -interface DiagnosticReportToastLabels { - readonly label: string; - readonly failureTitle: string; - readonly failureDescription: string; -} - /** * The renderer's toast layer, with the Desktop diagnostic report offered on * error toasts. * - * The report action is rebuilt only when a label changes; the injected - * services are created once at composition, so toast consumers do not see a - * new action on unrelated renders. + * The action's words stay in the shared shell catalog beside the Error + * Boundary and command palette copy that use the same ones. The report action + * is rebuilt only when the locale changes; the injected services are created + * once at composition, so toast consumers do not see a new action on + * unrelated renders. */ -export function DiagnosticReportToastProvider(props: { - readonly labels: DiagnosticReportToastLabels; - readonly children?: ReactNode; -}) { +export function DiagnosticReportToastProvider(props: { readonly children?: ReactNode }) { const services = useDiagnosticsServices(); - const { label, failureTitle, failureDescription } = props.labels; + const copy = getShellCopy(useUiLocale()); + const label = copy.errorBoundary.copyReport; + const failureTitle = copy.commandActions.copyFailedTitle; + const failureDescription = copy.commandActions.clipboardDenied; const errorAction = useMemo( () => ({ label, diff --git a/apps/desktop/src/renderer/features/diagnostics/ui/report-consumers.tsx b/apps/desktop/src/renderer/features/diagnostics/ui/report-consumers.tsx new file mode 100644 index 0000000000..5a14c545e0 --- /dev/null +++ b/apps/desktop/src/renderer/features/diagnostics/ui/report-consumers.tsx @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import type { ReactNode } from 'react'; +import type { DiagnosticsServices } from '../ports.js'; +import { useDiagnosticsServices, useOptionalDiagnosticsServices } from '../services-context.js'; + +export type CopyManualDiagnosticReport = DiagnosticsServices['copyManualReport']; +export type CopyRendererCrashReport = DiagnosticsServices['copyRendererCrashReport']; + +/** + * Hands the manual report command to About, and to AppShell for the command + * palette's options. Both are legacy readers that take feature capabilities + * through render props rather than hooks of their own. + */ +export function ManualDiagnosticReportConsumer(props: { + readonly children: (copyManualReport: CopyManualDiagnosticReport) => ReactNode; +}): ReactNode { + return props.children(useDiagnosticsServices().copyManualReport); +} + +/** + * Hands the crash report command to the Error Boundary. Outside Desktop + * composition (Storybook, renderer tests) there is none, and the boundary + * copies its own browser report instead: the crash surface must not depend on + * a provider being mounted. + */ +export function RendererCrashReportConsumer(props: { + readonly children: (copyCrashReport: CopyRendererCrashReport | undefined) => ReactNode; +}): ReactNode { + return props.children(useOptionalDiagnosticsServices()?.copyRendererCrashReport); +} diff --git a/apps/desktop/src/renderer/platform/desktop/create-diagnostics-services.ts b/apps/desktop/src/renderer/platform/desktop/create-diagnostics-services.ts index cff6408da7..62e0492a7f 100644 --- a/apps/desktop/src/renderer/platform/desktop/create-diagnostics-services.ts +++ b/apps/desktop/src/renderer/platform/desktop/create-diagnostics-services.ts @@ -34,6 +34,15 @@ export function createDesktopDiagnosticsServices( ...(report.diagnosticDetails ? { details: report.diagnosticDetails } : {}), ...(report.diagnosticTarget ? { target: report.diagnosticTarget } : {}), }), + copyManualReport: (target) => bridge.diagnostics.copyReport({ + surface: 'manual', + ...(target ? { target } : {}), + }), + copyRendererCrashReport: (report) => bridge.diagnostics.copyReport({ + surface: 'renderer_crash', + title: report.title, + details: report.details, + }), takePreviousMainProcessInterruption: () => bridge.diagnostics.takePreviousMainProcessInterruption(), copyPreviousMainProcessInterruption: () => bridge.diagnostics.copyPreviousMainProcessInterruption(), }; diff --git a/apps/desktop/src/renderer/settings/about-settings-page.tsx b/apps/desktop/src/renderer/settings/about-settings-page.tsx index c3fdea6875..4c2e3832e3 100644 --- a/apps/desktop/src/renderer/settings/about-settings-page.tsx +++ b/apps/desktop/src/renderer/settings/about-settings-page.tsx @@ -32,6 +32,10 @@ import { AppUpdateAboutProjectionConsumer, type AppUpdateAboutProjection, } from '../features/app-update/index.js'; +import { + ManualDiagnosticReportConsumer, + type CopyManualDiagnosticReport, +} from '../features/diagnostics/index.js'; import { SettingsPage, SettingsRow, SettingsSection } from './settings-section.js'; import { settingsActionErrorMessage } from './settings-error-copy.js'; import { SettingsSkeletonStack } from './settings-skeleton.js'; @@ -162,11 +166,11 @@ export function AboutSettingsPage(props: { onOpenKeyboardHelp?(): void }) { }; }, [copy.loadFailed, locale, toast]); - async function copyDiagnostics() { + async function copyDiagnostics(copyManualReport: CopyManualDiagnosticReport) { if (!diagnosticCopyGuard.begin('copy')) return; setCopyingDiagnostics(true); try { - await window.maka.diagnostics.copyReport({ surface: 'manual' }); + await copyManualReport(); if (aboutPageMountedRef.current) toast.success(copy.copied, copy.pasteHint); } catch { if (aboutPageMountedRef.current) { @@ -255,13 +259,17 @@ export function AboutSettingsPage(props: { onOpenKeyboardHelp?(): void }) { label={copy.copyDiagnostics} description={copy.copyHelp} end={( -