Skip to content

Commit 42e6cfa

Browse files
fi3eworkfi3ework
authored
fix(vscode): treat any unresolved bare config import as not installed (#57)
The not-installed classifier proved a bare specifier's package was absent by walking up the physical node_modules from a fixed root (project root, config directory or workspace root). Under pnpm's isolated layout that walk cannot see a plugin's private dependency, so a real missing-subpath error (`dep/missing` where `dep` lives only under the plugin) was reported as "not installed" and the stack went disabled instead of surfacing it. Drop the filesystem lookup. Every Node module-not-found on a bare specifier, subpath included, is now the same disabled state, and the loader's own first line (which names the specifier and the importer) is the one warn line. Recovery is unchanged: the shell's poll already covers disabled states. Relative, absolute and file: specifiers stay real errors. - `classifyMissingDependencyMessage` loses its `resolveFrom` parameter; `missingDependencyCause(code, message)` is the code-gated entry point and `missingDependencyCauseOf(error)` its Error wrapper. - Rslint's config-dependency observer no longer needs `resolveFrom`. - Status wording: "<config> has an import Node cannot resolve".
1 parent 02f4be1 commit 42e6cfa

14 files changed

Lines changed: 81 additions & 146 deletions

File tree

‎packages/vscode/AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten
2727

2828
- **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.
2929
- **The three tools are treated uniformly by default.** Detection, dependency-change retry, restart semantics, version gating and status reporting follow one shared pattern across the lint/test/fmt stacks; a stack diverges only when its tool forces it, and the divergence is recorded here as a gotcha. When adding behavior to one stack, first ask whether it belongs to all three. This is about behavior, not code — the upstream copies still must not be deduplicated.
30-
- **Not installed is a state, not an error — uniformly.** A folder or project whose dependencies are not installed (no `rstack`, no `@rstest/core`, no `@rslint/core`, a config importing a package that is not there) is the normal state of a fresh clone and of scaffolded templates beside their generator (`create-rstack`'s `template-*`, which declare their own dependencies and are never installed). Every stack reports it the same way: a `disabled` status whose reason keeps the restart command as an explicit fallback, one `warn` line per unresolved episode in the output channel without a stack trace, never a `crashed` status and never a notification. The shell owns one 60-second recursive poll while any controller's raw folder/project/runtime state is disabled, crashed or version-mismatched; it enters the existing serialized queue, forces the same detection notification as a lockfile event, and stops when no failed state remains (ADR 0005). A mid-install retry can read half-written `node_modules` and produce a real syntax error; continuing through failed states makes that transient harmless without a provisional-error heuristic. Real errors remain visible in status and Output, deduplicated by message rather than logged every minute. The words come from one place, `shared/notInstalled.ts` (the `formatVersionMismatch` precedent) — each stack keeps its own status machinery, none its own wording; the restart hint is derived from `stackCommandTitle`, which `tests/extension.test.ts` checks against the manifest. Rstest classifies config-import failures in its worker (`missingDependencyCauseOf`: Node's `code`, a bare package specifier, and for a subpath a walk-up proving the package really is absent) because IPC drops the `code`; Rslint makes the same code-gated decision where its worker still has structured loader results and sends a dedicated verdict to the editor; fmt intercepts only the exact `rs fmt cannot format this workspace:` Error notification and applies the shared message classifier. A typo'd relative import or a missing subpath of an installed package stays a real error in all three.
30+
- **Not installed is a state, not an error — uniformly.** A folder or project whose dependencies are not installed (no `rstack`, no `@rstest/core`, no `@rslint/core`, a config importing a package that is not there) is the normal state of a fresh clone and of scaffolded templates beside their generator (`create-rstack`'s `template-*`, which declare their own dependencies and are never installed). Every stack reports it the same way: a `disabled` status whose reason keeps the restart command as an explicit fallback, one `warn` line per unresolved episode in the output channel without a stack trace, never a `crashed` status and never a notification. The shell owns one 60-second recursive poll while any controller's raw folder/project/runtime state is disabled, crashed or version-mismatched; it enters the existing serialized queue, forces the same detection notification as a lockfile event, and stops when no failed state remains (ADR 0005). A mid-install retry can read half-written `node_modules` and produce a real syntax error; continuing through failed states makes that transient harmless without a provisional-error heuristic. Real errors remain visible in status and Output, deduplicated by message rather than logged every minute. The words come from one place, `shared/notInstalled.ts` (the `formatVersionMismatch` precedent) — each stack keeps its own status machinery, none its own wording; the restart hint is derived from `stackCommandTitle`, which `tests/extension.test.ts` checks against the manifest. Rstest classifies config-import failures in its worker because IPC drops the `code`; Rslint makes the same code-gated decision where its worker still has structured loader results and sends a dedicated verdict to the editor; fmt intercepts only the exact `rs fmt cannot format this workspace:` Error notification and applies the shared message classifier. Any Node module-not-found on a bare specifier is the not-installed state, subpath included; the classifier never touches the filesystem (`shared/missingDependency.ts`, #52). Only relative, absolute and `file:` specifiers stay real errors.
3131
- One stack failing to register or crashing must never take another stack (or the shell) down.
3232
- The shell always activates; per-folder config detection decides which stacks start, and re-runs on config/lockfile changes without a window reload. The per-stack enable settings are coarse kill switches only.
3333
- Reconciles and restarts share one serialized queue (`enqueue`); a reconcile leaves a live stack alone, so the restart path — the commands, and the full pass any relevant settings change triggers — is the only thing that rebuilds one. Do not add a second queue.

‎packages/vscode/e2e/lint/suite-jsconfig/config-transaction.test.ts‎

Lines changed: 13 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,13 @@ function loadRequest(transactionId = 'tx-1'): LoadConfigsRequest {
255255
}
256256

257257
suite('LSP config discovery transactions', () => {
258+
const rejectsConfigDependencies: ConstructorParameters<
259+
typeof LspConfigTransactionAdapter
260+
>[4] = {
261+
report: () => assert.fail('unexpected missing dependency'),
262+
reportError: () => assert.fail('unexpected config error'),
263+
};
264+
258265
test('the extension watcher leaves gitignore ownership to Go', () => {
259266
assert.match(CONFIG_REFRESH_WATCH_GLOB, /rslint\.config\.js/);
260267
assert.match(CONFIG_REFRESH_WATCH_GLOB, /rslint\.config\.mjs/);
@@ -282,11 +289,7 @@ suite('LSP config discovery transactions', () => {
282289
pool,
283290
() => 'fingerprint-1',
284291
CONFIG_DISCOVERY_PROTOCOL_VERSION,
285-
{
286-
resolveFrom: (candidate) => candidate.configDirectory,
287-
report: () => assert.fail('unexpected missing dependency'),
288-
reportError: () => assert.fail('unexpected config error'),
289-
},
292+
rejectsConfigDependencies,
290293
);
291294

292295
const loaded = await adapter.loadConfigs(loadRequest());
@@ -332,11 +335,7 @@ suite('LSP config discovery transactions', () => {
332335
pool,
333336
() => 'fingerprint-1',
334337
CONFIG_DISCOVERY_PROTOCOL_VERSION,
335-
{
336-
resolveFrom: (candidate) => candidate.configDirectory,
337-
report: () => assert.fail('unexpected missing dependency'),
338-
reportError: () => assert.fail('unexpected config error'),
339-
},
338+
rejectsConfigDependencies,
340339
);
341340

342341
await adapter.loadConfigs(loadRequest('tx-abort'));
@@ -399,11 +398,7 @@ suite('LSP config discovery transactions', () => {
399398
pool,
400399
() => 'fingerprint-degraded',
401400
CONFIG_DISCOVERY_PROTOCOL_VERSION,
402-
{
403-
resolveFrom: (candidate) => candidate.configDirectory,
404-
report: () => assert.fail('unexpected missing dependency'),
405-
reportError: () => assert.fail('unexpected config error'),
406-
},
401+
rejectsConfigDependencies,
407402
);
408403

409404
await adapter.loadConfigs(loadRequest('tx-degraded'));
@@ -439,11 +434,7 @@ suite('LSP config discovery transactions', () => {
439434
pool,
440435
() => 'fingerprint-before-prepare',
441436
CONFIG_DISCOVERY_PROTOCOL_VERSION,
442-
{
443-
resolveFrom: (candidate) => candidate.configDirectory,
444-
report: () => assert.fail('unexpected missing dependency'),
445-
reportError: () => assert.fail('unexpected config error'),
446-
},
437+
rejectsConfigDependencies,
447438
);
448439

449440
await adapter.loadConfigs(loadRequest('tx-prepare-race'));
@@ -468,11 +459,7 @@ suite('LSP config discovery transactions', () => {
468459
pool,
469460
() => 'fingerprint-1',
470461
CONFIG_DISCOVERY_PROTOCOL_VERSION,
471-
{
472-
resolveFrom: (candidate) => candidate.configDirectory,
473-
report: () => assert.fail('unexpected missing dependency'),
474-
reportError: () => assert.fail('unexpected config error'),
475-
},
462+
rejectsConfigDependencies,
476463
);
477464

478465
await adapter.loadConfigs(loadRequest('tx-response-lost'));
@@ -510,11 +497,7 @@ suite('LSP config discovery transactions', () => {
510497
new TestPluginPool(),
511498
() => 'fingerprint-1',
512499
CONFIG_DISCOVERY_PROTOCOL_VERSION,
513-
{
514-
resolveFrom: (candidate) => candidate.configDirectory,
515-
report: () => assert.fail('unexpected missing dependency'),
516-
reportError: () => assert.fail('unexpected config error'),
517-
},
500+
rejectsConfigDependencies,
518501
);
519502

520503
await assert.rejects(adapter.loadConfigs(loadRequest()), /load failed/);

‎packages/vscode/src/shared/missingDependency.ts‎

Lines changed: 21 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,24 @@
11
import path from 'node:path';
2-
import { findPackageJsonUncached } from './packageResolve';
32

4-
export function isMissingDependencyCode(
3+
function isMissingDependencyCode(
54
code: unknown,
65
): code is 'ERR_MODULE_NOT_FOUND' | 'MODULE_NOT_FOUND' {
76
return code === 'ERR_MODULE_NOT_FOUND' || code === 'MODULE_NOT_FOUND';
87
}
98

109
/**
11-
* The classifier behind the "config imports a package that is not installed"
12-
* verdict of the uniform not-installed policy (AGENTS.md). Nothing in it is
13-
* Rstest-specific — it reads Node's loader errors — and lint/fmt will need
14-
* the same verdict where their configs load (#30), which is why it lives in
15-
* `shared/` beside the walk-up it uses rather than in one stack.
16-
*
17-
* Returns the one-line cause when a config evaluation failed on a package
18-
* that is not installed, or `undefined` for a real error. Only a bare
19-
* specifier — a package name, read from the message since CJS carries no
20-
* structured one — counts, and anything unrecognized fails towards the full
21-
* error report. Only the first line comes back: the rest of a CJS message is
22-
* the require stack, and the not-installed state is one warn line without one.
10+
* The "config import cannot be resolved" verdict of the not-installed policy
11+
* (AGENTS.md); in `shared/` because lint/fmt need it too (#30). Returns the
12+
* loader's first line for a bare specifier — the policy's one warn line, with
13+
* the CJS require stack dropped — or `undefined` for a real error. A bare
14+
* specifier failed in the dependency graph, where an install, a lockfile
15+
* event or the poll can change the answer; a relative, absolute or `file:`
16+
* specifier failed inside the user's own source, where nothing external will.
17+
* #52 removed a filesystem walk-up: it could not see a pnpm-isolated private
18+
* dependency.
2319
*/
2420
export function classifyMissingDependencyMessage(
2521
message: string,
26-
resolveFrom: string,
2722
): string | undefined {
2823
const [firstLine] = message.split('\n', 1);
2924
const specifier = /^Cannot find (?:package|module) '([^']+)'/.exec(
@@ -37,35 +32,21 @@ export function classifyMissingDependencyMessage(
3732
) {
3833
return undefined;
3934
}
40-
// `installed-package/missing-subpath` wears the same bare shape, but the
41-
// package itself is there — installing dependencies cannot fix it either,
42-
// so a subpath is checked against the physical `node_modules` with the
43-
// same uncached walk-up every stack resolves packages with.
44-
const packageName = specifier.startsWith('@')
45-
? specifier.split('/').slice(0, 2).join('/')
46-
: specifier.split('/', 1)[0];
47-
if (
48-
packageName !== specifier &&
49-
findPackageJsonUncached(packageName, resolveFrom) !== undefined
50-
) {
51-
return undefined;
52-
}
5335
return firstLine;
5436
}
5537

56-
/**
57-
* Error-object entry point used where Node's loader code survives. The code is
58-
* still required there: arbitrary user errors may contain loader-like prose.
59-
* Worker/protocol boundaries that already carry a separately checked code use
60-
* `classifyMissingDependencyMessage` directly because serialization can drop
61-
* custom Error fields.
62-
*/
63-
export function missingDependencyCauseOf(
64-
error: unknown,
65-
resolveFrom: string,
38+
/** Use for a (code, message) pair; bare messages (fmt) use the classifier directly. */
39+
export function missingDependencyCause(
40+
code: unknown,
41+
message: string,
6642
): string | undefined {
43+
if (!isMissingDependencyCode(code)) return undefined;
44+
return classifyMissingDependencyMessage(message);
45+
}
46+
47+
/** Use when the caller holds an Error rather than a (code, message) pair. */
48+
export function missingDependencyCauseOf(error: unknown): string | undefined {
6749
if (!(error instanceof Error)) return undefined;
6850
const { code } = error as NodeJS.ErrnoException;
69-
if (!isMissingDependencyCode(code)) return undefined;
70-
return classifyMissingDependencyMessage(error.message, resolveFrom);
51+
return missingDependencyCause(code, error.message);
7152
}

‎packages/vscode/src/shared/notInstalled.ts‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,20 +29,18 @@ export const formatNotInstalledStatus = (
2929
`${packageName} is not installed (node_modules missing) — install it, ${restartHint(stack)}`;
3030

3131
/**
32-
* The `disabled` reason for a config that evaluates but imports a package
33-
* that is not there. `configPath` is workspace-relative: the status has no
34-
* room for more.
32+
* The `disabled` reason for a config with an import Node cannot resolve.
33+
* `configPath` is workspace-relative: the status has no room for more.
3534
*/
3635
export const formatConfigDependencyMissingStatus = (
3736
stack: StackId,
3837
configPath: string,
3938
): string =>
40-
`${configPath} imports a package that is not installed — install the project dependencies, ${restartHint(stack)}`;
39+
`${configPath} has an import Node cannot resolve — install the project dependencies, ${restartHint(stack)}`;
4140

4241
/**
43-
* The output-channel line for a config that imports a package that is not
44-
* installed. `cause` is the loader's own first line, which names the
45-
* specifier and the importer.
42+
* The output-channel line for the same verdict. `cause` is the loader's own
43+
* first line, which names the specifier and the importer.
4644
*/
4745
export const formatConfigDependencyMissingLog = (
4846
stack: StackId,

‎packages/vscode/src/stacks/fmt/index.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,6 @@ class FmtFolderRuntime {
239239
if (configPath !== undefined) {
240240
const cause = classifyMissingDependencyMessage(
241241
firstLine.replace(/^Error(?: \[[A-Z_]+\])?: /, ''),
242-
this.folderPath,
243242
);
244243
if (cause !== undefined) {
245244
this.#sessionError.clear();

‎packages/vscode/src/stacks/lint/worker/ConfigTransactionAdapter.ts‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,15 @@ import type {
22
ActivateConfigsRequest,
33
ActivateConfigsResponse,
44
ConfigModuleActivationPlan,
5-
ConfigModuleCandidate,
65
ConfigModuleEslintPluginEntry,
76
ConfigModulePluginDescriptor,
87
LoadConfigsRequest,
98
LoadConfigsResponse,
109
} from '@rslint/core/config-loader';
11-
import {
12-
classifyMissingDependencyMessage,
13-
isMissingDependencyCode,
14-
} from '../../../shared/missingDependency';
10+
import { missingDependencyCause } from '../../../shared/missingDependency';
1511
import type { ConfigDependencyFailure } from '../../../shared/notInstalled';
1612

1713
interface ConfigDependencyObserver {
18-
resolveFrom(candidate: ConfigModuleCandidate): string;
1914
report(failure: ConfigDependencyFailure): void;
2015
reportError(message: string): void;
2116
}
@@ -128,14 +123,10 @@ export class LspConfigTransactionAdapter {
128123
results: response.results.map((result, index) => {
129124
if (result.status !== 'failed') return result;
130125
const candidate = request.candidates[index];
131-
const cause =
132-
candidate !== undefined &&
133-
isMissingDependencyCode(result.error.code)
134-
? classifyMissingDependencyMessage(
135-
result.error.message,
136-
this.configDependencyObserver.resolveFrom(candidate),
137-
)
138-
: undefined;
126+
const cause = missingDependencyCause(
127+
result.error.code,
128+
result.error.message,
129+
);
139130
// Scan every failure: a later real error must not be hidden by the
140131
// first missing dependency, even though only that result is rewritten.
141132
if (cause === undefined || candidate === undefined) {

‎packages/vscode/src/stacks/lint/worker/index.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,10 +234,6 @@ export async function runLintWorker(
234234
(activation) => fingerprinter.compute(activation),
235235
installation.protocolVersion,
236236
{
237-
resolveFrom: (candidate) =>
238-
candidate.configPath === options.configPath
239-
? process.cwd()
240-
: candidate.configDirectory,
241237
report: (failure) => {
242238
configDependencyFailure ??= failure;
243239
},

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -691,8 +691,9 @@ export class Project implements vscode.Disposable {
691691
return `config-deps:${this.sourceUri.toString()}`;
692692
}
693693

694-
// The config imports a package that is not installed: the not-installed
695-
// state (AGENTS.md), one step past a missing `@rstest/core` — some install
694+
// The config imports a package Node cannot resolve — a missing package
695+
// or a missing subpath of an installed one: the not-installed state
696+
// (AGENTS.md), one step past a missing `@rstest/core` — some install
696697
// *above* the project satisfied the shim, so the config itself is what
697698
// failed. A scaffolded template beside its generator is the usual shape.
698699
// Latched under this project's key, which `dispose` forgets.

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ export type WorkerInitOptions = RstestConfig & {
99
};
1010

1111
/**
12-
* What the worker answers `getNormalizedConfig` with. A config that fails to
13-
* evaluate because a dependency is not installed is a result, not a rejection:
14-
* the IPC channel would strip the error's `code` (see
12+
* What the worker answers `getNormalizedConfig` with. A config that fails
13+
* to evaluate because Node cannot resolve one of its imports is a result,
14+
* not a rejection: the IPC channel would strip the error's `code` (see
1515
* `missingDependencyCauseOf`), so the worker classifies it and reports the
1616
* loader's own first line as data.
1717
*/

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,7 @@ export class Worker {
9292
// Classified here and not in the master: `code` does not survive the
9393
// IPC round-trip. Only this unprompted, per-config evaluation gets the
9494
// treatment — a run or list the user asked for reports its failure.
95-
// The worker's spawn cwd is the project root (adaptation #5), which is
96-
// where the config's dependencies are installed.
97-
const cause = missingDependencyCauseOf(error, process.cwd());
95+
const cause = missingDependencyCauseOf(error);
9896
if (cause !== undefined) {
9997
return { ok: false, message: cause };
10098
}

0 commit comments

Comments
 (0)