Skip to content

Commit 323dc05

Browse files
debugmcpdevclaude
andauthored
fix(docker): ship the JavaScript exit-code shim in the image and test exit codes end to end (#796, #735) (#809)
JavaScript programs ran fine in the Docker image but their sessions ended stopped with no exitCode. js-debug never sends a DAP exited event; the code comes from a preload the adapter injects (assets/exitcode-shim.cjs, #247) and the adapter degraded silently without it. The Dockerfile re-materialised the adapter from dist, vendor and package.json only, so assets/ never reached the image (measured: the local images hold dist package.json vendor and nothing else). The image now copies the adapter's assets and fails to build if the shim is missing (the same guard the CodeLLDB engine has, #387); the npm bundle build fails without it too (bundle-cli.js throws instead of warning); a JavaScript launch that cannot find the preload says so in its start_debugging warning through a new launch-scoped diagnostic (LaunchConfigDiagnostic.scope 'launch', rendered as-is by the launcher, whose existing rendering matches diagnostics against caller keys only); a Docker smoke case waits for the exit code itself (7) in debug mode and under noDebug on the new examples/javascript/exit_code_test.js; and the CI-run JavaScript integration file gains a debugger-on run-to-completion case asserting exitCode 7 in list_debug_sessions (and on the summary when the fixture beats launch readiness) — #735 item 4, where #794's case covered noDebug only. Closes #796 Refs #735 Review and verification: - Multi-agent code review: no correctness defects, 5 altitude findings; 4 fixed (the bundle gate moved from the never-run test-bundle.cjs into bundle-cli.js; the missing-shim launch notice; pollUntil in the Docker test; a race-honest description of the summary assertion), 1 deferred to #811 (re-materialise adapter packages from their package.json files field). The notice change surfaced three #709 diagnostics tests whose harness had no file system; they now model the preload as present. - Local: the rebuilt image contains the shim and passes the guard; Docker JS smoke 8/8 including both new cases; JS integration file 11/11 on the merged tree; unit and launcher suites green; pre-push (6402 unit, 92 integration) and CI green on both OSes including Container Tests. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 45c7e62 commit 323dc05

12 files changed

Lines changed: 150 additions & 8 deletions

File tree

‎Dockerfile‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ RUN rm -rf /app/node_modules/@debugmcp && \
138138
cp /app/packages/adapter-python/package.json /app/node_modules/@debugmcp/adapter-python/ && \
139139
cp -r /app/packages/adapter-javascript/dist /app/node_modules/@debugmcp/adapter-javascript/ && \
140140
cp -r /app/packages/adapter-javascript/vendor /app/node_modules/@debugmcp/adapter-javascript/ && \
141+
cp -r /app/packages/adapter-javascript/assets /app/node_modules/@debugmcp/adapter-javascript/ && \
141142
cp /app/packages/adapter-javascript/package.json /app/node_modules/@debugmcp/adapter-javascript/ && \
142143
mkdir -p /app/node_modules/@debugmcp/adapter-javascript/node_modules && \
143144
cp -rL /app/packages/adapter-javascript/node_modules/dotenv /app/node_modules/@debugmcp/adapter-javascript/node_modules/ && \
@@ -272,6 +273,11 @@ COPY --from=builder /app/node_modules/.pnpm/isexe@4.0.0/node_modules/isexe /app/
272273
# runtime dependency copy is incomplete, instead of shipping a broken launch.
273274
RUN node --input-type=module -e "await import('@debugmcp/adapter-javascript')"
274275

276+
# The JavaScript adapter's exit-code preload (issue #796): js-debug never
277+
# sends a DAP exited event, so exitCode comes from this shim, which the
278+
# adapter degrades silently without. Fail the build if the copy is missing.
279+
RUN test -f /app/node_modules/@debugmcp/adapter-javascript/assets/exitcode-shim.cjs
280+
275281
# Expose ports
276282
EXPOSE 3001 5679
277283

‎changelog.d/796.fixed.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
**Docker JavaScript launches report `exitCode` again** — the image copied the JavaScript adapter's `dist`, `vendor` and `package.json` but not its `assets/`, so `exitcode-shim.cjs` (the preload that records the debuggee's exit code, since js-debug never sends `exited`) was missing and every JavaScript session in the container ended `stopped` with no `exitCode`. The image now ships the asset and fails to build without it (the same guard the CodeLLDB engine has), the npm bundle build fails without it too, a JavaScript launch that cannot find the preload says so in its `warning` instead of only in the server log, and the Docker smoke test waits for the non-zero exit code itself, in debug and noDebug mode (#796)
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#!/usr/bin/env node
2+
/**
3+
* Exit-code fixture for MCP debugger smoke tests.
4+
*
5+
* Prints one line and exits with status 7, so a test can tell "the program
6+
* ran to completion" from "the debugger reported its exit code" (issue #796:
7+
* the Docker image shipped without the JavaScript exit-code preload, and
8+
* sessions ended `stopped` with no exitCode at all).
9+
*/
10+
console.log('exit_code_test: exiting with status 7');
11+
process.exitCode = 7;

‎packages/adapter-javascript/src/javascript-debug-adapter.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -922,9 +922,11 @@ export class JavascriptDebugAdapter extends EventEmitter implements IDebugAdapte
922922
}
923923

924924
if (!shimPath) {
925-
this.dependencies.logger?.warn?.(
926-
'[JavascriptDebugAdapter] exitcode-shim.cjs not found; debuggee exit code will not be captured'
927-
);
925+
// Not a caller key: a launch-scoped notice the launcher renders as-is, so
926+
// the first launch says it instead of the server log alone (issue #796).
927+
const message = `exit codes will not be captured: exitcode-shim.cjs not found (looked in ${candidates.join(', ')})`;
928+
this.dependencies.logger?.warn?.(`[JavascriptDebugAdapter] ${message}`);
929+
this.launchConfigDiagnostics.push({ key: 'exitcode-shim.cjs', scope: 'launch', message });
928930
return;
929931
}
930932

‎packages/adapter-javascript/tests/unit/javascript-debug-adapter.transform.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,27 @@ describe('JavascriptDebugAdapter.transformLaunchConfig', () => {
332332
expect(requireArg).not.toContain('\\');
333333
});
334334

335+
it('reports a missing shim as a launch notice instead of degrading silently (issue #796)', async () => {
336+
const withoutShim = new JavascriptDebugAdapter({
337+
...deps,
338+
fileSystem: { existsSync: () => false }
339+
} as unknown as import('@debugmcp/shared').AdapterDependencies);
340+
const cfg = await withoutShim.transformLaunchConfig({
341+
program: path.resolve('/proj/app.js')
342+
} as any);
343+
344+
const env = cfg.env as Record<string, string>;
345+
expect(env.MCP_DEBUGGER_EXITCODE_FILE).toBeUndefined();
346+
expect(env.NODE_OPTIONS ?? '').not.toContain('exitcode-shim');
347+
expect(withoutShim.consumeLaunchConfigDiagnostics()).toContainEqual(
348+
expect.objectContaining({
349+
key: 'exitcode-shim.cjs',
350+
scope: 'launch',
351+
message: expect.stringContaining('exit codes will not be captured: exitcode-shim.cjs not found')
352+
})
353+
);
354+
});
355+
335356
it('preserves pre-existing NODE_OPTIONS content', async () => {
336357
const withFs = new JavascriptDebugAdapter(depsWithFs);
337358
const cfg = await withFs.transformLaunchConfig({

‎packages/adapter-javascript/tests/unit/launch-config-diagnostics.test.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,12 @@ import { describe, expect, it, vi } from 'vitest';
22
import type { AdapterDependencies, GenericLaunchConfig } from '@debugmcp/shared';
33
import { JavascriptDebugAdapter } from '../../src/javascript-debug-adapter.js';
44

5+
// The exit-code preload resolves through fileSystem.existsSync; a harness
6+
// without one would carry the missing-shim launch notice on every transform (#796).
7+
const shimPresent = { existsSync: () => true };
8+
59
function adapter() {
6-
return new JavascriptDebugAdapter({ logger: { info: vi.fn(), warn: vi.fn() } } as unknown as AdapterDependencies);
10+
return new JavascriptDebugAdapter({ logger: { info: vi.fn(), warn: vi.fn() }, fileSystem: shimPresent } as unknown as AdapterDependencies);
711
}
812

913
describe('JavaScript launch diagnostics (#709)', () => {
@@ -27,7 +31,7 @@ describe('JavaScript launch diagnostics (#709)', () => {
2731

2832
it('accepts explicit empty lists, null source-map locations and matching pinned values', async () => {
2933
const logger = { info: vi.fn(), warn: vi.fn() };
30-
const subject = new JavascriptDebugAdapter({ logger } as unknown as AdapterDependencies);
34+
const subject = new JavascriptDebugAdapter({ logger, fileSystem: shimPresent } as unknown as AdapterDependencies);
3135
const result = await subject.transformLaunchConfig({ program: '/project/app.js', outFiles: [], skipFiles: [],
3236
runtimeArgs: [], resolveSourceMapLocations: null, console: 'internalConsole', type: 'pwa-node', envFile: null,
3337
request: 'launch'
@@ -57,7 +61,7 @@ describe('JavaScript launch diagnostics (#709)', () => {
5761

5862
it('consumes envFile and keeps null env entries in the DAP result', async () => {
5963
const subject = new JavascriptDebugAdapter({
60-
logger: { info: vi.fn(), warn: vi.fn() }, fileSystem: { readFile: vi.fn(async () => 'REMOVE=file\nFILE_ONLY=works\n') }
64+
logger: { info: vi.fn(), warn: vi.fn() }, fileSystem: { ...shimPresent, readFile: vi.fn(async () => 'REMOVE=file\nFILE_ONLY=works\n') }
6165
} as unknown as AdapterDependencies);
6266
const result = await subject.transformLaunchConfig({ program: '/project/app.js', envFile: 'app.env', env: { REMOVE: null } } as GenericLaunchConfig);
6367
expect(result.env).toMatchObject({ REMOVE: null, FILE_ONLY: 'works' });

‎packages/mcp-debugger/scripts/bundle-cli.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,9 @@ async function bundleCLI() {
274274
fs.copyFileSync(exitShimSrc, path.join(assetsDest, 'exitcode-shim.cjs'));
275275
console.log('Copied exitcode-shim.cjs.');
276276
} else {
277-
console.warn('Warning: exitcode-shim.cjs not found; JS debuggee exit codes will not be captured in NPX distribution.');
277+
// Fail the bundle rather than ship a package whose JavaScript sessions end
278+
// with no exitCode (issue #796, the same class the Docker image had).
279+
throw new Error(`exitcode-shim.cjs not found at ${exitShimSrc}; the npm bundle must ship the JavaScript exit-code preload (issue #796)`);
278280
}
279281

280282
// Copy the agent skill so the package is a pi coding-agent package (issue

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,8 +404,16 @@ export interface GenericLaunchConfig {
404404

405405
/** An adapter's explanation of how it handled one launch input. */
406406
export interface LaunchConfigDiagnostic {
407+
/** The caller's launch key this is about — or, with scope 'launch', a label for a notice about the launch as a whole. */
407408
key: string;
408409
message: string;
410+
/**
411+
* 'launch': not tied to a caller input. The launcher renders the message
412+
* as-is instead of matching `key` against the caller's keys (issue #796 —
413+
* e.g. the JavaScript adapter's exit-code preload is missing from this
414+
* distribution, so exit codes will not be captured).
415+
*/
416+
scope?: 'launch';
409417
}
410418

411419
/**

‎src/session/launch/launch-config-diagnostics.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,5 +50,9 @@ export function collectLaunchConfigNotices(
5050
notices.push(`${name}: ${message}${suggestion && suggestion !== key ? ` (did you mean ${suggestion}?)` : ''}`);
5151
}
5252
}
53+
// Launch-scoped notices are about the launch as a whole, not a caller key (#796).
54+
for (const diagnostic of diagnostics) {
55+
if (diagnostic.scope === 'launch') notices.push(diagnostic.message);
56+
}
5357
return [...new Set(notices)];
5458
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* collectLaunchConfigNotices turns the adapter's per-key diagnostics into the
3+
* start_debugging warning by matching them against the caller's launch keys.
4+
* A scope: 'launch' diagnostic is about the launch as a whole (issue #796) and
5+
* must reach the warning with no caller key to match.
6+
*/
7+
import { describe, it, expect } from 'vitest';
8+
import type { IDebugAdapter, LanguageSpecificLaunchConfig } from '@debugmcp/shared';
9+
import { collectLaunchConfigNotices } from '../../../../../src/session/launch/launch-config-diagnostics.js';
10+
11+
describe('collectLaunchConfigNotices — launch-scoped diagnostics (issue #796)', () => {
12+
it('renders a scope: launch diagnostic as-is even though no caller key matches it', () => {
13+
const adapter = {
14+
supportedLaunchKeys: ['program'],
15+
consumeLaunchConfigDiagnostics: () => [
16+
{ key: 'exitcode-shim.cjs', scope: 'launch' as const, message: 'exit codes will not be captured: exitcode-shim.cjs not found (looked in /a, /b)' }
17+
]
18+
} as unknown as IDebugAdapter;
19+
20+
const notices = collectLaunchConfigNotices(adapter, {}, { program: '/app.js' } as LanguageSpecificLaunchConfig);
21+
22+
expect(notices).toEqual(['exit codes will not be captured: exitcode-shim.cjs not found (looked in /a, /b)']);
23+
});
24+
25+
it('still matches ordinary diagnostics on the caller key only', () => {
26+
const adapter = {
27+
supportedLaunchKeys: ['program'],
28+
consumeLaunchConfigDiagnostics: () => [{ key: 'runtimeArgs', message: 'expected an array of strings' }]
29+
} as unknown as IDebugAdapter;
30+
31+
expect(collectLaunchConfigNotices(adapter, {}, { program: '/app.js' } as LanguageSpecificLaunchConfig)).toEqual([]);
32+
});
33+
});

0 commit comments

Comments
 (0)