Skip to content

Commit 72b3184

Browse files
fi3eworkfi3ework
authored
feat(vscode): not-installed state for config imports, recover by polling (#51)
* feat(vscode): classify lint config dependency failures * feat(vscode): report lint config dependencies as not installed * feat(vscode): suppress fmt config dependency toasts * feat(vscode): poll for installed dependencies * docs(vscode): record not-installed recovery decision * fix(vscode): preserve config dependency failure states * refactor(vscode): tighten recovery integration seams * test(vscode): assert dependency recovery episodes * fix(vscode): complete recovery episode contracts * test(vscode): spawn pnpm portably in recovery E2E * fix(vscode): preserve rejected config refresh errors * fix(vscode): limit config retries and close failed workers * fix(vscode): attribute bridge failures to the root config * fix(vscode): retain config source-change retries * fix(vscode): retry missing Rstest core installations * fix(vscode): defer startup config race failures * refactor(vscode): remove dependency poll test counter * refactor(vscode): inline lint startup reporting guards * refactor(vscode): collapse fmt message presentation * refactor(vscode): inline raw not-installed folds * refactor(vscode): share config dependency failure type * refactor(vscode): discriminate lint config refresh verdicts * refactor(vscode): share not-installed warning episodes * refactor(vscode): reset config refresh errors per request * refactor(vscode): set fmt config after construction * refactor(vscode): carry root bridge config in detection * refactor(vscode): import lint protocol directly * refactor(vscode): require config dependency observer * refactor(vscode): make status report hook parameterless * refactor(vscode): consume config dependency verdict on read * refactor(vscode): record E2E warnings at output channels * refactor(vscode): name formatting episode mutation explicitly * fix(vscode): pick the root rstack config by loader precedence * fix(vscode): retry a core lost after config load * fix(vscode): refresh bridge attribution on detection * fix(vscode): prefer real config errors over missing dependencies * feat(vscode): poll every minute until every stack is running * fix(vscode): replace stopped runtimes on dependency polls * fix(vscode): keep failed fmt sessions polling, dedupe Rstest resolve toasts * fix(vscode): poll raw Rstest failures, unblock lint reconcile from hung refreshes * refactor(vscode): ablate unneeded abstractions and mirror tests * fix(vscode): report raw Rstest failures to the shell, rebuild watchers after recovery * refactor(vscode): simplify not-installed recovery paths * fix(vscode): recollect Rstest files after unchanged-config recovery, latch resolution logs
1 parent 7d0b829 commit 72b3184

64 files changed

Lines changed: 3176 additions & 294 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ packages/vscode/.playground/
155155
packages/vscode/e2e/fixtures/*/node_modules/
156156
packages/vscode/e2e/fixtures/*/pnpm-lock.yaml
157157
packages/vscode/e2e/lint/fixtures/pnpm-lock.yaml
158+
packages/vscode/e2e/lint/fixtures/*/pnpm-lock.yaml
158159
packages/vscode/e2e/rstest/fixtures/*/pnpm-lock.yaml
159160

160161
# Build-time copy of the workspace root LICENSE (see rslib.config.mts)

‎docs/adr/0002-fmt-lsp-on-user-node-runtime.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,6 @@ Falling back to the VS Code Node runtime stays rejected — ADR 0001's load-bear
3737
- There is no cold path any more, so formatting is briefly unavailable after activation, after a restart and after a config change, while that folder's server starts. A format requested before the client has registered the server's capability finds no formatter for the document; nothing falls back to a fresh process.
3838
- The server advertises document formatting only: no range or selection formatting, no format-on-type, no diagnostics. It also ignores the editor's `FormattingOptions` (tab size, spaces) and the client's language id — the file path picks the parser and the project's config decides the style. An editor setting that disagrees with the project config loses, which is the same answer `rs fmt` gives in a terminal.
3939
- Failures stay per folder and stay statuses: no `rstack` installed is `disabled`, an `rstack` below `0.5.2` is `version mismatch`, no Node clearing the floor is `version mismatch` with fmt's own consequence appended to the shared preflight message ("until then rs fmt will not format"), and only a server that fails to launch or stops on its own is `crashed`. One folder in any of those states does not affect another folder's server, and the stack's single status report is the folder set folded by severity (`crashed` > `version mismatch`, which is also where a running folder's pin advisory ranks > `disabled` > `starting` > `running`; the pure fold lives in `stacks/fmt/status.ts`), so a healthy sibling starting or recovering never overwrites another folder's failure — and the status says `starting`, not `running`, until a server actually formats.
40-
- An `rstack.config.*` create, change or delete restarts the server of the folder that contains it, and only that one; a detection change reconciles the folder set, leaves healthy servers alone — a healthy server's cached config is worth keeping — and restarts a folder whose runtime already failed (`disabled`, `version mismatch`, `crashed`) in place, on the same path a config change uses, which re-runs package resolution and the version check. Detection notifies on lockfile events even when the folder set is unchanged precisely so that the install or upgrade that fixes a failed resolution is picked up without a manual restart. The remaining blind spot is an install that changes no lockfile (a fresh clone whose lockfile is already current): no file event fires, so the `disabled` status names the restart command as the way out. Watching `node_modules` for that case was rejected (unreliable under pnpm's layout and excluded by VS Code's default watcher excludes), and a bundled fallback formatter — the usual way editor extensions mask this blind spot — is ruled out by resolve-from-project.
40+
- An `rstack.config.*` create, change or delete restarts the server of the folder that contains it, and only that one; a detection change reconciles the folder set, leaves healthy servers alone — a healthy server's cached config is worth keeping — and restarts a folder whose runtime already failed (`disabled`, `version mismatch`, `crashed`) in place, on the same path a config change uses, which re-runs package resolution and the version check. Dependency installation recovery, including the unchanged-lockfile blind spot and the rejected watcher alternatives, is governed by ADR 0005.
4141
- "A subproject becomes its own workspace folder" means a _sibling_ folder (or opening only the subproject). Keeping the parent **and** the nested subdirectory as workspace folders with fmt detected in both is a documented limitation: the parent's selector also matches the nested files, and which of the two servers VS Code asks is not defined. Per-document routing to the deepest folder was considered (lint carries a `WorkspaceDocumentRouter` for exactly this) and deferred — complexity the scenario does not yet justify.
4242
- The extension now holds one long-lived Node process per detected folder for fmt. Each is owned by the same process owner the lint client uses, so a stop is bounded (SIGTERM, then SIGKILL) and the automatic restart vscode-languageclient performs cannot leave an orphan behind.

