Skip to content

test(llm): release detached pipe regression worker - #671

Draft
zzwong wants to merge 2 commits into
mainfrom
draft/llm-detached-fixture-cleanup-20261008
Draft

zzwong wants to merge 2 commits into
mainfrom
draft/llm-detached-fixture-cleanup-20261008

Conversation

@zzwong

@zzwong zzwong commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Preserve the Unix cancellation regression: a Python worker escapes the launcher's process group and keeps the inherited output pipes open while cancellation must unblock the stdout reader.
  • Replace the fixed worker and launcher sleeps with an explicit, private control channel and shell wait. Bound readiness, confirm the holder is still alive after the cancellation assertion, then release it in cleanup.
  • Acknowledge release only after closing inherited stdout/stderr, observe control EOF, and retain a bounded safety lease for abnormal exits. Close and wait for the launcher resources in deferred cleanup.

Validation

  • Independent source review accepted the bounded cooperative-release design.
  • Nonwriting formatting validation passed.
  • The focused local test compiled, but its one runtime attempt failed when binding the private Unix-domain control socket at test line 41: operation not permitted. The result was one run/fail, zero pass/skip, exit 1, with no timeout. This occurred before LaunchProcess, so no shell or Python fixture helper was launched.
  • The focused test did not pass locally. Repeated, race, package-wide and repository-wide tests, lint, and the repository aggregate gate have not been established locally for this candidate.
  • Negative controls were not executed.
  • Local make check was not run.
  • This draft uses the repository's existing normal PR CI. No new workflow or manual workflow trigger is added. Hosted results for the exact successor head are recorded below.

Scope and limitations

Only internal/llm/subprocess_launch_unix_test.go changes, based on main 999e59ddfc74a67ffeef3c4b4a78a8528365afe6. Production source is unchanged.

This is fixture resource-release hygiene discovered while investigating #613. It does not fix, close, or establish a reproduction of the reported macOS production orphan issue. The Go test does not reap the adopted detached Python child; the release protocol establishes cooperative closure of the held resources, not OS reaping.

No alternate IPC transport was tried after the local permission denial. Linux runtime evidence is limited to the hosted test modes below; macOS runtime behavior remains unvalidated. Keep this PR in draft; no merge is authorized.

Hosted CI

  • Initial CI run 37841058107 at head fee40bd667a1b90a133d72bc0ce6c0e12c627363 completed with nine passing jobs and one lint failure: errorlint required errors.Is for the control-channel EOF check. The target test explicitly passed in its normal, opt-out and static-smoke jobs. That failed run remains part of the validation record.
  • The reviewed successor adds only the errors import and replaces the direct EOF comparison with errors.Is. Nonwriting local formatting checks passed; the denied local IPC test was not retried.
  • Successor CI run 37841612799 completed successfully for exact head 2d48d94caf01118c18cbd07b282287fa9dd2d51c: all 10 jobs passed, including lint, the test modes, three OS builds, the build aggregate, identity and title checks.
  • Logs explicitly record TestLaunchProcessUnblocksReadOnCanceledContext passing in the normal Linux test job (0.13s), 1Password-opt-out job (0.13s), and CGO-disabled static-smoke job (0.14s).
  • macOS and Windows jobs are builds, not runtime-test evidence. Negative controls, repeated/race validation, macOS runtime testing, and local make check remain unrun. Hosted success does not erase the separate local IPC failure or establish OS reaping or a production fix for Claude reviewer tool processes can outlive the review run (orphaned process at 100% CPU for 4 days) #613.

Both CI runs were triggered by ordinary PR activity; no manual workflow rerun was used. The PR remains a draft and unmerged.

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