feat(watch): add coco watch — continuous review/commit-draft daemon - #1983
Conversation
Extracts the debounce/quiesce + rename-survival logic from workstation/chrome/refreshWatcher.ts down into src/lib/watcher/ (repoChangeWatcher.ts) so src/commands/watch can reuse it without `commands` reaching sideways into `workstation`. refreshWatcher.ts becomes a thin re-export shim so existing workstation imports are unaffected. `coco watch` re-runs review and/or commit-draft generation (via src/operations/agent/) each time a settled change set differs from the last-seen diff digest, floored to one LLM call per --interval regardless of edit frequency. Supports --once for a single pass, --staged to scope to the index, and --json for line-delimited editor-integration output.
There was a problem hiding this comment.
🔎 Agent review (sonnet→opus) — CONCERNS
REVIEW: CONCERNS
RESOLVES: full
Implements coco watch per the work-item sketch — extracts refreshWatcher's debounce/rename-survival logic down into src/lib/watcher behind a compatible re-export shim, adds a well-tested throttled runner for cost containment, and wires --review/--draft/--staged/--once/--json through operations/agent with a content-digest guard. Layering and tests hold up; two non-blocking design concerns remain around redundant diff resolution and event ordering on shutdown.
2 concerns — 2 inline on the diff
- Thread an optional preResolved ResolvedChangeContext through runAgentOperation/generateAgent* so `coco watch` reuses the digest guard's resolveChangeSource snapshot instead of resolving the diff again per operation (redundant git diff/ls-files I/O). - Suppress the operation-loop error event once shutdown has set `stopped`, so a SIGINT-aborted in-flight operation never emits an `error` event after the terminal `stopped` event on the JSON stream. Addresses the two open review concerns on PR #1983. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The `.wasm-backed` tree-sitter parser suite pays the one-time `Parser.init` bootstrap cost on its first real test. Jest's default 5000ms budget was tight enough that the macOS CI runner (Node 22.22.2) intermittently exceeded it, unrelated to the runner's own logic. Bump the suite timeout to 20000ms to absorb cold-start variance.
There was a problem hiding this comment.
🔎 Agent review (sonnet→opus) — CONCERNS
REVIEW: CONCERNS
RESOLVES: full
The PR lands exactly the two planned fixes (single resolveChangeSource reused via an optional preResolved param, and error-after-stopped suppression) plus the full watch daemon, with faithful refactoring and matching tests. One real gap survives: the ordering guard covers only the error path, so a successful 'result' can still race past the terminal 'stopped' event on the JSON stream.
1 concern · 2 nits — 2 inline on the diff
- 🧹 Unrelated CI-timeout fix bundled in —
src/lib/parsers/default/__tree_sitter__/tsTreeSitterParser.test.ts— Thefix(test): raise tree-sitter WASM init timeoutcommit is unrelated to OSS-1598's watch daemon and isn't mentioned in the plan or work item. It's small and low-risk, but per the repo's guidance to keep changes scoped to the work item it would be cleaner as its own PR.
The abort-and-error race was already guarded, but a successful runAgentOperation resolving in the same tick as shutdown could still emit a `result` event after the terminal `stopped` event. Guard the success path the same way as the error path, and add a test mirroring the existing abort/error-ordering test for the success case.
There was a problem hiding this comment.
🔎 Agent review (sonnet→opus) — CONCERNS
REVIEW: CONCERNS
RESOLVES: full
The PR fully delivers OSS-1598 — coco watch with a cleanly extracted lib/watcher shared with the TUI's refreshWatcher via a re-export shim, a content-digest cost guard, a min-interval throttle, and thorough tests. Two settle-loop edge cases (a failed op permanently retiring its digest, and one unguarded idle emit after stopped) and one unrelated bundled CI-flake fix are the only concerns.
2 concerns · 1 nit — 3 inline on the diff
There was a problem hiding this comment.
🔎 Agent review (sonnet→opus) — LGTM
REVIEW: LGTM
RESOLVES: full
All four first-pass findings are resolved in the actual working-tree code: the digest is now committed only after a clean, non-aborted pass (Fix A), the idle emit is !stopped-guarded (Fix B), the tree-sitter timeout bump is reverted to match main (Fix C), and both regression tests are added. The one caveat is that these fixes are still uncommitted at review time, so the pushed PR diff must be updated for them to actually land.
1 nit — 1 inline on the diff
…topped A failed operation was marking the change-set digest as seen before any operation ran, so a transient failure (e.g. a dropped provider call) permanently retired that digest and silently skipped it on every future settle. Only commit lastDigest once every operation in the pass completes without error and shutdown hasn't landed. The NO_CHANGES/idle branch also emitted unconditionally, so a resolve that lands after SIGINT could still print an idle event after the terminal stopped event on the JSON stream — guard it the same way as the result/error paths already were. Also drops the unrelated tree-sitter WASM init timeout bump that had been bundled into this branch; it doesn't belong in this PR's scope.
There was a problem hiding this comment.
🔎 Agent re-review (sonnet, delta) — CONCERNS
REVIEW: CONCERNS
RESOLVES: full
The revise correctly commits handler.ts fixes (digest-after-clean-pass, idle-after-stopped guard) with matching tests, and cleanly reverts the out-of-scope tree-sitter timeout bump. One new behavioral tradeoff worth a judgment call: a partial failure now re-runs every operation in the pass, not just the failed one.
1 concern — 1 inline on the diff
A partial failure (e.g. review succeeds, commit-draft fails in the same pass) previously withheld the digest for the whole pass, so the next settle on the unchanged tree re-ran every operation, including the one that already succeeded. Track lastSucceededDigest per operation instead, so only the failed operation retries.
There was a problem hiding this comment.
🔎 Agent re-review (sonnet, delta) — LGTM
REVIEW: LGTM
RESOLVES: full
The revise replaces the per-pass lastDigest with a per-operation lastSucceededDigest map so a partial failure (e.g. review succeeds, commit-draft fails) only retries the failed operation on the next settle instead of re-billing the one that already succeeded. The change is small, correctly scoped to handler.ts, serialized by the existing throttledRunner (no concurrency risk), and covered by a new test verifying the exact call sequence.
There was a problem hiding this comment.
🔎 Agent re-review (sonnet, delta) — CONCERNS
REVIEW: CONCERNS
RESOLVES: partial
The revise correctly fixes the result/error/idle 'event after stopped' races and adds solid regression tests for them, but it missed applying the same !stopped guard to the 'skipped' event emit, leaving one instance of the exact race class the revise was meant to close.
1 concern — 1 inline on the diff
resolveChangeSource only checks the abort signal once at entry, so a late-landing SIGINT could let it resolve successfully after `stopped` was already true and the terminal `stopped` event already emitted, producing a `skipped` line after `stopped` in --json output.
There was a problem hiding this comment.
🔎 Agent re-review (sonnet, delta) — LGTM
REVIEW: LGTM
RESOLVES: full
The revise commits (per-operation digest tracking, clean-pass digest commit, and the two 'skipped'/'idle' terminal-race guards) are correct, narrowly scoped, and each ships a matching regression test — all 24 watch-related tests pass and lint is clean. No new bugs, scope creep, or missing coverage introduced.
There was a problem hiding this comment.
🔎 Agent re-review (sonnet, delta) — LGTM
REVIEW: LGTM
RESOLVES: full
The revise commit adds the missing !stopped guard around the skipped emit, matching the same guard pattern already used at every other post-await emit site in the file, and ships a targeted regression test; all 14 watch handler tests and lint pass.
…1955-feat-24-feat-watch-coco-watch- # Conflicts: # src/operations/agent/generate.ts
There was a problem hiding this comment.
🔎 Agent re-review (sonnet, delta) — LGTM
REVIEW: LGTM
RESOLVES: full
The only change since the last review is a merge commit pulling in origin/main; the one real conflict (src/operations/agent/generate.ts) was resolved correctly, cleanly re-threading the preResolved parameter through main's new streaming/conventions/condense-diff additions with no dropped logic. tsc is clean and all watch/agent/watcher test suites pass.
What
Adds
coco watch, a standing daemon that re-runsreviewand/orcommit-draftgeneration each time the watched change set (worktree or staged, per--staged) settles — turning coco from a command you remember to run into ambient feedback.Why
Plane: OSS-1598
Closes #1955
How
.git/index/HEAD/refs/logs/HEADrename-survival watch logic out ofsrc/workstation/chrome/refreshWatcher.tsdown intosrc/lib/watcher/repoChangeWatcher.ts, sosrc/commands/watchcan reuse it withoutcommandsreaching sideways intoworkstation(per the repo's layering rule).refreshWatcher.tsbecomes a thin re-export shim — every existing workstation import keeps working unchanged.src/lib/watcher/throttledRunner.ts: floors LLM calls to at most one per--interval(default 15s) regardless of how often the tree changes, and never overlaps runs — the cost-containment guard called out in the work item.src/commands/watch/{config,handler,index}.ts: re-runsreview/commit-draftthroughsrc/operations/agent/on each settled change, with a diff-content-digest guard (viaresolveChangeSource'smeta.digest) so an unchanged tree never triggers an LLM call even if the filesystem fires.--onceruns a single settle-triggered pass and exits (no fs watcher spun up);--stagedscopes togit diff --cached;--conventional/--languagepass through to generation;--jsonemits line-delimited events (ready,idle,skipped,running,result,error,stopped) for editor integration.watchinsrc/index.ts(command tree + fish completion) and adds the'watch'LLM usage surface toobservability.ts.Testing
npm run build)npx jest— full suite green aside from a pre-existing, unrelatedtsTreeSitterParser.test.tswasm-loading failure reproduced identically onmainbefore this branch's changes)npm run lint— 0 errors, only pre-existing unrelated react-hooks warnings)🤖 Generated with Claude Code
Closes #1955