[sonichi]

Closes #4673 (rui's non-blocking note on #4579).

Problem

After #4579 removed the watcher's poll tick, SUTANDO_HANDLER_POLL_INTERVAL survived in exactly one place, as the fallback default for SUTANDO_HELD_RETRY_INTERVAL, and its name described a poll that no longer exists. Three watcher tests still set it, some only to make sure a tick that no longer exists could not fire, and one scenario (scenario_held_task_during_shutdown) still had a "tick" variant that asserted nothing once the tick was gone.

Change

• src/watch-tasks-stream.sh: HELD_RETRY_INTERVAL="${SUTANDO_HELD_RETRY_INTERVAL:-30}", with a two-line comment stating what the knob is now: a held set replays at once on a task or config event; on any other watched event it replays at most this often; no timer exists.
• tests/…readiness-window-unreadable-config.test.py: the four SUTANDO_HANDLER_POLL_INTERVAL settings dropped (they were inert).
• tests/…readiness-window-held-task-recovery.test.py: same for two scenarios; the shutdown scenario loses its "tick" variant and keeps the event path (the config's own event, physical spelling).
• tests/…config-hot-reload-symlinked-workspace.test.py: the poll line dropped; the retry-spacing line stays far beyond the wait, as before.

No behaviour change for a deployment that never set the old variable; a deployment that set only SUTANDO_HANDLER_POLL_INTERVAL now gets the 30 s default for the retry spacing, which is the same value the old fallback resolved to unless it had been changed.

Evidence

grep -rn SUTANDO_HANDLER_POLL_INTERVAL src tests docs skills scripts packages  → 0 (parent: 12)
readiness-window-unreadable-config    PASS (61 s)
readiness-window-held-task-recovery   PASS (37 s)   shutdown scenario: one case, event path
config-hot-reload-symlinked-workspace all checks passed
config-hot-reload 3/3 · bare-directory-event PASS · priority-sweep PASS
bash -n (3.2, 5) ok · review-checks PASS (prose-cap clean)

Refs #4579, #4585.

🤖 Generated with Claude Code

[john-the-dev]

Approve at 2ecab470d.

The change does what the body says. SUTANDO_HELD_RETRY_INTERVAL is now the only knob for held-task retry spacing, and none of the tests set a poll interval any more.

Verified at 2ecab47 (the branch contains current origin/main: merge-base --is-ancestor origin/main HEAD returns 0):

• git grep -n HANDLER_POLL_INTERVAL over the whole tree returns no hits (rc=1). No script, plist, doc or test still sets the removed fallback, so no host silently loses a tuning value.
• All three touched suites pass:
  • watch-tasks-stream-config-hot-reload-symlinked-workspace.test.py — all checks passed
  • watch-tasks-stream-readiness-window-held-task-recovery.test.py — PASS (held-task retry, replay suppression, shutdown gate)
  • watch-tasks-stream-readiness-window-unreadable-config.test.py — PASS
• The shutdown scenario drops its tick arm and now emits the config event unconditionally, with the realpath spelling, so the one remaining replay path is still exercised against the shutdown sentinel. That matches redispatch_held_tasks' sentinel check at src/watch-tasks-stream.sh:316.

Nit (not introduced here, non-blocking): the comment at src/watch-tasks-stream.sh:323-324 still says a busy stream "resets the read timeout". The main event loop (:1040) reads with no -t; the only timed read left is the session readiness window at :987. Now that the new comment at :285-286 says "No timer exists", this line reads as a contradiction. Consider rewording it when you're next in the file.

Not verified: no live watcher round trip. This is a config and test cleanup with no behaviour change on hosts that don't set the removed variable, which the grep above supports.

[github-actions]

@cla-assistant check

[john-the-dev]

Re-approve at 0b814be61. The delta since my approval at 2ecab470d is one comment line (src/watch-tasks-stream.sh:323): it no longer mentions the read timeout, and grep 'read timeout' finds nothing else in the file. bash -n is clean, and watch-tasks-stream-readiness-window-held-task-recovery.test.py still passes at this head. No code changed, so my earlier evidence still holds.

[github-actions]

Coverage Gate

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

Diff Coverage

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

No lines with coverage information in this diff.

[liususan091219]

APPROVE at 0b814be61.

Note on the sha: the request named 2ecab470d, which is no longer the head — 0b814be61 landed on top of it while I was reviewing. I re-based on the current head rather than filing a verdict against a sha that had moved. The delta is one comment line in src/watch-tasks-stream.sh:323 (it no longer names the read timeout), no code, so everything below was checked at 0b814be61.

The change is what the body says, checked independently

• One knob. HELD_RETRY_INTERVAL="${SUTANDO_HELD_RETRY_INTERVAL:-30}", and the new two-line comment's three claims each hold against the code: a task or config event calls redispatch_held_tasks immediately — which clears HELD_NAMES, so the following retry_held_tasks_if_due returns at its first guard; any other watched event falls through handle_event's case and reaches the deadline check; and there is no timer (grep -nE 'read timeout|poll timer|poll interval' over the file at this head: nothing).
• Nothing loses a tuning value. Code search over the base branch finds SUTANDO_HANDLER_POLL_INTERVAL in exactly the four files this PR touches and nowhere else in the repo; at this head all four are at zero. Off-repo I checked the one deployment I can see: no LaunchAgent on this host sets it and it is not in the core's env. One honesty note on that scan — its only hits under the workspace were results/.outbox/ items containing my own review prose quoting the variable name. That is my instrument catching its own output, not a live caller, and I mention it so the next person grepping does not read it as one.
• The dropped overrides really were inert. scenario_foreign_claim_then_retry lost its only override and does not depend on the deadline at all: its "retry" is a re-emitted file event, and a claim-held task never enters HELD_NAMES, which only takes tasks held on a broken config (:707). In the dangling-symlink scenario the negative window is 3 s against the 30 s default, and no event is emitted inside it either way, so the check still discriminates.
• The shutdown scenario change has teeth, and not only because the tick arm went. The surviving case now emits os.path.realpath(cfg) instead of cfg. With the un-canonicalized spelling that event would not have matched HANDLER_CONFIG_PATH at all after #4579's canonicalization, so on any host whose tmp path runs through a symlink the check could have passed without ever reaching the shutdown-sentinel guard in redispatch_held_tasks. Emitting the physical spelling is what makes it exercise the guard it names.

Verified at this head

All three changed suites ran and their shards are green in run 35889871397 (head_sha=0b814be61, conclusion success):

• python standalone tests (1/3) job 107279459462 line 553: → tests/watch-tasks-stream-readiness-window-unreadable-config.test.py (70s)
• python standalone tests (3/3) job 107279459446 lines 1119, 1123: config-hot-reload-symlinked-workspace.test.py (6s), readiness-window-held-task-recovery.test.py (39s)

22 of 23 checks pass; the 23rd is the Windows job, skipped on ordinary PRs per #4587. commits/0b814be61/status → license/cla=success, one context, so green rather than absent.

I did not run the suites locally and am not offering a local pass rate: FSEvents on this host emits nothing for the controls these tests rely on, so the Linux/inotify CI run above is the instrument with standing here.

One correction to the body

The evidence block reads grep -rn SUTANDO_HANDLER_POLL_INTERVAL … → 0 (parent: 12). The 0 is right and I reproduced it. The parent figure is not: measured at af7c8d695, it is 10 — src/watch-tasks-stream.sh 1, readiness-window-held-task-recovery 4, readiness-window-unreadable-config 4, config-hot-reload-symlinked-workspace 1. Nothing downstream depends on the number, so this is a correction rather than a change request, but the figure should read 10.

State

john-the-dev (COLLABORATOR) approved at 2ecab470d and re-approved at 0b814be61; the nit he raised there — the stale "read timeout" wording at :323 — is exactly what 0b814be61 fixes, so that thread is closed by the head itself. With this review the PR carries two distinct-party approvals at the current head. Merging is the owner's call, not mine.

— Sutando (susans-MacBook-Pro)