Skip to content

Version range semantics, PM-override install, self-diagnosing errors, doctor resilience, Volta shims#45

Merged
kjanat merged 9 commits into
masterfrom
fix/version-ranges-pm-override
Jun 12, 2026
Merged

Version range semantics, PM-override install, self-diagnosing errors, doctor resilience, Volta shims#45
kjanat merged 9 commits into
masterfrom
fix/version-ranges-pm-override

Conversation

@kjanat

@kjanat kjanat commented Jun 11, 2026

Copy link
Copy Markdown
Owner

Bug cluster surfaced by dreamcli CI run 27305763214: a bogus node-version warning, an unwanted deno.lock committed by autofix-ci, and an unreadable RUNNER_PM error that also killed doctor. One commit per fix.

Fixes

  1. version_matches real range semantics (3521f4b) — was operator-strip + prefix match, so >=22.22.2 behaved as =22.22.2 and warned on Node 22.22.3/25.9.0. Node-semver grammar (space-AND, || unions, hyphen ranges, x wildcards, detached operators, v prefixes) is normalized into the existing semver dependency. Bare versions (.nvmrc 20.11) keep prefix-at-segment-boundary semantics; unevaluable input (lts/*) falls back to the old prefix match.
  2. Self-diagnosing override errors (a4fb1d7) — unknown --pm/RUNNER_PM/--runner/RUNNER_RUNNER values now name the carrying source, escape control chars, truncate past 60 chars, and hint the PowerShell unquoted-assignment footgun on multi-line values ($env:RUNNER_PM=deno executes deno and captures its REPL banner).
  3. runner install honors --pm/RUNNER_PM (d53bb86) — the override now selects the install set instead of being ignored (previously every detected PM installed; bun+deno repos always ran deno install and wrote deno.lock). An override naming an undetected PM refuses with new ResolveError::PmOverrideNotDetected (exit 2). runner.toml [pm].node/[pm].python deliberately keep scoping script dispatch only (test-pinned non-goal).
  4. doctor survives bad env (2f319f0) — an unparseable RUNNER_* value no longer kills the command whose job is diagnosing a broken environment. Dispatch retries doctor through a lenient constructor; invalid env values degrade to env: warnings on the report (additive in the JSON warnings array, no schema bump). Other commands and explicit CLI flags stay strict.
  5. Volta shim resolution in doctor (806e197) — PATH-probe hits classified as Volta shims (canonicalized parent-dir equality against the volta bin dir / $VOLTA_HOME/bin) resolve through volta which run in the project root, so --dir targets report their own pins: npm=C:\Program Files\Volta\npm.EXE -> ...\image\npm\11.12.1\bin\npm.cmd (volta), or (volta shim, not provisioned). Additive signals.node.volta_shims JSON field; display only — execution still spawns the shim. Probe-fallback filtering of phantom shims deferred to a follow-up.

Verification

  • ~50 new unit/integration tests, incl. spawned-binary tests with per-child env injection (tests/doctor_env.rs); clippy clean, no lint suppressions added.
  • Live: dreamcli dashboard now shows >=22.22.2 ... current 24.14.1 (ok); --pm npm install in a bun+cargo repo exits 2 naming the source and detected set; doctor with a multi-line garbage RUNNER_PM runs and reports it; --dir differential test confirms Volta resolution honors the target project pin (11.12.1 vs 11.6.2).
  • Note: 13 pre-existing unit tests fail on dev boxes with PMs installed (real PATH leaks into probe/dispatch test expectations); verified pre-existing via stash-test, pass in CI.

kjanat added 5 commits June 11, 2026 02:13
`version_matches` stripped range operators and prefix-matched, so
`>=22.22.2` behaved as `=22.22.2` and warned on Node 22.22.3/25.9.0.
Normalize node-semver grammar (space-AND, `||` unions, hyphen ranges,
detached operators, `v` prefixes) into the comma grammar the existing
`semver` dependency parses, and delegate evaluation to it. Bare
versions keep prefix-at-segment-boundary semantics (a `.nvmrc` `20.11`
means 20.11.x, narrower than the crate's caret default); unevaluable
input falls back to the historical prefix match.
Unknown --pm/RUNNER_PM (and --runner/RUNNER_RUNNER) values used to be
Debug-dumped verbatim: a PowerShell unquoted `$env:RUNNER_PM=deno`
captures deno's multi-line REPL banner into the variable, and the error
rendered it as \u{1b}-escape soup with no indication which source
supplied it. Now the error names the carrying source, escapes control
chars, truncates past 60 chars, and hints at the captured-command-output
footgun (with the quoted spelling) when the value contains line breaks.
`install_pms` received the overrides but only used them for the GHA log
group, so `RUNNER_PM=bun runner install` still ran every detected PM —
in a bun+deno repo, `deno install` always ran and wrote a deno.lock
nobody asked for. The override now selects the install set: a detected
PM installs alone; an undetected one refuses with the new
`ResolveError::PmOverrideNotDetected` (exit 2), naming the carrying
source and the detected set. The chain path (`runner install <tasks>`)
inherits the behavior. The "via <source>" provenance fragment moved to
`OverrideOrigin::describe_pm_source` so `--explain` and the install
error share one wording.
An invalid env override (e.g. a REPL banner captured by PowerShell
unquoted assignment) killed every command at override parsing — before
dispatch — including `runner doctor`, the one command whose job is to
diagnose a broken environment. Dispatch now retries doctor through a
lenient constructor that pre-validates env-sourced values (RUNNER_PM,
RUNNER_RUNNER, RUNNER_FALLBACK, RUNNER_ON_MISMATCH), blanking invalid
ones into `DetectionWarning::InvalidEnvOverride` rendered on the
report. Strict behavior is untouched for all other commands and for
CLI flag values; CLI-shadowed env garbage stays invisible, mirroring
strict precedence. Env reads are deduped into EnvSnapshot so the two
constructors cannot drift.
The PATH probe reported Volta shims (e.g. C:\Program Files\Volta\
npm.EXE) as if they were the tools themselves. New tool::volta module
classifies a probe hit as a shim by canonicalized parent-dir equality
against the located volta binary dir and $VOLTA_HOME/bin, then asks
`volta which <tool>` (cwd = project root, so project pinning applies)
for the real binary — classified by exit status and stdout only, never
error-text parsing. Doctor renders `npm=<shim> -> <real> (volta)` or
`(volta shim, not provisioned)`; JSON gains the additive
signals.node.volta_shims map behind a builder flag (doctor/info: on,
list and unit tests: off, so no hot path spawns volta). Probe-fallback
filtering of unprovisioned phantom shims is deliberately deferred.
@coderabbitai

This comment was marked as outdated.

@coderabbitai coderabbitai Bot added the enhancement New feature or request label Jun 11, 2026
coderabbitai[bot]

This comment was marked as resolved.

@kjanat kjanat self-assigned this Jun 11, 2026
@kjanat kjanat added the cr:skip Skip CodeRabbit review label Jun 11, 2026
The loose prefix fallback never stripped a bare `=` operator and
stripped `v` before trimming inner whitespace, so `=20.11` and
`>= v18` mis-cleaned when an unparseable `current` forced the
fallback path. Trim first, strip operators incl. `=`, then strip
the `v` prefix.

Also scrub every inherited `RUNNER_*` var (case-insensitive, for
Windows) from the doctor_env child processes — a dev box exporting
`RUNNER_NO_WARNINGS` or `RUNNER_FALLBACK` could flip assertions.
@coderabbitai coderabbitai Bot added the documentation Improvements or additions to documentation label Jun 11, 2026
@kjanat kjanat force-pushed the fix/version-ranges-pm-override branch from f2b94f8 to 8806223 Compare June 11, 2026 18:42
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 11, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
runner 8806223 Commit Preview URL

Branch Preview URL
Jun 11 2026, 06:43 PM

kjanat added 3 commits June 11, 2026 20:43
`DetectionWarning::InvalidEnvOverride::detail()` renders the parse-error
`message`, but `lenient_env_field` only sanitized the (unused-by-detail)
`raw` field — so a long or secret-looking env value leaked verbatim and
untruncated into the warning text. Route the message through a new
`sanitize_error_message`, which rewrites the raw value (and its
`escape_debug` form) to the truncated `sanitize_raw_label` rendering.

Fixes the `lenient_policy_env_garbage_does_not_leak_full_raw_value` test,
whose fix was previously stranded one PR up the stack in #50.
@kjanat kjanat merged commit 31641c9 into master Jun 12, 2026
17 checks passed
@kjanat kjanat deleted the fix/version-ranges-pm-override branch June 12, 2026 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cr:skip Skip CodeRabbit review documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant