diff --git a/CHANGELOG.md b/CHANGELOG.md index d0237a07..2aa046bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING (behavioral): launch sessions now default to `breakOnExceptions: "uncaught"`** — a crashing script pauses at the uncaught exception (`lastStop.reason: "exception"`, stack/locals inspectable, `exceptionInfo` where supported) instead of running to termination. Applies uniformly to Python, JavaScript, Java, Go (panics), .NET, Rust (panics), and the mock adapter; Ruby keeps the old run-to-termination behavior (rdbg has no uncaught-only filter). Pass `breakOnExceptions: "none"` to restore the old behavior per session. Attach sessions are unchanged — no default is ever applied on attach (fixes #244) ### Added +- **Logpoints** — `set_breakpoint` accepts an optional `logMessage`: instead of pausing, the adapter logs the message (expressions in `{curly braces}` interpolated with live values) as output that lands in `get_output` and the session output resource while the program runs at full speed — the prod-safe just-in-time diagnostics primitive, combinable with `condition`. A single shared `toSourceBreakpoint` mapper now feeds every setBreakpoints construction site (live re-send, launch-time initial breakpoints, js-debug handshake), which also fixes `suspendPolicy` being silently dropped on the launch path. Support is gated per adapter: Python/JavaScript/Go/Rust/mock work; Java/.NET fail fast with a clear error; unknown support (Ruby) is accepted with a warning and re-checked against the adapter's live capabilities at launch. The mock adapter simulates logpoints (logs without stopping) for hermetic e2e coverage (fixes #235) + - **Breakpoint management tools** — new `list_breakpoints` (per-session listing with verified state and adapter-assigned ids, optional file filter), `remove_breakpoint` (by breakpoint id, or by file+line which removes every breakpoint at that location), and `clear_breakpoints` (whole session or one file). Removal and clearing take effect immediately while the debuggee is running or paused (the file's remaining set is re-sent, DAP replace-all) and deliberately keep working after the program exits so breakpoints can be adjusted between launches. DAP `breakpoint` events — previously dropped — are now wired end-to-end, so deferred verifications (debugpy module load, JDI class load, js-debug async binding, netcoredbg pending breakpoints) update the stored state; pre-launch breakpoints are also re-synced once a launch completes so their verified state and adapter ids reach the store on adapters that don't push events (fixes #236) - **Break-on-exception support** — new `breakOnExceptions` option (`"uncaught"` | `"all"` | `"none"`, default `"none"`) on `start_debugging` and `attach_to_process`: an uncaught exception now pauses at the crash site with the stack and locals inspectable instead of terminating the session. The abstract mode is resolved to per-language debugger filter IDs by the adapter policy (Python `uncaught`/`raised`+`uncaught`, JavaScript `uncaught`/`all` — runtime-verified against js-debug, Java `uncaught`/`caught`+`uncaught`, .NET `user-unhandled`/`all`, Go `fatal`+`panic`, Rust `rust_panic`/`+cpp_throw`, Ruby `all`-only via `any`); an unsupported mode is skipped with a warning and never aborts the launch (fixes #220) diff --git a/docs/javascript/README.md b/docs/javascript/README.md index d1d9101a..031c778a 100644 --- a/docs/javascript/README.md +++ b/docs/javascript/README.md @@ -146,6 +146,8 @@ const child = spawn('node', ['child.js']); ### Log Points +A `logMessage` turns the breakpoint into a logpoint: execution does not pause — the interpolated message (expressions in `{curly braces}`) arrives in the session output, readable via `get_output`. + ```json { "tool": "set_breakpoint", diff --git a/docs/jit-diagnostics/README.md b/docs/jit-diagnostics/README.md index d62fd207..492081fb 100644 --- a/docs/jit-diagnostics/README.md +++ b/docs/jit-diagnostics/README.md @@ -85,7 +85,7 @@ Total pause time: a few hundred milliseconds around one request. No redeploy, no - **Never expose a debug port through a Service, Ingress, or LoadBalancer.** debugpy/rdbg listeners are unauthenticated and allow code execution. `kubectl port-forward` keeps the connection inside your kubeconfig's auth. - Prefer **on-demand listeners** over always-on ones: add the debug flag to a single quarantined pod when diagnosing (e.g. remove the pod from the Service selector, then `kubectl debug`/patch it), rather than baking it into the deployment as this tutorial image does for convenience. -- Target **staging, canaries, or quarantined sick pods** — pausing a pod that's in a live serving rotation stops its traffic for the duration of the pause. Logpoint-style non-breaking inspection is tracked in [#235](https://github.com/debugmcp/mcp-debugger/issues/235). +- Target **staging, canaries, or quarantined sick pods** — pausing a pod that's in a live serving rotation stops its traffic for the duration of the pause. For non-breaking inspection, pass `logMessage` to `set_breakpoint` (a logpoint, [#235](https://github.com/debugmcp/mcp-debugger/issues/235)): the pod keeps serving at full speed while interpolated values stream into `get_output`. - The same flow works for **Ruby** (`rdbg --open --port`) — see [docs/ruby/README.md](../ruby/README.md) — and **Java** (JDWP agent), covering three of the most common backend runtimes. ## Cleanup diff --git a/docs/tool-reference.md b/docs/tool-reference.md index cbabfb3b..3c50bc48 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -135,7 +135,8 @@ Sets a breakpoint in a source file. - `sessionId` (string, required): The ID of the debug session. - `file` (string, required): Path to the source file (absolute or relative to project root). - `line` (number, required): Line number where to set breakpoint (1-indexed). -- `condition` (string, optional): Conditional expression for the breakpoint *(not verified to work)*. +- `condition` (string, optional): Conditional expression — only break (or log) when it evaluates truthy. +- `logMessage` (string, optional): Create a **logpoint** instead of a pausing breakpoint — see [Logpoints](#logpoints) below. - `suspendPolicy` (string, optional): Suspend policy when the breakpoint is hit — `"all"` suspends all threads (default), `"thread"` suspends only the event thread. Only supported by the Java/JDI adapter. **Response:** @@ -166,6 +167,20 @@ Sets a breakpoint in a source file. - Setting breakpoints on non-executable lines (comments, blank lines, declarations) may cause unexpected behavior - Executable lines that work well: assignments, function calls, conditionals, returns +#### Logpoints + +Passing `logMessage` turns the breakpoint into a DAP logpoint: when the line is hit the program does **not** pause — the message is logged and execution continues at full speed. Expressions in `{curly braces}` are interpolated with live values (e.g. `"order={orderId} total={total}"`), and the messages arrive in the session output, readable via [`get_output`](#get_output) and the `debug://sessions/{id}/output` resource. `condition` may be combined with `logMessage` — the message is only logged when the condition holds. + +This is the prod-safe just-in-time diagnostics primitive: attach to a live process, plant logpoints at suspect lines, read interpolated values from `get_output` — no pauses, no pre-instrumented logging. + +Support is adapter-dependent: + +| Adapters | Behavior | +|---|---| +| Python, JavaScript/TypeScript, Go, Rust, mock | Supported — logs without pausing | +| Java, .NET | Not supported — `set_breakpoint` with `logMessage` fails fast with a clear error | +| Ruby | Unknown — accepted with a warning; validated against the adapter's capabilities at launch | + --- ### list_breakpoints diff --git a/packages/adapter-mock/src/mock-adapter-process.ts b/packages/adapter-mock/src/mock-adapter-process.ts index 09b7f0da..72ae5a95 100644 --- a/packages/adapter-mock/src/mock-adapter-process.ts +++ b/packages/adapter-mock/src/mock-adapter-process.ts @@ -97,7 +97,9 @@ function createConnection(input?: Readable, output?: Writable): DAPConnection { class MockDebugAdapterProcess { private connection?: DAPConnection; private server?: net.Server; - private breakpoints = new Map(); + // Stored breakpoints carry logMessage so the run simulation can treat + // logpoints as non-stopping (issue #235) + private breakpoints = new Map>(); private variableHandles = new Map }>(); private nextVariableReference = 1000; private currentLine = 1; @@ -305,7 +307,7 @@ class MockDebugAdapterProcess { supportSuspendDebuggee: false, supportsDelayedStackTraceLoading: false, supportsLoadedSourcesRequest: false, - supportsLogPoints: false, + supportsLogPoints: true, supportsTerminateThreadsRequest: false, supportsSetExpression: false, supportsTerminateRequest: true, @@ -406,6 +408,35 @@ class MockDebugAdapterProcess { } as DebugProtocol.StoppedEvent); } + /** + * Walk the sorted breakpoint list from a line: logpoint lines emit an + * output event (naive {expr} interpolation) and do NOT stop; the first + * plain breakpoint after the line is the next stop (issue #235). + */ + private findNextStop(fromLine: number): (DebugProtocol.Breakpoint & { logMessage?: string }) | undefined { + const allBreakpoints = Array.from(this.breakpoints.values()) + .flat() + .filter(bp => bp.line !== undefined) + .sort((a, b) => (a.line || 0) - (b.line || 0)); + + for (const bp of allBreakpoints) { + if ((bp.line || 0) <= fromLine) continue; + if (bp.logMessage !== undefined) { + const interpolated = bp.logMessage.replace(/\{[^}]*\}/g, '42'); + this.log(`Logpoint at line ${bp.line}: ${interpolated}`); + this.sendEvent({ + seq: 0, + type: 'event', + event: 'output', + body: { category: 'console', output: `${interpolated}\n` } + } as DebugProtocol.OutputEvent); + continue; + } + return bp; + } + return undefined; + } + private handleLaunch(request: DebugProtocol.LaunchRequest): void { const args = request.arguments as DebugProtocol.LaunchRequestArguments & { stopOnEntry?: boolean; program?: string }; this.log(`Launching with args: ${JSON.stringify(args)}`); @@ -437,16 +468,13 @@ class MockDebugAdapterProcess { }, 100); } else { this.log(`Running without stopOnEntry, will hit first breakpoint`); - // Simulate running to first breakpoint + // Simulate running to the first stopping breakpoint (logpoints log + // without stopping — see findNextStop) setTimeout(() => { - const allBreakpoints = Array.from(this.breakpoints.entries()) - .flatMap(([filePath, bps]) => bps.map(bp => ({ filePath, ...bp }))) - .filter(bp => bp.line !== undefined) - .sort((a, b) => (a.line || 0) - (b.line || 0)); + const firstStop = this.findNextStop(0); - if (allBreakpoints.length > 0) { - const firstBreakpoint = allBreakpoints[0]; - this.currentLine = firstBreakpoint.line || 1; + if (firstStop) { + this.currentLine = firstStop.line || 1; this.log(`Hit first breakpoint at line ${this.currentLine}`); this.sendEvent({ seq: 0, @@ -493,11 +521,13 @@ class MockDebugAdapterProcess { id: Math.floor(Math.random() * 100000), verified: true, line: bp.line, - source: args.source - }); + source: args.source, + // Retained for the run simulation: logpoint lines log instead of stopping + ...(bp.logMessage !== undefined ? { logMessage: bp.logMessage } : {}) + } as DebugProtocol.Breakpoint & { logMessage?: string }); } } - + this.breakpoints.set(args.source?.path || 'unknown', breakpoints); this.sendResponse({ @@ -650,20 +680,13 @@ class MockDebugAdapterProcess { } }); - // Simulate hitting a breakpoint or terminating + // Simulate hitting a breakpoint or terminating (logpoints log without + // stopping — see findNextStop) setTimeout(() => { - const allBreakpoints = Array.from(this.breakpoints.entries()) - .flatMap(([filePath, bps]) => bps.map(bp => ({ filePath, ...bp }))) - .filter(bp => bp.line !== undefined) - .sort((a, b) => (a.line || 0) - (b.line || 0)); + const nextBreakpoint = this.findNextStop(this.currentLine); - this.log(`Continue from line ${this.currentLine}. All breakpoints: ${allBreakpoints.map(bp => bp.line).join(', ')}`); + this.log(`Next stopping breakpoint after line ${this.currentLine}: ${nextBreakpoint ? nextBreakpoint.line : 'none'}`); - // Find next breakpoint after current line - const nextBreakpoint = allBreakpoints.find(bp => (bp.line || 0) > this.currentLine); - - this.log(`Next breakpoint after line ${this.currentLine}: ${nextBreakpoint ? nextBreakpoint.line : 'none'}`); - if (nextBreakpoint && nextBreakpoint.line) { // Hit the next breakpoint this.currentLine = nextBreakpoint.line; diff --git a/packages/adapter-mock/src/mock-debug-adapter.ts b/packages/adapter-mock/src/mock-debug-adapter.ts index 7abf4f16..f1209b86 100644 --- a/packages/adapter-mock/src/mock-debug-adapter.ts +++ b/packages/adapter-mock/src/mock-debug-adapter.ts @@ -131,7 +131,9 @@ export class MockDebugAdapter extends EventEmitter implements IDebugAdapter { DebugFeature.CONDITIONAL_BREAKPOINTS, DebugFeature.FUNCTION_BREAKPOINTS, DebugFeature.VARIABLE_PAGING, - DebugFeature.SET_VARIABLE + DebugFeature.SET_VARIABLE, + // Matches mock-adapter-process's initialize response (issue #235) + DebugFeature.LOG_POINTS ], }; } diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 415fe465..faf2144f 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -215,3 +215,4 @@ export { sanitizeStderrTail } from './utils/env-sanitizer.js'; export { LineBuffer } from './utils/line-buffer.js'; +export { toSourceBreakpoint, type BreakpointFields } from './utils/to-source-breakpoint.js'; diff --git a/packages/shared/src/interfaces/adapter-policy-dotnet.ts b/packages/shared/src/interfaces/adapter-policy-dotnet.ts index 8f0f33ab..7a644683 100644 --- a/packages/shared/src/interfaces/adapter-policy-dotnet.ts +++ b/packages/shared/src/interfaces/adapter-policy-dotnet.ts @@ -1,4 +1,4 @@ -/** +/** * DotnetAdapterPolicy - DAP proxy policy for the .NET debug adapter (netcoredbg) * * This policy encodes all netcoredbg-specific behaviors that the DAP proxy worker @@ -13,7 +13,7 @@ * ## DAP sequence * * netcoredbg follows the standard DAP sequence: - * initialize → response → initialized event → attach/launch → configurationDone + * initialize → response → initialized event → attach/launch → configurationDone * * ## Adapter ID * @@ -41,6 +41,7 @@ import type { DapClientBehavior, DapClientContext, ReverseRequestResult } from ' export const DotnetAdapterPolicy: AdapterPolicy = { name: 'dotnet', + supportsLogPoints: false, supportsReverseStartDebugging: false, childSessionStrategy: 'none', buildChildStartArgs: () => { @@ -226,7 +227,7 @@ export const DotnetAdapterPolicy: AdapterPolicy = { getInitializationBehavior: () => { return { // netcoredbg sends the `initialized` event immediately after the - // `initialize` response — before any launch/attach request. + // `initialize` response — before any launch/attach request. // We must defer configurationDone handling and send launch first, // because netcoredbg requires launch before configurationDone. sendLaunchBeforeConfig: true, diff --git a/packages/shared/src/interfaces/adapter-policy-go.ts b/packages/shared/src/interfaces/adapter-policy-go.ts index 9a16e021..729c4a73 100644 --- a/packages/shared/src/interfaces/adapter-policy-go.ts +++ b/packages/shared/src/interfaces/adapter-policy-go.ts @@ -1,4 +1,4 @@ -/** +/** * GoAdapterPolicy - policy for Go Debug Adapter (Delve/dlv) * * Encodes Delve-specific behaviors and variable handling logic. @@ -12,6 +12,7 @@ import type { DapClientBehavior, DapClientContext, ReverseRequestResult } from ' export const GoAdapterPolicy: AdapterPolicy = { name: 'go', + supportsLogPoints: true, supportsReverseStartDebugging: false, childSessionStrategy: 'none', buildChildStartArgs: () => { @@ -234,7 +235,7 @@ export const GoAdapterPolicy: AdapterPolicy = { sendLaunchBeforeConfig: true, // Delve has no caught/uncaught distinction; both modes arm the same // filters. IDs are Delve's real ones ('unrecovered-panic', - // 'runtime-fatal-throw', dlv 1.26) — the shorthand 'panic'/'fatal' + // 'runtime-fatal-throw', dlv 1.26) — the shorthand 'panic'/'fatal' // shipped in #220 was silently accepted-and-ignored by Delve, caught // live by the #243 capability drift warning during #244 validation. exceptionFilters: { diff --git a/packages/shared/src/interfaces/adapter-policy-java.ts b/packages/shared/src/interfaces/adapter-policy-java.ts index f61fbff5..72dab2f6 100644 --- a/packages/shared/src/interfaces/adapter-policy-java.ts +++ b/packages/shared/src/interfaces/adapter-policy-java.ts @@ -1,4 +1,4 @@ -/** +/** * JavaAdapterPolicy - policy for Java Debug Adapter (JDI bridge / JdiDapServer) * * JdiDapServer speaks DAP over TCP natively using JDI. It uses a non-standard @@ -13,6 +13,7 @@ import type { DapClientBehavior, DapClientContext, ReverseRequestResult } from ' export const JavaAdapterPolicy: AdapterPolicy = { name: 'java', + supportsLogPoints: false, supportsReverseStartDebugging: false, childSessionStrategy: 'none', buildChildStartArgs: () => { diff --git a/packages/shared/src/interfaces/adapter-policy-js.ts b/packages/shared/src/interfaces/adapter-policy-js.ts index c111c847..fa36c37d 100644 --- a/packages/shared/src/interfaces/adapter-policy-js.ts +++ b/packages/shared/src/interfaces/adapter-policy-js.ts @@ -10,6 +10,7 @@ import type { AdapterPolicy, AdapterSpecificState, CommandHandling } from './ada import { resolveExceptionFilters } from './adapter-policy.js'; import { SessionState } from '@debugmcp/shared'; import type { StackFrame, Variable } from '../models/index.js'; +import { toSourceBreakpoint, type BreakpointFields } from '../utils/to-source-breakpoint.js'; import type { DapClientBehavior, DapClientContext, ReverseRequestResult } from './dap-client-behavior.js'; /** @@ -23,6 +24,7 @@ export interface JsAdapterState extends AdapterSpecificState { export const JsDebugAdapterPolicy: AdapterPolicy = { name: 'js-debug', + supportsLogPoints: true, supportsReverseStartDebugging: true, childSessionStrategy: 'launchWithPendingTarget', buildChildStartArgs: (pendingId: string, parentConfig: Record) => { @@ -279,13 +281,14 @@ export const JsDebugAdapterPolicy: AdapterPolicy = { } try { - // Group queued breakpoints by file - const grouped: Map> = new Map(); + // Group queued breakpoints by file, mapping via the shared + // toSourceBreakpoint so no per-breakpoint field is dropped (#235) + const grouped: Map = new Map(); for (const bp of breakpoints.values()) { // Type assertion for bp since it's 'unknown' in the interface - const breakpoint = bp as { file: string; line: number; condition?: string }; + const breakpoint = bp as { file: string } & BreakpointFields; const arr = grouped.get(breakpoint.file) || []; - arr.push({ line: breakpoint.line, condition: breakpoint.condition }); + arr.push(toSourceBreakpoint(breakpoint)); grouped.set(breakpoint.file, arr); } for (const [file, bps] of grouped) { diff --git a/packages/shared/src/interfaces/adapter-policy-mock.ts b/packages/shared/src/interfaces/adapter-policy-mock.ts index 97875b83..d192436c 100644 --- a/packages/shared/src/interfaces/adapter-policy-mock.ts +++ b/packages/shared/src/interfaces/adapter-policy-mock.ts @@ -1,4 +1,4 @@ -/** +/** * MockAdapterPolicy - policy for Mock Debug Adapter (testing) * * Encodes mock adapter behaviors for testing purposes. @@ -10,6 +10,7 @@ import type { DapClientBehavior } from './dap-client-behavior.js'; export const MockAdapterPolicy: AdapterPolicy = { name: 'mock', + supportsLogPoints: true, supportsReverseStartDebugging: false, childSessionStrategy: 'none', buildChildStartArgs: () => { diff --git a/packages/shared/src/interfaces/adapter-policy-python.ts b/packages/shared/src/interfaces/adapter-policy-python.ts index a1d11473..5730f55e 100644 --- a/packages/shared/src/interfaces/adapter-policy-python.ts +++ b/packages/shared/src/interfaces/adapter-policy-python.ts @@ -1,4 +1,4 @@ -/** +/** * PythonAdapterPolicy - policy for Python Debug Adapter (debugpy) * * Encodes debugpy specific behaviors and variable handling logic. @@ -11,6 +11,7 @@ import type { DapClientBehavior, DapClientContext, ReverseRequestResult } from ' export const PythonAdapterPolicy: AdapterPolicy = { name: 'python', + supportsLogPoints: true, supportsReverseStartDebugging: false, childSessionStrategy: 'none', buildChildStartArgs: () => { @@ -294,7 +295,7 @@ export const PythonAdapterPolicy: AdapterPolicy = { // Attach: debugpy is already listening as a DAP server next to the target // (python -m debugpy --listen host:port), so there is no adapter process - // to spawn — connect directly. + // to spawn — connect directly. if (launchConfig.request === 'attach') { const connect = launchConfig.connect as { host?: string; port?: number } | undefined; const host = connect?.host diff --git a/packages/shared/src/interfaces/adapter-policy-rust.ts b/packages/shared/src/interfaces/adapter-policy-rust.ts index 607ea2bf..45d9cd28 100644 --- a/packages/shared/src/interfaces/adapter-policy-rust.ts +++ b/packages/shared/src/interfaces/adapter-policy-rust.ts @@ -1,4 +1,4 @@ -/** +/** * RustAdapterPolicy - policy for Rust Debug Adapter (CodeLLDB) * * Encodes CodeLLDB specific behaviors and variable handling logic. @@ -12,6 +12,7 @@ import type { DapClientBehavior, DapClientContext, ReverseRequestResult } from ' export const RustAdapterPolicy: AdapterPolicy = { name: 'rust', + supportsLogPoints: true, supportsReverseStartDebugging: false, childSessionStrategy: 'none', buildChildStartArgs: () => { diff --git a/packages/shared/src/interfaces/adapter-policy.ts b/packages/shared/src/interfaces/adapter-policy.ts index 2f5c10be..99784432 100644 --- a/packages/shared/src/interfaces/adapter-policy.ts +++ b/packages/shared/src/interfaces/adapter-policy.ts @@ -53,6 +53,15 @@ export interface AdapterPolicy { */ supportsReverseStartDebugging: boolean; + /** + * Static pre-launch knowledge of DAP logpoint (SourceBreakpoint.logMessage) + * support (issue #235). `true`/`false` gate set_breakpoint synchronously + * before live capabilities exist; `undefined` means unknown — the request + * is accepted with a warning and re-checked against the adapter's real + * capabilities at launch. + */ + supportsLogPoints?: boolean; + /** * Strategy for how to create/attach to the child session when reverse startDebugging occurs */ diff --git a/packages/shared/src/models/index.ts b/packages/shared/src/models/index.ts index 451c3209..cf5af957 100644 --- a/packages/shared/src/models/index.ts +++ b/packages/shared/src/models/index.ts @@ -215,6 +215,8 @@ export interface Breakpoint { line: number; /** Conditional expression (if any) */ condition?: string; + /** Logpoint: log this interpolated message instead of pausing (DAP logMessage) */ + logMessage?: string; /** Suspend policy: 'all' suspends all threads (default), 'thread' only suspends the event thread */ suspendPolicy?: 'all' | 'thread'; /** Whether the breakpoint is verified */ diff --git a/packages/shared/src/utils/to-source-breakpoint.ts b/packages/shared/src/utils/to-source-breakpoint.ts new file mode 100644 index 00000000..ec6f4ea9 --- /dev/null +++ b/packages/shared/src/utils/to-source-breakpoint.ts @@ -0,0 +1,28 @@ +/** + * The single mapper from stored breakpoint fields to a DAP SourceBreakpoint. + * + * There are four places that build setBreakpoints arrays (SessionManager live + * re-send, proxy worker initial breakpoints, connection-manager helper, and + * the js-debug handshake). Before this mapper existed each mapped its own + * subset of fields, so optional fields were silently dropped on some paths + * (suspendPolicy never survived a launch, for example). All four now share + * this function — add new per-breakpoint fields HERE, nowhere else (#235). + */ +import { DebugProtocol } from '@vscode/debugprotocol'; + +export interface BreakpointFields { + line: number; + condition?: string; + logMessage?: string; + /** Java/JDI-only suspend policy, passed through as a non-standard field */ + suspendPolicy?: 'all' | 'thread'; +} + +export function toSourceBreakpoint(bp: BreakpointFields): DebugProtocol.SourceBreakpoint { + return { + line: bp.line, + ...(bp.condition !== undefined ? { condition: bp.condition } : {}), + ...(bp.logMessage !== undefined ? { logMessage: bp.logMessage } : {}), + ...(bp.suspendPolicy !== undefined ? { suspendPolicy: bp.suspendPolicy } : {}), + }; +} diff --git a/packages/shared/tests/unit/to-source-breakpoint.test.ts b/packages/shared/tests/unit/to-source-breakpoint.test.ts new file mode 100644 index 00000000..050d663a --- /dev/null +++ b/packages/shared/tests/unit/to-source-breakpoint.test.ts @@ -0,0 +1,39 @@ +/** + * toSourceBreakpoint — the single mapper from stored Breakpoint fields to a + * DAP SourceBreakpoint (issue #235). Every construction site (live re-send, + * worker initial breakpoints, connection-manager helper, js-debug handshake) + * must use it so no optional field is silently dropped on any path. + */ +import { describe, it, expect } from 'vitest'; +import { toSourceBreakpoint } from '../../src/index.js'; + +describe('toSourceBreakpoint', () => { + it('maps a plain line-only breakpoint', () => { + expect(toSourceBreakpoint({ line: 10 })).toEqual({ line: 10 }); + }); + + it('includes condition when set', () => { + expect(toSourceBreakpoint({ line: 5, condition: 'x > 1' })) + .toEqual({ line: 5, condition: 'x > 1' }); + }); + + it('includes logMessage when set (logpoint)', () => { + expect(toSourceBreakpoint({ line: 7, logMessage: 'x={x}' })) + .toEqual({ line: 7, logMessage: 'x={x}' }); + }); + + it('allows condition and logMessage together (DAP-defined combination)', () => { + expect(toSourceBreakpoint({ line: 7, condition: 'x > 1', logMessage: 'x={x}' })) + .toEqual({ line: 7, condition: 'x > 1', logMessage: 'x={x}' }); + }); + + it('passes suspendPolicy through as a non-standard field', () => { + expect(toSourceBreakpoint({ line: 3, suspendPolicy: 'thread' })) + .toEqual({ line: 3, suspendPolicy: 'thread' }); + }); + + it('omits keys for absent optional fields entirely', () => { + const result = toSourceBreakpoint({ line: 10 }); + expect(Object.keys(result)).toEqual(['line']); + }); +}); diff --git a/skills/debugging/SKILL.md b/skills/debugging/SKILL.md index e34508d9..269ceab6 100644 --- a/skills/debugging/SKILL.md +++ b/skills/debugging/SKILL.md @@ -46,9 +46,10 @@ Rules that prevent 90% of failed sessions: 1. State a hypothesis about where reality diverges from expectation *before* setting breakpoints. 2. Set at most two breakpoints: last-known-good and first-known-bad. Run, inspect, halve the interval. Bisection beats stepping line-by-line from the top. Move the window mid-session with `remove_breakpoint` / `clear_breakpoints`; `list_breakpoints` shows what is currently set (with verified state and adapter ids). -3. At each pause, record what you *learned* (variable values, actual control flow), not just where you are. -4. When the diverging line is found, inspect every input to that line before concluding — the bug is usually an operand, not the operator. -5. Fix, then re-run the same session recipe to confirm the observed state changed as predicted. +3. When pausing is too disruptive (hot loops, live or attached processes), use a **logpoint**: `set_breakpoint` with `logMessage: "x={x}"` streams interpolated values into `get_output` without stopping the program (Python/JS/Go/Rust; Java and .NET reject it with a clear error). +4. At each pause, record what you *learned* (variable values, actual control flow), not just where you are. +5. When the diverging line is found, inspect every input to that line before concluding — the bug is usually an operand, not the operator. +6. Fix, then re-run the same session recipe to confirm the observed state changed as predicted. ## Program output diff --git a/skills/debugging/references/dotnet.md b/skills/debugging/references/dotnet.md index f6b17f17..c94692b8 100644 --- a/skills/debugging/references/dotnet.md +++ b/skills/debugging/references/dotnet.md @@ -57,6 +57,7 @@ attach_to_process {"sessionId": "", "processId": 12345, "sourcePaths": [ - Compiler-generated noise is filtered from variables automatically: `<>c__DisplayClass*` closures, `CS$<>*` temporaries, `<>t__`/`<>s__` async state-machine fields, `$VB$*`. - Stack traces show user code only by default — frames without source and `System.*`/`Microsoft.*` frames are hidden. Pass `includeInternals: true` to `get_stack_trace` to see everything. - Supports .NET Core / .NET 5+ and .NET Framework 4.8 (CoreCLR and Desktop CLR). +- Logpoints are NOT supported by netcoredbg: `set_breakpoint` with `logMessage` fails fast with a clear error. ## Troubleshooting diff --git a/skills/debugging/references/go.md b/skills/debugging/references/go.md index 17519922..88a34fb8 100644 --- a/skills/debugging/references/go.md +++ b/skills/debugging/references/go.md @@ -52,6 +52,7 @@ Not supported. The Go adapter implements launch mode only — `attach_to_process - Exception breakpoints `panic` and `fatal` are enabled by default — panics stop the debugger without any setup (`get_stack_trace` reports `stopReason`). - Delve's variable rendering auto-dereferences pointers, shows slices with len/cap, and maps as key-value pairs. - Use absolute paths for `file` and `scriptPath`; breakpoints must be on executable statements. +- Logpoints work: `set_breakpoint` with `logMessage: "x={x}"` logs interpolated values to `get_output` without pausing (Delve). ## Troubleshooting diff --git a/skills/debugging/references/java.md b/skills/debugging/references/java.md index b0684a8f..3695dd47 100644 --- a/skills/debugging/references/java.md +++ b/skills/debugging/references/java.md @@ -59,6 +59,7 @@ continue_execution {"sessionId": ""} // required with suspend=y to let - Breakpoints must sit on executable lines (assignments, calls, conditionals) — not blank lines, comments, imports, or bare declarations. Conditional breakpoints (`condition`) and exception breakpoints are supported. Exception stops carry `lastStop.description` ("FQCN: message") and, a moment after the pause, best-effort `lastStop.exceptionInfo` (exceptionId, breakMode, message, adapter-side stack trace). - `set_breakpoint` accepts a Java-only `suspendPolicy`: `"all"` (default) suspends every thread; `"thread"` suspends only the hitting thread. - Debuggee stdout/stderr is forwarded — `get_output {"sessionId": "", "since": 0}` works for Java; poll with the returned `nextSince`. +- Logpoints are NOT supported by the JDI bridge: `set_breakpoint` with `logMessage` fails fast with a clear error. ## Troubleshooting diff --git a/skills/debugging/references/javascript.md b/skills/debugging/references/javascript.md index 8195bdac..8201d3e7 100644 --- a/skills/debugging/references/javascript.md +++ b/skills/debugging/references/javascript.md @@ -64,6 +64,7 @@ Shorthand: `create_debug_session { "language": "javascript", "host": "127.0.0.1" - **Child processes are not auto-attached.** `autoAttachChildProcesses` defaults to `false`; pass it as `true` in `dapLaunchArgs` to debug `spawn`-ed Node children. - **Output capture works** (`outputCapture: 'std'`): stdout/stderr appear as `get_output` entries; entries without a category default to `console`. - **Frame IDs are adapter-assigned.** Use the `id` field from `get_stack_trace` frames for `get_scopes` — not the array index, not 0. Locals scopes are named `Local`/`Block`, and values come back as strings. +- Logpoints work: `set_breakpoint` with `logMessage: "x={x}"` logs interpolated values to `get_output` without pausing. ## Troubleshooting diff --git a/skills/debugging/references/python.md b/skills/debugging/references/python.md index 3c86c101..7f945057 100644 --- a/skills/debugging/references/python.md +++ b/skills/debugging/references/python.md @@ -59,6 +59,7 @@ Shorthand: `create_debug_session { "language": "python", "host": "127.0.0.1", "p - **Set breakpoints on executable lines** (assignments, calls, returns). Comments, blank lines, and bare `def`/`class` lines misbehave. - **evaluate_expression** runs in debugpy's `variables` (watch-style) context by default. Reads and arithmetic are reliable; whether mutations like `x = 5` take effect depends on debugpy — verify with a follow-up evaluate before relying on one. Collections are truncated at 300 items. - **Output capture works for Python** (`redirectOutput`): `print()` and stderr land in `get_output` entries, readable during and after the run until the session closes. +- Logpoints work: `set_breakpoint` with `logMessage: "x={x}"` logs interpolated values to `get_output` without pausing. ## Troubleshooting diff --git a/skills/debugging/references/rust.md b/skills/debugging/references/rust.md index 7cda76ae..42a0510f 100644 --- a/skills/debugging/references/rust.md +++ b/skills/debugging/references/rust.md @@ -61,6 +61,7 @@ Not supported. The Rust adapter implements launch mode only — `attach_to_proce - Debug builds only: release builds need `debug = true` in `[profile.release]` and still inline/optimize away variables. Prefer `opt-level = 0`. - GNU builds of crates that import Windows DLLs (`tokio`, `windows-sys`, `parking_lot_core`, ...) need full MinGW binutils — rustup's self-contained toolchain lacks `as.exe`, so `dlltool` fails. Install via MSYS2 (`mingw-w64-x86_64-binutils`, `-gcc`) and prepend `C:\msys64\mingw64\bin` to PATH. - Macro-generated and generic code can behave oddly: step targets may land in expansions, and generic fns need a concrete instantiation for breakpoints. For async (tokio), set breakpoints inside async blocks, not on the `async fn` line. +- Logpoints work: `set_breakpoint` with `logMessage: "x={x}"` logs interpolated values to `get_output` without pausing (CodeLLDB). ## Troubleshooting diff --git a/src/errors/debug-errors.ts b/src/errors/debug-errors.ts index 44f48324..59245f84 100644 --- a/src/errors/debug-errors.ts +++ b/src/errors/debug-errors.ts @@ -90,6 +90,25 @@ export class UnsupportedLanguageError extends McpError { } } +/** + * A debug feature was requested that the session's adapter does not support + * (e.g. a logpoint on an adapter without SourceBreakpoint.logMessage support). + */ +export class UnsupportedFeatureError extends McpError { + public readonly feature: string; + public readonly language: string; + + constructor(feature: string, language: string, detail?: string) { + super( + McpErrorCode.InvalidParams, + `${feature} not supported by the ${language} adapter${detail ? `: ${detail}` : ''}`, + { feature, language } + ); + this.feature = feature; + this.language = language; + } +} + /** * Proxy not running error */ diff --git a/src/proxy/dap-proxy-connection-manager.ts b/src/proxy/dap-proxy-connection-manager.ts index 7cb4d9ee..7c7cbab5 100644 --- a/src/proxy/dap-proxy-connection-manager.ts +++ b/src/proxy/dap-proxy-connection-manager.ts @@ -11,7 +11,7 @@ import { ExtendedInitializeArgs } from './dap-proxy-interfaces.js'; import type { AdapterPolicy } from '@debugmcp/shared'; -import { sanitizePayloadForLogging } from '@debugmcp/shared'; +import { sanitizePayloadForLogging, toSourceBreakpoint, type BreakpointFields } from '@debugmcp/shared'; export class DapConnectionManager { // Increased initial delay to give debugpy more time to start @@ -273,12 +273,9 @@ export class DapConnectionManager { async setBreakpoints( client: IDapClient, sourcePath: string, - breakpoints: { line: number; condition?: string }[] + breakpoints: BreakpointFields[] ): Promise { - const sourceBreakpoints: DebugProtocol.SourceBreakpoint[] = breakpoints.map(bp => ({ - line: bp.line, - condition: bp.condition - })); + const sourceBreakpoints: DebugProtocol.SourceBreakpoint[] = breakpoints.map(toSourceBreakpoint); const setBreakpointsArgs: DebugProtocol.SetBreakpointsArguments = { source: { path: sourcePath, name: path.basename(sourcePath) }, diff --git a/src/proxy/dap-proxy-interfaces.ts b/src/proxy/dap-proxy-interfaces.ts index aec8bf62..51b5b2a5 100644 --- a/src/proxy/dap-proxy-interfaces.ts +++ b/src/proxy/dap-proxy-interfaces.ts @@ -24,7 +24,7 @@ export interface ProxyInitPayload { scriptArgs?: string[]; stopOnEntry?: boolean; justMyCode?: boolean; - initialBreakpoints?: { file: string; line: number; condition?: string }[]; + initialBreakpoints?: { file: string; line: number; condition?: string; logMessage?: string; suspendPolicy?: 'all' | 'thread' }[]; dryRunSpawn?: boolean; /** Abstract break-on-exception mode; resolved to concrete DAP filters via the adapter policy (issue #220) */ breakOnExceptions?: 'uncaught' | 'all' | 'none'; diff --git a/src/proxy/dap-proxy-message-parser.ts b/src/proxy/dap-proxy-message-parser.ts index 5af81a25..48967945 100644 --- a/src/proxy/dap-proxy-message-parser.ts +++ b/src/proxy/dap-proxy-message-parser.ts @@ -138,6 +138,12 @@ export class MessageParser { if (bpObj.condition !== undefined && typeof bpObj.condition !== 'string') { throw new Error(`Breakpoint 'condition' must be a string if provided`); } + if (bpObj.logMessage !== undefined && typeof bpObj.logMessage !== 'string') { + throw new Error(`Breakpoint 'logMessage' must be a string if provided`); + } + if (bpObj.suspendPolicy !== undefined && bpObj.suspendPolicy !== 'all' && bpObj.suspendPolicy !== 'thread') { + throw new Error(`Breakpoint 'suspendPolicy' must be 'all' or 'thread' if provided`); + } } } diff --git a/src/proxy/dap-proxy-worker.ts b/src/proxy/dap-proxy-worker.ts index 9c7268c2..1673954d 100644 --- a/src/proxy/dap-proxy-worker.ts +++ b/src/proxy/dap-proxy-worker.ts @@ -27,7 +27,7 @@ import { } from '../utils/type-guards.js'; import { SilentDapCommandPayload } from './dap-extensions.js'; // Import adapter policies from shared package -import type { AdapterPolicy, AdapterSpecificState } from '@debugmcp/shared'; +import type { AdapterPolicy, AdapterSpecificState, BreakpointFields } from '@debugmcp/shared'; import { DefaultAdapterPolicy, JsDebugAdapterPolicy, @@ -746,16 +746,20 @@ export class DapProxyWorker { // Set initial breakpoints if provided if (this.currentInitPayload.initialBreakpoints?.length) { this.logger!.info('[Worker] Initial breakpoints payload:', this.currentInitPayload.initialBreakpoints); - const groupedBreakpoints = new Map(); + const groupedBreakpoints = new Map(); for (const breakpoint of this.currentInitPayload.initialBreakpoints) { const filePath = path.resolve(breakpoint.file); if (!groupedBreakpoints.has(filePath)) { groupedBreakpoints.set(filePath, []); } + // Full per-breakpoint fields — the connection manager maps them via + // the shared toSourceBreakpoint, so nothing is dropped here (#235) groupedBreakpoints.get(filePath)!.push({ line: breakpoint.line, - condition: breakpoint.condition + condition: breakpoint.condition, + logMessage: breakpoint.logMessage, + suspendPolicy: breakpoint.suspendPolicy }); } diff --git a/src/proxy/proxy-config.ts b/src/proxy/proxy-config.ts index bc5bdf54..630e1316 100644 --- a/src/proxy/proxy-config.ts +++ b/src/proxy/proxy-config.ts @@ -17,7 +17,7 @@ export interface ProxyConfig { scriptArgs?: string[]; stopOnEntry?: boolean; justMyCode?: boolean; - initialBreakpoints?: Array<{ file: string; line: number; condition?: string }>; + initialBreakpoints?: Array<{ file: string; line: number; condition?: string; logMessage?: string; suspendPolicy?: 'all' | 'thread' }>; dryRunSpawn?: boolean; breakOnExceptions?: ExceptionBreakMode; launchConfig?: LanguageSpecificLaunchConfig; diff --git a/src/server.ts b/src/server.ts index 73350418..31bff3c0 100644 --- a/src/server.ts +++ b/src/server.ts @@ -21,6 +21,7 @@ import { SessionNotFoundError, SessionTerminatedError, UnsupportedLanguageError, + UnsupportedFeatureError, ProxyNotRunningError } from './errors/debug-errors.js'; import { SessionManager, SessionManagerConfig } from './session/session-manager.js'; @@ -86,6 +87,7 @@ interface ToolArguments { file?: string; line?: number; condition?: string; + logMessage?: string; breakpointId?: string; scriptPath?: string; args?: string[]; @@ -307,6 +309,37 @@ export class DebugMcpServer { } } + /** + * Hybrid logpoint gating (issue #235): a known-unsupported adapter — the + * live DAP capabilities (post-launch) or the static policy table says + * supportsLogPoints is false — is a hard error; known-supported passes; + * unknown support passes with a warning and is re-checked against the + * adapter's real capabilities at launch (drift warning). + */ + private validateLogPointSupport(sessionId: string): { warning?: string } { + const session = this.sessionManager.getSession(sessionId); + const liveCaps = session?.adapterCapabilities; + const policy = this.sessionManager.getSessionPolicy(sessionId); + const language = session?.language ?? policy.name; + + if (liveCaps) { + if (liveCaps.supportsLogPoints === true) { + return {}; + } + throw new UnsupportedFeatureError('Logpoints (logMessage)', String(language), + 'the adapter did not advertise supportsLogPoints'); + } + if (policy.supportsLogPoints === false) { + throw new UnsupportedFeatureError('Logpoints (logMessage)', String(language)); + } + if (policy.supportsLogPoints === true) { + return {}; + } + return { + warning: `Logpoint support for ${String(language)} is unknown; it will be validated against the adapter's capabilities at launch` + }; + } + /** * Shared catch for the breakpoint management tools: session-lifecycle * failures become {success: false} results (same contract as @@ -444,11 +477,11 @@ export class DebugMcpServer { return fileCheck.effectivePath; } - public async setBreakpoint(sessionId: string, file: string, line: number, condition?: string, suspendPolicy?: 'all' | 'thread'): Promise { + public async setBreakpoint(sessionId: string, file: string, line: number, condition?: string, suspendPolicy?: 'all' | 'thread', logMessage?: string): Promise { this.validateSession(sessionId); const effectiveFile = await this.resolveBreakpointFile(sessionId, file, { requireExists: true }); - return this.sessionManager.setBreakpoint(sessionId, effectiveFile, line, condition, suspendPolicy); + return this.sessionManager.setBreakpoint(sessionId, effectiveFile, line, condition, suspendPolicy, logMessage); } // The breakpoint management tools below deliberately skip validateSession's @@ -667,7 +700,7 @@ export class DebugMcpServer { { name: 'create_debug_session', description: 'Create a new debugging session. Provide host and port to attach to a running process; omit them for launch mode', inputSchema: { type: 'object', properties: { language: { type: 'string', enum: supportedLanguages, description: 'Programming language for debugging' }, name: { type: 'string', description: 'Optional session name' }, executablePath: {type: 'string', description: 'Path to language executable (optional, will auto-detect if not provided)'}, host: { type: 'string', description: 'Host to attach to for remote debugging (optional, triggers attach mode)' }, port: { type: 'number', description: 'Debug port to attach to for remote debugging (optional, triggers attach mode)' }, timeout: { type: 'number', description: 'Connection timeout in milliseconds for attach mode (default: 30000)' }, verifyTimeout: { type: 'number', description: 'Attach mode only: how long to wait (ms) for the debugger to report at least one thread after attaching before failing the attach (default: 5000, max: 600000)' } }, required: ['language'] } }, { name: 'list_supported_languages', description: 'List all supported debugging languages with metadata', inputSchema: { type: 'object', properties: {} } }, { name: 'list_debug_sessions', description: 'List all active debugging sessions. Paused sessions include lastStop with the reason for the most recent stop (e.g. "breakpoint" vs "exception")', inputSchema: { type: 'object', properties: {} } }, - { name: 'set_breakpoint', description: 'Set a breakpoint. Setting breakpoints on non-executable lines (structural, declarative) may lead to unexpected behavior', inputSchema: { type: 'object', properties: { sessionId: { type: 'string' }, file: { type: 'string', description: 'Path to the source file or Java FQCN. For Java, passing a fully-qualified class name (e.g. "com.example.MyClass" or "com.example.Outer$Inner") is preferred — it works reliably with all classloaders including custom classloaders. Alternatively, use absolute file paths.' }, line: { type: 'number', description: 'Line number where to set breakpoint. Executable statements (assignments, function calls, conditionals, returns) work best. Structural lines (function/class definitions), declarative lines (imports), or non-executable lines (comments, blank lines) may cause unexpected stepping behavior' }, condition: { type: 'string' }, suspendPolicy: { type: 'string', enum: ['all', 'thread'], description: 'Suspend policy when breakpoint is hit: "all" suspends all threads (default), "thread" only suspends the event thread. Only supported by the Java/JDI adapter.' } }, required: ['sessionId', 'file', 'line'] } }, + { name: 'set_breakpoint', description: 'Set a breakpoint. Setting breakpoints on non-executable lines (structural, declarative) may lead to unexpected behavior', inputSchema: { type: 'object', properties: { sessionId: { type: 'string' }, file: { type: 'string', description: 'Path to the source file or Java FQCN. For Java, passing a fully-qualified class name (e.g. "com.example.MyClass" or "com.example.Outer$Inner") is preferred — it works reliably with all classloaders including custom classloaders. Alternatively, use absolute file paths.' }, line: { type: 'number', description: 'Line number where to set breakpoint. Executable statements (assignments, function calls, conditionals, returns) work best. Structural lines (function/class definitions), declarative lines (imports), or non-executable lines (comments, blank lines) may cause unexpected stepping behavior' }, condition: { type: 'string', description: 'Optional expression: only break (or log) when it evaluates truthy' }, logMessage: { type: 'string', description: 'Create a logpoint: instead of pausing, log this message when the line is hit. Expressions in {curly braces} are interpolated (e.g. "order={orderId} total={total}"). Messages arrive in get_output while the program runs at full speed. Supported by Python, JavaScript, Go, and Rust adapters; not by Java or .NET' }, suspendPolicy: { type: 'string', enum: ['all', 'thread'], description: 'Suspend policy when breakpoint is hit: "all" suspends all threads (default), "thread" only suspends the event thread. Only supported by the Java/JDI adapter.' } }, required: ['sessionId', 'file', 'line'] } }, { name: 'list_breakpoints', description: 'List all breakpoints in a session with their verified state and adapter-assigned ids. Works before launch (queued, verified=false), while running or paused, and after the program exits', inputSchema: { type: 'object', properties: { sessionId: { type: 'string' }, file: { type: 'string', description: 'Optional: only list breakpoints in this file' } }, required: ['sessionId'] } }, { name: 'remove_breakpoint', description: 'Remove a breakpoint by breakpointId (returned by set_breakpoint / list_breakpoints), or by file + line (removes all breakpoints at that location). Takes effect immediately while the program is running or paused; also works after the program exits, before a relaunch', inputSchema: { type: 'object', properties: { sessionId: { type: 'string' }, breakpointId: { type: 'string', description: 'Breakpoint id from set_breakpoint or list_breakpoints. Takes precedence over file + line' }, file: { type: 'string', description: 'Alternative addressing: source file path (use together with line)' }, line: { type: 'number', description: 'Alternative addressing: line number (use together with file)' } }, required: ['sessionId'] } }, { name: 'clear_breakpoints', description: 'Remove all breakpoints in a session, or all breakpoints in one file. Clearing zero breakpoints is success. Takes effect immediately while the program is running or paused', inputSchema: { type: 'object', properties: { sessionId: { type: 'string' }, file: { type: 'string', description: 'Optional: only clear breakpoints in this file' } }, required: ['sessionId'] } }, @@ -853,8 +886,14 @@ export class DebugMcpServer { } try { - const breakpoint = await this.setBreakpoint(args.sessionId, args.file, args.line, args.condition, args.suspendPolicy); - + // Logpoint gating (issue #235): hard error for known-unsupported + // adapters; a warning when support is unknown pre-launch. + const logPointGate = args.logMessage !== undefined + ? this.validateLogPointSupport(args.sessionId) + : {}; + + const breakpoint = await this.setBreakpoint(args.sessionId, args.file, args.line, args.condition, args.suspendPolicy, args.logMessage); + // Log breakpoint event this.logger.info('debug:breakpoint', { event: 'set', @@ -891,15 +930,17 @@ export class DebugMcpServer { }); } - result = { content: [{ type: 'text', text: JSON.stringify({ - success: true, - breakpointId: breakpoint.id, - file: breakpoint.file, - line: breakpoint.line, - verified: breakpoint.verified, - message: breakpoint.message || `Breakpoint set at ${breakpoint.file}:${breakpoint.line}`, - // Only add warning if there's a message from debugpy (indicating a problem) - warning: breakpoint.message || undefined, + const warnings = [breakpoint.message, logPointGate.warning].filter(Boolean); + result = { content: [{ type: 'text', text: JSON.stringify({ + success: true, + breakpointId: breakpoint.id, + file: breakpoint.file, + line: breakpoint.line, + verified: breakpoint.verified, + logMessage: breakpoint.logMessage, + message: breakpoint.message || `${breakpoint.logMessage !== undefined ? 'Logpoint' : 'Breakpoint'} set at ${breakpoint.file}:${breakpoint.line}`, + // Warn on adapter validation messages and unknown logpoint support + warning: warnings.length > 0 ? warnings.join('; ') : undefined, // Include context if available context: context || undefined }) }] }; diff --git a/src/session/session-manager-core.ts b/src/session/session-manager-core.ts index 51ac9114..00feea0b 100644 --- a/src/session/session-manager-core.ts +++ b/src/session/session-manager-core.ts @@ -551,6 +551,20 @@ export abstract class SessionManagerCore extends EventEmitter { `[SessionManager ${sessionId}] Exception filter drift check skipped: ${err instanceof Error ? err.message : String(err)}` ); } + + // Logpoint drift check (issue #235): a logpoint was accepted pre-launch + // (policy support true or unknown), but the live adapter does not + // advertise supportsLogPoints — it will likely pause instead of log. + if (capabilities.supportsLogPoints !== true) { + for (const bp of session.breakpoints.values()) { + if (bp.logMessage !== undefined) { + this.logger.warn( + `[SessionManager ${sessionId}] Logpoint at ${bp.file}:${bp.line} but the adapter does not advertise supportsLogPoints — it may pause instead of logging` + ); + bp.message = 'Adapter does not advertise logpoint support — this may pause instead of logging'; + } + } + } }; proxyManager.on('adapter-capabilities', handleAdapterCapabilities); handlers.set('adapter-capabilities', handleAdapterCapabilities); diff --git a/src/session/session-manager-operations.ts b/src/session/session-manager-operations.ts index cff52ea6..70604889 100644 --- a/src/session/session-manager-operations.ts +++ b/src/session/session-manager-operations.ts @@ -8,6 +8,7 @@ import { SessionState, SessionLifecycleState, sanitizePayloadForLogging, + toSourceBreakpoint, type ExceptionBreakMode } from '@debugmcp/shared'; import { ManagedSession, ToolchainValidationState } from './session-store.js'; @@ -128,11 +129,15 @@ export abstract class SessionManagerOperations extends SessionManagerData { const adapterPort = await this.findFreePort(); const initialBreakpoints = Array.from(session.breakpoints.values()).map((bp) => { - // Breakpoint file path has been validated by server.ts before reaching here + // Breakpoint file path has been validated by server.ts before reaching here. + // Carry every per-breakpoint field (condition, logMessage, suspendPolicy) — + // dropping one here silently loses it for the whole launch (#235). return { file: bp.file, // Use the validated path line: bp.line, condition: bp.condition, + logMessage: bp.logMessage, + suspendPolicy: bp.suspendPolicy, }; }); @@ -843,7 +848,8 @@ export abstract class SessionManagerOperations extends SessionManagerData { file: string, line: number, condition?: string, - suspendPolicy?: 'all' | 'thread' + suspendPolicy?: 'all' | 'thread', + logMessage?: string ): Promise { const session = this._getSessionById(sessionId); @@ -859,7 +865,7 @@ export abstract class SessionManagerOperations extends SessionManagerData { `[SessionManager setBreakpoint] Using validated file path "${file}" for session ${sessionId}` ); - const newBreakpoint: Breakpoint = { id: bpId, file, line, condition, suspendPolicy, verified: false }; + const newBreakpoint: Breakpoint = { id: bpId, file, line, condition, suspendPolicy, logMessage, verified: false }; if (!session.breakpoints) session.breakpoints = new Map(); session.breakpoints.set(bpId, newBreakpoint); @@ -905,11 +911,7 @@ export abstract class SessionManagerOperations extends SessionManagerData { 'setBreakpoints', { source: { path: file }, - breakpoints: allBpsForFile.map(bp => ({ - line: bp.line, - condition: bp.condition, - ...(bp.suspendPolicy ? { suspendPolicy: bp.suspendPolicy } : {}), - })), + breakpoints: allBpsForFile.map(toSourceBreakpoint), } ); if ( diff --git a/src/skill-content.ts b/src/skill-content.ts index 5b41e79c..525c8146 100644 --- a/src/skill-content.ts +++ b/src/skill-content.ts @@ -17,6 +17,7 @@ Key rules: - If a variable entry has a variablesReference, call get_variables with it to expand children. - Breakpoints may report unverified until the module/class loads — that is normal. - list_breakpoints shows every breakpoint with its verified state; remove_breakpoint (by id or file+line) and clear_breakpoints take effect immediately, even mid-run — use them to move a bisection window without restarting. +- Logpoints: set_breakpoint with logMessage ("order={orderId}") logs the interpolated message to get_output WITHOUT pausing — the prod-safe way to watch values on a hot path (Python/JS/Go/Rust; not Java/.NET). - get_output returns buffered debuggee stdout/stderr with a cursor; pass nextCursor back to read only new output. - attach_to_process connects to running/remote targets (debugpy --listen, rdbg --open, JVM JDWP), including pods via port-forward. - Launch sessions pause at uncaught exceptions by default (breakOnExceptions "uncaught"; Ruby excepted — rdbg has no uncaught filter). Pass "none" to let crashing scripts run to termination; attach applies no default. @@ -42,6 +43,7 @@ Prefer the debugger over print-debugging whenever you would need more than one e ## Root-cause discipline - State a hypothesis before setting breakpoints. - Set two breakpoints: last-known-good and first-known-bad; run, inspect, halve the interval (bisection beats line-by-line stepping). Use remove_breakpoint / clear_breakpoints to move the window mid-session, and list_breakpoints to see what is set. +- When pausing is too disruptive (hot loops, live/attached processes), use a logpoint instead: set_breakpoint with logMessage "x={x}" streams interpolated values into get_output at full speed (Python/JS/Go/Rust). - At each pause record what you learned, not just where you are. - When you find the diverging line, inspect every operand before concluding. - After fixing, re-run the same recipe to confirm the state changed as predicted. diff --git a/tests/core/unit/server/server-control-tools.test.ts b/tests/core/unit/server/server-control-tools.test.ts index a0555210..d3c536f0 100644 --- a/tests/core/unit/server/server-control-tools.test.ts +++ b/tests/core/unit/server/server-control-tools.test.ts @@ -83,6 +83,7 @@ describe('Server Control Tools Tests', () => { expect.stringContaining('/path/to/test.py'), 10, undefined, + undefined, undefined ); @@ -126,6 +127,7 @@ describe('Server Control Tools Tests', () => { expect.stringContaining('/path/to/test.py'), 20, 'x > 10', + undefined, undefined ); }); @@ -163,7 +165,8 @@ describe('Server Control Tools Tests', () => { expect.stringContaining('/path/to/test.py'), 30, undefined, - 'thread' + 'thread', + undefined ); }); diff --git a/tests/core/unit/server/server-logpoint-gating.test.ts b/tests/core/unit/server/server-logpoint-gating.test.ts new file mode 100644 index 00000000..62c8a2ed --- /dev/null +++ b/tests/core/unit/server/server-logpoint-gating.test.ts @@ -0,0 +1,148 @@ +/** + * set_breakpoint logMessage (logpoint) gating tests (issue #235). + * + * Hybrid static-first gating: a known-unsupported adapter (policy + * supportsLogPoints === false, or live capabilities false) is a hard error; + * known-supported passes; unknown support (policy undefined, e.g. ruby) + * passes with a warning and is validated again at launch via the + * capability drift warning. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { Server } from '@modelcontextprotocol/sdk/server/index.js'; +import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; +import { DebugMcpServer } from '../../../../src/server.js'; +import { SessionManager } from '../../../../src/session/session-manager.js'; +import { createProductionDependencies } from '../../../../src/container/dependencies.js'; +import { + createMockDependencies, + createMockServer, + createMockSessionManager, + createMockStdioTransport, + getToolHandlers +} from './server-test-helpers.js'; + +vi.mock('@modelcontextprotocol/sdk/server/index.js'); +vi.mock('@modelcontextprotocol/sdk/server/stdio.js'); +vi.mock('../../../../src/session/session-manager.js'); +vi.mock('../../../../src/container/dependencies.js'); + +describe('set_breakpoint logMessage gating', () => { + let mockServer: any; + let mockSessionManager: any; + let mockDependencies: any; + let callToolHandler: any; + + beforeEach(() => { + mockDependencies = createMockDependencies(); + vi.mocked(createProductionDependencies).mockReturnValue(mockDependencies); + + mockServer = createMockServer(); + vi.mocked(Server).mockImplementation(function() { return mockServer as any; }); + + const mockStdioTransport = createMockStdioTransport(); + vi.mocked(StdioServerTransport).mockImplementation(function() { return mockStdioTransport as any; }); + + mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); + vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); + + new DebugMcpServer(); + callToolHandler = getToolHandlers(mockServer).callToolHandler; + + mockSessionManager.getSession.mockReturnValue({ + id: 'test-session', + sessionLifecycle: 'active' + }); + mockSessionManager.setBreakpoint.mockResolvedValue({ + id: 'bp-1', + file: '/path/to/test.py', + line: 10, + logMessage: 'x is {x}', + verified: false + }); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + function callSetBreakpoint(extra: Record = {}) { + return callToolHandler({ + method: 'tools/call', + params: { + name: 'set_breakpoint', + arguments: { + sessionId: 'test-session', + file: '/path/to/test.py', + line: 10, + logMessage: 'x is {x}', + ...extra + } + } + }); + } + + it('rejects logMessage when the adapter policy declares no logpoint support', async () => { + mockSessionManager.getSessionPolicy.mockReturnValue({ name: 'java', supportsLogPoints: false }); + + await expect(callSetBreakpoint()).rejects.toThrow(/[Ll]ogpoint/); + expect(mockSessionManager.setBreakpoint).not.toHaveBeenCalled(); + }); + + it('rejects logMessage when live adapter capabilities deny logpoints', async () => { + mockSessionManager.getSessionPolicy.mockReturnValue({ name: 'python', supportsLogPoints: true }); + mockSessionManager.getSession.mockReturnValue({ + id: 'test-session', + sessionLifecycle: 'active', + adapterCapabilities: { supportsLogPoints: false } + }); + + await expect(callSetBreakpoint()).rejects.toThrow(/[Ll]ogpoint/); + }); + + it('accepts and forwards logMessage when the policy supports logpoints', async () => { + mockSessionManager.getSessionPolicy.mockReturnValue({ name: 'python', supportsLogPoints: true }); + + const result = await callSetBreakpoint(); + + expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith( + 'test-session', + expect.stringContaining('/path/to/test.py'), + 10, + undefined, + undefined, + 'x is {x}' + ); + const content = JSON.parse(result.content[0].text); + expect(content.success).toBe(true); + expect(content.logMessage).toBe('x is {x}'); + }); + + it('accepts with a warning when logpoint support is unknown', async () => { + mockSessionManager.getSessionPolicy.mockReturnValue({ name: 'ruby' }); + + const result = await callSetBreakpoint(); + + expect(mockSessionManager.setBreakpoint).toHaveBeenCalled(); + const content = JSON.parse(result.content[0].text); + expect(content.success).toBe(true); + expect(content.warning).toMatch(/unknown|not advertise/i); + }); + + it('applies no gating when logMessage is absent', async () => { + mockSessionManager.getSessionPolicy.mockReturnValue({ name: 'java', supportsLogPoints: false }); + mockSessionManager.setBreakpoint.mockResolvedValue({ + id: 'bp-2', file: '/path/to/test.py', line: 10, verified: false + }); + + const result = await callToolHandler({ + method: 'tools/call', + params: { + name: 'set_breakpoint', + arguments: { sessionId: 'test-session', file: '/path/to/test.py', line: 10 } + } + }); + + const content = JSON.parse(result.content[0].text); + expect(content.success).toBe(true); + }); +}); diff --git a/tests/core/unit/server/server-redefine-and-attach.test.ts b/tests/core/unit/server/server-redefine-and-attach.test.ts index 87caf817..bc0fecd3 100644 --- a/tests/core/unit/server/server-redefine-and-attach.test.ts +++ b/tests/core/unit/server/server-redefine-and-attach.test.ts @@ -1,4 +1,4 @@ -/** +/** * Tests for redefine_classes tool and attach stopOnEntry behavior */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; @@ -79,7 +79,7 @@ describe('redefine_classes and attach stopOnEntry tests', () => { const response = JSON.parse(result.content[0].text); expect(response.success).toBe(true); expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith( - 'attach-session', '/app/app.rb', 18, undefined, undefined + 'attach-session', '/app/app.rb', 18, undefined, undefined, undefined ); }); }); diff --git a/tests/core/unit/session/session-manager-dap.test.ts b/tests/core/unit/session/session-manager-dap.test.ts index 65bee684..d4e71b5c 100644 --- a/tests/core/unit/session/session-manager-dap.test.ts +++ b/tests/core/unit/session/session-manager-dap.test.ts @@ -152,6 +152,74 @@ describe('SessionManager - DAP Operations', () => { }); }); + describe('logpoints (logMessage)', () => { + it('stores logMessage and sends it on the live setBreakpoints path', async () => { + const session = await sessionManager.createSession({ + language: DebugLanguage.MOCK, + executablePath: 'python' + }); + await sessionManager.startDebugging(session.id, 'test.py'); + await vi.runAllTimersAsync(); + dependencies.mockProxyManager.dapRequestCalls = []; + + const bp = await sessionManager.setBreakpoint( + session.id, 'test.py', 20, undefined, undefined, 'value is {x}' + ); + + expect(bp.logMessage).toBe('value is {x}'); + expect(dependencies.mockProxyManager.dapRequestCalls[0].args.breakpoints[0]).toMatchObject({ + line: 20, + logMessage: 'value is {x}' + }); + const [listed] = sessionManager.listBreakpoints(session.id); + expect(listed.logMessage).toBe('value is {x}'); + }); + + it('carries logMessage and suspendPolicy into the launch-time initialBreakpoints snapshot', async () => { + const session = await sessionManager.createSession({ + language: DebugLanguage.MOCK, + executablePath: 'python' + }); + + await sessionManager.setBreakpoint( + session.id, 'test.py', 20, 'x > 1', 'thread', 'value is {x}' + ); + await sessionManager.startDebugging(session.id, 'test.py'); + await vi.runAllTimersAsync(); + + const startConfig = dependencies.mockProxyManager.startCalls[0]; + expect(startConfig.initialBreakpoints).toEqual([ + expect.objectContaining({ + file: 'test.py', + line: 20, + condition: 'x > 1', + suspendPolicy: 'thread', + logMessage: 'value is {x}' + }) + ]); + }); + + it('allows condition and logMessage together', async () => { + const session = await sessionManager.createSession({ + language: DebugLanguage.MOCK, + executablePath: 'python' + }); + await sessionManager.startDebugging(session.id, 'test.py'); + await vi.runAllTimersAsync(); + dependencies.mockProxyManager.dapRequestCalls = []; + + await sessionManager.setBreakpoint( + session.id, 'test.py', 21, 'x > 5', undefined, 'big x: {x}' + ); + + expect(dependencies.mockProxyManager.dapRequestCalls[0].args.breakpoints[0]).toMatchObject({ + line: 21, + condition: 'x > 5', + logMessage: 'big x: {x}' + }); + }); + }); + describe('launch-time breakpoint verification sync', () => { // Breakpoints queued before start_debugging are sent by the proxy worker // as initialBreakpoints, but that path's setBreakpoints responses never @@ -982,6 +1050,31 @@ describe('SessionManager - DAP Operations', () => { expect(sessionManager.getSession(session.id)?.adapterCapabilities).toEqual(capsWithExceptionInfo); }); + it('annotates stored logpoints when live capabilities do not advertise logpoint support (issue #235)', async () => { + const session = await createPausedSession(); + const bp = await sessionManager.setBreakpoint( + session.id, 'test.py', 20, undefined, undefined, 'x is {x}' + ); + + dependencies.mockProxyManager.simulateEvent('adapter-capabilities', { supportsLogPoints: false }); + + const [stored] = sessionManager.listBreakpoints(session.id); + expect(stored.id).toBe(bp.id); + expect(stored.message).toMatch(/logpoint/i); + }); + + it('leaves logpoints unannotated when live capabilities advertise support', async () => { + const session = await createPausedSession(); + await sessionManager.setBreakpoint( + session.id, 'test.py', 20, undefined, undefined, 'x is {x}' + ); + + dependencies.mockProxyManager.simulateEvent('adapter-capabilities', { supportsLogPoints: true }); + + const [stored] = sessionManager.listBreakpoints(session.id); + expect(stored.message).toBeUndefined(); + }); + it('requests exceptionInfo on exception stops and merges the result into lastStop', async () => { const session = await createPausedSession(); dependencies.mockProxyManager.simulateEvent('adapter-capabilities', capsWithExceptionInfo); diff --git a/tests/e2e/mcp-server-logpoints.test.ts b/tests/e2e/mcp-server-logpoints.test.ts new file mode 100644 index 00000000..8dfc6fac --- /dev/null +++ b/tests/e2e/mcp-server-logpoints.test.ts @@ -0,0 +1,145 @@ +/** + * E2E: logpoints (set_breakpoint logMessage) — issue #235. + * + * Supported adapters (python, javascript, go, rust, mock): a logpoint on a + * hot line does NOT pause execution; the interpolated message arrives as + * output readable via get_output. + * + * Known-unsupported adapters (java, dotnet): set_breakpoint with logMessage + * fails fast with a clear error. + * + * Unknown support (ruby): accepted with a warning; runtime behavior is + * adapter-dependent and not asserted. + */ +import { describe, it, expect, beforeAll, afterAll, afterEach } from 'vitest'; +import path from 'path'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'; +import { parseSdkToolResult, callToolSafely } from './smoke-test-utils.js'; +import { ROOT, createLanguageMatrix, prepareLanguageMatrix } from './language-matrix-utils.js'; + +const LANGUAGES = createLanguageMatrix(); + +// Per-language expectation: how the logpoint request should behave. +const EXPECTATION: Record = { + python: 'logs', + javascript: 'logs', + go: 'logs', + rust: 'logs', + mock: 'logs', + java: 'error', + dotnet: 'error', + ruby: 'warning', +}; + +// Python's bpLine has a=1, b=2 in scope — assert real interpolation there. +const LOG_MESSAGES: Record = { + python: { message: 'LP-MARK a={a}', expectInOutput: 'LP-MARK a=1' }, + default: { message: 'LP-MARK plain', expectInOutput: 'LP-MARK plain' }, +}; + +describe('Logpoints e2e (set_breakpoint logMessage)', () => { + let mcpClient: Client | null = null; + let transport: StdioClientTransport | null = null; + let currentSessionId: string | null = null; + + beforeAll(async () => { + transport = new StdioClientTransport({ + command: process.execPath, + args: [path.join(ROOT, 'dist', 'index.js'), '--log-level', 'info'], + env: { ...process.env, NODE_ENV: 'test' }, + }); + mcpClient = new Client( + { name: 'logpoints-test-client', version: '1.0.0' }, + { capabilities: {} }, + ); + await mcpClient.connect(transport); + prepareLanguageMatrix(LANGUAGES); + }, 120_000); + + afterAll(async () => { + if (mcpClient) { + await mcpClient.close().catch(() => {}); + } + transport = null; + }); + + afterEach(async () => { + if (mcpClient && currentSessionId) { + await callToolSafely(mcpClient, 'close_debug_session', { sessionId: currentSessionId }); + currentSessionId = null; + } + }); + + async function waitForState(sessionId: string, states: string[], timeoutMs = 30_000): Promise { + const deadline = Date.now() + timeoutMs; + let lastState = 'unknown'; + while (Date.now() < deadline) { + const res = await callToolSafely(mcpClient!, 'list_debug_sessions', {}); + const sessions = (res as { sessions?: Array<{ id: string; state: string }> }).sessions ?? []; + const session = sessions.find(s => s.id === sessionId); + lastState = session?.state ?? 'gone'; + if (session === undefined || states.includes(lastState)) { + return lastState; + } + await new Promise(resolve => setTimeout(resolve, 250)); + } + throw new Error(`Timed out waiting for state in [${states.join(', ')}]; last state: ${lastState}`); + } + + for (const lang of LANGUAGES) { + const expectation = EXPECTATION[lang.language] ?? 'warning'; + const itFn = lang.available ? it : it.skip; + const title = `logpoint ${expectation === 'logs' ? 'logs without pausing' : expectation === 'error' ? 'is rejected with a clear error' : 'is accepted with a warning'} (${lang.language})${lang.available ? '' : ` — ${lang.skipReason}`}`; + + itFn(title, async () => { + const createRes = await mcpClient!.callTool({ + name: 'create_debug_session', + arguments: { language: lang.language, name: `logpoint-${lang.language}` }, + }); + currentSessionId = parseSdkToolResult(createRes).sessionId as string; + expect(currentSessionId).toBeTruthy(); + + const { message, expectInOutput } = LOG_MESSAGES[lang.language] ?? LOG_MESSAGES.default; + const bpRes = await callToolSafely(mcpClient!, 'set_breakpoint', { + sessionId: currentSessionId, + file: lang.script, + line: lang.bpLine, + logMessage: message, + }); + + if (expectation === 'error') { + expect(bpRes.success).toBe(false); + expect(JSON.stringify(bpRes)).toMatch(/[Ll]ogpoint/); + return; + } + + expect(bpRes.success).toBe(true); + expect((bpRes as { logMessage?: string }).logMessage).toBe(message); + if (expectation === 'warning') { + expect(String((bpRes as { warning?: string }).warning ?? '')).toMatch(/unknown/i); + return; // runtime behavior for unknown-support adapters is not asserted + } + + // Supported adapter: launch and let it run — the logpoint must not pause. + const startRes = await callToolSafely(mcpClient!, 'start_debugging', { + sessionId: currentSessionId, + scriptPath: lang.launchScript ?? lang.script, + dapLaunchArgs: { stopOnEntry: false, ...(lang.dapLaunchArgs ?? {}) }, + }); + expect(startRes.success).toBe(true); + + // If the logpoint wrongly paused execution, this times out at 'paused'. + const finalState = await waitForState(currentSessionId, ['stopped', 'terminated']); + expect(['stopped', 'terminated', 'gone']).toContain(finalState); + + // The interpolated message must be in the captured output. + const outRes = await callToolSafely(mcpClient!, 'get_output', { + sessionId: currentSessionId, + since: 0, + }); + const outputText = JSON.stringify((outRes as { entries?: unknown[] }).entries ?? []); + expect(outputText).toContain(expectInOutput); + }, 60_000); + } +}); diff --git a/tests/proxy/dap-proxy-worker.test.ts b/tests/proxy/dap-proxy-worker.test.ts index 0c7e407c..32c7051c 100644 --- a/tests/proxy/dap-proxy-worker.test.ts +++ b/tests/proxy/dap-proxy-worker.test.ts @@ -504,6 +504,41 @@ describe('DapProxyWorker', () => { } }); + it('forwards logMessage and suspendPolicy on initial breakpoints (issue #235)', async () => { + const connectionStub = { + setBreakpoints: vi.fn().mockResolvedValue({ body: { breakpoints: [] } }), + sendConfigurationDone: vi.fn().mockResolvedValue(undefined), + setupEventHandlers: vi.fn() + }; + + (worker as any).logger = mockLogger; + (worker as any).dapClient = mockDapClient; + (worker as any).connectionManager = connectionStub; + (worker as any).adapterPolicy = DefaultAdapterPolicy; + (worker as any).adapterState = DefaultAdapterPolicy.createInitialState(); + (worker as any).currentSessionId = 'lp-session'; + (worker as any).currentInitPayload = { + cmd: 'init', + sessionId: 'lp-session', + executablePath: 'python', + adapterHost: 'localhost', + adapterPort: 5678, + logDir: '/tmp/logs', + scriptPath: '/work/app.py', + initialBreakpoints: [ + { file: '/work/app.py', line: 5, logMessage: 'x is {x}', suspendPolicy: 'thread' } + ] + }; + + await (worker as any).handleInitializedEvent(); + + expect(connectionStub.setBreakpoints).toHaveBeenCalledTimes(1); + const [, , sentBreakpoints] = connectionStub.setBreakpoints.mock.calls[0]; + expect(sentBreakpoints).toEqual([ + expect.objectContaining({ line: 5, logMessage: 'x is {x}', suspendPolicy: 'thread' }) + ]); + }); + it('forwards DAP breakpoint events to the parent (issue #236)', () => { const connectionStub = { setupEventHandlers: vi.fn((client: EventEmitter, handlers: Record void>) => { diff --git a/tests/unit/proxy/dap-proxy-connection-manager.test.ts b/tests/unit/proxy/dap-proxy-connection-manager.test.ts index ea35e6c4..51c2ff25 100644 --- a/tests/unit/proxy/dap-proxy-connection-manager.test.ts +++ b/tests/unit/proxy/dap-proxy-connection-manager.test.ts @@ -570,11 +570,34 @@ describe('DapConnectionManager', () => { expect(mockDapClient.sendRequest).toHaveBeenCalledWith('setBreakpoints', { source: { path: sourcePath, name: 'source.py' }, - breakpoints: [{ line: 10, condition: undefined }] + breakpoints: [{ line: 10 }] }); expect(result).toBe(response); }); + it('forwards logMessage and suspendPolicy fields (issue #235)', async () => { + const response: DebugProtocol.SetBreakpointsResponse = { + seq: 1, + type: 'response', + request_seq: 1, + command: 'setBreakpoints', + success: true, + body: { breakpoints: [{ verified: true, line: 10 }] } + }; + mockDapClient.sendRequest.mockResolvedValue(response); + + await connectionManager.setBreakpoints( + mockDapClient as any, + sourcePath, + [{ line: 10, condition: 'x > 1', logMessage: 'x is {x}', suspendPolicy: 'thread' }] + ); + + expect(mockDapClient.sendRequest).toHaveBeenCalledWith('setBreakpoints', { + source: { path: sourcePath, name: 'source.py' }, + breakpoints: [{ line: 10, condition: 'x > 1', logMessage: 'x is {x}', suspendPolicy: 'thread' }] + }); + }); + it('should set multiple breakpoints', async () => { const response: DebugProtocol.SetBreakpointsResponse = { seq: 1, @@ -607,9 +630,9 @@ describe('DapConnectionManager', () => { expect(mockDapClient.sendRequest).toHaveBeenCalledWith('setBreakpoints', { source: { path: sourcePath, name: 'source.py' }, breakpoints: [ - { line: 10, condition: undefined }, - { line: 20, condition: undefined }, - { line: 30, condition: undefined } + { line: 10 }, + { line: 20 }, + { line: 30 } ] }); expect(result.body.breakpoints).toHaveLength(3); diff --git a/tests/unit/proxy/dap-proxy-message-parser.test.ts b/tests/unit/proxy/dap-proxy-message-parser.test.ts index 281520bd..34b76675 100644 --- a/tests/unit/proxy/dap-proxy-message-parser.test.ts +++ b/tests/unit/proxy/dap-proxy-message-parser.test.ts @@ -223,6 +223,27 @@ describe('MessageParser', () => { }); }); + it('accepts logMessage and suspendPolicy on initial breakpoints (issue #235)', () => { + const payload = { + cmd: 'init', + sessionId: 'test-session', + executablePath: '/usr/bin/python3', + adapterHost: 'localhost', + adapterPort: 5678, + logDir: '/tmp/logs', + scriptPath: '/home/user/script.py', + initialBreakpoints: [ + { file: 'test.py', line: 10, logMessage: 'x is {x}', suspendPolicy: 'thread' } + ] + }; + + const result = MessageParser.validateInitPayload(payload); + expect(result.initialBreakpoints?.[0]).toMatchObject({ + logMessage: 'x is {x}', + suspendPolicy: 'thread' + }); + }); + it('should throw on invalid breakpoints', () => { const invalidBreakpoints = [ [{}], // Missing file and line @@ -230,7 +251,9 @@ describe('MessageParser', () => { [{ line: 10 }], // Missing file [{ file: 123, line: 10 }], // Invalid file type [{ file: 'test.py', line: 'abc' }], // Non-numeric line string still fails - [{ file: 'test.py', line: 10, condition: 123 }] // Invalid condition type + [{ file: 'test.py', line: 10, condition: 123 }], // Invalid condition type + [{ file: 'test.py', line: 10, logMessage: 123 }], // Invalid logMessage type + [{ file: 'test.py', line: 10, suspendPolicy: 'sometimes' }] // Invalid suspendPolicy value ]; invalidBreakpoints.forEach(breakpoints => { diff --git a/tests/unit/server-coverage.test.ts b/tests/unit/server-coverage.test.ts index a34f0255..006ad542 100644 --- a/tests/unit/server-coverage.test.ts +++ b/tests/unit/server-coverage.test.ts @@ -1,4 +1,4 @@ -/** +/** * Targeted tests to improve coverage for server.ts * Focus on error paths and edge cases */ @@ -159,7 +159,7 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { .rejects.toThrow('Cannot get stack trace: no active proxy'); }); - it('should handle getStackTrace without current thread — falls back to threads request', async () => { + it('should handle getStackTrace without current thread — falls back to threads request', async () => { const mockProxy = { getCurrentThreadId: () => null, isRunning: () => true, @@ -359,7 +359,7 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { expect(result.verified).toBe(true); expect(mockFileChecker.checkExists).not.toHaveBeenCalled(); - expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith('test-session', 'com.example.MyClass', 42, undefined, undefined); + expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith('test-session', 'com.example.MyClass', 42, undefined, undefined, undefined); }); it('should skip file existence check for inner class notation via policy', async () => { @@ -484,7 +484,7 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { sessionLifecycle: SessionLifecycleState.ACTIVE }); - // Non-Java policy (no isNonFileSourceIdentifier) — even "MyClass" gets file-checked + // Non-Java policy (no isNonFileSourceIdentifier) — even "MyClass" gets file-checked mockSessionManager.getSessionPolicy.mockReturnValue({}); (server as any).fileChecker = { @@ -709,10 +709,10 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { // Both calls should have been made expect(mockSessionManager.setBreakpoint).toHaveBeenCalledTimes(2); expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith( - 'test-session', 'com.example.Foo', 10, undefined, undefined + 'test-session', 'com.example.Foo', 10, undefined, undefined, undefined ); expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith( - 'test-session', 'com.example.Foo', 20, undefined, undefined + 'test-session', 'com.example.Foo', 20, undefined, undefined, undefined ); }); @@ -761,10 +761,10 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { // Both should be set independently expect(mockSessionManager.setBreakpoint).toHaveBeenCalledTimes(2); expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith( - 'test-session', 'com.a.Foo', 10, undefined, undefined + 'test-session', 'com.a.Foo', 10, undefined, undefined, undefined ); expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith( - 'test-session', 'com.b.Foo', 15, undefined, undefined + 'test-session', 'com.b.Foo', 15, undefined, undefined, undefined ); }); }); diff --git a/tests/unit/shared/adapter-policy-js.test.ts b/tests/unit/shared/adapter-policy-js.test.ts index 4c57a535..42a00bda 100644 --- a/tests/unit/shared/adapter-policy-js.test.ts +++ b/tests/unit/shared/adapter-policy-js.test.ts @@ -177,13 +177,49 @@ describe('JsDebugAdapterPolicy', () => { 'setBreakpoints', expect.objectContaining({ source: { path: '/workspace/app.js' }, - breakpoints: [{ line: 12, condition: undefined }] + breakpoints: [{ line: 12 }] }) ); expect(sendDapRequest).toHaveBeenCalledWith('configurationDone', {}); expect(sendDapRequest.mock.calls.some(([cmd]) => cmd === 'launch')).toBe(true); }); + it('forwards logMessage on handshake breakpoints (issue #235)', async () => { + vi.useFakeTimers(); + const events = new EventEmitter(); + const sendDapRequest = vi.fn().mockResolvedValue({}); + + const proxyManager = Object.assign(events, { + isRunning: () => true, + sendDapRequest, + removeListener: events.removeListener.bind(events) + }); + + const context = { + proxyManager, + sessionId: 'session-1', + dapLaunchArgs: { stopOnEntry: false }, + scriptPath: '/workspace/app.js', + breakpoints: new Map([ + ['bp1', { file: '/workspace/app.js', line: 12, logMessage: 'x is {x}' }] + ]) + }; + + const handshakePromise = JsDebugAdapterPolicy.performHandshake(context as any); + await Promise.resolve(); + events.emit('dap-event', { event: 'initialized' }); + await vi.advanceTimersByTimeAsync(0); + await handshakePromise; + vi.useRealTimers(); + + expect(sendDapRequest).toHaveBeenCalledWith( + 'setBreakpoints', + expect.objectContaining({ + breakpoints: [{ line: 12, logMessage: 'x is {x}' }] + }) + ); + }); + it('does not miss an initialized event emitted before the initialize response settles (issue #242)', async () => { vi.useFakeTimers(); try {