Skip to content

fix(setup): materialize the pinned package-runner cache before the first client start - #7032

Merged
morluto merged 3 commits into
morluto:mainfrom
wxj006007:fix/setup-prewarm-pinned-runner
Oct 24, 2026
Merged

morluto merged 3 commits into
morluto:mainfrom
wxj006007:fix/setup-prewarm-pinned-runner

Conversation

@wxj006007

@wxj006007 wxj006007 commented Oct 24, 2026 •

Copy link
Copy Markdown

Fixes #7027

Problem

Package runners key their package cache by the requested spec, so the _npx entry installed for the rea-agents@latest invocation that ran setup cannot serve the pinned rea-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 consecutive connection timed out after 30000ms failures, and a controlled cold install with every tarball already in _cacache took 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 --version subcommand) and materializes it once through execFileOutput with a bounded timeout. Direct launcher registrations return skipped. 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.systemSetupHost exposes this as the optional host capability prewarmRegistrationRunner; runSetupWorkflow invokes it after client configuration and skill installation succeed, before the final doctor summary. The step is best-effort and non-fatal: SetupResult carries an optional prewarm summary, 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 setup ready and emits a warning progress event; a host without the capability omits the summary. These hosts pin nodeVersion so 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 --noEmit clean; oxlint 0 errors with no new warnings on modified files (the two warnings in the new module mirror the existing control-character sanitization pattern in SetupHost.ts); oxfmt --check clean; verify:module-boundaries, verify-test-discovery (2424 test files), and verify:architecture-guards pass; knip output identical to the pristine tree.
  • Focused tests: 11/11 across both new files. Full services project run shows only the 4 failures in environmentBuild.test.ts and decompileAll.test.ts that reproduce identically on the pristine tree on this machine (not caused by this change).
  • End-to-end against the built output: warm prewarm returns {"status":"warmed"} in ~1.6 s from a REA source checkout (neutral-cwd fix above); after deleting the pinned _npx entry, 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 check pipeline on the repository-pinned Node and the built-lane suites; CI coverage for those is expected. Bun bunx warm-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

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

…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
@chatgpt-codex-connector

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 🔄 Running since 2026-10-24T18:01:45.826218Z c7b552a 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.

@greptile-apps

greptile-apps Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no blocking issue remains.

Summary

Setup prepares the pinned package-runner cache after applying approved client registrations. Failures remain warnings.

  • Setup warms the pinned package-runner entry before clients start.

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]
Loading

Reviews (3) · Last reviewed commit: "fix(setup): forward terminal interrupts ..." · Reviewed by Greptile

Comment thread src/application/Setup.ts Resolved
Comment thread src/application/Setup.ts Resolved
…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.
@morluto
morluto merged commit 6a96f4f into morluto:main Oct 24, 2026
3 checks passed
@morluto morluto mentioned this pull request Oct 25, 2026
3 tasks done
@morluto

morluto commented Oct 25, 2026

Copy link
Copy Markdown
Owner

Thanks, @wxj006007, for tackling the first-start timeout caused by separate @latest and pinned package caches. Preparing the pinned entry during setup moves that install ahead of the client’s MCP startup deadline, with a warning if it cannot finish. Welcome to REA!

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.

[Bug] First client start after setup times out: pinned npx spec cold-install exceeds the MCP startup deadline

2 participants