Uh oh!
There was an error while loading. Please reload this page.
test(runtime-host): drive liveness-crossing waits from an injected probe cadence - #2450
Conversation
…obe cadence Part of apache#2389. Two tests held work pending for a fixed 2.1s purely to outlive the client connection's hardcoded 2s liveness interval: - the Agent-graph fake authority's slow stop() (the fake-shutdown case named in the issue), proving agent.graph.stop survives probe cycles; - host-kernel's slow-domain-work test, holding an admitted request pending across a probe before releasing it. The interval is now injectable (ConnectRuntimeHostInput.livenessIntervalMs, default unchanged at 2s), so both tests measure their probe-crossing waits in an injected 100ms unit instead of wall-clock guessing against a constant they cannot see. The waits are derived (2 cycles + margin), not scheduler-load guesses, and the contract — probes never retire a request that has no explicit deadline (apache#2392) — is exercised identically. The remaining short sleeps in this workspace are all poll intervals inside bounded wait loops, which are already observable completion; they are untouched. Focused timing: the two affected tests drop from ~2.5s each to ~0.37s each (node --test, local). Full workspace suite: 734/734 pass after a clean build.
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for this. The injected cadence matches the existing connectTimeoutMs/handshakeTimeoutMs pattern, default behavior is unchanged (2s), and both tests are genuinely faster (430/450ms). The assertions still bite: if #2392 regresses, the pending request gets retired and the deepEqual fails.
One gap: the tests can't tell "probes fired and the request survived" from "probes never fired". If a future change stops using livenessIntervalMs in the constructor (types still compile), every assertion passes with zero probes in the window. A probe counter on the server side, asserted before release, would make the premise falsifiable. Cheap to add.
Nit: livenessIntervalMs is validated in the constructor, after connect + handshake, so a bad value surfaces as "handshake_failed" instead of a config error; the other timeouts validate up front. Approving.
…dence took Review follow-up on the injected liveness cadence: the tests could not tell "probes fired and the request survived" from "probes never fired" — if a future change stopped threading livenessIntervalMs, every assertion would still pass with zero probes in the window. ConnectRuntimeHostInput gains onLivenessProbe, invoked after a probe round-trips and validates its Host Epoch. Both tests now gate on two observed probe round-trips while their long-lived request is pending — the host-kernel test releases its admitted request only after the crossing (bounded, loud timeout if the cadence stops taking effect), and the Agent-graph fake's stop() awaits the crossing instead of a derived sleep, making the ordering causal with no fixed timing at all. Also validates livenessIntervalMs up front in connectResolvedRuntimeHost alongside the other connect timeouts, so a bad value is a config error instead of surfacing as handshake_failed. Both tests get faster again (~0.29s / ~0.33s); 734/734 pass after a clean build.
UncertaintyDeterminesYou4ndMe
commented
Aug 8, 2026
Both points addressed in bad4d15:
Both tests got faster again (~0.29s / ~0.33s); 734/734 after a clean build. |
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for the follow-up. Both review points are substantively addressed:
- the tests now wait for observed, epoch-validated probe round-trips instead of assuming the injected cadence fired;
livenessIntervalMsis validated before transport and handshake work, consistently with the existing timeout options.
The resulting tests are faster and their probe-crossing premise is now falsifiable.
P3 — Test observability is exposed through the public client input.
onLivenessProbe is documented as a test hook but is now part of the exported ConnectRuntimeHostInput. Its callback also runs inside the probe success chain, so an observer exception can fail the connection.
This is non-blocking because existing callers do not pass it and default production behavior is unchanged. Longer term, I would prefer observing probes through a package-internal test seam or Host/transport fixture. If the callback remains a public diagnostics hook, its exceptions should be isolated from connection health.
Small test-scoping suggestion: in the Agent Graph test, pass the probe observer only to the final TUI connection that issues agent.graph.stop. That makes it impossible for an earlier slow query on another connection to satisfy the shared counter.
The requested follow-up is effective, and I found no blocking issue. Approved.
简体中文
感谢继续修改。上一轮的两个问题都已经得到实质解决:
- 测试现在等待实际观察到、并完成 Host Epoch 校验的 probe round-trip,不再假设注入的 cadence 已经触发;
livenessIntervalMs现在与现有 timeout 配置一样,在 transport 和 handshake 之前完成校验。
测试变得更快,而且 probe-crossing 前提现在可以被证伪。
P3 — 测试观测能力进入了公开 client input。
onLivenessProbe 的文档明确说明它是 test hook,但它现在属于公开导出的 ConnectRuntimeHostInput。callback 还运行在 probe success chain 内,因此 observer 抛出的异常可能导致连接失败。
这是非阻塞问题,因为现有调用方不会传入它,默认生产行为也没有变化。长期来看,更适合通过 package-internal 测试 seam 或 Host/transport fixture 观察 probe。如果保留为公开 diagnostics hook,则 observer 异常不应影响连接健康。
还有一个小的测试作用域建议:Agent Graph 测试只给最终执行 agent.graph.stop 的 TUI connection 传入 probe observer。这样前面其他 connection 上的慢查询就不可能提前满足共享 counter。
本轮修改有效,没有发现阻塞问题。Approve。
Review follow-up (non-blocking P3): onLivenessProbe is a diagnostics hook, so an exception thrown by the observer is now swallowed instead of running inside the probe success chain where it would fail the connection it is watching; the input doc says so explicitly. Also scopes the Agent-graph test's observer to the one connection that issues agent.graph.stop, so no other connection's slow query could ever satisfy the shared probe counter.
Uh oh!
There was an error while loading. Please reload this page.
UncertaintyDeterminesYou4ndMe
commented
Aug 8, 2026
Both follow-ups applied in fc78317:
Suite green after a clean build; typecheck and biome pass. |
Part of #2389 (Runtime Host workspace; the Pi TUI workspace landed as #2448, desktop e2e and alignment audit follow separately).
What changed
Two tests held work pending for a fixed 2.1s purely to outlive the client connection's hardcoded
DEFAULT_LIVENESS_INTERVAL_MS = 2_000:agent-graph-two-client-uds.test.ts—FakeAgentGraphAuthority.stop()slept 2.1s (the fake-shutdown case named in the issue) so the pendingagent.graph.stoprequest would cross a liveness probe cycle.host-kernel.test.ts(slow domain work preserves multiplexed requests and retires only explicit deadlines) — the test slept 2.1s while holding an admitted request open before releasing its gate.Both waits existed to prove the #2392 contract: liveness probes never retire a request that has no explicit deadline. The 2s value is incidental to that contract — what matters is that at least one probe fires while the request is pending.
The probe interval is now injectable:
ConnectRuntimeHostInput.livenessIntervalMs(validated by the samerequireTimeoutas the existing timeout options, default unchanged at 2s), threaded throughconnectResolvedRuntimeHostinto the connection's#scheduleLivenessCheck. Both tests inject a 100ms cadence and derive their waits from it (2 cycles + margin) — the wait is now measured in a unit the test controls instead of guessed against a constant it cannot see.Retained contracts
DEFAULT_LIVENESS_TIMEOUT_MS), handshake/connect deadlines, and the explicit 50msread_timeoutcase in the host-kernel test are untouched.Production surface
One additive optional field on
ConnectRuntimeHostInput; passing nothing preserves today's behavior exactly.Timing (local, node --test)
Full workspace suite after a clean build: 734/734 pass.