Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/vscode/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten
- **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.
- **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.
- 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.
- 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.
- 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.
- 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`.
- 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.
- 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.
Expand Down
28 changes: 28 additions & 0 deletions packages/vscode/e2e/lint/suite-bridge/bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { spawnSync } from 'node:child_process';
import fs from 'node:fs';
import path from 'node:path';
import * as vscode from 'vscode';
import { findPackageJsonUncached } from '../../../src/shared/packageResolve';
import {
getRslintDiagnostics,
waitForRslintDiagnostics,
Expand Down Expand Up @@ -60,12 +61,38 @@ suite('Rstack lint bridge', function () {
const root = workspaceRoot();
const rstackConfigPath = path.join(root, 'rstack.config.ts');
const nativeConfigPath = path.join(root, nativeConfigName);
const nativeNodeModulesPath = path.join(root, 'node_modules');
const markerPath = path.join(root, '.lint-worker-config.json');
const originalConfig = fs.readFileSync(rstackConfigPath, 'utf8');

function installNativeCore(): void {
// The fixture intentionally depends only on rstack, so the first two tests
// prove bridged resolution against pnpm's isolated transitive dependency.
// A native config, however, needs its own project-visible @rslint/core.
// Stage that install only for the ownership-transition test, inside the
// sandbox workspace copy (torn down below): link the core rstack itself
// resolves — the same walk the extension performs — so no store layout is
// assumed here.
const rstackPackageJson = findPackageJsonUncached('rstack', root);
assert.ok(rstackPackageJson, 'the fixture install should provide rstack');
const corePackageJson = findPackageJsonUncached(
'@rslint/core',
path.dirname(rstackPackageJson),
);
assert.ok(corePackageJson, 'rstack should resolve its @rslint/core');
const scopeDir = path.join(nativeNodeModulesPath, '@rslint');
fs.mkdirSync(scopeDir, { recursive: true });
fs.symlinkSync(
path.dirname(corePackageJson),
path.join(scopeDir, 'core'),
process.platform === 'win32' ? 'junction' : 'dir',
);
}

teardown(async () => {
fs.writeFileSync(rstackConfigPath, originalConfig, 'utf8');
fs.rmSync(nativeConfigPath, { force: true });
fs.rmSync(nativeNodeModulesPath, { recursive: true, force: true });
fs.rmSync(markerPath, { force: true });
const document = vscode.workspace.textDocuments.find(
(candidate) =>
Expand Down Expand Up @@ -142,6 +169,7 @@ suite('Rstack lint bridge', function () {
const document = await openLintTarget();
await waitForRslintDiagnostics(document, hasNoDebugger);

installNativeCore();
fs.writeFileSync(
nativeConfigPath,
`export default [{ rules: { 'no-debugger': 'off' } }];\n`,
Expand Down
14 changes: 14 additions & 0 deletions packages/vscode/e2e/rstest/suite/bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
// detection change can deregister and re-register the stack, which publishes a
// fresh `TestController` (same reason as `workspace.test.ts`).
import assert from 'node:assert';
import fs from 'node:fs';
import path from 'node:path';
import vscode from 'vscode';
import {
Expand Down Expand Up @@ -116,6 +117,19 @@ suite('Rstack bridge suite', () => {
{ label: 'trims a string' },
]);
});

const sourceUri = vscode.Uri.file(
path.join(RSTACK_FIXTURE, 'rstack.config.ts'),
).toString();
const rstestPath = currentRstestExports().getResolvedRstestPath(sourceUri);
assert.ok(rstestPath, 'the bridged project should resolve @rstest/core');
// The resolved path is realpath'd; compare against the physical fixture.
assert.ok(
rstestPath.startsWith(
path.join(fs.realpathSync(RSTACK_FIXTURE), 'node_modules'),
),
`expected the fixture's own @rstest/core, got: ${rstestPath}`,
);
});

test('runs bridged tests through the rstack config shim', async () => {
Expand Down
1 change: 1 addition & 0 deletions packages/vscode/e2e/rstest/suite/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type { RstackExtensionExports } from '../../../src/types';
export interface RstestExports {
testController: vscode.TestController;
runProfile: vscode.TestRunProfile;
getResolvedRstestPath: (sourceUri: string) => string | undefined;
startTestRun: (
request: vscode.TestRunRequest,
token: vscode.CancellationToken,
Expand Down
24 changes: 7 additions & 17 deletions packages/vscode/e2e/setupFixtures.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ const install = (name) => {
throw new Error(`E2E fixture ${name} has no package.json at ${cwd}`);
}
console.log(`[e2e] installing fixture: ${name}`);
// Keep pnpm's default isolated layout. In the rstack fixture the tool cores
// are transitive dependencies beside rstack in the virtual store, matching
// the layout users get rather than masking resolution bugs with public
// hoisting.
const result = spawnSync(
pnpmCommand,
[
Expand All @@ -55,9 +59,9 @@ const install = (name) => {
// the moment a patch release lands.
'--no-frozen-lockfile',
'--prefer-offline',
// Changing a fixture's install config (its hoist patterns, say) makes
// pnpm want to purge `node_modules`, which it refuses to do without a
// TTY. The directory is disposable.
// Changing a fixture's install config makes pnpm want to purge
// `node_modules`, which it refuses to do without a TTY. The directory is
// disposable.
'--config.confirmModulesPurge=false',
// Fixtures deliberately install pinned published versions of the Rstack
// toolchain, which are often hours old — disable pnpm's
Expand All @@ -71,20 +75,6 @@ const install = (name) => {
// published packages exactly like a user project would, so run their
// build scripts as-is.
'--config.dangerouslyAllowAllBuilds=true',
// `@rslint/core` / `@rstest/core` may reach a fixture only as transitive
// dependencies of `rstack` (the rstack fixture depends on `rstack`
// alone), yet the extension resolves them with a node_modules walk-up
// from the project dir — which pnpm's isolated store defeats: the
// walk-up would climb out of the fixture and silently find THIS REPO's
// dev copies instead of the published ones. Public-hoisting the two
// reproduces the npm/Yarn layout the extension is designed against, and
// is inert for fixtures that already depend on them directly. It must
// be a CLI flag: pnpm 11 no longer reads `public-hoist-pattern` from a
// fixture-local `.npmrc` (verified — it lands as an empty
// `publicHoistPattern` in `.modules.yaml`), and `--ignore-workspace`
// also ignores a local pnpm-workspace.yaml.
'--config.publicHoistPattern=@rslint/core',
'--config.publicHoistPattern=@rstest/core',
],
{
cwd,
Expand Down
16 changes: 9 additions & 7 deletions packages/vscode/src/stacks/test/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ import { status } from './status';
* the worker's spawn cwd is the only anchor it has. That is why the synthesized
* `Project` must carry an explicit cwd (adaptation #5): pointing a `Project` at
* the shim without it would cwd the worker into `node_modules/rstack/dist/`,
* where the probe finds nothing and `@rstest/core` would resolve from the wrong
* root.
* where the probe finds nothing. Package resolution is anchored separately at
* the resolved `rstack` directory, where package managers such as pnpm install
* rstack's `@rstest/core` dependency.
*/

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

const configFilePath = path.join(
path.dirname(packageJsonPath),
SHIM_RELATIVE_PATH,
);
const packageDirectory = path.dirname(packageJsonPath);
const configFilePath = path.join(packageDirectory, SHIM_RELATIVE_PATH);
if (!existsSync(configFilePath)) {
if (!silent) {
logger.error(
Expand All @@ -106,8 +107,9 @@ export function resolveRstackShim(

logger.debug('Resolved the rstack Rstest config shim', {
configFilePath,
packageDirectory,
version,
});
status.versionOk(configDir);
return { configFilePath, version };
return { configFilePath, packageDirectory, version };
}
12 changes: 12 additions & 0 deletions packages/vscode/src/stacks/test/coreResolution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,18 @@
* does not resolve is a setting the user has to fix, so it is notified.
*/

/**
* Resolution failed after the actionable error was already logged or shown.
* Callers still reject so project initialization stops, but must not report the
* same failure again.
*/
export class ReportedRstestResolutionError extends Error {
constructor() {
super('Failed to resolve rstest path');
this.name = 'ReportedRstestResolutionError';
}
}

// Whether `specifier` itself is what could not be found. `MODULE_NOT_FOUND`
// alone is too broad: a package that is installed but whose entry file is gone
// (an interrupted install, or a workspace link that has not been built) throws
Expand Down
16 changes: 12 additions & 4 deletions packages/vscode/src/stacks/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,24 @@ class Rstest implements vscode.Disposable {
/**
* What upstream's `activate()` effectively exported (the `Rstest` instance):
* the E2E suites (`e2e/rstest/`) consume `testController`, `runProfile`
* and `startTestRun`. The shell republishes this object through the
* extension's public exports (`RstackExtensionExports.whenStackActive`).
* All three values are stable for the lifetime of one registration; a
* re-registration publishes a fresh object.
* and `startTestRun`, plus the repo-only resolved-path probe used by bridge
* coverage. The shell republishes this object through the extension's public
* exports (`RstackExtensionExports.whenStackActive`). These values are stable
* for the lifetime of one registration; a re-registration publishes a fresh
* object.
*/
buildExports(): Record<string, unknown> {
return {
testController: this.ctrl,
runProfile: this.runProfile,
startTestRun: this.startTestRun,
getResolvedRstestPath: (sourceUri: string) => {
for (const workspace of this.workspaces.values()) {
const project = workspace.projects.get(sourceUri);
if (project) return project.api.resolvedRstestPath;
}
return undefined;
},
};
}

Expand Down
32 changes: 24 additions & 8 deletions packages/vscode/src/stacks/test/master.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
formatConfiguredCoreNotFoundMessage,
formatCoreNotFoundMessage,
isModuleNotFoundError,
ReportedRstestResolutionError,
} from './coreResolution';
import type { RstestDiagnostics } from './diagnostics';
import type { TestErrorStore } from './errorStore';
Expand Down Expand Up @@ -117,12 +118,13 @@ export class RstestApi {
// restart — see `reportNodeRuntimeIssue` and the spawn abort in
// `createChildProcess`.
private disposed = false;
private lastResolvedRstestPath?: string;

constructor(
private workspace: vscode.WorkspaceFolder,
/**
* The worker spawn cwd, the `@rstest/core` resolution root, the terminal
* cwd and the base the terminal's `-c` path is relativized against.
* The worker spawn cwd, the terminal cwd and the base the terminal's `-c`
* path is relativized against.
*
* The worker-cwd decoupling adaptation: upstream derives this from
* `dirname(configFilePath)` inside `Project`. It is now passed in, so the
Expand All @@ -134,8 +136,19 @@ export class RstestApi {
private cwd: string,
private configFilePath: string,
private project: Project,
/**
* Where the default `@rstest/core` (and CLI bin) walk-up starts. Chosen
* by `Project` — see `ProjectSource.rstestResolutionDir`; an explicit
* `rstestPackagePath` bypasses it.
*/
private rstestResolutionDir: string,
) {}

/** E2E-only probe (`buildExports`): the last successfully resolved `@rstest/core` entry. */
get resolvedRstestPath(): string | undefined {
return this.lastResolvedRstestPath;
}

/**
* The failure-latch key for this master's status reports. The project's
* source URI is unique (the projects map is keyed by it), unlike `cwd`,
Expand Down Expand Up @@ -300,7 +313,7 @@ export class RstestApi {
formatConfiguredCoreNotFoundMessage(configuredPackagePath),
);
}
logger.error(formatCoreNotFoundMessage(this.cwd));
logger.error(formatCoreNotFoundMessage(fromDir));
return undefined;
}
}
Expand Down Expand Up @@ -340,12 +353,12 @@ export class RstestApi {
// answer, while the bare specifier keeps the exports map honored.
const found = findPackageJsonUncached(
dirname(CORE_PACKAGE_JSON),
this.cwd,
this.rstestResolutionDir,
);
if (!found) {
// The normal state of a repository whose dependencies are not
// installed yet: output channel only, never a notification.
logger.error(formatCoreNotFoundMessage(this.cwd));
logger.error(formatCoreNotFoundMessage(this.rstestResolutionDir));
return '';
}
corePackageJsonPath = found;
Expand Down Expand Up @@ -387,6 +400,7 @@ export class RstestApi {
}
}

this.lastResolvedRstestPath = nodeExport;
return nodeExport;
} catch (e) {
vscode.window.showErrorMessage(toErrorMessage(e));
Expand All @@ -406,9 +420,11 @@ export class RstestApi {
// Same uncached lookup as the worker resolution above.
pkgJsonPath = findPackageJsonUncached(
dirname(CORE_PACKAGE_JSON),
this.cwd,
this.rstestResolutionDir,
);
if (!pkgJsonPath) logger.error(formatCoreNotFoundMessage(this.cwd));
if (!pkgJsonPath) {
logger.error(formatCoreNotFoundMessage(this.rstestResolutionDir));
}
}
if (!pkgJsonPath) return undefined;
const pkg = (readPackageJson(pkgJsonPath) ?? {}) as {
Expand Down Expand Up @@ -613,7 +629,7 @@ export class RstestApi {
}
const rstestPath = this.resolveRstestPath();
if (!rstestPath) {
throw new Error('Failed to resolve rstest path');
throw new ReportedRstestResolutionError();
}
const debuggerPort = getConfigValue('debuggerPort', this.workspace);
const debuggerAddress = getConfigValue('debuggerAddress', this.workspace);
Expand Down
Loading