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

scripts/check-inbox.sh

@@ -141,19 +141,38 @@ for team in "${TEAM_LIST[@]}"; do
   esac
 
   RESULT=$(agmsg_sqlite "$DB" "
-    SELECT from_agent || char(31) || replace(replace(body, char(10), '\n'), char(9), '\t') || char(31) || created_at
+    SELECT id || char(31) || from_agent || char(31) || replace(replace(body, char(10), '\n'), char(9), '\t') || char(31) || created_at
     FROM messages WHERE team='$team_sql' AND to_agent='$AGENT_SQL' AND read_at IS NULL
     ORDER BY created_at ASC;
   ")
   if [ -n "$RESULT" ]; then
     COUNT=$(echo "$RESULT" | wc -l | tr -d ' ')
     OUTPUT+="$COUNT new message(s) in $team:"$'\n'
-    while IFS=$'\x1f' read -r from body ts; do
+    IDS=""
+    while IFS=$'\x1f' read -r id from body ts; do
       OUTPUT+="  [$ts] $from: $body"$'\n'
+      case "$id" in
+        ''|*[!0-9]*) ;; # defensive: never splice a non-numeric value into SQL
+        *) IDS="${IDS:+$IDS,}$id" ;;
+      esac
     done <<< "$RESULT"
     OUTPUT+=$'\n'
-    # Mark as read
-    agmsg_sqlite "$DB" "UPDATE messages SET read_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE team='$team_sql' AND to_agent='$AGENT_SQL' AND read_at IS NULL;" 2>/dev/null || true
+    # Test seam: a two-file barrier that lets the race regression test land a
+    # message deterministically between display and mark. No-op unless set.
+    if [ -n "${AGMSG_TEST_MARK_BARRIER:-}" ]; then
+      : > "$AGMSG_TEST_MARK_BARRIER.reached"
+      _agmsg_barrier_waited=0
+      while [ ! -e "$AGMSG_TEST_MARK_BARRIER.release" ]; do
+        sleep 0.05
+        _agmsg_barrier_waited=$((_agmsg_barrier_waited + 1))
+        [ "$_agmsg_barrier_waited" -ge 200 ] && break # 10s safety cap
+      done
+    fi
+    # Mark as read — only the ids captured above, so a message that arrives
+    # between the SELECT and this UPDATE is not marked read unseen.
+    if [ -n "$IDS" ]; then
+      agmsg_sqlite "$DB" "UPDATE messages SET read_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id IN ($IDS);" 2>/dev/null || true
+    fi
   fi
 done
 

scripts/inbox.sh

@@ -26,9 +26,10 @@ _agmsg_sqlesc() { printf %s "$1" | sed "s/'/''/g"; }
 TEAM_SQL="$(_agmsg_sqlesc "$TEAM")"
 AGENT_SQL="$(_agmsg_sqlesc "$AGENT")"
 
-# Get unread messages — escape newlines/tabs in body to keep one record per line
+# Get unread messages — id first so the mark step below targets exactly these
+# rows; escape newlines/tabs in body to keep one record per line
 UNREAD=$(agmsg_sqlite "$DB" "
-  SELECT from_agent || char(31) || replace(replace(body, char(10), '\n'), char(9), '\t') || char(31) || created_at
+  SELECT id || char(31) || from_agent || char(31) || replace(replace(body, char(10), '\n'), char(9), '\t') || char(31) || created_at
   FROM messages WHERE team='$TEAM_SQL' AND to_agent='$AGENT_SQL' AND read_at IS NULL
   ORDER BY created_at ASC;
 ")
@@ -39,14 +40,36 @@ if [ -z "$UNREAD" ]; then
   exit 0
 fi
 
-# Display
+# Display, collecting the ids actually shown
 COUNT=$(echo "$UNREAD" | wc -l | tr -d ' ')
 echo "$COUNT new message(s):"
 echo ""
-while IFS=$'\x1f' read -r from body ts; do
+IDS=""
+while IFS=$'\x1f' read -r id from body ts; do
   echo "  [$ts] $from: $body"
+  case "$id" in
+    ''|*[!0-9]*) ;; # defensive: never splice a non-numeric value into SQL
+    *) IDS="${IDS:+$IDS,}$id" ;;
+  esac
 done <<< "$UNREAD"
 echo ""
 
-# Mark as read (non-fatal — may fail in sandboxed environments)
-agmsg_sqlite "$DB" "UPDATE messages SET read_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE team='$TEAM_SQL' AND to_agent='$AGENT_SQL' AND read_at IS NULL;" 2>/dev/null || true
+# Test seam: a two-file barrier that lets the race regression test land a
+# message deterministically between display and mark. No-op unless set.
+if [ -n "${AGMSG_TEST_MARK_BARRIER:-}" ]; then
+  : > "$AGMSG_TEST_MARK_BARRIER.reached"
+  _agmsg_barrier_waited=0
+  while [ ! -e "$AGMSG_TEST_MARK_BARRIER.release" ]; do
+    sleep 0.05
+    _agmsg_barrier_waited=$((_agmsg_barrier_waited + 1))
+    [ "$_agmsg_barrier_waited" -ge 200 ] && break # 10s safety cap
+  done
+fi
+
+# Mark as read (non-fatal — may fail in sandboxed environments).
+# Only the ids displayed above: a blanket "WHERE read_at IS NULL" would also
+# swallow messages that arrived between the SELECT and this UPDATE — they
+# would be marked read without ever having been shown.
+if [ -n "$IDS" ]; then
+  agmsg_sqlite "$DB" "UPDATE messages SET read_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id IN ($IDS);" 2>/dev/null || true
+fi

tests/test_inbox.bats

@@ -0,0 +1,88 @@
+#!/usr/bin/env bats
+
+load test_helper
+
+setup() {
+  setup_test_env
+  bash "$SCRIPTS/join.sh" testteam alice claude-code /tmp/project-a
+  bash "$SCRIPTS/join.sh" testteam bob claude-code /tmp/project-a
+  DBPATH="$TEST_SKILL_DIR/db/messages.db"
+  BARRIER="$TEST_SKILL_DIR/mark-barrier"
+}
+
+teardown() {
+  teardown_test_env
+}
+
+unread_count() {
+  sqlite3 "$DBPATH" "SELECT COUNT(*) FROM messages WHERE team='testteam' AND to_agent='$1' AND read_at IS NULL;" | tr -d '\r'
+}
+
+# Wait until the script under test has displayed and is paused before its
+# mark UPDATE (barrier .reached appears), with a bounded wait.
+await_barrier_reached() {
+  for _ in $(seq 1 100); do
+    [ -e "$BARRIER.reached" ] && return 0
+    sleep 0.05
+  done
+  return 1
+}
+
+# --- inbox.sh -----------------------------------------------------------
+
+@test "inbox: displays unread messages and marks exactly those as read" {
+  bash "$SCRIPTS/send.sh" testteam bob alice "first"
+  bash "$SCRIPTS/send.sh" testteam bob alice "second"
+  run bash "$SCRIPTS/inbox.sh" testteam alice
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"2 new message(s):"* ]]
+  [[ "$output" == *"first"* ]]
+  [[ "$output" == *"second"* ]]
+  [ "$(unread_count alice)" -eq 0 ]
+}
+
+@test "inbox: --quiet is silent when there is nothing unread" {
+  run bash "$SCRIPTS/inbox.sh" testteam alice --quiet
+  [ "$status" -eq 0 ]
+  [ -z "$output" ]
+}
+
+@test "inbox: a message arriving between display and mark is NOT marked read unseen" {
+  bash "$SCRIPTS/send.sh" testteam bob alice "early"
+  # Pause the run between display and mark, land a message inside the window,
+  # then release. With the old blanket "WHERE read_at IS NULL" mark, the late
+  # message was silently marked read without ever having been displayed.
+  AGMSG_TEST_MARK_BARRIER="$BARRIER" bash "$SCRIPTS/inbox.sh" testteam alice > "$TEST_SKILL_DIR/first-run.out" &
+  bg_pid=$!
+  await_barrier_reached
+  bash "$SCRIPTS/send.sh" testteam bob alice "late"
+  : > "$BARRIER.release"
+  wait "$bg_pid"
+  run cat "$TEST_SKILL_DIR/first-run.out"
+  [[ "$output" == *"early"* ]]
+  [[ "$output" != *"late"* ]]
+  # The late message must still be unread…
+  [ "$(unread_count alice)" -eq 1 ]
+  # …and surface on the next check
+  run bash "$SCRIPTS/inbox.sh" testteam alice
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"late"* ]]
+  [ "$(unread_count alice)" -eq 0 ]
+}
+
+# --- check-inbox.sh ------------------------------------------------------
+
+@test "check-inbox: a message arriving between display and mark is NOT marked read unseen" {
+  bash "$SCRIPTS/send.sh" testteam bob alice "early"
+  AGMSG_TEST_MARK_BARRIER="$BARRIER" bash "$SCRIPTS/check-inbox.sh" claude-code /tmp/project-a > "$TEST_SKILL_DIR/check-run.out" 2>/dev/null &
+  bg_pid=$!
+  await_barrier_reached
+  bash "$SCRIPTS/send.sh" testteam bob alice "late"
+  : > "$BARRIER.release"
+  wait "$bg_pid" || true
+  run cat "$TEST_SKILL_DIR/check-run.out"
+  [[ "$output" == *"early"* ]]
+  [[ "$output" != *"late"* ]]
+  # The late message was not silently marked read by the first run
+  [ "$(unread_count alice)" -eq 1 ]
+}