fix(runtime-host): escalate reconnect delay while a Host never stabilizes

[me2seeks]

Mitigates #3458

Problem

When a Runtime Host dies immediately after spawn (transport read_eof on every attempt), the reconnect lifecycle retried roughly every maxMs (5s by default) forever. Each retry re-ran the full desktop registration stack — new candidate process, IPC handlers, subscriptions, capability publisher bind. ~12 hours of this churn produced hundreds of candidate generations and ended in an Electron main-process OOM (heap pinned at ~3.6 GB), with the log dominated by repeated MCP capability alignment failed errors.

Change (mitigation, not the OOM root fix)

This change bounds the churn that amplifies the OOM; it does not fix the retained root behind it.

RuntimeHostReconnectBackoff gains an optional unstableMaxMs ceiling (default 60s, validated >= maxMs; when omitted it derives from max(60s, maxMs) so a previously valid large maxMs keeps working). While the failure streak persists (no connection stays installed for stableConnectionMs, which already resets the streak), the jittered delay now keeps doubling past maxMs up to that ceiling instead of saturating at maxMs. As soon as a connection stabilizes, the ladder restarts from minMs.

• Worst-case regeneration cadence during a persistent startup-death loop drops from ~every 5s to ~every 60s (~12x less churn of processes and desktop-side registration).
• Callers that need the old flat ceiling can set unstableMaxMs equal to maxMs; callers with minMs: 0 are unaffected.
• Backwards compatibility: configurations without unstableMaxMs but with maxMs > 60s are preserved — the ceiling derives from maxMs instead of rejecting them.
• No behavior change for short transient outages: escalation only engages after the exponential ladder would have saturated anyway.

What this deliberately does not do

The retention root behind the OOM still needs a heap snapshot under reproduction. If cross-generation retention persists, this patch delays the same OOM rather than bounding it — the escalation reduces regeneration frequency ~12x, which converts an hours-scale OOM into a days-scale one at worst. A real bounded/circuit-breaking ownership rule or the retained-root fix should land as a follow-up before #3458 closes.

Testing

• New tests in reconnecting-connection.test.ts: delay escalates past maxMs and saturates at unstableMaxMs; a stabilized connection restarts the ladder; unstableMaxMs < maxMs is rejected; an omitted unstableMaxMs stays compatible with a large maxMs (the compatibility case from review).
• Full @maka/runtime-host suite green on the rebased head.

Follow-ups (not in this PR)

