Skip to content

fix(cli): wait for Host readiness before pairing - #4230

Merged
Astro-Han merged 2 commits into
apache:mainfrom
orangeCatDeveloper:fix/runtime-host-setup-readiness
Aug 30, 2026
Merged

fix(cli): wait for Host readiness before pairing#4230
Astro-Han merged 2 commits into
apache:mainfrom
orangeCatDeveloper:fix/runtime-host-setup-readiness

Conversation

@orangeCatDeveloper

@orangeCatDeveloper orangeCatDeveloper commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #4228

A fresh managed Runtime Host setup fails on the first attempt with The Runtime Host access service is unavailable, leaves setup pending, and succeeds on an immediate retry.

Root cause: verifyRuntimeHostLifecycleReady only checked host.diagnostics.query, a bootstrap operation the Host answers while its state is still starting. Setup therefore reached pairing before the Host admitted access operations, and access.credential.prepare was rejected with host_not_ready — a plain RPC error, not RuntimeHostAccessUnavailableError, so the pairing retry loop threw immediately rather than waiting out its 10 s budget.

Fix: after the diagnostics identity check, the lifecycle verifier waits for the Host's ready state with the existing waitForRuntimeHostReady helper (the on-demand activation path already does this), within the same readiness deadline. The pairing wait reuses that readiness budget instead of an unrelated 10 s constant, and the unavailable reason (not_registered, connect_failed, …) is carried into the setup error so genuine failures stay diagnosable.

Not covered: the in-flight connection.request inside pairing has no per-request timeout; Desktop's 10-minute setup bound still applies.

Verification

Fresh isolated setup on macOS (launchd), running the same runtime-host setup command Desktop issues, from a dev tarball built before and after the change:

before
 34.96s progress pairing_client
 34.96s error   {"code":"pairing_failed","message":"Runtime Host could not pair the requested Client identity: Runtime Host access service is unavailable"}
exit=1

after
 11.62s progress pairing_client
 11.64s progress verifying_connection
 11.65s complete {"version":"0.2.0-dev-ff2693d05db4","endpoint":"ws://127.0.0.1:58520/runtime-host",...}
exit=0

npm run typecheck and the full packages/cli suite (634 tests) pass. The new lifecycle test drives the verifier with an injected fake Host that reports starting twice before ready; it fails against the previous verifier (actual: [], expected ['starting', 'starting', 'ready']).

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code authored the fix and ran the before/after reproduction; commit carries a Generated-by trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

https://claude.ai/code/session_015tEEyuYS3GuntgpyBbHGqX

Lifecycle readiness only checked host.diagnostics.query, a bootstrap
operation the Host answers while still starting, so the first managed
setup could reach pairing before access operations were admitted and
fail with host_not_ready. Wait for the ready state within the same
readiness budget, align the pairing wait with that budget, and surface
the unavailable reason so genuine failures stay diagnosable.

Generated-by: Claude Code
@github-actions github-actions Bot added the effort/S Under 100 readable lines label Aug 30, 2026
Inject the existing-Host connection into the lifecycle deps so the
verifier can be driven by a fake Host that reports starting before
ready, without spawning a real process.

Generated-by: Claude Code

@hqhq1025 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed commit f8cd4b077cc0f8e111ff21311771868dcb4aa0a5. I found no P0-P3 issues.

The change now waits for the supervised Runtime Host to report ready after the existing PID/root identity checks (packages/cli/src/runtime-host-lifecycle-transaction.ts:725), gives pairing the same 45-second readiness budget while retaining the concrete unavailable reason (packages/cli/src/runtime-host-setup-command.ts:1059, packages/cli/src/runtime-host-access-command.ts:362), and adds coverage for both starting -> ready and timeout behavior (packages/cli/src/__tests__/runtime-host-lifecycle-transaction.test.ts:494).

Validation completed:

  • Hosted test check succeeded on this commit (run 33283512384).
  • The full maka-agent test suite passed locally: 634/634.
  • A conflict-free synthetic merge onto current main (d2346707d65144682d45e905a378ee57be469769) passed the same 634-test suite; git diff --check also passed.

I could not reproduce the original macOS launchd startup timing in this Linux environment, and I did not exercise a deliberately hung in-flight access request. The state-transition and deadline behavior were covered by the tests above.

Codex-assisted review performed under the maintainer-approved review workflow.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I reviewed exact head f8cd4b077cc0f8e111ff21311771868dcb4aa0a5 and found no blocking issue. Lifecycle verification now preserves the existing PID/root identity checks and additionally waits for the same Host connection to report ready within the bounded readiness deadline, preventing first-run pairing from racing startup. Concrete connection-unavailable reasons remain visible, and the new regression covers both starting → ready and timeout behavior. The full CLI suite (634/634), build, typecheck, formatting, ASF header audit, exact-head hosted check, and clean current-main merge pass.


Automated review notice: This comment was posted by an automated review agent operated by WAWQAQ. It is not an independent human review and does not replace one.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing the first-run pairing race through the existing Host readiness seam. Waiting on the Runtime Host's own readiness signal before pairing is the direct solution and avoids introducing a second readiness state.

I found no P0 or P1 issues on exact head f8cd4b077cc0f8e111ff21311771868dcb4aa0a5. One non-blocking P2 remains: the Host can still restart after the readiness check but before the pairing request, and that admission failure is not retried in the same command. The user can safely rerun setup, so this is a recoverable race rather than a merge blocker. Approving.

AI-assisted review: Codex helped inspect the setup composition, readiness authority, race reachability, focused tests, and exact-head CI. I verified the severity and final approval decision.

中文对照

谢谢复用现有 Host readiness 边界修复首次配对竞态。在配对前等待 Runtime Host 自己的就绪信号,是直接解法,也没有引入第二套 readiness 状态。

精确 head 上没有 P0/P1。还有一个不阻塞的 P2:Host 仍可能在 readiness 检查之后、pair 请求之前重启,这次命令不会自动重试该 admission failure;但用户可以安全地重新执行 setup,所以它是可恢复竞态,不是合并阻塞。批准。

本次评审使用 Codex 辅助检查 setup 生产组合、readiness 权威、竞态可达性、针对性测试和精确 head CI;分级和最终批准决定由我确认。

@Astro-Han
Astro-Han merged commit 4c8b37c into apache:main Aug 30, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/S Under 100 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(cli): first managed Runtime Host setup can time out before pairing is ready

4 participants