Skip to content

feat: add per-owner read/unread status for inbox messages - #84

Merged
addisonkline merged 1 commit into
mainfrom
kline/v2-server-reads
Jun 30, 2026
Merged

feat: add per-owner read/unread status for inbox messages#84
addisonkline merged 1 commit into
mainfrom
kline/v2-server-reads

Conversation

@addisonkline

Copy link
Copy Markdown
Collaborator

Summary

Adds server-side read/unread tracking for messages in a user-agent's inbox. A message is delivered unread and marked read when the owning user-agent opens it via GET /inbox/{message_id}.

Read state is per-owner: because one message fans out to many recipients who share a single inbox entry, the status is stored on the per-owner membership record, so one recipient reading a message never affects another's status. It is surfaced as a new is_read field on MAILInboxEntrySummary (GET /inbox).

Per discussion, the scope is intentionally minimal: auto-mark-on-open only (no explicit mark read/unread endpoints), and is_read as a field only (no ?read= filter or unread count).

Changes

  • Protocol: is_read: bool = False on MAILInboxEntrySummary (optional/defaulted → backward-compatible wire change).
  • SQLite backend: is_read column on MailboxItemRow; idempotent ALTER TABLE … ADD COLUMN in _ensure_schema_columns so existing DBs upgrade (legacy rows default to unread); mark_read()/read_states() repository methods; list_inbox() stitches per-owner read state; get_inbox_message() marks read in-txn.
  • Memory backend: read_inbox set with full parity (mark-on-open, stitched into list reads via copies, dropped on user-agent removal) plus filesystem persistence.
  • Spec: regenerated spec/openapi.yaml. (No SPEC.md prose change — kept implementation-only.)
  • Tests: 3 new integration tests run on both backends (starts-unread, open-marks-read, per-owner isolation).

Testing

  • Full suite: 769 passed, 1 skipped, 6 xfailed (pre-existing stubs).
  • ruff and mypy clean.

🤖 Generated with Claude Code

Messages are delivered unread and marked read when the owning user-agent
opens them via GET /inbox/{message_id}. Read state is tracked per-owner
on the inbox membership record (mailbox_items.is_read in SQLite, a
read_inbox set in the memory backend) so a fanned-out message's status is
independent per recipient. Surfaced as is_read on MAILInboxEntrySummary.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@rheaton64 rheaton64 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed and approved.

Scope verified

  • Protocol change is backward-compatible: is_read: bool = False on MAILInboxEntrySummary. Existing consumers
    that don't expect the field silently ignore it; new
    consumers can opt into rendering read/unread state.
  • Schema migration is idempotent (the _ensure_schema_columns
    hook adds the column only when missing). Existing DBs
    upgrade in place; legacy rows default to unread, which is
    the right default for a system that's never tracked reads.
  • Per-owner storage on mailbox_items.is_read is the
    correct shape — the fan-out semantic means one message
    has many membership rows, so one recipient reading doesn't
    affect another's status. SPEC.md scope wisely deferred;
    the change is implementation-only until usage patterns
    surface anything worth specifying.
  • 769 tests pass; integration coverage spans both backends.

Chorus impact

Chorus uses MAIL through two paths; neither breaks.

chorus.mail webhook receiver: delivers mail.delivered
events as conduit notifications to the destination entity.
The webhook payload doesn't include is_read (a newly-
delivered message is always unread at the moment of
delivery); the receiver's logic is unaffected.

chorus.mb CLI: the inbox + open commands hit the
endpoints this PR touches:

  • mb inbox lists inbox entries. With the protocol
    change, the response now includes is_read per entry.
    The current renderer doesn't display it; a small follow-
    up could add an unread marker (small UX win). Filing a
    card on my side; not part of this review.
  • mb open <message_id> calls GET /inbox/{id},
    which is now the action that marks the message read.
    This is a real semantic change — before this PR, mb open was idempotent on the server side; after merge,
    each open commits read state. It's the intended
    behavior
    and aligns with how email clients work; just
    worth naming for the dev list so other consumers don't
    trip on it later.

The behavior is exactly what an operator would want:
opening a message in mb behaves like opening an email in
any other client. Nothing on the chorus side needs to
change to accept the new state.

Net assessment

Clean PR, scoped exactly right, backward-compat, schema
migration is the right shape. The "auto-mark-on-open only,
no explicit mark read/unread endpoints, no ?read filter or
unread count" deferral is the right call for v1 — easier
to add later than to take away.

Ship it.

— minichorus-pm

@addisonkline
addisonkline merged commit cab0680 into main Jun 30, 2026
2 checks passed
@addisonkline
addisonkline deleted the kline/v2-server-reads branch June 30, 2026 21:11
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.

2 participants