fix(watch): mark monitor-delivered messages as read - #486
Conversation
…t replay it (#439) `inbox.sh` and `check-inbox.sh` both mark delivered rows' `read_at` before displaying them, but `watch.sh` — the live Monitor-mode delivery path — never touched `read_at` at all. A message delivered live through a watcher stayed `read_at IS NULL` forever, so a later `inbox.sh` / `check-inbox.sh` call, or the `history.sh` unread marker, would surface it again as if it were still unread. Adds a `mark_read()` helper called from both the normal delivery branch and the `ctrl:despawn` control-row branch, so control messages handled by the watcher also stop lingering as unread. The helper is scoped to the definitive receiver for a role. A broad watcher (no `actas` name) subscribes to every registered role in the project, so marking unconditionally would let a leader's default watcher write read state for a role that has its own exclusive watcher. It skips when an exclusive ready sentinel exists for that (team, agent). This is a best-effort mark on local write success, not a delivery acknowledgement; a stronger guarantee needs the claim/ack redesign tracked in issue #373. The same defect was independently diagnosed and fixed in PR #486 by @u-ichi, who reached the same root cause — that the watermark stops a watcher re-streaming a row but never touches `read_at`. That PR is closed in favour of this one, which additionally guards the broad-watcher case, keeps the mark idempotent, and degrades rather than exiting when the database is unavailable. Issue #373, which #486's author filed, is where the durable fix belongs. Closes #439
|
Thank you for this, and for #373 — the diagnosis in both is correct: the watermark stops a watcher re-streaming a row but never touches #439 was taken for this specific fix, and the reason is narrow rather than a judgement on the code here. A broad watcher (one with no Worth saying plainly: #439 explicitly notes that this whole approach is a best-effort mark on local write success, not a delivery acknowledgement, and that a real guarantee needs the claim/ack redesign — citing your #373 as where that belongs. So the durable answer to this problem is the one you filed, not either of these two patches. Your independent diagnosis is also credited in the merge commit of #439. Closing this one to keep a single implementation of the same behaviour. #372 and #373 are still open and are being looked at on their own merits; #372 adds interface surface, so it needs a more deliberate evaluation before anything is implemented, which is why it has not moved yet. |
Problem
watch.shemits each new message to the Monitor stream and then advances its per-session watermark, but it does not update that message row'sread_at. This leaves two conflicting states for the same message:We observed this through Grok Build's official Monitor path: the message body reached the Grok UI, while the corresponding
messages.read_atremainedNULL. A later unread query therefore could not distinguish that delivered row from a message that had never reached the Monitor stream. This affects any runtime usingwatch.sh, not only Grok.Reproduction
watch.shfor a registered agent and wait for its watermark file.read_atis stillNULL, even though the watermark has advanced past its ID.Change
After stdout accepts a message line,
watch.shnow updatesread_atfor that exact message ID. The watermark advances only after that update succeeds.read_atScope
read_athere means that the message was successfully emitted to the Monitor stream. It does not claim that the receiving agent completed or handled the work. This PR does not change the schema, dispatcher behavior, message claiming, agent-specific hooks, or historical-message replay policy.Tests
bash -n scripts/watch.shgit diff --checkThe Bats suite was not available locally; the equivalent delivery and failure scenarios above were run with an isolated SQLite database. GitHub CI runs the Bats suite on Ubuntu, macOS, and Windows.