Skip to content

Commit 748c546

Browse files
fi3eworkfi3ework
authored
feat(vscode): format through rs fmt --lsp on the User Node runtime (#12)
* feat(vscode): format through rs fmt --lsp on the User Node runtime rstack-cli 0.5.2 ships an LSP mode for rs fmt; the fmt stack becomes a vscode-languageclient client of one rs fmt --lsp server per detected workspace folder, replacing the spawn-per-request --stdin-filepath path and its pre-spawned standby machinery. - One server per workspace folder, anchored at the folder root so the editor formats exactly like rs fmt run from that root; deepest-config anchoring is removed (docs/adr/0002-fmt-lsp-on-user-node-runtime.md). - The server runs on the User Node runtime picked by the shared preflight; nodeResolution moves from stacks/test/ to shared/, and the escape hatch becomes the resource-scoped rstack.nodeExecutable (rstest.nodeExecutable and rstack.rstest.nodeExecutable migrate to it; two pins set in one layer plan one write plus a superseded skip). - Version gate SUPPORT_MATRIX.rstack >= 0.5.2, no stdin fallback. - Config create/change/delete restarts the owning folder's server (debounced); rstack.restart also resets the host-wide Node caches so a restart behaves like a window reload for runtime selection. - E2E: run locally as VSCODE_CLI=1 pnpm test:e2e <slice> so the extension host keeps the caller's PATH (documented in AGENTS.md). * fix(vscode): fold per-folder fmt states into one severity-ranked status In a multi-root workspace every FmtFolderRuntime wrote to the stack's single status reporter directly, so a folder that started or recovered after a sibling failed replaced the failure with a global 'running' — the displayed state depended on event ordering. Runtimes now record their state and detail and notify the controller, which is the one writer to the shell: the folder set is folded by severity (crashed > version mismatch > disabled > pin advisory > running), so a healthy sibling never overwrites another folder's failure and a recovery clears only its own. Failure details name the folder they belong to. ADR 0002 records the fold. * refactor(vscode): extract the fmt status fold into a testable pure module /simplify pass over the aggregation fix, converging with review feedback: - stacks/fmt/status.ts: foldFolderStatus() owns severity (an exhaustive rank table, so a new state cannot silently fall through to running), multi-root folder-name prefixes with tie joining, and the healthy-sibling-never-masks-a-failure invariant — now unit-tested, which the single-folder E2E fixture structurally cannot cover. - The status reports starting, not running, until a folder's server is actually up (during Node preflight and server initialization no formatting provider exists yet); stopped ranks with starting, so a config-change restart no longer flashes running. - A running folder's pin advisory folds at version-mismatch rank, above disabled — matching the test stack's configured-pin advisory instead of contradicting it. - Runtime details are folder-agnostic; the fold owns the prefixing, so the Node-preflight message is attributable in a multi-root window too. - setAdvisory() joins setState() as the only notify paths. * fix(vscode): make fmt teardown immune to a hung initialize and detection flaps Two lifecycle holes in the per-folder fmt runtime, from review: - A server that spawned but never answers the LSP initialize request held the runtime's serialized queue inside client.start(), so a queued stop or restart never reached the process teardown — a hung shell restart and a live orphan. stop()/restart() now interrupt an in-flight start by closing the process owner outside the queue, which fails the pending initialize and lets the queue drain. The interrupt window is exact (#startInFlight spans only the client.start() await): a healthy server keeps its graceful LSP shutdown, and a manufactured failure ends in 'stopped', not a spurious 'crashed'. - A folder that lost and regained detection across two passes could run two servers at once: reconcile deleted the map entry and stopped the runtime asynchronously, and the replacement spawned immediately. The controller now reserves the folder (#retiring, identity-guarded) and the replacement start awaits the predecessor's retirement inside its own queue, so a config event during the wait lines up behind it instead of spawning early. dispose() drains retiring runtimes too. * fix(vscode): scope the Node-memo reset to full restarts and log e2e failures Review round three, plus the CI diagnosis groundwork: - runRestart resets the host-scoped User Node preflight memo only when no consumer stack (rstest, fmt) survives the retire wave. A single-stack rstack.fmt.restart next to a live Rstest controller kept clearing the decision its existing workers were built on, so the next worker spawn could silently re-probe onto a different runtime. The full rstack.restart — the 'like a window reload' gesture — still always clears it, as does the batched restart a rstack.nodeExecutable change triggers. Two unit tests pin both sides. - Nested workspace folders (parent and subdirectory both detected for fmt) are recorded as a documented limitation in AGENTS.md and ADR 0002: the supported shape is sibling folders, and per-document routing was considered and deferred. - The rstest e2e suite names each failing test via console.error: the extension host's stdout (mocha's reporter) is not forwarded to CI logs, so a CI-only failure was previously unidentifiable. * test(vscode): mirror the rstest output channel to stderr in CI A CI-only E2E failure on windows-latest (every rstest suite discovering zero tests since @rstest/core 0.11.7 / @rspack/core 2.1.10 floated in) cannot be diagnosed from the CI log: worker stdout/stderr land only in the output channel, which CI cannot open. Gate a console.error mirror of every log entry behind RSTACK_E2E_MIRROR_LOGS=1 and set it from the rstest E2E harness in CI, so the next failing run names the actual error. * docs(vscode): record the uniform rstack floor as a decision Raising SUPPORT_MATRIX.rstack to >=0.5.2 for rs fmt --lsp also gates the Rstest bridge, which checks the same entry — a project on rstack 0.3.5-0.5.1 reports version mismatch for tests too. Review flagged the side effect; keeping one toolchain-wide floor is the deliberate answer (per-stack rstack floors considered and rejected), so state it in the matrix doc and ADR 0002 instead of splitting the entry. * docs(vscode): tighten the new AGENTS.md entries * fix(vscode): retry failed fmt folder runtimes on detection passes A folder whose runtime had failed (disabled, version mismatch, crashed) was kept as-is by every reconcile, so the install or upgrade that fixed it was never picked up — formatting stayed dead until a manual restart, while the test and lint stacks both retry on the same detection pass. Restart such a runtime in place, on the path a config change already uses, which re-runs package resolution, the version check and the Node preflight. Healthy and starting runtimes stay untouched. The one recovery no watcher sees — an install that changes no lockfile — now has its way out written into the disabled status message, which names the restart command. * chore(vscode): drop compat for this extension's unpublished states The rstack.rstest.nodeExecutable -> rstack.nodeExecutable mapping covered a rename that happened before the extension was ever published, so no settings file can hold the legacy key. Remove it together with the superseded-skip and collision machinery that existed only for the two-sources-one-target case, and the prompt clause naming the phantom source. Record the two policies this acts on in AGENTS.md: pre-1.0.0 the extension breaks freely (only the latest released tools need support), and the three tools are treated uniformly by default, diverging only when a tool forces it. * fix(vscode): answer any relevant settings change with one full restart The listener decided per stack between two paths: a gate change went to the reconcile, a declared restartOnSettings change to a targeted restart, and a stack in both sets was left to the reconcile. That split swallowed the restart when one save wrote a gate key at its already-effective value alongside a shared setting — the reconcile saw a live controller behind a still-open gate and kept it, so the stack stayed on the previous Node executable. Settings edits are rare, so selectivity bought nothing but that hole: any relevant key now triggers one full restart pass, which re-evaluates every gate and rebuilds every controller — a gate flip in either direction, a moved shared setting, or both in one save are handled by construction. The dead array arm of restart()/runRestart() and the unused reconcile() parameter go with it. * docs: scope the glossary to what this branch ships The Generated-shim and Bridged-folder entries describe the lint bridge, which is not on this branch — they move to the branch that introduces it. Config root claimed the workspace folder root for every tool, but the test stack keeps upstream's per-project cwd rule (adaptation 5); scope the anchor to the fmt server.
1 parent 5771367 commit 748c546

37 files changed

Lines changed: 1614 additions & 1795 deletions

‎CONTEXT.md‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ Glossary of terms used across rstack-editor. Code, docs, commit messages and rev
1414
- **VS Code Node runtime** — the Node.js shipped inside VS Code, which the extension host itself runs on. Its version follows VS Code's release cadence, and it is Electron's Node, on a different ABI line from plain Node. _Avoid_: host runtime, extension host runtime.
1515
- **User Node runtime** — the Node.js the user's own environment provides, discovered by the extension rather than shipped with it. _Avoid_: worker runtime, project-side Node.
1616
- **Load bound** — the limit on what a piece of work can end up loading: what the extension ships, plus ABI-stable N-API bindings. Work that stays inside the bound may run on the VS Code Node runtime; work that can load project code has no load bound and belongs on a User Node runtime. _Avoid_: load surface.
17-
- **Preflight** — the check that picks a User Node runtime, run once per extension host before any worker is spawned. Its failure is a status, never a crash.
17+
- **Preflight** — the check that picks a User Node runtime, run once per extension host and shared by every process that loads project code (the test worker, the fmt server). Its failure is a status, never a crash.
1818
- **Runtime floor** — the version range a User Node runtime must satisfy (`NODE_RUNTIME_RANGE` in `shared/versionCheck.ts`). A declared support contract, not a probed capability.
1919

2020
## Tools and configs
@@ -24,13 +24,10 @@ Glossary of terms used across rstack-editor. Code, docs, commit messages and rev
2424
- **Rstack config** — the unified `rstack.config.*` file consumed by rstack-cli (`rs`), holding per-tool sections. Tools never read it themselves; `rs` hands each tool its section through a shim.
2525
- **Shim** — the module rstack-cli ships per tool that loads the Rstack config and exposes that tool's section through the tool's ordinary explicit-config channel. The extension points upstream machinery at the shim rather than re-implementing Rstack config semantics.
2626
- **Bridged project** — a test project the extension synthesizes for a directory whose test signal is a Rstack config, wired to the shim. _Avoid_: virtual project, rstack project.
27+
- **Config root** — the directory a tool's config is loaded from, which is also the directory the tool's process stands in. For the fmt server the editor anchors it at the workspace folder root, so it loads the config a terminal opened on that folder would, and a subproject that needs its own config becomes its own workspace folder. The test stack does not share this anchor: a project's cwd is set per project (for native configs, upstream's config-file-directory rule). _Avoid_: config directory, project root.
2728
- **Ownership** — the editor-side rule assigning a directory to one tool when both a native config and a Rstack config are present there: the atomic tool's native config wins and the bridge yields. This rule exists only in the editor; upstream CLIs never face the choice, since each reads only its own config.
2829

2930
## fmt
3031

31-
- **Cold format** — a format request served by spawning a fresh `rs fmt` process at request time; the request pays the full process start-up cost.
32-
- **Standby** — the single pre-spawned `rs fmt` process held ready for one specific file, so the next format of that file skips the start-up cost. There is at most one standby, and it is only ever armed for the active editor's file ("the standby tracks the active editor"). An editor change that cannot be armed kills it; an editor holding nothing this stack formats leaves it to expire.
33-
- **Arm** — create the standby for a file. Arming happens when the active editor lands on an eligible file and again right after a format consumed the previous standby.
34-
- **Consume** — serve a format request with the armed standby. A standby serves exactly one request; a request the standby cannot serve falls back to a cold format.
35-
- **Hot format** — a format request served by consuming the standby.
36-
- **Expire** — kill an idle standby to reclaim its memory. An expired standby is not an error; the next eligible event simply arms a new one.
32+
- **Fmt server** — the `rs fmt` language server the extension runs for one workspace folder, and the only thing that formats documents in it. It loads that folder's config root once and holds it for its lifetime, so a config change is a **restart** of the server, never a message to it. _Avoid_: formatter daemon, fmt worker.
33+
- **Fmt folder set** — the workspace folders that currently have a fmt server, kept in step with detection: a newly detected folder gains one, a folder that loses detection loses its own, and a folder in both sets keeps the server it already has.

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ The extension takes its configuration from five sources. The tool-native configs
2121
| `rslint.config.*` | **Supported.** Diagnostics, quick fixes and the language server, all resolved from the `@rslint/core` installed in your project. |
2222
| `rstest.config.*` | **Supported.** Test discovery, run and debug, watch mode, coverage and snapshot updates in the Test Explorer. |
2323
| `define.test()` in `rstack.config.*` | **Supported.** Tests run through the same config shim `rs test` uses, so the editor and the CLI resolve the config identically. |
24-
| `define.fmt()` in `rstack.config.*` | **Supported.** Document formatting through the project-local `rs fmt --stdin-filepath`, resolving the config the same way the CLI does; an `rs fmt` language server is the longer-term path. |
24+
| `define.fmt()` in `rstack.config.*` | **Supported.** Document formatting through the project-local `rs fmt` language server (`rs fmt --lsp`), one per workspace folder, loading the config from the folder root — the same config `rs fmt` run there would use. Needs `rstack` 0.5.2 or newer. |
2525
| `define.lint()` in `rstack.config.*` | **Planned.** Linting a project configured only through `rstack.config.*` needs upstream changes in Rslint and rstack-cli before the editor can evaluate it correctly. `rs lint` on the command line is unaffected. |
2626

2727
## License

‎docs/adr/0001-node-runtime-selection.md‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ Native type stripping is the _only_ thing on the worker's path that needs more t
2222

2323
The interactive-shell probe runs with its cwd set to the first detected workspace folder that does not pin `nodeExecutable`. The probe is cwd-sensitive: version managers resolve version files (`.nvmrc`, `.node-version`) against the shell's working directory, and fnm's default `version-file-strategy = local` never walks upward — a shell spawned from the extension host's own cwd (typically `/`) cannot see any project's version file and answers with the manager's global default (measured: a repository pinning 26 in `.nvmrc`, the probe answering with the 20.x global default). Standing in the workspace folder is what makes the probe answer the question it exists to answer: what a terminal opened on this project would say.
2424

25-
The probe stays one-per-host — one PATH, one shell, one interactive start-up cost, and the fallback notice must fire once, not once per project — so one directory has to stand for the whole window. Two entry points share the memo, first caller wins: the activation warm-up, almost always first, derives its standpoint and its own reason to exist from one query — the first detected folder without a pinned `nodeExecutable` both proves the memo has a reader (pinned folders never read it) and is where the probe stands; the worker spawn path, first only when the warm-up found every folder pinned, stands in that project's cwd, the directory it is about to run the worker in. The folder root rather than a project directory is the deliberate default: version files overwhelmingly sit at the repository root, which in a monorepo is _above_ the package that owns the config.
25+
The probe stays one-per-host — one PATH, one shell, one interactive start-up cost, and the fallback notice must fire once, not once per project — so one directory has to stand for the whole window. The entry points share the memo, first caller wins: the activation warm-up, almost always first, derives its standpoint and its own reason to exist from one query — the first detected folder without a pinned `nodeExecutable` both proves the memo has a reader (pinned folders never read it) and is where the probe stands; the worker spawn path, first only when the warm-up found every folder pinned, stands in that project's cwd, the directory it is about to run the worker in; a fmt server start (ADR 0002) stands in its own workspace folder root, and gets there first whenever no rstest warm-up preceded it. The folder root rather than a project directory is the deliberate default: version files overwhelmingly sit at the repository root, which in a monorepo is _above_ the package that owns the config.
2626

2727
**Per-project probes** — rejected: N interactive shells for what is in practice a repository-level convention, and a window that genuinely needs a different Node per folder is `nodeExecutable`'s case — that setting is read per folder already.
2828

@@ -34,18 +34,18 @@ The rule is not "never use it". The line is the **load bound**: work whose loads
3434

3535
Note that _worker_ names a process, not a runtime. The worker is our own code; the runtime it runs on is the user's.
3636

37-
### The line is drawn for the test worker only
37+
### Where the line is drawn today
3838

39-
This decision is implemented for one path: the rstest worker. Two others sit on the wrong side of the line today, and this ADR does not move them. Naming them, so the rule is not read as an invariant the extension already holds:
39+
This decision was written for one path, the rstest worker, and named two others that sat on the wrong side of the line. One of them has since moved:
4040

41-
- **fmt** spawns the project's `rs` bin on `process.execPath` with `ELECTRON_RUN_AS_NODE=1` (`stacks/fmt/run.ts`) — the VS Code Node runtime — and `rs fmt` loads the project's config in that process (`stacks/fmt/index.ts`). Unbounded load, no floor, no preflight.
41+
- **fmt** used to spawn the project's `rs` bin on `process.execPath` with `ELECTRON_RUN_AS_NODE=1` (`stacks/fmt/run.ts`) — the VS Code Node runtime — and let `rs fmt` load the project's config in that process: unbounded load, no floor, no preflight. It now runs `rs fmt --lsp` as a language server on a User Node runtime chosen by this decision's own logic, against the same floor, with the shared `rstack.nodeExecutable` as its escape hatch. Why the server, and why one per workspace folder: `docs/adr/0002-fmt-lsp-on-user-node-runtime.md`.
4242
- **lint** imports the project's `@rslint/core/config-loader` into the extension host and loads the user's `rslint.config.ts` there (`stacks/lint/configLoader.ts`), and runs user plugin rules on the same runtime (`stacks/lint/PluginLintPool.ts`). `stacks/lint/jitiPreflight.ts` already records the resulting divergence in so many words: that loader "runs on the extension host's Node — whose version is fixed by VS Code, not by the user — so the jiti branch can trigger in the editor even when the CLI works fine". Its answer is a diagnostic, not a runtime choice.
4343

44-
Neither is cheap to move — each needs its own spawn-and-protocol work — and neither has a reported bug behind it yet. Known debt, deliberately: the next stack to load project code should follow the rule, and nobody should describe the rule as already universal.
44+
Lint is what moving costs when it is not cheap: fmt's move needed a whole upstream language server to exist first, and lint needs its own spawn-and-protocol work for the config loader and the plugin host, with no reported bug behind it yet. It stays known debt, deliberately — the rule is not universal until that entry is gone, and nobody should describe it as if it were.
4545

4646
## Consequences
4747

48-
- An explicit `rstack.rstest.nodeExecutable` is always honoured, but it is probed too: falling short of the floor produces a status, not a refusal. The escape hatch stays an escape hatch; it stops being silent.
48+
- An explicit `rstack.nodeExecutable` (shared with the fmt server since ADR 0002; the standalone Rstest extension's `rstest.nodeExecutable` migrates to it) is always honoured, but it is probed too: falling short of the floor produces a status, not a refusal. The escape hatch stays an escape hatch; it stops being silent.
4949
- A below-floor configured executable is reported through the same status as "no runtime found at all", so the two messages must state their _consequence_ explicitly — one says tests will not run, the other says the extension is running with it anyway.
5050
- The interactive-shell probe is the recovery path and does not exist on Windows (no `-i -c` equivalent reliably evaluates a user's profile across cmd and PowerShell). A Windows user whose PATH `node` is below the floor gets the failure status with no second candidate.
5151
- `NODE_OPTIONS` can carry `--no-strip-types`, which defeats the floor on any version. Deliberately not detected: the same setting breaks `rs test` in the terminal, so the editor failing identically is correct, and special-casing one flag would be permanent trivia bought for one diagnostic.

0 commit comments

Comments
 (0)