Uh oh!
There was an error while loading. Please reload this page.
fix(runtime-host): give owned-candidate connect a CI-safe election window - #3221
Conversation
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review. 📝 WalkthroughWhat problem this solvesThis PR reduces CI timing flakes in the Runtime Host owned-candidate connection test.
Source of truthThe PR extends the existing owned-candidate connection, election, and launch-cleanup paths. It does not create a parallel connection path or add a public API. Scope and simplificationThe changes are the smallest coherent solution for the reported timing failure and late-startup behavior. Asynchronous The diagnostic formatter, polling helper, and regression tests preserve failure details and cover the timing cases. No deletion or simplification is apparent without weakening diagnostics or regression coverage. Validation and risksFocused verification passed: 7/7 tests in The tests cover the longer election window, concrete failure reporting, unresolved candidates, and late candidate adoption. Full-suite validation is not reported, so required checks remain unverified. Complexity delta
Total maintenance complexity stays justified. The added branches and tests directly cover the reported failure modes. Review-relevant risksThe connection behavior changes when No security, licensing, release, or governance effect was identified in the current diff. Required checks remain unverified beyond the reported focused test result. The person performing the merge reviews the final diff. A maintainer makes the final determination. WalkthroughThe runtime-host client now classifies owned host startup failures and distinguishes launch failures from existing-host outcomes. Tests cover startup timeouts, late candidates, election timing, cleanup, and connection diagnostics. ChangesOwned runtime-host connection handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk:⚪ Minimal · up to This localized change widens the owned-candidate election window and makes missed-election failures diagnosable; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoStabilize owned-candidate election handling under CI load
AI Description
Diagram
High-Level Assessment
Files changed (2) |
Code Review by Qodo
1. |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 92e25a18-7c46-472b-b5f2-0caa3a90d1eb
📒 Files selected for processing (2)
packages/runtime-host/src/__tests__/owned-candidate.test.tspackages/runtime-host/src/client/connect-or-spawn.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Astro-Han
left a comment
There was a problem hiding this comment.
Returning at the election deadline instead of awaiting a never-settling spawn is the right goal, but the new fire-and-forget cleanup transfers an unsafe kill decision to the timed-out client. A late candidate can become the shared State Root host and accept another client before this cleanup fires; settle(1_000) then kills that live host. The owner of the process must decide whether it is idle, rather than a client that already lost the election. The regression test also needs to cover a candidate that resolves late, not only one that rejects.
AI-assisted review: Codex coordinated two independent reviewer passes and an OpenCode Go DeepSeek V4 Flash high-effort adversarial pass. I verified the exact head f944c30984deeeff827db3871f3cf122c4c249de, the launcher kill behavior, the State Root connection path, and current CI. No local tests were run in this review.
中文审查
不再等待永不结束的 spawn、按 election deadline 返回，这个目标正确；但新的后台清理把危险的 kill 决策交给了已经超时的客户端。迟到 candidate 可能已经成为同一 State Root 的共享 Host，并接入另一个客户端，随后仍会被 settle(1_000) 杀死。进程是否空闲应由 Host 自身判断，而不是由已经输掉选举的客户端决定。回归测试也需要覆盖 candidate 迟到但成功 resolve 的路径，而不只是 reject。
本次为 AI 辅助审查：Codex 协调两轮独立 reviewer 审查及 OpenCode Go DeepSeek V4 Flash high 对抗性复核，并核验了精确 head、launcher kill 行为、State Root 连接路径和当前 CI。本轮未运行本地测试。
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/runtime-host/src/__tests__/owned-candidate.test.ts (1)
301-317: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsolidate the duplicate connection-result formatters.
ownedConnectFailureandadoptedConnectFailurehave identical branches. Use one helper with a union of the two result types. This removes duplicate maintenance when result kinds change.Disposition: optional.
As per path instructions, “Flag concrete cases where code can be deleted or simplified.”
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e4a9d78f-8cc3-422b-a298-18cdecac0b9d
📒 Files selected for processing (2)
packages/runtime-host/src/__tests__/owned-candidate.test.tspackages/runtime-host/src/client/connect-or-spawn.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/runtime-host/src/client/connect-or-spawn.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 0 remain after this review.
Uh oh!
There was an error while loading. Please reload this page.
…ndow The promptness claim is Host exit after the first connection closes, not the connect itself. A two-second election misses spawn plus handshake under a full suite and only reports `failed`. Widen the window, keep the concrete reason on the assertion, and map a deadline throw to startup_timeout. Fixesapache#3190
Election already bounds candidate startup. Awaiting launch.spawned after a non-connected result hangs connectOwned when the candidate never reports. Return that result immediately and settle a late host in the background. Fixesapache#3190
A timed-out owned connect no longer settles a late candidate. That path sent SIGKILL after 1s and could terminate a host another client had already adopted. Release the process and let idle-grace / initial-connection policy exit it when unused. Fixesapache#3190
…tling The adopt regression only checked that the late pid stayed alive. Wrap the delivered candidate so a settle() cleanup cannot pass that check and kill the adopted host after the assertion. Fixesapache#3190
95e0072 to
8ca108eCompareAstro-Han
left a comment
There was a problem hiding this comment.
The current head resolves the earlier ownership race correctly. A missed election now returns at its own deadline, preserves the specific failure reason, and releases a late owned candidate to the Runtime Host lifecycle instead of killing a process another client may have adopted. The implementation extends the existing OwnedCandidateAttempt seam and does not create another election authority.
I found no reproducible P0–P3 issue on this head. I also checked two adversarial candidates and rejected them: root ownership prevents the pid-mismatch candidate from simultaneously being the adopted authority, and the test's deliberately unresolved spawn promise occupies the first election until its deadline, so the real candidate cannot make that call enter the connected branch.
The current windows_recovery failure is in the unchanged Local IPC trust fixture (insecure_endpoint_directory), while the affected Runtime Host checks pass. This approval is for the code; the PR is not merge-ready until required CI is green.
AI-assisted review disclosure: Codex performed the final review using two independent reviewer passes and OpenCode Go DeepSeek V4 Flash (high effort) as an adversarial advisory pass. I verified the election state machine, root ownership, tests, exact head 8ca108e3e, and the failing CI log. No local tests were run.
中文评论
当前 head 已正确解决此前的 ownership race：missed election 会在自己的 deadline 返回并保留具体失败原因；迟到的 owned candidate 会交还 Runtime Host lifecycle，而不是杀掉可能已被其他 client 接管的进程。实现直接扩展既有 OwnedCandidateAttempt seam，没有新增 election authority。
当前 head 未发现可复现的 P0–P3。我也复核并排除了两个对抗性候选：root ownership 保证 pid mismatch 的 candidate 不可能同时成为被接管的权威；测试中故意不 resolve 的 spawn promise 会占用第一次 election 直到 deadline，因此真实 candidate 不会让该调用进入 connected 分支。
当前 windows_recovery 失败来自未改动的 Local IPC trust fixture（insecure_endpoint_directory），受影响的 Runtime Host checks 已通过。本次 approve 针对代码；必需 CI 全绿前不能合并。
AI 辅助审查说明：Codex 使用两轮独立 reviewer 审查，并以 OpenCode Go DeepSeek V4 Flash（high effort）进行对抗性辅助审查；我已核对 election 状态机、root ownership、测试、精确 head 8ca108e3e 和失败 CI 日志。本轮未运行本地测试。
Astro-Han
commented
Aug 18, 2026
@1625567290 Hi, Could we state that if there is any ai tools used in the PR? |
Uh oh!
There was an error while loading. Please reload this page.
Summary
owned Host exits promptly after its first connection closesflakes under the full Runtime Host suite. CI run 32101082229 finished 962/963 tests and failed this one at 2.19s withfailedinstead ofconnected. The 2s election window is enough on an idle machine and too short under suite load. The promptness claim is Host exit after the first connection closes, not the connect itself.settle(500)as the prompt-exit check.failed:<reason>on the assertion so a later miss is diagnosable.startup_timeout.Fixes#3190
Verification
node --test packages/runtime-host/dist/__tests__/owned-candidate.test.js— 7/7Checklist
Does this PR entail a change in behavior?