Skip to content

Commit a42e222

Browse files
authored
fix(vscode): resolve @rstest/core from the rstack package for bridged projects (#23)
A folder driven through rstack's Rstest config shim resolved `@rstest/core` with a node_modules walk-up from the config directory. Under pnpm's isolated layout a project that depends on `rstack` alone has no `node_modules/@rstest/core` — the core sits beside `rstack` in the virtual store — so every bridged project failed with "Cannot find @rstest/core" and Rstest was unusable. The lint stack already anchors its core walk-up at the resolved `rstack` directory; the test stack now does the same: - `resolveRstackShim` returns the resolved `rstack` package directory and the bridge threads it into `RstestApi` as the default `@rstest/core` and CLI-bin resolution anchor. Native projects keep the cwd anchor and `rstack.rstest.rstestPackagePath` still overrides. - A missing core is reported once: `createChildProcess` throws a marker error the project initializer no longer re-logs. - E2E fixtures install with pnpm's default isolated layout — the `publicHoistPattern` flags for `@rslint/core` / `@rstest/core` are gone, since they hid this bug (and, without them, the walk-up used to escape the fixture into this repo's own dev copy). The rstest bridge suite now asserts the resolved core lives inside the fixture; the lint bridge suite stages a project-visible `@rslint/core` only for its native-ownership transition test. - Unit tests pin the anchor per project kind (native / bridged / configured) against a `.pnpm`-shaped tree.
1 parent ba497ae commit a42e222

13 files changed

Lines changed: 277 additions & 72 deletions

File tree

