Name the operation on mailbox change events - #9
Merged
Conversation
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
force-pushed
the
cl-5018-mailbox-event-op
branch
from
August 2, 2026 17:01
0aa2753 to
26c9822
Compare
Contributor
Author
|
Rebased onto main now that #8 is merged. Clean rebase, no conflicts — #8 never touched Two notes for review:
|
- 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
MailboxEventOp(create/mark_read/mark_unread/trash/archive/restore/enrich/assign) toMailboxEvent.publishMailboxEventcall site: the twodelivery paths (
writeMailboxMessage,deliverInboxItems) and thetransport dual-write (
createMailboxPersist) all publishcreate;mountMailbox's mutation routes publish their own action, reusingthe same identifiers
applyMailboxBulkActionalready defines so asingle-message mutation and a bulk one report the same op.
opis optional onMailboxEventSchemaand onpublishMailboxEvent. An existinglistener that only reads
id, or an existing caller ofpublishMailboxEventthat omitsop, is unaffected.can be missed, duplicated, or arrive out of order — in
MailboxEventBus's doc comment and the README's SSE clientcontract.
Scope note
No overlap with #8 (frame-size / recipient-fan-out caps) — that PR
does not touch
bus.ts, the publish call sites, ormount.ts's routetable. Both PRs touch the
Unreleasedsection of CHANGELOG.md and addprose to ARCHITECTURE.md/README.md; expect a straightforward textual
merge, not a semantic one.
Verification
Local
bun install/bun testwas 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 thepre-existing
{ type, id }shape and a knownop, rejects anunknown
op;publishMailboxEventomitsopwhen not given oneand includes it when given one.
packages/mailbox/src/mount-event-op.test.ts(new) — each singlemutation verb,
enrich,assign, and bulk publish their expectedopover a real mounted app.write.test.ts/persist.test.ts— extended existing bus-publishtests to assert
op === "create".examples/reference-host/test/acceptance.test.ts— updated the oneexact-JSON SSE assertion to include the new
opfield.Closes CL-5018