[sonichi]

Closes #5180

Cause

test_valid_launch_and_restart_do_not_manage_owned_helpers asserted that host-global /tmp/core-input-watch.log and /tmp/core-heartbeat.log (mtime_ns + sha256) were unchanged across a launch and restart. Since the reshard landed, python standalone shard 2/7 schedules tests/external-core-helpers.test.py (file 5) right next to tests/shutdown-sentinel-immediate-exit-controls.test.py (file 6), so they start together in parallel lanes. That suite runs the real src/agent/codex/cli/start-cli.sh (codex-detached, codex-tty branches) without --external-helpers, so ensure_core_monitor / ensure_core_heartbeat redirect into exactly those two /tmp files. Same file list and same failure in both runs:

• feat(task-progress): notify.py --task-file derives source/channel/thread from the task #4995 run 37543463833, job 112541788830 (attempt 1)
• fix(test): handler-terminal-rc waits on what the watcher writes, not on a window #5007 run 37543467850, job 112541870371 (attempt 1)

FAIL: test_valid_launch_and_restart_do_not_manage_owned_helpers
AssertionError: {'cor[24 chars]1327350833988961, 'e3b0c44298fc1c149afbf4c8996[148 chars]55')} != {'cor[24 chars]1327339978009586, 'c9ee6b8bc586da407ee8bbabcac[63 chars]None}

(core-heartbeat.log absent before, present after; core-input-watch.log truncated to empty.)

Fix (test only, no production change)

During the two launches the test swaps the fixture repo's copies of core-input-watch.py and core_heartbeat.py for a tripwire that writes a marker in the test's own temp dir. The externally owned helpers already running keep their loaded code, and the receipt validator checks paths, not contents, so validation is unaffected. Any start of either helper by this test's launcher, through any interpreter, now fails the test; writes by other suites to /tmp no longer can. The originals are restored in finally.

New regression test_concurrent_writer_of_host_global_helper_logs_does_not_fail_launch patches Path.stat / Path.read_bytes for the two global log paths only, so every read sees a new write (a deterministic concurrent writer), and runs the launch/restart test under it. It never touches the real /tmp files.

Not changed here: shutdown-sentinel-immediate-exit-controls.test.py still writes the host-global logs through the real launcher; that is a separate hygiene issue.

Before / after

Regression method applied to the origin/main (f7b69fc) version of the test file:

-                           'e7f6c011776e8db7cd330b54174fd76f7d0216b612387a5ffcfb81e6f0919683')}
+                           'd4735e3a265e16eee03f59718b9b5d03019c07d8b6c51f90da3a666eec13ab35')}
----------------------------------------------------------------------
Ran 2 tests in 2.656s

FAILED (failures=1)

HEAD:

..
----------------------------------------------------------------------
Ran 2 tests in 4.605s

OK

Full file at HEAD, 5 consecutive runs: Ran 23 tests ... OK x5. The real /tmp/core-*.log files had the same size and mtime before and after.

Mutation control (the test still catches the defect it exists for): making the codex launcher also start core_heartbeat.py in external-helpers mode fails it:

AssertionError: True is not false : core_heartbeat.py

git diff origin/main | bash scripts/review-checks.sh -> review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean)

🤖 Generated with Claude Code

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 91%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

No lines with coverage information in this diff.

[john-the-dev]

Approve at be76d36. Test-only: tests/external-core-helpers.test.py, +40/-18.

• The tripwire covers every launcher start path. start-cli.sh starts the helpers only as $REPO/src/core-input-watch.py and $REPO/src/core_heartbeat.py (:308, :346, plus the heartbeat --stop at :437), and $REPO is the fixture's temp repo. I mutated the launcher and checked each case:

  • external mode also starting core_heartbeat.py: fails, as it should;
  • external mode starting core-input-watch.py with bare python3 and no /tmp log: fails at this head, while main's version passes, so the new check is stricter;
  • restart running heartbeat --stop: fails.

  The launcher was restored byte-identical (sha256), with no __pycache__ left.

• Regression: head's new test, transplanted into main's file, fails with the mtime/sha mismatch, and passes here. At this head the file is 23/23, three runs out of three, and the real /tmp/core-*.log size and mtime are unchanged before and after.

• review-checks: PASS. 29 checks pass and 1 is skipped.

• Codex-core verification (Rui's Codex agent, at this head): 23/23 on a macOS Codex host. Injecting an extra helper start into either ensure function made the test fail, for both helpers, and the concurrent-log regression fails on the parent f7b69fc81.

Non-blocking:

• The fixed time.sleep(0.3) after each launch means that under heavy load a regressed start could land after the check, a false pass and never a flake. Polling for the marker would close it.
• Removing the early return in ensure_core_heartbeat is missed by both main and this head. The pgrep at :339 finds the already-running external helper, so nothing new starts. That gap predates this PR.
• shutdown-sentinel-immediate-exit-controls.test.py still writes the host-global logs, as the PR body notes. That's a separate hygiene follow-up.

[keweichen]

APPROVE at be76d36f8a0084a0fee4f72b85c611d26eca1967. The change is ready on technical merit with no blocking finding; the PR as a whole is not merge-ready until the requested qingyun-wu code-owner approval is recorded.

Independent evidence:

• The bug exists on the exact parent/current-main SHA f7b69fc81ce8f724ade258dd997457809d98aa86: two unrelated CI jobs ran this unchanged test and failed at the host-global /tmp snapshot while the neighboring real launcher test wrote those logs. Transplanting only the new concurrent-writer regression onto that parent collected the added test and failed with the expected mtime/hash mismatch.
• At this head, all 23 named tests pass in three consecutive full-file runs; the new regression passes in isolation. A mutation that starts the fixture heartbeat in external-helper mode, without depending on either global log, fails on the new marker with core_heartbeat.py, so the replacement instrument has a positive control.
• The diff is one test file (+40/-18), git diff --check and scripts/review-checks.sh pass, all current CI is successful or intentionally skipped (28 success, 1 Windows skip), and license/cla is successful. Worst-case user disruption is limited to test signal: no production code or runtime behavior changes, and fixture script edits are temp-repo scoped and restored in finally.

Non-blocking follow-up: the fixed time.sleep(0.3) leaves a false-negative window for an unusually delayed background child; a controlled delayed-marker mutation demonstrated that limitation. Polling/synchronizing the marker, and eventually tripwiring the helper-death replacement path too, would make the future-regression claim stronger. This does not reintroduce the current cross-shard false positive or change the present launcher behavior.

[sonichi]

@qingyun-wu — this PR is approved by john-the-dev and keweichen (2 non-codeowner approvals) at head be76d36, but .github/CODEOWNERS's fallback rule for the paths this PR touches requires either @sonichi or @qingyun-wu to approve. GitHub won't let the authoring account (sonichi) approve its own PR, so neither Chi nor I can supply that approval. Your review is the only way to clear this gate — could you take a look?

[qingyun-wu]

Code-owner approval by Qingyun via the [PA] Decisions page (Decision: approve) at be76d36. Posted by Qingyun's Sutando.

[github-actions]

@cla-assistant check