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
3 changes: 2 additions & 1 deletion packages/vscode/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten

- **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. Only the **latest released** `rstack`, `@rstest/core` and `@rslint/core` need support: whenever a change touches a floor in `SUPPORT_MATRIX`, set it to the latest release at that time — do not reason about which older release would still work — and raise it without a transition story (the floor status names the required version). 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.*`).
- **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.
- **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 names the restart command as the way out (ADR 0002: an install that changes no lockfile fires no detection pass), one `warn` line in the output channel without a stack trace, never a `crashed` status and never a notification. 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. Lint's report lives in the `onDocumentFailure` hook (`stacks/lint/index.ts`), which owns the log line too, so the upstream-tracked `RuntimeManager` only defers to it. Rstest classifies the config-import case in the worker (`missingDependencyCauseOf`: Node's `code`, a bare — package-name — specifier, and for a subpath a walk-up proving the package really is absent, so a typo'd relative import or a missing subpath of an installed package stays a real error) because the IPC channel drops the `code` — `NormalizedConfigResult` carries the verdict as data end to end, and `Project` branches on it. The config-import case is implemented for Rstest only today — lint and fmt load configs inside their own servers and cannot classify there yet (#30).
- One stack failing to register or crashing must never take another stack (or the shell) down.
- The shell always activates; per-folder config detection decides which stacks start, and re-runs on config/lockfile changes without a window reload. Enable-settings are coarse kill switches only.
- 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.
Expand All @@ -38,7 +39,7 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten

- 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.
- **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.
- **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). 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. 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`.
Expand Down
67 changes: 67 additions & 0 deletions packages/vscode/src/shared/notInstalled.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import {
COMMAND_CATEGORY,
STACK_LABELS,
type StackId,
stackCommandTitle,
} from '../types';

/**
* The not-installed policy's words, once for all three stacks (AGENTS.md
* rules): a project whose dependencies are not installed is a `disabled`
* status whose reason names the way out, plus one `warn` line in the output
* channel. The stacks share the wording the way they share
* `formatVersionMismatch` — each keeps its own status machinery, but what
* the user reads is one sentence, not three near-copies.
*
* The trailing hint covers the recovery no watcher sees: an install that
* changes no lockfile (a fresh clone whose lockfile is already current) fires
* no detection pass, so the restart command is the way out and the status is
* where it has to be named (ADR 0002).
*/
const restartHint = (stack: StackId): string =>
`then run "${COMMAND_CATEGORY}: ${stackCommandTitle(stack, 'restart')}" if this status stays`;

/** The `disabled` reason for a package the stack needs and cannot find. */
export const formatNotInstalledStatus = (
stack: StackId,
packageName: string,
): string =>
`${packageName} is not installed (node_modules missing) — install it, ${restartHint(stack)}`;

/**
* The `disabled` reason for a config that evaluates but imports a package
* that is not there. `configPath` is workspace-relative: the status has no
* room for more.
*/
export const formatConfigDependencyMissingStatus = (
stack: StackId,
configPath: string,
): string =>
`${configPath} imports a package that is not installed — install the project dependencies, ${restartHint(stack)}`;

/**
* The output-channel line for a config that imports a package that is not
* installed. `cause` is the loader's own first line, which names the
* specifier and the importer.
*/
export const formatConfigDependencyMissingLog = (
stack: StackId,
configPath: string,
cause: string,
): string =>
`Cannot load ${configPath}: ${cause}. Install the project dependencies to enable ${STACK_LABELS[stack]} for this config.`;

/**
* The output-channel line: where the stack looked, plus the stack's own
* consequence — the same shape as the shared Node preflight message
* (adaptation 6), where each caller appends what the state means for it.
*/
export const formatNotInstalledLog = (
packageName: string,
folderName: string,
searchedFrom: string,
consequence?: string,
): string =>
`${packageName} is not installed in ${folderName} (node_modules missing); searched from ${searchedFrom}${
consequence ? `; ${consequence}` : ''
}`;
11 changes: 6 additions & 5 deletions packages/vscode/src/stacks/fmt/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import {
} from 'vscode-languageclient/node';
import { RSTACK_CONFIG_GLOB } from '../../detection';
import { getConfiguredNodeExecutable } from '../../shared/nodeExecutableSetting';
import {
formatNotInstalledLog,
formatNotInstalledStatus,
} from '../../shared/notInstalled';
import {
configuredNodeBelowFloor,
NODE_EXECUTABLE_SETTING,
Expand Down Expand Up @@ -299,12 +303,9 @@ class FmtFolderRuntime {
// install that changes no lockfile (a fresh clone whose lockfile is
// already current) fires no file event, so nothing rebuilds this
// runtime — the status message is where the way out has to live.
this.setState(
'disabled',
'rstack is not installed (node_modules missing) — install it, then run "Rstack: Restart rs fmt" if this status stays',
);
this.setState('disabled', formatNotInstalledStatus('fmt', 'rstack'));
Comment thread
fi3ework marked this conversation as resolved.
context.output.warn(
`rstack is not installed in ${this.folder.name} (node_modules missing); searched from ${folderRoot}`,
formatNotInstalledLog('rstack', this.folder.name, folderRoot),
);
return;
}
Expand Down
27 changes: 17 additions & 10 deletions packages/vscode/src/stacks/lint/RuntimeManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ export interface DocumentResolutionFailure {
readonly error: unknown;
/** The core whose runtime failed to start; absent when resolution itself failed. */
readonly resolved?: ResolvedCoreRuntime;
/** The package directory of the runtime the document keeps (last-good), if any. */
readonly keeping?: string;
}

export interface RuntimeManagerOptions {
Expand Down Expand Up @@ -394,7 +396,9 @@ export class RuntimeManager {
* category. Here the same event becomes a folder status entry (the stack
* owns no UI chrome), so no deduplication is needed: a status is a value,
* not a notification, and the controller replaces the document's previous
* one. The Output-channel line stays.
* one. The hook owns the whole report, the Output-channel line included, so
* its level and wording live in lint-owned code; upstream's line is kept
* only for a manager without a hook.
*/
private reportFailure(
document: TextDocument,
Expand All @@ -403,19 +407,22 @@ export class RuntimeManager {
existing: RuntimeEntry | undefined,
resolved?: ResolvedCoreRuntime,
): void {
const suffix = existing
? ` (keeping ${existing.resolved.installation.packageDirectory} active)`
: '';
const keeping = existing?.resolved.installation.packageDirectory;
if (this.options.onDocumentFailure) {
this.options.onDocumentFailure({
document,
workspaceFolder,
error,
resolved,
keeping,
});
return;
}
const suffix = keeping ? ` (keeping ${keeping} active)` : '';
this.logger.error(
`Could not select an Rslint core for ${document.uri}${suffix}`,
error,
);
this.options.onDocumentFailure?.({
document,
workspaceFolder,
error,
resolved,
});
}

private isCurrentDocument(document: TextDocument, epoch: number): boolean {
Expand Down
32 changes: 31 additions & 1 deletion packages/vscode/src/stacks/lint/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
StackState,
} from '../../types';
import { NODE_EXECUTABLE_SETTING } from '../../shared/nodeResolution';
import { formatNotInstalledLog } from '../../shared/notInstalled';
import { CoreResolver, type ResolvedCoreRuntime } from './CoreResolver';
import { Logger } from './logger';
import { Rslint } from './Rslint';
Expand All @@ -17,6 +18,7 @@ import {
attributeToCore,
foldRslintFolderState,
statusForRslintStartFailure,
missingPackageOf,
} from './status';
import { WorkspaceDocumentRouter } from './WorkspaceDocumentRouter';

Expand Down Expand Up @@ -198,7 +200,35 @@ class RslintController implements StackController {
logger,
{
folderMode: (folder) => this.folderMode(folder),
onDocumentFailure: ({ document, workspaceFolder, error, resolved }) => {
onDocumentFailure: ({
document,
workspaceFolder,
error,
resolved,
keeping,
}) => {
// The hook owns the report. The Output-channel line: a folder whose
// `rstack` or `@rslint/core` is not installed is the not-installed
// state (AGENTS.md) — one warn line, no stack; anything else is
// upstream's error. A document with a last-good runtime still
// lints, so its consequence says what it keeps, not "will not".
const missing = missingPackageOf(error);
Comment thread
fi3ework marked this conversation as resolved.
if (missing !== undefined) {
logger.warn(
formatNotInstalledLog(
missing,
workspaceFolder.name,
workspaceFolder.uri.fsPath,
`${document.uri} ${keeping ? `keeps ${keeping}` : 'will not lint'} until it is installed`,
),
);
} else {
const suffix = keeping ? ` (keeping ${keeping} active)` : '';
logger.error(
`Could not select an Rslint core for ${document.uri}${suffix}`,
error,
);
}
// Last-good semantics: the document keeps whatever runtime it had.
// The failure is still the folder's worst news, so it is folded in
// beside the runtimes rather than shown as a toast. A start failure
Expand Down
5 changes: 4 additions & 1 deletion packages/vscode/src/stacks/lint/resolution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,11 @@ function resolveConfiguredCore(
try {
if (!fs.statSync(packageJsonPath).isFile()) throw new Error('not a file');
} catch (error) {
// Not `missing-core`: the user pointed `corePath` at this directory, so
// the fix is correcting the setting, not installing dependencies — it must
// not take the not-installed state (`missingPackageOf`).
throw new RslintResolutionError(
'missing-core',
'invalid-package',
`Could not access @rslint/core at ${directory}`,
{ cause: error },
);
Expand Down
Loading