Skip to content

Name the operation on mailbox change events - #9

Merged
TheGreatAxios merged 3 commits into
mainfrom
cl-5018-mailbox-event-op
Aug 2, 2026
Merged

Name the operation on mailbox change events#9
TheGreatAxios merged 3 commits into
mainfrom
cl-5018-mailbox-event-op

Conversation

@TheGreatAxios

Copy link
Copy Markdown
Contributor

Summary

The live change event carried only an id — a listener could not tell a
read from an archive from an assignment without re-fetching and
diffing against remembered state.

  • Add an optional MailboxEventOp (create / mark_read /
    mark_unread / trash / archive / restore / enrich /
    assign) to MailboxEvent.
  • Thread it through every publishMailboxEvent call site: the two
    delivery paths (writeMailboxMessage, deliverInboxItems) and the
    transport dual-write (createMailboxPersist) all publish create;
    mountMailbox's mutation routes publish their own action, reusing
    the same identifiers applyMailboxBulkAction already defines so a
    single-message mutation and a bulk one report the same op.
  • Additive, not a reshape: op is optional on
    MailboxEventSchema and on publishMailboxEvent. An existing
    listener that only reads id, or an existing caller of
    publishMailboxEvent that omits op, is unaffected.
  • Document delivery semantics that were previously unstated — events
    can be missed, duplicated, or arrive out of order — in
    MailboxEventBus's doc comment and the README's SSE client
    contract.

Scope note

No overlap with #8 (frame-size / recipient-fan-out caps) — that PR
does not touch bus.ts, the publish call sites, or mount.ts's route
table. Both PRs touch the Unreleased section of CHANGELOG.md and add
prose to ARCHITECTURE.md/README.md; expect a straightforward textual
merge, not a semantic one.

Verification

Local bun install / bun test was not run (sandboxed dev machine,
OOMs on install/build) — new and touched tests are written to the
repo's existing patterns and are expected to pass under CI:

  • packages/mailbox/src/bus.test.ts (new) — schema accepts the
    pre-existing { type, id } shape and a known op, rejects an
    unknown op; publishMailboxEvent omits op when not given one
    and includes it when given one.
  • packages/mailbox/src/mount-event-op.test.ts (new) — each single
    mutation verb, enrich, assign, and bulk publish their expected
    op over a real mounted app.
  • write.test.ts / persist.test.ts — extended existing bus-publish
    tests to assert op === "create".
  • examples/reference-host/test/acceptance.test.ts — updated the one
    exact-JSON SSE assertion to include the new op field.

Closes CL-5018

The live event carried only an id, so a listener could not tell a read
from an archive from an assignment without re-fetching and diffing
against remembered state.

Add an optional MailboxEventOp (create/mark_read/mark_unread/trash/
archive/restore/enrich/assign) to MailboxEvent and thread it through
every publishMailboxEvent call site: the two delivery paths and the
transport dual-write publish "create"; mountMailbox's mutation routes
publish their own action, reusing the same identifiers
applyMailboxBulkAction already uses so single and bulk mutations agree.

op is optional on MailboxEventSchema and on publishMailboxEvent, so an
existing listener reading only id, or an existing caller omitting op,
is unaffected.

Document delivery semantics that were previously unstated: events can
be missed, duplicated, or arrive out of order, in the bus module doc
comment and the README's SSE client contract.

Closes CL-5018
@TheGreatAxios
TheGreatAxios force-pushed the cl-5018-mailbox-event-op branch from 0aa2753 to 26c9822 Compare August 2, 2026 17:01
@TheGreatAxios

Copy link
Copy Markdown
Contributor Author

Rebased onto main now that #8 is merged. Clean rebase, no conflicts — #8 never touched bus.ts or mount.ts, and its frame-size/recipient guardrails in write.ts/persist.ts sit entirely before the existing publishMailboxEvent call sites this PR modifies, so there's no functional overlap. Re-checked git diff against #8's merge commit to confirm the three publish call sites (write.ts x2, persist.ts) are unchanged by it.

Two notes for review:

  • Additive-ness is visible directly in the bus.ts diff, not just the description: publishMailboxEvent's body is op ? { type: "mailbox", id, op } : { type: "mailbox", id } — omitting op reproduces the exact original two-field shape, byte for byte. bus.test.ts asserts this ("omitting op publishes the original two-field shape").
  • This PR does not touch the transport-write-swallows-refusal behavior a reviewer flagged separately (tracked elsewhere, blocking the HTTP delivery route work, not this PR). Worth noting explicitly: if that write is refused, announce/publish is never reached — no event fires at all, so a listener sees nothing rather than a wrong op. Unchanged by this PR either way.

- publishMailboxEvent's op parameter is now required; every call site
  already named one, so this makes the guarantee compiler-enforced
  instead of relying on convention. MailboxEventSchema's op field stays
  optional on the wire for callers outside this package and for
  historical events replayed from before the field existed.
- Add a test asserting every MailboxBulkAction value has a matching
  MailboxEventOp, so a future bulk verb without a corresponding op
  fails a test instead of silently drifting.
- Reword the duplication bullets in bus.ts and the README so the
  default (deduped, via a stable dedupe key) reads as the default and
  the undeduped-redelivery case reads as the exception it is.
- Align the ordering-guarantee wording between bus.ts and the README,
  and back-port the README's parallel missed/duplicated/out-of-order
  structure into bus.ts's doc comment.
- Document why MailboxEventOp keeps its own name instead of aliasing
  MailboxBulkAction: it's a strict superset (create/enrich/assign
  aren't bulk actions), so collapsing the names would claim an
  equivalence the two vocabularies don't have.
Array<{ op: string }> widened each case's op through inference,
losing the literal type publishMailboxEvent's now-required op
parameter needs. Annotate the table with MailboxEventOp instead of
string so the compiler still enforces the union at the one place a
test constructs it.
@TheGreatAxios
TheGreatAxios merged commit b91acdd into main Aug 2, 2026
1 check failed
@TheGreatAxios
TheGreatAxios deleted the cl-5018-mailbox-event-op branch August 2, 2026 17:39
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