diff --git a/README.md b/README.md index 9ab7a8a..6797c8f 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,18 @@ Rstack Editor provides unified editor support for [Rstack](https://rstack.rs), t | --- | --- | | [`packages/vscode`](./packages/vscode) | The VS Code extension (`rstack.rstack`) | +## Roadmap + +The extension takes its configuration from five sources. The tool-native configs are fully supported today; support for driving a stack from `rstack.config.*` is landing one stack at a time. + +| Config source | Status | +| --- | --- | +| `rslint.config.*` | **Supported.** Diagnostics, quick fixes and the language server, all resolved from the `@rslint/core` installed in your project. | +| `rstest.config.*` | **Supported.** Test discovery, run and debug, watch mode, coverage and snapshot updates in the Test Explorer. | +| `define.test()` in `rstack.config.*` | **Supported.** Tests run through the same config shim `rs test` uses, so the editor and the CLI resolve the config identically. | +| `define.fmt()` in `rstack.config.*` | **Planned.** Detected and reported in the status bar; formatting itself arrives next, first over `rs fmt --stdin-filepath` and later over an `rs fmt` language server. | +| `define.lint()` in `rstack.config.*` | **Planned.** Linting a project configured only through `rstack.config.*` needs upstream changes in Rslint and rstack-cli before the editor can evaluate it correctly. `rs lint` on the command line is unaffected. | + ## License Rstack Editor is licensed under the [MIT License](./LICENSE). diff --git a/packages/vscode/AGENTS.md b/packages/vscode/AGENTS.md index 23c1b18..dc72c6e 100644 --- a/packages/vscode/AGENTS.md +++ b/packages/vscode/AGENTS.md @@ -19,6 +19,8 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten - One stack failing to register or crashing must never take another stack (or the shell) down. - The shell always activates; per-folder config detection decides which stacks start, and re-runs on config/lockfile changes without a window reload. Enable-settings are coarse kill switches only. +- Reconciles and restarts share one serialized queue (`enqueue`); a reconcile leaves a live stack alone, so the restart commands are the only path that rebuilds one. Do not add a second queue. +- Restart is a shell concern, not a stack one: `rstack.restart` rebuilds every controller, `rstack..restart` rebuilds one. A stack must never register its own restart command — a shallower "bounce the tool's process" restart keeps that controller's stale package resolution and version check, which is the bug the command exists to clear. - Deprecated `rslint.json` / `rslint.jsonc` are unsupported by decision, not omission — never make them detection signals. - Never share a child process across stacks: the tools have incompatible cwd semantics (lint LSP anchors on spawn cwd; test worker pins to project root; `rs fmt` resolves config from spawn cwd with no upward walk). - In Restricted Mode (workspace trust), only the status bar runs — no process spawns, no project code loaded. @@ -30,6 +32,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 stub on purpose. The MVP will spawn `rs fmt --stdin-filepath` with cwd = the config directory (forced by rs fmt's cwd-only config resolution); the endgame is an upstream LSP, so do not add a warm-process middle tier or "fix" the stub into an error state. +- `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. ## Testing diff --git a/packages/vscode/README.md b/packages/vscode/README.md index 4c356ef..300c25a 100644 --- a/packages/vscode/README.md +++ b/packages/vscode/README.md @@ -26,7 +26,9 @@ The extension activates on startup, then decides **per workspace folder** which | Rstest | `rstest.config.{mjs,ts,js,cjs,mts,cts}` (configurable) or `rstack.config.*` | | rstack-cli | `rstack.config.*` or `node_modules/.bin/rs` | -Config files and lockfiles are watched, so detection re-runs without a window reload. Deprecated `rslint.json` / `rslint.jsonc` configs are **not** detection signals — migrate them with `rslint --init`. +Config files and lockfiles are watched, so detection re-runs without a window reload. When something changes that none of those files record — a reinstall that leaves the lockfile untouched, or a `node_modules` that ends up broken — run **Rstack: Relaunch Extension** from the Command Palette (also on the status bar hover) to tear every tool down and start over. To rebuild a single tool, use **Rstack: Restart Rslint** / **Restart Rstest** / **Restart rs fmt**. + +A restart re-resolves every binary and package version and respawns every tool process, but it cannot reload JavaScript the editor has already imported from your project — Node keeps those modules for the lifetime of the window. If a reinstall replaced `@rslint/core` in place and lint still behaves like the old version, reload the window. Deprecated `rslint.json` / `rslint.jsonc` configs are **not** detection signals — migrate them with `rslint --init`. ## Supported package versions diff --git a/packages/vscode/package.json b/packages/vscode/package.json index 380ae84..65532df 100644 --- a/packages/vscode/package.json +++ b/packages/vscode/package.json @@ -43,16 +43,17 @@ }, "contributes": { "commands": [ - { - "command": "rstack.showMenu", - "title": "Show Menu", - "category": "Rstack" - }, { "command": "rstack.showOutput", "title": "Show Extension Log", "category": "Rstack" }, + { + "command": "rstack.restart", + "title": "Relaunch Extension", + "category": "Rstack", + "icon": "$(debug-restart)" + }, { "command": "rstack.migrateSettings", "title": "Migrate Rslint/Rstest Settings", @@ -65,7 +66,7 @@ }, { "command": "rstack.rslint.restart", - "title": "Restart Rslint Language Server", + "title": "Restart Rslint", "category": "Rstack", "icon": "$(refresh)" }, @@ -74,6 +75,12 @@ "title": "Show Rstest Log", "category": "Rstack" }, + { + "command": "rstack.rstest.restart", + "title": "Restart Rstest", + "category": "Rstack", + "icon": "$(refresh)" + }, { "command": "rstack.rstest.updateSnapshot", "title": "Update Snapshot", @@ -107,6 +114,12 @@ "command": "rstack.fmt.output.focus", "title": "Show rs fmt Log", "category": "Rstack" + }, + { + "command": "rstack.fmt.restart", + "title": "Restart rs fmt", + "category": "Rstack", + "icon": "$(refresh)" } ], "configuration": [ @@ -347,10 +360,18 @@ "command": "rstack.rstest.output.focus", "when": "rstack.rstest.active" }, + { + "command": "rstack.rstest.restart", + "when": "rstack.rstest.active" + }, { "command": "rstack.fmt.output.focus", "when": "rstack.fmt.active" }, + { + "command": "rstack.fmt.restart", + "when": "rstack.fmt.active" + }, { "command": "rstack.rstest.updateSnapshot", "when": "false" diff --git a/packages/vscode/rstest.config.mts b/packages/vscode/rstest.config.mts index bb76e16..7ea13c6 100644 --- a/packages/vscode/rstest.config.mts +++ b/packages/vscode/rstest.config.mts @@ -10,5 +10,13 @@ export default defineConfig({ externals: { vscode: 'commonjs vscode', }, + // An externalized dependency is imported by the chunk itself, so it loads + // before any `rs.mock` can intervene — and `vscode-languageclient/node` + // does a bare `require('vscode')` at load time, which no mock can serve + // from plain Node. Bundling it routes that require through the bundler, + // where the `vscode` external (and therefore the mock) applies. This is + // what lets a test import the shell, whose module graph reaches the Rslint + // stack. + bundleDependencies: ['vscode-languageclient'], }, }); diff --git a/packages/vscode/src/detection.test.ts b/packages/vscode/src/detection.test.ts index 8469048..8811706 100644 --- a/packages/vscode/src/detection.test.ts +++ b/packages/vscode/src/detection.test.ts @@ -1,17 +1,51 @@ import { describe, expect, it, rs } from '@rstest/core'; +import type vscode from 'vscode'; +import type { DetectionSnapshot } from './types'; // `detection.ts` imports the `vscode` namespace for the watcher/`findFiles` // paths. `detectionWatchPatterns` is pure, but the module still has to load, so // the namespace is stubbed away: unit tests run in plain Node, with no -// extension host (unit tests are Rstest, E2E is Electron). +// extension host (unit tests are Rstest, E2E is Electron). The stub carries +// exactly what `DetectionService` touches with no workspace folder open — its +// event plumbing — so the notification rules can be exercised here too. rs.mock('vscode', () => { - const vscode = {}; + class EventEmitter { + readonly #listeners = new Set<(value: unknown) => void>(); + readonly event = (listener: (value: unknown) => void) => { + this.#listeners.add(listener); + return { + dispose: () => { + this.#listeners.delete(listener); + }, + }; + }; + fire(value: unknown): void { + for (const listener of [...this.#listeners]) { + listener(value); + } + } + dispose(): void { + this.#listeners.clear(); + } + } + const disposable = { dispose: () => undefined }; + const vscode = { + EventEmitter, + workspace: { + // No folder is open: every pass produces the empty snapshot, so the + // detection signature is unchanged by construction. + workspaceFolders: undefined, + onDidChangeWorkspaceFolders: () => disposable, + onDidChangeConfiguration: () => disposable, + }, + }; return { ...vscode, default: vscode }; }); import { DEFAULT_RSTEST_CONFIG_GLOBS, DETECTION_WATCH_NAMES, + DetectionService, detectionWatchPatterns, } from './detection'; @@ -160,3 +194,42 @@ describe('detectionWatchPatterns', () => { } }); }); + +/** + * With no workspace folder open every pass yields the empty snapshot, so the + * detection signature is identical across passes by construction — exactly the + * shape a lockfile write or a `rstack.restart` produces in a real workspace + * whose `node_modules` was replaced without touching a watched file. + */ +describe('DetectionService — notification rules', () => { + const fakeOutput = () => + ({ + info: () => undefined, + warn: () => undefined, + error: () => undefined, + }) as unknown as vscode.LogOutputChannel; + + const listen = (service: DetectionService) => { + const seen: DetectionSnapshot[] = []; + service.onDidChange((snapshot) => seen.push(snapshot)); + return seen; + }; + + it('stays quiet when the signature did not change', async () => { + const service = new DetectionService(fakeOutput()); + const seen = listen(service); + await service.initialize(); + await service.refresh(); + expect(seen).toHaveLength(0); + service.dispose(); + }); + + it('does not notify after disposal', async () => { + const service = new DetectionService(fakeOutput()); + const seen = listen(service); + await service.initialize(); + service.dispose(); + await service.refresh(); + expect(seen).toHaveLength(0); + }); +}); diff --git a/packages/vscode/src/detection.ts b/packages/vscode/src/detection.ts index fc07227..d765af5 100644 --- a/packages/vscode/src/detection.ts +++ b/packages/vscode/src/detection.ts @@ -240,7 +240,9 @@ export class DetectionService implements vscode.Disposable { // out identical while every project-resolved package (Rslint binary, Rstest // core, the rstack shim) may now resolve differently. Such a pass must // notify subscribers even when the signature is unchanged, or failed - // resolutions are never retried until a window reload. + // resolutions are never retried until a window reload. Set by the lockfile + // watcher only — a caller that drives the rebuild itself does not need the + // event, it already has the fresh snapshot. #notifyUnchanged = false; #watchers: vscode.Disposable[] = []; #debounce: ReturnType | undefined; @@ -301,12 +303,15 @@ export class DetectionService implements vscode.Disposable { // Virtual filesystems cannot host a project-local toolchain. (folder) => folder.uri.scheme === 'file', ); + // Consumed before the first `await`: a pass that rejects (a folder removed + // mid-scan, a filesystem provider erroring) must not leave the flag set for + // an unrelated later pass to act on. + const notifyUnchanged = this.#notifyUnchanged; + this.#notifyUnchanged = false; const detections = await Promise.all(folders.map(detectFolder)); const snapshot = new Snapshot(detections); const signature = signatureOf(snapshot); this.#snapshot = snapshot; - const notifyUnchanged = this.#notifyUnchanged; - this.#notifyUnchanged = false; if (signature !== this.#signature || notifyUnchanged) { this.#signature = signature; this.log(snapshot); diff --git a/packages/vscode/src/extension.test.ts b/packages/vscode/src/extension.test.ts new file mode 100644 index 0000000..c18ba1f --- /dev/null +++ b/packages/vscode/src/extension.test.ts @@ -0,0 +1,471 @@ +/** + * The shell itself is the unit under test: `activate()` runs for real and the + * commands it contributes are invoked through the recorded command registry, + * the way VS Code would invoke them. Everything around it is stubbed — the + * `vscode` namespace, detection, and the three stack factories — because unit + * tests run in plain Node with no extension host (unit tests are Rstest, E2E is + * Electron, and E2E stays the ground truth for editor behaviour). + */ +import { afterEach, beforeEach, describe, expect, it, rs } from '@rstest/core'; +import type vscode from 'vscode'; + +interface FakeController { + register(): Promise>; + dispose(): Promise; +} + +const harness = rs.hoisted(() => { + /** + * Every field a test may set or read, rebuilt between tests. A factory rather + * than a list of resets in `beforeEach`, so a field added here cannot leak + * into the next test by being forgotten there. + */ + const defaults = () => ({ + /** Stacks detection currently reports as detected. */ + detected: new Set(), + /** Stacks whose controller rejects in `register` / `dispose`. */ + failRegister: new Set(), + failDispose: new Set(), + /** + * Stacks whose `dispose` blocks until released, so a test can hold a + * teardown open and act while it is in flight — the Rslint client's + * graceful-then-forced shutdown, in miniature. + */ + blockDispose: new Map>(), + /** Stacks whose `dispose` has started but not yet returned. */ + disposing: new Set(), + /** Set once the shell tears down the output channels it owns. */ + channelsDisposed: false, + /** Stacks whose `register` blocks until released. */ + blockRegister: new Map>(), + /** Stacks whose `register` has started but not yet returned. */ + registering: new Set(), + /** + * Stacks whose teardown began while their own `register` was still in + * flight — the signature of two passes running at once. + */ + overlaps: [] as string[], + /** Ordered `register:` / `dispose:` trace. */ + events: [] as string[], + /** One entry per detection pass the shell asked for. */ + refreshes: 0, + /** Everything the shell wrote to its own output channel. */ + shellLog: [] as string[], + commands: new Map unknown>(), + contextKeys: new Map(), + }); + const state = { + ...defaults(), + reset(): void { + Object.assign(state, defaults()); + }, + controller(stack: string): FakeController { + return { + register: async () => { + state.events.push(`register:${stack}`); + const block = state.blockRegister.get(stack); + if (block) { + state.registering.add(stack); + await block; + state.registering.delete(stack); + } + if (state.failRegister.has(stack)) { + throw new Error(`${stack} refuses to register`); + } + return { stack }; + }, + dispose: async () => { + state.events.push(`dispose:${stack}`); + if (state.registering.has(stack)) { + state.overlaps.push(stack); + } + const block = state.blockDispose.get(stack); + if (block) { + state.disposing.add(stack); + await block; + state.disposing.delete(stack); + } + if (state.failDispose.has(stack)) { + throw new Error(`${stack} refuses to dispose`); + } + }, + }; + }, + }; + return state; +}); + +rs.mock('vscode', () => { + const disposable = { dispose: () => undefined }; + // Detection is stubbed out below, so nothing here ever fires the shell's + // emitter — only its construction and disposal are reached. + class EventEmitter { + readonly event = () => disposable; + fire(): void {} + dispose(): void {} + } + const createOutputChannel = (name: string) => { + // Only the shell channel is recorded — the failure reports the user is + // supposed to find live there. + const record = (level: string, message: string) => { + if (name === 'Rstack') { + harness.shellLog.push(`${level}: ${message}`); + } + }; + return { + name, + info: (message: string) => record('info', message), + warn: (message: string) => record('warn', message), + error: (message: string) => record('error', message), + show: () => undefined, + dispose: () => { + harness.channelsDisposed = true; + }, + }; + }; + const vscode = { + EventEmitter, + StatusBarAlignment: { Left: 1, Right: 2 }, + ThemeColor: class { + constructor(readonly id: string) {} + }, + MarkdownString: class { + value = ''; + isTrusted = false; + appendMarkdown(text: string): this { + this.value += text; + return this; + } + }, + window: { + createOutputChannel, + createStatusBarItem: () => ({ + name: '', + text: '', + tooltip: undefined as unknown, + command: '', + backgroundColor: undefined as unknown, + show: () => undefined, + hide: () => undefined, + dispose: () => undefined, + }), + showInformationMessage: async () => undefined, + }, + commands: { + registerCommand: ( + command: string, + handler: (...args: unknown[]) => unknown, + ) => { + harness.commands.set(command, handler); + return { + dispose: () => { + harness.commands.delete(command); + }, + }; + }, + executeCommand: async (command: string, ...args: unknown[]) => { + if (command === 'setContext') { + harness.contextKeys.set(String(args[0]), Boolean(args[1])); + return undefined; + } + const handler = harness.commands.get(command); + if (!handler) { + throw new Error(`unknown command: ${command}`); + } + return handler(...args); + }, + }, + workspace: { + isTrusted: true, + workspaceFolders: [], + getConfiguration: () => ({ + get: (_key: string, fallback?: unknown) => fallback, + }), + onDidChangeConfiguration: () => disposable, + onDidChangeWorkspaceFolders: () => disposable, + onDidGrantWorkspaceTrust: () => disposable, + }, + }; + return { ...vscode, default: vscode }; +}); + +rs.mock('./detection', () => { + const snapshot = () => ({ + folders: [], + isDetected: (stack: string) => harness.detected.has(stack), + foldersFor: () => [], + forFolder: () => undefined, + }); + class DetectionService { + readonly onDidChange = () => ({ dispose: () => undefined }); + get snapshot() { + return snapshot(); + } + async initialize() { + return this.snapshot; + } + async refresh() { + harness.refreshes += 1; + return this.snapshot; + } + dispose() {} + } + return { DetectionService }; +}); + +rs.mock('./stacks/lint', () => ({ + createRslintController: () => harness.controller('rslint'), +})); +rs.mock('./stacks/test', () => ({ + createRstestController: () => harness.controller('rstest'), +})); +rs.mock('./stacks/fmt', () => ({ + createFmtController: () => harness.controller('fmt'), +})); +rs.mock('./migration', () => ({ + maybePromptForMigration: async () => undefined, + runSettingsMigration: async () => undefined, +})); + +import { activate, deactivate } from './extension'; + +const context = { subscriptions: [] } as unknown as vscode.ExtensionContext; + +const stacksOf = (kind: 'register' | 'dispose'): string[] => + harness.events + .filter((event) => event.startsWith(`${kind}:`)) + .map((event) => event.slice(kind.length + 1)) + .sort(); + +const phasesOf = (): string[] => + harness.events.map((event) => event.split(':')[0]); + +const run = async (command: string): Promise => { + const handler = harness.commands.get(command); + if (!handler) { + throw new Error(`${command} is not registered`); + } + await handler(); +}; + +const restart = (): Promise => run('rstack.restart'); + +/** + * Lets every already-scheduled continuation run. A macrotask turn drains the + * whole microtask queue behind it, so this is "whatever was going to happen + * without further input has happened" — not a guess at a tick count. + */ +const settle = (): Promise => + new Promise((resolve) => setTimeout(resolve, 0)); + +describe('the shell restart command', () => { + beforeEach(async () => { + harness.reset(); + harness.detected = new Set(['rslint', 'rstest', 'fmt']); + await activate(context); + expect(stacksOf('register')).toEqual(['fmt', 'rslint', 'rstest']); + harness.events.length = 0; + harness.refreshes = 0; + harness.shellLog.length = 0; + }); + + afterEach(async () => { + await deactivate(); + }); + + it('is contributed unconditionally, so it is reachable with no stack active', () => { + expect(harness.commands.has('rstack.restart')).toBe(true); + + // The command exists for the state where nothing is active, so the palette + // must offer it then. VS Code hides a contributed command from the palette + // only through a `contributes.menus.commandPalette` entry whose `when` is + // false — every other `rstack.*` command has one, so the guard here is the + // *absence* of an entry, which is exactly the kind of thing a later edit + // adds back by symmetry. + const manifest = require('../package.json') as { + contributes: { + commands: Array<{ command: string }>; + menus: { commandPalette: Array<{ command: string; when: string }> }; + }; + }; + expect( + manifest.contributes.commands.map((entry) => entry.command), + ).toContain('rstack.restart'); + expect( + manifest.contributes.menus.commandPalette.map((entry) => entry.command), + ).not.toContain('rstack.restart'); + + // The per-stack ones are the opposite: they only make sense for a stack + // that is up, so each is gated on its own context key. + for (const stack of ['rslint', 'rstest', 'fmt']) { + expect(harness.commands.has(`rstack.${stack}.restart`)).toBe(true); + expect( + manifest.contributes.menus.commandPalette.find( + (entry) => entry.command === `rstack.${stack}.restart`, + )?.when, + ).toBe(`rstack.${stack}.active`); + } + }); + + it('rebuilds only the named stack on rstack..restart', async () => { + await run('rstack.rstest.restart'); + + expect(stacksOf('dispose')).toEqual(['rstest']); + expect(stacksOf('register')).toEqual(['rstest']); + expect(phasesOf()).toEqual(['dispose', 'register']); + // Detection is global and cheap, and the point of the command is that a + // stale resolution gets redone — so it re-runs even for a single stack. + expect(harness.refreshes).toBe(1); + expect(harness.contextKeys.get('rstack.rstest.active')).toBe(true); + }); + + it('leaves a single-stack restart to the gate, same as a full one', async () => { + harness.detected.delete('rstest'); + + await run('rstack.rstest.restart'); + + expect(stacksOf('dispose')).toEqual(['rstest']); + expect(stacksOf('register')).toEqual([]); + expect(harness.contextKeys.get('rstack.rstest.active')).toBe(false); + }); + + it('disposes every registered stack and rebuilds the ones that still pass the gate', async () => { + harness.detected.delete('fmt'); + + await restart(); + + expect(stacksOf('dispose')).toEqual(['fmt', 'rslint', 'rstest']); + expect(stacksOf('register')).toEqual(['rslint', 'rstest']); + // A full reset: nothing is rebuilt before everything is torn down. + expect(phasesOf()).toEqual([ + 'dispose', + 'dispose', + 'dispose', + 'register', + 'register', + ]); + expect(harness.contextKeys.get('rstack.fmt.active')).toBe(false); + expect(harness.contextKeys.get('rstack.rslint.active')).toBe(true); + }); + + it('rebuilds a stack whose detection did not move at all', async () => { + // The whole point of the command: nothing observable changed, yet every + // controller is replaced, because `node_modules` may have been. + await restart(); + expect(stacksOf('dispose')).toEqual(['fmt', 'rslint', 'rstest']); + expect(stacksOf('register')).toEqual(['fmt', 'rslint', 'rstest']); + }); + + it('restarts the other stacks when one throws while disposing', async () => { + harness.failDispose.add('rslint'); + + await restart(); + + expect(stacksOf('dispose')).toEqual(['fmt', 'rslint', 'rstest']); + expect(stacksOf('register')).toEqual(['fmt', 'rslint', 'rstest']); + expect( + harness.shellLog.some( + (line) => + line.startsWith('error:') && + line.includes('Rslint failed to dispose'), + ), + ).toBe(true); + }); + + it('restarts the other stacks when one throws while registering', async () => { + harness.failRegister.add('rstest'); + + await restart(); + + expect(stacksOf('register')).toEqual(['fmt', 'rslint', 'rstest']); + expect(harness.contextKeys.get('rstack.rstest.active')).toBe(false); + expect(harness.contextKeys.get('rstack.rslint.active')).toBe(true); + expect(harness.contextKeys.get('rstack.fmt.active')).toBe(true); + expect( + harness.shellLog.some( + (line) => + line.startsWith('error:') && + line.includes('Rstest failed to register'), + ), + ).toBe(true); + }); + + it('serialises concurrent invocations instead of interleaving them', async () => { + await Promise.all([restart(), restart()]); + + expect(harness.refreshes).toBe(2); + expect(phasesOf()).toEqual([ + 'dispose', + 'dispose', + 'dispose', + 'register', + 'register', + 'register', + 'dispose', + 'dispose', + 'dispose', + 'register', + 'register', + 'register', + ]); + }); + + it('serialises a restart invoked while activation is still running', async () => { + // `registerCommands` runs before activation's first await, so the palette + // can reach restart while activation is still bringing stacks up. If + // activation's own reconcile does not ride the queue, the restart runs + // straight through it and retires a controller whose `register()` has not + // returned — that controller then goes on to publish its exports and flip + // `active` on, having already been disposed. + await deactivate(); + harness.events.length = 0; + harness.refreshes = 0; + harness.overlaps.length = 0; + + const blockedRegister = Promise.withResolvers(); + harness.blockRegister.set('rslint', blockedRegister.promise); + + const activating = activate(context); + await settle(); + expect(harness.registering.has('rslint')).toBe(true); + + const restarting = restart(); + await settle(); + + // The restart must still be waiting its turn, not tearing down a stack + // that is in the middle of coming up. + expect(harness.overlaps).toEqual([]); + + blockedRegister.resolve(); + await Promise.all([activating, restarting]); + expect(harness.overlaps).toEqual([]); + }); + + it('does not tear down shell resources while a restart is still disposing', async () => { + // Covers the window `dispose()` documents: mid-restart the controller map + // is already empty while the Rslint client is still shutting down, so a + // teardown that only walked that map would pull the channels out from + // under it. + // + // The assertion is on the channels rather than on deactivate's promise: + // "has not resolved yet" is a race with the microtask queue, whereas "the + // channel this teardown is still logging to is alive" is a fact. + const blockedDispose = Promise.withResolvers(); + harness.blockDispose.set('rslint', blockedDispose.promise); + + const restarting = restart(); + await settle(); + expect(harness.disposing.has('rslint')).toBe(true); + + const shutdown = deactivate(); + await settle(); + + expect(harness.disposing.has('rslint')).toBe(true); + expect(harness.channelsDisposed).toBe(false); + + blockedDispose.resolve(); + await Promise.all([restarting, shutdown]); + expect(harness.disposing.has('rslint')).toBe(false); + expect(harness.channelsDisposed).toBe(true); + }); +}); diff --git a/packages/vscode/src/extension.ts b/packages/vscode/src/extension.ts index 867b310..b6bed7f 100644 --- a/packages/vscode/src/extension.ts +++ b/packages/vscode/src/extension.ts @@ -12,6 +12,7 @@ import { type StackState, STACK_IDS, STACK_LABELS, + stackCommand, } from './types'; import { createFmtController } from './stacks/fmt'; import { createRslintController } from './stacks/lint'; @@ -93,6 +94,10 @@ class ExtensionShell { ); await this.#detection.initialize(); + // On the queue, not beside it. `registerCommands` runs before this method's + // first await, so a restart can be invoked from the palette while detection + // is still initialising — and running beside it would let that restart + // retire a controller whose `register()` has not returned yet. await this.reconcile(); void maybePromptForMigration(this.context, this.#channels.shell); @@ -105,15 +110,20 @@ class ExtensionShell { ); }; - register('rstack.showMenu', () => this.#statusBar.showMenu()); register('rstack.showOutput', () => this.#channels.shell.show()); + register('rstack.restart', () => this.restart()); register('rstack.migrateSettings', () => runSettingsMigration(this.#channels.shell), ); for (const stack of STACK_IDS) { - register(`rstack.${stack}.output.focus`, () => + register(stackCommand(stack, 'output.focus'), () => this.#channels.forStack(stack).show(), ); + // Owned by the shell, not the stack: a stack cannot rebuild itself, and + // the shallower alternative (bouncing just the tool's own process) leaves + // the controller's package resolution and version check stale. Stacks + // reach the same operation through `StackContext.requestRestart`. + register(stackCommand(stack, 'restart'), () => this.restart(stack)); } } @@ -162,24 +172,135 @@ class ExtensionShell { return { ok: true }; } + /** + * The single queue every pass runs on — reconciles and restarts alike. Two + * passes must never overlap, and a caller awaiting the returned promise + * waits for its own pass only. A rejection belongs to that caller, so the + * chain keeps only the settled shape and survives it. + */ + private enqueue(task: () => Promise): Promise { + const pass = this.#reconciling.then(task); + this.#reconciling = pass.catch(() => undefined); + return pass; + } + + /** + * Queues a reconcile. The `run*` methods below are the bodies of a pass and + * assume they already own the queue — going through `enqueue` from inside + * one would wait on itself. Everything else calls the wrappers. + */ + private reconcile(stacks?: readonly StackId[]): Promise { + return this.enqueue(() => this.runReconcile(stacks)); + } + private scheduleReconcile(): void { if (this.#disposed) { return; } - this.#reconciling = this.#reconciling.then( - () => this.reconcile(), - () => this.reconcile(), + void this.reconcile(); + } + + /** + * `rstack.restart` (every stack) and `rstack..restart` (one) — a full + * reset, not a "retry whatever looks broken". + * + * `reconcileStack` deliberately leaves an already registered stack alone, so + * a stack that is up but wedged is never rebuilt: `node_modules` can be + * replaced or corrupted while every watched file stays untouched, and until + * now only a window reload recovered from that. This disposes the + * controllers, re-runs detection and registers every affected stack that + * still passes the gate, from scratch. + * + * `reason` is for the callers that are not a user picking the command — + * `StackContext.requestRestart` passes what moved. + */ + restart(stack?: StackId, reason?: string): Promise { + return this.enqueue(() => this.runRestart(stack, reason)); + } + + private async runRestart(only?: StackId, reason?: string): Promise { + if (this.#disposed) { + return; + } + const stacks = only ? [only] : STACK_IDS; + const what = only ? STACK_LABELS[only] : 'Rstack'; + this.#channels.shell.info( + `Restarting ${what}${reason ? ` (${reason})` : ''}`, ); + await this.retireAll(stacks, { kind: 'starting' }); + // Teardown is slow (closing the Rslint language client, `$close()`ing the + // Rstest workers), so a `deactivate()` can land inside it. From here on + // every shell resource — the output channels above all — may already be + // disposed, and touching one throws. + if (this.#disposed) { + return; + } + try { + // A plain pass: `refresh` updates the snapshot whether or not the + // signature moved, and the reconcile below rebuilds every stack from it. + // The forced notification the lockfile path uses exists to make *live* + // controllers retry — there are none left to tell. + await this.#detection.refresh(); + } catch (error) { + if (!this.#disposed) { + this.#channels.shell.error( + `Detection failed during restart: ${errorMessage(error)}`, + ); + } + } + await this.runReconcile(stacks); + if (!this.#disposed) { + this.#channels.shell.info(`${what} restart finished`); + } } - private async reconcile(): Promise { + /** + * Retires whichever of `stacks` are registered. Per-stack isolation covers + * teardown too: one throwing on the way down must not keep the others from + * coming back up, or from being collected at all. + */ + private async retireAll( + stacks: readonly StackId[], + next?: StackState, + ): Promise { + await Promise.allSettled( + [...this.#controllers] + .filter(([stack]) => stacks.includes(stack)) + .map(([stack, controller]) => this.retire(stack, controller, next)), + ); + } + + /** + * Retires a controller. Only the state left on the status bar says why + * (`starting` for a restart about to rebuild it, the gate's own state when it + * stopped qualifying, `crashed` when it failed to register). Pass no state to + * retire it silently, which is what a shell already on its way out wants. + */ + private async retire( + stack: StackId, + controller: StackController, + next?: StackState, + ): Promise { + this.#controllers.delete(stack); + await this.disposeController(stack, controller); + if (!next || this.#disposed) { + return; + } + await this.setContextKey(`rstack.${stack}.active`, false); + this.#statusBar.setActive(stack, false); + this.#statusBar.setState(stack, next); + } + + private async runReconcile( + stacks: readonly StackId[] = STACK_IDS, + ): Promise { if (this.#disposed) { return; } const snapshot = this.#detection.snapshot; // Per-stack isolation: one stack throwing must never affect the others. await Promise.allSettled( - STACK_IDS.map((stack) => this.reconcileStack(stack, snapshot)), + stacks.map((stack) => this.reconcileStack(stack, snapshot)), ); } @@ -191,18 +312,23 @@ class ExtensionShell { `rstack.${stack}.detected`, snapshot.isDetected(stack), ); + // `#disposed` flips outside the queue, so it can become true mid-pass even + // though the teardown that follows it cannot start until this pass ends. + // Bailing here is the optimisation, not the safety net: anything this pass + // did register lands in `#controllers` and the queued teardown collects it. + if (this.#disposed) { + return; + } const gate = this.gate(stack, snapshot); const existing = this.#controllers.get(stack); if (!gate.ok) { if (existing) { - this.#controllers.delete(stack); - await this.disposeController(stack, existing); - await this.setContextKey(`rstack.${stack}.active`, false); - this.#statusBar.setActive(stack, false); + await this.retire(stack, existing, gate.state); + } else { + this.#statusBar.setState(stack, gate.state); } - this.#statusBar.setState(stack, gate.state); return; } @@ -222,7 +348,15 @@ class ExtensionShell { status: this.#statusBar.reporterFor(stack), detection: snapshot, onDidChangeDetection: this.#detectionEmitter.event, + requestRestart: (reason) => this.restart(stack, reason), }); + // Retiring it here rather than leaving it to the queued teardown skips + // publishing exports and flipping `active` on for a stack the extension + // is already shutting down. + if (this.#disposed) { + await this.retire(stack, controller); + return; + } if (stackExports) { this.publishStackExports(stack, stackExports); } @@ -230,19 +364,18 @@ class ExtensionShell { this.#statusBar.setActive(stack, true); this.#channels.shell.info(`${STACK_LABELS[stack]} registered`); } catch (error) { - this.#controllers.delete(stack); - await this.disposeController(stack, controller); - await this.setContextKey(`rstack.${stack}.active`, false); - this.#statusBar.setActive(stack, false); + await this.retire(stack, controller, { + kind: 'crashed', + detail: error instanceof Error ? error.message : String(error), + }); + if (this.#disposed) { + return; + } const message = errorMessage(error); this.#channels.shell.error( `${STACK_LABELS[stack]} failed to register: ${message}`, ); this.#channels.forStack(stack).error(message); - this.#statusBar.setState(stack, { - kind: 'crashed', - detail: error instanceof Error ? error.message : String(error), - }); } } @@ -254,6 +387,12 @@ class ExtensionShell { try { await controller.dispose(); } catch (error) { + // `dispose()` closes the channels after its controller loop, and a stack + // can still be shutting down then; a failed dispose must not become an + // unhandled "channel closed" on the way out. + if (this.#disposed) { + return; + } this.#channels.shell.error( `${STACK_LABELS[stack]} failed to dispose: ${errorMessage(error)}`, ); @@ -303,16 +442,24 @@ class ExtensionShell { async dispose(): Promise { this.#disposed = true; - for (const [stack, controller] of [...this.#controllers]) { - this.#controllers.delete(stack); - await this.disposeController(stack, controller); - } + // Before the wait below, not after it: the service holds a debounce timer + // and its own watchers, so leaving it live means a file touched during + // shutdown can start a fresh detection pass behind us. + this.#detection.dispose(); + // Behind the shared queue rather than beside it. `retire` drops a + // controller from `#controllers` before awaiting its teardown, so a + // restart in flight leaves a window where the map is already empty and the + // Rslint client is still shutting down: disposing the channels there would + // pull them out from under it, and `deactivate()` would resolve before the + // child processes are gone. The queue is what makes "whatever was in + // flight has finished" something this can wait for, and `#disposed` above + // stops that pass from rebuilding anything on its way out. + await this.enqueue(() => this.retireAll(STACK_IDS)); for (const subscription of this.#subscriptions) { subscription.dispose(); } this.#subscriptions.length = 0; this.#detectionEmitter.dispose(); - this.#detection.dispose(); this.#statusBar.dispose(); this.#channels.dispose(); } diff --git a/packages/vscode/src/stacks/lint/index.ts b/packages/vscode/src/stacks/lint/index.ts index 235f028..c4c0599 100644 --- a/packages/vscode/src/stacks/lint/index.ts +++ b/packages/vscode/src/stacks/lint/index.ts @@ -116,7 +116,6 @@ class RslintController implements StackController { #snapshot: DetectionSnapshot | undefined; readonly #subscriptions: vscode.Disposable[] = []; readonly #folderStates = new Map(); - #pending: Promise = Promise.resolve(); #disposed = false; async register(context: StackContext): Promise> { @@ -125,9 +124,6 @@ class RslintController implements StackController { this.#logger = new Logger(context.output); this.#subscriptions.push( - vscode.commands.registerCommand('rstack.rslint.restart', () => { - void this.restart(); - }), context.onDidChangeDetection((snapshot) => { this.#snapshot = snapshot; this.reconcileFolders({ added: [], removed: [] }); @@ -141,14 +137,16 @@ class RslintController implements StackController { }), // A `binPath`/`customBinPath` change must re-resolve the binary, which // only happens on a fresh start (upstream documents `customBinPath` as - // requiring a reload; a restart is strictly better). + // requiring a reload; a restart is strictly better). It asks the shell + // for a full rebuild rather than restarting locally — a local restart + // would replace the coordinator but keep this controller's + // already-resolved binary and version check. vscode.workspace.onDidChangeConfiguration((event) => { - if ( - event.affectsConfiguration('rstack.rslint.binPath') || - event.affectsConfiguration('rstack.rslint.customBinPath') || - event.affectsConfiguration('rstack.rslint.trace.server') - ) { - void this.restart(); + for (const setting of ['binPath', 'customBinPath', 'trace.server']) { + if (event.affectsConfiguration(`rstack.rslint.${setting}`)) { + void context.requestRestart(`rstack.rslint.${setting} changed`); + return; + } } }), ); @@ -284,33 +282,6 @@ class RslintController implements StackController { ); } - /** - * Serializes restart/dispose. Two restarts racing (a settings change plus the - * palette command) would otherwise interleave close and start and leak a - * coordinator that nothing holds a reference to any more. - */ - private enqueue(task: () => Promise): Promise { - this.#pending = this.#pending.then(task, task); - return this.#pending; - } - - /** - * `rstack.rslint.restart`. The coordinator is single-use once closed, so a - * restart replaces it (and the document router) wholesale — the same shape - * upstream's commented-out `rslint.restart` would have needed. - */ - private async restart(): Promise { - await this.enqueue(async () => { - if (this.#disposed || !this.#context) { - return; - } - this.#logger?.info('Restarting the Rslint language server'); - await this.closeCoordinator(); - this.#folderStates.clear(); - this.startCoordinator(); - }); - } - private async closeCoordinator(): Promise { const coordinator = this.#coordinator; this.#coordinator = undefined; @@ -329,15 +300,11 @@ class RslintController implements StackController { for (const subscription of this.#subscriptions.splice(0)) { subscription.dispose(); } - // Behind the same queue as `restart`, so an in-flight restart finishes - // (or no-ops on `#disposed`) before the language servers are torn down. - await this.enqueue(async () => { - await this.closeCoordinator(); - this.#folderStates.clear(); - this.#logger = undefined; - this.#context = undefined; - this.#snapshot = undefined; - }); + await this.closeCoordinator(); + this.#folderStates.clear(); + this.#logger = undefined; + this.#context = undefined; + this.#snapshot = undefined; } } diff --git a/packages/vscode/src/stacks/lint/projectModules.ts b/packages/vscode/src/stacks/lint/projectModules.ts index 73953fd..4ac83dd 100644 --- a/packages/vscode/src/stacks/lint/projectModules.ts +++ b/packages/vscode/src/stacks/lint/projectModules.ts @@ -16,6 +16,18 @@ import { pathToFileURL } from 'node:url'; * exactly the bundling seam the resolve-from-project adaptation deletes. * - Windows: only `file:`/`data:`/`node:` URLs are accepted by Node's default * ESM loader, so absolute paths are converted with `pathToFileURL`. + * + * There is deliberately no invalidation hook, and a restart does not get one. + * The memo below is the *second* cache in front of these modules: Node's own + * ESM registry is keyed by resolved URL and lives as long as the extension + * host, so once a path has loaded, re-importing it returns the identical + * module object no matter what this map says. Adding a cache-busting query to + * the specifier does reload the entry module, but a relative specifier inside + * it does not inherit the query — the result is a fresh entry wired to its own + * stale dependencies, which is worse than being consistently stale. A + * `node_modules` replaced in place under an unchanged path therefore needs a + * window reload; see the README's note on what restart does and does not + * recover. */ const cache = new Map>(); @@ -35,8 +47,3 @@ export const importProjectModule = async ( } return pending; }; - -/** Test seam / teardown helper: drops the memoized module promises. */ -export const clearProjectModuleCache = (): void => { - cache.clear(); -}; diff --git a/packages/vscode/src/statusBar.ts b/packages/vscode/src/statusBar.ts index 2e48559..b629c91 100644 --- a/packages/vscode/src/statusBar.ts +++ b/packages/vscode/src/statusBar.ts @@ -5,26 +5,31 @@ import { type StatusReporter, STACK_IDS, STACK_LABELS, + stackCommand, } from './types'; -/** Commands the status bar hover links to, per stack. */ -const OUTPUT_COMMANDS: Readonly> = { - rslint: 'rstack.rslint.output.focus', - rstest: 'rstack.rstest.output.focus', - fmt: 'rstack.fmt.output.focus', -}; - -const RESTART_COMMANDS: Partial>> = { - rslint: 'rstack.rslint.restart', -}; - -const STATE_ICONS: Readonly> = { - 'not-detected': '$(circle-slash)', - disabled: '$(circle-slash)', - starting: '$(loading~spin)', - running: '$(check)', - crashed: '$(error)', - 'version-mismatch': '$(warning)', +/** + * Icon and hover colour per state. `color` is a theme colour id with `.` + * replaced by `-`, the form VS Code exposes as a CSS variable; the markdown + * sanitizer accepts `var(--vscode-*)` on a `` and nothing else, so the + * hover picks up the user's theme instead of hard-coded hexes. + */ +const STATE_STYLES: Readonly< + Record +> = { + // The two off-states share a glyph but not a colour: nothing found here (the + // weakest thing on the row) versus somebody turned it off on purpose, which + // is worth reading. Keep every state on the plain codicon set — glyphs from + // the debug sets are drawn at their own optical size and stick out. + 'not-detected': { icon: '$(circle-slash)', color: 'disabledForeground' }, + disabled: { icon: '$(circle-slash)', color: 'descriptionForeground' }, + starting: { icon: '$(loading~spin)', color: 'descriptionForeground' }, + running: { icon: '$(check)', color: 'testing-iconPassed' }, + crashed: { icon: '$(error)', color: 'testing-iconFailed' }, + 'version-mismatch': { + icon: '$(warning)', + color: 'editorWarning-foreground', + }, }; const stateText = (state: StackState): string => { @@ -44,21 +49,52 @@ const stateText = (state: StackState): string => { } }; +const escapeAttribute = (value: string): string => + value + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"'); + +/** + * The one anchor builder, so escaping is a property of the markup rather than + * something each call site has to remember. `title` becomes the icon's native + * tooltip, which is where the wording goes for the icon-only actions. + */ +const anchor = (command: string, body: string, title?: string): string => + `${body}`; + +/** + * A labelled command as a table row, so its icon lands in the same column as + * the stack rows'. Icon and label are separate cells and therefore separate + * anchors to the same command — one `` cannot span two cells. + */ +const actionRow = (command: string, icon: string, label: string): string => + `${anchor(command, icon)}` + + `${anchor(command, ` ${label}`)}`; + /** * The single always-present status bar item. It is visible * whenever the extension is installed and enabled, even when nothing is * detected — it is the answer to "is the extension broken or just idle?". + * + * The hover is the only surface: it carries the per-stack state and every + * action, and clicking the item goes straight to the extension log. There is + * deliberately no QuickPick behind the item — a menu that repeats what the + * hover already shows is two renderings of one model, and every peer + * (Biome, oxc, Prettier) ships exactly one of the two. */ export class StatusBar implements vscode.Disposable { readonly #item: vscode.StatusBarItem; readonly #states = new Map( STACK_IDS.map((stack) => [stack, { kind: 'not-detected' }]), ); - // Stacks whose controller is currently registered. Restart availability - // tracks this, not the state kind: a state cannot distinguish "crashed - // while running" (controller alive, its restart command exists) from - // "failed to register" (controller disposed, the command with it), and a - // disabled stack never registered the command at all. + // Stacks whose controller is currently registered. The restart action tracks + // this rather than the state kind, which cannot tell "crashed while running" + // (controller alive, worth rebuilding) from "failed to register" (already + // disposed, and the reconcile that follows will retry it anyway). readonly #active = new Set(); constructor() { @@ -68,7 +104,10 @@ export class StatusBar implements vscode.Disposable { 100, ); this.#item.name = 'Rstack'; - this.#item.command = 'rstack.showMenu'; + // Clicking goes to the extension log, the way Prettier's item does. It is + // the one action that is useful in every state, including the states where + // no stack is active and there is nothing else to offer. + this.#item.command = 'rstack.showOutput'; this.render(); this.#item.show(); } @@ -104,51 +143,6 @@ export class StatusBar implements vscode.Disposable { this.render(); } - #canRestart(stack: StackId): boolean { - return RESTART_COMMANDS[stack] !== undefined && this.#active.has(stack); - } - - /** The QuickPick behind the status bar item. */ - async showMenu(): Promise { - type Item = vscode.QuickPickItem & { readonly command?: string }; - const items: Item[] = []; - for (const stack of STACK_IDS) { - const state = this.stateOf(stack); - items.push({ - label: `${STATE_ICONS[state.kind]} ${STACK_LABELS[stack]}`, - description: stateText(state), - detail: 'Show output', - command: OUTPUT_COMMANDS[stack], - }); - const restart = RESTART_COMMANDS[stack]; - if (restart && this.#canRestart(stack)) { - items.push({ - label: `$(refresh) Restart ${STACK_LABELS[stack]}`, - command: restart, - }); - } - } - items.push( - { label: '', kind: vscode.QuickPickItemKind.Separator }, - { - label: '$(output) Show Rstack extension log', - command: 'rstack.showOutput', - }, - { - label: '$(arrow-right) Migrate Rslint/Rstest settings', - command: 'rstack.migrateSettings', - }, - ); - - const picked = await vscode.window.showQuickPick(items, { - title: 'Rstack', - placeHolder: 'Select an action', - }); - if (picked?.command) { - await vscode.commands.executeCommand(picked.command); - } - } - private render(): void { const states = STACK_IDS.map((stack) => this.stateOf(stack)); const worst = states.find((state) => state.kind === 'crashed') @@ -179,7 +173,7 @@ export class StatusBar implements vscode.Disposable { this.#item.backgroundColor = undefined; break; default: - this.#item.text = '$(layers) Rstack'; + this.#item.text = '$(zap) Rstack'; this.#item.backgroundColor = undefined; break; } @@ -187,20 +181,77 @@ export class StatusBar implements vscode.Disposable { const tooltip = new vscode.MarkdownString(undefined, true); // Command links are only rendered in trusted markdown. tooltip.isTrusted = true; - tooltip.appendMarkdown('**Rstack**\n\n'); - for (const stack of STACK_IDS) { + // The stack rows are a raw `` so the three columns line up; markdown + // has no alignment short of a table with a visible header row, and one row + // per paragraph left the action icons ragged. Raw html suppresses markdown + // inside it, so the cells use ``/`` rather than `**`/`[]()` — the + // sanitizer keeps `command:` hrefs as long as the string stays trusted. + tooltip.supportHtml = true; + const rows = STACK_IDS.map((stack) => { const state = this.stateOf(stack); - const links = [`[Output](command:${OUTPUT_COMMANDS[stack]})`]; - const restart = RESTART_COMMANDS[stack]; - if (restart && this.#canRestart(stack)) { - links.push(`[Restart](command:${restart})`); + const style = STATE_STYLES[state.kind]; + const label = STACK_LABELS[stack]; + // The per-stack actions repeat once per row, so they are icon-only: the + // row already names the stack and the anchor title carries the wording. + const actions = [ + anchor( + stackCommand(stack, 'output.focus'), + '$(selection)', + `Show the ${label} log`, + ), + ]; + if (this.#active.has(stack)) { + // Titled apart from "Relaunch" below: this one rebuilds only this + // stack and leaves the others running. + actions.push( + anchor( + stackCommand(stack, 'restart'), + '$(refresh)', + `Restart ${label}`, + ), + ); } - tooltip.appendMarkdown( - `${STATE_ICONS[state.kind]} **${STACK_LABELS[stack]}** — ${stateText( - state, - )} · ${links.join(' · ')}\n\n`, + // The state text is the icon's title rather than row text: spelling out + // "running — 2 folders" on every row is noise once the icon says it, and + // the details worth reading (a crash message, a version mismatch) are + // exactly the long ones. `state.detail` is arbitrary text a stack + // produced, hence the escaping. + const status = + `${style.icon}`; + return ( + `` + + `` ); - } + }); + // In the same table as the stacks so all six icons share one column; a + // second table would size its columns independently and the two halves + // would drift apart. One row per action rather than three across, for the + // same reason the actions above are icon-only — the hover sizes to its + // content, so the widest line sets the card's width. + // + // Unlike the per-stack restarts, "Relaunch" is unconditional: it is the + // action for "nothing is active", which is precisely when no per-stack + // restart is offered. + const shellActions = [ + actionRow('rstack.restart', '$(debug-restart)', 'Relaunch'), + actionRow('rstack.showOutput', '$(selection)', 'Extension log'), + actionRow('rstack.migrateSettings', '$(arrow-right)', 'Migrate settings'), + ]; + // The gap under the divider is an empty spacer row with an explicit + // `height`, the one pixel-precise spacing lever sanitized html has left: + // cell padding is unreachable (`style` survives only on a span, colours + // only) and everything line-based is quantized to a whole row. An empty + // cell has no line box, so its `height` is what it says. Above the rule the + // hover's own `hr { margin-top: 4px }` is enough — and its + // `margin-bottom: -4px` is why the underside needs the spacer at all. + const body = [ + ...rows, + '', + '', + ...shellActions, + ].join(''); + tooltip.appendMarkdown(`
${status} ${label}  ${actions.join(' ')}
${body}
`); this.#item.tooltip = tooltip; } diff --git a/packages/vscode/src/types.ts b/packages/vscode/src/types.ts index 5f31eb6..96851a3 100644 --- a/packages/vscode/src/types.ts +++ b/packages/vscode/src/types.ts @@ -15,6 +15,17 @@ export const STACK_LABELS: Readonly> = { fmt: 'rs fmt', }; +/** + * The one place a per-stack command id is spelled. Both ends — the shell that + * registers these and the status bar that links to them — go through here, so + * a renamed verb cannot leave one side pointing at a command that no longer + * exists (a dead hover link is not a type error). + */ +export const stackCommand = ( + stack: StackId, + verb: 'restart' | 'output.focus', +): string => `rstack.${stack}.${verb}`; + /** * Per-stack state machine surfaced by the status bar hover. * @@ -94,6 +105,17 @@ export interface StackContext { * only has to reconcile its own per-folder runtimes. */ readonly onDidChangeDetection: vscode.Event; + /** + * Asks the shell to rebuild this stack from scratch — the same thing + * `rstack..restart` does. A stack cannot rebuild itself (its own + * controller is what gets replaced), and settling for a shallower local + * restart would keep the controller's already-resolved binary and version + * check, which is the staleness the rebuild exists to clear. + * + * `reason` goes to the shell log, so a restart nobody asked for out loud + * still says where it came from. + */ + readonly requestRestart: (reason: string) => Promise; } /** diff --git a/packages/vscode/tests/e2e/suite/shell.test.ts b/packages/vscode/tests/e2e/suite/shell.test.ts index 8431366..b7d64e7 100644 --- a/packages/vscode/tests/e2e/suite/shell.test.ts +++ b/packages/vscode/tests/e2e/suite/shell.test.ts @@ -1,6 +1,7 @@ import assert from 'node:assert/strict'; import * as vscode from 'vscode'; -import { delay, eventually } from './helpers'; +import type { RstackExtensionExports } from '../../../src/types'; +import { eventually } from './helpers'; const EXTENSION_ID = 'rstack.rstack'; @@ -31,39 +32,53 @@ suite('shell', () => { test('registers the shell commands', async () => { const commands = await vscode.commands.getCommands(true); for (const command of [ - 'rstack.showMenu', 'rstack.showOutput', + 'rstack.restart', 'rstack.migrateSettings', 'rstack.rslint.output.focus', + 'rstack.rslint.restart', 'rstack.rstest.output.focus', + 'rstack.rstest.restart', 'rstack.fmt.output.focus', + 'rstack.fmt.restart', ]) { assert.ok(commands.includes(command), `missing command ${command}`); } }); - test('has a status bar item whose menu opens', async () => { + test('has a status bar item whose click target works', async () => { // VS Code exposes no API to enumerate another extension's status bar items, // so the item itself cannot be asserted on directly. What *is* observable - // is its command: the item is created with `command = 'rstack.showMenu'` - // and shown unconditionally, so a `showMenu` that opens a QuickPick without - // throwing is the strongest available evidence that the always-present - // status bar item exists and is wired up. - let failure: unknown; - const menu = Promise.resolve( - vscode.commands.executeCommand('rstack.showMenu'), - ).catch((error: unknown) => { - failure = error; - }); + // is its command: the item is created with `command = 'rstack.showOutput'` + // and shown unconditionally, so a `showOutput` that reveals the channel + // without throwing is the strongest available evidence that the + // always-present status bar item exists and is wired up. + await vscode.commands.executeCommand('rstack.showOutput'); + }); + + test('rebuilds the live stacks on rstack.restart', async () => { + // The restart is a full reset: every controller is disposed and rebuilt, + // so the exports a stack publishes at registration must be a *different* + // object afterwards. That is the only externally visible proof that the + // stack was rebuilt rather than left alone (`reconcileStack` returns early + // for a stack that is already registered). + const extension = + vscode.extensions.getExtension(EXTENSION_ID); + assert.ok(extension, `${EXTENSION_ID} is not installed in the test host`); + const api = await extension.activate(); + + const before = await api.whenStackActive('rstest'); - await delay(1_000); - await vscode.commands.executeCommand('workbench.action.closeQuickOpen'); - await Promise.race([menu, delay(5_000)]); + // The command resolves only once the whole restart is done, so no polling + // is needed to observe the result. + await vscode.commands.executeCommand('rstack.restart'); - assert.equal( - failure, - undefined, - `rstack.showMenu failed: ${String(failure)}`, + const after = api.getStackExports('rstest'); + assert.ok(after, 'the Rstest stack did not come back after the restart'); + assert.notEqual( + after, + before, + 'the restart must rebuild the controller, not keep the old one', ); });