Repository navigation
fix(daemon): make the daemon reachable on native Windows hosts (#3291) - #3330
Conversation
Windows hosts have no `ps`, so every daemon-ownership start-time and command read answered null and a live daemon could never prove its lifetime (#3291). Add a Windows branch to the same host-process seam the POSIX reads use: one PowerShell CIM query answers start time (CreationDate), command line, and liveness per pid, with a budget that covers PowerShell startup. A failed or unanswered query stays unknown evidence, and a live CIM row is never a zombie because terminated Windows processes leave the table.
…cate
On Windows a handle opened append-only ('a') rejects ftruncate with
EPERM, so daemon startup died inside truncateDaemonLog before it could
listen (#3291). Create the log with 'a', then truncate through an 'r+'
handle; a failing truncation still propagates instead of being
swallowed. The regression test pins the Windows handle rule at the fs
seam.
thymikee
added
the
ready-for-human
Size Report
Startup median (7 runs, lower is better):
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Keeping the create-then-reopen on every host dropped the inode pin between handles, so a rotated log turned publication into an ENOENT startup failure or a truncate of the wrong file. POSIX keeps the original single pinned append handle (it permits ftruncate there); only win32 opens the write handle, directly 'r+' for an existing log, with append-creation confined to the missing-file path. A removal inside the remaining creation race still fails startup loudly (PR #3330 review).
The Windows identity branch made every synchronous probe pay a process-tool startup, and per-record ownership checks issued three of them. The owned- process reaper now answers zombie/startTime/command from one async readProcessIdentityFacts per decision, owner-liveness classification reads one batch snapshot instead of a state probe plus a birth probe, and the Windows host answers the zombie question without any probe because a terminated process leaves the CIM table rather than lingering unreaped. Also pin PowerShell's console to UTF-8: its default OEM code page corrupted non-ASCII CommandLine rows that identity equality later compares byte-wise (PR #3330 review).
iOS Smoke 'Preflight iOS runner through public CLI' — ruled out as caused by this diffEvidence on the failing head
Unresolved risk stated plainly: I cannot reproduce the lane locally, so contention on the shared runner is a best-supported diagnosis, not a proven one; the android-branch lane still reproduces without this PR's code, so it stays out of scope here. |
|
Lane settled: environment flake, diff explicitly ruled out. Rerun of the exact failing job (run 37832908816 attempt 2, same head Cross-evidence that the step is flaking independent of any daemon change:
The one place this diff could plausibly bite the macOS path was the 2b double-open, and it cannot: at head No code change this round; head remains |
process-lock.test.ts crossed the 1,000-line tripwire once the snapshot-aware fixture landed, and the ratchet refuses growth. The three owner-liveness judgment cases (zombie reclaim, null-start fail-closed, guard-free live judgment) mirror the classification seam, not lock mechanics, so they move — unchanged — to process-lock-owner-liveness.test.ts with the probe fixture they consume. Lock-mechanics cases stay. No assertions changed.
|
Coverage ratchet fixed at Verification:
|
|
The code looks right for the native-Windows route, but the Windows part has no live proof yet, so it is not ready to merge. I reviewed 1869aed. All 19 checks pass at that commit, but no CI lane runs on Windows, so the win32 route is not exercised in CI at all. The whole change sits on a path no test has run on real Windows: CIM identity, the PowerShell UTF-8 pin, Not blocking, take or leave: the Is there a materially smaller design? I found none, since the platform branch sits at the owning seam and every identity consumer inherits it. The only optional split is to land the POSIX-touching reaper and On the open threads: the P2 thread on the remaining synchronous PowerShell polling still applies (#3330 (comment)), and its latency belongs in the evidence above. The two P3 threads are fixed at this head and can be resolved: the UTF-8 pin is in place (#3330 (comment)), and the POSIX path keeps the single I could not read the test move or ratchet fix the PR body cites at 5932300, because that commit is not in my checkout. I did not run the mutation checks it claims. No Windows host was available to me, so PowerShell output format, CIM stability, truncation with a held append handle, and cold-start latency are all unverified. The claim that Windows needs no zombie probe rests on libuv's The head moved to 5932300 after this review. That commit only moves the lock-owner liveness tests back into |
…SIX probe The snapshot predated this PR on PATH 'ps' while HOST_PS_COMMAND pinned the per-field probes to /bin/ps. One binary for every POSIX process-table read restores the trust posture HOST_PS_COMMAND exists for, and its dead win32 arm goes: every Windows caller branches to the CIM query first. Snapshot and facts tests now pin the binary the probe asks for.
The EPERM-on-append-handle emulation was copied verbatim into the existing and missing log cases. The flags it records still differ per case, so the helper returns them and each test keeps pinning its own handle sequence.
Windows live verification: not executed here — the gap I am knowingly leaving openNo native Windows host is reachable from this environment, no agent in this fleet drives Windows, and no CI lane runs on Windows ( What I did verify, at the seam the platform branch sits on:
On (5), the measured latency: unmeasurable here, stated plainly. What I can say about the shape: the budget is already Residual risks I am knowingly shipping (all fail-closed, none signal a possibly-recycled pid):
Correcting one premise in the review:
Also landed at head |
|
Nothing in this PR proves the #3291 route on a real Windows host, so I don't think it is ready to close that issue. At bad805c, all Windows coverage feeds hand-written CIM rows at the exec seam and emulates the NTFS EPERM rule at the fs seam (host-process.ts#L266). If the real PowerShell output, the 2 s timing budget or the spawn route differs from those fixtures, Windows users stay broken and the issue is already closed. Please run one build of this head on Windows 11 (the reporter or any host). The run must reach Not blocking, and you can take or leave these: the row body at host-process.ts#L268 calls The Windows branch sits at the right place, the host-process seam, and I found no smaller owner. One question: what enumerates the five The earlier thread on the synchronous PowerShell spawns still applies (discussion); it is a latency cost, and the Windows run should report it. The two P3 threads on the UTF-8 output encoding (discussion) and on the log truncation (discussion) are fixed at this head. I read the code at bad805c and did not run the test suites or any Windows host. The PowerShell output shape, the null |
…mp culture-pinned The row body claimed a culture-invariant stamp while ToString took no culture argument, so a host calendar could reformat the same process birth between the daemon's publish and a client's verify. And Formatting a null CreationDate (the System Idle Process row) raises mid-pipeline: exit 1 blanks the whole snapshot rather than that row, turning one odd process into unknown evidence for every pid. The row rule is now what it claims: null fields print empty, never raise; birth is formatted with the invariant culture. Reaper coverage gains the zombie identity it never seeded, and owner-liveness guards collapse into one place so the single snapshot path stops paying two kill(pid, 0) per poll.
…eness rules The two stdout parsers had grown an identical five-line pid/ppid walk, and the shared liveness judge stacked its probe-mode ternaries past the complexity threshold. Both formats now feed their own pattern and builder through one row walk, and the zombie and PID-reuse questions answer from named rules that own their unknown-evidence semantics. No baseline moved: the findings resolved at their shapes.
|
Blocking gate applied and all four findings fixed at their owning rules. Head Refs, not Closes — body changed; 1 + 2 — the row body ( 3 — zombie coverage seeded ( 4 — guard collapse ( The design question — six Also from this round's earlier pass: fallow's changed-file audit flagged a new clone group (the two parsers' shared five-line pid/ppid walk) and a complexity breach on the extracted judge; both fixed by shape in |
|
Lane note for Because my recent commits did touch POSIX code ( |
|
First red at |
|
Timing test run on this macOS host, preflight shape (fresh
Head is not materially slower than base — medians 2.67 s vs 2.66 s, inside run-to-run noise, and base's own first run matched head's worst. Six clean head cycles at this exact checkout plus three base cycles: daemon always published a reachable Bisect status: the 16ff5d8 iOS rerun was cancelled at 03:50:33Z by workflow |
|
Closing the lane question. The cross-head bisect is dead as evidence and I have stopped using it: my 16ff5d8 re-run (attempt 3) was still burning a lane slot under the same Attempt 4 at |
|
Thanks for the update. At a7821ef the code delta looks clean, and it leaves POSIX startup unchanged. The one thing still missing is a run on Windows. Every Windows route is still proved only by hand-written fixtures at the exec and fs seams: the CIM row body, the PowerShell spawn, the 2 s budget and the NTFS log truncate (host-process.ts#L287). No lane runs on Windows, and no Windows host run exists at this head. If the real PowerShell output, cold-start latency or spawn behavior differs from the fixtures, users of #3291 stay broken. The PR now says Refs, so the issue will not auto-close, but merging would still ship an unproven fix for the reported route. Please run one build of this head on Windows 11 and reach On the open threads: the readWindowsProcessRows sync spawn with a 2 s budget thread still applies. It is a latency cost, not a correctness defect, so the Windows run should measure it. The UTF-8 output encoding thread and the log-truncate handle thread are both fixed at this head, so you can resolve them. CI is green on the final state (18/18 non-skipped). iOS Smoke preflight failed on attempts 1-3 with daemon_startup_failed and an empty daemon.log. That looks unrelated, because the delta only touches the shared row walk and an equivalent liveness-guard refactor, not the POSIX startup path. I did not run the test suites or the mutation checks, so I have not verified those claims. Before merge, we need the Windows 11 run above for a7821ef. |
|
Native Windows run of the #3330 checklist at head Host: Windows 11 Enterprise 10.0.26200, Node 24.21.0, plus Electron 44.4.2 as runtime (
Baseline on the same host (0.21.12, HTTP, Electron): Items the PR body says are still owed:
Not covered: a cold-boot CIM timing, and iOS/macOS paths. One unrelated Windows build issue showed up while building this head. |
|
@pai-scaffolde — thank you. This is the evidence this PR could not produce from its own environment, and the run is exactly the shape the review asked for. Stating plainly which of my own open questions your run closes:
What stays open, recorded honestly and not going back to you for more numbers: the reviewer asked for the full-table CIM query wall time with exit 0, and what exists is a warm single-PID query at 324/331/345 ms (your own caveat: not cold-boot). Cold-boot latency and the PowerShell-per-poll share of the 1.2–1.4 s stop cost remain unmeasured — a latency note the reviewer already ruled "cost, not correctness defect," now bounded by your end-to-end stop numbers far tighter than anything I could claim. The PR body's evidence section now records your run (head, host, runtime versions, both transports, the baseline contrast) so it survives being buried in this thread. The |
|
Thanks @pai-scaffolde. Your native Windows run at a7821ef gives the live evidence the earlier review asked for: start without EPERM, the CIM start-time identity, daemon reuse, and graceful stop, on both transports and with non-ASCII paths. The 0.21.12 baseline on the same host also shows the change is what fixes it. Nothing from the review is still open at a7821ef, and there are no conflicts. |
|
2 participants
Summary
On native Windows the daemon never became reachable:
truncateDaemonLogno longerftruncates an append-only handle (Windows EPERMs that). POSIX keeps the original pinned'a'handle, byte-identical toorigin/main; win32 empties the log through an'r+'handle, append-creation confined to the missing-file path.host-process.tsgains a Windows branch at the existing host-process seam: one PowerShell CIM (Win32_Process) query answers birth,CommandLine, and liveness per identity decision. The CIM row rule is "null field prints empty, never raises"; PowerShell's console is pinned to UTF-8; failed queries stay unknown evidence, fail-closed. All POSIX probes pin/bin/ps.Refs #3291 — the live Windows run below now exists; closure is the maintainer's call.
Validation
pnpm check:affected --runpassed at heada7821ef58. Mutation evidence: dropping the reaper's command/zombie comparisons or reverting either fix fails the named regression tests.Live Windows 11 run by @pai-scaffolde at head
a7821ef58(evidence): Windows 11 Enterprise 10.0.26200, Node 24.21.0 + Electron 44.4.2, socket + HTTP, fresh state dir:doctor --debugexit 0, zero EPERM;processStartTime= exact CIMCreationDate(202610091601014504740); daemon reused on both transports; graceful stop, pid gone. Non-ASCII paths (üïøéjunction + state dir) start/reuse/stop cleanly. Baseline 0.21.12 on the same host publishes noprocessStartTime; stop fails.Residual (note, not blocker): full-table/cold-boot CIM timings unmeasured (warm single-PID 324–345 ms vs the 2 s budget); PowerShell-per-poll stop cost folded into the 1.2–1.4 s stop total.
iOS Smoke 'Preflight' reds were lane flakes (#3342): pass on rerun, failed identically on PRs lacking these hunks.