Skip to content

fix(inbox): mark only the displayed messages as read#361

Open
ryofukutani wants to merge 1 commit into
fujibee:mainfrom
ryofukutani:fix/inbox-mark-only-displayed
Open

fix(inbox): mark only the displayed messages as read#361
ryofukutani wants to merge 1 commit into
fujibee:mainfrom
ryofukutani:fix/inbox-mark-only-displayed

Conversation

@ryofukutani

Copy link
Copy Markdown

Problem

inbox.sh / check-inbox.sh select the unread messages, display them, then mark as read with a blanket UPDATE … WHERE team=… AND to_agent=… AND read_at IS NULL. The WHERE clause is re-evaluated at UPDATE time, so a message that arrives between the SELECT and the UPDATE is marked read without ever having been displayed — it silently disappears from every future inbox check.

We hit this in production-style multi-lane use (several agents + watchers on one machine): a reply landing while a turn-hook check was mid-flight was swallowed unseen.

Fix

Carry the message ids through the display loop and mark exactly those ids (WHERE id IN (…)). A late-arriving message stays unread and surfaces on the next check. Ids are validated numeric before being spliced into SQL (mirrors the #87 hardening posture).

Tests

New tests/test_inbox.bats (the first coverage for these two scripts):

  • display/mark and --quiet behavior pins
  • a deterministic race regression test for both scripts, using a tiny two-file barrier seam (AGMSG_TEST_MARK_BARRIER, no-op unless set, 10s safety cap) — the test lands a message inside the display→mark window and asserts it is not marked read unseen
  • the race test fails against the old blanket UPDATE (verified by mutation) and passes 5/5 consecutive runs against the fix
  • neighboring suites green locally: test_delivery 125/125, test_codex_bridge 27/27, test_messaging, test_storage, test_watch_once, test_dispatch, test_team, test_install

If you'd prefer a different seam mechanism (or none), happy to adjust — the id-based marking stands on its own.

inbox.sh and check-inbox.sh selected the unread messages, displayed them,
then marked as read with a blanket
`UPDATE ... WHERE team=... AND to_agent=... AND read_at IS NULL`.
The WHERE clause is re-evaluated at UPDATE time, so a message that arrives
between the SELECT and the UPDATE is marked read without ever having been
displayed — it silently disappears from every future inbox check.

Carry the message ids through the display loop and mark exactly those ids
(`WHERE id IN (...)`), so a late-arriving message stays unread and surfaces
on the next check. Ids are validated numeric before splicing into SQL.

Adds tests/test_inbox.bats: basic display/mark and --quiet pins, plus a
deterministic race regression test for both scripts using a small two-file
barrier seam (AGMSG_TEST_MARK_BARRIER, no-op unless set; 10s safety cap).
The race test fails against the old blanket UPDATE (verified by mutation)
and passes 5/5 against the fix.
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