From b53b5ce1773a60a5f04b92522ec27b80758e60bd Mon Sep 17 00:00:00 2001 From: fi3ework Date: Fri, 7 Aug 2026 14:44:16 +0800 Subject: [PATCH 1/6] feat(vscode): keep a pre-spawned rs fmt standby for the active editor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A cold format pays ~130ms of process start-up and config load before any formatting happens. The fmt stack now keeps one pre-spawned rs fmt child parked on stdin for the active editor's file, so the next format of that file writes to a process that has already paid both. The standby is bounded on purpose: one process, one file, one request. It is armed 2s after the active editor settles (immediately at registration and after a format), killed whenever it could have gone stale — a config content edit, a detection change, restart or dispose — and reaped after five idle minutes. Anything it cannot serve falls back to the cold path unchanged, and the whole mechanism is invisible above debug level except for the hot/cold marker on the completion line. CONTEXT.md records the vocabulary the code and logs use, and the fmt gotcha in AGENTS.md now states the exception and its limits. --- CONTEXT.md | 19 ++ packages/vscode/AGENTS.md | 2 +- packages/vscode/src/stacks/fmt/index.ts | 284 ++++++++++++++--- packages/vscode/src/stacks/fmt/run.test.ts | 30 +- packages/vscode/src/stacks/fmt/run.ts | 299 ++++++++++++------ .../vscode/src/stacks/fmt/standby.test.ts | 284 +++++++++++++++++ packages/vscode/src/stacks/fmt/standby.ts | 172 ++++++++++ packages/vscode/src/stacks/fmt/stubProcess.ts | 45 +++ packages/vscode/tests/e2e/suite/fmt.test.ts | 49 +++ 9 files changed, 1025 insertions(+), 159 deletions(-) create mode 100644 CONTEXT.md create mode 100644 packages/vscode/src/stacks/fmt/standby.test.ts create mode 100644 packages/vscode/src/stacks/fmt/standby.ts create mode 100644 packages/vscode/src/stacks/fmt/stubProcess.ts diff --git a/CONTEXT.md b/CONTEXT.md new file mode 100644 index 0000000..30412b0 --- /dev/null +++ b/CONTEXT.md @@ -0,0 +1,19 @@ +# Ubiquitous language + +Glossary of terms used across rstack-editor. Code, docs, commit messages and reviews use these words with exactly these meanings. + +## Core + +- **Stack** — one tool integration (lint, test, fmt) hosted by the extension shell. A stack registers against the shell and reports status through it; stacks never own UI chrome. +- **Shell** — the always-activating extension core: detection, status bar, output channels, settings migration, stack lifecycle. +- **Detection** — the per-workspace-folder scan deciding which stacks a folder lights up. Detection signals are config files and installed tool binaries, never user settings. +- **Gate** — the per-stack activation condition: detected, workspace trusted, and the enable settings on. + +## fmt + +- **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. +- **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 always targets the active editor's file ("the standby tracks the active editor"). +- **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. +- **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. +- **Hot format** — a format request served by consuming the standby. +- **Reap** — kill an idle standby to reclaim its memory. A reaped standby is not an error; the next eligible event simply arms a new one. diff --git a/packages/vscode/AGENTS.md b/packages/vscode/AGENTS.md index 7cc491e..da096e1 100644 --- a/packages/vscode/AGENTS.md +++ b/packages/vscode/AGENTS.md @@ -31,7 +31,7 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten - The lint × `rstack.config.*` bridge was built and deliberately removed: a partial editor-side bridge gave wrong results, and a correct one needs upstream work first. `TODO(rstack-bridge)` markers carry the plan. Do not reintroduce a partial bridge. - 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. Never re-implement rstack config semantics in the extension. -- The fmt stack is a spawn-per-request `rs fmt --stdin-filepath` MVP. Its cwd is the governing config directory because rs fmt resolves config from cwd only, and formatting errors are log-only by design. The endgame is an upstream LSP, so do not add a warm-process middle tier. +- The fmt stack is a spawn-per-request `rs fmt --stdin-filepath` MVP. Its cwd is the governing config directory because rs fmt resolves config from cwd only, and formatting errors are log-only by design. A single pre-spawned standby that tracks the active editor (see CONTEXT.md) is the accepted, bounded exception to "no warm tier". Do not grow it into a daemon: no long-lived protocol, no process pool, no cross-request state. The endgame is an upstream LSP; the standby retires with it. - `projectModules.ts` has no cache-invalidation hook and restart must not grow one. Node's ESM registry is keyed by resolved URL and process-lifetime, so clearing the local memo hands back the identical module object (verified); a `?epoch=` query does reload the entry but relative specifiers inside it do not inherit the query, yielding a fresh entry over stale dependencies. In-place reinstalls under an unchanged path need a window reload — say so, don't fake it. - The VSIX is platform-targeted for exactly one reason: the test stack's AST collection loads a native parser binding. Do not add another native dependency — it multiplies the release matrix. diff --git a/packages/vscode/src/stacks/fmt/index.ts b/packages/vscode/src/stacks/fmt/index.ts index fb2645e..bae9e28 100644 --- a/packages/vscode/src/stacks/fmt/index.ts +++ b/packages/vscode/src/stacks/fmt/index.ts @@ -1,11 +1,12 @@ import path from 'node:path'; import vscode from 'vscode'; +import { RSTACK_CONFIG_GLOB } from '../../detection'; import { findPackageJsonUncached, readPackageJson, } from '../../shared/packageResolve'; import { - readPackageVersion, + checkPackageVersion, reportVersionCheck, } from '../../shared/versionCheck'; import type { @@ -20,6 +21,7 @@ import { runRsFmt, stderrTail, } from './run'; +import { FmtStandby, type StandbyKey } from './standby'; // prettier 3.9.6 getSupportInfo() vscodeLanguageIds snapshot (rs fmt's pinned // prettier). Revisit when the pinned prettier changes. @@ -56,9 +58,48 @@ const SELECTOR: vscode.DocumentSelector = LANGUAGE_IDS.map((language) => ({ })); /** - * Spawn-per-request formatter backed by the project-resolved rstack CLI. The - * process cwd selects the nearest governing rstack config because `rs fmt` - * intentionally performs cwd-only config resolution. + * How long the active editor must hold still before it gets a standby. Arming + * spawns a process, so scrolling through a dozen tabs must not spawn a dozen + * children; the first arm at registration and the re-arm right after a consume + * skip the wait because both target an editor that is already settled. + */ +const ARM_DEBOUNCE_MS = 2_000; + +/** Everything a format request needs once the folder has been resolved. */ +interface FmtTarget { + readonly cwd: string; + readonly rsBinJs: string; +} + +const standbyKey = (uri: vscode.Uri, target: FmtTarget): StandbyKey => ({ + cwd: target.cwd, + filePath: uri.fsPath, + rsBinJs: target.rsBinJs, +}); + +/** + * Resolution outcome, kept free of side effects so the arming path (which must + * stay silent) and the format path (which reports and logs) can share it. + */ +type FmtResolution = + | { readonly kind: 'ok'; readonly target: FmtTarget } + | { readonly kind: 'no-folder' } + | { readonly kind: 'undetected'; readonly folder: vscode.WorkspaceFolder } + | { + readonly kind: 'missing-package'; + readonly folder: vscode.WorkspaceFolder; + readonly cwd: string; + } + | { readonly kind: 'version-mismatch'; readonly version: string | undefined }; + +/** + * Formatter backed by the project-resolved rstack CLI. The process cwd selects + * the nearest governing rstack config because `rs fmt` intentionally performs + * cwd-only config resolution. + * + * A request is served either by consuming the standby (hot) or by spawning a + * process for it (cold); the two paths differ only in where the process came + * from. */ class FmtController implements StackController { readonly id = 'fmt' as const; @@ -71,18 +112,40 @@ class FmtController implements StackController { readonly #loggedOnce = new Set(); readonly #abortController = new AbortController(); #disposed = false; + #standby: FmtStandby | undefined; + #armTimer: NodeJS.Timeout | undefined; + /** E2E-only: how the most recent format request was served. */ + #lastServe: 'hot' | 'cold' | undefined; async register(context: StackContext): Promise> { this.#context = context; this.#snapshot = context.detection; + this.#standby = new FmtStandby({ + log: (message) => context.output.debug(message), + }); const provider: vscode.DocumentFormattingEditProvider = { provideDocumentFormattingEdits: (document, _options, token) => this.provideDocumentFormattingEdits(document, token), }; + // `rs fmt` loads the project config while it drains stdin, so a parked + // process already carries the old config. Detection does not fire on config + // *content* edits — its signature only tracks which files exist — so the + // standby needs its own watcher over the same glob. + // Create and delete already arrive as detection changes (the file set is + // part of its signature), so this watcher covers only the content edits + // detection cannot see — the same split `stacks/test/project.ts` uses. + const configWatcher = vscode.workspace.createFileSystemWatcher( + RSTACK_CONFIG_GLOB, + true, + false, + true, + ); this.#subscriptions.push( context.onDidChangeDetection((snapshot) => { this.#snapshot = snapshot; this.#loggedOnce.clear(); + // The cwd, the resolved bin and the config set can all have moved. + this.invalidateStandby('detection changed'); // The reconcile leaves a still-detected controller alone, so the // running reason must follow the new snapshot here rather than wait // for the next successful format. @@ -92,9 +155,128 @@ class FmtController implements StackController { SELECTOR, provider, ), + configWatcher, + configWatcher.onDidChange((uri) => + this.invalidateStandby(`${uri.fsPath} changed`), + ), + vscode.window.onDidChangeActiveTextEditor(() => this.scheduleArm()), ); this.reportRunning(context, context.detection); - return { languages: LANGUAGE_IDS, provider }; + // The editor the user is already looking at needs no settling wait, but + // arming spawns a process and `register()` must return fast. + this.scheduleArm(0); + return { + languages: LANGUAGE_IDS, + provider, + armedFilePath: (): string | undefined => this.#standby?.armedFilePath, + lastServe: (): 'hot' | 'cold' | undefined => this.#lastServe, + }; + } + + /** Kills the standby, then re-arms the active editor through the debounce. */ + private invalidateStandby(reason: string): void { + this.#standby?.kill(reason); + this.scheduleArm(); + } + + private scheduleArm(delayMs = ARM_DEBOUNCE_MS): void { + clearTimeout(this.#armTimer); + this.#armTimer = setTimeout(() => { + this.#armTimer = undefined; + this.armActiveEditor(); + }, delayMs); + } + + /** + * Arms a standby for whatever the active editor is *now* — the invariant is + * "the standby tracks the active editor", so the editor is never captured + * when the arm was scheduled. Ineligible editors leave the standby alone + * rather than killing it: a peek at the output panel must not cost the user + * their warm process. + */ + private armActiveEditor(): void { + const context = this.#context; + const standby = this.#standby; + if (!context || !standby) { + return; + } + const document = vscode.window.activeTextEditor?.document; + // The provider's own selector is the eligibility rule, so the two cannot + // drift apart. + if (!document || vscode.languages.match(SELECTOR, document) === 0) { + return; + } + // Every invalidation kills the standby before asking for a re-arm, so an + // armed standby on this file is still valid — and resolving is synchronous + // filesystem work that runs on the UI thread. + if (standby.armedFilePath === document.uri.fsPath) { + return; + } + const resolution = this.resolve(document.uri); + if (resolution.kind !== 'ok') { + // Arming is silent: an unresolvable editor only means the next format + // there is cold, which is exactly what happened before the standby. + context.output.debug( + `Standby not armed for ${document.uri.fsPath}: ${resolution.kind}`, + ); + return; + } + standby.arm(standbyKey(document.uri, resolution.target)); + } + + /** + * Where a document's `rs fmt` would run. Pure: it reports no status and logs + * nothing, because the arming path must not move the status bar. + */ + private resolve(uri: vscode.Uri): FmtResolution { + const snapshot = this.#snapshot; + const folder = vscode.workspace.getWorkspaceFolder(uri); + if (!snapshot || !folder) { + return { kind: 'no-folder' }; + } + const fmtDetection = snapshot.forFolder(folder)?.stacks.fmt; + if (!fmtDetection?.detected) { + return { kind: 'undetected', folder }; + } + + const cwd = pickConfigDir( + uri.fsPath, + fmtDetection.rstackConfigFiles.map((configUri) => configUri.fsPath), + folder.uri.fsPath, + ); + const pkgJsonPath = findPackageJsonUncached('rstack', cwd); + if (!pkgJsonPath) { + return { kind: 'missing-package', folder, cwd }; + } + + // One read for both the version and the bin entry: `resolve` now runs on + // the arming path too, and `readPackageJson` re-reads from disk by design. + const pkg = readPackageJson(pkgJsonPath); + const version = typeof pkg?.version === 'string' ? pkg.version : undefined; + if (checkPackageVersion('rstack', version).kind === 'mismatch') { + // Reporting stays with the caller: the arming path must not move the + // status bar, and the format path goes through `reportVersionCheck` so + // the shared contract has one implementation. + return { kind: 'version-mismatch', version }; + } + + const bin = pkg?.bin; + let binEntry = 'bin/rs.js'; + if (typeof bin === 'string') { + binEntry = bin; + } else if (bin && typeof bin === 'object') { + const rs = (bin as Record).rs; + if (typeof rs === 'string') { + binEntry = rs; + } + } + return { + kind: 'ok', + target: { + cwd, + rsBinJs: path.resolve(path.dirname(pkgJsonPath), binEntry), + }, + }; } /** `running` always carries the reason the stack is on: where it was detected. */ @@ -130,15 +312,15 @@ class FmtController implements StackController { return []; } - const folder = vscode.workspace.getWorkspaceFolder(document.uri); - if (!folder) { + const resolution = this.resolve(document.uri); + if (resolution.kind === 'no-folder') { return []; } - const fmtDetection = snapshot.forFolder(folder)?.stacks.fmt; - if (!fmtDetection?.detected) { + if (resolution.kind === 'undetected') { // The formatter is offered per language, so a request can land in a // folder without an rstack setup. That is routine, not a fault — one // info line per folder says why nothing happened. + const folder = resolution.folder; if (!this.#loggedOnce.has(`undetected:${folder.uri.toString()}`)) { this.#loggedOnce.add(`undetected:${folder.uri.toString()}`); context.output.info( @@ -148,50 +330,27 @@ class FmtController implements StackController { return []; } - const cwd = pickConfigDir( - document.uri.fsPath, - fmtDetection.rstackConfigFiles.map((uri) => uri.fsPath), - folder.uri.fsPath, - ); // Per-request logging follows prettier-vscode's shape (same in-host, // work-per-request architecture): a fixed entry and outcome line at info, // resolution detail at debug — the channel is a LogOutputChannel, so the // user raises the level from its context menu when needed. const startedAt = Date.now(); context.output.info(`Formatting ${document.uri.fsPath}`); - const pkgJsonPath = findPackageJsonUncached('rstack', cwd); - if (!pkgJsonPath) { - const reason = `rstack is not installed in ${folder.name} (node_modules missing)`; + if (resolution.kind === 'missing-package') { + const reason = `rstack is not installed in ${resolution.folder.name} (node_modules missing)`; context.status.report({ kind: 'disabled', reason }); - if (!this.#loggedOnce.has(`missing:${cwd}`)) { - this.#loggedOnce.add(`missing:${cwd}`); - context.output.warn(`${reason}; searched from ${cwd}`); + if (!this.#loggedOnce.has(`missing:${resolution.cwd}`)) { + this.#loggedOnce.add(`missing:${resolution.cwd}`); + context.output.warn(`${reason}; searched from ${resolution.cwd}`); } return []; } - - if ( - !reportVersionCheck( - context.status, - 'rstack', - readPackageVersion(pkgJsonPath), - ) - ) { + if (resolution.kind === 'version-mismatch') { + reportVersionCheck(context.status, 'rstack', resolution.version); return []; } - const pkg = readPackageJson(pkgJsonPath); - const bin = pkg?.bin; - let binEntry = 'bin/rs.js'; - if (typeof bin === 'string') { - binEntry = bin; - } else if (bin && typeof bin === 'object') { - const rs = (bin as Record).rs; - if (typeof rs === 'string') { - binEntry = rs; - } - } - const rsBinJs = path.resolve(path.dirname(pkgJsonPath), binEntry); + const { cwd, rsBinJs } = resolution.target; context.output.debug(`cwd: ${cwd}; bin: ${rsBinJs}`); const text = document.getText(); @@ -206,18 +365,37 @@ class FmtController implements StackController { requestController.abort(); } + const key = standbyKey(document.uri, resolution.target); + // An already-cancelled request must not burn the standby. + const hot = requestController.signal.aborted + ? undefined + : this.#standby?.consume(key, { + text, + signal: requestController.signal, + }); + const serve = hot ? 'hot' : 'cold'; + this.#lastServe = serve; + let result; try { - result = await runRsFmt({ - text, - filePath: document.uri.fsPath, - cwd, - rsBinJs, - signal: requestController.signal, - }); + result = await (hot ?? + runRsFmt({ + text, + filePath: document.uri.fsPath, + cwd, + rsBinJs, + signal: requestController.signal, + })); } finally { cancellation.dispose(); this.#abortController.signal.removeEventListener('abort', abortRequest); + // A standby serves exactly one request, and a real format request is + // itself proof the active file is worth one — re-arm after cold serves + // too, so a reaped or crashed standby comes back on the next use + // instead of leaving the file cold until the editor changes. Scheduled + // rather than immediate: spawning here would delay the edits the caller + // is waiting for. + this.scheduleArm(0); } if ( @@ -246,8 +424,12 @@ class FmtController implements StackController { // have changed while the format was in flight. this.reportRunning(context, this.#snapshot ?? snapshot); const edit = minimalEdit(text, result.formatted); + // The hot/cold marker is the only way to tell from a log whether the + // measured time included a process start-up. context.output.info( - `Formatting completed in ${elapsed}ms${edit ? '' : ' (already formatted)'}`, + `Formatting completed in ${elapsed}ms (${serve}${ + edit ? '' : ', already formatted' + })`, ); if (!edit) { return []; @@ -282,6 +464,12 @@ class FmtController implements StackController { dispose(): void { this.#disposed = true; this.#abortController.abort(); + // Covers `rstack.fmt.restart` (the shell rebuilds the controller) and a + // workspace losing its trust: neither may leave a process behind. + clearTimeout(this.#armTimer); + this.#armTimer = undefined; + this.#standby?.dispose(); + this.#standby = undefined; for (const subscription of this.#subscriptions.splice(0)) { subscription.dispose(); } diff --git a/packages/vscode/src/stacks/fmt/run.test.ts b/packages/vscode/src/stacks/fmt/run.test.ts index 51a1c6e..006612e 100644 --- a/packages/vscode/src/stacks/fmt/run.test.ts +++ b/packages/vscode/src/stacks/fmt/run.test.ts @@ -3,6 +3,7 @@ import os from 'node:os'; import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from '@rstest/core'; import { minimalEdit, pickConfigDir, runRsFmt, type RsFmtRun } from './run'; +import { createStubRoot, type StubRoot } from './stubProcess'; describe('pickConfigDir', () => { let root: string; @@ -145,26 +146,17 @@ describe('minimalEdit', () => { }); describe('runRsFmt', () => { - let root: string; + let stubs: StubRoot; beforeEach(() => { - root = fs.realpathSync( - fs.mkdtempSync(path.join(os.tmpdir(), 'rstack-fmt-run-')), - ); + stubs = createStubRoot('run'); }); afterEach(() => { - fs.rmSync(root, { recursive: true, force: true }); + stubs.remove(); }); - const writeStub = (source: string): string => { - const filePath = path.join( - root, - `rs-${Math.random().toString(16).slice(2)}.js`, - ); - fs.writeFileSync(filePath, source); - return filePath; - }; + const writeStub = (source: string): string => stubs.write(source); const run = ( text: string, @@ -175,8 +167,8 @@ describe('runRsFmt', () => { > => { const options: RsFmtRun = { text, - filePath: path.join(root, 'input.ts'), - cwd: root, + filePath: path.join(stubs.path, 'input.ts'), + cwd: stubs.path, rsBinJs, signal, }; @@ -235,12 +227,16 @@ describe('runRsFmt', () => { const controller = new AbortController(); controller.abort(); await expect( - run('const value = 1;', path.join(root, 'missing.js'), controller.signal), + run( + 'const value = 1;', + path.join(stubs.path, 'missing.js'), + controller.signal, + ), ).resolves.toEqual({ kind: 'cancelled' }); }); it('names an unloadable rs entry path', async () => { - const missing = path.join(root, 'missing.js'); + const missing = path.join(stubs.path, 'missing.js'); const result = await run('const value = 1;', missing); expect(result.kind).toBe('error'); if (result.kind === 'error') { diff --git a/packages/vscode/src/stacks/fmt/run.ts b/packages/vscode/src/stacks/fmt/run.ts index 82209c4..d9effff 100644 --- a/packages/vscode/src/stacks/fmt/run.ts +++ b/packages/vscode/src/stacks/fmt/run.ts @@ -53,7 +53,7 @@ export type RsFmtResult = | { readonly kind: 'cancelled' } | { readonly kind: 'error'; readonly message: string }; -const errorMessage = (error: unknown): string => +export const errorMessage = (error: unknown): string => error instanceof Error ? error.message : String(error); /** Bounds any CLI stderr headed for a log line to its meaningful tail. */ @@ -69,18 +69,176 @@ const launchError = (rsBinJs: string, detail: string): RsFmtResult => ({ export const isRsFmtLaunchError = (result: RsFmtResult): boolean => result.kind === 'error' && result.message.startsWith(LAUNCH_ERROR_PREFIX); -export const runRsFmt = async (run: RsFmtRun): Promise => { - if (run.signal.aborted) { - return { kind: 'cancelled' }; +/** How an `rs fmt` child ended. */ +type RsFmtExit = + | { readonly kind: 'error'; readonly error: unknown } + | { readonly kind: 'close'; readonly code: number | null }; + +/** + * Only the tail of stderr is ever reported, but a parked standby can hold a + * child for minutes — a chatty one must not grow the buffer without bound. + */ +const MAX_STDERR_CHARS = 64 * 1024; + +/** Built once: `process.env` does not change over an extension host's life. */ +const CHILD_ENV = { ...process.env, ELECTRON_RUN_AS_NODE: '1' }; + +/** + * A spawned `rs fmt --stdin-filepath` child whose streams drain from the moment + * it starts. Both fmt paths share it: a cold format writes stdin immediately, + * while a standby parks the child on stdin — `rs fmt` loads the project config + * concurrently with draining stdin, so a parked child has already paid that + * cost — and writes only when a request consumes it. + */ +export interface RsFmtProcess { + readonly rsBinJs: string; + /** + * The terminal event. It is a value rather than a callback slot because the + * child has two independent observers over its life: the standby watches it + * while parked, and `serveRsFmt` watches it for the duration of a request. + */ + readonly exited: Promise; + /** Sends the document and closes stdin. */ + write(text: string): void; + kill(): void; + /** Chunks collected so far; joined once the child closes. */ + readonly stdout: readonly string[]; + readonly stderr: readonly string[]; +} + +export interface RsFmtSpawn { + readonly filePath: string; + readonly cwd: string; + readonly rsBinJs: string; +} + +export type RsFmtSpawnResult = + | { readonly kind: 'spawned'; readonly process: RsFmtProcess } + | { readonly kind: 'error'; readonly error: unknown }; + +/** Starts an `rs fmt` child and begins draining its streams. */ +export const spawnRsFmt = (options: RsFmtSpawn): RsFmtSpawnResult => { + let child: ChildProcessWithoutNullStreams; + try { + child = spawn( + process.execPath, + [ + options.rsBinJs, + 'fmt', + '--stdin-filepath', + options.filePath, + '--ignore-unknown', + ], + { + cwd: options.cwd, + env: CHILD_ENV, + stdio: 'pipe', + }, + ); + } catch (error) { + return { kind: 'error', error }; + } + + const stdout: string[] = []; + const stderr: string[] = []; + let stderrChars = 0; + child.stdout.setEncoding('utf8'); + child.stderr.setEncoding('utf8'); + child.stdout.on('data', (chunk: string) => stdout.push(chunk)); + child.stderr.on('data', (chunk: string) => { + stderr.push(chunk); + stderrChars += chunk.length; + while (stderrChars > MAX_STDERR_CHARS && stderr.length > 1) { + const dropped = stderr.shift(); + stderrChars -= dropped?.length ?? 0; + } + }); + // A child may reject the request before consuming stdin. Its exit status is + // authoritative; EPIPE and write-after-end must not become unhandled errors. + child.stdin.on('error', () => {}); + + const exited = new Promise((resolve) => { + child.on('error', (error) => resolve({ kind: 'error', error })); + child.on('close', (code) => resolve({ kind: 'close', code })); + }); + + return { + kind: 'spawned', + process: { + rsBinJs: options.rsBinJs, + stdout, + stderr, + exited, + write(text) { + try { + child.stdin.end(text); + } catch { + // Wait for the child's close/error event. + } + }, + kill() { + child.kill(); + }, + }, + }; +}; + +const interpretExit = ( + rsFmt: RsFmtProcess, + text: string, + code: number | null, +): RsFmtResult => { + const formatted = rsFmt.stdout.join(''); + const stderrText = rsFmt.stderr.join(''); + if (code === 0) { + if (formatted.length > 0 || text.trim() === '') { + return { kind: 'ok', formatted, stderr: stderrText }; + } + return { kind: 'skipped', stderr: stderrText }; + } + const tail = stderrTail(stderrText); + const missingEntry = stderrText + .split(/\r?\n/) + .some( + (line) => + line.includes('Cannot find module') && line.includes(rsFmt.rsBinJs), + ); + if (missingEntry) { + return launchError(rsFmt.rsBinJs, tail); } + return { + kind: 'error', + message: + tail || + `rs fmt exited with code ${code === null ? 'unknown' : String(code)}`, + }; +}; + +export interface RsFmtServe { + readonly process: RsFmtProcess; + readonly text: string; + readonly signal: AbortSignal; + /** + * Guard timeout. It measures the request, so for a standby it starts at + * consume time rather than at spawn time. + */ + readonly timeoutMs?: number; +} - return new Promise((resolve) => { +/** Writes the document to a child's stdin and resolves with its verdict. */ +export const serveRsFmt = (serve: RsFmtServe): Promise => + new Promise((resolve) => { + const rsFmt = serve.process; + const signal = serve.signal; + const timeoutMs = serve.timeoutMs ?? TIMEOUT_MS; let settled = false; + // Boxed rather than a plain `let`: the timeout is assigned below the + // closure that clears it, and `prefer-const` mis-reads that order as + // "never reassigned". const guard: { timeout?: NodeJS.Timeout } = {}; - let child: ChildProcessWithoutNullStreams | undefined; const onAbort = (): void => { - child?.kill(); + rsFmt.kill(); settle({ kind: 'cancelled' }); }; const settle = (result: RsFmtResult): void => { @@ -88,104 +246,59 @@ export const runRsFmt = async (run: RsFmtRun): Promise => { return; } settled = true; - if (guard.timeout) { - clearTimeout(guard.timeout); - } - run.signal.removeEventListener('abort', onAbort); + clearTimeout(guard.timeout); + signal.removeEventListener('abort', onAbort); resolve(result); }; - try { - child = spawn( - process.execPath, - [ - run.rsBinJs, - 'fmt', - '--stdin-filepath', - run.filePath, - '--ignore-unknown', - ], - { - cwd: run.cwd, - env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' }, - signal: run.signal, - stdio: 'pipe', - }, - ); - } catch (error) { - settle( - run.signal.aborted - ? { kind: 'cancelled' } - : launchError(run.rsBinJs, errorMessage(error)), - ); - return; - } - - const stdout: string[] = []; - const stderr: string[] = []; - child.stdout.setEncoding('utf8'); - child.stderr.setEncoding('utf8'); - child.stdout.on('data', (chunk: string) => stdout.push(chunk)); - child.stderr.on('data', (chunk: string) => stderr.push(chunk)); - child.on('error', (error) => { - settle( - run.signal.aborted - ? { kind: 'cancelled' } - : launchError(run.rsBinJs, errorMessage(error)), - ); - }); - child.on('close', (code) => { - if (run.signal.aborted) { + void rsFmt.exited.then((exit) => { + if (signal.aborted) { settle({ kind: 'cancelled' }); return; } - - const formatted = stdout.join(''); - const stderrText = stderr.join(''); - if (code === 0) { - if (formatted.length > 0 || run.text.trim() === '') { - settle({ kind: 'ok', formatted, stderr: stderrText }); - } else { - settle({ kind: 'skipped', stderr: stderrText }); - } - return; - } - const tail = stderrTail(stderrText); - const missingEntry = stderrText - .split(/\r?\n/) - .some( - (line) => - line.includes('Cannot find module') && line.includes(run.rsBinJs), - ); - if (missingEntry) { - settle(launchError(run.rsBinJs, tail)); - return; - } - settle({ - kind: 'error', - message: - tail || - `rs fmt exited with code ${code === null ? 'unknown' : String(code)}`, - }); + settle( + exit.kind === 'error' + ? launchError(rsFmt.rsBinJs, errorMessage(exit.error)) + : interpretExit(rsFmt, serve.text, exit.code), + ); }); + if (signal.aborted) { + // `addEventListener` never fires on an already-aborted signal, and a + // standby's child is not bound to the request signal at spawn time. + onAbort(); + return; + } - run.signal.addEventListener('abort', onAbort, { once: true }); + signal.addEventListener('abort', onAbort, { once: true }); guard.timeout = setTimeout(() => { - child?.kill(); + rsFmt.kill(); settle({ kind: 'error', - message: `rs fmt at ${run.rsBinJs} timed out after ${TIMEOUT_MS / 1000} seconds`, + message: `rs fmt at ${rsFmt.rsBinJs} timed out after ${timeoutMs / 1000} seconds`, }); - }, TIMEOUT_MS); - - // A child may reject the request before consuming stdin. Its exit status is - // authoritative; EPIPE and write-after-end must not become unhandled errors. - child.stdin.on('error', () => {}); - try { - child.stdin.end(run.text); - } catch { - // Wait for the child's close/error event. - } + }, timeoutMs); + + rsFmt.write(serve.text); + }); + +export const runRsFmt = async (run: RsFmtRun): Promise => { + if (run.signal.aborted) { + return { kind: 'cancelled' }; + } + + const spawned = spawnRsFmt({ + filePath: run.filePath, + cwd: run.cwd, + rsBinJs: run.rsBinJs, + }); + if (spawned.kind === 'error') { + return launchError(run.rsBinJs, errorMessage(spawned.error)); + } + + return serveRsFmt({ + process: spawned.process, + text: run.text, + signal: run.signal, }); }; diff --git a/packages/vscode/src/stacks/fmt/standby.test.ts b/packages/vscode/src/stacks/fmt/standby.test.ts new file mode 100644 index 0000000..11e397b --- /dev/null +++ b/packages/vscode/src/stacks/fmt/standby.test.ts @@ -0,0 +1,284 @@ +import path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from '@rstest/core'; +import { FmtStandby, type StandbyKey } from './standby'; +import { createStubRoot, type StubRoot } from './stubProcess'; + +/** + * The stub scripts stand in for `rs fmt`: they are spawned exactly the way the + * CLI is, so the tests exercise the real parking, stdin write and exit + * interpretation rather than a mocked child process (same technique as + * `run.test.ts`). + */ +describe('FmtStandby', () => { + let stubs: StubRoot; + let standbys: FmtStandby[]; + let logs: string[]; + + beforeEach(() => { + stubs = createStubRoot('standby'); + standbys = []; + logs = []; + }); + + afterEach(() => { + for (const standby of standbys) { + standby.dispose(); + } + stubs.remove(); + }); + + const writeStub = (source: string): string => stubs.write(source); + const echoStub = (): string => stubs.echo(); + + const key = (rsBinJs: string, file = 'input.ts'): StandbyKey => ({ + cwd: stubs.path, + filePath: path.join(stubs.path, file), + rsBinJs, + }); + + const createStandby = ( + options: { idleTimeoutMs?: number; requestTimeoutMs?: number } = {}, + ): FmtStandby => { + const standby = new FmtStandby({ + log: (message) => logs.push(message), + ...options, + }); + standbys.push(standby); + return standby; + }; + + const consume = ( + standby: FmtStandby, + standbyKey: StandbyKey, + text: string, + signal: AbortSignal = new AbortController().signal, + ) => standby.consume(standbyKey, { text, signal }); + + const eventually = async ( + probe: () => boolean, + what: string, + ): Promise => { + const deadline = Date.now() + 5_000; + while (!probe()) { + if (Date.now() >= deadline) { + throw new Error(`timed out waiting for ${what}`); + } + await new Promise((resolve) => setTimeout(resolve, 10)); + } + }; + + it('arms a parked process that has not been written to', async () => { + // The stub only ever prints once it has read stdin, so seeing no output + // after arming is what "parked" means. + const stub = writeStub( + 'process.stdin.on("data", () => process.stdout.write("served"));\n', + ); + const standby = createStandby(); + const armed = key(stub); + expect(standby.arm(armed)).toBe(true); + expect(standby.armedFilePath).toBe(armed.filePath); + expect(logs).toContain(`Standby armed for ${armed.filePath}`); + + await new Promise((resolve) => setTimeout(resolve, 100)); + expect(standby.armedFilePath).toBe(armed.filePath); + }); + + it('serves the request it was armed for from the parked process', async () => { + const standby = createStandby(); + const armed = key(echoStub()); + expect(standby.arm(armed)).toBe(true); + const text = `const value = 1;\n${'x'.repeat(1024 * 64)}\n`; + await expect(consume(standby, armed, text)).resolves.toEqual({ + kind: 'ok', + formatted: text, + stderr: '', + }); + expect(logs).toContain(`Standby consumed for ${armed.filePath}`); + }); + + it('serves exactly one request', async () => { + const standby = createStandby(); + const armed = key(echoStub()); + standby.arm(armed); + await consume(standby, armed, 'const value = 1;\n'); + expect(consume(standby, armed, 'const value = 1;\n')).toBeUndefined(); + expect(standby.armedFilePath).toBeUndefined(); + }); + + it('misses when the request key does not match', async () => { + const standby = createStandby(); + const stub = echoStub(); + const armed = key(stub); + standby.arm(armed); + + expect(consume(standby, key(stub, 'other.ts'), 'x')).toBeUndefined(); + expect( + consume(standby, { ...armed, cwd: path.join(stubs.path, 'nested') }, 'x'), + ).toBeUndefined(); + expect( + consume(standby, { ...armed, rsBinJs: echoStub() }, 'x'), + ).toBeUndefined(); + // A miss leaves the standby armed for the file it does match. + expect(standby.armedFilePath).toBe(armed.filePath); + await expect(consume(standby, armed, 'x')).resolves.toEqual({ + kind: 'ok', + formatted: 'x', + stderr: '', + }); + }); + + it('re-arms on another file by replacing the parked process', () => { + const standby = createStandby(); + const stub = echoStub(); + standby.arm(key(stub)); + const next = key(stub, 'other.ts'); + expect(standby.arm(next)).toBe(true); + expect(standby.armedFilePath).toBe(next.filePath); + expect(logs).toContain( + 'Standby killed (the active editor moved to another file)', + ); + }); + + it('keeps the same process when re-armed for the same key', () => { + const standby = createStandby(); + const armed = key(echoStub()); + standby.arm(armed); + expect(standby.arm(armed)).toBe(true); + expect( + logs.filter((line) => line.startsWith('Standby armed')), + ).toHaveLength(1); + }); + + it('kills a parked process on invalidation', () => { + const standby = createStandby(); + standby.arm(key(echoStub())); + standby.kill('the config changed'); + expect(standby.armedFilePath).toBeUndefined(); + expect(logs).toContain('Standby killed (the config changed)'); + expect(consume(standby, key(echoStub()), 'x')).toBeUndefined(); + }); + + it('discards a parked process that exits on its own', async () => { + const standby = createStandby(); + const armed = key(writeStub('process.exit(3);\n')); + expect(standby.arm(armed)).toBe(true); + await eventually( + () => standby.armedFilePath === undefined, + 'the parked process to be discarded', + ); + expect(logs).toContain('Standby killed (the parked process exited)'); + // The next request misses and the caller formats cold. + expect(consume(standby, armed, 'x')).toBeUndefined(); + }); + + it('reaps a standby nobody consumed', async () => { + const standby = createStandby({ idleTimeoutMs: 20 }); + const armed = key(echoStub()); + standby.arm(armed); + await eventually( + () => standby.armedFilePath === undefined, + 'the standby to be reaped', + ); + expect(logs).toContain('Standby killed (reaped after idling)'); + expect(consume(standby, armed, 'x')).toBeUndefined(); + }); + + it('does not reap a standby that is serving a request', async () => { + const standby = createStandby({ idleTimeoutMs: 30 }); + const armed = key( + writeStub( + 'process.stdin.on("data", (chunk) => setTimeout(() => { process.stdout.write(chunk); process.exit(0); }, 150));\n', + ), + ); + standby.arm(armed); + await expect(consume(standby, armed, 'late')).resolves.toEqual({ + kind: 'ok', + formatted: 'late', + stderr: '', + }); + }); + + it('starts the guard timeout at consume, not at arm', async () => { + const standby = createStandby({ requestTimeoutMs: 100 }); + const stub = writeStub('setTimeout(() => {}, 30_000);\n'); + const armed = key(stub); + standby.arm(armed); + // The parked process outlives the guard window before the request starts. + await new Promise((resolve) => setTimeout(resolve, 200)); + const result = await consume(standby, armed, 'const value = 1;\n'); + expect(result).toEqual({ + kind: 'error', + message: `rs fmt at ${stub} timed out after 0.1 seconds`, + }); + }); + + it('cancels a hot request and kills its process', async () => { + const standby = createStandby(); + const armed = key(writeStub('setTimeout(() => {}, 30_000);\n')); + standby.arm(armed); + const controller = new AbortController(); + const result = consume( + standby, + armed, + 'const value = 1;\n', + controller.signal, + ); + setTimeout(() => controller.abort(), 50); + await expect(result).resolves.toEqual({ kind: 'cancelled' }); + }); + + it('reports the CLI verdict for a failing hot request', async () => { + const standby = createStandby(); + const armed = key( + writeStub( + 'process.stdin.on("data", () => { console.error("boom"); process.exit(2); });\n', + ), + ); + standby.arm(armed); + await expect(consume(standby, armed, 'broken')).resolves.toEqual({ + kind: 'error', + message: 'boom', + }); + }); + + it('distinguishes an ignored file from a whitespace-only document', async () => { + const standby = createStandby(); + const silent = writeStub('process.stdin.resume();\n'); + const ignored = key(silent); + standby.arm(ignored); + await expect( + consume(standby, ignored, 'const value = 1;\n'), + ).resolves.toEqual({ kind: 'skipped', stderr: '' }); + + const blank = key(silent, 'blank.ts'); + standby.arm(blank); + await expect(consume(standby, blank, ' \n')).resolves.toEqual({ + kind: 'ok', + formatted: '', + stderr: '', + }); + }); + + it('does not arm after dispose', () => { + const standby = createStandby(); + standby.arm(key(echoStub())); + standby.dispose(); + expect(standby.armedFilePath).toBeUndefined(); + expect(logs).toContain('Standby killed (the fmt stack was disposed)'); + expect(standby.arm(key(echoStub()))).toBe(false); + expect(standby.armedFilePath).toBeUndefined(); + }); + + it('discards a standby whose CLI entry cannot be loaded', async () => { + const standby = createStandby(); + // The spawn succeeds — node reports an unloadable entry by exiting — so an + // arm on a broken resolution costs nothing but a debug line. + const armed = key(path.join(stubs.path, 'missing.js')); + expect(standby.arm(armed)).toBe(true); + await eventually( + () => standby.armedFilePath === undefined, + 'the unloadable standby to be discarded', + ); + expect(consume(standby, armed, 'x')).toBeUndefined(); + }); +}); diff --git a/packages/vscode/src/stacks/fmt/standby.ts b/packages/vscode/src/stacks/fmt/standby.ts new file mode 100644 index 0000000..d320ff6 --- /dev/null +++ b/packages/vscode/src/stacks/fmt/standby.ts @@ -0,0 +1,172 @@ +import { + errorMessage, + type RsFmtProcess, + type RsFmtResult, + serveRsFmt, + spawnRsFmt, +} from './run'; + +/** + * What a standby is bound to. `rs fmt` reads `--stdin-filepath` from argv and + * resolves its config from the spawn cwd, so none of the three can change + * after the process starts: a request that does not match all three has to be + * formatted cold. + */ +export interface StandbyKey { + readonly cwd: string; + readonly filePath: string; + readonly rsBinJs: string; +} + +export const sameStandbyKey = (left: StandbyKey, right: StandbyKey): boolean => + left.cwd === right.cwd && + left.filePath === right.filePath && + left.rsBinJs === right.rsBinJs; + +/** + * How long an armed-but-unconsumed standby is kept. Reaping is not an error — + * it reclaims the memory of a process nobody is going to use, and the next + * eligible event arms a new one. + */ +const IDLE_TIMEOUT_MS = 5 * 60_000; + +export interface FmtStandbyOptions { + /** + * Debug-level sink. The standby is invisible to the user: arming never + * reports status and never logs above debug. + */ + readonly log: (message: string) => void; + /** Idle lifetime of an armed standby; injected short by the unit tests. */ + readonly idleTimeoutMs?: number; + /** Guard timeout for a hot format; starts at consume, not at spawn. */ + readonly requestTimeoutMs?: number; +} + +interface ArmedStandby { + readonly key: StandbyKey; + readonly process: RsFmtProcess; + readonly idleTimer: NodeJS.Timeout; +} + +/** + * The single pre-spawned `rs fmt` process that tracks the active editor. + * + * It is a bounded exception to the extension's "no warm tier" rule: at most one + * process, bound to one file, serving exactly one request. There is no + * protocol, no pool and no state carried between requests — everything the + * standby saves is the process start-up plus config load a cold format pays on + * the critical path. + * + * Deliberately vscode-free (like `run.ts`) so the whole lifecycle is unit + * testable; the controller owns every VS Code event that drives it. + */ +export class FmtStandby { + #armed: ArmedStandby | undefined; + #disposed = false; + + constructor(private readonly options: FmtStandbyOptions) {} + + /** The file the standby currently targets, if any. */ + get armedFilePath(): string | undefined { + return this.#armed?.key.filePath; + } + + /** + * Spawns the standby for `key`, replacing any standby on another key. + * Returns whether a standby is now armed for `key`. Failures are reported + * through `log` only: a standby that cannot be armed costs a cold format, + * never a user-visible error. + */ + arm(key: StandbyKey): boolean { + if (this.#disposed) { + return false; + } + if (this.#armed) { + if (sameStandbyKey(this.#armed.key, key)) { + return true; + } + // Replacing, not invalidating: the caller is already arming the + // replacement, so this kill must not ask for another one. + this.kill('the active editor moved to another file'); + } + + const spawned = spawnRsFmt({ + cwd: key.cwd, + filePath: key.filePath, + rsBinJs: key.rsBinJs, + }); + if (spawned.kind === 'error') { + this.options.log( + `Standby not armed for ${key.filePath}: ${errorMessage(spawned.error)}`, + ); + return false; + } + + const armed: ArmedStandby = { + key, + process: spawned.process, + idleTimer: setTimeout(() => { + this.kill('reaped after idling'); + }, this.options.idleTimeoutMs ?? IDLE_TIMEOUT_MS), + }; + this.#armed = armed; + // A parked child that dies on its own (a crash, a config load throwing) is + // not an error either — the next request simply formats cold. The identity + // check is what makes this a no-op once the child has been consumed or + // replaced, so nothing has to unsubscribe. + void spawned.process.exited.then(() => { + if (this.#armed === armed) { + this.kill('the parked process exited'); + } + }); + this.options.log(`Standby armed for ${key.filePath}`); + return true; + } + + /** + * Serves one format request from the standby, or returns `undefined` when it + * cannot — nothing armed, a different key, or a standby already taken by a + * concurrent request. The caller then formats cold. + */ + consume( + key: StandbyKey, + request: { readonly text: string; readonly signal: AbortSignal }, + ): Promise | undefined { + const armed = this.#armed; + if (!armed || !sameStandbyKey(armed.key, key)) { + return undefined; + } + // A standby serves exactly one request. Taking it here is also what makes a + // second, concurrent request miss and go cold. + this.#armed = undefined; + clearTimeout(armed.idleTimer); + this.options.log(`Standby consumed for ${key.filePath}`); + return serveRsFmt({ + process: armed.process, + text: request.text, + signal: request.signal, + timeoutMs: this.options.requestTimeoutMs, + }); + } + + /** + * Invalidates the standby. The caller re-arms afterwards if the active editor + * still qualifies — a parked process has already loaded the project config, + * so a config edit (or any detection change) makes it wrong, not stale. + */ + kill(reason: string): void { + const armed = this.#armed; + if (!armed) { + return; + } + this.#armed = undefined; + clearTimeout(armed.idleTimer); + armed.process.kill(); + this.options.log(`Standby killed (${reason})`); + } + + dispose(): void { + this.#disposed = true; + this.kill('the fmt stack was disposed'); + } +} diff --git a/packages/vscode/src/stacks/fmt/stubProcess.ts b/packages/vscode/src/stacks/fmt/stubProcess.ts new file mode 100644 index 0000000..3d23c99 --- /dev/null +++ b/packages/vscode/src/stacks/fmt/stubProcess.ts @@ -0,0 +1,45 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +/** + * Stub-script scaffolding shared by the fmt unit suites. + * + * The suites stand in for `rs fmt` with tiny node scripts spawned exactly the + * way the CLI is, so they exercise real process behaviour — parking on stdin, + * back-pressure, EPIPE, exit codes — instead of a mocked child. That technique + * is load-bearing for both `run.test.ts` and `standby.test.ts`, so it lives + * here rather than being copied into each. + * + * Not a `*.test.ts` file (the runner would collect it as a suite) and not a + * build entry, so it never ships in the VSIX. + */ +export interface StubRoot { + /** Temp directory the stubs live in; also used as the spawn cwd. */ + readonly path: string; + /** Writes a stub script and returns its path. */ + write(source: string): string; + /** Echoes stdin back, i.e. an `rs fmt` that finds nothing to change. */ + echo(): string; + remove(): void; +} + +export const createStubRoot = (prefix: string): StubRoot => { + const root = fs.realpathSync( + fs.mkdtempSync(path.join(os.tmpdir(), `rstack-fmt-${prefix}-`)), + ); + const write = (source: string): string => { + const filePath = path.join( + root, + `rs-${Math.random().toString(16).slice(2)}.js`, + ); + fs.writeFileSync(filePath, source); + return filePath; + }; + return { + path: root, + write, + echo: () => write('process.stdin.pipe(process.stdout);\n'), + remove: () => fs.rmSync(root, { recursive: true, force: true }), + }; +}; diff --git a/packages/vscode/tests/e2e/suite/fmt.test.ts b/packages/vscode/tests/e2e/suite/fmt.test.ts index 1cc0933..7729212 100644 --- a/packages/vscode/tests/e2e/suite/fmt.test.ts +++ b/packages/vscode/tests/e2e/suite/fmt.test.ts @@ -5,6 +5,8 @@ import { eventually } from './helpers'; const EXTENSION_ID = 'rstack.rstack'; let provider: vscode.DocumentFormattingEditProvider; +let armedFilePath: () => string | undefined; +let lastServe: () => 'hot' | 'cold' | undefined; const folderNamed = (name: string): vscode.WorkspaceFolder => { const folder = (vscode.workspace.workspaceFolders ?? []).find( @@ -23,6 +25,12 @@ suite('fmt', () => { const exports = await api.whenStackActive('fmt'); assert.ok(exports.provider, 'the fmt stack did not export its provider'); provider = exports.provider as vscode.DocumentFormattingEditProvider; + assert.ok( + typeof exports.armedFilePath === 'function', + 'the fmt stack did not export its standby hook', + ); + armedFilePath = exports.armedFilePath as () => string | undefined; + lastServe = exports.lastServe as () => 'hot' | 'cold' | undefined; }); test('formats through the provider without touching the workspace', async () => { @@ -60,6 +68,47 @@ suite('fmt', () => { assert.equal(applied, 'const answer = { value: "42" };\n'); }); + test('formats from the standby armed for the active editor', async () => { + const uri = vscode.Uri.joinPath( + folderNamed('rstack').uri, + 'src', + 'needs-format.ts', + ); + // Showing the document is what arms the standby: the invariant is that the + // standby tracks the active editor. + const editor = await vscode.window.showTextDocument(uri); + // Arming is debounced, and an earlier test may have left a standby on + // another file, so poll until the armed file is this one. + await eventually(() => { + assert.equal(armedFilePath(), uri.fsPath); + }, 'the standby to be armed for needs-format.ts'); + + const edits = await vscode.commands.executeCommand( + 'vscode.executeFormatDocumentProvider', + uri, + { tabSize: 2, insertSpaces: true }, + ); + assert.ok(edits && edits.length > 0, 'the formatter returned no edits'); + + const text = editor.document.getText(); + let applied = text; + // The command post-processes our single minimal edit through VS Code's + // `computeMoreMinimalEdits`, so apply its result from the end backwards. + for (const edit of [...edits].sort( + (left, right) => + editor.document.offsetAt(right.range.start) - + editor.document.offsetAt(left.range.start), + )) { + const start = editor.document.offsetAt(edit.range.start); + const end = editor.document.offsetAt(edit.range.end); + applied = applied.slice(0, start) + edit.newText + applied.slice(end); + } + assert.equal(applied, 'const answer = { value: "42" };\n'); + // The cold path produces the same text, so only this proves the request + // actually consumed the standby. + assert.equal(lastServe(), 'hot'); + }); + test('returns no edits for a folder where fmt is not detected', async () => { const uri = vscode.Uri.joinPath( folderNamed('rslint').uri, From 53424eff2eb5e291a39f7f31f6b04473d736f9a1 Mon Sep 17 00:00:00 2001 From: fi3ework Date: Fri, 7 Aug 2026 14:44:17 +0800 Subject: [PATCH 2/6] chore(vscode): quiet the F5 playground's launch notifications The isolated playground profile installs no extensions, so --disable-extensions only contributed its own notification on every launch. Git is a built-in and ignores that flag entirely: it offered to open the parent repository each time, because every fixture lives inside this repo. Disable that one extension instead. --- .vscode/launch.json | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index f1450c2..62d990c 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -12,13 +12,19 @@ "runtimeExecutable": "${execPath}", "args": [ "--extensionDevelopmentPath=${workspaceFolder}/packages/vscode", - "--disable-extensions", - // An isolated, auto-created profile, so personal user settings - // (formatters, format-on-save, keybindings) cannot leak into the - // playground. `--user-data-dir` would be the stronger isolation, but - // the extension-host debugger strips it; `--profile` is the supported - // mechanism for debug launches. + // An isolated, auto-created profile, so neither personal user settings + // (formatters, format-on-save, keybindings) nor personal extensions + // can leak into the playground — the profile starts with none + // installed, which is why `--disable-extensions` is absent: it would + // only add its "all installed extensions are temporarily disabled" + // notification to every launch. `--user-data-dir` would be the + // stronger isolation, but the extension-host debugger strips it; + // `--profile` is the supported mechanism for debug launches. "--profile=rstack-playground", + // Built-in extensions ignore the empty profile. Git is the one that + // speaks up: every fixture sits under this repo, so it offers to open + // the parent repository on each launch. + "--disable-extension=vscode.git", // A fresh profile would otherwise prompt for workspace trust and greet // with the welcome tour on every first launch. "--disable-workspace-trust", From 1b0efb09ebf4fbe6e4ffae49f790d0e84cb3851e Mon Sep 17 00:00:00 2001 From: fi3ework Date: Fri, 7 Aug 2026 15:08:21 +0800 Subject: [PATCH 3/6] fix(vscode): drop the standby when the active editor cannot be armed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An active editor the provider matches but the stack cannot resolve — an undetected folder, a missing or too-old rstack package — logged the skip and returned, leaving the previous file's standby parked for up to five idle minutes. The editor had still moved on, so that standby no longer tracked it and could serve a programmatic request for a file the user had left. Arming already kills a standby it replaces; this path has nothing to arm, so it has to kill on its own. An editor holding nothing formattable at all still leaves the standby alone: a peek at the output panel must not cost the user their warm process. --- packages/vscode/src/stacks/fmt/index.ts | 13 +++++-- packages/vscode/tests/e2e/suite/fmt.test.ts | 42 +++++++++++++++++---- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/packages/vscode/src/stacks/fmt/index.ts b/packages/vscode/src/stacks/fmt/index.ts index bae9e28..6c6c31b 100644 --- a/packages/vscode/src/stacks/fmt/index.ts +++ b/packages/vscode/src/stacks/fmt/index.ts @@ -190,9 +190,12 @@ class FmtController implements StackController { /** * Arms a standby for whatever the active editor is *now* — the invariant is * "the standby tracks the active editor", so the editor is never captured - * when the arm was scheduled. Ineligible editors leave the standby alone - * rather than killing it: a peek at the output panel must not cost the user - * their warm process. + * when the arm was scheduled. + * + * Nothing formattable being active leaves the standby alone: a peek at the + * output panel must not cost the user their warm process. A formattable + * document that cannot be armed is the other case — the editor really did + * move on, so the standby goes with it. */ private armActiveEditor(): void { const context = this.#context; @@ -219,6 +222,10 @@ class FmtController implements StackController { context.output.debug( `Standby not armed for ${document.uri.fsPath}: ${resolution.kind}`, ); + // The editor still moved to another file, so the previous file's standby + // no longer tracks it. `arm` would have killed it; there is nothing to + // arm here, so this path has to. + standby.kill('the active editor moved to a file that cannot be armed'); return; } standby.arm(standbyKey(document.uri, resolution.target)); diff --git a/packages/vscode/tests/e2e/suite/fmt.test.ts b/packages/vscode/tests/e2e/suite/fmt.test.ts index 7729212..c162b60 100644 --- a/packages/vscode/tests/e2e/suite/fmt.test.ts +++ b/packages/vscode/tests/e2e/suite/fmt.test.ts @@ -8,6 +8,20 @@ let provider: vscode.DocumentFormattingEditProvider; let armedFilePath: () => string | undefined; let lastServe: () => 'hot' | 'cold' | undefined; +/** + * Shows a document and waits for it to own the standby. Showing it is what arms + * one — the invariant is that the standby tracks the active editor — but arming + * is debounced and an earlier test may have left a standby on another file, so + * the wait polls until the armed file is this one. + */ +const armStandbyFor = async (uri: vscode.Uri): Promise => { + const editor = await vscode.window.showTextDocument(uri); + await eventually(() => { + assert.equal(armedFilePath(), uri.fsPath); + }, `the standby to be armed for ${uri.fsPath}`); + return editor; +}; + const folderNamed = (name: string): vscode.WorkspaceFolder => { const folder = (vscode.workspace.workspaceFolders ?? []).find( (candidate) => candidate.name === name, @@ -74,14 +88,7 @@ suite('fmt', () => { 'src', 'needs-format.ts', ); - // Showing the document is what arms the standby: the invariant is that the - // standby tracks the active editor. - const editor = await vscode.window.showTextDocument(uri); - // Arming is debounced, and an earlier test may have left a standby on - // another file, so poll until the armed file is this one. - await eventually(() => { - assert.equal(armedFilePath(), uri.fsPath); - }, 'the standby to be armed for needs-format.ts'); + const editor = await armStandbyFor(uri); const edits = await vscode.commands.executeCommand( 'vscode.executeFormatDocumentProvider', @@ -109,6 +116,25 @@ suite('fmt', () => { assert.equal(lastServe(), 'hot'); }); + test('clears the standby when the active editor cannot be armed', async () => { + await armStandbyFor( + vscode.Uri.joinPath(folderNamed('rstack').uri, 'src', 'needs-format.ts'), + ); + + // TypeScript, so the provider matches it, but fmt is not detected for this + // folder — nothing can be armed, and the previous file's standby must not + // outlive the editor that owned it. + const unarmable = vscode.Uri.joinPath( + folderNamed('rslint').uri, + 'src', + 'index.ts', + ); + await vscode.window.showTextDocument(unarmable); + await eventually(() => { + assert.equal(armedFilePath(), undefined); + }, 'the standby to be cleared'); + }); + test('returns no edits for a folder where fmt is not detected', async () => { const uri = vscode.Uri.joinPath( folderNamed('rslint').uri, From 10d704baa3e6a9af19114f4e90fb0c0bd1c1eee0 Mon Sep 17 00:00:00 2001 From: fi3ework Date: Fri, 7 Aug 2026 15:22:55 +0800 Subject: [PATCH 4/6] docs(vscode): correct the standby's active-editor wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The docstring justified keeping a standby across an ineligible editor with a peek at the output panel, which cannot happen: the active editor is the focused one or, when nothing is focused, the most recently changed one, so focusing a panel leaves it pointing at the same file and fires no change. Name the cases that do reach it — no text document at all, or one this stack does not format — and say why neither is worth a kill. CONTEXT.md stated the invariant more strongly than the code holds it; it now states both outcomes. --- CONTEXT.md | 2 +- packages/vscode/src/stacks/fmt/index.ts | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 30412b0..6bb47a1 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -12,7 +12,7 @@ Glossary of terms used across rstack-editor. Code, docs, commit messages and rev ## fmt - **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. -- **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 always targets the active editor's file ("the standby tracks the active editor"). +- **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 the reap. - **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. - **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. - **Hot format** — a format request served by consuming the standby. diff --git a/packages/vscode/src/stacks/fmt/index.ts b/packages/vscode/src/stacks/fmt/index.ts index 6c6c31b..512e621 100644 --- a/packages/vscode/src/stacks/fmt/index.ts +++ b/packages/vscode/src/stacks/fmt/index.ts @@ -192,10 +192,12 @@ class FmtController implements StackController { * "the standby tracks the active editor", so the editor is never captured * when the arm was scheduled. * - * Nothing formattable being active leaves the standby alone: a peek at the - * output panel must not cost the user their warm process. A formattable - * document that cannot be armed is the other case — the editor really did - * move on, so the standby goes with it. + * Nothing formattable being active leaves the standby alone, whether the + * active editor holds no text document at all (a settings tab, an image + * preview) or holds one this stack does not format. Neither is worth a kill: + * the reap already bounds an idle standby, and both are a keystroke away + * from the file that owns it. A formattable document that cannot be armed is + * the other case — the editor really did move on, so the standby goes too. */ private armActiveEditor(): void { const context = this.#context; From 54789e4133b0f1f5970cbc3f2e826821636828fe Mon Sep 17 00:00:00 2001 From: fi3ework Date: Fri, 7 Aug 2026 15:23:58 +0800 Subject: [PATCH 5/6] docs(vscode): correct the standby's active-editor wording, rename reap to expire MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The docstring justified keeping a standby across an ineligible editor with a peek at the output panel, which cannot happen: the active editor is the focused one or, when nothing is focused, the most recently changed one, so focusing a panel leaves it pointing at the same file and fires no change. Name the cases that do reach it — no text document at all, or one this stack does not format — and say why neither is worth a kill. CONTEXT.md stated the invariant more strongly than the code holds it; it now states both outcomes. 'Reap' reads as jargon for what the idle timer does, so the term is now 'expire' in the glossary, the log reason and the tests. --- CONTEXT.md | 4 ++-- packages/vscode/src/stacks/fmt/index.ts | 4 ++-- packages/vscode/src/stacks/fmt/standby.test.ts | 8 ++++---- packages/vscode/src/stacks/fmt/standby.ts | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 6bb47a1..2d0a372 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -12,8 +12,8 @@ Glossary of terms used across rstack-editor. Code, docs, commit messages and rev ## fmt - **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. -- **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 the reap. +- **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. - **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. - **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. - **Hot format** — a format request served by consuming the standby. -- **Reap** — kill an idle standby to reclaim its memory. A reaped standby is not an error; the next eligible event simply arms a new one. +- **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. diff --git a/packages/vscode/src/stacks/fmt/index.ts b/packages/vscode/src/stacks/fmt/index.ts index 512e621..1e6e956 100644 --- a/packages/vscode/src/stacks/fmt/index.ts +++ b/packages/vscode/src/stacks/fmt/index.ts @@ -195,7 +195,7 @@ class FmtController implements StackController { * Nothing formattable being active leaves the standby alone, whether the * active editor holds no text document at all (a settings tab, an image * preview) or holds one this stack does not format. Neither is worth a kill: - * the reap already bounds an idle standby, and both are a keystroke away + * an idle standby expires on its own, and both are a keystroke away * from the file that owns it. A formattable document that cannot be armed is * the other case — the editor really did move on, so the standby goes too. */ @@ -400,7 +400,7 @@ class FmtController implements StackController { this.#abortController.signal.removeEventListener('abort', abortRequest); // A standby serves exactly one request, and a real format request is // itself proof the active file is worth one — re-arm after cold serves - // too, so a reaped or crashed standby comes back on the next use + // too, so an expired or crashed standby comes back on the next use // instead of leaving the file cold until the editor changes. Scheduled // rather than immediate: spawning here would delay the edits the caller // is waiting for. diff --git a/packages/vscode/src/stacks/fmt/standby.test.ts b/packages/vscode/src/stacks/fmt/standby.test.ts index 11e397b..fd2d40e 100644 --- a/packages/vscode/src/stacks/fmt/standby.test.ts +++ b/packages/vscode/src/stacks/fmt/standby.test.ts @@ -171,19 +171,19 @@ describe('FmtStandby', () => { expect(consume(standby, armed, 'x')).toBeUndefined(); }); - it('reaps a standby nobody consumed', async () => { + it('expires a standby nobody consumed', async () => { const standby = createStandby({ idleTimeoutMs: 20 }); const armed = key(echoStub()); standby.arm(armed); await eventually( () => standby.armedFilePath === undefined, - 'the standby to be reaped', + 'the standby to expire', ); - expect(logs).toContain('Standby killed (reaped after idling)'); + expect(logs).toContain('Standby killed (expired after idling)'); expect(consume(standby, armed, 'x')).toBeUndefined(); }); - it('does not reap a standby that is serving a request', async () => { + it('does not expire a standby that is serving a request', async () => { const standby = createStandby({ idleTimeoutMs: 30 }); const armed = key( writeStub( diff --git a/packages/vscode/src/stacks/fmt/standby.ts b/packages/vscode/src/stacks/fmt/standby.ts index d320ff6..8a56ce8 100644 --- a/packages/vscode/src/stacks/fmt/standby.ts +++ b/packages/vscode/src/stacks/fmt/standby.ts @@ -24,7 +24,7 @@ export const sameStandbyKey = (left: StandbyKey, right: StandbyKey): boolean => left.rsBinJs === right.rsBinJs; /** - * How long an armed-but-unconsumed standby is kept. Reaping is not an error — + * How long an armed-but-unconsumed standby is kept. Expiry is not an error — * it reclaims the memory of a process nobody is going to use, and the next * eligible event arms a new one. */ @@ -106,7 +106,7 @@ export class FmtStandby { key, process: spawned.process, idleTimer: setTimeout(() => { - this.kill('reaped after idling'); + this.kill('expired after idling'); }, this.options.idleTimeoutMs ?? IDLE_TIMEOUT_MS), }; this.#armed = armed; From a6ee6176f5c170a1d75e047ec12032097558dd28 Mon Sep 17 00:00:00 2001 From: fi3ework Date: Fri, 7 Aug 2026 15:51:18 +0800 Subject: [PATCH 6/6] fix(vscode): invalidate the standby on every rstack config event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The config watcher listened for content changes only, on the reasoning that a create or a delete moves the detection signature and arrives as a detection change instead. That holds for a real create or delete, not for the delete/create pair an editor emits when it saves by atomically replacing the file: the config path is gone and back within one scan, the signature records paths rather than content, and no detection change fires. Both halves were also ignored here, so nothing invalidated the standby and the parked process kept serving the config it had loaded at spawn — the same file then formatted one way hot and another way cold. Watch all three events. A redundant kill costs nothing: it is idempotent and the active editor re-arms through the usual debounce. --- packages/vscode/src/stacks/fmt/index.ts | 27 +++++++++++-------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/packages/vscode/src/stacks/fmt/index.ts b/packages/vscode/src/stacks/fmt/index.ts index 1e6e956..ba3f0b5 100644 --- a/packages/vscode/src/stacks/fmt/index.ts +++ b/packages/vscode/src/stacks/fmt/index.ts @@ -128,18 +128,15 @@ class FmtController implements StackController { this.provideDocumentFormattingEdits(document, token), }; // `rs fmt` loads the project config while it drains stdin, so a parked - // process already carries the old config. Detection does not fire on config - // *content* edits — its signature only tracks which files exist — so the - // standby needs its own watcher over the same glob. - // Create and delete already arrive as detection changes (the file set is - // part of its signature), so this watcher covers only the content edits - // detection cannot see — the same split `stacks/test/project.ts` uses. - const configWatcher = vscode.workspace.createFileSystemWatcher( - RSTACK_CONFIG_GLOB, - true, - false, - true, - ); + // process already carries the old config and every config event has to + // invalidate it. Detection cannot stand in for this watcher: its signature + // records only which config files exist, so it misses a content edit, and + // it misses the delete/create pair an atomic save produces for one + // unchanged path just the same. + const configWatcher = + vscode.workspace.createFileSystemWatcher(RSTACK_CONFIG_GLOB); + const onConfigEvent = (uri: vscode.Uri): void => + this.invalidateStandby(`${uri.fsPath} changed`); this.#subscriptions.push( context.onDidChangeDetection((snapshot) => { this.#snapshot = snapshot; @@ -156,9 +153,9 @@ class FmtController implements StackController { provider, ), configWatcher, - configWatcher.onDidChange((uri) => - this.invalidateStandby(`${uri.fsPath} changed`), - ), + configWatcher.onDidCreate(onConfigEvent), + configWatcher.onDidChange(onConfigEvent), + configWatcher.onDidDelete(onConfigEvent), vscode.window.onDidChangeActiveTextEditor(() => this.scheduleArm()), ); this.reportRunning(context, context.detection);