‎docs/adr/0003-lint-through-editor-worker.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,5 +27,5 @@ Rslint's language server is two halves: the Go process (`rslint --lsp`) lints na
2727
- **One override, and it names a core, not a binary.** `rstack.rslint.binPath` / `customBinPath` are removed in favour of `rstack.rslint.corePath` — the setting upstream introduced in rslint #1617: a path to an `@rslint/core` package directory, resource-scoped, from which the binary, config host, protocol version and plugin host all derive. In a bridged folder it overrides the rstack → `@rslint/core` hop only; the shim stays rstack's. A binary chosen independently of its core cannot be supported: the two must speak the same protocol. The rest of #1617 — per-document core resolution, one runtime per physical installation — has since been synced (issue #13): a **Lint runtime** is now one Rslint core inside one workspace folder, resolved per open document and refcounted by it, so a folder runs as many workers as its files have distinct cores (a bridged folder always exactly one, rstack's) and none at all while nothing is open. The worker never noticed: it still takes explicit `--core` / `--config` paths, which is precisely why that change did not touch it.
2828
- **Ownership is per folder, native wins.** One server holds one config choice for its lifetime (the supported config protocols lock `configPath` per process), and explicit and automatic modes cannot mix, so a folder is bridged only when no `rslint.config.*` exists anywhere in it and a `rstack.config.*` sits at its root; a subdirectory `rstack.config.*` lights nothing (`rs lint` in a terminal reads its cwd only — the same reason ADR 0002 rejected deepest-config-wins for fmt). Detection lights a bridged folder on the file's presence and never reads it: a `rstack.config.*` without `define.lint()` runs an empty config, as `rs lint` does.
2929
- **Config changes refresh, mode changes restart.** Rslint has a live refresh (`rslint/configRefresh` with the same `configPath`), unlike `rs fmt --lsp`, so the extension keeps its watcher-driven refresh — extended, for a bridged folder, with the root `rstack.config.*` — and the worker re-stamps `protocolVersion` and its `configPath` on every refresh (the extension does not know either). Only a native ↔ bridged flip, or a dependency change the refresh cannot absorb, restarts the server. This is the "diverge only when the tool forces it" rule: rslint can refresh, fmt cannot.
30-
- **Failure states mirror fmt.** Bridged folder: no `rstack` → `disabled`; `rstack` or the chained `@rslint/core` below floor, or no Node clearing the floor → `version mismatch`; worker or Go dying → `crashed`. Native folder missing `@rslint/core` stays `crashed` — the user asked for Rslint by name.
30+
- **Failure states mirror fmt.** No `rstack`, no `@rslint/core`, or a config importing an absent package → `disabled`; `rstack` or the chained `@rslint/core` below floor, or no Node clearing the floor → `version mismatch`; worker or Go dying → `crashed`. Config-import failures are classified where the worker still has the loader's structured error and carried to the editor as data; the live Go server remains available for a later refresh.
3131
- The lint copy diverges further from upstream: the reverse-request adapter and plugin pool move into the worker unchanged in logic, and the extension-side `Rslint.ts` keeps only the language-client half. Recorded as an adaptation in `packages/vscode/AGENTS.md`.