• Heap snapshot under reproduction to find and fix the retained root (the actual Desktop main process OOM after prolonged Runtime Host connection flapping (repeated MCP capability alignment failures) #3458 close).
• Circuit-breaking / bounded ownership rule if retention proves unavoidable during flapping.
• Surface diagnostics when candidates die at startup so the flapping root cause (read_eof origin) is observable.

AI use

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

Tool(s) and scope: Maka (AI coding agent) authored the implementation and tests; the diff was human-reviewed before push. Generated-by: Maka trailers are present on the branch commits (the tip commit was missing its trailer on the previously reviewed head — now added).

中文说明

本 PR 是对 #3458 的缓解而非根因修复：持续启动即死的 Host 触发的重建风暴从约每 5 秒一次降级到最多每 60 秒一次（新增 unstableMaxMs 上限，默认 60s 且省略时从 max(60s, maxMs) 派生，保证旧的大 maxMs 配置兼容）。OOM 的保留根因仍需堆快照定位；若保留仍在，此补丁只是把小时级 OOM 推迟到天级，#3458 的关闭需要后续的边界/熔断修复。已按 review 意见补充 maxMs 兼容回归测试，tip commit 补上 Generated-by: Maka trailer。

[jackwener]

Automated review of exact head 6e323981ddd625ca6eab0ee3aa70ad3be83ad677 against main@f1f4e71a9540a4da23159052c924fee72202e989.

Two actionable blockers:

1. Existing valid maxMs configurations now throw. requireDelay allows maxMs through 120000, and before this change a caller could set (for example) { minMs: 100, maxMs: 90000 }. Lines 116–123 default unstableMaxMs to 60000 and then require it to be at least maxMs, so that previously valid call now rejects during construction even though the caller never opted into the new field. When unstableMaxMs is omitted, derive it as Math.max(DEFAULT_UNSTABLE_MAX_MS, this.#maxMs) (or otherwise preserve the old valid range), and add the missing compatibility test.

2. This does not fix the issue it closes. #3458 requires that prolonged flapping not exhaust the main-process heap and calls for backoff escalation / circuit-breaking instead of unbounded regeneration. This patch only changes the unbounded loop from roughly once per 5 seconds to once per 60 seconds; if the observed cross-generation retention remains, it delays the same OOM rather than bounding it. The PR itself says the retention root still needs investigation. Either re-scope this as a mitigation (Refs #3458, keep #3458 open) or add a real bounded/circuit-breaking ownership rule or fix the retained root before using Fixes #3458.

Required conclusions:

1. Optimal for the actual problem: no for the OOM claim; it is a useful churn mitigation, not the root fix.
2. Production code to delete: none identified.
3. Tests to delete/replace: none; add the omitted-unstableMaxMs / large-maxMs compatibility case.
4. Deeper refactor: not required if the PR is honestly narrowed to mitigation; the retention/circuit-break owner still needs a separate completed fix.
5. Ready to merge: no on this head; both points above need resolution, required test has not run, and the PR also omits the CONTRIBUTING-required AI-use declaration (state explicitly “none” if none was used).
6. Residual risks: the flapping root and retained heap owner remain unknown; the new cadence still retries forever.

This changes exported reconnect behavior and user-visible recovery timing. Independent human review is required; this automated review is not approval.

[me2seeks]

Both blockers addressed at 5f24d20a4 (rebased onto current main).

1. maxMs compatibility. When unstableMaxMs is omitted, the ceiling now derives as Math.max(DEFAULT_UNSTABLE_MAX_MS, maxMs) instead of defaulting to a hard 60s and then rejecting — so { minMs: 100, maxMs: 90_000 } keeps working exactly as before the change. The explicit unstableMaxMs < maxMs rejection is unchanged for callers that opt in with an inconsistent pair.

New compatibility test: an omitted unstableMaxMs stays compatible with a large maxMs — drives { minMs: 100, maxMs: 90_000 } (no unstableMaxMs) through a persistent-failure loop and asserts the ladder escalates past the old 60s default and saturates at the derived 90s ceiling, i.e. the derived ceiling is honored, not just tolerated at construction.

2. Scope. Agreed — this bounds the churn that amplifies #3458; it does not bound or fix the retained root. Re-scoped: the PR now says Mitigates #3458, adds a "What this deliberately does not do" section stating plainly that if cross-generation retention persists, this delays the same OOM (hours-scale → days-scale at worst) rather than bounding it, and moves the heap-snapshot/circuit-breaking work to Follow-ups so #3458 stays open for the real fix. The commit message carries the same framing.

AI declaration: the tip commit was missing its Generated-by: trailer on the previously reviewed head — added now (Generated-by: Maka), and the body's AI-use section notes the correction.

Local verification on the rebased head: full @maka/runtime-host reconnect suite green (13/13 in reconnecting-connection.test.ts, including the new case), package builds clean after a fresh dependency install.

中文说明

两个 blocker 均已在 5f24d20a4 处理：(1) 省略 unstableMaxMs 时上限改为从 max(60s, maxMs) 派生，旧的大 maxMs 配置不再报错，并补了兼容性回归测试（验证阶梯实际爬升到派生的 90s 上限）；(2) 按意见把 PR 重定性为对 #3458 的缓解——正文明确说明若保留根因仍在，本补丁只是把小时级 OOM 推迟到天级，堆快照定位与熔断方案移入 Follow-ups，#3458 保持开放；tip commit 补上缺失的 Generated-by: trailer。

[Astro-Han]

[P1] maxMs no longer bounds anything — the two ceilings are folded into one.

const ceiling = Math.max(maxMs, unstableMaxMs);

The constructor already validates unstableMaxMs >= maxMs (and derives it as Math.max(60_000, maxMs) when omitted). So Math.max(maxMs, unstableMaxMs) is identically unstableMaxMs — maxMs is dead in the delay computation and survives only as a validation input. There is no streak-gated second phase here; there is one ladder that climbs straight to unstableMaxMs.

This is the exact-head CI failure. apps/desktop/src/main/__tests__/runtime-host-desktop-manager.test.ts:585-587 configures minMs: 100, maxMs: 150 and asserts the ladder:

actual:   [ 100, 200 ]
expected: [ 100, 150 ]

The second attempt jumps past the configured maxMs instead of clamping to it.

Why it contradicts the PR description. The PR says escalation happens "while the failure streak persists" and "only engages after the exponential ladder would have saturated anyway." The saturation point is maxMs — so for any caller whose maxMs is close to minMs, the ladder never has an unchanged window at all. The desktop test is exactly that caller.

Reach, stated honestly: I grepped for production callers of reconnectBackoff — runtime-host-desktop-manager.ts:566 is the only one, and it forwards an optional config that no production site sets. So today the observable blast radius is the defaults (100 / 5_000 / 60_000), where the first six rungs are unchanged and the intended 12x churn reduction lands as designed. The regression is in the contract, not yet in shipped behavior — but the contract is public API on RuntimeHostReconnectBackoff, and the test that broke is the one documenting it.

Two ways out, and they are genuinely different:

1. Keep maxMs as the stable-phase ceiling and gate escalation on the streak — clamp to maxMs until the streak passes some threshold, then let it climb to unstableMaxMs. This is what the PR description already claims the code does, and it keeps the existing desktop test green untouched.
2. Accept that maxMs is now just the floor of the ceiling — then say so: rename or re-document it, drop the no-op Math.max, and update the desktop test deliberately rather than letting it fail.

I would not pick option 2 silently. The desktop test failing is the codebase telling you the parameter meant something to somebody.

[me2seeks]

Confirmed locally before fixing — reproduced actual: [ 100, 200 ] / expected: [ 100, 150 ] on the exact head, and verified the two claims behind it: the \[100, 150\] desktop assertion is pre-existing on main (untouched by this PR), and the folded ceiling is identically unstableMaxMs given the constructor invariant.

Fixed at 6d8a2eb26: the regular ladder clamps at maxMs again; escalation toward unstableMaxMs engages only once the natural doubling leaves the ceiling band (exponential >= 2 * maxMs) — i.e. after the ladder has genuinely saturated. The one assertion that had encoded the fold ([3, 6], added by this PR) now pins the plateau ([3, 5], next delay > 5). Desktop manager suite passes unmodified; runtime-host reconnect suite 13/13; biome + typecheck clean.

[Astro-Han]

Independent review of 5f24d20a41a7db3ec3e3e460659dcee0e84faf45. One [P1], posted inline on the folded ceiling: #3462 (comment)

Short version: const ceiling = Math.max(maxMs, unstableMaxMs) is identically unstableMaxMs, because the constructor already validates unstableMaxMs >= maxMs. maxMs therefore stops bounding the delay at all, and the exact-head test job fails on it — apps/desktop/src/main/__tests__/runtime-host-desktop-manager.test.ts expects [100, 150] and gets [100, 200].

What I want to be fair about: I grepped the tree for production callers that set maxMs. There are none — runtime-host-desktop-manager.ts:566 forwards an optional config that no production site populates. On the defaults (100 / 5_000 / 60_000) the first six rungs are byte-identical to before and the ~12x churn reduction lands exactly as the PR describes. So this is a broken contract and a red build, not a shipped behavior regression. I would rather say that plainly than inflate it.

On the PR as a whole: the framing is the best part of it. Calling this a mitigation for #3458 and writing out — in the PR body — that the retained root still needs a heap snapshot, and that without it this converts an hours-scale OOM into a days-scale one, is the kind of honesty that makes the change reviewable. The follow-up list is concrete rather than decorative. No objection to the direction at all.

The inline comment lays out two ways to resolve it; they lead to different APIs, so it is worth picking deliberately rather than just making the test match the code.

Reviewed at 2026-08-23 12:15 UTC. No P0, no P2, no P3 beyond the above.

[Astro-Han]

Approving 6d8a2eb26618c35dafabdca55a278ccb9cf04ef7. Required test is completed / success bound to that exact SHA. No P0–P2.

Re-review at the current head; the CHANGES_REQUESTED on this PR is bound to the older 6e323981, so every earlier finding was re-derived rather than assumed closed.

The [P1] is closed, and the fix is better than the one that was asked for. The complaint was that Math.max(maxMs, unstableMaxMs) made maxMs inert. The ceiling is now selected rather than merged:

const ceiling = exponential >= 2 * maxMs ? unstableMaxMs : maxMs;

So maxMs still governs the ordinary ladder, and the extended ceiling only takes over once natural doubling has already carried the delay past 2 * maxMs — which is the point at which "this Host is not coming back soon" has actually been demonstrated rather than assumed. A configured maxMs keeps its meaning instead of being silently widened.

Also closed: the compatibility case when unstableMaxMs is omitted with a large maxMs; the AI-use trailer; and the description's claim, which now says Mitigates rather than Fixes #3458 and names the retained-root and circuit-breaker work as follow-ups. That last one is worth calling out — a PR that narrows its own claim to what it actually does is doing the reader a favour.

The increment since the earlier reviewed head is 2 files, +10/−4. It does not touch retry authority, state transitions, failure classification or cancellation.

Two [P3]s, neither blocking:

• The exported JSDoc still says omitting unstableMaxMs defaults to 60s, but the code uses Math.max(60_000, maxMs) (reconnect-lifecycle.ts:136). With a maxMs above 60s the documented default is not what a caller gets.
• The "stabilized connection restarts the ladder" test does not actually pin the reset — delete the production failureCount = 0 and it still passes. Worth strengthening while the reasoning is fresh.

Recorded rather than escalated: the 12-hour Windows OOM, the retained heap root, and a genuinely bounded/circuit-breaker design are not verified here. The PR does not claim them, which is why this is a note and not a finding.

Disclosure, because it changes what this approval is worth: this is an AI review. Under CONTRIBUTING.md §Review it does not count as the required independent human review. Note also that the existing CHANGES_REQUESTED remains on the PR and gates merge independently of this approval.

Resolved on exact head 6d8a2eb after Kabi incremental re-review: omitted unstableMaxMs now derives max(60s, maxMs) with a 90s compatibility regression test; the PR now says Mitigates #3458, keeps #3458 open, and explicitly scopes retained-root/circuit-breaker work as follow-up; AI declaration/trailers are present. Exact-head test completed/success. Dismissing only this old-head review; current P3 documentation/test-strength notes remain nonblocking.