From 53927a7e55db691f9f5cd9a9278567d62d6fb31c Mon Sep 17 00:00:00 2001 From: minpeter Date: Fri, 24 Jul 2026 18:15:01 +0900 Subject: [PATCH] fix(coding-agent): isolate inspector import crashes --- packages/coding-agent/src/changes.md | 27 +++ packages/coding-agent/src/cli.ts | 2 + packages/coding-agent/src/inspector-policy.ts | 36 ++++ .../src/modes/interactive/changes.md | 24 +++ .../src/modes/interactive/interactive-mode.ts | 12 +- .../test/fixtures/inspector-fixed-port.ts | 12 ++ .../test/fixtures/inspector-recovery-env.ts | 20 +++ .../inspector-vm-import-crash.test.ts | 167 ++++++++++++++++++ 8 files changed, 298 insertions(+), 2 deletions(-) create mode 100644 packages/coding-agent/src/inspector-policy.ts create mode 100644 packages/coding-agent/test/fixtures/inspector-fixed-port.ts create mode 100644 packages/coding-agent/test/fixtures/inspector-recovery-env.ts create mode 100644 packages/coding-agent/test/suite/regressions/inspector-vm-import-crash.test.ts diff --git a/packages/coding-agent/src/changes.md b/packages/coding-agent/src/changes.md index c673600d7..fce1f6fd6 100644 --- a/packages/coding-agent/src/changes.md +++ b/packages/coding-agent/src/changes.md @@ -8,6 +8,33 @@ # changes +## Inspector handoff and VM-import crash isolation (2026-07-24) + +### What changed + +- The launcher closes an inherited startup Inspector endpoint immediately before spawning `cli-main`, allowing the + child process to bind the same configured endpoint instead of failing with `address already in use`. +- With `SENPI_RECOVER_INSPECTOR_VM_IMPORT=1` set at process start, interactive mode recovers only the exact unhandled + Inspector-eval rejection produced when `import()` runs without a VM dynamic-import callback. Recovery is fail-closed + by default; application-owned VM failures and unrelated uncaught exceptions remain fatal. + +### Why + +- The launcher and child previously inherited one fixed Inspector port, so developers attached to the wrapper rather + than the TUI process. Running asynchronous `import()` in Node's Inspector VM then terminated the attached process. + Node exposes no non-spoofable Inspector provenance on the global exception, so continuing requires an explicit + developer opt-in rather than weakening the default fatal boundary. + +### Why extension system couldn't handle this + +- Inspector ownership is decided before extensions load, and process-wide uncaught-exception handling belongs to the + host's terminal-restoration boundary. + +### Expected merge conflict zones + +- LOW: `cli.ts` immediately before the `cli-main` spawn. +- LOW: `modes/interactive/interactive-mode.ts` uncaught-exception handler. + ## Multi-session RPC mode, session-owned MCP/config-reload state, and back-compat guarantee (2026-07-23) ### What changed diff --git a/packages/coding-agent/src/cli.ts b/packages/coding-agent/src/cli.ts index f8e85ebd2..150a571bb 100644 --- a/packages/coding-agent/src/cli.ts +++ b/packages/coding-agent/src/cli.ts @@ -4,6 +4,7 @@ import { existsSync } from "node:fs"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; import { APP_NAME, getPackageDir, VERSION } from "./config.ts"; +import { releaseInheritedInspectorForChild } from "./inspector-policy.ts"; import { handleBootstrapSelfUpdate } from "./self-update-bootstrap.ts"; process.title = APP_NAME; @@ -35,6 +36,7 @@ function isMissingBundledWorkspaceDependencies(packageDir: string): boolean { async function runFullCli(): Promise { const extension = import.meta.url.endsWith(".ts") ? ".ts" : ".js"; const fullCliPath = fileURLToPath(new URL(`./cli-main${extension}`, import.meta.url)); + releaseInheritedInspectorForChild(); return await new Promise((resolve, reject) => { const child = spawn(process.execPath, [...process.execArgv, fullCliPath, ...args], { env: process.env, diff --git a/packages/coding-agent/src/inspector-policy.ts b/packages/coding-agent/src/inspector-policy.ts new file mode 100644 index 000000000..5701b7fff --- /dev/null +++ b/packages/coding-agent/src/inspector-policy.ts @@ -0,0 +1,36 @@ +import { close as closeInspector, url as inspectorUrl } from "node:inspector"; + +type UncaughtExceptionOrigin = "uncaughtException" | "unhandledRejection"; + +const VM_DYNAMIC_IMPORT_CALLBACK_MISSING = "ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING"; +const INSPECTOR_IMPORT_FRAME = "at importModuleDynamicallyCallback (node:internal/modules/esm/"; +const INSPECTOR_TIMEOUT_FRAME = /\bat Timeout\._onTimeout \(:\d+:\d+\)/; +const RECOVER_INSPECTOR_VM_IMPORT = process.env.SENPI_RECOVER_INSPECTOR_VM_IMPORT === "1"; + +function hasInheritedInspectorOption(): boolean { + return ( + process.execArgv.some((argument) => argument.startsWith("--inspect")) || + process.env.NODE_OPTIONS?.includes("--inspect") === true + ); +} + +export function releaseInheritedInspectorForChild(): void { + if (inspectorUrl() !== undefined && hasInheritedInspectorOption()) { + closeInspector(); + } +} + +export function isRecoverableInspectorVmImportError(error: unknown, origin: UncaughtExceptionOrigin): boolean { + if (!RECOVER_INSPECTOR_VM_IMPORT || origin !== "unhandledRejection" || inspectorUrl() === undefined) { + return false; + } + if (typeof error !== "object" || error === null || !("code" in error) || !("stack" in error)) { + return false; + } + return ( + error.code === VM_DYNAMIC_IMPORT_CALLBACK_MISSING && + typeof error.stack === "string" && + error.stack.includes(INSPECTOR_IMPORT_FRAME) && + INSPECTOR_TIMEOUT_FRAME.test(error.stack) + ); +} diff --git a/packages/coding-agent/src/modes/interactive/changes.md b/packages/coding-agent/src/modes/interactive/changes.md index 285a1a066..92d767710 100644 --- a/packages/coding-agent/src/modes/interactive/changes.md +++ b/packages/coding-agent/src/modes/interactive/changes.md @@ -1,5 +1,29 @@ # changes +## Inspector VM-import rejection recovery (2026-07-24) + +### What changed + +- With `SENPI_RECOVER_INSPECTOR_VM_IMPORT=1` set at process start, interactive mode keeps running when an active Node + Inspector evaluation creates the exact `ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING` unhandled rejection from an + `` timer callback. Recovery remains disabled by default. +- The TUI shows guidance to use `require()` or a target-side loader. +- Application-owned `evalmachine.` failures and every unrelated uncaught exception retain the existing + terminal restoration and exit-1 behavior. + +### Why + +- Node's Inspector evaluator does not provide a dynamic-import callback. A delayed `import()` from `node inspect exec` + previously surfaced as a process-wide uncaught exception and terminated an otherwise healthy debugging session. + +### Why extension system couldn't handle this + +- The rejection reaches the process-wide fatal handler before extension-level tool or event hooks can intercept it. + +### Expected merge conflict zones + +- LOW: `interactive-mode.ts` around `uncaughtCrash()` and `registerSignalHandlers()`. + ## accepted-only compaction queue transfer (2026-07-24) ### What changed diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index 38b3ccd10..6f4789dff 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -94,6 +94,7 @@ import type { SourceInfo } from "../../core/source-info.ts"; import { isInstallTelemetryEnabled } from "../../core/telemetry.ts"; import type { TruncationResult } from "../../core/tools/truncate.ts"; import { hasTrustRequiringProjectResources, ProjectTrustStore } from "../../core/trust-manager.ts"; +import { isRecoverableInspectorVmImportError } from "../../inspector-policy.ts"; import { getChangelogPath, getNewEntries, normalizeChangelogLinks, parseChangelog } from "../../utils/changelog.ts"; import { copyToClipboard, readClipboardText } from "../../utils/clipboard.ts"; import { extensionForImageMimeType, readClipboardImage } from "../../utils/clipboard-image.ts"; @@ -4154,10 +4155,16 @@ export class InteractiveMode { * call ui.stop() to restore cooked mode, the cursor, and disable bracketed * paste / Kitty / modifyOtherKeys sequences. */ - private uncaughtCrash(error: Error): never { + private uncaughtCrash(error: Error, origin: "uncaughtException" | "unhandledRejection"): void { if (this.isShuttingDown) { process.exit(1); } + if (isRecoverableInspectorVmImportError(error, origin)) { + this.showWarning( + "Node Inspector dynamic import is unsupported; use require() or a target-side loader. Senpi kept running.", + ); + return; + } this.isShuttingDown = true; try { this.unregisterSignalHandlers(); @@ -4217,7 +4224,8 @@ export class InteractiveMode { // Restore the terminal before the process dies on any uncaught throw. // Without this, an unhandled exception from extension code (or anywhere // in pi) leaves the terminal in raw mode with no cursor. - const uncaughtExceptionHandler = (error: Error) => this.uncaughtCrash(error); + const uncaughtExceptionHandler = (error: Error, origin: "uncaughtException" | "unhandledRejection") => + this.uncaughtCrash(error, origin); process.prependListener("uncaughtException", uncaughtExceptionHandler); this.signalCleanupHandlers.push(() => process.off("uncaughtException", uncaughtExceptionHandler)); } diff --git a/packages/coding-agent/test/fixtures/inspector-fixed-port.ts b/packages/coding-agent/test/fixtures/inspector-fixed-port.ts new file mode 100644 index 000000000..f48e1d1df --- /dev/null +++ b/packages/coding-agent/test/fixtures/inspector-fixed-port.ts @@ -0,0 +1,12 @@ +import { url as inspectorUrl } from "node:inspector"; +import { pathToFileURL } from "node:url"; + +const [cliPath, ...cliArgs] = process.argv.slice(2); +if (!cliPath) throw new Error("Expected a CLI source path"); + +const activeInspectorUrl = inspectorUrl(); +if (!activeInspectorUrl) throw new Error("Expected an active Inspector"); + +process.env.NODE_OPTIONS = `--inspect=127.0.0.1:${new URL(activeInspectorUrl).port}`; +process.argv = [process.execPath, cliPath, ...cliArgs]; +await import(pathToFileURL(cliPath).href); diff --git a/packages/coding-agent/test/fixtures/inspector-recovery-env.ts b/packages/coding-agent/test/fixtures/inspector-recovery-env.ts new file mode 100644 index 000000000..f26c6bb25 --- /dev/null +++ b/packages/coding-agent/test/fixtures/inspector-recovery-env.ts @@ -0,0 +1,20 @@ +import { close as closeInspector, open as openInspector } from "node:inspector"; +import { isRecoverableInspectorVmImportError } from "../../src/inspector-policy.ts"; + +process.env.SENPI_RECOVER_INSPECTOR_VM_IMPORT = "1"; +openInspector(0, "127.0.0.1", false); + +const error = Object.assign(new TypeError("A dynamic import callback was not specified."), { + code: "ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING", +}); +error.stack = [ + "TypeError [ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING]: A dynamic import callback was not specified.", + " at importModuleDynamicallyCallback (node:internal/modules/esm/utils:279:9)", + " at Timeout._onTimeout (:1:16)", +].join("\n"); + +try { + console.log(isRecoverableInspectorVmImportError(error, "unhandledRejection")); +} finally { + closeInspector(); +} diff --git a/packages/coding-agent/test/suite/regressions/inspector-vm-import-crash.test.ts b/packages/coding-agent/test/suite/regressions/inspector-vm-import-crash.test.ts new file mode 100644 index 000000000..b6acf814b --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/inspector-vm-import-crash.test.ts @@ -0,0 +1,167 @@ +import { spawnSync } from "node:child_process"; +import { close as closeInspector, url as inspectorUrl, open as openInspector } from "node:inspector"; +import { fileURLToPath } from "node:url"; +import { afterAll, afterEach, describe, expect, test, vi } from "vitest"; +import { InteractiveMode } from "../../../src/modes/interactive/interactive-mode.ts"; + +type UncaughtExceptionOrigin = "uncaughtException" | "unhandledRejection"; + +const originalRecoveryFlag = vi.hoisted(() => { + const original = process.env.SENPI_RECOVER_INSPECTOR_VM_IMPORT; + process.env.SENPI_RECOVER_INSPECTOR_VM_IMPORT = "1"; + return original; +}); + +type UncaughtCrashThis = { + isShuttingDown: boolean; + showWarning: (message: string) => void; + ui: { stop: () => void }; + unregisterSignalHandlers: () => void; +}; + +type InteractiveModePrototypeWithUncaughtCrash = { + uncaughtCrash(this: UncaughtCrashThis, error: Error, origin: UncaughtExceptionOrigin): void; +}; + +class ProcessExitError extends Error { + readonly code: string | number | null | undefined; + + constructor(code: string | number | null | undefined) { + super(`process.exit(${String(code)})`); + this.code = code; + } +} + +const interactiveModePrototype = InteractiveMode.prototype as unknown as InteractiveModePrototypeWithUncaughtCrash; + +function callUncaughtCrash(context: UncaughtCrashThis, error: Error, origin: UncaughtExceptionOrigin): void { + interactiveModePrototype.uncaughtCrash.call(context, error, origin); +} + +function createVmImportError(source: "" | "evalmachine."): Error { + const error = Object.assign(new TypeError("A dynamic import callback was not specified."), { + code: "ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING", + }); + error.stack = [ + "TypeError [ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING]: A dynamic import callback was not specified.", + " at importModuleDynamicallyCallback (node:internal/modules/esm/utils:279:9)", + ` at Timeout._onTimeout (${source}:1:16)`, + " at listOnTimeout (node:internal/timers:605:17)", + ].join("\n"); + return error; +} + +function createCrashContext(): UncaughtCrashThis { + return { + isShuttingDown: false, + showWarning: vi.fn(), + ui: { stop: vi.fn() }, + unregisterSignalHandlers: vi.fn(), + }; +} + +afterEach(() => { + vi.restoreAllMocks(); +}); + +afterAll(() => { + if (originalRecoveryFlag === undefined) { + delete process.env.SENPI_RECOVER_INSPECTOR_VM_IMPORT; + } else { + process.env.SENPI_RECOVER_INSPECTOR_VM_IMPORT = originalRecoveryFlag; + } +}); + +describe("Inspector VM dynamic import crash handling", () => { + test("hands the fixed Inspector endpoint from the launcher to cli-main", () => { + const fixturePath = fileURLToPath(new URL("../../fixtures/inspector-fixed-port.ts", import.meta.url)); + const cliPath = fileURLToPath(new URL("../../../src/cli.ts", import.meta.url)); + const result = spawnSync(process.execPath, ["--import", "tsx", fixturePath, cliPath, "--help"], { + encoding: "utf8", + env: { + ...process.env, + NODE_OPTIONS: "--inspect=127.0.0.1:0", + PI_OFFLINE: "1", + }, + timeout: 30_000, + }); + const output = `${result.stdout}${result.stderr}`; + const endpoints = [...output.matchAll(/Debugger listening on ws:\/\/127\.0\.0\.1:(\d+)\//g)].map( + (match) => match[1], + ); + + expect(result.status).toBe(0); + expect(output).not.toContain("address already in use"); + expect(endpoints).toHaveLength(2); + expect(new Set(endpoints).size).toBe(1); + }); + + test("keeps the interactive child running for the exact Inspector eval rejection", () => { + const context = createCrashContext(); + const openedInspector = inspectorUrl() === undefined; + if (openedInspector) openInspector(0, "127.0.0.1", false); + + try { + expect(() => + callUncaughtCrash(context, createVmImportError(""), "unhandledRejection"), + ).not.toThrow(); + expect(context.showWarning).toHaveBeenCalledWith( + "Node Inspector dynamic import is unsupported; use require() or a target-side loader. Senpi kept running.", + ); + } finally { + if (openedInspector) closeInspector(); + } + }); + + test("keeps application-owned VM failures on the existing fatal path", () => { + const context = createCrashContext(); + const exit = vi.spyOn(process, "exit").mockImplementation((code) => { + throw new ProcessExitError(code); + }); + const openedInspector = inspectorUrl() === undefined; + if (openedInspector) openInspector(0, "127.0.0.1", false); + + try { + expect(() => + callUncaughtCrash(context, createVmImportError("evalmachine."), "unhandledRejection"), + ).toThrow(ProcessExitError); + expect(exit).toHaveBeenCalledWith(1); + expect(context.showWarning).not.toHaveBeenCalled(); + } finally { + if (openedInspector) closeInspector(); + } + }); + + test("does not enable recovery when the environment changes after policy import", () => { + const fixturePath = fileURLToPath(new URL("../../fixtures/inspector-recovery-env.ts", import.meta.url)); + const env = { ...process.env }; + delete env.SENPI_RECOVER_INSPECTOR_VM_IMPORT; + const result = spawnSync(process.execPath, ["--import", "tsx", fixturePath], { + encoding: "utf8", + env, + timeout: 30_000, + }); + + expect(result.status).toBe(0); + expect(result.stdout.trim()).toBe("false"); + }); + + test("keeps direct uncaught exceptions fatal with recovery enabled", () => { + const context = createCrashContext(); + const exit = vi.spyOn(process, "exit").mockImplementation((code) => { + throw new ProcessExitError(code); + }); + const openedInspector = inspectorUrl() === undefined; + if (openedInspector) openInspector(0, "127.0.0.1", false); + + try { + expect(() => callUncaughtCrash(context, createVmImportError(""), "uncaughtException")).toThrow( + ProcessExitError, + ); + expect(exit).toHaveBeenCalledWith(1); + expect(context.showWarning).not.toHaveBeenCalled(); + } finally { + if (openedInspector) closeInspector(); + } + }); +});