packages/vscode/AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten
3939
- **Yarn Plug'n'Play is unsupported by decision, extension-wide.** Every stack resolves through physical `node_modules` (`shared/packageResolve.ts`, `resolution.ts`'s rstack → `@rslint/core` chain, the fmt bin probe, the rstest package lookup) and the lint worker's own `createRequire` from the core directory does too. Lint once carried a `.pnp.cjs` branch for the find-`@rslint/core` hop only; nothing after that hop (config evaluation, plugin resolution, the other stacks) had PnP hooks, so it never produced a working folder, and upstream removed its own PnP path in the same refactor that introduced `corePath`. Real support would be a PnP editor-SDK-shaped project across all three stacks, not a resolver branch — do not reintroduce one.
4040
- **A Lint runtime lives as long as a document needs it, and a folder with none is `running: idle`.** Since the #1617 sync, `RuntimeManager` refcounts each runtime by open document: the first document to resolve a core starts one, the last to release it closes it, so a detected folder with nothing open holds zero workers and zero Go processes. That folder still reports `running` — with the detail `idle` — because it is live and will start a runtime on the next `didOpen`; do **not** add a `StackState` kind for it (the shell's status bar and `when` clauses read the kinds, and idle is not a kind of health). A folder's state is the **worst of** its runtimes plus any document whose core resolution currently fails (last-good: that document keeps the runtime it already had), so one failing core is never masked by a healthy sibling — the same invariant fmt pins across folders, applied inside one and across them alike (lint's rank table matches fmt's: `disabled` there means "no `rstack`", not the kill switch). Triggers: the shell's detection pass (which already covers lockfiles) plus one lint-owned watcher on `node_modules/@rslint/core/package.json` — upstream's glob minus the lockfiles detection owns. Failures report through the status only: upstream's `window.showWarningMessage` is dropped, since stacks own no UI chrome. Consequently `whenStackActive('rslint')` means "the controller registered its folders", not "a server is up" — E2E suites open a document and await diagnostics.
4141
- The lint worker is deliberately vscode-free so it can move upstream whole. It takes explicit `--core` / `--config` native paths, writes logs only to stderr because stdout is LSP, and owns the Go child plus config/plugin lifecycles. Config edits use `rslint/configRefresh` with the same pinned path; a native ↔ bridged ownership change replaces the whole folder runtime because protocol 2 locks that choice for the process lifetime.
42-
- The test × `rstack.config.*` bridge stays thin on purpose: it points the upstream machinery at rstack's shipped shim and lets the shim interpret the config inside the worker, same as the CLI. Never re-implement rstack config semantics in the extension.
42+
- The test × `rstack.config.*` bridge stays thin on purpose: it points the upstream machinery at rstack's shipped shim and lets the shim interpret the config inside the worker, same as the CLI. Bridged projects resolve `@rstest/core` from the resolved rstack package directory, mirroring lint, so rstack's dependency remains visible under isolated installs. Never re-implement rstack config semantics in the extension.
4343
- The fmt stack is an LSP client: one `rs fmt --lsp` server per detected workspace folder, spawned at the **folder root** even when a deeper `rstack.config.*` exists. Deepest-config-wins was removed deliberately — `rs fmt` loads one config from its cwd with no upward walk, so anchoring deeper made the editor disagree with `rs fmt` in a terminal; a subproject that needs its own fmt config becomes its own workspace folder. The stack registers **no** `DocumentFormattingEditProvider`: the client registers the provider from the server's `documentFormattingProvider` capability, and adding one by hand would double-register. A config create/change/delete **restarts** the owning folder's server (the server caches its config for its process lifetime and has no config-change message), which is also why the stack watches `RSTACK_CONFIG_GLOB` itself instead of relying on detection — a detection signature records which config files exist, not their contents. A detection pass keeps healthy servers and restarts failed ones in place (`isFailedFmtState`) — lockfile events notify even when the folder set is unchanged, precisely so a completed install or upgrade is retried without a manual restart. There is no stdin fallback below `SUPPORT_MATRIX.rstack`; that is a version gate, not an omission. **Nested workspace folders are a documented limitation, by decision**: when a folder and its subdirectory are both workspace folders and both detect fmt, the parent's per-folder selector also matches the nested folder's files, and which server VS Code hands the request to is not defined — the supported shape is subprojects as _sibling_ workspace folders (or only the subproject opened), not parent-plus-child. Routing (lint's `WorkspaceDocumentRouter` shape) was considered and deferred. Why all of it: `docs/adr/0002-fmt-lsp-on-user-node-runtime.md`.
4444
- fmt importing `stacks/lint/LanguageServerProcessOwner.ts` is not a refactor across the copies: that file has no lint imports and no lint behaviour, it only owns the native children of one language client — including the ones vscode-languageclient's automatic restart creates, which is exactly the leak an ad-hoc copy would reintroduce. Lint's `ManagedLanguageClient` is _restated_ in `stacks/fmt/index.ts` instead, because importing it from `Rslint.ts` would couple fmt to the lint stack's runtime graph. Keep that line where it is: shared process ownership yes, shared stack runtime no.
4545
- The VSIX is platform-targeted for exactly one reason: the test stack's AST collection loads a native parser binding. Do not add another native dependency — it multiplies the release matrix.

packages/vscode/e2e/lint/suite-bridge/bridge.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { spawnSync } from 'node:child_process';
33
import fs from 'node:fs';
44
import path from 'node:path';
55
import * as vscode from 'vscode';
6+
import { findPackageJsonUncached } from '../../../src/shared/packageResolve';
67
import {
78
getRslintDiagnostics,
89
waitForRslintDiagnostics,
@@ -60,12 +61,38 @@ suite('Rstack lint bridge', function () {
6061
const root = workspaceRoot();
6162
const rstackConfigPath = path.join(root, 'rstack.config.ts');
6263
const nativeConfigPath = path.join(root, nativeConfigName);
64+
const nativeNodeModulesPath = path.join(root, 'node_modules');
6365
const markerPath = path.join(root, '.lint-worker-config.json');
6466
const originalConfig = fs.readFileSync(rstackConfigPath, 'utf8');
6567

68+
function installNativeCore(): void {
69+
// The fixture intentionally depends only on rstack, so the first two tests
70+
// prove bridged resolution against pnpm's isolated transitive dependency.
71+
// A native config, however, needs its own project-visible @rslint/core.
72+
// Stage that install only for the ownership-transition test, inside the
73+
// sandbox workspace copy (torn down below): link the core rstack itself
74+
// resolves — the same walk the extension performs — so no store layout is
75+
// assumed here.
76+
const rstackPackageJson = findPackageJsonUncached('rstack', root);
77+
assert.ok(rstackPackageJson, 'the fixture install should provide rstack');
78+
const corePackageJson = findPackageJsonUncached(
79+
'@rslint/core',
80+
path.dirname(rstackPackageJson),
81+
);
82+
assert.ok(corePackageJson, 'rstack should resolve its @rslint/core');
83+
const scopeDir = path.join(nativeNodeModulesPath, '@rslint');
84+
fs.mkdirSync(scopeDir, { recursive: true });
85+
fs.symlinkSync(
86+
path.dirname(corePackageJson),
87+
path.join(scopeDir, 'core'),
88+
process.platform === 'win32' ? 'junction' : 'dir',
89+
);
90+
}
91+
6692
teardown(async () => {
6793
fs.writeFileSync(rstackConfigPath, originalConfig, 'utf8');
6894
fs.rmSync(nativeConfigPath, { force: true });
95+
fs.rmSync(nativeNodeModulesPath, { recursive: true, force: true });
6996
fs.rmSync(markerPath, { force: true });
7097
const document = vscode.workspace.textDocuments.find(
7198
(candidate) =>
@@ -142,6 +169,7 @@ suite('Rstack lint bridge', function () {
142169
const document = await openLintTarget();
143170
await waitForRslintDiagnostics(document, hasNoDebugger);
144171

172+
installNativeCore();
145173
fs.writeFileSync(
146174
nativeConfigPath,
147175
`export default [{ rules: { 'no-debugger': 'off' } }];\n`,

packages/vscode/e2e/rstest/suite/bridge.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
// detection change can deregister and re-register the stack, which publishes a
1616
// fresh `TestController` (same reason as `workspace.test.ts`).
1717
import assert from 'node:assert';
18+
import fs from 'node:fs';
1819
import path from 'node:path';
1920
import vscode from 'vscode';
2021
import {
@@ -116,6 +117,19 @@ suite('Rstack bridge suite', () => {
116117
{ label: 'trims a string' },
117118
]);
118119
});
120+
121+
const sourceUri = vscode.Uri.file(
122+
path.join(RSTACK_FIXTURE, 'rstack.config.ts'),
123+
).toString();
124+
const rstestPath = currentRstestExports().getResolvedRstestPath(sourceUri);
125+
assert.ok(rstestPath, 'the bridged project should resolve @rstest/core');
126+
// The resolved path is realpath'd; compare against the physical fixture.
127+
assert.ok(
128+
rstestPath.startsWith(
129+
path.join(fs.realpathSync(RSTACK_FIXTURE), 'node_modules'),
130+
),
131+
`expected the fixture's own @rstest/core, got: ${rstestPath}`,
132+
);
119133
});
120134

121135
test('runs bridged tests through the rstack config shim', async () => {

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+
getResolvedRstestPath: (sourceUri: string) => string | undefined;
1718
startTestRun: (
1819
request: vscode.TestRunRequest,
1920
token: vscode.CancellationToken,

packages/vscode/e2e/setupFixtures.mjs

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ const install = (name) => {
4444
throw new Error(`E2E fixture ${name} has no package.json at ${cwd}`);
4545
}
4646
console.log(`[e2e] installing fixture: ${name}`);
47+
// Keep pnpm's default isolated layout. In the rstack fixture the tool cores
48+
// are transitive dependencies beside rstack in the virtual store, matching
49+
// the layout users get rather than masking resolution bugs with public
50+
// hoisting.
4751
const result = spawnSync(
4852
pnpmCommand,
4953
[
@@ -55,9 +59,9 @@ const install = (name) => {
5559
// the moment a patch release lands.
5660
'--no-frozen-lockfile',
5761
'--prefer-offline',
58-
// Changing a fixture's install config (its hoist patterns, say) makes
59-
// pnpm want to purge `node_modules`, which it refuses to do without a
60-
// TTY. The directory is disposable.
62+
// Changing a fixture's install config makes pnpm want to purge
63+
// `node_modules`, which it refuses to do without a TTY. The directory is
64+
// disposable.
6165
'--config.confirmModulesPurge=false',
6266
// Fixtures deliberately install pinned published versions of the Rstack
6367
// toolchain, which are often hours old — disable pnpm's
@@ -71,20 +75,6 @@ const install = (name) => {
7175
// published packages exactly like a user project would, so run their
7276
// build scripts as-is.
7377
'--config.dangerouslyAllowAllBuilds=true',
74-
// `@rslint/core` / `@rstest/core` may reach a fixture only as transitive
75-
// dependencies of `rstack` (the rstack fixture depends on `rstack`
76-
// alone), yet the extension resolves them with a node_modules walk-up
77-
// from the project dir — which pnpm's isolated store defeats: the
78-
// walk-up would climb out of the fixture and silently find THIS REPO's
79-
// dev copies instead of the published ones. Public-hoisting the two
80-
// reproduces the npm/Yarn layout the extension is designed against, and
81-
// is inert for fixtures that already depend on them directly. It must
82-
// be a CLI flag: pnpm 11 no longer reads `public-hoist-pattern` from a
83-
// fixture-local `.npmrc` (verified — it lands as an empty
84-
// `publicHoistPattern` in `.modules.yaml`), and `--ignore-workspace`
85-
// also ignores a local pnpm-workspace.yaml.
86-
'--config.publicHoistPattern=@rslint/core',
87-
'--config.publicHoistPattern=@rstest/core',
8878
],
8979
{
9080
cwd,

packages/vscode/src/stacks/test/bridge.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@ import { status } from './status';
3131
* the worker's spawn cwd is the only anchor it has. That is why the synthesized
3232
* `Project` must carry an explicit cwd (adaptation #5): pointing a `Project` at
3333
* the shim without it would cwd the worker into `node_modules/rstack/dist/`,
34-
* where the probe finds nothing and `@rstest/core` would resolve from the wrong
35-
* root.
34+
* where the probe finds nothing. Package resolution is anchored separately at
35+
* the resolved `rstack` directory, where package managers such as pnpm install
36+
* rstack's `@rstest/core` dependency.
3637
*/
3738

3839
/** Relative to the `rstack` package root. Same file `rs test` injects. */
@@ -41,6 +42,8 @@ const SHIM_RELATIVE_PATH = path.join('dist', 'rstestConfig.js');
4142
export type RstackShim = {
4243
/** Absolute path of `<rstack>/dist/rstestConfig.js`. */
4344
readonly configFilePath: string;
45+
/** Absolute path of the resolved `rstack` package root. */
46+
readonly packageDirectory: string;
4447
/** The installed `rstack` version, when it could be read. */
4548
readonly version?: string;
4649
};
@@ -76,10 +79,8 @@ export function resolveRstackShim(
7679
return undefined;
7780
}
7881

79-
const configFilePath = path.join(
80-
path.dirname(packageJsonPath),
81-
SHIM_RELATIVE_PATH,
82-
);
82+
const packageDirectory = path.dirname(packageJsonPath);
83+
const configFilePath = path.join(packageDirectory, SHIM_RELATIVE_PATH);
8384
if (!existsSync(configFilePath)) {
8485
if (!silent) {
8586
logger.error(
@@ -106,8 +107,9 @@ export function resolveRstackShim(
106107

107108
logger.debug('Resolved the rstack Rstest config shim', {
108109
configFilePath,
110+
packageDirectory,
109111
version,
110112
});
111113
status.versionOk(configDir);
112-
return { configFilePath, version };
114+
return { configFilePath, packageDirectory, version };
113115
}

packages/vscode/src/stacks/test/coreResolution.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,18 @@
1010
* does not resolve is a setting the user has to fix, so it is notified.
1111
*/
1212

13+
/**
14+
* Resolution failed after the actionable error was already logged or shown.
15+
* Callers still reject so project initialization stops, but must not report the
16+
* same failure again.
17+
*/
18+
export class ReportedRstestResolutionError extends Error {
19+
constructor() {
20+
super('Failed to resolve rstest path');
21+
this.name = 'ReportedRstestResolutionError';
22+
}
23+
}
24+
1325
// Whether `specifier` itself is what could not be found. `MODULE_NOT_FOUND`
1426
// alone is too broad: a package that is installed but whose entry file is gone
1527
// (an interrupted install, or a workspace link that has not been built) throws

packages/vscode/src/stacks/test/index.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,16 +73,24 @@ class Rstest implements vscode.Disposable {
7373
/**
7474
* What upstream's `activate()` effectively exported (the `Rstest` instance):
7575
* the E2E suites (`e2e/rstest/`) consume `testController`, `runProfile`
76-
* and `startTestRun`. The shell republishes this object through the
77-
* extension's public exports (`RstackExtensionExports.whenStackActive`).
78-
* All three values are stable for the lifetime of one registration; a
79-
* re-registration publishes a fresh object.
76+
* and `startTestRun`, plus the repo-only resolved-path probe used by bridge
77+
* coverage. The shell republishes this object through the extension's public
78+
* exports (`RstackExtensionExports.whenStackActive`). These values are stable
79+
* for the lifetime of one registration; a re-registration publishes a fresh
80+
* object.
8081
*/
8182
buildExports(): Record<string, unknown> {
8283
return {
8384
testController: this.ctrl,
8485
runProfile: this.runProfile,
8586
startTestRun: this.startTestRun,
87+
getResolvedRstestPath: (sourceUri: string) => {
88+
for (const workspace of this.workspaces.values()) {
89+
const project = workspace.projects.get(sourceUri);
90+
if (project) return project.api.resolvedRstestPath;
91+
}
92+
return undefined;
93+
},
8694
};
8795
}
8896

packages/vscode/src/stacks/test/master.ts

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
formatConfiguredCoreNotFoundMessage,
2222
formatCoreNotFoundMessage,
2323
isModuleNotFoundError,
24+
ReportedRstestResolutionError,
2425
} from './coreResolution';
2526
import type { RstestDiagnostics } from './diagnostics';
2627
import type { TestErrorStore } from './errorStore';
@@ -117,12 +118,13 @@ export class RstestApi {
117118
// restart — see `reportNodeRuntimeIssue` and the spawn abort in
118119
// `createChildProcess`.
119120
private disposed = false;
121+
private lastResolvedRstestPath?: string;
120122

121123
constructor(
122124
private workspace: vscode.WorkspaceFolder,
123125
/**
124-
* The worker spawn cwd, the `@rstest/core` resolution root, the terminal
125-
* cwd and the base the terminal's `-c` path is relativized against.
126+
* The worker spawn cwd, the terminal cwd and the base the terminal's `-c`
127+
* path is relativized against.
126128
*
127129
* The worker-cwd decoupling adaptation: upstream derives this from
128130
* `dirname(configFilePath)` inside `Project`. It is now passed in, so the
@@ -134,8 +136,19 @@ export class RstestApi {
134136
private cwd: string,
135137
private configFilePath: string,
136138
private project: Project,
139+
/**
140+
* Where the default `@rstest/core` (and CLI bin) walk-up starts. Chosen
141+
* by `Project` — see `ProjectSource.rstestResolutionDir`; an explicit
142+
* `rstestPackagePath` bypasses it.
143+
*/
144+
private rstestResolutionDir: string,
137145
) {}
138146

147+
/** E2E-only probe (`buildExports`): the last successfully resolved `@rstest/core` entry. */
148+
get resolvedRstestPath(): string | undefined {
149+
return this.lastResolvedRstestPath;
150+
}
151+
139152
/**
140153
* The failure-latch key for this master's status reports. The project's
141154
* source URI is unique (the projects map is keyed by it), unlike `cwd`,
@@ -300,7 +313,7 @@ export class RstestApi {
300313
formatConfiguredCoreNotFoundMessage(configuredPackagePath),
301314
);
302315
}
303-
logger.error(formatCoreNotFoundMessage(this.cwd));
316+
logger.error(formatCoreNotFoundMessage(fromDir));
304317
return undefined;
305318
}
306319
}
@@ -340,12 +353,12 @@ export class RstestApi {
340353
// answer, while the bare specifier keeps the exports map honored.
341354
const found = findPackageJsonUncached(
342355
dirname(CORE_PACKAGE_JSON),
343-
this.cwd,
356+
this.rstestResolutionDir,
344357
);
345358
if (!found) {
346359
// The normal state of a repository whose dependencies are not
347360
// installed yet: output channel only, never a notification.
348-
logger.error(formatCoreNotFoundMessage(this.cwd));
361+
logger.error(formatCoreNotFoundMessage(this.rstestResolutionDir));
349362
return '';
350363
}
351364
corePackageJsonPath = found;
@@ -387,6 +400,7 @@ export class RstestApi {
387400
}
388401
}
389402

403+
this.lastResolvedRstestPath = nodeExport;
390404
return nodeExport;
391405
} catch (e) {
392406
vscode.window.showErrorMessage(toErrorMessage(e));
@@ -406,9 +420,11 @@ export class RstestApi {
406420
// Same uncached lookup as the worker resolution above.
407421
pkgJsonPath = findPackageJsonUncached(
408422
dirname(CORE_PACKAGE_JSON),
409-
this.cwd,
423+
this.rstestResolutionDir,
410424
);
411-
if (!pkgJsonPath) logger.error(formatCoreNotFoundMessage(this.cwd));
425+
if (!pkgJsonPath) {
426+
logger.error(formatCoreNotFoundMessage(this.rstestResolutionDir));
427+
}
412428
}
413429
if (!pkgJsonPath) return undefined;
414430
const pkg = (readPackageJson(pkgJsonPath) ?? {}) as {
@@ -613,7 +629,7 @@ export class RstestApi {
613629
}
614630
const rstestPath = this.resolveRstestPath();
615631
if (!rstestPath) {
616-
throw new Error('Failed to resolve rstest path');
632+
throw new ReportedRstestResolutionError();
617633
}
618634
const debuggerPort = getConfigValue('debuggerPort', this.workspace);
619635
const debuggerAddress = getConfigValue('debuggerAddress', this.workspace);

0 commit comments

Comments
 (0)