Skip to content

refactor(runtime-host): split residency kinds and give idle visibility to handshakes - #4771

Merged
me2seeks merged 1 commit into
mainfrom
refactor/4760-residency-idle-handshake
Sep 5, 2026
Merged

refactor(runtime-host): split residency kinds and give idle visibility to handshakes#4771
me2seeks merged 1 commit into
mainfrom
refactor/4760-residency-idle-handshake

Conversation

@me2seeks

@me2seeks me2seeks commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Closes #4760. Sub-issue of #3709; origin findings in this comment (items 3a/3b).

Summary

Residency kinds. Residencies answered two questions with one mechanism: #retainUntilProcessExit() acquired a never-released process-retention residency to block idle exit, but #closeResources() waits for every residency — so a poisoned Host could never close gracefully, always fell through to the shutdownGraceMs deadline, and skipped the cleanup steps ordered after the residency wait. #hasUpgradeBlockingActivity() then had to filter that label back out. The registry now splits drain (work in flight: blocks idle exit, must settle before close) from idle (markers: block idle exit only). The probe reads drainCount with no label filter, and a poisoned Host closes gracefully and releases the State Root writer lease through the ordered path.

Handshake visibility. #isTrueIdle() did not count #handshakingTransports, so after the first accepted connection the idle timer could fire under a connecting Client, and the local-owner true-idle takeover shared the blind spot. The kernel now counts in-flight handshakes, re-evaluates idle when a handshake settles (the failure path previously re-armed nothing), and the takeover excludes only the transport being admitted — otherwise every takeover would observe itself.

Deliberate non-change: the replacement advice

The replacement hint in rejection handshakes still ignores in-flight handshakes. It is advice a stale Client acts on; two racing millisecond probes must not flip each other to blocked_by_residency. The takeover decision counts handshakes; the advice does not. A test caught this distinction (two parallel legacy probes racing), and the difference is documented at both call sites.

Test notes

  • New: registry kind semantics; poisoned-Host graceful close (fails before the fix by waiting out the deadline); in-flight handshake surviving the idle deadline and the idle re-arm after it settles (both halves fail before the fix).
  • Updated: blocks incompatible replacement... now drives the resident-blocked probe with a raw socket instead of connectOrSpawnRuntimeHost — the spawn-capable client left a dying candidate's retries handshaking into the idle window, which is exactly the traffic now visible. The final replacement pair keeps the real client.
  • The old process-exit retention test asserted the deadline-forced, lease-retaining outcome; it now asserts the graceful close with lease release, matching the [Tracking] Runtime Host maintenance and single-writer retirement #3231 retirement contract. Forced termination still applies when real (drain-kind) work hangs.

Verification

  • npm run build; npx biome check on changed files
  • node --test packages/runtime-host/dist/__tests__/*.test.js (1679 passed, 12 platform skips)
  • No protocol/ change: wire shape and epoch untouched.

AI use

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

Tool(s) and scope: Maka assisted investigation, adversarial review, implementation, and tests; the commit includes the required 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 — poisoned Hosts close gracefully and release the writer lease; the idle timer and takeover now see in-flight handshakes.
  • No

…y to handshakes

The never-released process-retention marker blocked graceful close: the
drain waited for every residency, so a poisoned Host always fell through
to the shutdown deadline and never released the writer lease. Residencies
now split into drain (work in flight; blocks idle exit and must settle
before close) and idle (markers that only block idle exit). The
maintenance probe no longer filters the marker label out.

The idle timer and the local-owner true-idle takeover were blind to
in-flight handshakes after the first accepted connection, so the Host
could exit under a connecting Client. #isTrueIdle now counts handshaking
transports, the idle timer re-evaluates when a handshake settles, and the
takeover excludes only the transport being admitted. The replacement
advice in rejections keeps ignoring handshakes: it is what a stale Client
acts on, and millisecond probes must not flip it.

Closes #4760.

Generated-by: Maka
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Sep 4, 2026
@me2seeks
me2seeks requested a review from Astro-Han September 4, 2026 10:45

@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.

Approving at exact head 18769977. No findings. Both checks pass.

The residency split fixes a mechanism that was answering two questions at once

#retainUntilProcessExit() acquired a never-released process-retention residency to block idle exit, while #closeResources() waited on every residency. A poisoned Host therefore could never close gracefully — it always fell through to shutdownGraceMs and skipped the cleanup ordered after the residency wait, including the State Root writer lease release. #hasUpgradeBlockingActivity() then had to filter that same label back out, which is the shape of a mechanism carrying a meaning it was not built for.

Verified on this head:

  • HostResidencyKind = 'drain' | 'idle', and acquire() defaults to 'drain' — so every existing call site keeps its blocking behaviour without being touched. That is the conservative direction for a default, and it means the split cannot silently weaken an unrelated caller.
  • process-retention is the sole 'idle' acquisition (host-kernel.ts:678).
  • Close waits on #drainWaiters only, and the probe reads drainCount with no label filter (:874) — the filter that used to undo the mislabelling is gone rather than moved.

The handshake counting is correct at the edge that usually breaks

#isTrueIdle() now counts in-flight handshakes, and the takeover excludes the transport being admitted — otherwise a takeover would observe itself and never proceed. The exclusion is written defensively:

exceptHandshaking !== undefined && this.#handshakingTransports.has(exceptHandshaking)
  ? this.#handshakingTransports.size - 1
  : this.#handshakingTransports.size

It checks membership before subtracting, so a transport that already settled does not cause an off-by-one against a set it is no longer in. The set is maintained on both paths — added at :406, removed at :408 on failure and :560 on acceptance — so the failure path that previously re-armed nothing now settles the count too.

On the deliberate non-change

Leaving the replacement hint blind to in-flight handshakes is the right call, and the reasoning is worth keeping visible: the takeover decision must count handshakes, while the advice must not, because two racing millisecond probes would otherwise flip each other to blocked_by_residency — and that advice is something a stale Client acts on. A test pins the distinction with two parallel legacy probes, and both call sites document it. Asymmetry between a decision and the advice about that decision is easy to read as an inconsistency later; documenting it at both ends is what stops that.

Tests

The new cases fail before the fix rather than merely covering it: the poisoned-Host close waits out the deadline, and both halves of the handshake case — surviving the idle deadline, and the re-arm after a handshake settles — fail on the old code.

Two changes worth noting because they alter what a test asserts rather than adding one:

  • blocks incompatible replacement… now drives the resident-blocked probe with a raw socket instead of connectOrSpawnRuntimeHost. The spawn-capable client left a dying candidate's retries handshaking into the idle window — which is exactly the traffic this PR makes visible, so the old harness would have been measuring the fix with the fix's own subject.
  • The process-exit retention test asserted the deadline-forced, lease-retaining outcome and now asserts graceful close with lease release. That is the contract changing, not a test being relaxed — the old assertion described the bug. Forced termination still applies when real drain-kind work hangs, so the escape hatch is intact.

No protocol/ change, so wire shape and epoch are untouched.

This is a refactor, so the merge decision remains a human's.

简体中文

18769977 上批准。没有发现问题。 两项检查均通过。

residency 拆分修好的,是一个「用一套机制回答两个问题」的结构

#retainUntilProcessExit() 取得一个永不释放process-retention residency 来阻止 idle 退出,而 #closeResources() 等待的是每一个 residency。于是一个中毒的 Host 永远无法优雅关闭——它总是落到 shutdownGraceMs,并跳过排在 residency 等待之后的清理,包括 State Root writer 租约的释放。随后 #hasUpgradeBlockingActivity() 又不得不把同一个标签过滤回去——这正是「一个机制承担了它并非为之而建的含义」的形状。

在这个 head 上核过:

  • HostResidencyKind = 'drain' | 'idle',而 acquire() 默认是 'drain'——所以每一个现有调用点不必改动就保持原有的阻塞行为。 对一个默认值而言这是保守的方向,也意味着这次拆分不可能悄悄削弱某个无关的调用方。
  • process-retention 是唯一的 'idle' 获取点(host-kernel.ts:678)。
  • close 只等待 #drainWaiters,而探针读的是不带标签过滤的 drainCount(:874)——那个原本用来抵消错误标注的过滤器是被删掉了,而不是被挪走了。

handshake 计数在最容易出错的那条边上是正确的

#isTrueIdle() 现在计入在飞的 handshake,而 takeover 排除掉正在被接纳的那个 transport——否则一次 takeover 会观察到它自己,于是永远不会推进。 这个排除写得有防御性:

exceptHandshaking !== undefined && this.#handshakingTransports.has(exceptHandshaking)
  ? this.#handshakingTransports.size - 1
  : this.#handshakingTransports.size

它在相减之前先检查成员资格,所以一个已经 settle 的 transport 不会对一个它已不在其中的集合造成 off-by-one。这个集合在两条路径上都被维护——:406 加入,失败时 :408 移除、被接纳时 :560 移除——所以此前「什么也不重新武装」的失败路径,现在也会把计数结算掉。

关于那处有意不改

replacement 提示对在飞 handshake 保持无视是正确的,而其中的推理值得保持可见:takeover 的决定必须计入 handshake,而建议必须不计,否则两个毫秒级竞速的探针会把对方翻成 blocked_by_residency——而那条建议是一个过期 Client 会照着做的东西。 一条用两个并行 legacy 探针的测试钉住了这个区分,两处调用点也都有文档。「一个决定」与「关于该决定的建议」之间的不对称,日后很容易被读成不一致;在两端都写下来,正是防止这一点的做法。

测试

新增用例是在修复前会失败,而不只是覆盖了它:中毒 Host 的关闭会一直等到期限;而 handshake 那条的两半——熬过 idle 期限、以及 handshake settle 之后的重新武装——在旧代码上都失败。

有两处值得点出,因为它们改变的是测试断言什么,而不是新增一条:

  • blocks incompatible replacement… 现在用裸 socket 驱动 resident-blocked 探针,而不是 connectOrSpawnRuntimeHost。那个具备 spawn 能力的客户端会让一个垂死候选者的重试在 idle 窗口里 handshaking——而那恰恰是本 PR 让其变得可见的流量,所以旧的 harness 等于在用「修复自己的对象」来测量这次修复。
  • process-exit retention 测试原本断言的是「期限强制、保留租约」的结果,现在断言的是优雅关闭并释放租约那是合同在改变,而不是测试被放宽——旧的那条断言描述的正是那个 bug。 当真正的 drain 类工作挂起时,强制终止仍然适用,所以那条逃生通道完好。

没有 protocol/ 改动,因此线上形状与 epoch 未被触及。

这是一次重构,合并与否仍由人决定。


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

@me2seeks
me2seeks merged commit 1e543a7 into main Sep 5, 2026
2 checks passed
@me2seeks
me2seeks deleted the refactor/4760-residency-idle-handshake branch September 5, 2026 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(runtime-host): split residency kinds and give idle visibility to handshaking transports

2 participants