Skip to content

feat(staged): honor npm config for managed installs and surface missing managed bridges in Doctor - #998

Merged
matt2e merged 7 commits into
mainfrom
homebrew-agent
Oct 27, 2026
Merged

matt2e merged 7 commits into
mainfrom
homebrew-agent

Conversation

@matt2e

@matt2e matt2e commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Staged hardcoded Block's Artifactory URLs for every npm command and for the managed Node.js download. That meant from-source builds and Macs off WARP could never get a managed Claude/Codex bridge. When a managed install failed, Doctor still showed a green "Installed" row pointing at a Homebrew or npm-global copy, so nothing told you Staged's copy was missing.

No hardcoded Block URLs

  • npm: STAGED_NPM_REGISTRY overrides the registry if set. Otherwise Staged passes no --registry and npm uses its own config (NPM_CONFIG_REGISTRY, ~/.npmrc including scoped entries and auth, then registry.npmjs.org).
  • Node.js: downloaded from https://nodejs.org/dist by default, or from STAGED_NODE_DIST_URL if set. It is still SHA-256-pinned by node-runtime.lock.json.
  • Build-time fallbacks: STAGED_FALLBACK_NPM_REGISTRY and STAGED_FALLBACK_NODE_DIST_URL are compiled in via option_env!. Each is retried once, and only when there's no explicit override:
    • npm: only when registry.npmjs.org returns a Cloudflare Gateway block page.
    • Node: only on network errors, never on a bad checksum, bad archive or HTTP status.
  • staged-release.yml reads both fallbacks from repository variables, so no internal URL lives in the tree.
  • Removed the no-block-npm-registry feature, BLOCK_NPM_REGISTRY_URL and BLOCK_NODE_DIST_BASE_URL, and the Artifactory default in update-node-runtime-lock.mjs.
  • Managed installs now run under the same sanitized login-shell env snapshot that the freshness probe and Doctor use. Before, a registry exported from the shell rc steered the probe but not the install when Staged was launched from Finder.
  • Doctor-crate npm fixes and updates get the same one-shot gateway fallback.
  • Freshness probes use --fetch-retries=0 and a bounded --fetch-timeout, so a dead ~/.npmrc host can't stall them.

Doctor surfaces a missing managed copy

  • If a Claude/Codex row resolves to anything other than the managed shim, it drops to Warn with one of:
    • "Using <path> — Staged's managed install hasn't completed yet" (first launch)
    • "… Staged's managed copy is missing: <npm error code>" (after a failed install)
  • The row always offers a Fix that runs the managed install.
  • Copy details has the full recorded error, a classified hint (gateway block, mirror 404, unreachable host, or a bad Node download URL), the last reconcile's outcome and age, and the detected npm registry.
  • The registry probe runs in parallel with the other checks and is cached per env snapshot for 60 s, so it adds no serial latency.
  • When managed installs are disabled (STAGED_ACP_TOOLS_DIR, the no-managed-acp-tools build, or an unsupported platform), the rows say why.
  • The Node.js Runtime row now warns when a runtime download failed, instead of staying hidden.
  • Recorded npm install errors now include npm's npm error code block. These are error strings only; the state.json schema is unchanged and there are no new persisted fields.

Before merging

For Block release builds to keep working on managed laptops with no ~/.npmrc, set the STAGED_FALLBACK_NPM_REGISTRY and STAGED_FALLBACK_NODE_DIST_URL repository variables.

Testing

  • New unit tests use fake npm binaries and Node download fixtures. They cover:
    • the registry precedence and fallback gates
    • the stderr classifier
    • the env snapshot reaching the install
    • Doctor row text, fixes and hints
    • the registry-probe cache
  • cargo fmt --check, cargo clippy --lib -D warnings, cargo test --lib -- doctor managed_acp_tools, pnpm run check and the Doctor vitest suite all pass. The pre-push CI hooks also passed.

🤖 Generated with Claude Code

matt2e and others added 4 commits October 27, 2026 12:00
…stead of hardcoding Block URLs

Staged passed `--registry <Block Artifactory>` to every npm command and
downloaded the managed Node.js runtime from the Artifactory mirror, so a
from-source build or a personal Mac off WARP could never get a managed
Claude/Codex bridge. Follow Berd #1107's split: public code carries no
Block URL, and the one population that needs one (Block laptops with no
~/.npmrc behind Cloudflare Gateway) gets it at build time.

