Skip to content

fix(evals): signal POSIX process groups when captured pipes time out - #511

Draft
u7-u7 wants to merge 2 commits into
Tencent:mainfrom
u7-u7:fix/eval-posix-process-group-timeout
Draft

u7-u7 wants to merge 2 commits into
Tencent:mainfrom
u7-u7:fix/eval-posix-process-group-timeout

Conversation

@u7-u7

@u7-u7 u7-u7 commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Problem

A timed-out command adapter can hang the browser evaluation runner on POSIX. runProcess signals only its immediate child, but waits for the child's close event. If a launcher starts a worker that inherits stdout/stderr, that worker keeps the pipes open after the launcher is killed. The same problem occurs if the launcher exits before the deadline. The existing SIGKILL escalation still targets the exited launcher rather than the worker.

Change

Start commands in a separate POSIX process group and send the existing SIGTERM/SIGKILL timeout signals to that group. This reaches workers even after the launcher exits. Clear the escalation timer on completion so it cannot signal a later reused group ID. Preserve the existing direct-child behavior on Windows.

Add real-process regressions for a running launcher, an already-exited launcher, and a descendant that ignores SIGTERM. Each descendant also has a bounded self-exit so the regression safely fails on unmodified code.

This fixes descendants that inherit the command's POSIX process group and keep its captured pipes open. Once the launcher and its pipes close, escalation is canceled; this is not a general background-process cleanup guarantee. Independently detached processes and Windows tree termination are outside this change.

Verification

On Linux / Node 24.22.1:

  • Before the fix: the three regressions fail. A 500 ms timeout finishes only after descendants self-exit at approximately 5.1 / 5.1 / 6.1 seconds.
  • After the fix: the same cases finish at approximately 0.52 / 0.51 / 2.52 seconds, including the existing two-second escalation grace period.
  • node --test evals/browser/tests/*.test.mjs: 30 passed, 0 failed.
  • node evals/browser/cli.mjs validate: 10 cases / 19 fixture routes validated.
  • Biome 2.3.14 check of both changed files: passed, no fixes.
  • git diff --check: passed.

Windows/macOS execution, browser smoke, Rust/extension suites and full workspace lint were not run. No browser or Rust implementation changed.

AI assistance

Prepared and tested with OpenAI Codex. Submitted as a draft; no human-review attestation is claimed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant