Skip to content

Commit e364fd1

Browse files
committed
feat(vscode): poll every minute until every stack is running
1 parent 6593c86 commit e364fd1

16 files changed

Lines changed: 195 additions & 93 deletions

File tree

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ The result is not explained by pnpm symlinks: the hoisted core was an ordinary d
1818

1919
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.
2020

21-
**Decision.** The extension shell owns one recursive 10-second timer. It exists only while any live controller's raw folder/project state says dependencies are not installed, 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 config dependencies, fmt restarts failed folder runtimes in place, and Rstest re-resolves shims and retries failed config evaluation. The timer stops as soon as no not-installed state remains. Lockfile watchers stay as the lower-latency path.
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.
2222

23-
The aggregate status is deliberately not the predicate: a crash or version mismatch can outrank an unrelated missing folder. Polling continues while any raw state is not installed and stops when none is, including when a retry replaces not-installed with a real config error surfaced in status and Output for the user to fix. Warnings are deduplicated per unresolved episode so a persistent missing install does not add a line every ten seconds. The restart hint remains in the status as an explicit fallback.
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.
2424

2525
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. 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.
2626

@@ -35,5 +35,5 @@ fmt has one tool-forced limitation. Restarting `rs fmt --lsp` re-runs package re
3535

3636
- Healthy workspaces incur no polling work. An unresolved workspace retries at most once per timer interval, through the existing serialized shell queue.
3737
- Recovery no longer depends on installer-specific file events; lockfile watchers remain the faster path when they do fire.
38-
- A new, real config error replaces not-installed and stops its poll. Fixing that error still uses config events or the explicit restart command.
38+
- A real config error replaces not-installed without stopping recovery. Config events and the explicit restart command remain available alongside the minute poll.
3939
- 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.

