From e722ee68692f6ce721604f69e30322c1bfbdec06 Mon Sep 17 00:00:00 2001 From: winewill Date: Tue, 20 Oct 2026 19:21:31 +0800 Subject: [PATCH] fix(desktop): cancel diagnostic workers during shutdown --- apps/desktop/README.i18n.yaml | 4 +- apps/desktop/README.md | 2 +- apps/desktop/README.zh.md | 2 +- apps/desktop/src/main.ts | 58 +++++++++++++--- apps/desktop/src/operation-history.ts | 7 +- apps/desktop/src/operation-report-job.ts | 55 ++++++++++++---- apps/desktop/src/operation-report-worker.ts | 12 +++- apps/desktop/src/operation-report.ts | 8 ++- apps/desktop/tests/main-startup.spec.ts | 22 ++++++- .../tests/operation-report-job.spec.ts | 66 +++++++++++++++---- .../tests/operation-report-worker.e2e.ts | 45 ++++++++++++- 11 files changed, 234 insertions(+), 47 deletions(-) diff --git a/apps/desktop/README.i18n.yaml b/apps/desktop/README.i18n.yaml index 62dd871839..3586a7f616 100644 --- a/apps/desktop/README.i18n.yaml +++ b/apps/desktop/README.i18n.yaml @@ -36,8 +36,8 @@ en: 22153a298a6662e1 zh: 3600803c8c9d0274 /deepseek-harness-desktop/installation-ownership/runtime-and-plugin-activation: - en: 0c4fbb6e01824e02 - zh: 96a69e571601f82b + en: b80deb2229d886d9 + zh: fa0b82b1064f127d /deepseek-harness-desktop/installation-ownership/host-environment: en: 98cf848762482d48 zh: 74f9100a39ec8c2f diff --git a/apps/desktop/README.md b/apps/desktop/README.md index 3b4f3bf601..96477982b1 100644 --- a/apps/desktop/README.md +++ b/apps/desktop/README.md @@ -180,7 +180,7 @@ Before the first fatal dialog opens, Electron writes a crash report to the platf Operation records and the application single-instance lock are isolated by `userData`; the shared platform logs directory does not store operation records. History from older test builds under `logs/plugin-operations` is not imported automatically because it may contain other profiles. Repairing an unfinished append preserves complete lines in the current segment and the previous segment. Host `transportDropped` is a lower bound as of the last successful delivery; Desktop rejects malformed or mismatched-run records without adding them to that counter. -After saving the basic report, the shell attempts to append recent plugin-management records in an isolated Worker packaged as `lib/operation-report-worker.js` inside `app.asar`. The completion marker and omission counters distinguish a finished collection from missing or partial material. The Host announces its run ID before profile loading; reported Host errors carry that ID as `hostRunId`, matching operation records from the same lifetime. Failure before this announcement leaves the run identity unknown. Recent operations are context, not proof of cause. Collection reads only two owned files under `userData/plugin-operations`, at most 64 KiB and 100 records; the complete appended section is limited to 64 KiB. A 500 ms deadline requests Worker termination; completion waits for its exit. The basic report remains available independently. Only one collector runs at a time; concurrent reports keep their basic content without an auxiliary section. A rejected termination request does not release that slot until exit is observed. Get diagnostic report can reveal the basic file before the append finishes. The completion marker confirms only that collection ended, not that all operations were captured; queued bytes and omission counters describe missing material. +After saving the basic report, the shell attempts to append recent plugin-management records in an isolated Worker packaged as `lib/operation-report-worker.js` inside `app.asar`. The completion marker and omission counters distinguish a finished collection from missing or partial material. The Host announces its run ID before profile loading; reported Host errors carry that ID as `hostRunId`, matching operation records from the same lifetime. Failure before this announcement leaves the run identity unknown. Recent operations are context, not proof of cause. Collection reads only two owned files under `userData/plugin-operations`, at most 64 KiB and 100 records; the complete appended section is limited to 64 KiB. The 500 ms collection budget reserves its final 100 ms for cooperative cancellation before requesting forced Worker termination; completion waits for its exit. Application exit, recovery exit or restart, and confirmed update installation also cancel collection and wait at most 500 ms for Worker exit. Shutdown prevents late basic-report saves from starting another collector. An update installation failure permits future collection again. Worker messages interrupt collection between bounded records; an in-flight file open, stat, read or close must settle, and synchronous parsing cannot be preempted within one record. The final append uses an abort signal but can leave partial bytes. Forced termination remains the fallback when cooperative cleanup does not finish. The basic report remains available independently. Only one collector runs at a time; concurrent reports keep their basic content without an auxiliary section. A rejected termination request does not release that slot until exit is observed. Get diagnostic report can reveal the basic file before the append finishes. The completion marker confirms only that collection ended, not that all operations were captured; queued bytes and omission counters describe missing material. The journal retains two 256 KiB segments, accepts records up to 4 KiB, and bounds queued writes to 64 KiB. Exit, recovery restart and installer preparation wait at most 500 ms for queued writes; a timeout disables further writes, while an already-issued OS request may still settle. Records cover manager operations observed after Host startup, not direct CLI commands or manual file changes. Package-manager output remains in its existing detailed logs and is not copied into this summary. Missing records, a start without an end, and damaged history do not establish a successful operation. This collection does not diagnose kernel hangs, OOM or native crashes. Persistence failures also disable the writer for the rest of this shell lifetime, including after recovery or installer failure. This bounds repeated failing I/O and avoids appending behind an uncertain partial write. Restarting the application creates a new writer. Omission and repair counters cover only the current shell lifetime. Retained segments provide a larger local troubleshooting window than the deliberately smaller report attachment. diff --git a/apps/desktop/README.zh.md b/apps/desktop/README.zh.md index 717424ea11..fe38779163 100644 --- a/apps/desktop/README.zh.md +++ b/apps/desktop/README.zh.md @@ -180,7 +180,7 @@ macOS 上自定义菜单保留 Electron 的标准 Window 菜单及应用隐藏 操作记录目录与应用单实例锁均按 `userData` 隔离;共享平台日志目录不用于保存操作记录。旧测试包的 `logs/plugin-operations` 不自动导入,避免混入其他 profile 的历史。修复未写完的尾行时保留当前段的完整行与上一段。Host 的 `transportDropped` 只表示最近一次成功发送时的计数下界;Desktop 拒绝的非法或运行 ID 不匹配记录不计入该值。 -基本报告保存后,壳会尝试通过独立 Worker 追加近期插件管理记录;该 Worker 以 `lib/operation-report-worker.js` 打包在 `app.asar` 中。完成标记和遗漏计数用于区分已完成收集与缺失或不完整的材料。Host 在加载 profile 前报告运行 ID;Host 错误中的 `hostRunId` 与该次运行的操作记录使用同一标识。在此通知之前发生的失败,其运行标识仍为未知。近期操作只是上下文,不是因果证据。收集只读取 `userData/plugin-operations` 下归其管理的两个文件,最多读取 64 KiB 和 100 条记录;完整追加内容不超过 64 KiB。500 ms 期限到达后请求终止 Worker,并在其退出后才确认收集结束。基本报告独立可用。同时只运行一个收集器;并发报告保留基本内容,不追加辅助部分。终止请求被拒绝时,必须观察到退出才能释放该名额。“获取诊断报告”可能在追加完成前定位基本文件。完成标记仅表示收集结束,不表示捕获了全部操作;排队字节数和遗漏计数说明缺失材料。 +基本报告保存后,壳会尝试通过独立 Worker 追加近期插件管理记录;该 Worker 以 `lib/operation-report-worker.js` 打包在 `app.asar` 中。完成标记和遗漏计数用于区分已完成收集与缺失或不完整的材料。Host 在加载 profile 前报告运行 ID;Host 错误中的 `hostRunId` 与该次运行的操作记录使用同一标识。在此通知之前发生的失败,其运行标识仍为未知。近期操作只是上下文,不是因果证据。收集只读取 `userData/plugin-operations` 下归其管理的两个文件,最多读取 64 KiB 和 100 条记录;完整追加内容不超过 64 KiB。500 ms 收集预算中的最后 100 ms 用于协作取消,随后才请求强制终止 Worker;只有 Worker 退出后才确认收集结束。应用退出、恢复退出或重启、确认安装更新也会取消收集,并最多等待 500 ms 确认 Worker 退出。退出过程中,延迟保存完成的基本报告不会再启动收集器。更新安装失败后可再次收集。Worker 在有界记录之间处理取消消息;已经发出的文件打开、状态读取、读取或关闭操作必须等待完成,单条记录的同步解析不能被抢占。最后的追加写入传入取消信号,但仍可能留下部分字节。协作清理未及时完成时仍需强制终止。基本报告独立可用。同时只运行一个收集器;并发报告保留基本内容,不追加辅助部分。终止请求被拒绝时,必须观察到退出才能释放该名额。“获取诊断报告”可能在追加完成前定位基本文件。完成标记仅表示收集结束,不表示捕获了全部操作;排队字节数和遗漏计数说明缺失材料。 操作历史保留两个 256 KiB 文件段,单条记录不超过 4 KiB,排队写入不超过 64 KiB。退出、恢复重启和安装准备最多等待排队写入 500 ms;超时会停用后续写入,但已经发出的操作系统请求仍可能随后结束。记录覆盖 Host 启动后观察到的管理器操作,不包含直接执行的 CLI 命令或手工文件修改。包管理器输出保留在原有详细日志中,不复制到摘要。缺失记录、只有开始没有结束的操作以及损坏的历史,都不能说明操作已成功。此收集不诊断内核卡死、OOM 或原生崩溃。持久化失败也会在本次壳生命周期内停用写入,包括恢复或安装器失败后继续运行的情况,以限制重复失败的 I/O,并避免在可能残缺的写入后追加记录。重启应用会创建新写入器。遗漏和修复计数仅覆盖当前壳生命周期。保留文件段提供比报告附件更大的本地排障历史窗口。 diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index f9091a31b5..0efdf3a0ae 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -66,7 +66,7 @@ import { DesktopTray } from './tray.ts' import { DesktopBackgroundNotice } from './background-notice.ts' let focusPrimaryWindow = (): void => {} -let stopForRecovery = async (): Promise => {} +let stopForRecovery = async (): Promise => { await stopOperationReports() } let shuttingDown = false /** * Set by quit entries that must not ask: crash recovery exit and restart, and the @@ -112,11 +112,34 @@ const recovery = new DesktopFatalRecovery({ let operationJournal: OperationJournal | undefined let operationReportJob: Promise | undefined +let operationReportAbort: AbortController | undefined +let operationReportsStopping = false +let operationReportStop: Promise | undefined + +/** Bound application shutdown waiting without claiming that an unresponsive Worker has exited. */ +function stopOperationReports(): Promise { + operationReportStop ??= waitForOperationReports() + return operationReportStop +} + +async function waitForOperationReports(): Promise { + operationReportsStopping = true + operationReportAbort?.abort() + const job = operationReportJob + if (job === undefined) return + let timer: ReturnType | undefined + try { + const exited = await Promise.race([job.then(() => true), new Promise((resolve) => { + timer = setTimeout(() => { resolve(false) }, 500) + })]) + if (!exited) console.error('dsh desktop: operation report Worker has not exited; continuing shutdown') + } finally { clearTimeout(timer) } +} function persistCrashReport(error: unknown, source: CrashReportSource): Promise { const directory = app.getPath('logs') const report = writeCrashReport(directory, { - operationHistoryPlanned: true, + operationHistoryPlanned: !shuttingDown && !operationReportsStopping, source, phase: backendReady ? 'running' : 'startup', error, @@ -129,12 +152,13 @@ function persistCrashReport(error: unknown, source: CrashReportSource): Promise< time: new Date(), }) void report.then((path) => { - if (path !== undefined && operationReportJob === undefined) { + if (path !== undefined && operationReportJob === undefined && !shuttingDown && !operationReportsStopping) { + operationReportAbort = new AbortController() operationReportJob = runOperationReportJob({ reportPath: path, directory: join(app.getPath('userData'), 'plugin-operations'), limits: { bytes: 64 * 1024, recordBytes: 4096, records: 100, outputBytes: 64 * 1024 }, journal: operationJournal?.status() ?? null, - }, 500) - void operationReportJob.finally(() => { operationReportJob = undefined }) + }, { timeoutMs: 500, cancellationGraceMs: 100 }, operationReportAbort.signal) + void operationReportJob.finally(() => { operationReportJob = undefined; operationReportAbort = undefined }) } }).catch((_error: unknown) => { /* Basic persistence failure is owned by fatal recovery. */ }) return report @@ -147,7 +171,12 @@ function reportFatal(error: unknown, source: CrashReportSource): void { void persistCrashReport(error, source) return } - void recovery.report(error, source).catch((failure: unknown) => { console.error(failure); app.exit(1) }) + void recovery.report(error, source).catch(async (failure: unknown) => { + console.error(failure) + shuttingDown = true + await stopOperationReports() + app.exit(1) + }) } protocol.registerSchemesAsPrivileged([{ @@ -562,6 +591,10 @@ async function main(): Promise { } if (state.phase === 'error' && state.failedOperation !== 'check') { const restoreHost = state.failedOperation === 'install' && updateStoppedHost && !quitting + if (state.failedOperation === 'install' && !quitting) { + operationReportsStopping = shuttingDown + operationReportStop = undefined + } shellInstallerOwnsQuit = false updateStoppedHost = false if (restoreHost) { @@ -591,8 +624,9 @@ async function main(): Promise { return welcomeBackend.read() } stopForRecovery = async () => { + const reports = stopOperationReports() try { await backend.close() } - finally { await operationJournal?.drain(500) } + finally { await Promise.all([operationJournal?.drain(500), reports]) } } const reconcileBackend = (): Promise => { @@ -650,7 +684,7 @@ async function main(): Promise { await platformView.closeAndWait() requireCleanStop = true updateStopFailure = undefined - await backend.stop() + await Promise.all([backend.stop(), stopOperationReports()]) updateStoppedHost = true // The backend's async cleanup callback can assign this after the reset above. const stopFailure = updateStopFailure as DesktopHostUncleanExitError | undefined @@ -659,6 +693,8 @@ async function main(): Promise { updateJournal?.action('install-confirmed') shellInstallerOwnsQuit = true } catch (error) { + operationReportsStopping = shuttingDown + operationReportStop = undefined if (!updateStoppedHost) await host.updateTasks('unlock').catch((unlockError: unknown) => { console.error(unlockError) }) throw error } finally { @@ -1287,7 +1323,7 @@ async function main(): Promise { updateSchedule.dispose() updateDialog.dispose() mandatoryUI?.dispose() - void Promise.all([Promise.resolve(mandatoryPolicy?.dispose()).then(() => policyAuth?.dispose()), + void Promise.all([stopOperationReports(), Promise.resolve(mandatoryPolicy?.dispose()).then(() => policyAuth?.dispose()), backend.close().then(async () => { await operationJournal?.drain(500) }, async (error: unknown) => { await operationJournal?.drain(500) throw error @@ -1383,7 +1419,9 @@ if (ownsDesktopInstance) void app.whenReady().then(main).catch(async (error: unk await writeFile(diagnosticFile, `${error instanceof Error ? error.stack ?? message : message}\n`).catch(() => undefined) } reportFatal(error, 'main') -}).catch((error: unknown) => { +}).catch(async (error: unknown) => { console.error(error) + shuttingDown = true + await stopOperationReports() app.exit(1) }) diff --git a/apps/desktop/src/operation-history.ts b/apps/desktop/src/operation-history.ts index 70902528f9..72082a552e 100644 --- a/apps/desktop/src/operation-history.ts +++ b/apps/desktop/src/operation-history.ts @@ -1,6 +1,7 @@ /** Bounded, cancellable reads of Desktop-owned plugin operation history. */ import { open } from 'node:fs/promises' import { join } from 'node:path' +import { setImmediate } from 'node:timers/promises' import { parsePluginOperationRecord, type PluginOperationRecord } from '@deepseek-ai/dsh-plugin-manager/diagnostic' /** Resource budgets for auxiliary history collection, independent of basic crash report writing. */ @@ -24,7 +25,8 @@ export interface OperationHistoryReadResult { /** * Read only the bounded tails of the two owned history files, newest segment first. - * Cancellation stops subsequent I/O and parsing; an already-issued OS read must settle before its handle closes. + * Cancellation is observed between bounded records after yielding to Worker messages and stops subsequent I/O and parsing. + * An already-issued OS read must settle before its handle closes. * @param directory - Desktop's operation history directory; unrelated files are never enumerated. * @param limits - Positive byte, record-size and record-count budgets. * @param signal - Caller-owned collection deadline or shutdown cancellation. @@ -36,6 +38,7 @@ export async function readOperationHistory( if (![limits.bytes, limits.recordBytes, limits.records].every(value => Number.isSafeInteger(value) && value > 0)) { throw new RangeError('Operation history budgets must be positive safe integers') } + await setImmediate() const cancelled = (): boolean => signal.aborted const records: PluginOperationRecord[] = [] let bytesRead = 0 @@ -72,6 +75,8 @@ export async function readOperationHistory( let end = buffer.lastIndexOf(10, offset - 1) if (end !== offset - 1) truncated = true while (end >= 0 && records.length < limits.records && !cancelled()) { + await setImmediate() + if (cancelled()) break const preceding = end === 0 ? -1 : buffer.lastIndexOf(10, end - 1) if (preceding < 0 && start > 0) { truncated = true; break } const size = end - preceding - 1 diff --git a/apps/desktop/src/operation-report-job.ts b/apps/desktop/src/operation-report-job.ts index c8f731c0ab..55cf911783 100644 --- a/apps/desktop/src/operation-report-job.ts +++ b/apps/desktop/src/operation-report-job.ts @@ -11,40 +11,71 @@ export interface OperationReportJobInput { readonly journal: OperationJournalStatus | null } -/** A timed-out job can have appended a partial section; the saved basic report remains valid. */ +/** Collection and cooperative shutdown share one total budget. */ +export interface OperationReportJobOptions { + /** Deadline for requesting forced termination, including Worker startup. */ + readonly timeoutMs: number + /** Reserved part of timeoutMs for cancellation and resource cleanup. */ + readonly cancellationGraceMs: number +} + +/** Cancellation can leave a partial append; the independently saved basic report remains valid. */ export interface OperationReportJobResult { readonly appended: boolean readonly timedOut: boolean + readonly cancelled: boolean } /** - * Start one isolated collector and await its exit, terminating it at the deadline. + * Collect history, request cooperative cancellation, then force termination if cleanup exceeds its budget. * @param input - Bounded collection request for an existing basic report. - * @param timeoutMs - Wall-clock budget including worker startup, before termination is requested. - * @returns Append outcome only after the worker exits; worker startup/error failures remain auxiliary failures. + * @param options - Total deadline and the cleanup window reserved within it. + * @param signal - Application shutdown or recovery cancellation; an already-aborted signal prevents startup. + * @returns Append outcome only after Worker exit; cancellation or termination requests alone do not confirm exit. */ -export async function runOperationReportJob(input: OperationReportJobInput, timeoutMs: number): Promise { - if (!Number.isSafeInteger(timeoutMs) || timeoutMs < 1) throw new RangeError('Invalid operation report deadline') +export async function runOperationReportJob( + input: OperationReportJobInput, options: OperationReportJobOptions, signal: AbortSignal, +): Promise { + const { timeoutMs, cancellationGraceMs } = options + if (!Number.isSafeInteger(timeoutMs) || !Number.isSafeInteger(cancellationGraceMs) + || cancellationGraceMs < 1 || timeoutMs <= cancellationGraceMs) throw new RangeError('Invalid operation report deadlines') + if (signal.aborted) return { appended: false, timedOut: false, cancelled: true } + const deadline = performance.now() + timeoutMs let worker: Worker try { worker = new Worker(new URL('./operation-report-worker.js', import.meta.url), { workerData: input, resourceLimits: { maxOldGenerationSizeMb: 32, maxYoungGenerationSizeMb: 8, stackSizeMb: 2 }, }) } - catch (_error) { return { appended: false, timedOut: false } } + catch (_error) { return { appended: false, timedOut: false, cancelled: false } } return new Promise((resolve) => { let appended = false let failed = false let timedOut = false - const timer = setTimeout(() => { + let cancelled = false + let termination: ReturnType | undefined + const force = (): void => { void worker.terminate().catch(() => { failed = true }) } + const cancel = (): void => { + if (cancelled) return + cancelled = true + clearTimeout(collection) + try { worker.postMessage('cancel') } + catch (_error) { failed = true } + termination = setTimeout(force, Math.max(0, Math.min(cancellationGraceMs, deadline - performance.now()))) + } + const collection = setTimeout(() => { timedOut = true - void worker.terminate().catch(() => { failed = true }) - }, timeoutMs) + cancel() + }, Math.max(0, deadline - performance.now() - cancellationGraceMs)) + signal.addEventListener('abort', cancel, { once: true }) worker.on('message', (message: unknown) => { appended = message === true }) worker.once('error', () => { failed = true }) worker.once('exit', (code) => { - clearTimeout(timer) - resolve({ appended: appended && code === 0 && !failed && !timedOut, timedOut }) + clearTimeout(collection) + clearTimeout(termination) + signal.removeEventListener('abort', cancel) + resolve({ appended: appended && code === 0 && !failed && !cancelled, timedOut, cancelled }) }) + if (signal.aborted) cancel() }) } diff --git a/apps/desktop/src/operation-report-worker.ts b/apps/desktop/src/operation-report-worker.ts index c652fe50f3..29df4658b7 100644 --- a/apps/desktop/src/operation-report-worker.ts +++ b/apps/desktop/src/operation-report-worker.ts @@ -11,5 +11,13 @@ const input = z.object({ repairedSegments: z.number().int().nonnegative(), queuedBytes: z.number().int().nonnegative(), disabled: z.boolean() }).nullable(), }).parse(workerData) -const appended = await appendOperationReport(input.reportPath, input.directory, input.limits, new AbortController().signal, input.journal) -parentPort?.postMessage(appended) +const cancellation = new AbortController() +const cancel = (message: unknown): void => { if (message === 'cancel') cancellation.abort() } +parentPort?.on('message', cancel) +try { + const appended = await appendOperationReport(input.reportPath, input.directory, input.limits, cancellation.signal, input.journal) + parentPort?.postMessage(appended) +} finally { + parentPort?.off('message', cancel) + parentPort?.close() +} diff --git a/apps/desktop/src/operation-report.ts b/apps/desktop/src/operation-report.ts index 87dd323afc..3839bca2f1 100644 --- a/apps/desktop/src/operation-report.ts +++ b/apps/desktop/src/operation-report.ts @@ -1,5 +1,6 @@ /** Best-effort operation history appended only after the independent basic report exists. */ import { writeFile } from 'node:fs/promises' +import { setImmediate } from 'node:timers/promises' import { readOperationHistory, type OperationHistoryReadLimits } from './operation-history.ts' import type { OperationJournalStatus } from './operation-journal.ts' @@ -24,8 +25,9 @@ export async function appendOperationReport( ): Promise { try { if (!Number.isSafeInteger(limits.outputBytes) || limits.outputBytes < 1) return false + const cancelled = (): boolean => signal.aborted const history = await readOperationHistory(directory, limits, signal) - if (signal.aborted) return false + if (cancelled()) return false const { records, ...facts } = history const header = (omitted: number): string => '\n--- recent plugin operations (context, not proof of cause) ---\n' + JSON.stringify({ journal, ...facts, outputRecordsOmitted: omitted }).slice(0, -1) + ',"records":[' @@ -34,6 +36,8 @@ export async function appendOperationReport( if (bytes > limits.outputBytes) return false const retained: string[] = [] for (const record of records) { + await setImmediate() + if (cancelled()) return false const encoded = JSON.stringify(record) const addition = Buffer.byteLength(encoded) + (retained.length === 0 ? 0 : 1) if (bytes + addition > limits.outputBytes) break @@ -41,6 +45,8 @@ export async function appendOperationReport( bytes += addition } const section = header(records.length - retained.length) + retained.join(',') + footer + await setImmediate() + if (cancelled()) return false await writeFile(reportPath, section, { flag: 'a', signal }) return true } catch (_error) { diff --git a/apps/desktop/tests/main-startup.spec.ts b/apps/desktop/tests/main-startup.spec.ts index e53e3c5f13..99738380f3 100644 --- a/apps/desktop/tests/main-startup.spec.ts +++ b/apps/desktop/tests/main-startup.spec.ts @@ -20,7 +20,7 @@ type InvokeEvent = { sender?: unknown; senderFrame: { url: string } } type InvokeHandler = (event: InvokeEvent, ...args: unknown[]) => unknown vi.mock('../src/web-document.ts', () => ({ authenticateWebHost: async () => 'test-cookie', serveWebDocument: vi.fn(), forwardWebRequest: vi.fn() })) -vi.mock('../src/operation-report-job.ts', () => ({ runOperationReportJob: vi.fn(async () => ({ appended: false, timedOut: false })) })) +vi.mock('../src/operation-report-job.ts', () => ({ runOperationReportJob: vi.fn(async () => ({ appended: false, timedOut: false, cancelled: false })) })) // Report persistence has its own unit tests; here it resolves within microtasks so the fatal // dialog never outlives the test that triggered it. vi.mock('../src/crash-report.ts', async importOriginal => ({ @@ -2176,7 +2176,7 @@ describe('desktop main startup', () => { expect(runOperationReportJob).toHaveBeenCalledWith({ reportPath: 'desktop-test-logs/crash-test.log', directory: join(harness.app.getPath('userData'), 'plugin-operations'), limits: { bytes: 65536, recordBytes: 4096, records: 100, outputBytes: 65536 }, journal: null, - }, 500) + }, { timeoutMs: 500, cancellationGraceMs: 100 }, expect.any(AbortSignal)) expect((harness.dialog.showMessageBox.mock.calls[0]![0] as MessageBoxOptions).detail).toContain('profile has no cordis.yml') }) @@ -2211,7 +2211,14 @@ describe('desktop main startup', () => { await harness.quitCompleted.promise }) - it.each([0, 1, 2])('waits for Host exit before recovery action %s', async (response) => { + it.each([{ response: 0, stuck: false }, { response: 1, stuck: false }, { response: 2, stuck: false }, + { response: 0, stuck: true }])('waits for Host exit and bounds Worker cleanup for recovery $response (stuck=$stuck)', async ({ response, stuck }) => { + let finishReport!: (result: { appended: boolean; timedOut: boolean; cancelled: boolean }) => void + let reportSignal: AbortSignal | undefined + vi.mocked(runOperationReportJob).mockImplementationOnce(async (_input, _options, signal) => { + reportSignal = signal + return new Promise((resolve) => { finishReport = resolve }) + }) harness.dialog.showMessageBox.mockResolvedValue({ response, checkboxChecked: false }) await import('../src/main.ts') await harness.preparing.promise @@ -2225,7 +2232,16 @@ describe('desktop main startup', () => { expect(harness.app.relaunch).not.toHaveBeenCalled() expect(harness.disableAllPlugins).not.toHaveBeenCalled() host.exited.resolve() + await vi.advanceTimersByTimeAsync(0) + expect(reportSignal?.aborted).toBe(true) + expect(harness.app.quit).not.toHaveBeenCalled() + if (stuck) await vi.advanceTimersByTimeAsync(500) + else finishReport({ appended: false, timedOut: false, cancelled: true }) await harness.quitCompleted.promise + if (stuck) { + expect(console.error).toHaveBeenCalledWith('dsh desktop: operation report Worker has not exited; continuing shutdown') + finishReport({ appended: false, timedOut: false, cancelled: true }) + } expect(harness.app.relaunch).toHaveBeenCalledTimes(response === 0 ? 0 : 1) expect(harness.disableAllPlugins).toHaveBeenCalledTimes(response === 2 ? 1 : 0) if (response === 2) { diff --git a/apps/desktop/tests/operation-report-job.spec.ts b/apps/desktop/tests/operation-report-job.spec.ts index c8402971b2..82ea02fcd8 100644 --- a/apps/desktop/tests/operation-report-job.spec.ts +++ b/apps/desktop/tests/operation-report-job.spec.ts @@ -10,15 +10,18 @@ const input = { reportPath: 'saved.log', directory: 'history', it('does not report cancellation complete until the terminated worker exits', async () => { vi.useFakeTimers() - const worker = Object.assign(new EventEmitter(), { terminate: vi.fn(async () => 1) }) + const worker = Object.assign(new EventEmitter(), { postMessage: vi.fn(), terminate: vi.fn(async () => 1) }) create.mockReturnValue(worker) let settled = false - const result = runOperationReportJob(input, 500).then((value) => { settled = true; return value }) + const result = runOperationReportJob(input, { timeoutMs: 500, cancellationGraceMs: 100 }, new AbortController().signal).then((value) => { + settled = true + return value + }) await vi.advanceTimersByTimeAsync(500) expect(worker.terminate).toHaveBeenCalledTimes(1) expect(settled).toBe(false) worker.emit('exit', 1) - expect(await result).toEqual({ appended: false, timedOut: true }) + expect(await result).toEqual({ appended: false, timedOut: true, cancelled: true }) expect(vi.getTimerCount()).toBe(0) }) @@ -26,34 +29,75 @@ it('requires successful worker exit after the complete append message', async () vi.useFakeTimers() const worker = new EventEmitter() create.mockReturnValue(worker) - const result = runOperationReportJob(input, 500) + const result = runOperationReportJob(input, { timeoutMs: 500, cancellationGraceMs: 100 }, new AbortController().signal) worker.emit('message', true) worker.emit('exit', 0) - expect(await result).toEqual({ appended: true, timedOut: false }) + expect(await result).toEqual({ appended: true, timedOut: false, cancelled: false }) expect(vi.getTimerCount()).toBe(0) }) it('contains startup and runtime worker failures', async () => { create.mockImplementationOnce(() => { throw new Error('worker unavailable') }) - expect(await runOperationReportJob(input, 500)).toEqual({ appended: false, timedOut: false }) + expect(await runOperationReportJob(input, { timeoutMs: 500, cancellationGraceMs: 100 }, new AbortController().signal)) + .toEqual({ appended: false, timedOut: false, cancelled: false }) const worker = new EventEmitter() create.mockReturnValue(worker) - const result = runOperationReportJob(input, 500) + const result = runOperationReportJob(input, { timeoutMs: 500, cancellationGraceMs: 100 }, new AbortController().signal) worker.emit('error', new Error('collector failed')) worker.emit('exit', 1) - expect(await result).toEqual({ appended: false, timedOut: false }) + expect(await result).toEqual({ appended: false, timedOut: false, cancelled: false }) }) it('retains ownership after termination rejects until the worker actually exits', async () => { vi.useFakeTimers() - const worker = Object.assign(new EventEmitter(), { terminate: vi.fn(async () => { throw new Error('termination failed') }) }) + const worker = Object.assign(new EventEmitter(), { postMessage: vi.fn(), terminate: vi.fn(async () => { throw new Error('termination failed') }) }) create.mockReturnValue(worker) let settled = false - const result = runOperationReportJob(input, 500).then((value) => { settled = true; return value }) + const result = runOperationReportJob(input, { timeoutMs: 500, cancellationGraceMs: 100 }, new AbortController().signal).then((value) => { + settled = true + return value + }) await vi.advanceTimersByTimeAsync(500) expect(worker.terminate).toHaveBeenCalledTimes(1) expect(settled).toBe(false) worker.emit('exit', 1) - expect(await result).toEqual({ appended: false, timedOut: true }) + expect(await result).toEqual({ appended: false, timedOut: true, cancelled: true }) expect(vi.getTimerCount()).toBe(0) }) + + +it('allows cooperative exit during the reserved cleanup window', async () => { + vi.useFakeTimers() + const worker = Object.assign(new EventEmitter(), { postMessage: vi.fn(), terminate: vi.fn() }) + create.mockReturnValue(worker) + const result = runOperationReportJob(input, { timeoutMs: 500, cancellationGraceMs: 100 }, new AbortController().signal) + await vi.advanceTimersByTimeAsync(400) + expect(worker.postMessage).toHaveBeenCalledExactlyOnceWith('cancel') + expect(worker.terminate).not.toHaveBeenCalled() + worker.emit('exit', 0) + expect(await result).toEqual({ appended: false, timedOut: true, cancelled: true }) + expect(vi.getTimerCount()).toBe(0) +}) + +it('cancels on application shutdown without extending the cleanup window', async () => { + vi.useFakeTimers() + const worker = Object.assign(new EventEmitter(), { postMessage: vi.fn(), terminate: vi.fn(async () => 1) }) + create.mockReturnValue(worker) + const controller = new AbortController() + const result = runOperationReportJob(input, { timeoutMs: 500, cancellationGraceMs: 100 }, controller.signal) + await vi.advanceTimersByTimeAsync(200) + controller.abort() + controller.abort() + expect(worker.postMessage).toHaveBeenCalledExactlyOnceWith('cancel') + await vi.advanceTimersByTimeAsync(100) + expect(worker.terminate).toHaveBeenCalledTimes(1) + worker.emit('exit', 1) + expect(await result).toEqual({ appended: false, timedOut: false, cancelled: true }) + expect(vi.getTimerCount()).toBe(0) +}) + +it('does not start a collector after shutdown was requested', async () => { + expect(await runOperationReportJob(input, { timeoutMs: 500, cancellationGraceMs: 100 }, AbortSignal.abort())) + .toEqual({ appended: false, timedOut: false, cancelled: true }) + expect(create).not.toHaveBeenCalled() +}) diff --git a/apps/desktop/tests/operation-report-worker.e2e.ts b/apps/desktop/tests/operation-report-worker.e2e.ts index 27907e64f5..fd89dd11da 100644 --- a/apps/desktop/tests/operation-report-worker.e2e.ts +++ b/apps/desktop/tests/operation-report-worker.e2e.ts @@ -3,7 +3,7 @@ import { existsSync } from 'node:fs' import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { fileURLToPath } from 'node:url' +import { fileURLToPath, pathToFileURL } from 'node:url' import { execa } from 'execa' import { expect, it, onTestFinished } from 'vitest' @@ -26,13 +26,13 @@ it.skipIf(!existsSync(new URL('../lib/types/', import.meta.url)))('appends opera limits: { bytes: 65536, recordBytes: 4096, records: 100, outputBytes: 65536 } } const script = join(directory, mode + '.mjs') await writeFile(script, mode === 'job' - ? `import { runOperationReportJob } from ${JSON.stringify(job.href)};\nconsole.log(JSON.stringify(await runOperationReportJob(${JSON.stringify(input)}, 10000)));\n` + ? `import { runOperationReportJob } from ${JSON.stringify(job.href)};\nconsole.log(JSON.stringify(await runOperationReportJob(${JSON.stringify(input)}, { timeoutMs: 10000, cancellationGraceMs: 100 }, new AbortController().signal)));\n` : `import { Worker } from 'node:worker_threads';\nconst worker = new Worker(${JSON.stringify(fileURLToPath(worker))}, { workerData: ${JSON.stringify(input)} });\nworker.on('error', error => { throw error });\nworker.on('exit', code => { if (code !== 0) process.exitCode = 1 });\n`) const result = await execa(process.execPath, [script], { timeout: 30000, reject: false }) expect(result.timedOut).toBe(false) expect(result.signal).toBeUndefined() expect(result.exitCode, result.stderr).toBe(0) - if (mode === 'job') expect(JSON.parse(result.stdout)).toEqual({ appended: true, timedOut: false }) + if (mode === 'job') expect(JSON.parse(result.stdout)).toEqual({ appended: true, timedOut: false, cancelled: false }) const report = await readFile(reportPath, 'utf8') expect(report.startsWith('basic report\n')).toBe(true) const summary = JSON.parse(report.split('\n').find(line => line.startsWith('{'))!) as { records: unknown[] } @@ -40,3 +40,42 @@ it.skipIf(!existsSync(new URL('../lib/types/', import.meta.url)))('appends opera expect(report).toContain('operation history collection complete') } }) + +it.skipIf(!existsSync(new URL('../lib/types/', import.meta.url)))('cancels the packaged Worker after an in-flight open settles', async () => { + const directory = await mkdtemp(join(tmpdir(), 'desktop-operation-cancel-')) + onTestFinished(async () => { await rm(directory, { recursive: true, force: true }) }) + const reportPath = join(directory, 'report.log') + await writeFile(reportPath, 'basic report\n') + await writeFile(join(directory, 'operations.jsonl'), '{}\n') + const hook = join(directory, 'hook.mjs') + await writeFile(hook, `import fs from 'node:fs/promises'; +import { syncBuiltinESMExports } from 'node:module'; +import { parentPort } from 'node:worker_threads'; +const original = fs.open; +fs.open = async (...args) => { + const handle = await original(...args); + parentPort.postMessage('opened'); + await new Promise(resolve => parentPort.once('message', resolve)); + return handle; +}; +syncBuiltinESMExports(); +`) + const input = { reportPath, directory, journal: null, + limits: { bytes: 65536, recordBytes: 4096, records: 100, outputBytes: 65536 } } + const script = join(directory, 'cancel.mjs') + await writeFile(script, `import { Worker } from 'node:worker_threads'; +const worker = new Worker(${JSON.stringify(fileURLToPath(worker))}, { + workerData: ${JSON.stringify(input)}, execArgv: ['--import', ${JSON.stringify(pathToFileURL(hook).href)}] +}); +worker.on('message', message => { + if (message === 'opened') worker.postMessage('cancel'); + else console.log(JSON.stringify(message)); +}); +worker.on('error', error => { throw error }); +worker.on('exit', code => { process.exitCode = code }); +`) + const result = await execa(process.execPath, [script], { timeout: 30000, reject: false }) + expect(result.exitCode, result.stderr).toBe(0) + expect(result.stdout).toBe('false') + expect(await readFile(reportPath, 'utf8')).toBe('basic report\n') +})