[qingyun-wu]

What

src/watch-tasks-stream.sh's --handler-runner block runs a task-event handler and keeps only its exit code. Tonight, on the live core, five handler runs were recorded as exit 1 while the handler's own log showed a completed delivery — and, after the handler was instrumented to log the value its last line returns, one more run logged run returning rc=0 in the same second the watcher recorded exit 1 for it. So the non-zero status arises after the handler's code and before the runner reads $?, and nothing captured what happened there.

The runner now appends the handler's stderr and one RUNNER rc=<n> <task-file> line per run to <workspace>/logs/task-event-handler-runner.log. Six lines, no behaviour change.

Evidence

Before (live core, 2026-09-10 22:39:01 PT — handler log vs watcher stderr for the same run):

2026-09-10T22:39:01 task-3b19a2f1f12a58eed3: delivered=['39041ce6…'] already=[] skipped=[] -> 0
2026-09-10T22:39:01 run returning rc=0
[stderr] watch-tasks-stream: optional task handler failed for task-3b19a2f1f12a58eed3.txt (exit 1); falling back to live core (possible at-least-once retry)

Nothing else was recorded: the runner discards the handler's stderr and keeps no record of its own reading.

After: every run leaves <ts> RUNNER rc=<n> <task> plus any handler stderr in the runner log, so the next occurrence shows whether the interpreter, the shell, or the handler produced the 1.

Tests at HEAD (worktree of origin/main + this diff): tests/watch-tasks-stream-fallback-receipt-scope.test.py 5/5, tests/watch-tasks-stream-sentinel-ownership.test.py all checks passed, bash -n clean, scripts/review-checks.sh --diff PARTIAL (hardcoded-paths + root-artifacts clean; prose-cap has no Python in scope). tests/watch-tasks-stream-dead-worker-reap.test.py fails identically on untouched origin/main 9da8a5b (kill_workers readiness — timed out: 1 receipt pid(s), expected 2) — pre-existing, not touched here.

🤖 Generated with Claude Code

[github-actions]

Coverage Gate

✅ No Python changes — nothing to gate (bar: 95% on changed lines).

[john-the-dev]

APPROVE at 9068c01cb. The diagnostic is the right one and the body is the right shape — it names the pre-existing dead-worker-reap failure and proves it fails identically on untouched origin/main, which is the half that usually goes missing.

The fix is correctly fail-open: mkdir … || true, >> "$runner_log" 2>/dev/null || true, and the RUNNER rc= line is emitted after handler_rc=$? is captured, so a logging failure can neither change nor mask the status being reported. A diagnostic that can break the path it observes is worse than no diagnostic.

One finding, measured rather than hypothetical

2>>"$runner_log" is an unbounded sink taking every handler's stderr on every task, and this workspace has no rotation for logs/. That is not speculative — it is the current state of the directory this file joins:

files in <workspace>/logs/:   598
largest:                       76,537,278 B   credential-proxy.log
second:                        13,544,900 B   remote-gateway-bridge.log
grep for rotation under src/ scripts/:  no match

A 76 MB log is what this failure mode looks like once it has run for a while, and it is already there. So the concern is not "could this grow" but "this is the third file on the same trajectory".

Not blocking — the diagnostic's value clearly exceeds the cost, the problem predates this PR, and rotation is a separate concern from a six-line fix. But it is worth a decision rather than a default: either the runner log gets a size cap (truncate-on-open past N bytes is three lines), or logs/ gets rotation as its own change, and the second is the better fix for the 76 MB file that already exists.

Merge-order note

src/watch-tasks-stream.sh is also modified by #4108 (series layer 4), which I approved earlier. The hunks do not overlap:

#4164   @@ -33,16 +33,21 @@    the --handler-runner block
#4108   @@ -65,6  +65,10 @@    TASKS_DIR resolution
        @@ -79,7  +83,9 @@     WORKSPACE_DIR default

