fix(eval): make process termination work on Windows

[jjj-n]

Summary

• Add a shared Eval process-termination helper that preserves POSIX signals and terminates the full Windows process tree with taskkill.exe /T /F from the first cleanup stage.
• Use the helper for harness trials, external subject wrappers, and preflight timeout/cancellation paths.
• Cover the Windows supervisor/descendant lifecycle with a native integration regression.

Fixes #3892.

Verification

• npm run lint passed.
• npm run format:check passed.
• npm --workspace @maka/eval run typecheck passed.
• npm --workspace @maka/eval run build passed.
• npx knip --workspace apps/desktop passed.
• npx knip --workspace packages/ui passed.
• node --test packages/eval/dist/__tests__/process-termination.test.js packages/eval/dist/__tests__/process-termination.windows.test.js packages/eval/dist/__tests__/install-preflight.test.js passed on Windows: 11 passed, 3 skipped because this environment does not grant symlink/permission prerequisites.
• Root npm run typecheck is blocked by existing @maka/ui errors for settledText, autoScroll, and trailingAction; Eval and its dependencies typecheck successfully.
• Root npm run build reaches the same existing @maka/ui type errors after building Eval and its dependencies.
• The full Eval suite was not reported as passing locally because this Windows environment exposes existing POSIX/shebang, symlink, and path assumptions in unrelated tests.

AI use

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

Tool(s) and scope: Codex implemented the process-termination helper, call-site updates, regression tests, review fix, and verification. The human contributor of record reviews the implementation for correctness and licensing.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes - described under Summary above
• No

[hqhq1025]

Reviewed exact head be9891952c83919a17872eb31e092356c5932488. P1 NO-GO: the Windows graceful phase kills only the supervisor, so a normal prompt exit prevents the later taskkill /T /F escalation and leaves descendants running.

Actual change: adds a shared platform-aware termination helper and applies it to harness trials, external subject wrappers, and preflight timeout/cancellation. Scope inspected: the complete five-file diff, waitForTrial cleanup state machine, Python relay teardown, external-subject process ownership, preflight abort/timeout behavior, issue #3892, current main, existing review surfaces, and hosted checks.

Validation: clean npm run build:test; full Eval test:dist passed (80 Node tests plus 74 Python tests, 4 skipped); focused lifecycle/preflight/termination tests passed 32/32; changed-file Biome and git diff --check passed; hosted test and label checks are green; merge-tree against current main is clean. A direct process-tree probe produced supervisorExited=true, taskkills=0, workerAlive=true for the new Windows SIGTERM branch.

Unable to determine: this host is Linux, so I could not execute native Windows taskkill.exe or a real Windows Harbor trial. The control-flow failure occurs before that executable is invoked.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[hqhq1025]

[P1] The normal Windows cleanup path never reaches the tree kill. waitForTrial() first calls this helper with SIGTERM, and this branch calls only child.kill(); the new unit test explicitly asserts that runTaskkill is not invoked. On Windows, Node treats the default/SIGTERM child kill as an abrupt termination of the target process, so the supervisor exits promptly and waitForTrial() returns from its terminated branch before the later SIGKILL escalation can call taskkill /T /F. Any worker already spawned by the supervisor can therefore survive cancellation or timeout. A direct probe of this control flow produced supervisorExited=true, taskkills=0, workerAlive=true. Please make the Windows first-stage cleanup tree-aware (or request graceful teardown over an IPC channel and always retain a tree-kill fallback), and add an integration regression where the supervisor exits but a descendant stays alive.

[jjj-n]

Fixed in 7b103e06b: Windows now invokes taskkill.exe /T /F from the first termination stage, so waitForTrial() cannot return after killing only the supervisor. I added process-termination.windows.test.ts, which spawns a real supervisor and descendant on Windows and verifies that both exit after the first-stage SIGTERM request. Verified on Windows with Eval typecheck/build, repository lint/format, and the focused lifecycle suite (11 passed, 3 permission-related skips).

[Astro-Han]

Thanks—the current head closes the Windows process-tree issue from the earlier review.

Windows now uses taskkill.exe /T /F from the first termination stage, so the supervisor cannot exit and bypass descendant cleanup before a later escalation. The added Windows-only integration test exercises the real boundary by spawning a supervisor and worker, requesting the first-stage SIGTERM, and requiring both processes to exit. POSIX signal handling remains unchanged.

I found no remaining P0–P2 issue on exact head 7b103e06bae002adce2ec45c24b75b4b50f6f144. Hosted checks have not reported for this head yet, so this approval is a code-review conclusion rather than a merge-readiness claim.

中文对照

谢谢，当前 head 已经修复此前的 Windows 进程树问题。Windows 从第一次终止请求起就使用 taskkill /T /F，并用真实 supervisor 与 descendant 的测试覆盖。当前没有剩余 P0–P2；但新 head 的 CI 尚未返回，因此这里只批准代码，不代表现在即可合并。

AI-assisted review: Codex traced the updated termination path and drafted this review; the maintainer verified the fix, reachability, and severity.

[hqhq1025]

Reviewed exact head 7b103e06bae002adce2ec45c24b75b4b50f6f144. I found no remaining P0-P3 issue.

The previous Windows process-tree defect is fixed: terminateProcess now invokes taskkill.exe /T /F for the first Windows termination request, before the supervisor can exit and make descendant traversal impossible. The new unit coverage pins that control flow, and the Windows-only integration test exercises a real supervisor-worker tree. POSIX signaling remains unchanged.

Scope inspected: the complete six-file diff, all terminateProcess call sites in harness execution, external-subject shutdown, and install preflight timeout/cancellation; the prior finding and author response; current main; mergeability; existing reviews; and hosted checks.

Validation: clean npm run build:test; full-repository typecheck; Eval test:dist passed (81 Node tests with 1 Windows-only skip, plus 74 Python tests with 4 platform skips); focused termination/preflight/lifecycle tests passed 32/33 with only the Windows-native test skipped; changed-file Biome and git diff --check passed; merge-tree against current main is clean.

Unable to determine: this host is Linux, so I could not execute taskkill.exe or the new native Windows process-tree test. GitHub currently reports no hosted check runs for this exact head, so this is a code-review conclusion, not a merge-readiness claim.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.