Skip to content

Commit 7fe84bd

Browse files
authored
fix(vscode): detect rslint configs above the workspace folder (#89)
* fix(vscode): detect rslint configs above the workspace folder * refactor(vscode): simplify ancestor rslint config detection * test(vscode): cover a nearer ancestor rslint config created while linting * fix(vscode): treat ancestor lockfile changes as dependency changes * test(vscode): temporary macOS CI diagnostics for the ancestor suite * test(vscode): dump ancestor suite traces without the probe delay * fix(vscode): refresh rslint config on ancestor edits before the server boots * test(vscode): remove temporary macOS CI diagnostics * fix(vscode): watch ancestor rslint configs for every native runtime
1 parent 9c6175b commit 7fe84bd

17 files changed

Lines changed: 346 additions & 46 deletions

File tree

‎CONTEXT.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ Glossary of terms used across rstack-editor. Code, docs, commit messages and rev
66

77
- **Stack** — one tool integration (lint, test, fmt) hosted by the extension shell. A stack registers against the shell and reports status through it; stacks never own UI chrome.
88
- **Shell** — the always-activating extension core: detection, status bar, output channels, stack lifecycle.
9-
- **Detection** — the per-workspace-folder scan deciding which stacks a folder lights up. Detection signals are config files and installed tool binaries, never user settings.
9+
- **Detection** — the per-workspace-folder scan deciding which stacks a folder lights up. The decision is per folder, but native lint's config signal may come from an ancestor directory. Detection signals are config files and installed tool binaries, never user settings.
1010
- **Gate** — the per-stack activation condition: detected, workspace trusted, and that stack's enable setting on.
1111

1212
## Runtimes

‎packages/vscode/AGENTS.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten
4747

4848
- **Why test file ownership exists** (adaptation 10): native configs can overlap upstream too, and the Rstack bridge adds a root contender. VS Code merges same-location items into one gutter action; routing chooses among the request's published items, which already reflect each config's include/exclude rules.
4949
- fmt's `handleShowMessage` suppresses only the classified config-dependency Error. Non-classified `window/showMessage` notifications are re-presented exactly as vscode-languageclient's default handler would (Error/Warning/Info toast). An exact-prefix `rs fmt cannot format this workspace:` Error that is not a missing dependency also reports `crashed` and one Output error line per distinct message; the known config error survives server restarts so polling continues until a format produces edits. This passes through the server's protocol UI request unchanged; "stacks own no UI chrome" constrains UI the stack originates, not protocol UI it relays. Warning and error episodes end on nonempty formatting edits, not merely a response with no new notification: the server returns empty edits on failure and deduplicates showMessage.
50+
- **Native lint detection looks above the workspace folder; nothing else does.** The rslint CLI and its Go server walk up to the filesystem root for a config, so a folder without its own `rslint.config.*` and without a root `rstack.config.*` uses the nearest one above it (skipping `node_modules`); fmt, Rstest and bridged lint stay folder-local because `rs fmt` / `rs lint` read only their cwd. The ancestor counts only while `@rslint/core` resolves from the folder, so a stray `~/rslint.config.*` does not put every folder below it into the not-installed state (`corePath` does not satisfy this). Folder-relative watchers cannot see ancestors, so detection watches each ancestor directory non-recursively for the config names and lockfiles; an ancestor lockfile counts as a dependency change exactly like a folder one, since a monorepo opened at a subdirectory installs at the root. Every native runtime also watches the ancestor directories for `rslint.config.*` (the set Go watches) and sends `rslint/configRefresh`; the watchers are installed before the server boots: Go registers its own ancestor watchers only after initialization, and a watcher misses writes made right after it is created (macOS CI lost an edit made ~40 ms after Go's registration). Known gap: a folder whose ancestor config is gated out is not detected, so the ADR 0005 poll never runs for it; an install that leaves the lockfile untouched (`npm ci`, `pnpm install --frozen-lockfile`) is picked up only on the next detection pass or a window reload.
5051
- The lint × `rstack.config.*` bridge stays thin on purpose: only a root Rstack config can claim a bridged folder, any native config anywhere in the folder wins ownership, and the worker evaluates rstack's published shim from the folder root. Never generate a shim, load the Rstack config in the extension host, or interpret `define.lint()` ourselves.
5152
- **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.
5253
- **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 "a package is not installed" — no `rstack`, or no `@rslint/core` — not the kill switch). Dependency retries come only through the shell's detection pass: lockfile events are the low-latency path and ADR 0005's conditional poll covers unchanged lockfiles. The former lint-owned `node_modules/@rslint/core/package.json` watcher was removed because pnpm produced no event in either isolated or hoisted layout. 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.

‎packages/vscode/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ The extension activates on startup, then decides **per workspace folder** which
2222

2323
| Tool | Started when the folder contains |
2424
| --- | --- |
25-
| Rslint | `rslint.config.{js,mjs,ts,mts}` anywhere in the folder, or `rstack.config.*` at the folder root when no native config exists |
25+
| Rslint | `rslint.config.{js,mjs,ts,mts}` anywhere in the folder, or `rstack.config.*` at the folder root when no native config exists. A folder with neither uses the nearest `rslint.config.*` above it, provided `@rslint/core` is installed for the folder |
2626
| Rstest | `rstest.config.{mjs,ts,js,cjs,mts,cts}` (configurable) or `rstack.config.*` |
2727
| rstack-cli | `rstack.config.*` or `node_modules/.bin/rs` |
2828

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export function pause(): void {
2+
debugger;
3+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"compilerOptions": {
3+
"target": "ES2020",
4+
"module": "ESNext",
5+
"strict": true
6+
},
7+
"include": ["src/**/*.ts"]
8+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// The only Rslint config in this fixture. The opened workspace folder is
2+
// `packages/app`, so detection must find this file above the folder.
3+
export default [
4+
{
5+
files: ['**/*.ts'],
6+
rules: {
7+
'no-debugger': 'error',
8+
},
9+
},
10+
];

‎packages/vscode/e2e/lint/runTest.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,14 @@ async function main(): Promise<void> {
268268
workspaceEntry: 'nested-initial.code-workspace',
269269
workspaceFolders: ['parent', 'parent/nested', 'sentinel'],
270270
},
271+
{
272+
// The opened folder is a sub-package; the only config is above it.
273+
name: 'Ancestor config tests',
274+
workspace: fixture('ancestor'),
275+
tests: suiteDir('suite-ancestor'),
276+
workspaceEntry: 'packages/app',
277+
workspaceFolders: ['packages/app'],
278+
},
271279
{
272280
name: 'No config tests',
273281
workspace: fixture('noconfig'),
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
// The opened workspace folder is `packages/app`; the fixture's only
2+
// `rslint.config.*` sits two levels above it. The rslint CLI and its Go server
3+
// walk up to the filesystem root for a config, so native lint detection must
4+
// light this folder, and its ancestor watchers must keep that decision fresh.
5+
import * as vscode from 'vscode';
6+
import path from 'node:path';
7+
import fs from 'node:fs';
8+
import {
9+
diagnosticRuleIdIncludes,
10+
waitForRslintDiagnostics as waitForDiagnostics,
11+
} from '../utils/diagnostics';
12+
import { waitForLintStackRegistration } from '../utils/extension';
13+
14+
function configWithNoDebugger(severity: 'error' | 'off'): string {
15+
return `export default [
16+
{
17+
files: ['**/*.ts'],
18+
rules: {
19+
'no-debugger': '${severity}',
20+
},
21+
},
22+
];
23+
`;
24+
}
25+
26+
suite('rslint config above the workspace folder', function () {
27+
this.timeout(120000);
28+
29+
const folderPath = (): string =>
30+
vscode.workspace.workspaceFolders![0].uri.fsPath;
31+
const rootConfigPath = (): string =>
32+
path.join(folderPath(), '..', '..', 'rslint.config.mjs');
33+
const nearerConfigPath = (): string =>
34+
path.join(folderPath(), '..', 'rslint.config.mjs');
35+
36+
async function openIndex(): Promise<vscode.TextDocument> {
37+
const document = await vscode.workspace.openTextDocument(
38+
path.join(folderPath(), 'src', 'index.ts'),
39+
);
40+
await vscode.window.showTextDocument(document);
41+
return document;
42+
}
43+
44+
const hasNoDebugger = (diagnostics: vscode.Diagnostic[]): boolean =>
45+
diagnostics.some((diagnostic) =>
46+
diagnosticRuleIdIncludes(diagnostic, 'no-debugger'),
47+
);
48+
49+
teardown(async () => {
50+
fs.rmSync(nearerConfigPath(), { force: true });
51+
fs.writeFileSync(rootConfigPath(), configWithNoDebugger('error'));
52+
await waitForLintStackRegistration(true);
53+
});
54+
55+
test('lints with the config above the folder and follows its edits', async () => {
56+
const document = await openIndex();
57+
await waitForDiagnostics(document, hasNoDebugger);
58+
59+
fs.writeFileSync(rootConfigPath(), configWithNoDebugger('off'));
60+
await waitForDiagnostics(
61+
document,
62+
(diagnostics) => !hasNoDebugger(diagnostics),
63+
);
64+
65+
fs.writeFileSync(rootConfigPath(), configWithNoDebugger('error'));
66+
await waitForDiagnostics(document, hasNoDebugger);
67+
});
68+
69+
// The live runtime keeps its key; the extension's ancestor watchers send
70+
// configRefresh, as do the Go server's own (`ancestorJSConfigFileWatchers`,
71+
// registered through the worker) once they are live.
72+
test('applies a nearer config created above the folder while linting', async () => {
73+
const document = await openIndex();
74+
await waitForDiagnostics(document, hasNoDebugger);
75+
76+
fs.writeFileSync(nearerConfigPath(), configWithNoDebugger('off'));
77+
await waitForDiagnostics(
78+
document,
79+
(diagnostics) => !hasNoDebugger(diagnostics),
80+
);
81+
82+
fs.rmSync(nearerConfigPath());
83+
await waitForDiagnostics(document, hasNoDebugger);
84+
});
85+
86+
test('re-runs detection when the config above the folder is deleted or created', async () => {
87+
fs.rmSync(rootConfigPath());
88+
await waitForLintStackRegistration(false);
89+
90+
fs.writeFileSync(rootConfigPath(), configWithNoDebugger('error'));
91+
await waitForLintStackRegistration(true);
92+
const document = await openIndex();
93+
await waitForDiagnostics(document, hasNoDebugger);
94+
});
95+
});
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { createRun } from '../runSuite';
2+
3+
// The opened folder has no config of its own; the one above it must light the
4+
// lint stack.
5+
export const run = createRun();

‎packages/vscode/e2e/lint/suite-noconfig/noconfig.test.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,15 @@
33
// (origin/main), with the expectations rewritten for this extension's
44
// detection rules:
55
//
6-
// - Upstream treated `rslint.json` as a working fallback config: creating it
7-
// produced diagnostics, and the suite walked a JSON → JS → JSON lifecycle.
8-
// - This extension does not support `rslint.json` at all. It is not a
9-
// detection signal, so a folder with only `rslint.json` stays
10-
// `not detected`, the lint stack is never registered, and zero diagnostics
11-
// is the *designed* outcome — not a fallback state.
6+
// - Upstream's suite walked a JSON → JS → JSON lifecycle from when
7+
// `rslint.json` was a working fallback config. Upstream main no longer
8+
// loads legacy JSON configs, and this extension never supported them.
9+
// `rslint.json` is not a detection signal, so a folder with only
10+
// `rslint.json` stays `not detected`, the lint stack is never registered,
11+
// and zero diagnostics is the *designed* outcome — not a fallback state.
12+
// - Native detection also looks above the folder (`suite-ancestor`). The
13+
// sandbox's ancestors hold no `rslint.config.*`, so this suite doubles as
14+
// the negative case: no config anywhere still means `not detected`.
1215
// - `rslint.config.*` remains a live detection signal: creating one must
1316
// register the stack without a window reload, and deleting the last one
1417
// must deregister it (the config-glob + lockfile detection watcher).

0 commit comments

Comments
 (0)