Skip to content

fix(windows): stop killing orphan workers behind reused parent PIDs (0.7.2) - #23

Merged
keli-wen merged 6 commits into
masterfrom
fix/windows-stale-parent-kill
Sep 13, 2026
Merged

keli-wen merged 6 commits into
masterfrom
fix/windows-stale-parent-kill

Conversation

@keli-wen

Copy link
Copy Markdown
Owner

Problem

Every Windows CI run since process-tree cleanup landed in #18 had roughly a 50% chance of failing, each time on a different test (a9887f1: dx.test staffer prompt; 07aaca0: recovery-regressions hard expiry; PR runs: hard stop escalates, group membership, continue --job, npm archive, ...). PRs were merged after their last run happened to be green; master gets one roll of the dice.

The common signature: a detached worker vanishes with no status file and a log of exactly 178 bytes (the two startup lines and nothing else), so wait reports crashed (exit 3). No internal error path can produce that: the top-level catch always writes agy-staff error: to the log. Only TerminateProcess from outside does.

Root cause

On Windows a detached worker keeps its exited dispatcher's PID in ParentProcessId. Windows reuses PIDs quickly, and with three test files running in parallel one of them soon spawns an agy process that receives that PID. When that job finishes, two independent tree walks both take our worker for its descendant and kill it:

  1. tree() in companion/stream-worker.mjs follows parent links with no check that the child is younger than the parent.
  2. taskkill /PID <root> /T /F does its own walk over the same stale ParentProcessId links.

Neither happened before #18 (Windows never walked process trees at all), which is why six earlier master builds were green. Diagnosis and fix design were independently reviewed by an agy review job (verdict: confirmed; it found the second, taskkill /T, path).

Fix

  • tree() follows a parent link only when child.born >= parent.born. A process cannot be older than its parent; a row born before its recorded parent is an orphan behind a reused PID. This is sound: the reuser can only be born after the dispatcher exited, and the worker was born before that.
  • PowerShell reports CreationDate via ToString('o') so the comparison has 100 ns precision; the default locale rendering is second-granular and would miss a reuse within the same second. parseBorn puts ISO-8601, WMIC and legacy stamps on one BigInt tick scale.
  • taskkill runs without /T on Windows. Descendants are terminated one by one after passing the identity and birth-order checks, as stopExecution already did for the JS-side members.
  • stopExecution never signals its own PID and takes an injectable table source.
  • Docs updated (docs/REFERENCE.md, zh-CN).

Tests

  • Real-process regression test: a real detached orphan, a real root with a real child, real signals. The only synthetic element is one ParentProcessId in the table, because no kernel lets a test choose the next PID it hands out (and POSIX reparents orphans to init). With the birth-order check disabled the test fails; with it, the root and its child are stopped and the orphan survives.
  • Unit tests for parseBorn (ISO with offsets, WMIC, legacy), tree() stale-link pruning, the round-trip PowerShell query and the new taskkill arguments.
  • Local suite: 194/194 on macOS. The Windows job will be run several times on this branch to measure the pass rate instead of relying on one green run.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RKgBCYMWQru5SawpfCLxFY

