Skip to content

feat: add codevhub doctor to check the environment before installing - #213

Merged
quickbeard merged 16 commits into
mainfrom
feat/doctor-preflight-command
Jul 29, 2026
Merged

feat: add codevhub doctor to check the environment before installing#213
quickbeard merged 16 commits into
mainfrom
feat/doctor-preflight-command

Conversation

@quickbeard

Copy link
Copy Markdown
Owner

Why

Users on the internal network find out their machine can't run CoDev only partway through codevhub install — as a bare fetch failed, after npm i -g has already mutated the machine. codevhub doctor runs every precondition first, read-only, and tells them what to fix.

Two concrete gaps this also closes:

  • src/index.tsx gated at Node 22.5.0, but the install guide requires 22.21.0. That is not a round number by accident: 22.21.0 is where HTTP_PROXY/HTTPS_PROXY support was backported to the 22 LTS line (nodejs/node#57872). Below it Node silently ignores proxy env vars, so a user on 22.10 passed our gate and then could not sign in under any configuration.
  • grep -rn "PROXY" src/ returned nothing. TLS interception was handled well (src/lib/tls.ts); proxying was not handled at all.

What codevhub doctor checks

Group Checks
Environment Node version, npm present, global prefix (on PATH + writable), npm registry/proxy/TLS config, proxy & TLS env audit, OS certificate store
Network CoDev backend reachable, npm registry reachable (npm ping + npm view)
Account SSO sign-in, gateway API key, CoDev config, Supabase (used by upload)
LLM key valid, models listable, a real one-token completion
This machine installed agents, CoDev-managed configs, backups, shims, editor CLIs

Read-only — it installs and configures nothing. Exit 0 when nothing failed (warnings don't fail it), 1 otherwise. --force tests a real sign-in instead of reusing the cached session.

Only the LLM completion proves inference is permitted; /key/info and /v1/models both pass for a key that is then 403'd on every completion.

The diagnosis layer is the point

describeNetworkError unwraps one level of err.cause and only special-cases cert codes, so everything else still reached users as fetch failed (connect ECONNREFUSED 10.0.0.1:8080).

diagnoseError walks the whole chain — including the AggregateError Node emits one entry per address family into — and returns four parts:

✗ Reach the CoDev backend
  What happened  Connection refused by 127.0.0.1:9.
  Likely cause   That address is your proxy — nothing is listening on it,
                 so the proxy host or port is wrong.
  What to do     Double-check HTTP_PROXY / HTTPS_PROXY. The value must
                 include the scheme and port, e.g. http://10.0.0.1:8080.
  Context        POST https://netmind.viettel.vn/... (timeout 20s)
                 proxy: HTTPS_PROXY=http://127.0.0.1:9 · NODE_USE_ENV_PROXY: on
                 NO_PROXY: unset · node v24.15.0 · darwin arm64
  Raw            TypeError: fetch failed
                 └ Error: connect ECONNREFUSED 127.0.0.1:9  (code
                   ECONNREFUSED · syscall connect · address 127.0.0.1 · port 9)

Also handled:

  • HTTP responses (diagnoseResponse) — a non-2xx isn't an exception, so describeNetworkError never saw it, yet 407 and a proxy-issued 403 are exactly what internal users hit. Interceptor headers (via, proxy-authenticate, …) distinguish "the network blocked this" from "your account lacks access".
  • npm's stderr in full (diagnoseExec) — it names the registry, proxy and .npmrc in play; truncating it destroys the diagnosis.
  • Redaction — raw chains and response bodies go through extra, never unsafeUnredacted, and terminal output is scrubbed with the logger's existing SCRUB_PATTERNS, since users paste this into chats.

Failures expand inline. There is deliberately no --verbose flag: explaining the failure is the command.

Proxy remediation

When the network group fails, doctor offers to re-run everything through a proxy the user types in (nothing written to disk), so they see the fix work before it prints the exact export / setx commands to make it permanent. NO_PROXY entries covering our own backend are dropped in the retry child.

The re-exec is handed to index.tsx rather than run in the component — spawnSync with inherited stdio while Ink still owns the TTY corrupts the terminal.

Also included (both requested)

Node floor 22.5.0 → 22.21.0, with the reason in the error text, plus the missing engines field so npm i -g warns before the binary ever runs.

The whole CLI now honors proxies. Node needs NODE_USE_ENV_PROXY=1, read at bootstrap, so users who export only the proxy variables get nothing. lib/proxy.ts enables it at the entry point via http.setGlobalProxyFromEnv() — verified empirically to route global fetch, not just http.Agent — falling back to a re-exec on older Node. install, login, upload, model and the launch-time key refresh all benefit.

install/config gain a pre-flight (Node version + proxy env only) so a misconfigured proxy surfaces before the user invests in the wizard. Login/FetchApiKey render the full diagnosis for transport errors while leaving precise backend HTTP messages alone.

Notes for review

  • The install pre-flight is deliberately smaller than the doctor's environment group. The npm checks each spawn npm config get (~300ms), and the OS-trust-store read blocks the event loop for 300ms+ on Windows — the exact stall AGENTS.md documents as having previously broken three timing-sensitive Ink tests. It is advisory and never blocks; the one condition that genuinely can't proceed (Node below the floor) is already refused at startup.
  • One existing test changed meaning. tests/components/Login.test.tsx asserted the old terse Login failed: fetch failed (getaddrinfo ENOTFOUND …) — precisely the output this PR removes — so it now asserts the diagnosis instead.
  • tlsApi.supported() stays. At a 22.21 floor every 22.x is fine, but Node 24.0–24.4 predates getCACertificates, so the guard is still load-bearing.
  • Two bugs surfaced from running the built binary against a dead proxy and are fixed here: rootLink now recovers an errno from message text when a re-throw dropped .code, and applyEnvProxy records NODE_USE_ENV_PROXY after enabling the proxy — it previously claimed "Node is IGNORING your proxy settings" about a request that had visibly just gone through the proxy.

Testing

pnpm fix, pnpm typecheck, pnpm test (1125 passing, 126 new), pnpm build + smoke-test — all clean.

New: tests/lib/doctor.test.ts (table-driven per error code, asserting all four parts are populated and that no diagnosis renders a bare fetch failed; real nested fetch error shapes, AggregateError, a real AbortSignal.timeout, redaction), tests/lib/proxy.test.ts, tests/DoctorApp.test.tsx, and Node-gate boundary cases in tests/lib/const.test.ts (a naive compare would reject all of 23.x/24.x).

Not covered: a full codevhub doctor run against the live backend blocks on interactive SSO, so it was driven non-interactively via CODEV_BYPASS_LOGIN=1 with a dead proxy. Worth one manual run on a real proxied machine before release.

🤖 Generated with Claude Code

quickbeard and others added 16 commits July 28, 2026 15:48
Users on the internal network discover their machine can't run CoDev only
partway through `codevhub install` — as a bare `fetch failed`, after
`npm i -g` has already mutated the machine. `codevhub doctor` runs every
precondition first, read-only, and explains what to fix.

Five groups: environment (Node, npm, global prefix, registry/proxy config,
proxy & TLS env, OS cert store), network (backend + npm registry
reachability), account (sign-in, API key, config, Supabase), LLM (key,
models, and a real one-token completion — the only check that proves
inference is permitted), and an informational report on what is already
installed.

The diagnosis layer is the point of the command. `describeNetworkError`
unwraps one level of `err.cause` and only special-cases cert codes, so
everything else still reached users as `fetch failed (connect ECONNREFUSED
10.0.0.1:8080)`. `diagnoseError` walks the whole chain — including the
AggregateError Node emits per address family — and returns what happened in
plain language, the likely cause given this machine's proxy state, the fix,
and the raw chain for support. It also diagnoses HTTP responses (407, a
proxy-issued 403, 502), which are not exceptions and so were never seen at
all, and reproduces npm's stderr in full because it names the registry,
proxy and .npmrc in play. Nothing bypasses the logger's redaction.

When the network checks fail it offers to re-run everything through a proxy
the user types in, then prints the exact export/setx commands to make the
working settings permanent. The re-exec is handed to index.tsx rather than
run in the component: spawnSync with inherited stdio while Ink owns the TTY
corrupts the terminal.

Also raises the Node floor from 22.5.0 to 22.21.0 and adds the missing
`engines` field. That is not a round number by accident — 22.21.0 is where
HTTP_PROXY/HTTPS_PROXY support was backported to the 22 LTS line
(nodejs/node#57872). Below it Node silently ignores proxy environment
variables, so a user on 22.10 passed the old gate and then could not sign in
under any configuration.

And teaches the whole CLI to honor proxies: Node needs NODE_USE_ENV_PROXY=1,
read at bootstrap, so users who export only the proxy variables get nothing.
`lib/proxy.ts` enables it at the entry point via http.setGlobalProxyFromEnv()
(verified to route global fetch, not just http.Agent), falling back to a
re-exec on older Node. It also flags a NO_PROXY entry covering our own
backend — the documented cause of `Login failed`.

`install`/`config` gain a pure pre-flight (Node version + proxy env only) so
a misconfigured proxy surfaces before the user invests in the wizard. The
npm and OS-cert-store checks are deliberately excluded: each npm probe costs
~300ms, and the trust-store read blocks the event loop for 300ms+ on Windows
— the stall documented in lib/tls.ts. Login/FetchApiKey now render the full
diagnosis for transport errors while leaving precise backend HTTP messages
alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…the npm README

The "22.21 is the release that added HTTP_PROXY support" explanation was
detail readers don't need at the top of a README; the requirement itself is
enough. The rationale is still recorded in AGENTS.md and in the startup
error message, where it's actionable.

The npm package page also no longer carries the full `codevhub doctor`
walkthrough — the repo README keeps it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`codevhub doctor` failed the backend reachability check with a connect
timeout on machines where `codevhub install` worked perfectly. Two causes,
both in the probe rather than the network.

It probed `GET /codev-backend` — the API base path, which is not an endpoint
the CLI ever calls. The gateway 301s it to its own internal origin
(`http://netmind.viettel.vn:9096/codev-backend/` — note the port and the
downgrade to http), `fetch` followed that redirect by default, and the
connect to :9096 times out from outside the corporate network. So the probe
measured an address no CoDev command touches and blamed the backend.

Now it probes `POST /codev-backend/config`, a route the CLI actually calls,
which answers 401 without a token — proving DNS, TCP, TLS and the real API
route in one round trip. `reach()` also sets `redirect: "manual"`: a redirect
response is already proof the transport works, and chasing it measures a
different host and port. A 401/403 is labelled as expected for an
unauthenticated probe so it doesn't read like a failure.

Two related false diagnoses found while verifying the fix against the real
backend:

`diagnoseError` blamed the proxy for errors that never reached the network.
`Backend /auth/exchange failed (401)` has no `err.cause` and no errno, so it
fell through to the generic branch and told users with demonstrably working
networks — the server had just answered them — that they were missing a
proxy. Application-level errors now report the server's own message, and an
auth status points at `codevhub login --force`. Transport errors keep their
proxy reasoning, including causeless ones whose errno survives only in the
message text.

And the proxy setup block was appended whenever anything in the account or
LLM group failed, burying "log in again" under a wall of export lines. It
now keys off the diagnosis naming the proxy environment variables. Matching
the variable names rather than the word "proxy" matters: the internal npm
mirror's URL ends in `/npm-proxy`, which a loose match pulled the block in
on a run whose only issue was the registry setting.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…l cache

The three LLM checks called backend.ts without a base URL, so it fell back to
AI_GATEWAY_URL(), which reads `gateway_url` out of ~/.codev-hub/auth.json and
throws when it is absent. On a machine that has never run `codevhub install`
— precisely the audience this command exists for — all three failed with
"Missing gateway_url in ~/.codev-hub/auth.json. Run `codevhub install`".
Circular advice from a pre-flight tool, and wrong: the config check fetched
the URL one step earlier and left it sitting unused in the context. doctor
deliberately never writes that cache, so the value has to be threaded through.

They now also skip, rather than fail, when the URL could not be fetched at
all — the config check above already reports why, and repeating it three
times as a failure just buries it.

While verifying, a bare `fetch failed` was still reaching the user:
smokeTestModel stringifies its own errors, so an unreachable gateway arrived
as "Couldn't reach the gateway to test <model>: fetch failed". That case is
now diagnosed as a connectivity problem and cross-referenced to the model
list check, which hit the same host and carries the full error chain. The
raw string is kept for support.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… an IP

WHATWG URL parses bare integers as 32-bit IPv4 addresses, so typing just the
port at the "Proxy (host:port)" prompt was accepted silently: `8080` became
`http://0.0.31.144`, `3128` became `http://0.0.12.56`. The retry then failed
as an unexplained connection timeout to an address the user never typed —
against a prompt where entering only the port is a very plausible slip.

Numeric-only input is now rejected at the prompt, where we can still name the
mistake: `"8080" looks like just the port. Enter the host too, e.g.
10.0.0.1:8080`.

Also pins the accepted formats, which were previously untested: hostnames,
bracketed IPv6, embedded `user:pass@` credentials, an optional scheme, and
rejection of space-separated `host port`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The prompt was suppressed when a proxy was already configured and active, on
the reasoning that "it's set up, so something else is wrong". That was
backwards: a wrong proxy address is among the likeliest reasons the network
checks failed at all, and suppressing the prompt left exactly that user with
no way to try a different one. The retry sentinel is now the only guard, so
it is still offered at most once per run.

When a proxy is present the copy shifts from "do you need a proxy?" to "is
this one wrong?" — it names the current value, and Enter keeps it rather than
skipping.

The field also lists concrete examples now. "host:port" alone left real
questions unanswered — does a hostname work, how do I pass a password, do I
need to type http:// — so there is one example per question:

  10.60.129.1:3128            IP and port
  proxy.corp.vn:8080          hostname and port
  user:pass@10.60.129.1:3128  proxy that needs a login
  http://10.60.129.1:3128     full URL (http:// is assumed if you omit it)

Also fixes a self-contradiction spotted while reviewing the rendered output:
an ECONNREFUSED carrying no address fell back to the request host, so the
report read "Connection refused by netmind.viettel.vn" immediately above
"That address is your proxy". The proxy claim is now made only when the error
actually named the address it tried.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The examples render on both variants, but only the no-proxy path asserted
them — nothing would have caught them disappearing from the prompt shown to
someone correcting a wrong proxy, who needs the syntax just as much.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`rerunDoctorWithProxy` lived in index.tsx, unexported and untested — the only
coverage was DoctorApp asserting it does NOT spawn. So the one place `doctor`
shells out had nothing pinning its argv or environment, and answering "what
command does this actually run?" meant reading the source.

Moved into lib/doctor.ts, which is also the right layer (index.tsx is a
dispatcher; logic belongs in lib/). The tests now state the command
literally:

  <node> [...process.execArgv] <process.argv[1]> doctor [...args]

with assertions that it invokes node directly rather than a shell or the
`codevhub` bin, that stdio is inherited, and that execArgv is forwarded —
which matters because a `pnpm dev` run carries tsx's loader flags and a child
without them cannot load TypeScript at all.

Also pins the environment (HTTP_PROXY/HTTPS_PROXY, NODE_USE_ENV_PROXY=1,
NODE_USE_SYSTEM_CA=1, CODEV_DOCTOR_PROXY=1), that a NO_PROXY entry covering
the backend is dropped from the child while the user's own environment is
left untouched, and that the child's exit code propagates — including a
killed child reporting failure rather than success.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"What is this thing actually running on my machine?" is a fair question for a
diagnostic tool people run on locked-down boxes, often while on a call with
IT — and the only answer was "read the source". These tests are the answer:
the complete ordered list per group, failing the moment a command is added,
removed or reworded.

  environment  npm -v, npm config get {prefix,registry,proxy,https-proxy,
               strict-ssl,cafile}
  network      npm ping, npm view codev-ai version
  account/llm  none — pure HTTP
  state        npm root -g
  pre-flight   none (it is embedded in `codevhub install`, where even one
               ~300ms `npm config get` would be felt on every run)

Ten processes for a full run, plus a guard that every one is `npm` and none
mutates — no install, uninstall, publish, link or `config set`. `doctor`
promises to be read-only and that assertion is what enforces it.

Measuring this found `installedAgentsCheck` spawning `npm root -g` four
times, in series, one per agent, for an answer that cannot differ between
them — roughly a second wasted in a command people run while already
frustrated. It now resolves the root once.

The spy sits at the `execFile` boundary rather than on `lib/npm.ts#execAsync`
because helpers inside npm.ts call `execAsync` through their module-local
binding, which a spy on the export cannot intercept. An earlier draft used
that spy and reported the state group as spawning nothing while it was in
fact shelling out four times — the bug above would have stayed invisible.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`codevhub doctor` produced a screenful of diagnosis and nothing durable. The
results did reach the NDJSON diagnostic log, but extracting one run from an
append-only trail is not something a user on a support call is going to do.

Every run now writes a self-contained JSON report and replaces the previous
one — deliberately a single file rather than a dated series, because it is a
snapshot of "how is this machine right now" and a stale one is worse than
none when it gets attached to a ticket.

It carries the timestamp, CoDev/Node/platform versions, the full proxy and
TLS environment, every check outcome with its diagnosis, the summary counts,
and the suggested next steps. The summary line names the path, tilde-
abbreviated so it stays on one line — a wrapped path picks up the Step's
`│  ` gutter on continuation lines and is corrupted when copied, the same
trap Login.tsx documents for the sign-in URL.

Two properties it has to hold. It is scrubbed with the same patterns as the
terminal output and the log, because this is the file most likely to be
emailed around and so the last place a token should survive; scrubbing after
stringification is safe since no pattern can match across a JSON quote or
escape. And writing is best-effort in the sense lib/log.ts established — a
diagnostic that breaks the command it is diagnosing is worse than no
diagnostic — so an unwritable location returns null and the run continues.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The command inventory existed only in tests, so the answer to "what did this
just run on my machine?" was still "read the source" — a fair question for a
diagnostic tool people run on locked-down boxes, often while on a call with
IT.

`doctor` now records every child process it spawns and prints them:

  ◇  Commands run
  │  ✓ npm -v  (97ms)
  │  ✓ npm config get prefix  (343ms)
  │  ...
  │  ✓ npm root -g  (79ms)
  │  All read-only — doctor never installs, uninstalls or changes
  │  configuration.

with duration and outcome per command. The report file carries the same list
under `commands`, so an attached report answers it too.

The recorder sits inside `execAsync` rather than wrapping it from doctor.ts:
helpers in npm.ts (`npmGlobalRoot`, `verifyInstall`, …) call `execAsync`
through their module-local binding, which no external wrapper can see — the
same property that made an earlier test miss four `npm root -g` spawns
entirely. It is off by default so `update` and the upload daemon, which share
the seam, do not accumulate an unbounded buffer.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"Commands run" listed only subprocesses, so it answered half the question.
The connection tests — codev-backend, Supabase, the gateway, SSO — are HTTP,
never touch execAsync, and appeared nowhere. On a corporate network they are
the more useful half: the endpoints are what IT needs in order to allow-list
them.

`loggedFetch` gains an opt-in recorder, the sibling of npm.ts#commandLog, and
doctor renders a second section:

  ◇  Endpoints contacted
  │  ✓ POST https://netmind.viettel.vn/codev-backend/config  (401, 96ms)
  │  ✓ POST https://netmind.viettel.vn/codev-backend/auth/exchange  (401, 248ms)
  │  ...
  │  Reachability only — a 401 here means the endpoint answered. Query
  │  strings are omitted; they can carry tokens.

The report file carries the same list under `requests`.

Two things the first attempt got wrong, both caught by running it:

Scoring on `res.ok` painted the expected 401s red, directly contradicting the
check rows above that correctly call them a pass. This section answers "was
the endpoint reachable", so any response counts and only "no response at all"
is a failure — recorded as a null status.

And each row was three flex children, so a long URL wrapped and knocked the
status icon out of its column. Both sections now render one Text per row.

Query strings are dropped rather than redacted: OAuth codes and signed-URL
signatures live there and none of it helps a user see which endpoint was
contacted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous version collected the commands and endpoints into two sections
at the end of the run, which left the reader correlating a flat list back to
the step that produced it. Put the evidence on the row instead:

  ✓ List available models
    2 models available (MiniMax/MiniMax-M2.7, zai-org/GLM-4.7-cc).
    ↳ GET https://netmind.viettel.vn/gateway/v1/models → 200  (142ms)
  ✓ Send a test request to the LLM
    MiniMax/MiniMax-M2.7 answered a test prompt.
    ↳ POST https://netmind.viettel.vn/gateway/v1/chat/completions → 200  (1.2s)

`runChecks` slices whatever the recorders gained while each check ran onto
that check's `activity`. Exact, because checks run strictly in sequence — and
it still holds for a check that fans out internally: npm-registry issues five
`npm config get` concurrently and all five land on its row. Sign-in is marked
by hand, since <Login> owns it rather than runChecks, so its SSO requests are
not the only work in the run with no row to sit under.

Activity lines carry no status icon of their own — the row's icon is the
verdict, and the separate-section version scored an expected 401 red directly
under a check that correctly called it a pass. They render after the fix
(status, what, what to do, then evidence); above it they pushed the one
actionable line down the screen.

The report file keeps the flat `commands`/`requests` arrays as the complete
machine-readable record, and now also carries the per-check attribution.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ch request used

The environment row listed five hard-coded variables, so anything outside
that set was invisible — including NODE_EXTRA_CA_CERTS, which is the remedy
our own TLS guidance hands out, plus NODE_OPTIONS and npm's npm_config_*
overrides. `readProxyEnv` normalizes to fixed fields because that is what the
logic needs, but on a machine where the network misbehaves the variable
nobody thought to look at is usually the one causing it. It now reports
whatever is actually set, verbatim and in the user's own spelling:

  ▲ Proxy & TLS environment
    HTTP_PROXY=http://user:***@10.60.129.1:3128 ·
    HTTPS_PROXY=http://user:***@10.60.129.1:3128 · NO_PROXY=localhost ·
    NODE_USE_ENV_PROXY=1 · NODE_EXTRA_CA_CERTS=/etc/ssl/corp-root.pem

Each request's activity line now also names the proxy it went through:

  ↳ POST https://netmind.viettel.vn/codev-backend/config → 401
    via http://user:***@10.60.129.1:3128

which is the difference between "this endpoint is unreachable" and "your
proxy could not reach this endpoint" — and the only place a NO_PROXY
exemption becomes visible, as one request quietly going direct while the rest
are proxied. It accounts for scheme and for NODE_USE_ENV_PROXY, so a proxy
that is configured but ignored is never claimed as used.

Credentials are masked at every display boundary — the check row, the
activity lines, the diagnosis context and the report file all end up pasted
into tickets, and proxy URLs routinely carry user:pass. `readProxyEnv` keeps
the real value; the retry child has to authenticate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reporting only what was set meant a reader scanning for HTTP_PROXY had to
infer its absence from a list that showed everything except it — and most of
the failures this command exists for are a *missing* variable, so "unset" is
an answer rather than noise:

  ✓ Proxy & TLS environment
    HTTP_PROXY=unset · HTTPS_PROXY=unset · NO_PROXY=unset ·
    NODE_USE_ENV_PROXY=unset · NODE_USE_SYSTEM_CA=unset ·
    NODE_EXTRA_CA_CERTS=unset · NODE_TLS_REJECT_UNAUTHORIZED=unset

Seven core variables are now always listed. The lowercase spellings, ALL_PROXY,
NODE_OPTIONS and npm's npm_config_* stay in a when-set tier: eleven more
`unset` entries would bury the seven that matter, and `http_proxy=unset`
beside `HTTP_PROXY=unset` reads as a duplicate rather than as information.

A set lowercase spelling is reported under its own name while the uppercase
stays listed as unset, rather than the uppercase silently absorbing the value
— which is what the reader needs to see when only one of the two is exported.

The report file carries the same view, with null for unset.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI failed on windows-latest only. Both failures traced to the same fact:
Windows environment variables are case-insensitive, so `http_proxy` and
`HTTP_PROXY` are one variable.

One of them was a real bug, not a test artifact. The proxy retry built its
child environment as `{...process.env, HTTP_PROXY: proxy.http}`, which on
Windows yields an object holding *both* the user's original key with the
stale value and ours with the new one. Which of the two the child sees is
left to chance, and if the stale one wins the retry silently tests a proxy
address the user never typed. Overrides now go through `overrideEnvVar`,
which removes every other spelling first. That is right on POSIX too: there
both spellings are real and both are consulted, so leaving the old one behind
is the same ambiguity by a different mechanism. NO_PROXY stripping likewise
covers every spelling rather than the two that were hardcoded.

The other was the test asserting that setting `http_proxy` leaves
`HTTP_PROXY` unset — true on POSIX, unknowable on Windows. It and its
neighbours now pass an explicit environment to `proxyEnvSummary` instead of
mutating the process, which is hermetic and platform-independent.

The retry test also looked up `env.NO_PROXY` directly; it now searches
case-insensitively and ignores empty spellings, since an unset variant
legitimately stays empty because there is nothing in it to strip.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@quickbeard
quickbeard merged commit b9e9a32 into main Jul 29, 2026
4 checks passed
@quickbeard
quickbeard deleted the feat/doctor-preflight-command branch July 29, 2026 04:32
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