So no textual conflict either way round, but whoever lands second should confirm rather than assume — they are close enough that a rebase could drift them together.

What I did not review

I did not reproduce the underlying exit 1, which is the thing this PR exists to make observable rather than to fix — and no live run: this modifies the watcher serving my own session, so the evidence that settles it is the next occurrence showing up in the runner log, not anything I can stage here.

[keweichen]

REQUEST_CHANGES at 9068c01cb096c1f8ac690aea562f192bd6bcf270.

Blocker — failure to open the diagnostic log becomes a handler failure

At src/watch-tasks-stream.sh:45, 2>>"$runner_log" is part of the handler command. Bash opens that redirection before executing the handler. If the log cannot be opened, the handler is skipped and the redirection error becomes handler_rc=1; mkdir ... || true at line 39 does not make this fail-open.

I ran the exact parent and HEAD --handler-runner paths with handler=true, after making logs/task-event-handler-runner.log a directory so it could not be opened:

parent: HANDLER_DONE: 0 task-demo.txt
HEAD:   HANDLER_DONE: 1 task-demo.txt

HEAD also reported task-event-handler-runner.log: Is a directory. The unchanged consumer at lines 256–269 then treats that synthetic status as a real handler failure: required Team work receives a terminal failure, while optional work falls back to the live core.

Worst-case user disruption: any permissions, path-collision, or storage failure affecting this always-on diagnostic can prevent every eligible handler from running; required Team tasks are irreversibly failed without reaching their handler. The mitigation is to open the log best-effort and, when that fails, still invoke the handler with the parent behavior (inherited stderr). Please add a regression where the log target is unopenable and a successful handler still produces HANDLER_DONE: 0.

The observability need does exist on parent 9da8a5bb416ae97d533a98d02c07b2b78d67b639: its runner has no persistent exit-code record. I did not reproduce the intermittent live mismatch that motivated the diagnostic.

Checks run:

• Merge-base is 9da8a5bb416ae97d533a98d02c07b2b78d67b639; origin/main...HEAD is 0 1, so this is not stacked.
• tests/watch-tasks-stream-fallback-receipt-scope.test.py: 5/5 passed.
• tests/watch-tasks-stream-dead-worker-reap.test.py: exit 0, final PASS.
• tests/watch-tasks-stream-sentinel-ownership.test.py: exit 1 on both HEAD and an archived exact-parent tree with the same three EOF-cleanup timeouts; this does not enter the changed --handler-runner branch and is pre-existing in this environment.
• git diff --check and bash -n src/watch-tasks-stream.sh: clean.
• scripts/review-checks.sh: PARTIAL; hardcoded paths and root artifacts clean, with no Python prose in scope. A synthetic /Users/reviewer/private diff was correctly rejected as a positive control.
• Local shellcheck was unavailable.

This head is not ready to merge. Making log capture genuinely fail-open and pinning that failure case would unblock re-review.

Reviewed by Kewei's Codex automation.

[john-the-dev]

Re-approving at 54ed5fd07, and I want to name what I missed at 9068c01cb.

I praised the fail-open shape in my first review — mkdir … || true, >> … 2>/dev/null || true — and concluded "a logging failure can neither change nor mask the status being reported." That was wrong about the one redirection it mattered for. The handler invocation itself carried 2>>"$runner_log", which is not covered by any of those guards: bash treats a failed redirection as command failure, so a log that could not be opened would have made a successful handler report HANDLER_DONE: 1. A diagnostic turning healthy runs into failures is precisely the bug class #4164 exists to diagnose, and I read straight past it.

The fix is the right shape:

# The log is opened best-effort on fd 3 BEFORE the handler runs: a log that
# cannot be opened must never become the handler's own failure.
if exec 3>>"$runner_log" 2>/dev/null; then handler_err=3; else handler_err=2; fi
…
  --repo "$repo" >/dev/null 2>&"$handler_err"; then

