fix(runtime-host): preserve Windows startup diagnostics - #3238
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (19)
📝 WalkthroughProblem solvedThis PR preserves actionable Windows Runtime Host startup diagnostics instead of returning only It:
Design and scopeThe PR extends the existing Candidate startup-failure and Local IPC paths. It does not replace their classification or retry behavior. It adds a parallel diagnostic evidence channel in the private control namespace. The channel supports cross-process persistence because the detached Candidate cannot return all evidence directly. Attempt IDs, root validation, election settlement, pruning, and conditional cleanup limit stale or incorrect records. This is the smallest coherent solution shown by the diff. Cross-process persistence and attempt correlation are necessary to identify the selected Candidate failure. Bounded collection and read-only probes are necessary to preserve evidence without weakening the Local IPC trust boundary or mutating isolated state. The new tests are required for serialization, election behavior, redaction, permissions, cleanup, ACL evidence, and release diagnostics. No clear deletion or simplification is identified without reducing regression coverage. Complexity delta
The implementation increases local complexity, but the added states and APIs support required cross-process evidence and bounded cleanup. The total maintenance complexity remains justified by the diagnostic and retry requirements. ValidationValidation included Runtime Host typecheck/build, focused startup and endpoint tests, Host-kernel tests, Biome, The Runtime Host suite passed 980 of 982 tests. Two process-timing failures remain tracked separately. Review-relevant risks
Material changes to public contracts, user-visible behavior, security, or releases require independent human review under repository policy. Required-check status is based on the stated validation summary and is not independently verified here. The person performing the merge must review the final diff, and a maintainer makes the final determination. WalkthroughRuntime Host startup failures now carry UUID-based attempt identities, persist bounded diagnostics, and participate in election-aware cleanup. Windows ACL failures expose structured evidence. Release smoke tooling collects and renders diagnostics before cleanup. ChangesRuntime Host startup diagnostics
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CandidateLauncher
participant RuntimeHostCandidate
participant ConnectOrSpawn
participant DiagnosticCollector
participant RuntimeHostSmoke
CandidateLauncher->>RuntimeHostCandidate: launch with startupAttemptId
RuntimeHostCandidate->>DiagnosticCollector: persist startup failure diagnostic
RuntimeHostCandidate-->>ConnectOrSpawn: return failure report
ConnectOrSpawn->>DiagnosticCollector: select or clear attempt diagnostic
RuntimeHostSmoke->>DiagnosticCollector: collect and render failure evidence
Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Classify Local IPC security failures across the detached Candidate boundary without making them permanent. Preserve bounded PowerShell and installed-package ACL evidence so intermittent Windows startup failures remain actionable after cleanup. Generated-by: Codex
Persist bounded, redacted Candidate startup evidence in the private control namespace and remove it after a successful election. Release validation now observes that authority and canonical registration state through independent read-only probes, preserving the failed filesystem state and explicit Windows owner/SDDL evidence.
M4n5ter
force-pushed
the
fix/windows-runtime-host-startup-diagnostics
branch
from
August 19, 2026 08:50
383b40a to
16b1593
Compare
Detached elections can observe several retryable Candidates, so a root-wide last-writer sidecar could disagree with the failure selected by the client. Bind each exit report to an attempt-scoped artifact, publish only the selected evidence, and retire or bound abandoned records after use.\n\nCollect Windows path security in one PowerShell process under the production 30-second budget so the failure reporter remains useful under the slow-start condition it is diagnosing.\n\nGenerated-by: Codex
Member
Author
|
/agentic_review |
Code Review by Qodo
1. Late reports erase diagnostics
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
M4n5ter
marked this pull request as ready for review
August 19, 2026 10:15
Member
Author
|
I have reviewed the changes in this PR, which mainly focus on the diagnosis of a CLI release with a limited impact scope, and I take responsibility for this result. |
PR Summary by QodoPreserve Windows Runtime Host startup diagnostics
AI Description
Diagram
High-Level Assessment
Files changed (19)
|
Comment on lines
+277
to
+281
| if (electionSettled) { | ||
| void clearCandidateStartupDiagnostic( | ||
| capability.rootId, | ||
| failure.startupAttemptId, | ||
| ).catch(() => undefined); |
There was a problem hiding this comment.
1. Late reports erase diagnostics 🐞 Bug ☼ Reliability
When an election settles while a Candidate failure report is still pending, the late-report branch deletes that attempt's diagnostic even though no Candidate failure was selected. A Candidate that wrote actionable Local IPC evidence near the deadline can therefore be returned as a generic startup_timeout or host_unresponsive with its evidence removed.
Agent Prompt
## Issue description
A Candidate failure promise can resolve after the election has settled. The current late-report branch deletes that attempt's diagnostic even when the election selected no Candidate failure, losing the evidence this PR is intended to preserve.
## Issue Context
This is a fix-now issue and is distinct from stale shared-diagnostic cleanup: it affects a fresh per-attempt diagnostic whose report was pending at election settlement. Reuse the existing pending-report and diagnostic-selection seams rather than adding another diagnostic state or public surface; deletion or unconditional cleanup cannot satisfy the evidence-preservation invariant.
## Fix Focus Areas
- packages/runtime-host/src/client/connect-or-spawn.ts[271-303]
- packages/runtime-host/src/client/connect-or-spawn.ts[318-333]
- packages/runtime-host/src/__tests__/host-kernel.test.ts[210-257]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+44
to
+51
| if (rootId && startupAttemptId) { | ||
| await writeCandidateStartupDiagnostic({ | ||
| rootId, | ||
| startupAttemptId, | ||
| failure, | ||
| error, | ||
| logs, | ||
| }).catch(() => undefined); |
There was a problem hiding this comment.
2. Early failures lose diagnostic 🐞 Bug ☼ Reliability
execution-candidate-main ignores diagnostic-write failures, but writeCandidateStartupDiagnostic assumes the per-root control directory already exists. A valid Candidate whose root validation fails before ownership acquisition (for example root_identity_changed) therefore exits with the classified code without persisting any diagnostic; the new regression test masks this by manually creating that directory first.
Agent Prompt
## Issue description
Candidate startup failures that occur before `tryAcquireStateRootOwner` cannot persist their startup diagnostic because the per-root control directory has not yet been created. The write error is intentionally swallowed, so installed-CLI diagnostics have no evidence for those failures.
## Issue Context
`startInteractiveRuntimeHostCandidate` validates the root before acquiring the owner, while owner acquisition is the existing path that creates the per-root control directory. The test currently creates that directory manually before launching the Candidate, which does not reflect this failure path.
## Fix Focus Areas
- packages/runtime-host/src/control/startup-diagnostic.ts[79-134]
- packages/runtime-host/src/execution-candidate-main.ts[41-53]
- packages/runtime-host/src/server/candidate.ts[18-38]
- packages/runtime-host/src/__tests__/execution-candidate-main.test.ts[43-76]
Reuse the storage authority's existing private-control-directory preparation seam (or expose a narrowly scoped equivalent) before writing the diagnostic, rather than adding an unchecked `mkdir` in the diagnostic writer: the directory must retain the storage authority's ownership/privacy validation. Extend the subprocess regression test so it does not pre-create the control directory and still observes the persisted diagnostic.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit f5032f8 |
6 tasks
Merged
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
English
Summary
Preserve actionable evidence when Windows Runtime Host startup fails:
Fixes #3237
Verification
node --check, andgit diff --checkThe full Runtime Host suite reached 980/982; its two process-timing failures pass in isolation and are tracked in #3190 and #3239. The Windows workflows remain the authoritative platform validation for this draft.
AI use
Tool(s) and scope: OpenAI Codex assisted with implementation, tests, and validation. M4n5ter is the contributor of record.
Checklist
Does this PR entail a change in behavior?
中文
摘要
在 Windows Runtime Host 启动失败时保留可定位的证据：
修复 #3237
验证
node --check与git diff --checkRuntime Host 全量套件通过 980/982；两项进程时序失败均可在独立运行时通过，并已由 #3190 和 #3239 跟踪。此 Draft PR 的 Windows workflow 是权威平台验证。
AI 使用
工具与范围：OpenAI Codex 协助实现、测试和验证。M4n5ter 是记录在案的贡献者。
检查清单
本 PR 是否改变行为？