Skip to content

Commit d4ce62c

Browse files
debugmcpdevCI Botclaude
authored
test(go): assert the exit code, not Delve's console line, so Delve 1.27+ passes (#753) (#874)
Delve 1.27.0 sends a DAP `exited` event (go-delve/delve#4371). The proxy already uses it and so skips the early disconnect it sends for older Delve, which means the "Process N has exited with status S" line now arrives only at teardown and never reaches get_output. The exit code is still correct; only go-exit-code.test.ts's console-line assertion failed. No product change. - Drop the status-line assertion; the exit-code checks cover both paths - CI: new go-current job (Go 1.27 + dlv 1.27.2, pinned) runs the Go integration tests and fails on skips; required by Test Summary - Docs and comments no longer say Delve never sends `exited`; the policy notes the fallback can go once the Go floor is 1.25+ - changelog.d fragment Co-authored-by: CI Bot <ci@example.com> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent d4f204a commit d4ce62c

9 files changed

Lines changed: 85 additions & 29 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -428,9 +428,51 @@ jobs:
428428
path: artifacts/cobol/
429429
if-no-files-found: warn
430430

431+
go-current:
432+
# build-and-test pins Go 1.21 + dlv 1.24.2, the oldest pair we support:
433+
# that Delve never sends a DAP `exited`, so the console-line exit-code
434+
# path (#753) is what it covers. Delve 1.27+ sends `exited` itself and
435+
# needs Go 1.25+, so a current pair runs the Go integration tests here.
436+
# Pinned, not @latest, so a Go or Delve release never turns main red on
437+
# its own; bump both together.
438+
name: Go Current (Go 1.27 + Delve 1.27.2)
439+
runs-on: ubuntu-latest
440+
timeout-minutes: 20
441+
steps:
442+
- name: Checkout code
443+
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
444+
- name: Setup pnpm
445+
uses: pnpm/action-setup@ea17c68df8912ef543352723c149a84f56e3d413 # v6.1.0
446+
- name: Setup Node.js
447+
uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
448+
with:
449+
node-version: '22.x'
450+
cache: 'pnpm'
451+
- name: Setup Go
452+
uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
453+
with:
454+
go-version: '1.27'
455+
- name: Install and verify Delve
456+
run: |
457+
go install github.com/go-delve/delve/cmd/dlv@v1.27.2
458+
dlv version | tee /tmp/dlv-version.txt
459+
grep -E '^Version: 1\.27\.2$' /tmp/dlv-version.txt
460+
- name: Install dependencies
461+
run: pnpm install --frozen-lockfile
462+
env:
463+
SKIP_ADAPTER_VENDOR: 'true'
464+
- name: Build project
465+
run: pnpm run build:ci
466+
# The Go suites skip without Go/Delve; a skip here would mean this job
467+
# proved nothing, so the report must show passes and no skips.
468+
- name: Run Go integration tests
469+
run: |
470+
pnpm exec vitest run --project integration tests/adapters/go/integration --reporter=default --reporter=json --outputFile=/tmp/go-current.json
471+
node -e "const r=require('/tmp/go-current.json'); console.log('passed', r.numPassedTests, 'skipped', r.numPendingTests + r.numTodoTests); if (r.numPassedTests === 0 || r.numPendingTests + r.numTodoTests > 0) process.exit(1)"
472+
431473
test-summary:
432474
name: Test Summary
433-
needs: [build-and-test, windows-python-integration, lint, container-tests, cobol-host]
475+
needs: [build-and-test, windows-python-integration, lint, container-tests, cobol-host, go-current]
434476
runs-on: ubuntu-latest
435477
if: always()
436478

@@ -442,11 +484,13 @@ jobs:
442484
echo "lint: ${{ needs.lint.result }}"
443485
echo "container-tests: ${{ needs.container-tests.result }}"
444486
echo "cobol-host: ${{ needs.cobol-host.result }}"
487+
echo "go-current: ${{ needs.go-current.result }}"
445488
if [ "${{ needs.build-and-test.result }}" != "success" ] \
446489
|| [ "${{ needs.windows-python-integration.result }}" != "success" ] \
447490
|| [ "${{ needs.lint.result }}" != "success" ] \
448491
|| [ "${{ needs.container-tests.result }}" != "success" ] \
449-
|| [ "${{ needs.cobol-host.result }}" != "success" ]; then
492+
|| [ "${{ needs.cobol-host.result }}" != "success" ] \
493+
|| [ "${{ needs.go-current.result }}" != "success" ]; then
450494
echo "❌ One or more required jobs did not succeed"
451495
exit 1
452496
fi
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
**Go: Delve 1.27's own `exited` event is supported alongside the older console-line exit status** — Delve 1.27 sends a DAP `exited` event with the program's exit code (go-delve/delve#4371). mcp-debugger already uses that event when it arrives, so `exitCode` in `list_debug_sessions` and the `start_debugging` run-to-completion summary is the same on every Delve. Older Delve, which is all a Go before 1.25 can run, is still covered by reading Delve's `Process N has exited with status S` console line. One visible difference: with Delve 1.27+ in debug mode, that console line no longer shows up in `get_output`, because Delve prints it only while the session is being torn down. CI now runs the Go integration tests on both Go 1.21 + dlv 1.24.2 and Go 1.27 + dlv 1.27.2 (#753)

‎docs/development/dap-sequence-reference.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ For Node, the **event order** might differ slightly depending on how it’s impl
3838

3939
### **Go (Delve)**
4040

41-
The **Delve** debugger (which has a DAP mode, often invoked via `dlv dap`) likewise follows the protocol. According to the Delve DAP documentation, *when the debugged program terminates, Delve sends a `terminated` event*, and it expects the client (IDE) to then issue a `disconnect` request to finish the session (and shut down the adapter). This design implies that Delve treats the `terminated` event as the primary signal that “the debug session is done.” Delve does **not** send an `exited` event (measured in this project, issue #753): the exit status is printed as a console `output` line, `Process N has exited with status S` — before `terminated` under `noDebug`, and in debug mode only in reply to the client's `disconnect` — which mcp-debugger's proxy reads back and replays as `exited` ahead of `terminated`. (If you attach to an external process and then detach, Delve would just send `terminated` since the process is still running and there is no exit code to report.)
41+
The **Delve** debugger (which has a DAP mode, often invoked via `dlv dap`) likewise follows the protocol. According to the Delve DAP documentation, *when the debugged program terminates, Delve sends a `terminated` event*, and it expects the client (IDE) to then issue a `disconnect` request to finish the session (and shut down the adapter). This design implies that Delve treats the `terminated` event as the primary signal that “the debug session is done.” Delve before 1.27 does **not** send an `exited` event (measured in this project, issue #753; 1.27 added one, go-delve/delve#4371): the exit status is printed as a console `output` line, `Process N has exited with status S` — before `terminated` under `noDebug`, and in debug mode only in reply to the client's `disconnect` — which mcp-debugger's proxy reads back and replays as `exited` ahead of `terminated`. (If you attach to an external process and then detach, Delve would just send `terminated` since the process is still running and there is no exit code to report.)
4242

4343
One particular difference in Delve’s design is that the **`terminated` event triggers the client to clean up**. The VS Code Go extension, upon seeing `terminated`, will send a `disconnect` to Delve DAP, which then causes Delve to shut down the debugger backend. In effect, **Delve emits**: `terminated` → (awaits `disconnect` from client) → a console `output` line carrying the exit status; no `exited` event at all. If the debug session is ended by user (stop command), the sequence is similar except the client may initiate a `terminate` or `disconnect` request first, and Delve will still emit `terminated` during the shutdown.
4444

@@ -50,7 +50,7 @@ To summarize the behaviors of these adapters, here’s a comparison:
5050
| ------------------------- | ------------------------------------------------- | --------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
5151
| **debugpy (Python)** | Detects Python script exit (including exit code). | `exited` (with exit code) → `terminated` | Always sends both events on a normal exit to signal end. Handles special cases like `sys.exit()` as normal termination (no lingering pause). May delay termination if configured to wait for input on exit. |
5252
| **js-debug / pwa-node (Node.js)** | Detects end-of-script or process exit. | `exited` (with code) → `terminated` *(order in practice)* | Sends both events. Handles Node’s historical behavior of waiting for detach – the adapter ensures it sends `terminated` to close session, allowing Node process to exit. In attach scenarios, if user disconnects, it might only send `terminated` (since the process continues running). |
53-
| **Delve (Go)** | Detects program termination. | `terminated` → client `disconnect` → console `output` with the exit status (no `exited`) | Sends `terminated` and expects a client `disconnect`; prints `Process N has exited with status S` to the console — under `noDebug` before `terminated`, in debug mode only in reply to `disconnect` — and mcp-debugger synthesizes `exited` from that line (#753). In attach mode (no process exit) just `terminated` on detach. |
53+
| **Delve (Go)** | Detects program termination. | Before 1.27: `terminated` → client `disconnect` → console `output` with the exit status (no `exited`). 1.27+: `exited` → `terminated` | Sends `terminated` and expects a client `disconnect`; before 1.27 prints `Process N has exited with status S` to the console — under `noDebug` before `terminated`, in debug mode only in reply to `disconnect` — and mcp-debugger synthesizes `exited` from that line (#753). In attach mode (no process exit) just `terminated` on detach. |
5454

5555
*Note:* Nearly all debug adapters follow the rule that **the session ends with a `terminated` event**. The `exited` event is used when applicable to convey the exit code or confirmation that the debuggee process ended. Some adapters might emit these in quick succession such that the order doesn't visibly matter to the user, but the sequence above (exit then terminate) is typical. Importantly, a `stopped` event is **not** used for program end – receiving `stopped` means a pause, whereas `terminated`/`exited` mean finish.
5656

‎docs/go/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ The Go adapter supports exception breakpoints with two built-in filters: `unreco
169169

170170
### Exit Code
171171

172-
Delve never sends a DAP `exited` event; it prints `Process N has exited with status S` to the console instead — under `noDebug` before `terminated`, and in debug mode only in reply to the `disconnect` request. mcp-debugger reads that line back (issue #753), so `exitCode` appears in `list_debug_sessions` and in the `start_debugging` run-to-completion summary exactly as for every other language, and the line itself stays visible in `get_output` (with Delve's `Detaching`). A session closed while the program is still running reports no code — Delve prints `Detaching and terminating target process` instead, and nothing is guessed.
172+
Delve 1.27 and later send a DAP `exited` event with the code. Older Delve — the only kind a Go older than 1.25 can run — never does; it prints `Process N has exited with status S` to the console instead — under `noDebug` before `terminated`, and in debug mode only in reply to the `disconnect` request. mcp-debugger reads that line back (issue #753). Either way `exitCode` appears in `list_debug_sessions` and in the `start_debugging` run-to-completion summary exactly as for every other language. The line itself is visible in `get_output` (with Delve's `Detaching`) under `noDebug`, and in debug mode on Delve older than 1.27; Delve 1.27+ in debug mode prints it only as the session is torn down, after output capture has ended. A session closed while the program is still running reports no code — Delve prints `Detaching and terminating target process` instead, and nothing is guessed.
173173

174174
## Debugging Tips
175175

‎docs/tool-reference.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ See the [JavaScript configuration guide](javascript/README.md#custom-configurati
453453
- When a stop and the exit arrived in the same tick (a breakpoint hit as the program ended), the breakpoints were not all missed: the list is withheld and the sentence names the stop (`after a stop the launch could not report (last stop: breakpoint)`), which `list_debug_sessions` also shows as `lastStop`.
454454
- `restart_debugging` returns the same fields for a relaunch that ends the same way.
455455

456-
**Exit code:** when the debuggee terminates, the exit code reported by the adapter is surfaced as `exitCode` in `list_debug_sessions` (and, for a launch that ended before reporting a pause, as `data.exitCode` above), so a crash (non-zero) is distinguishable from a clean exit. Adapters that never send `exited` still get one where the debugger states the code some other way: js-debug through the launch shim's recorded code (#247), rdbg through the adapter process's own exit status (#258), Delve through its `Process N has exited with status S` console line (#753 — in debug mode Delve prints it only in reply to `disconnect`, so the proxy sends the disconnect before forwarding `terminated`). No line, no code: nothing is guessed.
456+
**Exit code:** when the debuggee terminates, the exit code reported by the adapter is surfaced as `exitCode` in `list_debug_sessions` (and, for a launch that ended before reporting a pause, as `data.exitCode` above), so a crash (non-zero) is distinguishable from a clean exit. Adapters that never send `exited` still get one where the debugger states the code some other way: js-debug through the launch shim's recorded code (#247), rdbg through the adapter process's own exit status (#258), Delve before 1.27 through its `Process N has exited with status S` console line (#753 — in debug mode Delve prints it only in reply to `disconnect`, so the proxy sends the disconnect before forwarding `terminated`; Delve 1.27+ sends `exited` itself). No line, no code: nothing is guessed.
457457

458458
**Between launches:** a session whose program has exited (or that has only dry-run) is still present, with its breakpoints, its last real launch and — until the next launch or dry run — the previous run's output and exit code. `start_debugging` (a new configuration), `restart_debugging` (the same one), `set_breakpoint`, `list_breakpoints`, `remove_breakpoint`, `clear_breakpoints` and `get_output` all work on it, so "ran to completion without hitting my breakpoint → set an earlier one → `restart_debugging`" needs no new session; each launch starts a fresh output buffer and re-applies the stored breakpoints with their verified state reset. Tools that need a live debuggee — stepping, `continue_execution`, `pause_execution`, `list_threads`, `get_stack_trace`/`get_scopes`/`get_variables`/`get_local_variables`/`evaluate_expression`, `get_source_context`, `expose_session` — answer `Session is terminated: <id>` until the next launch (issues #793, #806).
459459

‎packages/shared/src/interfaces/adapter-policy-go.ts‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -345,12 +345,18 @@ export const GoAdapterPolicy = {
345345
},
346346

347347
/**
348-
* Delve never sends a DAP `exited` event; the status is one console line,
349-
* proc.ErrProcessExited.Error() — "Process %d has exited with status %d" —
350-
* logged before `terminated` under noDebug and, in debug mode, only from
351-
* stopDebugSession in reply to `disconnect` (issue #753). Console category
352-
* only: a debuggee printing the same words to stdout is not Delve speaking.
353-
* Go's ExitCode() is -1 for a signal-killed process — not a code.
348+
* Delve before 1.27 never sends a DAP `exited` event; the status is one
349+
* console line, proc.ErrProcessExited.Error() — "Process %d has exited with
350+
* status %d" — logged before `terminated` under noDebug and, in debug mode,
351+
* only from stopDebugSession in reply to `disconnect` (issue #753). Console
352+
* category only: a debuggee printing the same words to stdout is not Delve
353+
* speaking. Go's ExitCode() is -1 for a signal-killed process — not a code.
354+
*
355+
* Delve 1.27+ sends `exited` itself (go-delve/delve#4371) and the worker
356+
* then never consults this. It stays as the fallback for older Delve, which
357+
* is all a Go older than 1.25 can run (Delve 1.27 supports Go 1.25+); drop
358+
* it, and exitStatusReportedOnDisconnect, once the supported Go floor is
359+
* 1.25 or later.
354360
*/
355361
debuggeeExitCodeFromOutput: (category: string, text: string): number | undefined => {
356362
if (category !== 'console') {
@@ -364,7 +370,7 @@ export const GoAdapterPolicy = {
364370
return Number.isSafeInteger(status) && status >= 0 ? status : undefined;
365371
},
366372

367-
/** In debug mode Delve prints the status line only in reply to `disconnect` (issue #753). */
373+
/** In debug mode Delve before 1.27 prints the status line only in reply to `disconnect` (issue #753). */
368374
exitStatusReportedOnDisconnect: true,
369375

370376
/**

‎packages/shared/src/interfaces/adapter-policy.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ export interface AdapterPolicy {
453453
/**
454454
* Read the debuggee's exit code out of a DAP 'output' event, for adapters
455455
* that state it only as console text and never send `exited` (issue #753 —
456-
* Delve prints `Process N has exited with status S`). Consulted by the proxy
456+
* Delve before 1.27 prints `Process N has exited with status S`). Consulted by the proxy
457457
* worker on every output event, which is still forwarded unchanged so
458458
* get_output keeps the adapter's line; the first code returned is replayed
459459
* as a synthesized DAP `exited` event ahead of `terminated`, exactly as the

‎tests/adapters/go/integration/go-exit-code.test.ts‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,20 @@
11
/**
2-
* Real MCP -> Delve -> Go coverage for issue #753. Delve never sends a DAP
3-
* `exited` event; it prints "Process N has exited with status S" to the
4-
* console — under noDebug before `terminated`, in debug mode only in reply
5-
* to `disconnect` — and the proxy reads that line back so `exitCode` appears
6-
* like every other language's. Runs in CI (Go 1.21 + dlv 1.24.2 on ubuntu
7-
* and windows) — the first CI-run test that spawns real Delve; `dist/` must
8-
* be built (CI builds before the integration project).
2+
* Real MCP -> Delve -> Go coverage for issue #753: the debuggee's exit code
3+
* reaches the launch result and list_debug_sessions. Delve before 1.27 never
4+
* sends a DAP `exited` event; it prints "Process N has exited with status S"
5+
* to the console — under noDebug before `terminated`, in debug mode only in
6+
* reply to `disconnect` — and the proxy reads that line back. Delve 1.27+
7+
* sends `exited` itself (go-delve/delve#4371), so the proxy's early
8+
* disconnect never fires and the status line, printed only at teardown, no
9+
* longer reaches get_output. The tests assert the exit code, which both
10+
* paths must produce, not the mechanism. CI runs this file on dlv 1.24.2
11+
* (Go 1.21, ubuntu and windows: the console-line path) and on the go-current
12+
* job (the native `exited` path); `dist/` must be built (CI builds before the
13+
* integration project).
914
*
10-
* The fixture is its own Go module pinned to `go 1.21`: dlv 1.24.2 accepts
11-
* binaries built by Go 1.21–1.24 only, and the root examples/go module pins a
12-
* newer Go. A developer whose local Go is newer than their dlv's window fails
15+
* The fixture is its own Go module pinned to `go 1.21` so the oldest pinned
16+
* Delve can build it: dlv 1.24.2 accepts binaries built by Go 1.21–1.24
17+
* only. A developer whose local Go is newer than their dlv's window fails
1318
* (not skips) this test — upgrade dlv.
1419
*/
1520
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
@@ -102,18 +107,17 @@ describe.skipIf(!hasGo)('Go exit code from Delve\'s console status line (issue #
102107
await expect.poll(async () => (await listedSession())?.exitCode, { timeout: 30_000 }).toBe(expected);
103108
}
104109

105-
it('reports a non-zero exit code in debug mode, where Delve prints the status only in reply to disconnect', async (ctx) => {
110+
it('reports a non-zero exit code in debug mode', async (ctx) => {
106111
await launchAndExpectExit({ scriptPath: program, args: ['7'], dapLaunchArgs: { stopOnEntry: false, cwd: fixtureDir } }, 7, ctx);
107112
const text = await outputText();
108113
expect(text).toContain('exit_code fixture: exiting with status 7');
109-
expect(text).toMatch(/has exited with status 7/);
110114
}, 90_000);
111115

112116
it('reports exit code 0 for a clean run in debug mode', async (ctx) => {
113117
await launchAndExpectExit({ scriptPath: program, dapLaunchArgs: { stopOnEntry: false, cwd: fixtureDir } }, 0, ctx);
114118
}, 90_000);
115119

116-
it('reports the exit code under noDebug, where Delve prints the status before terminated', async (ctx) => {
120+
it('reports the exit code under noDebug', async (ctx) => {
117121
// Delve runs a noDebug target through Go's exec, so on Windows the binary needs its .exe.
118122
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mcp-go-exit-'));
119123
try {

0 commit comments

Comments
 (0)