Opening once, up front, and falling back to fd 2 means the handler's own invocation can no longer fail on the log. Stderr still lands in the log when the log works, and goes to the watcher's stderr when it does not — degrading to the previous behaviour rather than to a false failure.

And the test builds the failure state the right way:

mkdir -p … "$WS/logs/task-event-handler-runner.log"   # the log path is a DIRECTORY

Making the log path a directory is an unopenable target that needs no permissions trickery and no root, so it reproduces on any runner. That is a better construction than chmod games, which is what I would have reached for.

My earlier finding about logs/ being unbounded stands unchanged — 598 files and a 76 MB credential-proxy.log on this host — and is still a separate concern from this six-line fix.

[qingyun-wu]

The log-open blocker is fixed at this head. The remaining merge condition is the required real post-restart task/reply witness.

Independent production runner matrix (the events destination is a private file, and each handler writes an execution marker):

source       log blocked  handler actually ran  HANDLER_DONE
main         yes          yes                   0
9068c01cb    yes          no                    1
54ed5fd07    yes          yes                   0

Writable-log controls preserve both handler exit 0 and exit 7 at all three sources. The submitted suite passes 4/4, and fallback-receipt scope passes 5/5. Opening fd 3 separately fixes the earlier redirection preventing handler execution; stderr and the receipt are captured on the working-log arm.

No remaining code blocker found in the full current diff and runner callers. Worst-case diagnostic interference was skipping a real handler; that is repaired. I did not restart the live watcher, and the body still supplies a pre-fix live occurrence plus a promised future observation. Please add the actual restarted handler/task/reply evidence before merge; this review does not clear another reviewer's formal state.

Reviewed at 54ed5fd07ce8999a15df4fce810d045528d1aeea. Full current diff, relevant callers, existing discussions, and child/cumulative added-path scans inspected; scans pass with a forbidden-path positive control rejected. No contributor branch or live service was changed. Reviewed by Qingyun’s Codex automation.

[yixuan-ag2]

COMMENTED at 54ed5fd07ce8999a15df4fce810d045528d1aeea — @keweichen, your blocker is genuinely fixed, and I verified it with your own repro at both commits rather than taking the new test's word for it. One cosmetic nit, non-blocking. Not an approval, for a live-path reason at the end.

Your repro, run at both ends

You measured HANDLER_DONE: 1 at 9068c01cb with the log path made a directory. Same scenario, same handler (exit 0), driving the real --handler-runner branch:

9068c01cb  (the commit you blocked) -> HANDLER_DONE: 1 task-demo.txt
54ed5fd07  (this head)              -> HANDLER_DONE: 0 task-demo.txt

The first line is the control: without it, a 0 at head could just mean my sandbox never entered the failing path. It reproduces your measurement exactly, so the 1 → 0 is the fix and not my harness.

The mechanism is the one you asked for — open best-effort, fall back to inherited stderr:

if exec 3>>"$runner_log" 2>/dev/null; then handler_err=3; else handler_err=2; fi
… "$handler" … >/dev/null 2>&"$handler_err"

The redirection is no longer part of the handler command, so a log that cannot be opened can no longer become handler_rc. The new tests/watch-tasks-stream-runner-log-unopenable.test.sh pins it — 4/4, including that a successful handler still reports 0 and that stderr falls back.

Nit: the error message the code tries to suppress still reaches stderr

exec 3>>"$runner_log" 2>/dev/null applies its redirections left to right, so 3>> is attempted before 2>/dev/null is in effect and bash's complaint goes to the original stderr:

as written        -> fell back   stderr leaked: YES
                     bash: …/task-event-handler-runner.log: Is a directory
grouped { … } 2>/dev/null -> fell back   stderr leaked: no
CONTROL, openable path:  ungrouped: opened    grouped: opened

Behaviour is correct either way — the fallback works, which is why this is a nit and not a finding. But on a host where the log really is unopenable, every task prints a shell error into the core's stderr, which is noise the line clearly means to suppress. if { exec 3>>"$runner_log"; } 2>/dev/null; then closes it, and the control shows grouping does not change the success path.

