feat: per-stack restart commands and a hover-only status bar - #4
Merged
Conversation
Restart is now a shell concern with two levels: rstack.restart ("Relaunch
Extension") tears down every controller, re-runs detection and registers
again from scratch; the new rstack.<stack>.restart commands do the same
for a single stack. The Rslint stack's own LSP-only restart is removed —
bouncing just the server kept the controller's stale package resolution
and version check, which is exactly what these commands exist to clear.
Its settings listener now routes through the shell command instead.
The status bar QuickPick is gone: the hover card is the single surface.
It renders all six rows in one table (aligned state/label/action columns),
colours state icons with theme variables, moves state text into the icon's
native tooltip, and stacks the three global actions under a divider.
Clicking the item opens the extension log directly.
Unit tests cover the per-stack restart scope and the manifest contract
(rstack.restart stays palette-unconditional; per-stack restarts gate on
their context keys).
Three follow-ups from a cleanup pass over the restart work: - `stackCommand(stack, verb)` in types.ts is now the single place a per-stack command id is spelled. The shell registers through it and the status bar links through it, so the two OUTPUT_COMMANDS/RESTART_COMMANDS tables are gone — they had become hand-copied templates whose drift from the registration site would not be a type error. - `StackContext.requestRestart(reason)` replaces the Rslint stack reaching back through the command registry. Restart was already a shell service; it is now injected like every other one, and the reason reaches the log so a settings-triggered restart says what moved. - `dispose()` tears controllers down in parallel, matching the restart path, and drops `clearProjectModuleCache` which had no callers. Also folds the status bar's two anchor builders into one so escaping is a property of the markup rather than a per-call-site obligation, inlines `#canRestart`, and trims the rejected-alternative journal from render().
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0779bf26ac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A restart re-resolves binaries and package versions and respawns every tool process, but the config-loader and eslint-plugin modules imported from the project stay in Node's ESM registry for the lifetime of the window. Verified: clearing the local memo in projectModules.ts hands back the identical module object, and a cache-busting query reloads only the entry module — relative specifiers inside it do not inherit the query, so the result is a fresh entry over stale dependencies. Records the limit in the README and the reasoning in AGENTS.md so nobody adds an invalidation hook that cannot work.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f20df29672
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
`retire` drops a controller from the shell's map 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. `dispose()` walked that map, found nothing, and went on to dispose the channels out from under the running teardown — letting deactivate() resolve while the child processes were still alive. Putting the teardown pass on the same queue as reconciles and restarts makes "whatever was in flight has finished" something dispose can wait for; the `#disposed` flag it sets first still stops that pass from rebuilding anything on its way out. The regression test asserts on the output channels rather than on deactivate's promise: "has not resolved yet" races the microtask queue, whereas "the channel this teardown still logs to is alive" is a fact. Confirmed to fail against the previous dispose().
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e817a867a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
States, per config source, what the extension supports today and what is still coming, plus the cross-cutting items (re-detection command, bounded version ranges, retiring the standalone extensions).
`registerCommands()` runs before activation's first await, so the palette can reach a restart while activation is still bringing stacks up. The activation reconcile called `reconcile()` directly rather than through `enqueue`, so that restart ran straight through it and could retire a controller whose `register()` had not returned — the retired controller then went on to publish its exports and set `active` to true. Every pass now rides the one queue, which is what AGENTS.md already says. The regression test blocks the Rslint controller's `register()`, invokes the restart, and asserts no teardown begins for a stack whose register is still in flight. Confirmed to fail against the direct call.
The two concurrency fixes in this PR both came from a pass that bypassed the shell's single queue, so encode the rule instead of restating it: - `reconcile()` is now the queued wrapper and `runReconcile()` the body, matching the existing `restart()`/`runRestart()` pair. The `run*` prefix marks "already owns the queue" at every call site. - `retireAll()` replaces the two hand-rolled `Promise.allSettled` loops in `runRestart` and `dispose`. - `dispose()` drops the detection service before waiting on the queue, so a file touched during shutdown cannot arm a fresh pass behind it. The reconcileStack comments still described the pre-fix model where a teardown could run beside a reconcile; they now describe what actually happens. On the test side: drop the dead `blockDetection` field, rebuild harness state from a factory so a new field cannot leak between tests, and use `Promise.withResolvers`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two things that turned out to be the same thing: a way to rebuild a stack without reloading the window, and a status bar that has somewhere to put it.
Restart
Detection re-runs on config and lockfile changes, but a reinstall can leave the lockfile untouched and a
node_modulescan end up broken with every watched file intact.reconcileStackdeliberately leaves an already-registered stack alone, so a stack that is up but wedged was never rebuilt — only a window reload recovered from that.rstack.restart("Relaunch Extension") — disposes every controller, re-runs detection, registers every stack that still passes the gate. Deliberately not gated in the command palette: it is the action for "nothing is active".rstack.<stack>.restart— the same for one stack. All three now have one.binPath/customBinPath/trace.serverlistener now asks the shell for a full rebuild through the newStackContext.requestRestart(reason), and the reason reaches the log.Status bar
The item had both a hover card and a click-through QuickPick showing the same per-stack rows from two code paths. The QuickPick is gone; the hover is the single surface.
testing.iconPassed,editorWarning.foreground,disabledForeground, …) rather than hard-coded hexes, and the state text moved into the icon's native tooltip.Command ids are no longer written twice:
stackCommand(stack, verb)is the one place arstack.<stack>.*id is spelled, used by both the registration loop and the hover.Testing
pnpm lint(includes type-check) — 0 errorspnpm test:unit— 125 passing, including new coverage for the per-stack restart scope, failure isolation across stacks, and the manifest contract (rstack.restartstays palette-unconditional; per-stack restarts gate on their context keys)pnpm test:e2e:vscode— 9 passingpnpm test:e2e:lint— all six suites passingNotes
rstack.showMenuis removed.rstack.rslint.restartkeeps its id but now means "rebuild the stack" rather than "bounce the server".