packages/vscode/AGENTS.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten
99
- **Tracked upstream state.** `stacks/lint` is synced to web-infra-dev/rslint `packages/vscode-extension` at **39536fd6** (#1617 — per-document core resolution, `CoreResolver` + `RuntimeManager`, `corePath`, PnP removed) and **892482e0** (#1630`configPath` on `rslint/configRefresh`). Targeted later ports are **84f9c9b5** (#1967 — languageclient-owned live LSP tracing) and **b7176723** (#1951 — remove legacy JSON config watching); the Unicode BOM E2E comes from **5fc197a5** (#1560), with its native-config fixture shape from **b7176723**. `CoreResolver.ts` / `RuntimeManager.ts` / `WorkspaceDocumentRouter.ts` / `Rslint.ts` are the files to diff when syncing further; record the new commits here when you do.
1010
- **Ahead of upstream — offer these back when syncing** (bug fixes, not adaptations): (1) `RuntimeManager.reconcile` resolves the document's core **before** sweeping pending uses (`planDocumentCore`), so a reconcile landing on the key a pending start is already producing adopts that start instead of tearing it down mid-`initialize` — the teardown made vscode-languageclient force-notify ("couldn't create connection to server") whenever the register-time pass, a detection change and `didOpen` landed inside one worker startup window (`tests/stacks/lint/runtimeManager.test.ts`). (2) `Rslint.close()` gives a still-Starting language client a bounded chance to settle before tearing down its transport, so a legitimate mid-start close (document closed during start, core key changed) stops cleanly instead of triggering the same force-notified toasts. (3) The registry-harness E2E gives its never-settling startup operation 500ms to begin and accepts only the in-flight timeout message, so a stalled runner cannot satisfy the assertion through the already-expired path (`e2e/lint/suite/registry-harness.test.ts`). (4) `Project.retryFailedConfig()` keeps a failed Rstest project and retries its config evaluation in place with one single-flight promise, so repeated dependency-change passes neither overlap workers nor repeat an unchanged not-installed warning.
1111

12-
- **Targeted Rstest lifecycle port:** `RstestApi.getNormalizedConfig()` closes its worker in `finally`, including rejected config evaluation, matching web-infra-dev/rstest `packages/vscode/src/master.ts` at `d82db4fc31a61ee74b2a74917f14a458e1bca419`. This fixes a leak in our older copy; it is already fixed upstream. Dependency passes retry only projects with a not-installed latch under their core or config-import source; real config errors retain config-edit/restart recovery.
12+
- **Targeted Rstest lifecycle port:** `RstestApi.getNormalizedConfig()` closes its worker in `finally`, including rejected config evaluation, matching web-infra-dev/rstest `packages/vscode/src/master.ts` at `d82db4fc31a61ee74b2a74917f14a458e1bca419`. This fixes a leak in our older copy; it is already fixed upstream. Dependency passes retry failed projects, including real config errors, while preserving single-flight loading and worker cleanup.
1313

1414
## The nine adaptations
1515

@@ -27,7 +27,7 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten
2727

2828
- **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. 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.*`). Testing and fixtures track only the latest published releases, pinned exactly and bumped by Renovate; a green E2E run speaks only for those releases. `SUPPORT_MATRIX` floors are the minimum versions the extension accepts: each entry is the lowest release evidence shows works with the current code, and its comment records that evidence. Move a floor only when a change makes older releases stop working, never because a devDependency or fixture moved. Raising a floor needs no transition story; the status names the required version.
2929
- **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.
30-
- **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 keeps the restart command as an explicit fallback, one `warn` line per unresolved episode in the output channel without a stack trace, never a `crashed` status and never a notification. The shell owns one 10-second recursive poll while any controller's raw folder/project state is not installed; it enters the existing serialized queue, forces the same detection notification as a lockfile event, and stops when no such state remains (ADR 0005). 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. Rstest classifies config-import failures in its worker (`missingDependencyCauseOf`: Node's `code`, a bare package specifier, and for a subpath a walk-up proving the package really is absent) because IPC drops the `code`; Rslint makes the same code-gated decision where its worker still has structured loader results and sends a dedicated verdict to the editor; fmt intercepts only the exact `rs fmt cannot format this workspace:` Error notification and applies the shared message classifier. A typo'd relative import or a missing subpath of an installed package stays a real error in all three.
30+
- **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 keeps the restart command as an explicit fallback, one `warn` line per unresolved episode in the output channel without a stack trace, never a `crashed` status and never a notification. The shell owns one 60-second recursive poll while any controller's raw folder/project/runtime state is disabled, crashed or version-mismatched; it enters the existing serialized queue, forces the same detection notification as a lockfile event, and stops when no failed state remains (ADR 0005). A mid-install retry can read half-written `node_modules` and produce a real syntax error; continuing through failed states makes that transient harmless without a provisional-error heuristic. Real errors remain visible in status and Output, deduplicated by message rather than logged every minute. 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. Rstest classifies config-import failures in its worker (`missingDependencyCauseOf`: Node's `code`, a bare package specifier, and for a subpath a walk-up proving the package really is absent) because IPC drops the `code`; Rslint makes the same code-gated decision where its worker still has structured loader results and sends a dedicated verdict to the editor; fmt intercepts only the exact `rs fmt cannot format this workspace:` Error notification and applies the shared message classifier. A typo'd relative import or a missing subpath of an installed package stays a real error in all three.
3131
- One stack failing to register or crashing must never take another stack (or the shell) down.
3232
- The shell always activates; per-folder config detection decides which stacks start, and re-runs on config/lockfile changes without a window reload. The per-stack enable settings are coarse kill switches only.
3333
- 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.

packages/vscode/src/extension.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ const STACK_FACTORIES: Readonly<Record<StackId, StackControllerFactory>> = {
2828
/** Stacks that run project-loading children on the shared User Node runtime. */
2929
const USER_NODE_STACKS: readonly StackId[] = ['rslint', 'rstest', 'fmt'];
3030

31-
const DEFAULT_DEPENDENCY_POLL_INTERVAL_MS = 10_000;
31+
const DEFAULT_DEPENDENCY_POLL_INTERVAL_MS = 60_000;
3232

3333
const errorMessage = (error: unknown): string =>
3434
error instanceof Error ? (error.stack ?? error.message) : String(error);
@@ -229,14 +229,14 @@ class ExtensionShell {
229229
return (
230230
!this.#disposed &&
231231
[...this.#controllers.values()].some((controller) =>
232-
controller.hasNotInstalledState(),
232+
controller.hasFailedState(),
233233
)
234234
);
235235
}
236236

237237
/**
238238
* Starts one recursive timer only while a live controller owns a
239-
* not-installed state. The timer enters the same shell queue as every
239+
* failed state. The timer enters the same shell queue as every
240240
* reconcile/restart, then sends the same forced detection event as a
241241
* lockfile change; each stack therefore reuses its existing retry path.
242242
*/

packages/vscode/src/stacks/fmt/index.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ class FmtFolderRuntime {
163163
#configPath: string | undefined;
164164
readonly #packageEpisode = new NotInstalledEpisode();
165165
readonly #configDependencyEpisode = new NotInstalledEpisode();
166+
#startError: string | undefined;
166167
suppressedShowMessages = 0;
167168
#closing = false;
168169
#disposed = false;
@@ -451,9 +452,17 @@ class FmtFolderRuntime {
451452
error instanceof Error ? error.message : String(error)
452453
}`,
453454
);
454-
context.output.error('Failed to start the rs fmt language server', error);
455+
const message = error instanceof Error ? error.message : String(error);
456+
if (this.#startError !== message) {
457+
this.#startError = message;
458+
context.output.error(
459+
'Failed to start the rs fmt language server',
460+
error,
461+
);
462+
}
455463
return;
456464
}
465+
this.#startError = undefined;
457466
context.output.info(`rs fmt language server started for ${folderRoot}`);
458467
}
459468

