Skip to content

fix(watcher): fswatch through a line-atomic relay; run the pointer migration once; pin the answered rule (stacked on #5162) - #5170

Merged
qingyun-wu merged 2 commits into
fix/watcher-retire-answered-pointersfrom
fix/watcher-sweep-followups
Oct 7, 2026
Merged

qingyun-wu merged 2 commits into
fix/watcher-retire-answered-pointersfrom
fix/watcher-sweep-followups

Conversation

@qingyun-wu

@qingyun-wu qingyun-wu commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #5162. Base is fix/watcher-retire-answered-pointers. Merge order: #5162 first, then this PR. After #5162 merges I will rebase this onto main, change the base, and rerun the full checks.

What changed, and why

This PR covers marklysze's three non-blocking asks on #5162.

1. FIFO line integrity under an fswatch burst: measured, then fixed

I built a harness, tests/inbox-events-burst.test.py, to test this. It works as follows:

  • A real fswatch with the watcher's exact flags writes into a FIFO.
  • At the same time, a sweep-shaped writer (one printf per line, 3,000 lines) writes into the same FIFO.
  • The test creates thousands of files at once.
  • A deliberately slow reader takes 256 B every 2 ms, so the pipe fills.
  • Every line read is checked against the exact set of expected paths.
  • BURST_LAUNCH=direct runs the launch that fix(watcher): retire answered inbox pointers in the archive step; a restart sweep never blocks new tasks #5162 ships (fswatch writing straight into the FIFO), for comparison.

Measured on macOS 26 with fswatch 1.18.3 and 3,000 sweep lines:

launch files lines read malformed / merged burst events lost fswatch
direct (#5162) 300 3300 0 0 alive
direct (#5162) 600 3601 0 0 alive
direct (#5162) 1000 3374 0 627 SIGSEGV
direct (#5162) 3000 3240 0 2760 SIGSEGV
relay (this PR) 1000 4000 0 0 alive
relay (this PR) 3000 6000 0 0 alive

Raw output, direct launch:

bash: line 1:   999 Segmentation fault: 11  fswatch -l 0.5 --event Created --event Renamed --event Updated ".../inbox" > ".../events" 2> /dev/null
  launch=direct files=3000 sweep_lines=3000 lines_read=3240 malformed=0 burst_missing=2760 sweep_missing=0 fswatch_alive=False took=11.0s

Raw output, relay launch:

  launch=relay files=3000 sweep_lines=3000 lines_read=6000 malformed=0 burst_missing=0 sweep_missing=0 fswatch_alive=True took=18.1s

I also isolated the crash without any sweep writer:

  • With no reader at all, fswatch blocks and stays alive.
  • It crashes when a large backed-up batch starts draining, even with a fast reader. A burst of 400–600 events survives; 3,000 crashes every time.

On Linux (fswatch 1.14.0 with inotify, in Docker), both launches read all 9,000 lines (Created plus Updated events) with 0 malformed lines and no crash.

Conclusion.

Fix. fswatch never writes to the FIFO itself:

  • src/inbox-events.sh start_inbox_events launches fswatch ... > >("$py" src/line_relay.py > "$fifo") &.
  • $! is still fswatch's pid, so FSWATCH_PID keeps its meaning for liveness checks, wait and cleanup.
  • src/line_relay.py reads fswatch's output into memory as soon as it arrives, so fswatch is never blocked. It forwards each line in its own write(), which keeps the line whole beside the sweep's lines as long as it is ≤ PIPE_BUF (512 bytes on macOS).
  • When fswatch exits, the relay exits too, so the watcher's existing EOF → relaunch → sweep path is unchanged.

Two test shims needed adjusting, because the relay runs on the injected interpreter:

  • readiness-window-decision-instant wraps SUTANDO_PY in a shim that buffers all output.
  • priority-sweep uses a broken-helper stub that exits 0.

Both now pass line_relay.py straight through to the real interpreter. The second one failed in this PR's first CI run on Linux.

2. retire-archived-pointers: documented and run once automatically

  • Docs. docs/worker-pool-design.md now has a section "Retiring a finished sentinel". It says the archive step is the one owner of "done", what the migration moves, why it is idempotent, and when it runs.
  • Wiring. retire_archived_pointers_once in src/watch-tasks-stream.sh works as follows:
    • It runs inside the sweep's background producer, before the first plan, so it never delays reading a new event.
    • It runs only when state/migrations/retire-archived-pointers.v1.done is absent.
    • Each inbox's pointers move under that inbox's .lock, the same lock the archive step uses.
    • The run is recorded in the marker, written to a temp file and then renamed into place.
    • A failed run writes no marker and retries on the next start.
    • It logs only when it retired something.
  • Why automatic rather than manual. It is idempotent and never touches a pending pointer. Its only effect is to shrink the next sweep, and a manual step would be forgotten on every other install.
  • I did not run it on the live seat. It will run once on the seat's next watcher start after this merges.

tests/watch-tasks-stream-pointer-migration.test.py: a real watcher starts on a temp worker inbox with 3 archived pointers and 1 pending pointer.

At the parent (#5162 head):

  FAIL the first start migrated the archived pointers
  FAIL ...before the sweep planned the inbox
  FAIL the marker records the run
FAIL — 3 check(s) failed

At HEAD:

  ok   the first start migrated the archived pointers
  ok   ...before the sweep planned the inbox
  ok   ...and left the pending pointer in place
  ok   the marker records the run
  ok   a later start does not run it again
  ok   ...so a pointer archived afterwards stays for the archive step
PASS

watch-tasks-stream-sweep-does-not-block-new-tasks now pre-writes the marker. Its 3,000 pointers therefore stand for leftovers on a migrated workspace; otherwise the migration would retire them all before the plan ran.

3. Parity of the "answered" rule, shell vs Python

tests/answered-rule-parity.test.py runs one table of 19 cases through both implementations:

  • the shell handler_result_is_answer, lifted verbatim from src/watch-tasks-stream.sh together with its own TERMINAL_REFUSAL_MARK;
  • task_dispatch._is_answer.

The cases cover: no result, an answer with and without a trailing newline, a refusal with and without a trailing newline, CRLF, the refusal phrase mid-line, a refusal on line 2, a bare CR, an indented refusal, whitespace-only and empty placeholders, archive-only results, an empty live file with an archived answer or refusal, non-UTF-8 bytes, and a directory at the live name.

The test found a real divergence, so I fixed the shell rule. The old line was IFS= read -r first < "$ready" || first="". On a last line with no newline, read returns 1 even though it filled $first, and the old line then cleared it. So a refusal written without a trailing newline was treated as an answer, and the task was never re-run. The watcher's own refusal writer always appends a newline, so only refusals from other writers or truncated ones were affected. The new line is || true.

At the parent:

  FAIL terminal refusal without a trailing newline: shell=True python=False expected=False
  FAIL refusal prefix and nothing else: shell=True python=False expected=False
FAIL — 2 of 19 case(s) disagree

At HEAD: all 19 cases agree. PASS.

Review first

  • src/line_relay.py and src/inbox-events.sh, plus the one-line change in launch_fswatch.
  • retire_archived_pointers_once in src/watch-tasks-stream.sh.
  • The handler_result_is_answer line change.

Docs

docs/worker-pool-design.md: a new "Retiring a finished sentinel" section, and the relay is described in the watcher-liveness paragraph. docs/src-map.md is regenerated for the two new src/ files.

Tests run

All suites ran with every SUTANDO_* variable unset and a temp workspace. The set is: tests/watch-tasks-stream*, watcher-*, task-dispatch-*, task-archive, delivery-pointer-retirement, tests/skills/worker-pool/*, worker-pool-design-*, pool-*, check-pending-tasks-*, core-never-names-the-worker-pool-skill, sync-from-src-repo-relative, watch-tasks-dispatch-activity, plus the three new tests.

CI at HEAD

Shell lanes, serial tail, shellcheck, ruff and the license (CLA) check are green. Only three python tests fail:

  • tmux-session-targets-are-exact
  • agy-task-notifier
  • agy-notifier-dependency-gate

These three fail on main and on #5162 too.

external-core-helpers failed once and passed on rerun. It compares a shared /tmp/core-heartbeat.log that parallel tests write. tsc + tests (clean install) is the job that aggregates those python shards.

No host paths appear in added lines.

🤖 Generated with Claude Code

…ter migration once; pin the answered rule

Follow-up to #5162 (review asks from marklysze):

- fswatch now writes to src/line_relay.py, which drains it at once and forwards
  each line in its own write() to the event FIFO. Measured on macOS: fswatch
  1.18.3 segfaults when a burst (~1,000+ events) backs up behind a full pipe;
  through the relay it survives, and no line is ever split or merged beside
  the sweep's writes. src/inbox-events.sh `start_inbox_events` is the launcher.
- The watcher runs `task_archive.py retire-archived-pointers` once per
  workspace, before its first sweep plan, under each inbox's .lock, and records
  the run in state/migrations/retire-archived-pointers.v1.done. Documented in
  docs/worker-pool-design.md.
- tests/answered-rule-parity.test.py runs the shell and Python "answered" rules
  over one table. It found that the shell rule read a refusal with no trailing
  newline as an answer (`read || first=""` cleared the line); fixed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@qingyun-wu
qingyun-wu requested a review from sonichi as a code owner October 6, 2026 21:19
@qingyun-wu
qingyun-wu requested a review from marklysze October 6, 2026 21:19
The stub interpreter that proves the sweep's mtime fallback now also stood in for
src/line_relay.py, so no event reached the watcher on Linux. It passes the relay
through to the real interpreter, as readiness-window-decision-instant does.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

@cla-assistant check

@qingyun-wu
qingyun-wu merged commit 553ef5e into fix/watcher-retire-answered-pointers Oct 7, 2026
32 of 40 checks passed
@qingyun-wu
qingyun-wu deleted the fix/watcher-sweep-followups branch October 7, 2026 01:12
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