Skip to content

Commit f5741b1

Browse files
authored
fix(vscode)!: launch the Rstest worker under js-debug when debugging (#76)
* fix(vscode): launch the Rstest worker under js-debug when debugging Let js-debug own the worker and pool processes, using a debug-only local socket for JSON RPC while normal runs retain Node IPC. Bound startup and close the session on cancellation, transport failure, or worker disposal. Cover real breakpoints with port 9229 occupied and verify process cleanup after continuing, cancelling, and stopping the debug session. BREAKING CHANGE: remove rstack.rstest.debuggerPort and rstack.rstest.debuggerAddress without aliases or migration. js-debug manages inspector endpoints automatically. Amp-Thread-ID: T-01a0ec42-b8e2-71ba-b623-05b375f9e885 * refactor(vscode): simplify the debug launch worker Remove redundant launch state, listeners, and timeout handling. Share the debug socket environment key and keep unit coverage focused on behavior not already exercised by the real debug E2E suite. Record debugger-owned test processes as an upstream adaptation. Amp-Thread-ID: T-01a0ec42-b8e2-71ba-b623-05b375f9e885 * fix(vscode): authenticate the debug RPC socket * test(vscode): tolerate expected socket resets in debug tests * fix(vscode): handle resets on the debug RPC readline interface * fix(vscode): report unexpected debug worker exits as crashed
1 parent 7bff94a commit f5741b1

13 files changed

Lines changed: 787 additions & 138 deletions

File tree

‎packages/vscode/AGENTS.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten
55
## The copies are intentional
66

77
- `stacks/lint` and `stacks/test` are deliberate near-verbatim copies of the upstream extensions, kept close to upstream so changes can be synced by diffing. Do NOT deduplicate or refactor across the two stacks — the duplication is the point; consolidation is a later, explicit phase.
8-
- The copies diverge from upstream in exactly ten ways (the "adaptations" below). When syncing upstream, preserve them. An eleventh divergence is either a bug or must be added to this list.
8+
- The copies diverge from upstream in exactly eleven ways (the "adaptations" below). When syncing upstream, preserve them. A twelfth divergence is either a bug or must be added to this list.
99
- **Tracked upstream state.** `stacks/lint` tracks web-infra-dev/rslint `packages/vscode-extension` at **e5d76242** (0.9.3); `stacks/test` tracks web-infra-dev/rstest `packages/vscode` at **41daaa1b** (0.12.2). Additional ports: Rslint **84f9c9b5** trace wording and **72cd2b1f** fixAll waits; Rstest **988f8e1d** per-bundle license notices, with explicit native-binding inclusion and no network license-text replenishment. We replace upstream's plugin-host failure toast with status (adaptations 4/7). Diff `CoreResolver.ts`, `RuntimeManager.ts`, `WorkspaceDocumentRouter.ts`, and `Rslint.ts` on future syncs. The rstest#1888 IPC port uses default JSON instead of advanced serialization: values must survive a JSON round-trip, and both `createBirpc` calls spread `rpcErrorCodec` (`stacks/test/shared/rpc.ts`) to preserve thrown errors. Failed-project retry and worker cleanup remain ahead of upstream.
1010
- **Ahead of upstream — offer these back when syncing** (bug fixes, not adaptations): (1) `RuntimeManager.reconcile` resolves the document's core **before** sweeping pending uses (`planDocumentCore`), so a reconcile landing on the key a pending start is already producing adopts that start instead of tearing it down mid-`initialize` — the teardown made vscode-languageclient force-notify ("couldn't create connection to server") whenever the register-time pass, a detection change and `didOpen` landed inside one worker startup window (`tests/stacks/lint/runtimeManager.test.ts`). (2) `Rslint.close()` gives a still-Starting language client a bounded chance to settle before tearing down its transport, so a legitimate mid-start close (document closed during start, core key changed) stops cleanly instead of triggering the same force-notified toasts. (3) The registry-harness E2E gives its never-settling startup operation 500ms to begin and accepts only the in-flight timeout message, so a stalled runner cannot satisfy the assertion through the already-expired path (`e2e/lint/suite/registry-harness.test.ts`). (4) `Project.retryFailedConfig()` keeps a failed Rstest project and retries its config evaluation in place with one single-flight promise, so repeated dependency-change passes neither overlap workers nor repeat an unchanged not-installed warning. (5) `RuntimeManager` retires a stopped client even when its resolved key is unchanged. The existing closing barrier and pending-use adoption share one replacement across documents; running and starting clients remain untouched (`tests/stacks/lint/runtimeManager.test.ts`).
1111

12-
## The ten adaptations
12+
## The eleven adaptations
1313

1414
1. **Shell activation** — stacks never self-activate; `register()` returns fast and never blocks on starting a server/worker.
1515
2. **Namespace** — everything user-visible is `rstack.*`. Legacy `rslint.*` / `rstest.*` settings and command ids are not read, aliased or migrated (breaking old settings and keybindings was an accepted cost).
@@ -23,6 +23,8 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten
2323

2424
10. **Test file ownership** (test) — publication mirrors the CLI per project. When a request carries the same file or the same case from several projects (merged gutter, run-at-cursor, run-current-file; all three profiles reach `startTestRun`), only items of the deepest project root run and the others are reported skipped to clear stale merged gutter state — implemented in `runRouting.ts`, `index.ts` (`startTestRun`), and `master.ts` (`RstestApi.project` made public). Explicit single-project selections, project/folder/Run All, and `rstack.rstest.runInTerminal` (VS Code supplies one explicitly picked item) keep CLI scope.
2525

26+
11. **Debugger-owned test processes** (test) — debug runs use js-debug `launch` with child-process auto-attach instead of upstream's `--inspect-wait` plus `attach`. js-debug owns inspector endpoints and process teardown; `debuggerPort` / `debuggerAddress` are removed without migration. `debugWorker.ts` owns the session and a debug-only local socket carrying JSON birpc messages (`shared/socketRpc.ts`) after a first-line per-launch secret authenticates the worker; normal runs retain Node IPC. Preserve the 9229-occupied breakpoint and continue/cancel/stop cleanup regression in `e2e/rstest/suite/debug.test.ts` when syncing.
27+
2628
## Rules
2729

2830
- **Pre-1.0.0 the extension breaks freely.** No compatibility is owed with earlier unpublished states of this extension — settings, command ids and behavior may change without deprecation paths, and dead compat code for them is removed, not kept. No settings migration exists either — not for earlier states of this extension, and not for the two retired standalone extensions (removed in #15; users re-enter their settings under `rstack.*`). Testing and fixtures track only the latest published releases, pinned exactly and bumped by Renovate; a green E2E run speaks only for those releases. `SUPPORT_MATRIX` floors are the minimum versions the extension accepts: each entry is the lowest release evidence shows works with the current code, and its comment records that evidence. Move a floor only when a change makes older releases stop working, never because a devDependency or fixture moved. Raising a floor needs no transition story; the status names the required version.

‎packages/vscode/README.md‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,6 @@ All settings live under the unified `rstack.*` namespace. There are no `rslint.*
9090
| `rstack.rstest.debugNodeEnv` | `null` | Extra env when debugging tests. |
9191
| `rstack.rstest.debugExclude` | `["<node_internals>/**"]` | Debug `skipFiles`. |
9292
| `rstack.rstest.debugOutFiles` | `[]` | Debug `outFiles`. |
93-
| `rstack.rstest.debuggerPort` | — | Debugger port. |
94-
| `rstack.rstest.debuggerAddress` | — | Debugger address. |
9593
| `rstack.rstest.terminalShellPath` | — | Shell used by **Run in Terminal**. |
9694
| `rstack.rstest.terminalShellArgs` | `[]` | Shell args for **Run in Terminal**. |
9795
| `rstack.fmt.enable` | `true` | Enable/disable the formatter integration. |
Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
import assert from 'node:assert';
2+
import net from 'node:net';
3+
import path from 'node:path';
4+
import vscode from 'vscode';
5+
import {
6+
FIXTURES_ROOT,
7+
getRstestExports,
8+
getTestItemByLabels,
9+
waitFor,
10+
} from './helpers';
11+
12+
suite('Rstest debug launch', () => {
13+
for (const finish of ['continue', 'cancel', 'stop'] as const) {
14+
test(`hits a breakpoint with port 9229 occupied and cleans up after ${finish}`, async () => {
15+
const occupied = net.createServer();
16+
await new Promise<void>((resolve, reject) => {
17+
occupied.once('error', reject);
18+
occupied.listen(9229, '127.0.0.1', resolve);
19+
});
20+
const cancellation = new vscode.CancellationTokenSource();
21+
const sessions = new Map<string, vscode.DebugSession>();
22+
const processIds = new Set<number>();
23+
const disposables: vscode.Disposable[] = [];
24+
const file = path.join(FIXTURES_ROOT, 'workspace-1/test/index.test.ts');
25+
const breakpoint = new vscode.SourceBreakpoint(
26+
new vscode.Location(vscode.Uri.file(file), new vscode.Position(5, 0)),
27+
);
28+
let stopped:
29+
{ session: vscode.DebugSession; threadId: number } | undefined;
30+
const passed: vscode.TestItem[] = [];
31+
let ended = false;
32+
let running: Promise<void> | undefined;
33+
try {
34+
disposables.push(
35+
vscode.debug.onDidStartDebugSession((session) => {
36+
sessions.set(session.id, session);
37+
}),
38+
vscode.debug.onDidTerminateDebugSession((session) =>
39+
sessions.delete(session.id),
40+
),
41+
vscode.debug.registerDebugAdapterTrackerFactory('*', {
42+
// js-debug resolves the public 'node' type to 'pwa-node'.
43+
createDebugAdapterTracker: (session) => ({
44+
onDidSendMessage: (message) => {
45+
if (
46+
message.type === 'event' &&
47+
message.event === 'stopped' &&
48+
message.body.reason === 'breakpoint'
49+
) {
50+
stopped = { session, threadId: message.body.threadId };
51+
}
52+
},
53+
}),
54+
}),
55+
);
56+
vscode.debug.addBreakpoints([breakpoint]);
57+
const api = await getRstestExports();
58+
const item = await waitFor(() =>
59+
getTestItemByLabels(api.testController.items, [
60+
'test',
61+
'index.test.ts',
62+
]),
63+
);
64+
const request = new vscode.TestRunRequest(
65+
[item],
66+
undefined,
67+
api.debugProfile,
68+
);
69+
// A real TestRun is required for startDebugging's testRun association.
70+
running = api.startTestRun(
71+
request,
72+
cancellation.token,
73+
false,
74+
(runRequest) => {
75+
const run = api.testController.createTestRun(runRequest);
76+
const originalPassed = run.passed.bind(run);
77+
const originalEnd = run.end.bind(run);
78+
run.passed = (test, duration) => {
79+
passed.push(test);
80+
originalPassed(test, duration);
81+
};
82+
run.end = () => {
83+
ended = true;
84+
originalEnd();
85+
};
86+
return run;
87+
},
88+
);
89+
await waitFor(
90+
() => assert.ok(stopped, 'debuggee should stop at the breakpoint'),
91+
{ timeoutMs: 60_000 },
92+
);
93+
assert.ok(stopped);
94+
const stack = await stopped.session.customRequest('stackTrace', {
95+
threadId: stopped.threadId,
96+
});
97+
assert.equal(
98+
path.normalize(stack.stackFrames[0].source.path),
99+
path.normalize(file),
100+
);
101+
assert.equal(stack.stackFrames[0].line, 6);
102+
const root = [...sessions.values()].find(
103+
(session) => session.name === 'Rstest Debug',
104+
);
105+
assert.ok(root);
106+
for (const session of sessions.values()) {
107+
if (session.id === root.id) continue;
108+
const evaluation = await session.customRequest('evaluate', {
109+
expression: 'process.pid',
110+
context: 'repl',
111+
});
112+
const pid = Number(evaluation.result);
113+
assert.ok(Number.isInteger(pid) && pid > 0, evaluation.result);
114+
processIds.add(pid);
115+
}
116+
assert.ok(
117+
processIds.size >= 2,
118+
'worker and pool child must be debugged',
119+
);
120+
if (finish === 'continue') {
121+
await stopped.session.customRequest('continue', {
122+
threadId: stopped.threadId,
123+
});
124+
} else if (finish === 'cancel') {
125+
cancellation.cancel();
126+
} else {
127+
await vscode.debug.stopDebugging(root);
128+
}
129+
await waitFor(
130+
() => {
131+
assert.equal(ended, true, 'TestRun must end');
132+
if (finish === 'continue') {
133+
assert.deepEqual(passed.map((test) => test.label).sort(), [
134+
'Index',
135+
'index.test.ts',
136+
'should add two numbers correctly',
137+
'should test source code correctly',
138+
]);
139+
}
140+
assert.equal(sessions.size, 0, 'all debug sessions must end');
141+
for (const pid of processIds) {
142+
assert.throws(
143+
() => process.kill(pid, 0),
144+
{ code: 'ESRCH' },
145+
`process ${pid} must exit`,
146+
);
147+
}
148+
},
149+
{ timeoutMs: 60_000 },
150+
);
151+
await running;
152+
} finally {
153+
cancellation.cancel();
154+
vscode.debug.removeBreakpoints([breakpoint]);
155+
await Promise.all(
156+
[...sessions.values()].map((session) =>
157+
vscode.debug.stopDebugging(session),
158+
),
159+
);
160+
for (const disposable of disposables) disposable.dispose();
161+
cancellation.dispose();
162+
await new Promise<void>((resolve) => occupied.close(() => resolve()));
163+
}
164+
});
165+
}
166+
});

‎packages/vscode/e2e/rstest/suite/helpers.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import type { RstackExtensionExports } from '../../../src/types';
1414
export interface RstestExports {
1515
testController: vscode.TestController;
1616
runProfile: vscode.TestRunProfile;
17+
debugProfile: vscode.TestRunProfile;
1718
getResolvedRstestPath: (sourceUri: string) => string | undefined;
1819
startTestRun: (
1920
request: vscode.TestRunRequest,

‎packages/vscode/package.json‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -274,18 +274,6 @@
274274
"scope": "resource",
275275
"markdownDescription": "When source maps are enabled, glob patterns locating the generated JavaScript files (maps to the debug session's `outFiles`)."
276276
},
277-
"rstack.rstest.debuggerPort": {
278-
"order": 11,
279-
"type": "number",
280-
"scope": "resource",
281-
"description": "Port the debugger attaches to. Defaults to Node's inspector behavior (9229, or a free port if taken)."
282-
},
283-
"rstack.rstest.debuggerAddress": {
284-
"order": 12,
285-
"type": "string",
286-
"scope": "resource",
287-
"description": "TCP/IP address the debugger attaches to. Defaults to localhost."
288-
},
289277
"rstack.rstest.terminalShellPath": {
290278
"order": 13,
291279
"type": "string",

‎packages/vscode/src/stacks/test/config.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import {
44
fallback,
55
type InferOutput,
66
literal,
7-
number,
87
object,
98
optional,
109
parse,
@@ -30,8 +29,6 @@ const configSchema = object({
3029
nodeExecArgs: fallback(array(string()), []),
3130
nodeEnv: fallback(optional(record(string(), string())), undefined),
3231
debugNodeEnv: fallback(optional(record(string(), string())), undefined),
33-
debuggerPort: fallback(optional(number()), undefined),
34-
debuggerAddress: fallback(optional(string()), undefined),
3532
debugExclude: fallback(array(string()), ['<node_internals>/**']),
3633
debugOutFiles: fallback(array(string()), []),
3734
configFileGlobPattern: fallback(array(string()), [

0 commit comments

Comments
 (0)