diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index bba3a028c..f2ea1dd3a 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `debug` launch/attach failures so `configurationDone` no longer masks the underlying DAP launch error, early stop-outcome watchers cannot emit unhandled rejections, and directory-valued launch programs are rejected before adapter selection. ([#1187](https://github.com/can1357/oh-my-pi/issues/1187)) + ## [15.1.6] - 2026-05-19 ### Fixed diff --git a/packages/coding-agent/src/dap/session.ts b/packages/coding-agent/src/dap/session.ts index 29c389128..400ab5c1b 100644 --- a/packages/coding-agent/src/dap/session.ts +++ b/packages/coding-agent/src/dap/session.ts @@ -105,6 +105,41 @@ function toErrorMessage(value: unknown): string { return String(value); } +interface DapStartRequestFailure { + rejected: boolean; + error?: unknown; +} + +function trackDapStartRequest(promise: Promise, failure: DapStartRequestFailure): Promise { + return promise.catch(error => { + failure.rejected = true; + failure.error = error; + throw error; + }); +} + +function combineDapStartErrors(command: "launch" | "attach", startError: unknown, configurationError: unknown): Error { + const startMessage = toErrorMessage(startError); + const configurationMessage = toErrorMessage(configurationError); + if (startMessage === configurationMessage) { + return startError instanceof Error ? startError : new Error(startMessage); + } + return new Error( + `DAP ${command} failed: ${startMessage}\nDAP configurationDone also failed: ${configurationMessage}`, + ); +} + +async function throwPreferredDapStartError( + command: "launch" | "attach", + startFailure: DapStartRequestFailure, + configurationError: unknown, +): Promise { + await Promise.resolve(); + if (startFailure.rejected) { + throw combineDapStartErrors(command, startFailure.error, configurationError); + } + throw configurationError; +} function normalizePath(filePath: string): string { return path.resolve(filePath); } @@ -209,12 +244,20 @@ export class DapSessionManager { // DAP spec: many adapters do not respond to launch until after // configurationDone. Fire launch, complete the config handshake, // then await the launch response. - const launchPromise = client.sendRequest("launch", launchArguments, signal, timeoutMs); + const launchFailure: DapStartRequestFailure = { rejected: false }; + const launchPromise = trackDapStartRequest( + client.sendRequest("launch", launchArguments, signal, timeoutMs), + launchFailure, + ); // Mark handled so a fast error response doesn't become an unhandled // rejection while we await the config handshake. The actual error // still propagates when we await launchPromise below. launchPromise.catch(() => {}); - await this.#completeConfigurationHandshake(session, signal, timeoutMs); + try { + await this.#completeConfigurationHandshake(session, signal, timeoutMs); + } catch (error) { + await throwPreferredDapStartError("launch", launchFailure, error); + } await launchPromise; // Try to capture initial stopped state (e.g. stopOnEntry). // Timeout is acceptable — the program may simply be running. @@ -262,9 +305,17 @@ export class DapSessionManager { signal, Math.min(timeoutMs, STOP_CAPTURE_TIMEOUT_MS), ); - const attachPromise = client.sendRequest("attach", attachArguments, signal, timeoutMs); + const attachFailure: DapStartRequestFailure = { rejected: false }; + const attachPromise = trackDapStartRequest( + client.sendRequest("attach", attachArguments, signal, timeoutMs), + attachFailure, + ); attachPromise.catch(() => {}); - await this.#completeConfigurationHandshake(session, signal, timeoutMs); + try { + await this.#completeConfigurationHandshake(session, signal, timeoutMs); + } catch (error) { + await throwPreferredDapStartError("attach", attachFailure, error); + } await attachPromise; try { await untilAborted(signal, initialStopPromise); @@ -1085,7 +1136,9 @@ export class DapSessionManager { for (const p of promises) { p.catch(() => {}); } - return Promise.race(promises); + const outcome = Promise.race(promises); + outcome.catch(() => {}); + return outcome; } /** diff --git a/packages/coding-agent/src/prompts/tools/debug.md b/packages/coding-agent/src/prompts/tools/debug.md index f9abcb226..9817f97c7 100644 --- a/packages/coding-agent/src/prompts/tools/debug.md +++ b/packages/coding-agent/src/prompts/tools/debug.md @@ -4,6 +4,7 @@ Use for launching or attaching debuggers, setting breakpoints, stepping through - Prefer over bash for program state, breakpoints, stepping, thread inspection, or interrupting a running process. - `action: "launch"` starts a session; `program` is required, `adapter` optional (auto-selected from target path and workspace). + For Python, set `adapter: "debugpy"` and `program` to the target `.py` file; put interpreter/script flags in `args`. - `action: "attach"` connects to an existing process: `pid` for local attach, `port` for remote attach (where the adapter supports it), `adapter` to force a specific debugger. - **Breakpoints**: `set_breakpoint`/`remove_breakpoint` with source (`file`+`line`) or function (`function`); optional `condition` for conditional breakpoints. - **Flow control**: `continue` (resumes; briefly waits to observe whether the program stops or keeps running), `step_over`/`step_in`/`step_out` (single-step), `pause` (interrupt a running program so you can inspect state). @@ -15,6 +16,7 @@ Use for launching or attaching debuggers, setting breakpoints, stepping through - Only one active debug session is supported at a time. - Some adapters require a launched session to receive `configurationDone` before the target actually runs; if the tool says configuration is pending, set breakpoints and then call `continue`. - Adapter availability depends on local binaries. Common built-ins: `gdb`, `lldb-dap`, `python -m debugpy.adapter`, `dlv dap`. +- `program` must be an executable file or debug target, not a directory or interpreter name that resolves to a workspace directory. @@ -25,6 +27,9 @@ Use for launching or attaching debuggers, setting breakpoints, stepping through 4. If the program appears hung: `debug(action: "pause")` 5. Inspect state with `threads`, `stack_trace`, `scopes`, and `variables` + +# Launch a Python script with debugpy +`debug(action: "launch", adapter: "debugpy", program: "scripts/job.py", args: ["--flag"])` # Raw debugger command through repl `debug(action: "evaluate", expression: "info registers", context: "repl")` diff --git a/packages/coding-agent/src/tools/debug.ts b/packages/coding-agent/src/tools/debug.ts index 04a87e09c..c940dafce 100644 --- a/packages/coding-agent/src/tools/debug.ts +++ b/packages/coding-agent/src/tools/debug.ts @@ -1,3 +1,4 @@ +import * as fs from "node:fs/promises"; import type { AgentTool, AgentToolContext, @@ -6,7 +7,7 @@ import type { RenderResultOptions, } from "@oh-my-pi/pi-agent-core"; import { type Component, Text } from "@oh-my-pi/pi-tui"; -import { prompt } from "@oh-my-pi/pi-utils"; +import { isEnoent, prompt } from "@oh-my-pi/pi-utils"; import * as z from "zod/v4"; import { type DapBreakpointRecord, @@ -37,7 +38,7 @@ import { renderStatusLine } from "../tui"; import { CachedOutputBlock } from "../tui/output-block"; import type { ToolSession } from "."; import type { OutputMeta } from "./output-meta"; -import { resolveToCwd } from "./path-utils"; +import { formatPathRelativeToCwd, resolveToCwd } from "./path-utils"; import { formatExpandHint, formatStatusIcon, @@ -469,6 +470,21 @@ function getConfiguredAdapters(cwd: string): string { const adapters = getAvailableAdapters(cwd).map(adapter => adapter.name); return adapters.length > 0 ? adapters.join(", ") : "none"; } +async function validateLaunchProgram(program: string, cwd: string): Promise { + let isDirectory: boolean; + try { + isDirectory = (await fs.stat(program)).isDirectory(); + } catch (error) { + if (isEnoent(error)) return; + throw error; + } + if (!isDirectory) return; + + const displayPath = formatPathRelativeToCwd(program, cwd, { trailingSlash: true }); + throw new ToolError( + `launch program resolves to a directory: ${displayPath}. Pass an executable file path, or for Python use adapter "debugpy" with program set to the .py file.`, + ); +} interface DebugRenderArgs extends Partial {} @@ -628,6 +644,7 @@ export class DebugTool implements AgentTool void | Promise; + +class FakeDapClient { + readonly proc: DapClientState["proc"]; + readonly #exited = Promise.withResolvers(); + readonly #handlers = new Map>(); + #alive = true; + + constructor( + readonly adapter: DapResolvedAdapter, + readonly cwd: string, + readonly options: { + launchError?: string; + attachError?: string; + configurationDoneError?: string; + rejectStopWaiters?: boolean; + }, + ) { + this.proc = { + exited: this.#exited.promise, + exitCode: null, + stdin: { write: () => 0, flush: () => undefined }, + stdout: new ReadableStream(), + stderr: new ReadableStream(), + peekStderr: () => "", + kill: () => { + this.#alive = false; + this.#exited.resolve(); + return true; + }, + } as unknown as DapClientState["proc"]; + } + + async initialize(): Promise { + queueMicrotask(() => this.#emit("initialized", {})); + return { supportsConfigurationDoneRequest: true }; + } + + async sendRequest(command: string): Promise { + if (command === "launch" && this.options.launchError) { + throw new Error(this.options.launchError); + } + if (command === "attach" && this.options.attachError) { + throw new Error(this.options.attachError); + } + if (command === "configurationDone" && this.options.configurationDoneError) { + throw new Error(this.options.configurationDoneError); + } + return {}; + } + + waitForEvent(event: string): Promise { + if (this.options.rejectStopWaiters && (event === "stopped" || event === "terminated" || event === "exited")) { + return Promise.reject(new Error(`DAP event ${event} timed out after 1ms`)); + } + const { promise, resolve } = Promise.withResolvers(); + const unsubscribe = this.onEvent(event, body => { + unsubscribe(); + resolve(body); + }); + return promise; + } + + onEvent(event: string, handler: DapEventHandler): () => void { + let handlers = this.#handlers.get(event); + if (!handlers) { + handlers = new Set(); + this.#handlers.set(event, handlers); + } + handlers.add(handler); + return () => handlers?.delete(handler); + } + + onReverseRequest(): () => void { + return () => {}; + } + + isAlive(): boolean { + return this.#alive; + } + + async dispose(): Promise { + this.#alive = false; + this.#exited.resolve(); + } + + #emit(event: string, body: unknown): void { + const message: DapEventMessage = { seq: 1, type: "event", event, body }; + for (const handler of this.#handlers.get(event) ?? []) { + void handler(body, message); + } + } +} + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("DAP launch failure handling", () => { + it("surfaces the launch failure when configurationDone also fails", async () => { + const manager = new DapSessionManager(); + const fake = new FakeDapClient(TEST_ADAPTER, process.cwd(), { + launchError: "launch: 'C:\\repo\\python' is not a valid executable", + configurationDoneError: "configurationDone: Expected process to be stopped.", + }); + spyOn(DapClient, "spawn").mockResolvedValue(fake as unknown as DapClient); + + let message = ""; + try { + await manager.launch({ adapter: TEST_ADAPTER, program: "C:\\repo\\python", cwd: process.cwd() }); + } catch (error) { + expect(error).toBeInstanceOf(Error); + message = (error as Error).message; + } + + expect(message).toContain("launch: 'C:\\repo\\python' is not a valid executable"); + expect(message).toContain("configurationDone: Expected process to be stopped."); + }); + + it("surfaces the attach failure when configurationDone also fails", async () => { + const manager = new DapSessionManager(); + const fake = new FakeDapClient(TEST_ADAPTER, process.cwd(), { + attachError: "attach: target process exited", + configurationDoneError: "configurationDone: Expected process to be stopped.", + }); + spyOn(DapClient, "spawn").mockResolvedValue(fake as unknown as DapClient); + + let message = ""; + try { + await manager.attach({ adapter: TEST_ADAPTER, cwd: process.cwd(), pid: 123 }); + } catch (error) { + expect(error).toBeInstanceOf(Error); + message = (error as Error).message; + } + + expect(message).toContain("attach: target process exited"); + expect(message).toContain("configurationDone: Expected process to be stopped."); + }); + + it("does not emit an unhandled rejection when launch fails before initial stop watchers settle", async () => { + const manager = new DapSessionManager(); + const fake = new FakeDapClient(TEST_ADAPTER, process.cwd(), { + launchError: "launch: failed before stop outcome", + rejectStopWaiters: true, + }); + const unhandled: unknown[] = []; + const onUnhandled = (reason: unknown) => unhandled.push(reason); + process.on("unhandledRejection", onUnhandled); + spyOn(DapClient, "spawn").mockResolvedValue(fake as unknown as DapClient); + + try { + await expect( + manager.launch({ adapter: TEST_ADAPTER, program: "/bin/echo", cwd: process.cwd() }), + ).rejects.toThrow("launch: failed before stop outcome"); + await Bun.sleep(10); + expect(unhandled).toEqual([]); + } finally { + process.off("unhandledRejection", onUnhandled); + } + }); +}); + +describe("DebugTool launch validation", () => { + it("rejects directory-valued launch programs before adapter selection", async () => { + const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "omp-debug-program-")); + try { + await fs.mkdir(path.join(cwd, "python")); + const session: ToolSession = { + cwd, + hasUI: false, + getSessionFile: () => null, + getSessionSpawns: () => "*", + settings: Settings.isolated({ "debug.enabled": true }), + }; + const tool = new DebugTool(session); + + await expect(tool.execute("call", { action: "launch", program: "python" })).rejects.toThrow( + /launch program resolves to a directory.*python/, + ); + } finally { + await fs.rm(cwd, { recursive: true, force: true }); + } + }); +});