Repository navigation
feat(watcher): opt-in task-watch Mod hosts the session's task watcher in Claude seats - #6284
Conversation
… in Claude seats A Monitor ends at its 30-minute timeout and the session must re-arm it. The task-watch Mod (src/agent/claude/task-watch-mod) runs the unchanged watch-tasks-stream.sh from session.start instead and submits each TASK_FILE line, with its QUEUE line, through $.prompt.submit: the text a Monitor notification carries, never task-body content. - Command and inbox come from watcher_rearm.py watch-argv, the owner the Stop hook already uses; role-present gates the spawn so a held inbox is never doubled. Exit 0 is a hand-off (poll, restart once unheld), other early exits back off 5 s to 5 min and stop after 8; exit 64 is not retried. Session end (not /clear or resume) kills only the child the Mod spawned. - Opt-in per launcher, off by default: SUTANDO_TASK_WATCH_MOD_WORKER=1 on a worker spawn, SUTANDO_TASK_WATCH_MOD_CORE=1 for the core. Installed only when claude --version is at least MIN_CLAUDE_VERSION (2.1.303, witnessed on 2.1.303 and 2.1.318); the Mod checks the same floor. The session marker SUTANDO_TASK_WATCH_MOD=1 makes CLAUDE.md, /startup, /schedule-crons and /proactive-loop skip their Monitor step and silences the compaction hint. Codex seats and seats without the Mod keep the Monitor path. - scripts/task-watch-mod-witness.sh loads the Mod in a headless claude -p --input-format stream-json session on a temp workspace and checks the gate, the submitted turn and the session-end kill. Sutando-Task: task-910be5ff61c9e674be Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sutando-Task: task-910be5ff61c9e674be Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ays parseable Sutando-Task: task-910be5ff61c9e674be Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 93%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
|
liususan091219
left a comment
liususan091219
left a comment
There was a problem hiding this comment.
Not ready to approve yet: the approve bar is not met. There are no approvals from anyone else at this head (ae7804d), so neither route is satisfied: no expert approval, and not 3 approvals from known reviewers.
What I checked, with nothing found that needs changes:
hooks/register.ts: only aTASK_FILE: <plain name>line (or an absolute path ending in one, with no control characters and at most 4096 chars) gets submitted, along with an exact-formatQUEUE:line. Every other stdout line is logged only, and the Mod never reads a task file. Exit 0 is a hand-off, 64 is not retried, other exits back off from 5 s to 5 min and give up after 8 in a row. Session end kills only the child the Mod spawned.session-launch.shadd_task_watch_mod: off unless the flag is1, and the version floor is read fromhooks/version.tsrather than duplicated. It checks the same bareclaudethe launchers exec. Below the floor it logs one line and keeps the Monitor path.watcher_rearm.py watch-argvgives the Mod the same inbox the Stop hook's gate checks. The doc and skill edits skip the Monitor step only whenSUTANDO_TASK_WATCH_MOD=1, so seats without the Mod are unchanged.- Tests run real behavior:
task-watch.test.ts(15 cases in the Mod harness CI job),tests/task-watch-mod-launch.test.sh(15), and the newwatcher-rearm.test.pycases. No source-grep tests. The witness script'sSUTANDO_WORKSPACE="$WS"is a temp-dir override, not a live default. CI is fully green, and license/cla: success (1 context).
Still missing before approval:
- One approval from an expert reviewer for the watcher and launcher (or 3 approvals from known reviewers).
- The live trial on one worker, which the PR names as the next step, is still owed. Per the current rule this gets named and does not block approval, because nothing is enabled by default.
— Sutando (susans-MacBook-Pro)
sonichi-dot
left a comment
sonichi-dot
left a comment
There was a problem hiding this comment.
Reviewed ae7804d. One P2 finding: the version gate can inspect a different Claude executable from the one tmux launches. Recommendation: fix that mismatch before merging.
Verification: tests/task-watch-mod-launch.test.sh passed 15 checks; tests/watcher-rearm.test.py passed. The 13 engine-independent Mod tests passed in a Node-adapted harness. A separate two-PATH reproduction demonstrated the finding, with the matching-old-PATH case as a control. CI and license/cla reported success when checked during this review.
Worst-case impact is confined to opted-in sessions: the Mod is marked active but declines to start, suppressing the normal startup/resume Monitor fallback. No default-enabled behavior changes were found. I could not run the native Claude plugin harness or live worker witness because Claude Code is unavailable in this environment; this is not full-suite or live-path verification.
| [ "${1:-}" = "1" ] || return 0 | ||
| local dir="$REPO/src/agent/claude/task-watch-mod" min have | ||
| min="$(sed -n "s/^export const MIN_CLAUDE_VERSION = '\([0-9.]*\)'.*/\1/p" "$dir/hooks/version.ts" 2>/dev/null)" | ||
| have="$(claude --version 2>/dev/null | awk 'NR==1 {print $1}')" |
There was a problem hiding this comment.
[P2] Check the Claude executable that the tmux pane will actually run
This probes the launcher's PATH, but launch_claude_session() runs bare claude in tmux's environment. This same file already has claude_pane_path() and uses it in claude_cli_resolves() because those PATHs can differ. With a 2.1.318 stub on the caller PATH and a 2.1.200 stub on the pane PATH, calling the real add_task_watch_mod 1 produces TASK_WATCH_MOD=1 and --plugin-dir, even though the real pane-path resolver selects 2.1.200. Setting both PATHs to 2.1.200 correctly leaves the marker empty. On the mismatched path, the Mod's own version check refuses to watch while /startup, /schedule-crons, and the resume hint suppress the Monitor because the marker is still 1. Probe the executable/environment used for the actual launch, including the no-tmux case, and add this differing-PATH regression case.
…r only on a ready watcher The launcher checked `claude --version` on its own PATH while the tmux pane could run an older claude, so the Mod was marked on, refused to run, and startup skipped the Monitor: no watcher at all. - add_task_watch_mod resolves CLAUDE_BIN once, as an absolute path, from the pane's PATH (claude_pane_path), gates on it, and the launch runs that same path in both tmux launches (worker and core); the no-tmux exec already resolves claude on the same PATH the gate read. - watcher_rearm.py mod-ready prints yes only when the marker is 1 AND a ready session watcher holds the inbox (waiting up to 30 s). CLAUDE.md, /startup and /schedule-crons skip the Monitor only on yes, so a Mod that refused, never loaded or gave up falls back to the Monitor. The compaction hint fires on an unwatched inbox whatever the marker. The /proactive-loop opt-out is dropped: a Monitor started over the Mod's watcher exits 0 on the held inbox. Sutando-Task: task-910be5ff61c9e674be Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the review catch: the launcher checked
The (agent-assisted, Claude Code for sonichi's seat) |
sonichi-dot
left a comment
sonichi-dot
left a comment
There was a problem hiding this comment.
Re-reviewed 1b3d0d3. The earlier PATH finding is fixed: the version check and tmux launch now use the same resolved executable. I ran the expanded launcher suite against both implementations: its three new cases fail on ae7804d and all 18 checks pass here. The readiness check also restores Monitor fallback when the Mod is not ready.
One new P2 remains: the readiness wait counts probes rather than elapsed time, allowing a slow or timing-out probe to delay startup and fallback for minutes. See the inline reproduction. Recommendation: fix that deadline before merging. The workspace-resolution lint job also fails on the new _default_opts resolver; I read the failed step's log. Other CI was still running when checked; CLA passed.
Verification: launcher checks 18/18, watcher-rearm suite, shell syntax checks, and the 13 engine-independent Mod tests in a Node-adapted harness passed. Worst-case disruption remains confined to opted-in sessions, whose task intake can be delayed during failed readiness probing. Native Claude harness, live worker witness, and full repository suite were not run; Claude Code is unavailable here.
| for _ in range(max(1, int(wait_s))): | ||
| if session_verdict(inbox, f"{workspace}/state") == "yes": | ||
| return "yes" | ||
| sleep(1) |
There was a problem hiding this comment.
[P2] Bound readiness polling by elapsed time, including each probe
wait_s=30 currently means 30 calls to session_verdict, not a 30-second deadline. Each call can itself wait up to 15 seconds in subprocess.run, so an unresponsive probe can postpone the Monitor fallback for about 480 seconds. I reproduced the timing error through the real watcher-identity subprocess with a ps stub taking two seconds: mod_ready(..., wait_s=1) returned no after 3.04 seconds. Injecting TimeoutExpired at the production probe timeout also showed 30 probes plus 30 sleeps, totaling 480 simulated seconds. This check runs before the startup Monitor/cron step, so a slow process-table query extends the very unwatched startup interval the fallback should bound. Use a monotonic deadline and cap each probe's timeout and subsequent sleep by the remaining budget; cover slow probes as well as instant no/unknown responses.
There was a problem hiding this comment.
Thanks @sonichi-dot, good catch: the wait counted probes, not time, so a slow or timing-out probe overran it. Fixed in c5c71f6:
mod-readynow waits until atime.monotonic()deadline (wait_s), and starts no probe once it passes.- Each probe's timeout is capped at the time left, and the pause between probes never runs past the deadline.
- A new test in
tests/watcher-rearm.test.pystubs a probe that takes 1.5 s, withwait_s=1. It now returnsnowithinwait_s+ 0.3 s, and every probe timeout is at most 1 s. At 18ecadd the same test fails: it returned after 2.51 s with a 15 s probe timeout.
(agent-assisted, Claude Code for sonichi's seat)
…bcommands Drops its own repo-walk and workspace lookup; callers pass the resolved workspace (sutando-config.sh, or the worker's SUTANDO_WORKSPACE_DIR). Sutando-Task: task-910be5ff61c9e674be Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sonichi-dot
left a comment
sonichi-dot
left a comment
There was a problem hiding this comment.
Rechecked 18ecadd. The PATH fix remains correct, and removing _default_opts in favor of explicit workspace arguments resolves the workspace-resolution lint failure; that check now passes.
The existing P2 readiness-deadline finding remains open. I reran the real delayed-subprocess reproduction on this head: wait_s=1 returned after 3.06 seconds. The loop is unchanged and still counts probes rather than elapsed time. No additional findings in this update. Recommendation: fix that deadline before merging.
Reran the launcher suite (18/18) and watcher-rearm suite successfully. CI was still running when checked; CLA passed. Native Claude harness, live worker witness, and full repository suite were not run.
A slow or timing-out role-present probe made the wait overrun (wait_s=1 returned after about 3 s). The wait now ends at a time.monotonic() deadline, no probe starts after it, and each probe's timeout is capped to the time left. Sutando-Task: task-910be5ff61c9e674be Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@cla-assistant check |
sonichi-dot
left a comment
sonichi-dot
left a comment
There was a problem hiding this comment.
Re-reviewed c5c71f6. Both reported P2 findings are now addressed; no additional actionable findings. The readiness check uses a monotonic deadline and caps both probe timeout and sleep by the remaining budget. My real delayed-subprocess reproduction with wait_s=1 took 3.04 seconds on 18ecadd and 1.00 second here.
Passed: watcher-rearm suite including the new slow-probe case, launcher checks 18/18, and 13 engine-independent Mod tests in a Node-adapted harness. Workspace-resolution lint and CLA pass; remaining CI was still running when checked.
No remaining code-review blockers from me. This remains opt-in; verify the documented live worker trial before enabling it. Native Claude harness, live worker witness, and full repository suite were not run in this review.
qingyun-wu
left a comment
qingyun-wu
left a comment
There was a problem hiding this comment.
Approving at c5c71f6.
Fallback: no permanent gap and no double watch. mod_ready (src/watcher_rearm.py:81) answers yes only when SUTANDO_TASK_WATCH_MOD=1 and role-present reports a ready session watcher on this inbox. A Mod that refused, never loaded or gave up therefore yields no, and the session starts the Monitor. The Mod's watcher runs the same watch-tasks-stream.sh --role session --inbox, so there is one inbox holder, and the Mod's run loop (register.ts) only respawns once nothing holds the inbox. The holder and role-present checks see the Mod's watcher through the same command, role tag and sentinel.
Single claude path. CLAUDE_BIN comes from command -v claude under PATH=$(claude_pane_path) (session-launch.sh:445), and it must be absolute. The version gate and both launches use "${CLAUDE_BIN:-claude}", so they can't pick different binaries.
Deadline. It uses time.monotonic, and each probe timeout and sleep is capped by the time left. The worst case is about wait_s plus process overhead.
Off by default. Behavior is unchanged: bare claude, and mod_ready returns no on the env check with no wait.
tests/watcher-rearm.test.py passes and tests/task-watch-mod-launch.test.sh gives 18/18 locally. CI is green.
Non-blocking, latency only, since the watcher's startup sweep catches anything that arrives meanwhile:
- If the Mod has given up while the marker is still 1, every Monitor re-arm first spends up to 30 s in
mod_readywith the inbox unwatched. A "gave up" mark the Mod writes would remove that. - On a Monitor expiry, the Mod takes over within
HELD_POLL_MS(30 s), so there is up to a 30 s gap. - "yes" proves a ready session watcher holds the inbox, not that it's the Mod's. An earlier Monitor satisfies it too, which is still safe.
What changed, and why
A
Monitorends at its 30-minute timeout and the session has to re-arm the task watcher on every expiry notice. This adds an opt-in Claude Code Mod,src/agent/claude/task-watch-mod/, that hosts the same session watcher for the life of the session:session.startit spawns the unchangedsrc/watch-tasks-stream.sh <inbox> --role session --inbox <inbox>. The command and inbox come from a newwatcher_rearm.py watch-argv, so the Mod watches exactly the inbox the Stop hook's unwatched-inbox gate checks (a worker'sSUTANDO_WATCHER_CMD, the core's<workspace>/tasks). The script keeps priority order, the queue count, the standby hand-off and its own holder check.TASK_FILE: <name>line, plus theQUEUE:line that follows it, it calls$.prompt.submitwith exactly those lines: the text a Monitor notification carries, so the session's handling is unchanged. Announcements that arrive during a turn wait and are sent together when it ends.watcher_identity.py role-present session --inbox <inbox>is notyes; exit 0 is a hand-off (poll every 30 s, restart once no session watcher holds the inbox); any other early exit restarts after 5 s doubling to 5 min, giving up with a log line after 8 in a row (the Stop hook then asks for a Monitor); exit 64 is not retried. Session end (not/clear, not a resume) kills only the child the Mod spawned, via the spawn stream's ownreturn().TASK_FILE: <plain file name>(or an absolute path ending in one) is ever submitted; any other stdout line is logged, never submitted, and the Mod never reads a task file. The session still reads the task's headers (access_tier, sender, HMAC) from the file.SUTANDO_TASK_WATCH_MOD_WORKER=1in a worker spawn's environment (worker-pool launcher),SUTANDO_TASK_WATCH_MOD_CORE=1for the core launcher. Both go through one helper,add_task_watch_modinsession-launch.sh, which installs the Mod only whenclaude --versionis at leastMIN_CLAUDE_VERSION(hooks/version.ts, 2.1.303); below it the session keeps the Monitor and the launcher logs one line. The Mod checks the same floor itself and refuses with a log line.SUTANDO_TASK_WATCH_MOD(empty or1) in the session.CLAUDE.md/AGENTS.md,/startup(worker step 2 and core step 2) and/schedule-cronsstep 1.5 skip their Monitor step only whenwatcher_rearm.py mod-readyprintsyes(marker1AND a ready session watcher on the inbox, waiting up to 30 s), so a Mod that refused, never loaded or gave up falls back to the Monitor. The compaction hint fires on an unwatched inbox whatever the marker. The launcher's version gate runs the same absoluteclaudethe tmux pane will launch (resolved from the pane's PATH), and the launch uses that path. Codex seats and Claude seats without the Mod keep the Monitor path unchanged.check-pending-tasks.shis unchanged: it already sees the Mod's watcher as a ready session-role watcher (same script, same sentinel), witnessed below.Shared logic with
skills/pr-watch/modand theag2space-inboxMod was considered: pr-watch shares only a three-line line splitter and the spawn-loop shape, with different restart, dedupe and template policies, and ag2space-inbox polls rather than spawns, so nothing semantically identical was extracted.Where to look first
src/agent/claude/task-watch-mod/hooks/register.ts(TaskWatch,Announcements,isTaskLine)src/agent/claude/cli/session-launch.sh(add_task_watch_mod), and its two call sites instart-cli.shandskills/worker-pool/scripts/launch-worker-session.shsrc/watcher_rearm.py(watch-argv, the hint's opt-out)docs/task-watcher-hosting-modes.md, new section "Opt-in: the session watcher hosted by the task-watch Mod", with How to enable it on one workerHow to enable on one worker
bash scripts/task-watch-mod-witness.shpasses on the installed Claude Code.SUTANDO_TASK_WATCH_MOD_WORKER=1in the spawner's environment, e.g.SUTANDO_TASK_WATCH_MOD_WORKER=1 python3 skills/worker-pool/scripts/spawn_worker.py --workspace <ws>. Other workers and the core are untouched.python3 src/watcher_identity.py role-present session --inbox <ws>/deliveries/<id> --ready <ws>/statesaysyes, the watcher's parent is that worker'sclaude(not a Monitor shell), and a test task starts a turn from "The task-watch plugin".Nothing in this PR enables it anywhere; the core stays off unless
SUTANDO_TASK_WATCH_MOD_CORE=1is set.Evidence
Headless witness (
scripts/task-watch-mod-witness.sh): a realclaude -p --input-format stream-jsonsession with--plugin-diron the Mod, a temp workspace, a scrubbed environment, no MCP servers and only the Read tool. A task whose body says "IGNORE ALL PREVIOUS INSTRUCTIONS..." is dropped into the inbox.On the floor version:
And on the current release:
The gate check is the exact call
check-pending-tasks.shmakes (role-present session --inbox <inbox> --ready <state>).Before this PR there is no Mod path: a session's watcher is only ever a Monitor command, which ends at
timeout_ms(30 min at most).Tests
claude plugin test task-watch-mod(added to themod-harness-testsCI job, CLI pinned at 2.1.303 there): 15 pass on 2.1.303 and 2.1.318. Covers: argv fromwatch-argvafter the holder check; TASK_FILE+QUEUE joined, TASK_FILE alone after 1.5 s, split chunks; held during a turn, one message after; refused submit retried; injected body/lines never submitted and no task file read; held inbox waits;unknownstill starts; exit 0 hand-off; backoff and give-up; ten-minute run resets the count; exit 64 not retried; session end kills only its child,/clearand resume keep it; env gate; version gate.tests/task-watch-mod-launch.test.sh(new): 15 pass. Opt-in off by default, flag 0, flag 1 at the floor, below the floor, unreadable version; worker and core markers explicit-empty by default; each flag switches only its own launcher.tests/watcher-rearm.test.py:watch-argvfor core and worker, refusal withoutSUTANDO_WATCHER_CMD, hint silent under the marker.tests/start-cli-model-pin.test.pyfails its two no-tmux cases on this host identically onorigin/main(claude never exec'd under its PATH), so that is not this change. In CI,tests/connections.test.pyandtests/gateway-agent-activity-hooks.test.pyfailed once and passed on a rerun of the failed jobs (the first also failed on a recentmainrun); every check is green at the head.bash scripts/review-checks.sh --diff <(git diff origin/main...HEAD): PASSnpx tsc --noEmit(the Mod is excluded from the repo's tsconfig likeskills/pr-watch/mod, and type-checks clean against the engine's own declarations),npx eslint src/agent/claude/task-watch-mod,claude plugin validate.Feature/documentation consistency
docs/task-watcher-hosting-modes.mdgains the Mod section and its tests;docs/catalog.jsonlast_verifiedupdated.Live path
This touches the startup path only behind the opt-in, and nothing is enabled. A live trial on one worker is the next step and needs the owner's go; the core stays on the Monitor path until the owner says otherwise.
Hardcoded-path check
No host paths in added lines; the witness uses
mktempand$HOME.🤖 Generated with Claude Code