npm registry precedence:
1. `STAGED_NPM_REGISTRY` (explicit override, passed as `--registry`);
2. otherwise no `--registry` — npm resolves `NPM_CONFIG_REGISTRY`,
   `~/.npmrc` (incl. scoped entries and auth), then registry.npmjs.org;
3. if that fails with a gateway block page (stderr names
   blocked.teams.cloudflare.com, or FETCH_ERROR/EJSONPARSE/"Unexpected
   token"/"invalid json" mentioning registry.npmjs.org), no override was
   given, and `option_env!("STAGED_FALLBACK_NPM_REGISTRY")` is set, reset
   the staging dir and retry once through the fallback. 404s, dead
   mirrors and ETARGET are not retried.

Both the floating bridge install and the `npm view` freshness probe
follow the same two-step. Probes additionally pass `--fetch-retries=0`
and `--fetch-timeout=<probe budget>` so a dead `~/.npmrc` host costs
15 s, not npm's ~70 s retry schedule; installs keep npm's defaults.
Install errors name the registry used ("npm default (per npm config)"
when none was passed) so Doctor can show it without a state.json change.

Node dist: `STAGED_NODE_DIST_URL` override, else https://nodejs.org/dist
(SHA-256 pinned by node-runtime.lock.json, so the host is not a trust
boundary). `option_env!("STAGED_FALLBACK_NODE_DIST_URL")` is retried once
on `ManagedNodeError::Network` only — never for a bad checksum, archive,
or HTTP status. Network errors now name the URL.

Removed: the `no-block-npm-registry` Cargo feature,
`BLOCK_NPM_REGISTRY_URL`, `BLOCK_NODE_DIST_BASE_URL`, and the Artifactory
default in scripts/update-node-runtime-lock.mjs. Doctor's option
builders take `npm_registry_override()` so checks and fixes still agree.
staged-release.yml exports both `STAGED_FALLBACK_*` values from
repository variables (no literal in the workflow); the README documents
the precedence and that Block builds must set those variables.

Tests: env-override/fallback helpers, the stderr classifier, the
fallback gate, a fake npm that fails with a block page until `--registry`
is passed (install retries once; override/404/no-fallback do not), probe
flag bounds and probe retry, and Node fixtures for refused-primary →
fallback success plus checksum-failure → no retry.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…ighten the registry fallback

Follow-ups from the review of 4628d2b (honor npm config instead of
hardcoding Block URLs).

Install/probe env divergence: the managed `npm install` inherited the
Tauri process env while `latest_published_version` and Doctor's checks
and fixes ran under the sanitized login-shell snapshot, so an
`NPM_CONFIG_REGISTRY` exported from the shell rc steered the probe but
not the install when the app was launched from Finder. `install_managed_tool`
now takes the snapshot (`doctor::doctor_env_vars`, now pub(crate); the
reconciler builds the same one), runs npm with `env_clear()` + snapshot,
and overlays only the managed node bin on PATH and the private
`NPM_CONFIG_CACHE`; `--prefix` stays on the command line. The
`latest_published_version` doc now describes exactly that.

Classifier: `classify_npm_failure` reports GatewayBlocked only for
`blocked.teams.cloudflare.com` itself, or a FETCH_ERROR / EJSONPARSE /
"invalid json" / "Unexpected token" failure whose *request* went to
`registry.npmjs.org` (`request to https://registry.npmjs.org/`,
`response body at https://registry.npmjs.org/`, or a
`registry: https://registry.npmjs.org` line). A mirror's error body
mentioning its upstream, ENOTFOUND for a custom mirror, and E404 are
Other. A truncated body from the public registry is still accepted as
blocked — one cheap extra attempt — and documented as such.

When the fallback attempt also fails, the surfaced error keeps the
original diagnosis: "npm default registry (registry.npmjs.org) was
blocked by a network gateway; fallback <url> also failed: <err>".

stderr capture keeps the TAIL (npm writes its `npm error code` block
last, after warnings), dropping whole lines from the front past 64 KiB.

Doctor-crate npm fixes: `run_doctor_fix` and `run_doctor_update` go
through `execute_fix_with_gateway_fallback`, which — for an npm-backed
command, with no `STAGED_NPM_REGISTRY` and a compiled-in fallback — reuses
`gateway_fallback_registry` on the executor's error string (the doctor
crate already surfaces the failed command's trimmed stderr as its error)
and retries once with `ExecuteFixOptions { npm_registry: Some(fallback) }`.
No doctor-crate plumbing was needed.

Node dist: an explicit `STAGED_NODE_DIST_URL` now suppresses
`STAGED_FALLBACK_NODE_DIST_URL`, matching the npm rule that an explicit
override is never second-guessed (`fallback_for`); mid-body read
`Network` errors name the URL like the `send()` error does. README and
module docs updated.

Tests: classifier cases (real block page, inline block page, `registry:`
line, truncated body → blocked; Artifactory upstream mention, custom
mirror ENOTFOUND, E404, offline npmjs → other), combined fallback error
message, stderr tail (whole-line drop + oversized single line), a fake
npm that dumps its env proving `NPM_CONFIG_REGISTRY`/`npm_config_registry`
from the snapshot reach the install while the managed cache overlay and
node-bin-first PATH win, a fallback-also-fails install, and the Node
override-suppresses-fallback gate.

No new backend data model fields or types; `NpmFailure`,
`classify_npm_failure`, `gateway_fallback_registry` and
`gateway_retry_failed_message` became pub(crate).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…and offer the managed install

Doctor labels a Claude/Codex row "Managed by Staged" only when the bridge
resolved from ~/.staged/packages/bin. Because that dir is first on the
probe PATH, a Homebrew or npm-global copy can only win while the managed
copy is absent — the launch reconcile has not run yet, or it failed (no
route to the registry, a gateway block page, a mirror 404, a validation
failure). The row then read as a plain green "Installed" pointing at
/opt/homebrew/bin, indistinguishable from a healthy setup, with no hint
that Staged's copy was missing and no way to get it short of waiting for
a newer release to make the update button appear.

Implements the Doctor surfacing from the registry-precedence note and
proposals 1, 2 and 5 of the classification note.

Claude/Codex rows (`flag_missing_managed_copy`): on a managing build, a
row whose executable is not the managed shim drops to Warn (a row already
warning keeps its own message in front) with "Using <path> — Staged's
managed copy is missing", followed by the last reconcile's recorded error
for that tool when there is one. raw_output gains the expected shim path,
the last reconcile's outcome and age, the error with a classified hint,
and the npm registry Doctor detected. The row's fix becomes
`FixType::Bridge` with `managed_install_description(...)` — offered
whether or not an update is available — which `run_doctor_fix` already
routes to `install_managed_tool_logged` for these two checks. The
freshness-pass redirect (`redirect_unmanaged_update_to_managed_install`)
still fires and produces the identical description, so the Update and
Fix buttons never disagree.

Management off (`note_managed_installs_disabled`): when
`managed_shim_bin_dir()` is None, the two rows keep their status and gain
"(managed installs disabled: <reason>)" in message and raw_output, where
`managed_tools_disabled_reason()` names the STAGED_ACP_TOOLS_DIR override
dir, the `no-managed-acp-tools` build, or an unsupported platform.

Node runtime row: a Missing runtime with a failed `lastReconcile` on
record now warns — "Staged-managed Node.js <ver> could not be installed:
<error>" — even with no installed tools, leading with the runtime
download error (minus the reconciler's `<tool id>: ` prefix) and listing
every recorded error, which already embed the URL, plus a hint; the
reinstall fix is kept. Missing with no record, or a clean one, behaves as
before.

Hint classification lives next to `classify_npm_failure` as
`classify_install_failure` / `install_failure_hint`: a gateway block page
(including the "blocked by a network gateway; fallback … also failed"
message) → add `registry=<mirror>` to ~/.npmrc or set STAGED_NPM_REGISTRY;
E404 → the configured registry does not carry <package>;
ENOTFOUND/ECONNREFUSED/ETIMEDOUT/reqwest "error sending request"/an
install timeout naming its registry → check `registry=` in ~/.npmrc or
STAGED_NODE_DIST_URL. Validation and entrypoint failures get no hint.

For that to classify anything, the recorded npm error had to carry more
than an exit status: `run_floating_npm_install_capturing` now appends
npm's own `npm error code <CODE>` line and up to three detail lines
(`npm_error_summary`, minus the `errno` repeat and log pointer, clipped)
to the `NpmInstall` error. Error strings only — state.json's schema is
unchanged, and nothing new is persisted.

Registry detection: `configured_npm_registry` runs `npm config get
registry` with the snapshot's npm (`find_on_env_path`, now pub(crate))
under the snapshot env, 5 s timeout, only when a report actually has an
unmanaged Claude/Codex row; the label reads "<url> (per npm config)",
"<url> (STAGED_NPM_REGISTRY)", or "npm default (…)" with no npm to ask.

Frontend: no change needed — `canFix` already accepts a `bridge` fix on a
non-pass row and `.check-message` wraps long text. README documents what
the rows now say.

Tests: user copy flagged with Bridge fix and otherwise byte-identical;
recorded error + hint + registry on the row, other tool's error kept off
it, already-warning row keeps its message; disabled note without a
downgrade; Node Missing + failed reconcile → Warn with all errors and a
hint, Missing + no/clean record unchanged; classifier cases for the three
categories and the no-hint ones; npm error summary (old/new npm spelling,
clipping, no block); disabled-reason parts; a fake npm proving the
registry probe asks the snapshot's npm under the snapshot env.

Verified: cargo fmt --check; cargo clippy --lib -- -D warnings (the three
`cloned_ref_to_slice_refs` hits under `--tests` are pre-existing in
session_queue/tests.rs and session_runner.rs); cargo test --lib -- doctor
managed_acp_tools: 84 passed; pnpm run check: 0 errors 0 warnings; vitest
src/lib/features/doctor: 59 passed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
… path and soften the first-launch row

Follow-ups from the review of e905c94 (Doctor says when a managed bridge
is missing).

Report latency: `configured_npm_registry` ran after the `tokio::join!` of
the other probes, so an unmanaged Claude/Codex row — the fresh-profile and
failed-reconcile cases this feature targets — added up to 5 s of serial
latency to `run_doctor_report`. The probe now joins the other probes.
Whether to run it is decided up front from the shim dir
(`any_managed_shim_missing`: the shim dir is first on the probe PATH, so a
row can only resolve elsewhere while a shim is absent) rather than from
the finished report; `managed_install_diagnostics` is now a synchronous
assembly of the record and the label. The answer is cached per env
snapshot for 60 s (`NpmRegistryCache`, the `LatestVersionCache` pattern;
a `None` answer is cached too, since re-asking a hung or absent npm every
pass is exactly the stall being avoided). The 5 s probe timeout is kept.
`npm_registry` became `Option<String>`; a row rendered without a probe
says "npm registry: not probed".

First-launch window: with no reconcile on record, the row says "Using
<path> — Staged's managed install hasn't completed yet" (still Warn, Fix
kept) and the detail reads "last managed install: not run yet (runs at
launch and daily)", so a healthy first launch no longer reads as a
failure.

Message length: the row stops at "… — Staged's managed copy is missing"
plus `short_error_fragment` of the recorded error — npm's own `npm error
code <CODE>` when present, else the first 120 chars ellipsized. The full
error, hint and registry stay in raw_output for Copy details.

Node runtime hint: `install_failure_hint` is consulted only when the
headline is a runtime download failure ("Node.js runtime download
failed…"), so a bridge's npm E404 as the headline can no longer yield
"the configured registry does not carry Node.js". New
`InstallFailureHint::NodeDownloadRejected(status)` for `HTTP 4xx` from
the dist host → "the Node.js download URL returned HTTP <code> — check
STAGED_NODE_DIST_URL"; 5xx gets no hint. `reconcile_headline` still picks
any "Node.js runtime" error (archive, checksum, incomplete) as before.

README: the row says "hasn't completed yet" before the first reconcile
completes, and the hint list gains the download-URL case.

No new persisted fields; state.json is unchanged.

Tests: first-launch row (message, detail, Fix kept, already-warning
variant); error-code fragment on the row with the full error in detail;
ellipsized no-code error and "not probed"; `short_error_fragment` cases;
shim-dir probe gate; Node row with a bridge E404 headline (no hint), an
HTTP 404 download (URL hint), a checksum mismatch (no hint); classifier
cases for HTTP 404/403/503 and the hint text; registry cache per snapshot
(one npm spawn for two asks, `None` cached) and TTL expiry.

Verified: cargo fmt --check (clean); cargo clippy --lib -- -D warnings
(clean); cargo test --lib -- doctor managed_acp_tools: 90 passed;
pnpm run check: 3453 files, 0 errors, 0 warnings; vitest
src/lib/features/doctor: 4 files, 59 passed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-27T02:36:07.522347Z 80fb461 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

if let Some(registry) = registry {
command.arg("--registry").arg(registry);

P1 Badge Override the scoped registry used by managed packages

When ~/.npmrc contains @agentclientprotocol:registry=..., adding only --registry does not force either managed bridge to STAGED_NPM_REGISTRY, because both package names are in that scope. npm's bundled registry documentation states that the registry URL is determined by package scope; locally, npm config get @agentclientprotocol:registry --registry=https://forced.example/ still returns the scoped URL. Thus users cannot escape a stale or unreachable scoped mirror with the advertised override; override the @agentclientprotocol registry configuration for these commands as well.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 1242 to 1244
npm_registry_override().as_deref(),
fallback_npm_registry(),
env_vars,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Key latest-version results by registry configuration

When a user changes ~/.npmrc or STAGED_NPM_REGISTRY while Staged is running, this probe now targets a dynamically selected registry, but the preceding one-hour cache remains keyed only by package name. A Doctor refresh can therefore show the old registry's version and confirmation text while the managed installer fetches @latest from the new registry; include the effective registry/environment in the cache key or invalidate the cache when it changes.

Useful? React with 👍 / 👎.

Comment on lines +56 to +57
// -----------------------------------------------------------------------------
// npm registry policy

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Split the new Rust responsibilities into focused submodules

This change expands managed_acp_tools.rs from 1,806 to 3,287 lines and doctor.rs from 2,569 to 3,560 lines by embedding registry policy, process execution, caching, diagnostics, reconciliation, and extensive tests in the existing modules. Split these responsibilities into focused submodules rather than making two already-large library files substantially harder to navigate and maintain.

AGENTS.md reference: apps/staged/AGENTS.md:L28-L31

Useful? React with 👍 / 👎.

matt2e and others added 3 commits October 27, 2026 14:06
… force the scoped registry and key the version cache by registry config

Addresses the Codex review of PR #998 (80fb461).

Scoped registry (P1): `--registry` only sets npm's *default* registry. Both
managed bridges live under `@agentclientprotocol`, and npm resolves a
scoped package's registry from its `@scope:registry` config first, so a
`~/.npmrc` carrying `@agentclientprotocol:registry=<stale mirror>` kept
winning over `STAGED_NPM_REGISTRY` and the gateway fallback alike (verified
with npm 11.19: `npm config get @agentclientprotocol:registry
--registry=<forced>` still answers the scoped URL; adding
`--@agentclientprotocol:registry=<forced>` wins). `npm_registry_args`
now emits `--registry <url>` plus `--@<scope>:registry=<url>` for a scoped
package, and every site that passed a registry uses it: the floating
bridge install, the `npm view` freshness probe, and — in the doctor crate —
`apply_npm_registry` (fix commands, taking the scope from the first
`@scope/` token in the command) and `latest_npm`. README and the policy
module doc say so.

Latest-version cache (P2): `LatestVersionCache` was keyed by package name
only, so a `~/.npmrc` or `STAGED_NPM_REGISTRY` change while Staged ran let
Doctor show the old registry's version for up to an hour while the
installer fetched `@latest` from the new one. Entries are now keyed by
`LatestVersionKey { package, RegistryFingerprint }`, where the fingerprint
is the override, the env snapshot (its `npm`, `HOME`, `NPM_CONFIG_*`
exports) and the user npmrc's identity (path, size, mtime —
`NPM_CONFIG_USERCONFIG` in either spelling, else `$HOME/.npmrc`). Inserts
drop entries past the TTL so superseded fingerprints do not linger.

Module split (P1, AGENTS.md "split growing modules"): the two files had
reached 3,287 and 3,560 lines, half of it tests. Pure moves, no behaviour
change; cross-module items became `pub(super)`.

  managed_acp_tools/
    mod.rs          layout, env, the managed set, state.json, re-exports
    registry.rs     registry policy: override/fallback, classifiers, hints
    install.rs      install_managed_tool: stage, validate, swap, shim
    probe.rs        npm view / npm config probes and their caches
    reconcile.rs    reconcile epilogue
    validation.rs   (unchanged)
    tests/{support,layout,registry,install,probe,reconcile,validation}.rs
  doctor/
    mod.rs          commands, option builders, fix/update dispatch
    login.rs        interactive login runs (pub: the four login commands)
    managed_bridges.rs  Claude/Codex row readouts and missing-copy flags
    runtime_checks.rs   Node.js runtime and session-queue rows
    tests/{support,commands,login,managed_bridges,runtime_checks}.rs

Each submodule includes its tests with `#[path]`; shared fixtures live in
a `test_support` module. `lib.rs` registers the login commands as
`doctor::login::*` (Tauri's handler macro needs the command's own module);
`crate::doctor::start_doctor_login` etc. stay re-exported for web_server.
The self-respawning validation test names its new test path.

