Version range semantics, PM-override install, self-diagnosing errors, doctor resilience, Volta shims#45
Merged
Merged
Conversation
`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.
This comment was marked as outdated.
This comment was marked as outdated.
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.
f2b94f8 to
8806223
Compare
Deploying with
|
| 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 |
`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.
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.
Bug cluster surfaced by dreamcli CI run 27305763214: a bogus node-version warning, an unwanted
deno.lockcommitted by autofix-ci, and an unreadableRUNNER_PMerror that also killeddoctor. One commit per fix.Fixes
version_matchesreal range semantics (3521f4b) — was operator-strip + prefix match, so>=22.22.2behaved as=22.22.2and warned on Node 22.22.3/25.9.0. Node-semver grammar (space-AND,||unions, hyphen ranges,xwildcards, detached operators,vprefixes) is normalized into the existingsemverdependency. Bare versions (.nvmrc20.11) keep prefix-at-segment-boundary semantics; unevaluable input (lts/*) falls back to the old prefix match.a4fb1d7) — unknown--pm/RUNNER_PM/--runner/RUNNER_RUNNERvalues 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=denoexecutes deno and captures its REPL banner).runner installhonors--pm/RUNNER_PM(d53bb86) — the override now selects the install set instead of being ignored (previously every detected PM installed; bun+deno repos always randeno installand wrotedeno.lock). An override naming an undetected PM refuses with newResolveError::PmOverrideNotDetected(exit 2). runner.toml[pm].node/[pm].pythondeliberately keep scoping script dispatch only (test-pinned non-goal).doctorsurvives bad env (2f319f0) — an unparseableRUNNER_*value no longer kills the command whose job is diagnosing a broken environment. Dispatch retries doctor through a lenient constructor; invalid env values degrade toenv:warnings on the report (additive in the JSONwarningsarray, no schema bump). Other commands and explicit CLI flags stay strict.806e197) — PATH-probe hits classified as Volta shims (canonicalized parent-dir equality against the volta bin dir /$VOLTA_HOME/bin) resolve throughvolta whichrun in the project root, so--dirtargets 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). Additivesignals.node.volta_shimsJSON field; display only — execution still spawns the shim. Probe-fallback filtering of phantom shims deferred to a follow-up.Verification
tests/doctor_env.rs); clippy clean, no lint suppressions added.>=22.22.2 ... current 24.14.1 (ok);--pm npm installin a bun+cargo repo exits 2 naming the source and detected set;doctorwith a multi-line garbageRUNNER_PMruns and reports it;--dirdifferential test confirms Volta resolution honors the target project pin (11.12.1 vs 11.6.2).PATHleaks into probe/dispatch test expectations); verified pre-existing via stash-test, pass in CI.