‎docs/adr/0005-not-installed-recovery-by-polling.md‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
status: accepted
3+
---
4+
5+
# Recover not-installed stacks by polling only while recovery is needed
6+
7+
Installing an already-locked project can populate `node_modules` without changing any config or lockfile. Detection's config and lockfile watchers then have no event to send, even though every stack deliberately resolves its toolchain from the project and needs another resolution pass. The status names the restart command as a fallback, but a fresh clone should recover without requiring it.
8+
9+
The lint stack previously added a direct watcher for `**/node_modules/@rslint/core/package.json`. A controlled VS Code Extension Host experiment opened a project with no `node_modules`, waited for `disabled`, installed with a frozen lockfile, and observed for 90 seconds without invoking a restart command. Lockfile bytes and nanosecond mtime were verified unchanged in every run:
10+
11+
| Install layout | Watcher event | Automatic recovery |
12+
| -------------- | ------------- | ------------------ |
13+
| npm flat | 2/2 | 2/2 |
14+
| pnpm isolated | 0/2 | 0/2 |
15+
| pnpm hoisted | 0/2 | 0/2 |
16+
17+
The result is not explained by pnpm symlinks: the hoisted core was an ordinary directory and still produced no matching event. Versions were VS Code 1.136.1, pnpm 11.20.0, and npm 11.17.0.
18+
19+
The watcher's original rationale was also factually wrong. At Microsoft VS Code commit [`008427a`](https://github.com/microsoft/vscode/commit/008427a901bf4aa79b47f175ccc8da1731750f78), the default `files.watcherExclude` contains only `.git/objects`, `.git/subtree-cache`, and `.hg/store`, each at the root and one directory below; it does not exclude `node_modules` ([`files.contribution.ts:294-310`](https://github.com/microsoft/vscode/blob/008427a901bf4aa79b47f175ccc8da1731750f78/src/vs/workbench/contrib/files/browser/files.contribution.ts#L294-L310)). The failure is the absent pnpm per-file event observed above, not a VS Code default exclude.
20+
21+
**Decision.** The extension shell owns one recursive 60-second timer. It exists while any live controller's raw folder/project/runtime state is disabled, crashed or version-mismatched, enters the shell's existing serialized queue, and forces the same detection notification as a lockfile event even when the detection signature is unchanged. The three stacks reuse their existing dependency-change paths: lint reconciles open documents and refreshes failed configs, fmt restarts failed folder runtimes in place, and Rstest re-resolves shims and retries failed config evaluation. The timer stops when no failed state remains (running, starting or idle). Lockfile watchers stay as the lower-latency path.
22+
23+
The aggregate status is deliberately not the predicate: every owned raw failure needs recovery. A retry landing mid-install can read half-written `node_modules` and fail with a syntax error instead of a missing dependency. Continuing every minute through that real error makes the transient harmless without a provisional-error heuristic. Real errors still replace not-installed in status and Output; persistent error messages and not-installed warnings are deduplicated so retries do not log every minute. The restart hint remains in the status as an explicit fallback.
24+
25+
fmt has one tool-forced limitation. Restarting `rs fmt --lsp` re-runs package resolution, but the server loads project config lazily on the next formatting request. After a missing dependency, a poll can therefore move the folder to `running` before config loading has been proved; the next format either succeeds or reports the same config failure and returns the folder to `disabled`, which restarts polling. A known real config error instead remains `crashed` across restarts until a format produces edits.
26+
27+
## Considered options
28+
29+
- **Direct `node_modules` watchers** — rejected by the experiment: they recovered npm but missed both pnpm layouts.
30+
- **Package-manager marker files** such as `node_modules/.modules.yaml`, `node_modules/.package-lock.json`, or `.yarn-integrity` — rejected because each covers one installer/layout and makes recovery depend on private install artifacts rather than the state being recovered.
31+
- **Retry on window focus** — rejected because an install can finish while focus never leaves VS Code, and unrelated focus changes would cause unbounded retries.
32+
- **Bundled tool fallbacks** — rejected by the resolve-from-project contract: editor and CLI must run the same installed versions.
33+
34+
## Consequences
35+
36+
- Healthy workspaces incur no polling work. An unresolved workspace retries at most once per timer interval, through the existing serialized shell queue.
37+
- Recovery no longer depends on installer-specific file events; lockfile watchers remain the faster path when they do fire.
38+
- A real config error replaces not-installed without stopping recovery. Config events and the explicit restart command remain available alongside the minute poll.
39+
- fmt cannot prove config recovery at initialize time. Only a later format producing edits ends its warning episode; empty edits are ambiguous because the server uses them for both no-op formatting and failures whose showMessage may already have been sent.

0 commit comments

Comments
 (0)