Skip to content

fix(test): match the substrate refusal separator-agnostically on win32 - #524

Merged
KnockOutEZ merged 1 commit into
studio-handoff-corefrom
sd-199-fix-test-win32-path-separator-in
Aug 29, 2026
Merged

fix(test): match the substrate refusal separator-agnostically on win32#524
KnockOutEZ merged 1 commit into
studio-handoff-corefrom
sd-199-fix-test-win32-path-separator-in

Conversation

@KnockOutEZ

@KnockOutEZ KnockOutEZ commented Aug 29, 2026

Copy link
Copy Markdown
Owner

Closes wigolo-studio-run#199.

#523's absolute-link containment arm pinned the refusal with /bin\/run/, but the message
names the offending entry through relative(destDir, child) (substrate-acquire.ts:301/308),
so the separator is the host's. On win32 that renders bin\run and the arm fails while the
refusal 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 5fc9287e are red on exactly
this assertion; every other #523 arm 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 99145066991 full test suite (windows) + 99145067043 lint + build + unit (windows-latest)),
not reconstructed:

link escapes its directory: bin\run -> C:\Users\RUNNER~1\AppData\Local\Temp\wigolo-outside-XET8Gg\payload.sh
OK   OLD /bin\/run/   vs win32 rendering -> false (expected false)   <- reproduces the CI red
OK   OLD /bin\/run/   vs posix rendering -> true  (expected true)
OK   NEW /bin[\\/]run/ vs win32 rendering -> true  (expected true)
OK   NEW /bin[\\/]run/ vs posix rendering -> true  (expected true)
OK   NEW vs unrelated escaping-link refusal (must NOT match) -> false (expected false)
OK   NEW vs refusal naming another path  (must NOT match) -> false (expected false)

ALL 6 ROWS AS EXPECTED

The last two rows are the point of the widening being safe: the arm still requires the message to
name the offending entry. bin does not occur anywhere else in either rendering, so the only
string 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, so
the regex line is never evaluated:

const escaping = null as string | null;   ->  4 failed | 67 passed (71)
                                              all four dying at :555, not at the regex

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:

error: `link escapes its directory`      ->  1 failed | 70 passed (71)
  AssertionError: expected 'link escapes its directory' to match /bin[\\/]run/
  ❯ tests/unit/studio/substrate-acquire.test.ts:560:23

Both mutants were reverse-edited back; git diff -- src/ is empty on this branch.

Separator sweep

Every string assertion in the files #523 touched, checked for a hardcoded separator:

File Verdict
tests/unit/studio/substrate-acquire.test.ts :556 was the only one — /network unreachable/, /symlink/ and the readlinkSync(...).not.toContain(frameworkDir) comparison are all separator-free or compare two same-host renderings
tests/unit/studio/auto-launch.test.ts none — /approval|confirm|prompt\(|readline/i, /spawn EACCES/
tests/unit/cli/studio.test.ts none — its 'apps/studio' and logger.js literals match source text and module specifiers, which are POSIX on every host, not joined paths

Confirms the issue's "swept: only this one".

Verification

CI deferred — quota.

Local, exit codes read directly and never through a pipe:

  • npm run build — 0
  • npx tsc --noEmit — 0
  • npm run gate:studio — 0
  • npm test12099 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 existing
    assertion rather than adding an arm.
  • tests/unit/studio/substrate-acquire.test.ts71 passed (71)

One red suite run is recorded here rather than dropped. The first npm test came back
1 failed | 12098 passed, failing tests/unit/fetch/tls-tier.test.ts"still bounds the in-flight
request 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 is
vitest's ceiling, not the test's budget — its internal timeoutMs is 10, so a 20s duration is a hang,
not slow code.
The mocked backend awaits a promise resolved only by an abort listener it attaches
at tls-tier.test.ts:638-641, guarding if (!sig) but not if (sig.aborted); when load delays entry
past 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 this
session'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 acceptance
criterion 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.

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.
@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 222fb404-c4a8-4a6a-a5fc-18f17d0d17d0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@KnockOutEZ
KnockOutEZ merged commit 6c33d69 into studio-handoff-core Aug 29, 2026
20 checks passed
@KnockOutEZ
KnockOutEZ deleted the sd-199-fix-test-win32-path-separator-in branch August 29, 2026 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant