Skip to content

fix(runtime-host): bound a never-connected ephemeral candidate's lifetime - #3031

Merged
Astro-Han merged 2 commits into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:fix/ephemeral-initial-connection-deadline
Aug 16, 2026
Merged

fix(runtime-host): bound a never-connected ephemeral candidate's lifetime#3031
Astro-Han merged 2 commits into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:fix/ephemeral-initial-connection-deadline

Conversation

@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor

Summary

Fixes#2943.

An ephemeral candidate's initial-connection timeout was implemented only as the true-idle timer (#scheduleIdleIfNeeded), which never arms while composition startup is still pending or any residency is held. A candidate whose composition startup hung or that held boot-time residencies before any Client ever connected therefore outlived --initial-connection-timeout-ms indefinitely — the orphaned execution-candidate-main.js processes in #2943 survived 26+ hours.

What changed

  • RuntimeHostKernel now arms a dedicated first-connection deadline for ephemeral lifecycles. If no handshake has been accepted when it fires, the kernel requests a drain, which already carries the bounded fail-stop shutdown deadline (shutdownGraceMsRuntimeHostProcessTerminationRequiredErrorprocess.exit(1)), so the candidate's lifetime is bounded regardless of what the composition is doing.
  • The deadline is cancelled by the first accepted connection, so a Client waiting through a slow recovery is unaffected (Clients handshake before polling waitForRuntimeHostReady).
  • A handshake in flight defers the deadline by the handshake budget instead of draining under a connecting Client — bounded to 3 deferrals, so a reconnect loop that never completes a handshake cannot push the deadline out indefinitely.
  • The deadline is armed only after #compositionStartup is assigned: an earlier fire would drive #closeResources past an undefined startup await and let shutdown complete without closing the composition created afterwards.

Notes on the issue's analysis

  • #armShutdownDeadline() is already called from #requestDrain() (since feat(runtime-host): bound shutdown with process fail-stop #1355), so the poison path (retainUntilProcessExit + requestDrain) was already bounded; suggested direction 2 was in place.
  • process-retention deliberately blocks the clean-close path so a poisoned host fail-stops instead of pretending a clean shutdown; excluding it from #isTrueIdle() (direction 1) is unnecessary once the pre-connection lifetime is bounded, and would change post-connection semantics.
  • The sustained 70–82% CPU is consistent with a hung/spinning composition startup (state never leaves recovering, so neither the idle timer nor any drain existed to stop it). This change does not diagnose the spin, but it bounds it: the process now fail-stops after initialConnectionTimeoutMs + shutdownGraceMs.

Tests

  • New: never-connected ephemeral candidate drains after the initial connection timeout despite a boot residency
  • New: never-connected ephemeral candidate with a hung composition startup fails stop at the deadlines
  • Full @maka/runtime-host suite: 935 pass.

🤖 Generated with Claude Code

…time
An ephemeral candidate's initial-connection timeout was implemented only
as the true-idle timer, which never arms while composition startup is
still pending or any residency is held. A candidate whose composition
startup hung or that acquired boot-time residencies before any Client
ever connected therefore outlived --initial-connection-timeout-ms
indefinitely (observed as orphaned execution-candidate-main.js processes
surviving for 26+ hours).
Arm a dedicated first-connection deadline as soon as the listeners are
up: if no handshake has been accepted when it fires, the kernel requests
a drain, which already carries the bounded fail-stop shutdown deadline.
The deadline is cancelled by the first accepted connection and defers
itself by the handshake budget while a handshake is in flight, so slow
recovery under a waiting Client is unaffected.
Fixesapache#2943

@Astro-HanAstro-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 unbounded lifetime of never-connected ephemeral candidates. The kernel is the right owner for this deadline, and the two new tests cover the boot-residency and hung-startup paths well.

I found one lifecycle overlap that should be resolved before merge:

[P2] Keep the pre-connection lifetime under one timer authority

After the composition reaches ready, #scheduleIdleIfNeeded() still arms the existing idle timer with initialConnectionTimeoutMs while no connection has yet been accepted. The new initial-connection deadline is already governing that same period.

This defeats the new handshake deferral in the ordinary no-residency path:

  1. The initial-connection deadline fires while a handshake is in progress and defers by handshakeTimeoutMs.
  2. The old idle timer then fires independently.
  3. #isTrueIdle() does not account for handshaking transports, so it drains the Host underneath the connecting Client.

The new tests do not expose this because their boot residency or hung startup prevents the ready-state idle timer from being armed.

The smallest complete fix is to make #scheduleIdleIfNeeded() return until the first connection has been accepted, then use only idleGraceMs. That leaves one authority per lifecycle phase:

  • before the first accepted connection: #initialConnectionDeadline, including bounded handshake deferral;
  • after the first accepted connection: #idleTimer with idleGraceMs.

Please add a regression test with a ready, residency-free Host and a silent handshake transport, verifying that it is not drained before the deferred deadline. I would keep the two existing tests; they exercise distinct and useful shutdown paths.

中文对照

感谢修复从未连接的 ephemeral candidate 可能无限存活的问题。deadline 放在 Runtime Host kernel 是正确的,新增的两条测试也分别覆盖了 boot residency 和启动永久挂起。

这里还有一个应在合并前解决的生命周期重叠:

[P2] 首次连接前只保留一个定时器 authority

composition 进入 ready 后,#scheduleIdleIfNeeded() 仍会在尚未接受连接时用 initialConnectionTimeoutMs 启动旧 idle timer;新增的 initial-connection deadline 已经在管理同一个阶段。

这会让新增的 handshake 延期在普通、无 residency 的路径上失效:新 deadline 因正在握手而延期后,旧 idle timer 仍会独立触发;而 #isTrueIdle() 不检查 handshaking transport,于是会在 Client 正在连接时 drain Host。

最小完整修复是:首次连接成功前不启动 idle timer;首次连接后才由 idleGraceMs 管理空闲退出。这样首次连接前后各自只有一个生命周期 owner。

建议补一条 ready、无 residency、存在静默 handshake 的回归测试。现有两条测试覆盖不同的 shutdown 路径,应当保留。

Disclosure: I used Codex reviewers and Claude Opus to assist with lifecycle tracing and adversarial checks. I reviewed the evidence and own this feedback.

Review follow-up: after ready, #scheduleIdleIfNeeded still armed the idle
timer with initialConnectionTimeoutMs while no connection had been
accepted, racing the dedicated deadline. #isTrueIdle() cannot see an
in-flight handshake, so on the ordinary no-residency path the idle timer
would drain the Host underneath a connecting Client that the deadline had
deliberately deferred for.
The idle timer now arms only once the first connection has been accepted
and always uses idleGraceMs; before that, #initialConnectionDeadline —
including its bounded handshake deferral — is the only authority.
Regression test: a ready, residency-free Host with a silent handshake
transport survives past the initial connection timeout and drains only
after the deferral.
@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
ContributorAuthor

Addressed in 1cac184. #scheduleIdleIfNeeded() now returns until the first connection has been accepted and always uses idleGraceMs, so each lifecycle phase has exactly one timer authority as you described: #initialConnectionDeadline (with its bounded handshake deferral) before the first accepted connection, #idleTimer after.

Added the requested regression test (a silent handshake defers the never-connected drain instead of being drained under it): a ready, residency-free Host with a silent handshake transport is still ready past initialConnectionTimeoutMs, and drains only after the deferral once the transport goes away. Verified it fails on the previous commit (drained at ~500ms under the pending handshake) and passes now. Both earlier tests kept; full @maka/runtime-host suite: 936 pass.

@Astro-HanAstro-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 the follow-up. I rechecked 1cac1843a against the current main.

The previous P2 is resolved: before the first accepted connection, #initialConnectionDeadline is now the sole lifetime authority; after that transition, #idleTimer owns the normal idle grace period. The silent-handshake regression exercises the overlap that prompted the earlier feedback.

I found no remaining P0–P3 issues. The three new tests cover distinct lifecycle paths and are worth keeping; I do not see code or test deletion, or a refactor, that would make this materially simpler without weakening the invariant.

I also verified the automatic merge result with the current main: the build completed successfully and host-kernel.test.ts passed 45/45. From the code-review perspective, this is a go.

One delivery item before merge: please ensure the final squash commit carries the required Generated-by: Claude Code trailer. The PR body discloses the tooling, but the current commits do not contain the trailer.

中文对照

我重新检查了 1cac1843a,并与当前 main 的自动合并结果一起做了验证。

之前的 P2 已解决:首次连接成功前只有 #initialConnectionDeadline 管理生命周期;连接成功后才由 #idleTimer 管理正常空闲退出。新增的 silent-handshake 测试也准确覆盖了之前指出的竞态。

没有发现剩余的 P0–P3。三条新增测试覆盖不同生命周期路径,均应保留;目前也没有可以删除或通过重构进一步简化、同时又不削弱生命周期约束的代码。

当前 main 与该 PR 的合并结果构建成功,host-kernel.test.ts 45/45 通过。从代码审查角度可以继续。

合并前还有一项交付要求:最终 squash commit 需要包含 Generated-by: Claude Code trailer。PR 正文已有工具披露,但当前两个提交都没有该 trailer。

Disclosure: Codex assisted this re-review with lifecycle tracing, current-main merge validation, and focused test execution. This does not replace the repository’s required independent human review.

@Astro-Han
Astro-Han merged commit 91c54fc into apache:mainAug 16, 2026
10 checks passed
Sign up for freeto 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.

fix(runtime-host): ephemeral candidate never exits after retainUntilProcessExit disables the initial connection timeout

2 participants

@UncertaintyDeterminesYou4ndMe@Astro-Han