test(vscode): cover the rstack config bridge end to end - #9
Merged
Merged
Conversation
- Add e2e/rstest/suite/bridge.test.ts: a folder whose only test signal is rstack.config.ts gets a bridged project - explorer tree (node-less layout + AST test cases) and a real run through rstack's shipped shim. - Bump the rstack fixture to rstack@0.5.0-alpha.1 (newest published); move its public-hoist settings from the fixture .npmrc (no longer read by pnpm 11) to shared install flags in setupFixtures.mjs. - Replace the enumerated test:e2e:* script chain with e2e/run.mjs: a SLICES table declares each slice's fixtures and entry, package.json scripts become thin forwards, and tsc compiles first to fail fast. - Record the tools-and-configs vocabulary (atomic tool, Rstack config, shim, bridged project, ownership) in CONTEXT.md.
![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: b00aaf1bbd
ℹ️ 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".
With shell: true Node concatenates command and args unescaped, so spawning process.execPath or an entry path containing spaces through the shared option would break argument boundaries on Windows. Only the pnpm .cmd shim needs a shell (CVE-2024-27980 hardening); node spawns now run shell-less. Raised by PR review.
fi3ework
added a commit
that referenced
this pull request
Aug 25, 2026
The master hard-coded FORCE_COLOR=1 into the rstest worker env (as the upstream extension does), so a project whose config sets process.env.NO_COLOR = '1' at load time hit Node's "'NO_COLOR' env is ignored" warning in every pool process. Mirror the CLI's getForceColorEnv semantics instead (adaptation #9): the master injects FORCE_COLOR=1 into the composed spawn env only when the user set neither color standard (marking the injection), and the worker retracts the marked injection right after config load — the CLI's own decision point — when the config turned color off. A user-set FORCE_COLOR beside a config-set NO_COLOR still warns, exactly as the bare CLI does. Also apply the review cleanups from the same pass: versionOk now retires a root's not-installed latch too (dropping the paired installed() calls), versionMismatch gains the same re-raise dedupe as notInstalled, the missing-package verdict rides on RslintResolutionError instead of a partial code table, missingDependencyCauseOf moves to shared/ for the lint/fmt config loaders (#30), stackCommandTitle loses its single-value verb parameter and now also feeds the status-bar hover, the unused NormalizedConfigResult.reason discriminant is dropped, the lint core-selection failure wording is spelled once, and the worker spawn reuses one resolved rstest path and one IPC send callback.
fi3ework
added a commit
that referenced
this pull request
Aug 25, 2026
) * fix(vscode): stop forcing color when the project config disables it The master hard-coded FORCE_COLOR=1 into the rstest worker env (as the upstream extension does), so a project whose config sets process.env.NO_COLOR = '1' at load time hit Node's "'NO_COLOR' env is ignored" warning in every pool process. Mirror the CLI's getForceColorEnv semantics instead (adaptation #9): the master injects FORCE_COLOR=1 into the composed spawn env only when the user set neither color standard (marking the injection), and the worker retracts the marked injection right after config load — the CLI's own decision point — when the config turned color off. A user-set FORCE_COLOR beside a config-set NO_COLOR still warns, exactly as the bare CLI does. Also apply the review cleanups from the same pass: versionOk now retires a root's not-installed latch too (dropping the paired installed() calls), versionMismatch gains the same re-raise dedupe as notInstalled, the missing-package verdict rides on RslintResolutionError instead of a partial code table, missingDependencyCauseOf moves to shared/ for the lint/fmt config loaders (#30), stackCommandTitle loses its single-value verb parameter and now also feeds the status-bar hover, the unused NormalizedConfigResult.reason discriminant is dropped, the lint core-selection failure wording is spelled once, and the worker spawn reuses one resolved rstest path and one IPC send callback. * fix(vscode): address review findings on quiet classification, latches, color marker Three review findings, each verified before fixing: - The missing-cwd spawn refusal threw a plain Error, so callers re-logged the already-warned stale-project state as an error with a stack. The guard now throws ReportedRstestResolutionError (which gained an optional message), and the four catch sites above RstestApi share one logUnlessReported helper next to the class instead of re-deciding. - The re-raise dedupe in versionMismatch/notInstalled short-circuited before the package-state restatement, so a crash latched between two identical verdicts survived a retry that aborted before spawning. Both observations now fold into one #observePackageState that restates first and skips only the repaint. - retractForceColorIfDisabled left RSTACK_FORCE_COLOR_INJECTED in the env on the no-NO_COLOR path; the marker is now removed once the decision is complete, so pool processes and user test code never observe it. * fix(vscode): keep the spawn-race cwd classification quiet through pending RPCs The delete-after-check handler logged the stale-project warning but closed the worker with birpc's default error, so pending RPCs rejected with a bare '[birpc] rpc is closed' and the callers' catches re-logged the failure. birpc's $close(customError) rejects pending calls with the given error; the cwd-gone branch now passes a ReportedRstestResolutionError carrying the same message, so logUnlessReported stays quiet — the same contract the pre-spawn guard already follows. The branch's race window (cwd deleted between the guard and spawn, with no 'spawn' event timing guarantee) has no deterministic unit test; the classification is covered by types and the existing spawn-failure suite. * fix(vscode): retract only the injected FORCE_COLOR value A config that assigns both FORCE_COLOR and NO_COLOR at load time owns the FORCE_COLOR value; the bare CLI, deciding after config load, leaves both intact. The retraction now removes FORCE_COLOR only while it still holds the injected '1', so pools keep a config-set force-color preference.
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.
Summary
The rstack config bridge (a folder whose only test signal is
rstack.config.*drives Rstest through rstack's shipped shim) was implemented but had no E2E coverage beyond detection. This PR locks it in end to end, against the newest published rstack:e2e/rstest/suite/bridge.test.ts— adds the sharedrstackfixture as a second workspace folder and asserts the bridged project's explorer tree (node-less layout, AST-collected test cases) and a real run through the shim (1 passed), restoring the single-folder state for the suites that follow.e2e/fixtures/rstacknow pinsrstack@0.5.0-alpha.1(newest published; npmlatestis 0.4.0). The version gate already passes prereleases inside a supported range by policy, so no source change. The fixture's.npmrcpublic-hoist settings moved into shared install flags insetupFixtures.mjs— pnpm 11 no longer readspublic-hoist-patternfrom a project-local.npmrc.test:e2e:*script chain is replaced bye2e/run.mjs: aSLICEStable declares each slice's fixtures and entry, package.json scripts become thin forwards (pnpm test:e2e [slice ...]),tsccompiles first to fail fast. CI'spnpm run test:e2eis unaffected.Extension source (
src/) is unchanged.Verified:
pnpm lint,pnpm test:unit, and the full E2E chain (smoke, vscode, rstest, lint) locally green.Related Links
Checklist