feat(vscode): lint bridged folders from rstack.config via a generated shim - #10
feat(vscode): lint bridged folders from rstack.config via a generated shim#10fi3ework wants to merge 9 commits into
Conversation
… shim A workspace folder with no native rslint.config.* and an rstack.config.* at its root now lints through the Rstack config: the extension writes a generated shim (loadRstackConfig with the absolute config path baked in, lint section default-exported) and pins the language server to it through the optional configPath of rslint/configRefresh (config-discovery protocol 2, rslint PR web-infra-dev/rslint#1630). Bridged mode is gated on the capability, not a version guess: the project's @rslint/core/config-loader must report protocol >= 2. Below that (every released version today) the folder reports a version mismatch naming the resolved version instead of starting a half-bridge, and never reports crashed - @rslint/core reaches such projects only as rstack's transitive dependency. Ownership stays folder-level: one native config anywhere and the bridge yields; a mode flip restarts the server through the coordinator's replacement path, since the explicit-config choice is fixed for a server process. The E2E bridge suite asserts the gate path against today's @rslint/core 0.7.3 and carries the explicit-mode happy path behind a runtime skip that unlocks when a protocol-2 release ships.
a54f01e to
678fd84
Compare
Distilled from the rationale that shipped inline with the bridge: the generated shim, folder-level Ownership, the capability gate and the version-mismatch-not-crashed rule now have a decision record, following the precedent ADR 0002 set for fmt. The AGENTS.md gotchas keep their operational rules and point at the ADR for the why; ADR 0001's lint entry cross-references it. Protocol 2 is recorded as first released in @rslint/core 0.8.0 (verified against the npm package).
@rslint/core 0.8.0 is the first release carrying rslint#1630
(config-discovery protocol 2), and rstack 0.6.1 depends on it. Bumping
the shared rstack fixture to rstack 0.6.1 flips the bridge E2E suite's
runtime switch: the explicit-mode happy path now runs for real and the
capability-gate test skips — the same switch, other direction; the gate
stays covered as a pure unit.
The two native install roots move to ^0.8.0 as well, so the ported
Rslint suites exercise the protocol {1, 2} client set against the
release that actually speaks 2.
![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: ed49e119f5
ℹ️ 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".
Prompted by PR #10 review: the bridge crashed a PnP project with a misleading missing-rstack error after resolving @rslint/core through .pnp.cjs. The PnP lookup covered only that first hop — config evaluation, the plugin host and the fmt/test stacks all resolve without PnP hooks — so it could never light a working folder, and upstream's own resolver has since gone 'no PnP or fallback'. Remove the hop instead of extending it: resolution walks physical node_modules only, and a PnP project surfaces the ordinary resolution failure, whose message now names the layout as the blocker (a .pnp.* probe kept as diagnostic only). For a bridged folder that reports through the existing toolchain gate as version mismatch. Also from the same review round: the bridged-lint README requirement now names @rslint/core >= 0.8.0 as the protocol-2 release instead of claiming no release has it, and the settings surfaces stop advertising PnP support. Decision recorded in ADR 0003; gotcha in AGENTS.md.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16f00946a0
ℹ️ 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".
Pre-1.0 the floor follows what is actually tested: the E2E fixtures all install 0.8.0 now, and 0.8.0 is the first release speaking config-discovery protocol 2. The resolution error messages derive the range from SUPPORT_MATRIX instead of hardcoding it, so the two can no longer drift.
All four align behavior with contracts the bridge already documents: - A dependency event coalesced with a config edit inside the watcher debounce no longer loses shim re-materialization: the bridge keeps its own pending flag, accumulated per event and consumed by the refresh that fires (the debounce keeps only the last event's reason). The flag is lifecycle-scoped alongside pluginDependencyRevision. - The bridged toolchain gate now wraps only the core-not-found failure (new RslintCoreNotFoundError): a resolvable @rslint/core whose later pieces fail (platform binary, exports, custom binary path) keeps its ordinary classification instead of a misleading 'install @rslint/core' version mismatch. - rstack-loader prerequisites (rstack missing, or predating the ./config export) are RstackBridgeGateError now, so they report version mismatch as the bridge documentation promises; an unusable ./config shape stays a crash — the install is not old, 'upgrade' would mislead. - A bridged folder leaving detection entirely (root rstack.config.* deleted, no native start to clean up) removes its generated shim on the prune path; dispose deliberately keeps it.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db789cbcb9
ℹ️ 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 probe behind the core-not-found message checked only the workspace folder root for .pnp.cjs/.pnp.js, but the package lookup it diagnoses walks ancestors — and in a PnP monorepo the manifest sits at the repository root, not in the subpackage opened as the folder. Such a folder got the generic 'install @rslint/core' remedy, which under PnP creates no physical node_modules and fixes nothing. The probe now walks the same ancestor chain, so the message names the unsupported layout.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eed2a719b8
ℹ️ 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 shim can vanish with no watcher event at all — node_modules events are swallowed by the default files.watcherExclude, and a frozen-lockfile reinstall or a cache wipe rewrites no lockfile — so keying its re-materialization on dependency-change events left the pin pointing at a missing file until a manual restart. Every debounced refresh now re-materializes the shim right before the server is told to reload (no-churn write: the intact case stays a pure read), which also covers the initial refresh a server restart replays; the accumulated bridgeShimRefreshPending flag and its lifecycle resets go away entirely. The shim-rewrite failure note clears itself on the first refresh that succeeds, instead of outliving the condition it describes. Also, an unreadable rstack manifest now throws the ordinary bridge error instead of joining the no-./config prerequisite gate: a manifest that cannot be parsed is a broken install, not an old one, and 'upgrade rstack' would mislead. Docs updated where they contradicted the above (the config watcher does not normally see shim writes; the no-churn rule is what prevents a refresh loop, not watcher ordering).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7072cf849
ℹ️ 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".
pickImportTarget walked a fixed preference list (import, node, module,
default) over a conditional ./config export, but Node walks the
condition map in declaration order and takes the first enabled key —
{ default: A, import: B } resolves to A, and module is a bundler-only
condition Node never enables (both verified empirically against Node).
The probe now iterates the entry's own keys in order, matching only the
conditions Node enables for the shim's ESM import ({node, import,
default}), so the loader baked into the generated shim is the file an
import of rstack/config would actually execute. Two unit tests pin the
declaration-order and module-skipping rules.
Also stop promising a version-mismatch status for the host-runtime
type-stripping condition: the README listed it beside the three package
prerequisites under one 'shows version mismatch' sentence, but the
preflight only warns and a config that fails to load stays a crash by
design. The README now separates the editor-owned condition, and ADR
0003's consequence states the classification.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fce8a2fc93
ℹ️ 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".
| // Every refresh retries, so a past failure is cleared the moment one | ||
| // succeeds — the note must not outlive the condition it describes. | ||
| this.removeStatusNote(SHIM_REFRESH_FAILURE_NOTE); | ||
| } catch (error) { |
There was a problem hiding this comment.
When a lockfile or config refresh runs after rstack is removed or downgraded below the ./config export, materializeShim() throws RstackBridgeGateError, but this catch converts it into a nonfatal note and the request proceeds with the stale or missing shim. The folder consequently remains running instead of reporting the required version mismatch until a manual restart; preserve the gate classification when re-materialization fails.
AGENTS.md reference: packages/vscode/AGENTS.md:L39-L39
Useful? React with 👍 / 👎.
| } | ||
| } | ||
|
|
||
| private removeStatusNote(note: string): void { |
There was a problem hiding this comment.
This helper was added solely for the bridge refresh path—its only caller removes SHIM_REFRESH_FAILURE_NOTE—but it sits outside the required bridge marker pairs. Because upstream synchronization identifies adaptations through those paired regions, a sync can miss or overwrite this bridge-only divergence; wrap the method in a marker pair like the other bridge wiring.
AGENTS.md reference: packages/vscode/AGENTS.md:L18-L18
Useful? React with 👍 / 👎.
1 participant
What
A workspace folder whose only lint signal is a root
rstack.config.*now lints through the Rstack config. The extension writes a generated shim into the project (node_modules/.cache/rstack-editor/rslint.config.mjs) that loads the config through rstack's publishedrstack/configexport with the absolute config path baked in, and pins that folder's language server to the shim through the optionalconfigPathofrslint/configRefresh(config-discovery protocol 2, added by web-infra-dev/rslint#1630, first released in@rslint/core0.8.0).rslint.config.*anywhere in the folder and the bridge yields entirely. Only a folder-rootrstack.config.*bridges (the explicit-config choice is fixed per server process, one server per folder, cwd = folder root). A mode flip restarts the server through the coordinator's existing replacement path.@rslint/core/config-loaderto report protocol >= 2. Below that the folder reports a version mismatch naming the resolved version — nevercrashed, since@rslint/corereaches such projects only as rstack's transitive dependency.configPathkey is simply absent and the server keeps doing its own discovery. Every bridge line in the portedRslint.tssits inside paired--- rstack config bridge ---markers (adaptation refactor(vscode): split tests out of src into sibling tests/ and e2e/ trees #7 in AGENTS.md).docs/adr/0003-lint-rstack-bridge.md(generated shim, folder-level Ownership, capability gate, mismatch-not-crashed — following the ADR 0002 precedent); the AGENTS.md gotchas keep the operational rules and point at it.No longer a draft
When this PR was opened, no released
@rslint/corespoke protocol 2 (latest was 0.7.3), so the E2E bridge suite asserted the gate path and carried the explicit-mode happy path behind a runtime skip.@rslint/core0.8.0 now ships protocol 2 (verified against the npm package:CONFIG_DISCOVERY_PROTOCOL_VERSION = 2), andrstack0.6.1 depends on~0.8.0— so the fixtures were bumped (rstackfixture → rstack 0.6.1; both@rslint/coreinstall roots →^0.8.0) and the suite's runtime switch flipped: the happy path now runs for real (diagnostics from a rule that exists only insiderstack.config.ts, plus a config-change refresh), and the gate test skips on the same switch. The gate itself stays covered as pure units.Test plan
pnpm lint— 0 errors, 0 type errorspnpm test:unit— all green (bridge rule/shim/gate covered as pure units)@rslint/core0.8.0 /rstack0.6.1, and the nine ported native suites run against 0.8.0, exercising the client's protocol{1, 2}set on the release that actually speaks 2.