Tests: `npm_registry_args` scoped/unscoped; scoped-flag assertions on the
install fallback/override argv and the probe argv; cache keyed by override,
env and npmrc; `user_npmrc_path` precedence; a fake npm that answers with
its npmrc's contents proving an edited npmrc re-probes within the TTL and
an untouched one is a cache hit with npm gone; TTL insert pruning; doctor
crate `apply_npm_registry` with a scoped package.

Verified: cargo fmt --check; cargo clippy --lib -- -D warnings (clean; the
three `--tests` lints in session_queue/tests.rs and session_runner.rs are
pre-existing); cargo test --lib -- doctor managed_acp_tools: 94 passed
(90 before + 4 new; test names diffed against HEAD); crates/doctor clippy
--lib --tests clean and cargo test --lib: 152 passed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…ure scripts

Staged CI on PR #998 failed nondeterministically in
`install_runs_npm_under_the_env_snapshot` (env dump lacked the snapshot's
NPM_CONFIG_REGISTRY, or carried the runner's hermit NPM_CONFIG_CACHE) and
`configured_npm_registry_is_cached_per_env_snapshot` (npm "spawned" twice).

Cause: `wait_until_executable`, the Linux ETXTBSY workaround behind
`write_executable`, spawned each fresh fixture script with no arguments
under the inherited test-process env and then killed it. `spawn` returns
after `sh` has exec'd, so on Linux the kill can land after the script's
first lines ran — and both fixtures put their side effect there (`env >
log`, `printf x >> calls`). A stray run overwrote the real env dump with
the CI env, or appended an extra call marker. macOS never loses the race,
which is why it was green locally.

Fix, in tests/support.rs:
- `with_exec_probe_guard` inserts `[ -z "$STAGED_EXEC_PROBE" ] || exit 0`
  after the shebang; `write_executable` applies it to every script, and
  the validation tests' `write_node_shim` applies it before the
  production `write_shim`.
- `wait_until_executable` runs the script with `env_clear()` plus
  `STAGED_EXEC_PROBE=1` and waits for it (asserting a clean exit) instead
  of kill-racing it, and asserts the script carries the guard so an
  unguarded fixture fails loudly rather than racing.
- Dropped a redundant `wait_until_executable` after a `write_executable`
  in tests/probe.rs.

Test-only; no product code changes.

Verified: cargo fmt --check; cargo clippy --lib --tests (only the three
pre-existing session_queue/session_runner lints); cargo test --lib --
managed_acp_tools doctor: 94 passed, run alternately against a HEAD
worktree 3x each at load ~96 (all green, same timings). One run of each
of the two 1 s-timeout validation tests failed under that load on this
change, but neither repeated in the A/B runs. Linux is the real check
(macOS never reproduced the race).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
`timeout_kills_bridge_and_native_child_and_preserves_previous_install` and
`timeout_kills_native_child_after_bridge_exits_with_open_pipes` failed the
pre-push hook in two of three full-suite runs at load ~30-45 with "probe did
not spawn its native child: ... `--cli --version` timed out after 1 seconds".
The 1 s budget starts at spawn and has to cover exec'ing the node shim and
`sh` before the fixture even forks its native child; under parallel-suite
load the timeout fired first (no "waiting for native CLI" in the excerpt).

The tests only need the timeout to fire after the native child exists, so
widen it to 5 s and the elapsed bound to 15 s. Test-only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e
matt2e merged commit 856851e into main Oct 27, 2026
6 checks passed
@matt2e
matt2e deleted the homebrew-agent branch October 27, 2026 10:49
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