@@ -804,9 +813,9 @@ class FmtController implements StackController {
804813
);
805814
}
806815

807-
hasNotInstalledState(): boolean {
808-
return [...this.#runtimes.values()].some(
809-
(runtime) => runtime.state === 'disabled',
816+
hasFailedState(): boolean {
817+
return [...this.#runtimes.values()].some((runtime) =>
818+
isFailedFmtState(runtime.state),
810819
);
811820
}
812821

packages/vscode/src/stacks/lint/Rslint.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,7 @@ export class Rslint implements Disposable {
324324
private advisory: string | undefined;
325325
private readonly configDependencyEpisode = new NotInstalledEpisode();
326326
private configRefreshFailed = false;
327+
private configError: string | undefined;
327328
private startPromise: Promise<void> | undefined;
328329
private startOperation: Promise<void> | undefined;
329330
private clientStartPromise: Promise<void> | undefined;
@@ -378,11 +379,15 @@ export class Rslint implements Disposable {
378379
this.configRefreshFailed = true;
379380
this.report({ kind: 'crashed', detail: notification.message });
380381
this.configDependencyEpisode.clear();
381-
this.logger.error(
382-
`Failed to refresh config discovery: ${notification.message}`,
383-
);
382+
if (this.configError !== notification.message) {
383+
this.configError = notification.message;
384+
this.logger.error(
385+
`Failed to refresh config discovery: ${notification.message}`,
386+
);
387+
}
384388
return;
385389
}
390+
this.configError = undefined;
386391
const wasFailed = this.configRefreshFailed;
387392
this.configRefreshFailed = false;
388393
if (notification.kind === 'ok') {
@@ -715,7 +720,8 @@ export class Rslint implements Disposable {
715720
}
716721

717722
public retryConfigDependency(): Promise<void> | undefined {
718-
if (!this.hasConfigDependencyFailure()) return undefined;
723+
if (!this.hasConfigDependencyFailure() && !this.configRefreshFailed)
724+
return undefined;
719725
return this.requestConfigRefresh('dependency-change');
720726
}
721727

packages/vscode/src/stacks/lint/index.ts

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -193,10 +193,13 @@ class RslintController implements StackController {
193193
// lints, so its consequence says what it keeps, not "will not".
194194
const missing = missingPackageOf(error);
195195
const status = statusForRslintStartFailure(error);
196+
const attributed = resolved
197+
? attributeToCore(status, resolved.installation.packageDirectory)
198+
: status;
199+
const previous = this.#folderStates
200+
.get(folderKeyOf(workspaceFolder))
201+
?.failures.get(document.uri.toString());
196202
if (missing !== undefined) {
197-
const previous = this.#folderStates
198-
.get(folderKeyOf(workspaceFolder))
199-
?.failures.get(document.uri.toString());
200203
if (
201204
previous?.kind !== 'disabled' ||
202205
previous.reason !==
@@ -210,7 +213,12 @@ class RslintController implements StackController {
210213
);
211214
logger.warn(warning);
212215
}
213-
} else {
216+
} else if (
217+
previous?.kind !== attributed.kind ||
218+
!('detail' in previous) ||
219+
!('detail' in attributed) ||
220+
previous.detail !== attributed.detail
221+
) {
214222
logger.error(
215223
formatCoreSelectionFailure(document.uri.toString(), keeping),
216224
error,
@@ -224,9 +232,7 @@ class RslintController implements StackController {
224232
folderKeyOf(workspaceFolder),
225233
'failures',
226234
document.uri.toString(),
227-
resolved
228-
? attributeToCore(status, resolved.installation.packageDirectory)
229-
: status,
235+
attributed,
230236
);
231237
},
232238
onDocumentSettled: (document) => {
@@ -400,10 +406,13 @@ class RslintController implements StackController {
400406
);
401407
}
402408

403-
hasNotInstalledState(): boolean {
409+
hasFailedState(): boolean {
404410
return [...this.#folderStates.values()].some((states) =>
405411
[...states.runtimes.values(), ...states.failures.values()].some(
406-
(state) => state.kind === 'disabled',
412+
(state) =>
413+
state.kind === 'disabled' ||
414+
state.kind === 'crashed' ||
415+
state.kind === 'version-mismatch',
407416
),
408417
);
409418
}

packages/vscode/src/stacks/test/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -556,8 +556,8 @@ class RstestController implements StackController {
556556
return this.#rstest.buildExports();
557557
}
558558

559-
hasNotInstalledState(): boolean {
560-
return status.hasNotInstalled();
559+
hasFailedState(): boolean {
560+
return status.hasFailed();
561561
}
562562

563563
dispose(): void {

0 commit comments

Comments
 (0)