feat(vscode): format through rs fmt --lsp on the User Node runtime - #12
Merged
Conversation
![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: 8bd2a5efc9
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 520b3bf744
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ad89d35c8
ℹ️ 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".
rstack-cli 0.5.2 ships an LSP mode for rs fmt; the fmt stack becomes a vscode-languageclient client of one rs fmt --lsp server per detected workspace folder, replacing the spawn-per-request --stdin-filepath path and its pre-spawned standby machinery. - One server per workspace folder, anchored at the folder root so the editor formats exactly like rs fmt run from that root; deepest-config anchoring is removed (docs/adr/0002-fmt-lsp-on-user-node-runtime.md). - The server runs on the User Node runtime picked by the shared preflight; nodeResolution moves from stacks/test/ to shared/, and the escape hatch becomes the resource-scoped rstack.nodeExecutable (rstest.nodeExecutable and rstack.rstest.nodeExecutable migrate to it; two pins set in one layer plan one write plus a superseded skip). - Version gate SUPPORT_MATRIX.rstack >= 0.5.2, no stdin fallback. - Config create/change/delete restarts the owning folder's server (debounced); rstack.restart also resets the host-wide Node caches so a restart behaves like a window reload for runtime selection. - E2E: run locally as VSCODE_CLI=1 pnpm test:e2e <slice> so the extension host keeps the caller's PATH (documented in AGENTS.md).
In a multi-root workspace every FmtFolderRuntime wrote to the stack's single status reporter directly, so a folder that started or recovered after a sibling failed replaced the failure with a global 'running' — the displayed state depended on event ordering. Runtimes now record their state and detail and notify the controller, which is the one writer to the shell: the folder set is folded by severity (crashed > version mismatch > disabled > pin advisory > running), so a healthy sibling never overwrites another folder's failure and a recovery clears only its own. Failure details name the folder they belong to. ADR 0002 records the fold.
…dule /simplify pass over the aggregation fix, converging with review feedback: - stacks/fmt/status.ts: foldFolderStatus() owns severity (an exhaustive rank table, so a new state cannot silently fall through to running), multi-root folder-name prefixes with tie joining, and the healthy-sibling-never-masks-a-failure invariant — now unit-tested, which the single-folder E2E fixture structurally cannot cover. - The status reports starting, not running, until a folder's server is actually up (during Node preflight and server initialization no formatting provider exists yet); stopped ranks with starting, so a config-change restart no longer flashes running. - A running folder's pin advisory folds at version-mismatch rank, above disabled — matching the test stack's configured-pin advisory instead of contradicting it. - Runtime details are folder-agnostic; the fold owns the prefixing, so the Node-preflight message is attributable in a multi-root window too. - setAdvisory() joins setState() as the only notify paths.
…ion flaps Two lifecycle holes in the per-folder fmt runtime, from review: - A server that spawned but never answers the LSP initialize request held the runtime's serialized queue inside client.start(), so a queued stop or restart never reached the process teardown — a hung shell restart and a live orphan. stop()/restart() now interrupt an in-flight start by closing the process owner outside the queue, which fails the pending initialize and lets the queue drain. The interrupt window is exact (#startInFlight spans only the client.start() await): a healthy server keeps its graceful LSP shutdown, and a manufactured failure ends in 'stopped', not a spurious 'crashed'. - A folder that lost and regained detection across two passes could run two servers at once: reconcile deleted the map entry and stopped the runtime asynchronously, and the replacement spawned immediately. The controller now reserves the folder (#retiring, identity-guarded) and the replacement start awaits the predecessor's retirement inside its own queue, so a config event during the wait lines up behind it instead of spawning early. dispose() drains retiring runtimes too.
…ailures Review round three, plus the CI diagnosis groundwork: - runRestart resets the host-scoped User Node preflight memo only when no consumer stack (rstest, fmt) survives the retire wave. A single-stack rstack.fmt.restart next to a live Rstest controller kept clearing the decision its existing workers were built on, so the next worker spawn could silently re-probe onto a different runtime. The full rstack.restart — the 'like a window reload' gesture — still always clears it, as does the batched restart a rstack.nodeExecutable change triggers. Two unit tests pin both sides. - Nested workspace folders (parent and subdirectory both detected for fmt) are recorded as a documented limitation in AGENTS.md and ADR 0002: the supported shape is sibling folders, and per-document routing was considered and deferred. - The rstest e2e suite names each failing test via console.error: the extension host's stdout (mocha's reporter) is not forwarded to CI logs, so a CI-only failure was previously unidentifiable.
A CI-only E2E failure on windows-latest (every rstest suite discovering zero tests since @rstest/core 0.11.7 / @rspack/core 2.1.10 floated in) cannot be diagnosed from the CI log: worker stdout/stderr land only in the output channel, which CI cannot open. Gate a console.error mirror of every log entry behind RSTACK_E2E_MIRROR_LOGS=1 and set it from the rstest E2E harness in CI, so the next failing run names the actual error.
fi3ework
force-pushed
the
feat-vscode-fmt-lsp
branch
from
August 13, 2026 14:15
52a8c9f to
564f76f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 564f76fdd1
ℹ️ 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".
Raising SUPPORT_MATRIX.rstack to >=0.5.2 for rs fmt --lsp also gates the Rstest bridge, which checks the same entry — a project on rstack 0.3.5-0.5.1 reports version mismatch for tests too. Review flagged the side effect; keeping one toolchain-wide floor is the deliberate answer (per-stack rstack floors considered and rejected), so state it in the matrix doc and ADR 0002 instead of splitting the entry.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a4e2d52bf
ℹ️ 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 folder whose runtime had failed (disabled, version mismatch, crashed) was kept as-is by every reconcile, so the install or upgrade that fixed it was never picked up — formatting stayed dead until a manual restart, while the test and lint stacks both retry on the same detection pass. Restart such a runtime in place, on the path a config change already uses, which re-runs package resolution, the version check and the Node preflight. Healthy and starting runtimes stay untouched. The one recovery no watcher sees — an install that changes no lockfile — now has its way out written into the disabled status message, which names the restart command.
The rstack.rstest.nodeExecutable -> rstack.nodeExecutable mapping covered a rename that happened before the extension was ever published, so no settings file can hold the legacy key. Remove it together with the superseded-skip and collision machinery that existed only for the two-sources-one-target case, and the prompt clause naming the phantom source. Record the two policies this acts on in AGENTS.md: pre-1.0.0 the extension breaks freely (only the latest released tools need support), and the three tools are treated uniformly by default, diverging only when a tool forces it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e403423ed8
ℹ️ 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".
The listener decided per stack between two paths: a gate change went to the reconcile, a declared restartOnSettings change to a targeted restart, and a stack in both sets was left to the reconcile. That split swallowed the restart when one save wrote a gate key at its already-effective value alongside a shared setting — the reconcile saw a live controller behind a still-open gate and kept it, so the stack stayed on the previous Node executable. Settings edits are rare, so selectivity bought nothing but that hole: any relevant key now triggers one full restart pass, which re-evaluates every gate and rebuilds every controller — a gate flip in either direction, a moved shared setting, or both in one save are handled by construction. The dead array arm of restart()/runRestart() and the unused reconcile() parameter go with it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0720c78f86
ℹ️ 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".
The Generated-shim and Bridged-folder entries describe the lint bridge, which is not on this branch — they move to the branch that introduces it. Config root claimed the workspace folder root for every tool, but the test stack keeps upstream's per-project cwd rule (adaptation 5); scope the anchor to the fmt server.
This was referenced Aug 18, 2026
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.
Stacked on #10 — review only the top commit; retarget to
mainonce #10 merges.What
rstack-cli 0.5.2 ships
rs fmt --lsp. The fmt stack becomes avscode-languageclientclient of oners fmt --lspserver per detected workspace folder, replacing the spawn-per-request--stdin-filepathpath and its pre-spawned standby machinery (run.ts/standby.tsdeleted).Key decisions (details in
docs/adr/0002-fmt-lsp-on-user-node-runtime.md)rs fmtloads one config from its cwd with no upward walk, so the editor now formats exactly likers fmtrun from the folder root; deepest-config-wins anchoring is removed. A subproject needing its own fmt config becomes its own workspace folder.shared/nodeResolution.ts, moved fromstacks/test/), same floor and same escape hatch as the test worker: the new resource-scopedrstack.nodeExecutable.rstest.nodeExecutableandrstack.rstest.nodeExecutablemigrate to it; when both are set in one layer the plan writes the newer key's value and lists the older one as asupersededskip.SUPPORT_MATRIX.rstack >= 0.5.2; older rstack reportsversion mismatchinstead of a stdin fallback.rstack.config.*only — the server caches its config for its process lifetime).rstack.restartalso resets the host-wide Node caches, so a restart clears runtime selection like a window reload.documentFormattingProvidercapability; crash/recovery is reported through the shell status bar (crashedon stop, back torunningon recovery).Docs
packages/vscode/AGENTS.md: fmt gotchas rewritten; adaptation perf(vscode): pre-spawned rs fmt standby for the active editor #6 now covers test + fmt; local E2E must run asVSCODE_CLI=1 pnpm test:e2e <slice>(without it, VS Code overwrites the extension host PATH with a login-shell snapshot and the Node preflight correctly refuses).Test plan
pnpm lint(rs lint --type-check): 0 errors, 0 warningspnpm test:unit: 228/228 passedVSCODE_CLI=1 pnpm test:e2e vscode: exit 0 (fmt suite: provider formatting without touching the workspace, per-folder coverage, one server for the detected folder only)