fix(test): match the substrate refusal separator-agnostically on win32 - #524
Merged
KnockOutEZ merged 1 commit intoAug 29, 2026
Merged
Conversation
The absolute-link containment arm pinned the refusal with /bin\/run/, but the message names the offending entry via relative(), so the separator is the host's: bin/run on POSIX, bin\run on win32. Both Windows jobs at public tip 5fc9287 were red on this assertion while the refusal it exists to pin had fired correctly; every other arm passed on win32. Match either separator. The arm still requires the message to name the entry, so no kill is lost: stripping the entry from the refusal reds this assertion and nothing else.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
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.
Closes wigolo-studio-run#199.
#523's absolute-link containment arm pinned the refusal with/bin\/run/, but the messagenames the offending entry through
relative(destDir, child)(substrate-acquire.ts:301/308),so the separator is the host's. On win32 that renders
bin\runand the arm fails while therefusal it exists to pin has fired correctly — the containment walk found the escape, refused the
install, and left nothing behind. Both Windows jobs at public tip
5fc9287eare red on exactlythis assertion; every other
#523arm ran and passed on win32.One assertion, matched separator-agnostically. No product-code change — the behaviour is
correct on both platforms, which is the whole point of the finding.
Demo — the regex against both rendered strings
The win32 string is copied verbatim from the CI annotation on run
33269410846(jobs
99145066991full test suite (windows)+99145067043lint + build + unit (windows-latest)),not reconstructed:
The last two rows are the point of the widening being safe: the arm still requires the message to
name the offending entry.
bindoes not occur anywhere else in either rendering, so the onlystring the relaxation newly admits is the win32 spelling of the one it already admitted.
The mutation that actually kills this assertion
Deleting the containment guard is not the mutation that proves this arm — it reds the
earlier clause (
expect(r.outcome).toBe('failed'), line 555) in all four containment arms, sothe regex line is never evaluated:
The mutation aimed at what this assertion pins is the message — strip the entry name from the
refusal and exactly one test reds, at exactly this line:
Both mutants were reverse-edited back;
git diff -- src/is empty on this branch.Separator sweep
Every string assertion in the files
#523touched, checked for a hardcoded separator:tests/unit/studio/substrate-acquire.test.ts:556was the only one —/network unreachable/,/symlink/and thereadlinkSync(...).not.toContain(frameworkDir)comparison are all separator-free or compare two same-host renderingstests/unit/studio/auto-launch.test.ts/approval|confirm|prompt\(|readline/i,/spawn EACCES/tests/unit/cli/studio.test.ts'apps/studio'andlogger.jsliterals match source text and module specifiers, which are POSIX on every host, not joined pathsConfirms the issue's "swept: only this one".
Verification
CI deferred — quota.
Local, exit codes read directly and never through a pipe:
npm run build— 0npx tsc --noEmit— 0npm run gate:studio— 0npm test—12099 passed | 20 skipped | 7 todo (12126), 955 files passed / 5 skipped, exit 0.Identical to the baseline recorded for
#197, which is the expected result: this edits an existingassertion rather than adding an arm.
tests/unit/studio/substrate-acquire.test.ts—71 passed (71)One red suite run is recorded here rather than dropped. The first
npm testcame back1 failed | 12098 passed, failingtests/unit/fetch/tls-tier.test.ts— "still bounds the in-flightrequest by the INTERNAL timeout when the caller signal stays live" — with
Test timed out in 20000ms.That run was contended:
gate:studio's lint and four typechecks were on the same cores. The 20s isvitest's ceiling, not the test's budget — its internal
timeoutMsis 10, so a 20s duration is a hang,not slow code. The mocked backend awaits a promise resolved only by an
abortlistener it attachesat
tls-tier.test.ts:638-641, guardingif (!sig)but notif (sig.aborted); when load delays entrypast the 10ms timer the signal is already aborted, the listener never fires, and the await deadlocks.
Confirmed by probe rather than inferred: backend entered at 0ms resolves, at 50ms it never does. The
file passes alone (
85 passed (85)) and the uncontended full suite above is green, so it is thissession's own concurrency exposing a latent test bug — not this change, which touches one regex in an
unrelated subsystem.
tests/unit/fetch/**is outside this issue's territory and no acceptancecriterion was blocked, so per the brief it is one line in
known-issues.md, not a fix here.Territory:
tests/unit/studio/**only, one file, one assertion.