On Windows a detached worker keeps its exited dispatcher's PID in
ParentProcessId. Once that PID is reused by another job's agy process,
both tree walks used at cleanup (tree() in stream-worker and taskkill /T)
took the worker for a descendant and terminated it with no chance to
write its status file, so wait reported "crashed" with an empty log.
Every Windows CI run since process-tree cleanup landed (#18) had a
roughly even chance of losing one worker to this; which test broke was
whichever job happened to hold the reused PID.

- tree() follows a parent link only when the child was created after its
  parent; a row born before its recorded parent is an orphan behind a
  reused PID. PowerShell now reports CreationDate in round-trip ("o")
  format so the comparison has 100 ns precision instead of one second.
- taskkill runs without /T on Windows; descendants are terminated one by
  one after passing the identity and birth-order checks.
- stopExecution never signals its own process and accepts the table
  source so a test can inject the one thing no kernel lets it choose:
  a stale parent link over real processes.
- Regression test with real processes and real signals, plus unit tests
  for parseBorn, tree() and the taskkill arguments.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RKgBCYMWQru5SawpfCLxFY
@keli-wen
keli-wen deployed to manual-tests September 13, 2026 12:55 — with GitHub Actions Active
@keli-wen
keli-wen deployed to manual-tests September 13, 2026 12:55 — with GitHub Actions Active
@keli-wen
keli-wen deployed to manual-tests September 13, 2026 12:55 — with GitHub Actions Active
@keli-wen
keli-wen deployed to manual-tests September 13, 2026 12:55 — with GitHub Actions Active
@keli-wen

Copy link
Copy Markdown
Owner Author

Windows pass-rate measurement on this branch (each run approved through the manual-tests gate):

Commit Runs Tests (Windows) Note
fa0522e 4 (1 pull_request + 3 workflow_dispatch) 0 pass / 4 fail every failure was the new real-process test's exact-tree assertion (Windows adds conhost.exe helpers under the root); all pre-existing tests passed in all 4 runs
f2400ee 4 (1 pull_request + 3 workflow_dispatch) 4 pass / 4 pass membership assertion; 194 tests, ~4 min per run

Across the 8 runs the original signature (a worker vanishing with a 178-byte log and crashed) did not occur once. On master since #18 it hit roughly every second run.

@keli-wen keli-wen self-assigned this Sep 13, 2026
@keli-wen keli-wen added the enhancement New feature or request label Sep 13, 2026
keli-wen and others added 2 commits September 13, 2026 21:50
… tests

Follow-ups from the second agy review of the diff:

- stopExecution adopts members that appear between its two snapshots
  (a tool started during the grace period) from a still-live,
  identity-matched member under the same birth-order rule as tree().
  Without /T this window was otherwise left to leak.
- cancel compares worker identities with sameBirth(), which matches a
  0.7.1 locale-rendered stamp against the round-trip table at the coarser
  precision, so a job that spans the upgrade can still be canceled.
- Tests: the real-process test registers child cleanup before polling and
  polls at most 30 times (each table query costs seconds on Windows); the
  unreadable-stamp assertion now pairs an unreadable child with a readable
  parent; a POSIX real-process test covers late-child adoption.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RKgBCYMWQru5SawpfCLxFY
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RKgBCYMWQru5SawpfCLxFY
@keli-wen keli-wen changed the title fix(windows): stop killing orphan workers behind reused parent PIDs fix(windows): stop killing orphan workers behind reused parent PIDs (0.7.2) Sep 13, 2026
@keli-wen
keli-wen deployed to manual-tests September 13, 2026 13:50 — with GitHub Actions Active
@keli-wen
keli-wen deployed to manual-tests September 13, 2026 13:50 — with GitHub Actions Active
@keli-wen
keli-wen deployed to manual-tests September 13, 2026 13:50 — with GitHub Actions Active
@keli-wen
keli-wen deployed to manual-tests September 13, 2026 13:52 — with GitHub Actions Active
…ndler

On a fast Linux runner ps saw the root before Node had registered the
handler, so SIGTERM took its default action and no late child appeared.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RKgBCYMWQru5SawpfCLxFY
@keli-wen
keli-wen deployed to manual-tests September 13, 2026 13:57 — with GitHub Actions Active
@keli-wen
keli-wen deployed to manual-tests September 13, 2026 13:57 — with GitHub Actions Active
@keli-wen
keli-wen deployed to manual-tests September 13, 2026 13:57 — with GitHub Actions Active
Worker identities are compared as the exact strings the table produced,
as before. A job started by 0.7.1 and still running through the upgrade
is the only case affected; the release note says to let jobs finish
first.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RKgBCYMWQru5SawpfCLxFY
@keli-wen
keli-wen deployed to manual-tests September 13, 2026 15:05 — with GitHub Actions Active
@keli-wen
keli-wen deployed to manual-tests September 13, 2026 15:05 — with GitHub Actions Active
@keli-wen
keli-wen deployed to manual-tests September 13, 2026 15:05 — with GitHub Actions Active
@keli-wen
keli-wen merged commit e784ad4 into master Sep 13, 2026
9 checks passed
@keli-wen
keli-wen deleted the fix/windows-stale-parent-kill branch September 13, 2026 15:23

This branch was successfully deployed

1 active deployment
manual-tests — edaf893f Deployed Sep 13, 2026 by keli-wen via Tests (Windows) #98
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant