feat: unified VS Code extension for the Rstack toolchain - #1
Conversation
One extension (rstack.rstack) replacing the standalone rstack.rslint and rstack.rstest extensions: a thin shell (activation, per-folder detection, status bar, settings migration) hosting the Rslint and Rstest stacks as near-verbatim upstream copies, plus a detection-only stub for rstack-cli formatting. - pnpm monorepo with a single package (packages/vscode) - rstack-cli powers the repo harness: rs lint --type-check, rs fmt, rs setup git hooks and rs staged on pre-commit - E2E suites ported from both upstream extensions, running a real VS Code (8 shell/detection + 8 rstest + 133 lint tests), plus 107 unit tests - CI (Linux + Windows) and a 6-target VSIX release workflow - Contributor scaffolding: AGENTS.md/CLAUDE.md, CONTRIBUTING.md, issue/PR templates
![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: 2d2193e7ba
ℹ️ 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".
Node rejects spawning .cmd shims without a shell (CVE-2024-27980 hardening), so the E2E fixture installs failed with EINVAL on the Windows CI runner.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7622087c9c
ℹ️ 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 detection E2E asserted the extensionless POSIX bin name; on Windows the probe correctly finds the rs.cmd shim, so compare without the extension. - basic/src/gitignored.ts is a test asset deliberately listed in the lint fixture's own .gitignore, which also hid it from this repo's checkout — CI never had the file. Force-track it.
On the Windows runner the language server / VS Code watcher can still hold handles inside a nested-config directory, so an immediate recursive rmSync fails with EPERM. Use Node's built-in maxRetries/retryDelay, as the suite harness already does for the profile root.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b8d267993
ℹ️ 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".
- Broken-nested-config cleanup: wait for the restored root config to take effect before deleting the nested directory — on Windows the server holds handles inside it until the refresh lands, so the delete hit EPERM even with short retries. Also widen the retry window. - Parent-ignore catalog test: VS Code's watcher can miss events for files created inside a just-created directory, leaving the nested config undiscovered forever (observed twice on the Linux runner). Nudge the config file and retry discovery, and compare the evaluation marker against a baseline instead of a literal count.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ae725ac6e
ℹ️ 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".
Four fixes to code this port owns (upstream-inherited findings are left for upstream): - migration: rstack.rslint.enable is window-scoped in the manifest, so map the legacy rslint.enable with targetScope 'window' — folder-level legacy values are now skipped as not-folder-scoped instead of being migrated into a layer where they have no effect. - rstack bridge: a native rstest.config.* now suppresses a bridged rstack.config.* project only at its own directory, per the documented 'native wins at the same root' contract — a native config in a sibling monorepo package no longer disables every bridge in the folder. - status: crash / version-mismatch reports are latched so a detection refresh cannot paint 'running' over a live failure; the latch clears when a worker actually spawns or the version check passes again. - rstack bridge: watch rstack.config.* content changes and rebuild the bridged project, so define.test() edits reach the Test Explorer without a window reload (path-only detection signatures skip these).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2994ef4748
ℹ️ 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".
Two fixes from the second Codex review round: - A lockfile-only change (e.g. pnpm install) used to produce a detection pass whose signature was unchanged, so no event fired and stacks never retried failed resolutions until a window reload. Lockfile-triggered passes now always notify, and the lint stack explicitly re-attempts roots whose last start failed (a failed slot is generation-pinned and reconcile alone never retries it). - The rstest status failure latches were single-slot: in a multi-root workspace one root's successful version check or worker spawn cleared another root's live mismatch/crash. Latches are now keyed by the resolution root that reported them and only that root's recovery clears its entry.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 738892429b
ℹ️ 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".
Third Codex review round, both on the previous round's fixes: - syncBridgeProjects kept a live bridge without re-resolving the shim, so a dependency upgrade could leave the project pinned to a pruned pnpm store path. The shim is now re-resolved on every sync; a bridge is kept (worker stays warm) only while the resolution lands on the same file, and is rebuilt when it moved or stopped resolving. - A latched crash/mismatch now dies with its root: Project.dispose forgets its cwd's entries, and dangling mismatches from failed shim resolutions (which never create a project) are cleared when the directory stops being a bridge candidate or the folder is disposed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1965f8e5e1
ℹ️ 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".
Fourth Codex review round: - A native project whose one-shot config evaluation rejected (deps not installed yet) stayed an empty tree forever. Projects now record the failure and are recreated on the next detection pass (lockfile-driven installs included). Only detection events trigger the retry, so a persistently broken config cannot loop. - Master status reports now latch under the project source URI instead of cwd: two configs in one directory no longer share a latch key, so disposing or recovering one cannot clear its sibling's live failure. This also unshadows bridge resolution latches (keyed by config dir) from bridge project disposal -- previously a sync that latched a fresh mismatch and then dropped the dead bridge wiped its own report.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9cf8bb0178
ℹ️ 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".
Fifth Codex review round: - The status bar offered Restart for a detected-but-disabled stack (enable-setting off, Restricted Mode), whose controller never registered the command; selecting it failed with command-not-found. Restart now shows only for states a registered controller produces. - A directory shipping several rstack config names (e.g. .ts next to .js mid-migration) produced one bridge project per file, but the shim probes the default names in its cwd itself, so every sibling loaded the same winning config and duplicated its tests. One bridge per directory now, keyed to the file rstack's own probe order picks.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15a751fcb8
ℹ️ 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".
Sixth Codex review round: - Version checks read package.json through nodeRequire, whose module cache pinned the first-seen version for the extension host lifetime; an in-place upgrade (npm/yarn reuse the path) kept the old verdict. readPackageVersion() now does a plain filesystem read; used by both the @rstest/core check (master) and the rstack shim check (bridge). - Restart availability now tracks controller registration explicitly (StatusBar.setActive, reported by the shell) instead of being inferred from the state kind: a crashed state can mean either a live controller whose worker died (restart valid) or a failed registration whose command was disposed with the controller (restart dead).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9da99b21f
ℹ️ 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".
Seventh Codex review round, one root cause in three places: Node caches every successful require.resolve() for the process lifetime (request + paths in Module._pathCache, symlink targets in the CJS realpath cache), so once a package resolved, re-resolving after pnpm retargets the node_modules symlink replayed the pre-upgrade store path. Failed lookups are never cached, which is why resolve-after-install always worked; this closes the upgrade/downgrade half. findPackageJsonUncached() walks node_modules up with plain fs calls and an uncached realpathSync. It now anchors: - the Rslint core location (locateCore), whose cooperating pieces already resolve from the returned path (one-resolution-root rule); - the @rstest/core worker/bin resolution, with the package entry resolved from the realpath'd package dir so the cache key is version-pinned on pnpm and the exports map stays honored (a configured rstestPackagePath keeps its explicit-pin behavior); - the rstack shim resolution, replacing the previous resolve plus node_modules containment check (the walk-up only yields node_modules candidates).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 710a0220f2
ℹ️ 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".
Eighth Codex review round: - The Yarn PnP fallback required .pnp.cjs through the module cache, so a dependency-driven retry kept resolving against the map seen by the first load. The cache entry is evicted before each load -- the PnP flavor of the resolution staleness findPackageJsonUncached avoids. - A worker that spawned successfully but exited before being closed (e.g. an invalid nodeExecArgs option) cleared the crash latch on spawn and then vanished silently: birpc was unblocked but no state was reported, leaving the status on running over an empty explorer. Unexpected exits now report crashed; deliberate teardowns are recognized either by $close having run first or by an explicit expected-exit mark set by dispose and the failed-debug-attach path, so project rebuilds and disposals do not misreport.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da2918cff9
ℹ️ 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".
Ninth Codex review round, on the previous round's fix: the expected-exit early return also skipped worker.$close(), so a disposed or rebuilt project's pending RPC calls stayed alive until timeout and the worker lingered in the tracking set. Expected exits now skip only the crash report; birpc is always closed when the process exits before $close.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6fb5deb9ff
ℹ️ 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".
Tenth Codex review round: the 100-file findFiles cap is fine for rows that are mere detection signals (the stacks rescan their own configs), but the rstack config list is consumed as-is by the test stack's bridge sync, so a monorepo with more than 100 rstack configs silently lost the overflow's bridged projects. The rstack row is now unbounded; the cap stays for the signal-only rows.
1 participant
Summary
One extension (
rstack.rstack) replacing the standalonerstack.rslintandrstack.rstestextensions: a thin shell (activation, per-folder detection, a single status bar item, settings migration) hosting the Rslint and Rstest stacks as near-verbatim upstream copies, plus a detection-only stub for rstack-cli formatting.Highlights:
packages/vscode; flatsrc/shell +src/stacks/{lint,test,fmt}rstack.*namespace, resolve-from-project, status aggregation, worker-cwd decoupling) — seepackages/vscode/AGENTS.mdrstack.config.*lights Rstest (via rstack's shipped shim) and formatting detection; the Rslint bridge is deferred pending upstream support (TODO(rstack-bridge))rs lint --type-check,rs fmt,rs setupgit hooks withrs stagedon pre-commitRelated Links
Checklist