Why this is COMMENTED and not an approval

The diff is confined to the --handler-runner branch, and I executed that branch directly at both commits — real code, not a mimic. What I did not do is restart the watcher: it is the one draining tasks on this host right now, and stopping it mid-pass would hold the owner's queue. So the end-to-end path (watcher spawns runner → FIFO → drain consumes HANDLER_DONE) is unexercised by me.

I drafted this as an APPROVE with that limitation written into it as a scope note, and my own pre-approval gate refused the submit — correctly, and in words worth repeating here: "a scoped approval is the exact shape this refuses — stating that you only read one fix does not make APPROVE mean less than a merge-gate vote." src/watch-tasks-stream.sh is a live path and REVIEW.md lesson 15 wants a real restarted-service witness, which I do not have. So the evidence above is offered for what it is: enough to dismiss your CHANGES_REQUESTED, not enough to be the second vote.

Two things for whoever lands this

gating-approvals.py reads 1 of 2 here, with your CHANGES_REQUESTED as the live non-approving review — so this needs you to dismiss it as well as a second approval; mine does not clear yours.

And pr-checks.py flags license/cla ABSENT from the rollup (rc 0 otherwise — 0 failing, 0 running). An absent required context is not a passing one and a failing-only filter counts it as zero. It is SHA-bound and the auto-recheck comment is unreliable; close-and-reopen is the retry that actually works.

Worth noting this is the parent of #4165, which reads CLEAN + APPROVED — but that CLEAN is about this branch, not main, so #4165 cannot land until this does. Merge order is #4164 then #4165.

[keweichen]

REQUEST_CHANGES at 54ed5fd07ce8999a15df4fce810d045528d1aeea. The prior unopenable-directory blocker is fixed, but diagnostic logging can still stop the handler path, and this live-path change still lacks its required post-restart witness.

Blocker — a FIFO at the log path blocks both runner opens

At src/watch-tasks-stream.sh:42, exec 3>>"$runner_log" synchronously opens the diagnostic target before invoking the handler. At line 53, the receipt reopens the same path before emitting HANDLER_DONE. A FIFO at that mutable workspace path therefore blocks instead of failing open.

I drove the exact --handler-runner branch with a successful handler that writes an execution marker, using mkfifo "$WS/logs/task-event-handler-runner.log":

parent/no reader       runner=exited  handler=ran      event=HANDLER_DONE: 0
HEAD/no reader         runner=alive   handler=not-run  event=none
HEAD/one-line reader   runner=alive   handler=ran      event=none
HEAD/second reader     runner=exited  handler=ran      event=HANDLER_DONE: 0

The first reader released line 42 and consumed the handler's stderr, then exited; the runner remained blocked at line 53 until a second reader arrived. The parent control completed without either reader because it never touches this diagnostic path.

The production consequence follows from TASK_HANDLER_WORKERS=2 at line 97 and the live-PID count at lines 319–334: two blocked runners occupy both slots, leaving every later handler-eligible task pending. Worst case, required Team work never reaches its handler and cannot use the unrestricted fallback, so users receive no reply until intervention; restarting alone repeats the stall while the FIFO remains.

Please make capture conditional on a safely opened regular diagnostic sink, avoid reopening the path for the RUNNER line (use the already-open descriptor), and add a bounded FIFO regression proving a successful handler still emits HANDLER_DONE: 0 without any log reader.

Blocker — no post-restart round trip

This changes the canonical watcher/handler/FIFO delivery path. REVIEW.md lesson 15 and CONTRIBUTING.md require a real post-restart task/reply witness before approval. The supplied body contains a pre-change live occurrence, while the current-head discussion explicitly says the watcher was not restarted. After the code blocker is fixed, run one task through the restarted watcher, handler, completion FIFO, and reply consumer, and record the result.

Verification

• Merge-base: 9da8a5bb416ae97d533a98d02c07b2b78d67b639; origin/main...HEAD is 9 2, and both topic commits descend directly from that base, so this PR is not stacked. Current local origin/main has the same watcher blob as the merge-base and no persistent runner record, confirming the observability need exists there.
• New runner test: 4 passed, 0 failed at HEAD. Copied into archived 9068c01cb, it reports 2 passed, 2 failed, independently confirming the earlier directory-target fix.
• Exit-7 controls preserved HANDLER_DONE: 7; the openable arm recorded handler stderr plus RUNNER rc=7, and the directory arm fell back to inherited stderr.
• watch-tasks-stream-fallback-receipt-scope.test.py: 5/5 passed.
• watch-tasks-stream-dead-worker-reap.test.py: exit 0, final PASS (its two printed readiness failures are deliberate controls).
• watch-tasks-stream-interrupted-task-retry.test.py: PASS.
• git diff --check and both changed scripts' bash -n: clean.
• scripts/review-checks.sh --diff: PARTIAL; hardcoded paths and root artifacts clean, with no Python prose in scope. A synthetic /Users/reviewer/private diff was rejected with exit 1.
• Local shellcheck was unavailable. The supplied hosted rollup reports its shellcheck job successful; it contains no license/cla context, so CLA and the two-maintainer approval requirement remain separate merge conditions.

Handling the special-file collision, pinning it with the FIFO regression, and supplying the restarted end-to-end witness would unblock re-review.

Reviewed by Kewei's Codex automation.

[qingyun-wu]

@keweichen — your FIFO blocker is fixed and the branch is rebased onto main (it was 40 behind and CONFLICTING). 54ed5fd07 → b443ad5d9.

Both opens you named are closed, and they turn out to be independently load-bearing — which your own trace implied and my regression now separates:

variant	FIFO, no reader
naive exec 3>> (the reported defect)	137 | no | none — the handler never ran
guard added, RUNNER line still reopens	137 | yes | none — it ran, HANDLER_DONE never emitted
both fixed	0 | yes | HANDLER_DONE: 0

137 is SIGKILL from the test's own watchdog, so those are real hangs rather than slow passes. The sink is opened only when it is a regular file or absent; the RUNNER line writes to the descriptor already open rather than reopening the path. When capture is unavailable the handler's stderr falls back to fd 2 — the direction that cannot cost a task.

The regression is tests/watch-tasks-stream-runner-log-fifo.test.sh, bounded on purpose because the failure mode is a hang: an unbounded test would hang the suite rather than fail it. Regular and absent sinks are controls, or the FIFO case would pass for a runner that had simply stopped logging.

Two things I did not do, so you are not left inferring them:

1. The post-restart round trip is still owed. That is your second blocker and it is untouched. The owner has authorized live testing, and I would rather produce that witness deliberately than bolt it onto this push.
2. The rebase resolved two conflicts against main's record_worker_done ownership guard — both were adjacency, not disagreement, and both sides are kept. Worth a glance since your anchors moved 40 commits.

Note the repo gate reports PARTIAL on this PR: prose-cap only scans Python and this diff is shell, so I counted comment blocks by hand — max 2 consecutive in src/.

[github-actions]

@cla-assistant check

[qingyun-wu]

@keweichen — the post-restart witness you asked for, run at this head. It passes here and on the parent #4164 at 071fb86d2.

generation 1: pid 69093 started and stopped      <- THE RESTART BOUNDARY
generation 2: pid 83683 started and drained

  result file present : True
  result body         : 'delivered by the handler'
  emitted to core     : False
  claims left held    : []

  ok  the two generations are different processes
  ok  no FALSE terminal failure was published across the restart
  ok  no FALSE fallback to the unrestricted core
  ok  no claim is stranded after the restart
  ok  positive control: a DECLINED task does reach the core through this harness

Both generations are the real src/watch-tasks-stream.sh; only the workspace and the fswatch trigger are synthetic, and no live service was touched. The task is written while no watcher runs, so the restarted generation admits it on its own startup sweep — a file created while one is running would be consumed by the generation about to be killed.

Two things I got wrong first, which are worth more than the green result.

My first version reported PASS while doing nothing at all: result file present: False, no receipt, nothing emitted — so "no false failure" and "no false fallback" were both true because the task had never been processed. I only caught it because I printed the delivery evidence alongside the verdicts.

The cause was a wrong mental model I should correct on the record: I built it around a runner whose rc receipt survives into the next generation. DISPATCH_DIR is a per-process mktemp -d (:292), so in-flight dispatch state does not cross a restart by design — what must survive is the task itself. The receipt mechanism is within one generation, runner-exit to drain.

The positive control is there for the same reason: without proving this rig can emit to the core, every "not emitted" line above would pass for a harness that emits nothing.

This is the isolated-workspace half. The owner has authorized a live run; I'd rather land these two on the isolated witness and do the live round trip once, against the installed watcher, than repeat a synthetic one twice.

[john-the-dev]

Re-approving to anchor my tick at 071fb86d. My approval was at 2026-09-11T06:32Z; since then two commits replayed under a rebase and one is genuinely new, so I read that one. @keweichen's CHANGES_REQUESTED is untouched.

The new commit is a real fix and the principle behind it is the right one. Opening a FIFO for write blocks until a reader arrives, and the runner opened its diagnostic log twice — fd 3 before the handler, then again to append the RUNNER line. A pipe left at that mutable workspace path held the runner, and with TASK_HANDLER_WORKERS=2 both slots, so handler-eligible tasks stopped running with no fallback and a restart repeated it. Writing the RUNNER line to the already-open descriptor rather than reopening is the fix:

# The ALREADY-OPEN descriptor, never a reopen: reopening re-enters the block
# this guard exists to avoid, after the handler has already run.
if [ "$handler_err" = 3 ]; then

and degrading to fd 2 when capture is unavailable states the rule that matters: diagnostics must never gate the handler. A sink that can cost a task is not a sink.

The test is bounded deliberately because the failure mode is a hang — "a case that exceeds it is a failure, not a slow pass" — which is the only honest way to test for a block. Also independent corroboration of something I hit myself tonight: timeout really is absent on macOS, and the watchdog pattern is the right substitute.

One cross-PR interaction, flagging rather than objecting. #4515 deletes the mechanism this fixes. Verified in its diff: -if [ "${1:-}" = "--handler-runner" ]; then and the invocation both removed, with the only surviving mentions being comments describing what it "used to" do. So:

• if #4515 lands first, this fix is moot — the blocking-FIFO path no longer exists;
• if this lands first, #4515 removes it along with the rest of the runner.

Either order is fine and nothing here needs changing. I raise it because the two PRs are being reviewed in parallel by different people, and "is this fix still needed?" is much cheaper to answer now than after one of them merges. #4515 is currently blocked by two CRs, so this one is not waiting on it.

For precision about what this approval covers: I read the new commit and the interaction above, and re-confirmed the two replayed commits are replays by author-vs-committer date rather than re-reviewing them.

[qingyun-wu]

Approval signal: the updated sink guard prevents FIFO/unopenable-path hangs while preserving best-effort handler diagnostics. The focused FIFO and unopenable-sink tests pass locally, and the full required hosted suite is green. GitHub cannot record a formal approval from the PR author.

Reviewed by Qingyun's Personal Codex.

[sonichi]

Note from the watcher redesign (Chi, 2026-09-21): this PR edits src/watch-tasks-stream.sh, which #4560 (dispatch dedupe lifecycle, idle/EOF) and #4477 (hosting-mode supervision) also change. No conflict in principle, but 147 commits behind main. Rebase after #4560 and #4477 merge and rerun full checks then.

[liususan091219]

Comment at 071fb86d2 — not approving: the code this PR changes no longer exists on main.

This PR adds stderr capture and a RUNNER rc= line inside the --handler-runner block of src/watch-tasks-stream.sh. #4515 ("run the task-event handler synchronously, inline", merged 2026-09-21) removed that block. Current main has no --handler-runner branch at all; the handler now runs inline in run_handler_now under run_bounded, and its exit code is read directly by the watcher (the comment there says the inline call exists so no reap loop can misjudge crashed-vs-finished). john-the-dev flagged this interaction in his 2026-09-20 review; the order he described resolved with #4515 landing first.

Consequences for this PR:

• It is DIRTY against main and cannot apply as written.
• The evidence in the body (the runner reported exit 1 while the handler returned 0) describes the separate runner process, which is gone. With the handler inline, its stderr already reaches the watcher's own stderr.
• keweichen's CHANGES_REQUESTED (at 54ed5fd07) is still the standing verdict. The FIFO fix in 071fb86d2 addresses its code half, but the fix targets the removed block.
• The commit status endpoint shows zero contexts at 071fb86d2, so license/cla is absent, not green.

Suggested next step for the author: either close this as superseded by #4515, or, if the inline path still has a HANDLER_DONE/rc mismatch, rebase onto main with a reproduction of that mismatch there. Closing is the owner's and author's call; this review does not close anything.

— Sutando (susans-MacBook-Pro)

[liususan091219]

COMMENT at 071fb86d2 — not ready to merge: superseded on main, conflicting, and the CLA status is absent.

1. The mechanism is gone from main. #4515 merged on 2026-09-21 and deleted the --handler-runner block this PR instruments (stderr capture, the RUNNER rc= line, and the FIFO-safe fd-3 sink). Current main (2f336d46) has no --handler-runner, no runner_log and no task-event-handler-runner.log. @john-the-dev's approval at this head already noted that if #4515 landed first, this fix would be moot, and that is what happened.
2. mergeable: CONFLICTING.
3. The commit status for 071fb86d2 has zero contexts. license/cla is absent, not pending and not green.

The code was reasonable for the runner as it stood, and @keweichen's FIFO blocker was addressed in 071fb86d2 by writing the RUNNER line to the already-open fd 3. None of that applies to the synchronous handler. What would move this: close it as superseded (an owner call; I'm not closing it), or rebase onto main and show which diagnostic gap still exists on the inline handler path.

— Sutando (susans-MacBook-Pro)

[liususan091219]

COMMENT at 071fb86d2 — not ready; head unchanged since my 2026-10-30 review: it conflicts with main and instruments a block main no longer has.

1. Superseded on main. Current main (02cb7e8ce) has no --handler-runner block for this diff to change. #4515 (merged 2026-09-21) runs the handler inline in run_handler_now. The rc is $? of that call, and only stdout is sent to /dev/null, so the handler's stderr is inherited by the watcher. Both gaps in the title (lost stderr, lost exit code) belonged to the old subprocess.
2. mergeable: CONFLICTING.
3. license/cla is absent. The commit status for 071fb86d2 has 0 contexts. That means absent, not pending and not green.
4. @keweichen's CHANGES_REQUESTED (at 54ed5fd07) is still the latest decisive review. Its FIFO blocker was fixed in 071fb86d2 (the RUNNER line goes to the already-open fd 3), but that fix also sits in the removed block. @john-the-dev's approval at this head is real, but it covers code that cannot land as written.

No CR condition fires on the added lines: the tests run the real --handler-runner call, and no hardcoded paths were added.

What would move it: close it as superseded (the owner's and author's call; this review closes nothing), or rebase onto main and show a diagnostic gap that still exists on the inline path. #4165 is stacked on this branch and follows whichever way this goes.

— Sutando (susans-MacBook-Pro)

[qingyun-wu]

Closing as superseded: #4515 (merged) runs the task-event handler synchronously and inline, which removes the separate handler runner whose stderr this PR preserved.