Repository navigation
fix(setup): materialize the pinned package-runner cache before the first client start - #7032
Conversation
…rst client start Package runners key their package cache by the requested spec, so the entry installed for the @latest invocation that ran setup cannot serve the pinned rea-agents@<version> registration. The first client start installed that entry inside the client's MCP startup deadline and timed out (three consecutive 30 s timeouts observed in one client log; a controlled cold install with every tarball already cached took 22 s). Setup now materializes the pinned entry once after applying registrations: best-effort, bounded by a timeout, launched from a neutral working directory, and non-fatal on failure. The setup result carries the prewarm summary, interactive output prints a warning when the step fails, and direct launcher registrations keep skipping it. Fixes morluto#7027
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
[Medium impact] The PR appears safe to merge; no blocking issue remains. SummarySetup prepares the pinned package-runner cache after applying approved client registrations. Failures remain warnings.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Apply approved client registrations] --> B[Prepare pinned runner cache]
B --> C{Outcome}
C -->|Warmed or skipped| D[Run final doctor checks]
C -->|Failed| E[Show warning]
E --> D
B -->|Interrupt| F[Stop runner tree]
F --> G[Send interrupt to setup]
Reviews (3) · Last reviewed commit: "fix(setup): forward terminal interrupts ..." · Reviewed by Greptile |
…unner tree Address review findings on the prewarm step: - Gate the prewarm on at least one applied client registration (configured_<client>), so an empty plan or a skill-only run never installs into the package-runner cache; the step now only follows registrations the user approved in this transaction. - Replace execFileOutput with a supervised spawn: on timeout the whole runner tree stops through cleanupWindowsProcessTree on Windows and a process-group kill on POSIX, because killing only the direct child leaves npm holding the inherited output pipes past the deadline. - Emit started and completed progress events around the prewarm so an interactive cold install no longer looks like a frozen setup.
A detached POSIX package-runner group no longer receives the terminal's Ctrl-C, so setup could exit while the runner kept installing in the background. The supervised launch now registers SIGINT and SIGTERM forwarding for the duration of the prewarm: the runner tree is stopped through the same family cleanup as the timeout, and the default termination is restored afterwards. An interrupted launch reports a failure reason instead of a hang.
|
Thanks, @wxj006007, for tackling the first-start timeout caused by separate |
Fixes #7027
Problem
Package runners key their package cache by the requested spec, so the
_npxentry installed for therea-agents@latestinvocation that ran setup cannot serve the pinnedrea-agents@<version>registration that setup writes. The first client start after setup installs that entry inside the client's MCP startup deadline; one client log recorded three consecutiveconnection timed out after 30000msfailures, and a controlled cold install with every tarball already in_cacachetook 22 s on the same machine (issue has the full measurements).Change
src/application/SetupRegistrationPrewarm.ts: derives a cache-materializing variant of the registration command (same spec, terminating--versionsubcommand) and materializes it once throughexecFileOutputwith a bounded timeout. Direct launcher registrations returnskipped. The spawn runs in a neutral working directory: inside a REA source checkout the package runner resolves the local project instead of the published entry and the warm-up fails.SetupHost.systemSetupHostexposes this as the optional host capabilityprewarmRegistrationRunner;runSetupWorkflowinvokes it after client configuration and skill installation succeed, before the final doctor summary. The step is best-effort and non-fatal:SetupResultcarries an optionalprewarmsummary, interactive output prints a warning line on failure, and hosts without the capability are unaffected.docs/installation.md: documents why setup performs this step and what happens when it cannot run.Tests
src/application/SetupRegistrationPrewarm.test.ts(services): command derivation for npx/bunx/Windows-wrapped and direct launchers; warmed, skipped, bounded-failure, and timeout outcomes through an injected spawn.tests/boundary/filesystem/setupRegistrationPrewarm.test.ts(boundary): the workflow invokes the capability exactly once after configuring clients; a failed prewarm keeps setupreadyand emits a warning progress event; a host without the capability omits the summary. These hosts pinnodeVersionso readiness does not depend on the developer machine's runtime.Verification
Run locally on Windows 11, Node 24.14.0 (below the repository pin 24.22.1, engine checks relaxed):
tsc --noEmitclean;oxlint0 errors with no new warnings on modified files (the two warnings in the new module mirror the existing control-character sanitization pattern inSetupHost.ts);oxfmt --checkclean;verify:module-boundaries,verify-test-discovery(2424 test files), andverify:architecture-guardspass;knipoutput identical to the pristine tree.servicesproject run shows only the 4 failures inenvironmentBuild.test.tsanddecompileAll.test.tsthat reproduce identically on the pristine tree on this machine (not caused by this change).{"status":"warmed"}in ~1.6 s from a REA source checkout (neutral-cwd fix above); after deleting the pinned_npxentry, prewarm absorbs the 19 s cold install and returns{"status":"warmed"}, leaving nothing to install inside a client startup deadline.Not run locally: the full
checkpipeline on the repository-pinned Node and the built-lane suites; CI coverage for those is expected. Bunbunxwarm-up shares the command derivation but was not executed (no Bun on this machine).Ownership
The PR author is the human owner of this change per the contribution policy.
Author readiness
Human owner: @wxj006007