feat(messaging): state-machine invariant + narrow-response surfaces delivered_at (parity port of PR #923 + #927)#101
Merged
Conversation
…rfaces delivered_at (parity port of PR #923 + #927)
Batched OSS port of two private cueapi fixes:
**Finding A** (private cueapi PR #923 mergeCommit 7a29082): state-
machine invariant fix. Pre-fix: mark_read on queued msg →
delivery_state="read" with delivered_at=NULL (impossible per
canonical queued → delivered → read → acked path). Same fingerprint
anomaly on mark_acked from queued state. Caused diagnostic
confusion in private-side investigations.
Fix: mark_read fills delivered_at=now when null; mark_acked
dual-null-check fills both delivered_at + read_at when null. Pure-
SELECT bypass paths now produce equivalent timestamps; invariant
holds across all entry points.
Note: cueapi-core has no `bulk_mark_read` (hosted-only per parity-
manifest); third setter site from private #923 is omitted from this
port. Only mark_read + mark_acked apply on OSS.
**cmpg8jokl** (private cueapi PR #927 mergeCommit 34e3506): narrow-
response observability follow-on. After Finding A fills delivered_at
on /read + /ack, the narrow StateTransitionResponse omitted the
field. Consumers couldn't observe the just-set value without a
follow-up GET /v1/messages/{id}.
Fix (additive only per API Contract Rule):
- StateTransitionResponse adds `delivered_at: Optional[datetime] = None`
- /read endpoint populates delivered_at
- /ack endpoint populates all 3 setter timestamps (delivered_at +
read_at + acked_at) symmetric with mark_acked's dual-null-check
Tests:
- 4 new regression guards in tests/test_messages.py:
* test_mark_read_fills_delivered_at_when_null
* test_mark_read_preserves_existing_delivered_at
* test_mark_acked_fills_delivered_at_and_read_at_when_null
* test_mark_acked_preserves_existing_timestamps
- 22/22 message tests pass (existing 18 + 4 new)
Both private fixes empirically prod-verified before this port:
- Finding A T1+T3+T5 via path-d-test-recipient fixture
- cmpg8jokl T1+T5 via path-d-test-recipient fixture
Targets tag bump to messaging-v1.1.6 (supersedes v1.1.5-hotfix).
Downstream consumers (cue.dock.svc et al) should bump pin once tag cuts.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Parity checkThis PR modifies files tracked in
Please confirm one of the following in a reply or PR description update:
This is a soft check — it does not block merge. The goal is visibility, not friction. See HOSTED_ONLY.md for the open-core policy. |
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
Batched OSS port of two prod-verified private cueapi fixes:
7a29082): state-machine invariant fix — mark_read + mark_acked filldelivered_at(andread_atfor mark_acked) when null, so bypass paths don't produce impossibledelivery_state=read|acked + delivered_at=NULLfingerprints34e3506): narrow-response observability follow-on —StateTransitionResponsesurfacesdelivered_at(and full setter timestamps on /ack) so consumers see the just-set value without a follow-up GETTargets
messaging-v1.1.6tag bump (supersedesmessaging-v1.1.5-hotfix).Deviation from private
cueapi-core has no
bulk_mark_read(hosted-only per parity-manifest). Third setter site from private PR #923 is omitted; onlymark_read+mark_ackedapply on OSS. Verified viagrep -rn "bulk" app/— onlyevents.py:advance_ack_watermarkmatches, not messaging path.Changes (purely additive per API Contract Rule)
app/services/message_service.py:mark_read:if msg.delivered_at is None: msg.delivered_at = nowmark_acked: dual-null-check sets bothdelivered_atANDread_attonowif nullapp/schemas/message.py:StateTransitionResponseaddsdelivered_at: Optional[datetime] = None+ WHY-rationale docstringapp/routers/messages.py:/readendpoint populatesdelivered_at=msg.delivered_at/ackendpoint populates all 3 setter timestamps (delivered_at+read_at+acked_at)Tests
4 new regression guards in
tests/test_messages.py:test_mark_read_fills_delivered_at_when_null— Finding A invariant on fast-readtest_mark_read_preserves_existing_delivered_at— preserves post-poll valuetest_mark_acked_fills_delivered_at_and_read_at_when_null— symmetric dual-null-checktest_mark_acked_preserves_existing_timestamps— preserves poll→read→ack chain22/22 message tests pass locally (existing 18 + 4 new).
Empirical verification on private (pre-port)
Both fixes empirically prod-verified before this port:
path-d-test-recipientfixture onapi.cueapi.aiBackward compatibility
Purely additive per CLAUDE.md API Contract Rule. Existing consumers reading the previously-returned fields see no change. New consumers can opt into
delivered_at. No removal, no rename.Pre-PR reviews
msg_fggb87nn5zkb(private cycle)Test plan
ecd30cbmessaging-v1.1.6against squash🤖 Generated with Claude Code