diff --git a/packages/vscode/AGENTS.md b/packages/vscode/AGENTS.md index 1236507..d40b19f 100644 --- a/packages/vscode/AGENTS.md +++ b/packages/vscode/AGENTS.md @@ -17,7 +17,7 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten 4. **Status aggregation** — stacks own no UI chrome; they report to the shell's single status bar item, which always exists. Upstream's plugin-host failure toast becomes a status verdict (adaptation 7). Rslint LSP tracing shares the **Rstack: Rslint** Output channel to preserve the four-channel cap; see `stacks/lint/index.ts`. In CI the test stack's `MasterLogger` also mirrors every entry to stderr (`RSTACK_E2E_MIRROR_LOGS=1`, set by `e2e/rstest/runTest.ts`) — the output channel is unreadable there; rationale in `stacks/test/logger.ts`. 5. **Worker-cwd decoupling** (test) — a project's cwd is explicit, not derived from the config file path; for native configs behavior stays byte-identical to upstream. 6. **Node runtime selection** (lint, test, fmt) — the Node a project-loading child process runs on is a **User Node runtime** chosen by the extension against one uniform floor, never assumed from PATH; the recovery path is the user's own shell, and the dividing line is the **load bound** (terms in GLOSSARY.md; the full rule and rationale in `docs/adr/0001-node-runtime-selection.md`). All three callers — the lint worker, the rstest worker and the `rs fmt --lsp` server — take the decision from the one shared module (`shared/nodeResolution.ts`) and share one escape hatch, the resource-scoped `rstack.nodeExecutable` (`shared/nodeExecutableSetting.ts`); each appends its own consequence to the shared preflight message. -7. **Lint worker and Rstack bridge** — the extension host is only Rslint's language client. One vscode-free, editor-shipped lint worker per **Lint runtime** (one Rslint core inside one workspace folder — GLOSSARY.md) runs on the User Node runtime, owns the Go LSP plus all five reverse requests, and derives the binary/config/plugin pieces from one explicit `@rslint/core` directory. Upstream's `CoreResolver` loads that core in the extension host; ours only walks to the directory (`fs.stat` + `package.json` + semver) and hands the path to the worker, and its `CoreInstallation` therefore carries paths, not module factories; upstream's installation cache goes with the module loading it memoized (`clear()` is a no-op kept for the `RuntimeManager` contract). A bridged runtime passes only rstack's published `dist/rslintConfig.js` shim; neither the extension nor the worker re-implements Rstack config semantics. Because every supported config protocol locks `configPath` per process, the shim is part of the runtime key (`folder + core identity + shim`), which upstream — having no bridge — keys on the core alone. Why: `docs/adr/0003-lint-through-editor-worker.md`. The worker also sends the editor-only `rstack/rslintConfigDependency` notification (`stacks/lint/worker/configDependencyProtocol.ts`) when config loading finds a missing package. `ConfigTransactionAdapter` rewrites only that classified `rslint/loadConfigs` candidate's error message to its first line, so Go cannot echo a require stack beside the single warning. An initialized client whose initial configRefresh rejects with that verdict stays available for retry, rather than propagating a generic startup crash through RuntimeManager. Plugin-host startup failures use a separate, unclassified `plugin` verdict on `rstack/rslintConfigDependency`, report `disabled` (a plugin could not be loaded), and recover through the dependency poll. A bridged runtime whose Rstack config has no `define.lint()` gets a third verdict, `unconfigured` (#93; why it exists: the lint-shim gotcha below). It reports `running` with the detail `no define.lint() in rstack.config.*` and one `info` line per episode — healthy with nothing to lint, like `idle`: not `disabled` (no poll), not `crashed`, no warning. Adding `define.lint()` recovers in the same worker through the bridged `rstack.config.*` watcher, without a restart. +7. **Lint worker and Rstack bridge** — the extension host is only Rslint's language client. One vscode-free, editor-shipped lint worker per **Lint runtime** (one Rslint core inside one workspace folder — GLOSSARY.md) runs on the User Node runtime, owns the Go LSP plus all five reverse requests, and derives the binary/config/plugin pieces from one explicit `@rslint/core` directory. Upstream's `CoreResolver` loads that core in the extension host; ours only walks to the directory (`fs.stat` + `package.json` + semver) and hands the path to the worker, and its `CoreInstallation` therefore carries paths, not module factories; upstream's installation cache goes with the module loading it memoized (`clear()` is a no-op kept for the `RuntimeManager` contract). A bridged runtime passes only rstack's published `dist/rslintConfig.js` shim; neither the extension nor the worker re-implements Rstack config semantics. Because every supported config protocol locks `configPath` per process, the shim is part of the runtime key (`folder + core identity + shim`), which upstream — having no bridge — keys on the core alone. Why: `docs/adr/0003-lint-through-editor-worker.md`. The worker also sends the editor-only `rstack/rslintConfigDependency` notification (`stacks/lint/worker/configDependencyProtocol.ts`) when config loading finds a missing package. `ConfigTransactionAdapter` rewrites only that classified `rslint/loadConfigs` candidate's error message to its first line, so Go cannot echo a require stack beside the single warning. An initialized client whose initial configRefresh rejects with that verdict stays available for retry, rather than propagating a generic startup crash through RuntimeManager. Plugin-host startup failures use a separate, unclassified `plugin` verdict on `rstack/rslintConfigDependency`, report `disabled` (a plugin could not be loaded), and recover through the dependency poll. A bridged runtime whose Rstack config has no `define.lint()` gets a third verdict, `unconfigured` (#93; why it exists: the lint-shim gotcha below). It reports `not-detected` with the detail `no define.lint() in rstack.config.*` and one `info` line per episode. Why `not-detected`: the shim's refusal is a late detection signal, and `not-detected` is the status bar's word for "this tool has no configuration here"; `running` would show a green Rslint that lints nothing. The worker stays alive underneath — the state/process decoupling `running: idle` makes in the other direction — and there is no poll because `isFailedStackState` excludes `not-detected`; not `crashed`, no warning. A folder whose only runtimes are unconfigured folds to that `not-detected`; a healthy runtime beside it (a native `rslint.config.*`) wins, since the folder is linting. The `not-detected` detail exists for this verdict: detection never sets one, so only a stack with a runtime-level "nothing configured" verdict renders it, as one hover notice row. That grey `not-detected` is a runtime-level verdict and lives with the runtime: only the shim can decide it, and persisting it per folder would need its own invalidation on the bridged config watcher, so an unconfigured folder reads `running: idle` on startup and after its last document closes, until a document opens a runtime again. Adding `define.lint()` recovers to plain `running` in the same worker through the bridged `rstack.config.*` watcher, without a restart. 8. **Self-documenting Rslint diagnostics** — client-side providers parse Inline directives into per-rule hover, DocumentLink and underline-decoration affordances (the hover renders `Rslint(rule-id)`, the shape VS Code gives the published diagnostics), and the router enriches today's `[rule-id] message` diagnostics with a derived Rule docs link. No rule metadata or network lookup is bundled (ADR 0004). The hover provider yields whenever the owning language client's resolved capabilities advertise `hoverProvider`; an optional `Rslint.onClosed` hook identity-safely prunes the controller's capability mirror; the diagnostic synthesis is removed once upstream publishes `code` / `codeDescription` natively. 9. **Color env parity with the CLI** (test) — upstream hard-codes `FORCE_COLOR: '1'` into the worker's spawn env; ours mirrors the CLI's `getForceColorEnv` (rstest `packages/core/src/utils/logger.ts`) instead (`stacks/test/shared/colorEnv.ts`): the master injects `FORCE_COLOR=1` into the composed spawn env only when neither `FORCE_COLOR` nor `NO_COLOR` is already set (marking the injection with `RSTACK_FORCE_COLOR_INJECTED`), and the worker retracts the marked injection right after config load if the config set `NO_COLOR` — the CLI's own decision point. Otherwise a project whose config sets `process.env.NO_COLOR` (rstack-cli does) hits Node's "'NO_COLOR' env is ignored" warning in every pool process. A user-set `FORCE_COLOR` beside a config-set `NO_COLOR` still warns, exactly as the bare CLI does. @@ -51,9 +51,9 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten - **Lint ownership is per document, not per folder** (ADR 0006). The supported config protocols lock the config choice per _process_, and runtimes are keyed `folder + core + shim`, so one folder runs a bridged and a native runtime side by side. `decideDocumentMode` walks the document's ancestor chain only: an `rslint.config.*` between the document and the folder root → native; else a root `rstack.config.*` → bridged; else an `rslint.config.*` above the folder → native; else the document is not served, as the `rslint` CLI skips files outside every config. A sibling or descendant config never counts, so a subdirectory `rslint.config.*` no longer takes the root documents away from `define.lint()` (#85). Known divergence: `rs lint` at the root lints nested files with the root config; the editor uses the nearest `rslint.config.*`, which is what `rslint` run in that directory does. - The lint × `rstack.config.*` bridge stays thin on purpose: only a root Rstack config can bridge a document, 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. - **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 "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. +- **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`; this holds for a bridged folder without `define.lint()` too, since its grey `not-detected` (adaptation 7) is a runtime-level verdict that lives with the runtime, so it reads `running: idle` until a document opens; 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. - 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 document whose ownership flips native ↔ bridged moves to another runtime, because the supported config protocols lock that choice for the process lifetime; documents whose ownership did not change keep their runtime. -- **Lint is the only tool whose Rstack shim refuses an unconfigured config**, so "the three tools are treated uniformly" is knowingly broken for it. rstack's `rslintConfig.js` exits the process without `define.lint()` (deliberately, for `rs lint`; rstack-cli #490/#493), while `rstestConfig.js` returns `{}` without `define.test()`. Detection cannot tell the cases apart — a static probe for `define.lint` is forbidden (above) and wrong under shared config layers (rstack-cli #546) — so a formatting-only root config still lights lint, and only the shim, inside the worker, decides (the `unconfigured` verdict, adaptation 7). The bridged worker therefore builds its `ConfigModuleHost` with a `loadFresh` wrapped by `withProcessExitAsThrow`, so the exit becomes a tagged failed load result instead of a dead worker, Go, and transport. That is a workaround for rstack 0.8.2 as published: once the shim throws or exports a marker instead of exiting, classify that and drop the wrapper. The shim's own `No lint configuration found…` line (rslog, stderr) and Go's `Skipped config …` lines still reach the Rslint Output channel beside our `info` line. Known gap: after a catalog with `define.lint()` has committed, removing it makes Go reject the refresh and keep linting with the last-good catalog while the status says `no define.lint()`; `rs lint` would refuse. +- **Lint is the only tool whose Rstack shim refuses an unconfigured config**, so "the three tools are treated uniformly" is knowingly broken for it. rstack's `rslintConfig.js` exits the process without `define.lint()` (deliberately, for `rs lint`; rstack-cli #490/#493), while `rstestConfig.js` returns `{}` without `define.test()`. Detection cannot tell the cases apart — a static probe for `define.lint` is forbidden (above) and wrong under shared config layers (rstack-cli #546) — so a formatting-only root config still lights lint, and only the shim, inside the worker, decides (the `unconfigured` verdict, adaptation 7, reported as `not-detected` plus the reason). The bridged worker therefore builds its `ConfigModuleHost` with a `loadFresh` wrapped by `withProcessExitAsThrow`, so the exit becomes a tagged failed load result instead of a dead worker, Go, and transport. That is a workaround for rstack 0.8.2 as published: once the shim throws or exports a marker instead of exiting, classify that and drop the wrapper. The shim's own `No lint configuration found…` line (rslog, stderr) and Go's `Skipped config …` lines still reach the Rslint Output channel beside our `info` line. Known gap: after a catalog with `define.lint()` has committed, removing it makes Go reject the refresh and keep linting with the last-good catalog while the status says `not detected — no define.lint() …`; `rs lint` would refuse. - 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. - Rstest's upstream VS Code extension deep-imports `quoteFilter` from core to mark exact file filters. The published package does not export that helper, so our copy lives in `stacks/test/vendored/coreInternals.ts` beside the other core internals; keep it byte-identical when syncing filter behavior. - 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`. diff --git a/packages/vscode/e2e/lint/suite-fmt-only-config/fmtOnlyConfig.test.ts b/packages/vscode/e2e/lint/suite-fmt-only-config/fmtOnlyConfig.test.ts index d38a2ca..e8d43d3 100644 --- a/packages/vscode/e2e/lint/suite-fmt-only-config/fmtOnlyConfig.test.ts +++ b/packages/vscode/e2e/lint/suite-fmt-only-config/fmtOnlyConfig.test.ts @@ -12,9 +12,9 @@ import { extensionExports } from '../utils/extension'; // rstack's lint shim calls `process.exit(1)` when the Rstack config has no // `define.lint()` (#93). The root config still lights the lint stack, so the -// bridged runtime must survive the shim's refusal and report a healthy -// runtime with nothing to lint, then pick up `define.lint()` through its -// config watcher in the same worker. +// bridged runtime must survive the shim's refusal and report `not-detected` +// with the reason (the status bar's word for "no configuration here"), then +// pick up `define.lint()` through its config watcher in the same worker. function lintExports(): { getFolderStates(): ReadonlyMap; @@ -35,7 +35,7 @@ function lintStates(): StackState[] { function isUnconfigured(state: StackState): boolean { return ( - state.kind === 'running' && + state.kind === 'not-detected' && state.detail !== undefined && state.detail.includes('define.lint()') ); @@ -113,7 +113,7 @@ suite('Rstack fmt-only config', function () { fs.rmSync(markerPath, { force: true }); }); - test('a formatting-only config keeps a healthy runtime and recovers on define.lint()', async () => { + test('a formatting-only config reports not-detected and recovers on define.lint()', async () => { // Still formatting-only; the lint runtime starts on `didOpen`, so the // first shim evaluation records the lint worker's pid. fs.writeFileSync(configPath, configSource(markerPath, false), 'utf8'); @@ -122,9 +122,9 @@ suite('Rstack fmt-only config', function () { ); await vscode.window.showTextDocument(document); - // Bare `running` appears before the initial config refresh settles; - // only the detail proves the shim's refusal was classified. - await waitForLintStates('the unconfigured running detail', (states) => + // Bare `running` appears before the initial config refresh settles; the + // folder and its runtime must both show the classified refusal. + await waitForLintStates('the unconfigured not-detected detail', (states) => states.every(isUnconfigured), ); assert.ok(fs.existsSync(markerPath), 'the lint worker never ran the shim'); @@ -145,7 +145,7 @@ suite('Rstack fmt-only config', function () { workerPid, 'define.lint() must be picked up by the same lint worker, without a restart', ); - // A healthy state: logged at info, so no warning was recorded. + // Not a failure: logged at info, so no warning was recorded. assert.deepStrictEqual( extensionExports().getRecordedWarnings('rslint'), [], diff --git a/packages/vscode/src/stacks/lint/Rslint.ts b/packages/vscode/src/stacks/lint/Rslint.ts index 1f14443..119ee55 100644 --- a/packages/vscode/src/stacks/lint/Rslint.ts +++ b/packages/vscode/src/stacks/lint/Rslint.ts @@ -51,7 +51,7 @@ import { } from './worker/configDependencyProtocol'; import { RslintVersionMismatchError, - runningRslintStatus, + liveRslintStatus, statusForRslintStartFailure, } from './status'; import { @@ -322,7 +322,7 @@ export class Rslint implements Disposable { private readonly configDependencyEpisode = new NotInstalledEpisode(); private configDependencyRetryPending = false; private configRefreshFailed = false; - /** Holds the status detail while the bridged shim finds no `define.lint()` (#93). */ + /** Holds the `not-detected` detail while the bridged shim finds no `define.lint()` (#93). */ private readonly unconfigured = new MessageLatch(); private readonly configError = new MessageLatch(); private startPromise: Promise | undefined; @@ -352,17 +352,18 @@ export class Rslint implements Disposable { this.reportStatus(state); } - private reportRunning(): void { + private reportLive(): void { if (this.configRefreshFailed || this.hasConfigDependencyFailure()) return; - this.report(runningRslintStatus(this.advisory, this.unconfigured.current)); + this.report(liveRslintStatus(this.advisory, this.unconfigured.current)); } private handleConfigDependencyStatus( notification: ConfigDependencyStatusNotification, ): void { if (notification.kind === 'unconfigured') { - // Healthy, nothing to lint: no poll, no warning, no crash. The bridged - // config watcher re-runs the shim once `define.lint()` appears. + // Nothing to lint: no poll, no warning, no crash. The worker stays up + // and the bridged config watcher re-runs the shim once `define.lint()` + // appears. this.configError.clear(); this.configRefreshFailed = false; this.configDependencyEpisode.clear(); @@ -376,7 +377,7 @@ export class Rslint implements Disposable { `Rslint has nothing to lint: ${detail}. Add define.lint(...) to enable it.`, ); } - this.reportRunning(); + this.reportLive(); return; } const wasUnconfigured = this.unconfigured.current !== undefined; @@ -406,7 +407,7 @@ export class Rslint implements Disposable { if (notification.kind === 'ok') { const wasMissing = this.configDependencyEpisode.clear(); if ((wasMissing || wasFailed || wasUnconfigured) && this.isRunning()) { - this.reportRunning(); + this.reportLive(); } return; } @@ -543,7 +544,7 @@ export class Rslint implements Disposable { detail: 'the Rslint language server stopped', }); } else if (event.newState === State.Running) { - this.reportRunning(); + this.reportLive(); } }); @@ -602,7 +603,7 @@ export class Rslint implements Disposable { ); } this.logger.info('Rslint language client started successfully'); - this.reportRunning(); + this.reportLive(); } catch (error: unknown) { // Keep the initialized runtime available for configRefresh retries. // Rethrowing this classified rejection would make RuntimeManager close @@ -632,7 +633,7 @@ export class Rslint implements Disposable { void configuredNodeBelowFloor(configured).then((message) => { if (message !== undefined && !this.closing) { this.advisory = message; - if (this.isRunning()) this.reportRunning(); + if (this.isRunning()) this.reportLive(); } }); return configured; @@ -726,13 +727,13 @@ export class Rslint implements Disposable { this.configRefreshFailed = false; try { await client.sendRequest('rslint/configRefresh', { reason }); - if (wasFailed && this.isRunning()) this.reportRunning(); + if (wasFailed && this.isRunning()) this.reportLive(); } catch (error) { // The worker verdict already surfaced this rejection as a real config // error. Keep the live runtime for config edits without duplicate logs // or a generic startup failure replacing its precise status. // Source-change races must still reach the existing startup retry. - // An unconfigured verdict is a healthy state, so Go rejecting that + // An unconfigured verdict is not a failure, so Go rejecting that // refresh (it keeps a last-good catalog) is not reported either. if ( isConfigSourceChangeDuringTransaction(error) || diff --git a/packages/vscode/src/stacks/lint/status.ts b/packages/vscode/src/stacks/lint/status.ts index db847a2..3cf4231 100644 --- a/packages/vscode/src/stacks/lint/status.ts +++ b/packages/vscode/src/stacks/lint/status.ts @@ -1,4 +1,4 @@ -import type { StackState } from '../../types'; +import { type StackState, stackStateDetail } from '../../types'; import { formatNotInstalledStatus } from '../../shared/notInstalled'; import type { SupportedPackage } from '../../shared/versionCheck'; import { RslintResolutionError } from './resolution'; @@ -58,21 +58,21 @@ export const attributeToCore = ( }; /** - * An advisory wins over `detail`, which is the unconfigured bridge's - * `no define.lint()` note: healthy with nothing to lint, so `running` plus a - * detail like `idle`, not `disabled` — that would keep the dependency poll - * re-evaluating an unchanged config (#93). + * An advisory wins over everything: a configured Node below the floor is worth + * fixing whatever the config says. `unconfigured` is the bridged shim's + * `no define.lint()` note, reported as `not-detected` with that note as the + * detail while the worker stays up (#93; AGENTS.md adaptation 7). */ -export const runningRslintStatus = ( +export const liveRslintStatus = ( advisory?: string, - detail?: string, + unconfigured?: string, ): StackState => { if (advisory !== undefined) { return { kind: 'version-mismatch', detail: advisory }; } - return detail === undefined + return unconfigured === undefined ? { kind: 'running' } - : { kind: 'running', detail }; + : { kind: 'not-detected', detail: unconfigured }; }; /** A detected folder with no Lint runtime: `running` plus a detail, never a new kind (AGENTS.md, lint gotcha). */ @@ -85,6 +85,13 @@ const RSLINT_IDLE_DETAIL = 'idle'; * this folder needs is not installed, so it will not lint" (`missingPackageOf`), * a fact worth showing over a healthy runtime or sibling folder — unlike the * shell's kill switch. + * + * A runtime's `not-detected` (the unconfigured bridge) ranks below `running` + * on purpose: a folder that also has a healthy native runtime is linting, and + * showing "nothing configured" for its bridged half instead would hide that. + * Across folders the aggregate keeps details only from folders at the worst + * kind, so beside a `running` sibling an unconfigured folder's reason does not + * reach the hover. */ const STATE_RANK: Readonly> = { crashed: 5, @@ -95,20 +102,6 @@ const STATE_RANK: Readonly> = { 'not-detected': 0, }; -const detailOf = (state: StackState): string | undefined => { - switch (state.kind) { - case 'crashed': - case 'version-mismatch': - case 'starting': - case 'running': - return state.detail; - case 'disabled': - return state.reason; - case 'not-detected': - return undefined; - } -}; - const worstKind = (states: readonly StackState[]): StackState['kind'] => states.reduce( (worst, state) => @@ -143,7 +136,9 @@ export const foldRslintFolderState = ( const kind = worstKind(states); return withDetail( kind, - joinDetails(states.filter((state) => state.kind === kind).map(detailOf)), + joinDetails( + states.filter((state) => state.kind === kind).map(stackStateDetail), + ), ); }; @@ -172,7 +167,7 @@ export const aggregateFolderStates = ( statuses .filter((entry) => entry.state.kind === kind) .map((entry) => { - const detail = detailOf(entry.state); + const detail = stackStateDetail(entry.state); if (!detail) return multiRoot ? entry.name : undefined; return multiRoot ? `${entry.name}: ${detail}` : detail; }), @@ -202,6 +197,6 @@ const withDetail = ( case 'disabled': return { kind: 'disabled', reason: detail }; case 'not-detected': - return { kind: 'not-detected' }; + return { kind: 'not-detected', detail }; } }; diff --git a/packages/vscode/src/statusBar.ts b/packages/vscode/src/statusBar.ts index 2eb7f8d..2635e36 100644 --- a/packages/vscode/src/statusBar.ts +++ b/packages/vscode/src/statusBar.ts @@ -4,6 +4,8 @@ import { type StackState, type StatusReporter, STACK_IDS, + // The hover renders a state's two halves apart: the kind as the icon, this as prose. + stackStateDetail, STACK_LABELS, stackCommand, stackCommandTitle, @@ -56,7 +58,9 @@ const STATE_STYLES: Readonly< 'not-detected': { icon: '$(circle-slash)', color: 'disabledForeground', - spellsOutDetail: false, + // Only set when a runtime found nothing to do, and then that detail is + // the one thing telling the user the stack is not working on purpose. + spellsOutDetail: true, severity: 0, }, disabled: { @@ -135,27 +139,6 @@ const tableColumns = (slots: number): number => 2 + slots; const CARD_WIDTH = 140; const CARD_WIDTH_WITH_NOTICES = 250; -/** - * The free text a state carries, if any — a crash message, a version - * complaint, a disable reason. It is its own function because the hover renders - * the two halves of a state in different places — the kind is the icon, the - * detail is prose — and the switch is exhaustive, so a state kind added to the - * union has to say here whether it carries words. - */ -const stateDetail = (state: StackState): string | undefined => { - switch (state.kind) { - case 'not-detected': - return undefined; - case 'disabled': - return state.reason; - case 'starting': - case 'running': - case 'crashed': - case 'version-mismatch': - return state.detail; - } -}; - /** * The one-line form: the state's kind, plus its detail when it has one. This * is what the log records and what the icon's native tooltip says, so its @@ -166,7 +149,7 @@ const stateText = (state: StackState): string => { // The ids read as prose once their hyphen is a space ('version-mismatch' → // 'version mismatch'); no kind has a second one. const kind = state.kind.replace('-', ' '); - const detail = stateDetail(state); + const detail = stackStateDetail(state); return detail ? `${kind} — ${detail}` : kind; }; @@ -388,7 +371,7 @@ export class StatusBar implements vscode.Disposable { // `stateText` embeds arbitrary text a stack produced, hence the escaping. const status = stateIcon(style, stateText(state)); const detail = style.spellsOutDetail - ? stateDetail(state)?.trim() + ? stackStateDetail(state)?.trim() : undefined; if (detail) { notices.push({ style, label, detail }); diff --git a/packages/vscode/src/types.ts b/packages/vscode/src/types.ts index efd2b52..e2e8277 100644 --- a/packages/vscode/src/types.ts +++ b/packages/vscode/src/types.ts @@ -47,15 +47,36 @@ export const stackCommandTitle = (stack: StackId): string => * `disabled` covers every "we deliberately did not start" case: the kill-switch * settings, Restricted Mode, and phase-gated stacks. The reason is shown to the * user, so it must be a complete sentence fragment. + * + * `not-detected` carries a detail only to say why a runtime found nothing to + * do after detection lit the stack; detection itself never sets one. */ export type StackState = - | { readonly kind: 'not-detected' } + | { readonly kind: 'not-detected'; readonly detail?: string } | { readonly kind: 'disabled'; readonly reason?: string } | { readonly kind: 'starting'; readonly detail?: string } | { readonly kind: 'running'; readonly detail?: string } | { readonly kind: 'crashed'; readonly detail: string } | { readonly kind: 'version-mismatch'; readonly detail: string }; +/** + * The free text a state carries, if any — a crash message, a version + * complaint, a disable reason. The switch is exhaustive, so a state kind added + * to the union has to say here whether it carries words. + */ +export const stackStateDetail = (state: StackState): string | undefined => { + switch (state.kind) { + case 'disabled': + return state.reason; + case 'not-detected': + case 'starting': + case 'running': + case 'crashed': + case 'version-mismatch': + return state.detail; + } +}; + /** Raw runtime failures that need dependency recovery, not shell gate states. */ export const isFailedStackState = ( kind: StackState['kind'] | 'stopped', diff --git a/packages/vscode/tests/stacks/lint/start.test.ts b/packages/vscode/tests/stacks/lint/start.test.ts index b39e443..4377982 100644 --- a/packages/vscode/tests/stacks/lint/start.test.ts +++ b/packages/vscode/tests/stacks/lint/start.test.ts @@ -374,7 +374,7 @@ function refreshConfig(runtime: Rslint, reason: string): Promise { ).requestConfigRefresh(reason); } -it('keeps a bridged runtime running with a detail when the shim finds no define.lint()', async () => { +it('reports a bridged runtime as not-detected with a detail when the shim finds no define.lint()', async () => { refreshOutcome = 'unconfigured'; const { runtime, states, warnings, errors, infos } = createRuntime({ mode: 'bridged', @@ -383,7 +383,7 @@ it('keeps a bridged runtime running with a detail when the shim finds no define. }); runtime.setBridgeConfigPath('/project/rstack.config.ts'); const unconfigured = { - kind: 'running', + kind: 'not-detected', detail: 'no define.lint() in rstack.config.ts', }; @@ -396,7 +396,7 @@ it('keeps a bridged runtime running with a detail when the shim finds no define. expect(notices()).toEqual([ 'Rslint has nothing to lint: no define.lint() in rstack.config.ts. Add define.lint(...) to enable it.', ]); - // Healthy: the dependency poll has nothing to retry. + // Not a failure: the dependency poll has nothing to retry. expect(runtime.retryConfigDependency()).toBeUndefined(); // An unchanged episode logs once; Go rejecting with a last-good catalog is diff --git a/packages/vscode/tests/stacks/lint/status.test.ts b/packages/vscode/tests/stacks/lint/status.test.ts index da9b19e..c0e0536 100644 --- a/packages/vscode/tests/stacks/lint/status.test.ts +++ b/packages/vscode/tests/stacks/lint/status.test.ts @@ -4,12 +4,17 @@ import { aggregateFolderStates, attributeToCore, foldRslintFolderState, + liveRslintStatus, missingPackageOf, RslintVersionMismatchError, - runningRslintStatus, statusForRslintStartFailure, } from '../../../src/stacks/lint/status'; +const UNCONFIGURED = { + kind: 'not-detected', + detail: 'no define.lint() in rstack.config.ts', +} as const; + describe('Rslint status classification', () => { it('disables a folder whose package is not installed', () => { // Both not-installed shapes take the uniform disabled state (AGENTS.md): @@ -83,18 +88,24 @@ describe('Rslint status classification', () => { }); it('surfaces a configured Node advisory without stopping the worker', () => { - expect(runningRslintStatus()).toEqual({ kind: 'running' }); - expect(runningRslintStatus('Node 22.17 is below the floor')).toEqual({ + expect(liveRslintStatus()).toEqual({ kind: 'running' }); + expect(liveRslintStatus('Node 22.17 is below the floor')).toEqual({ kind: 'version-mismatch', detail: 'Node 22.17 is below the floor', }); expect( - runningRslintStatus('Node 22.17 is below the floor', 'idle'), + liveRslintStatus('Node 22.17 is below the floor', UNCONFIGURED.detail), ).toEqual({ kind: 'version-mismatch', detail: 'Node 22.17 is below the floor', }); }); + + it('reports an unconfigured bridge as not-detected with its reason', () => { + expect(liveRslintStatus(undefined, UNCONFIGURED.detail)).toEqual( + UNCONFIGURED, + ); + }); }); describe('attributeToCore', () => { @@ -183,6 +194,20 @@ describe('foldRslintFolderState', () => { ]), ).toEqual({ kind: 'disabled', reason: 'rstack is not installed in /w' }); }); + + it('keeps an unconfigured bridge as the folder state, detail included', () => { + // The fold starts from `not-detected`, so the shim's refusal is not + // masked into `running: idle`, and its reason survives for the hover. + expect(foldRslintFolderState([UNCONFIGURED])).toEqual(UNCONFIGURED); + }); + + it('lets a healthy native runtime outrank an unconfigured bridge', () => { + // The folder is linting; "nothing configured" for its bridged documents + // is not worth showing over that. + expect(foldRslintFolderState([UNCONFIGURED, { kind: 'running' }])).toEqual({ + kind: 'running', + }); + }); }); describe('aggregateFolderStates', () => { @@ -203,6 +228,19 @@ describe('aggregateFolderStates', () => { ).toEqual({ kind: 'running', detail: 'idle' }); }); + it('keeps a single-root unconfigured reason; a healthy sibling folder wins', () => { + expect( + aggregateFolderStates([{ name: 'app', state: UNCONFIGURED }]), + ).toEqual(UNCONFIGURED); + // Same rank as inside a folder: a healthy sibling folder wins. + expect( + aggregateFolderStates([ + { name: 'app', state: UNCONFIGURED }, + { name: 'lib', state: { kind: 'running', detail: 'idle' } }, + ]), + ).toEqual({ kind: 'running', detail: 'lib: idle' }); + }); + it('reports starting before any folder registered', () => { expect(aggregateFolderStates([])).toEqual({ kind: 'starting' }); }); diff --git a/packages/vscode/tests/statusBar.test.ts b/packages/vscode/tests/statusBar.test.ts index 3b5d518..6aadc54 100644 --- a/packages/vscode/tests/statusBar.test.ts +++ b/packages/vscode/tests/statusBar.test.ts @@ -257,6 +257,25 @@ describe('StatusBar hover', () => { expect(rowsOf(html())).toHaveLength(7); }); + it('spells out why a runtime found nothing to do, but not plain detection', () => { + const { bar, html } = build(); + // Detection's own `not-detected` carries no detail and adds nothing. + bar.setState('fmt', { kind: 'not-detected' }); + expect(noticesOf(html())).toEqual([]); + expect(rowsOf(html())).toHaveLength(7); + + bar.setState('rslint', { + kind: 'not-detected', + detail: 'no define.lint() in rstack.config.ts', + }); + expect(stackRows(html())[0]).toContain( + 'title="not detected — no define.lint() in rstack.config.ts"', + ); + expect(noticesOf(html())[0]).toContain( + 'Rslint
no define.lint() in rstack.config.ts', + ); + }); + it('spells out why a stack was deliberately turned off', () => { const { bar, html } = build(); bar.setState('fmt', {