diff --git a/apps/ui/src/live/comms-state.ts b/apps/ui/src/live/comms-state.ts index f8527a16..d1e06310 100644 --- a/apps/ui/src/live/comms-state.ts +++ b/apps/ui/src/live/comms-state.ts @@ -148,8 +148,8 @@ function upsertMessage( const next = list.slice(); if (next[i].atUnixMs === msg.atUnixMs) { // Same post time — the server never rewrites at_unix_ms on an update or a - // redelivery (UpdateMessageBlocks touches blocks/text only), so an in-place - // content replace preserves (atUnixMs, id) order. + // redelivery (a block/text update touches blocks and text only), so an + // in-place content replace preserves (atUnixMs, id) order. next[i] = msg; return next; } diff --git a/go/internal/store/messages.go b/go/internal/store/messages.go index 0f21ff3a..0913b69a 100644 --- a/go/internal/store/messages.go +++ b/go/internal/store/messages.go @@ -141,21 +141,17 @@ func (s *Store) MessagesHeadSeq(ctx context.Context) (uint64, error) { return head, nil } -// UpdateMessageBlocks replaces a message's block set — the streaming-turn update -// path (comms.proto:334-336, MessageUpdated). It rewrites both the JSONB blocks -// and the extracted text content so search stays consistent with the current -// blocks. An empty block set or an unknown message is rejected. ask_id is -// assigned once at append and immutable thereafter, so an ask block here must -// carry its existing id; an empty AskID is rejected rather than re-minted (a -// fresh id would orphan any pending RespondToAsk against the original). -func (s *Store) UpdateMessageBlocks(ctx context.Context, id MessageID, blocks []MessageBlock) error { - return updateMessageBlocksExec(ctx, s.pool, id, blocks) -} - -// updateMessageBlocksExec is the shared block-write core, run against either the -// pool (UpdateMessageBlocks) or a transaction (AnswerAsk's locked read-modify- -// write). It re-serializes the full block set and refreshes the text index, -// rejecting an empty set or an ask block missing its immutable id. +// updateMessageBlocksExec is the shared block-write core, run against +// AnswerAsk's locked read-modify-write transaction. It +// re-serializes the full block set and refreshes the extracted text content so +// search stays consistent with the current blocks. An empty block set or an +// unknown message is rejected. ask_id is assigned once at append and immutable +// thereafter, so an ask block here must carry its existing id; an empty AskID is +// rejected rather than re-minted (a fresh id would orphan any pending +// RespondToAsk against the original). +// +// It performs NO membership or authorship check, so it is deliberately +// unexported: every exported write path must authorize before reaching it. func updateMessageBlocksExec(ctx context.Context, db execer, id MessageID, blocks []MessageBlock) error { if len(blocks) == 0 { return fmt.Errorf("%w: message has no blocks", ErrInvalidArgument) @@ -227,13 +223,13 @@ func (s *Store) ListMessages(ctx context.Context, actor AccountID, container Con // captured on subscribe (seq <= SnapshotSeq, comms.proto:353-368, // design.md:807-817); zero reads the latest, no boundary. // The boundary is point-in-time on set membership (which messages the page - // returns, by insert seq), not on content: UpdateMessageBlocks/AnswerAsk - // mutate m.blocks in place without bumping m.seq, so a row present at the - // boundary but edited mid-catch-up returns its post-boundary blocks. This is - // sufficient, not a lost update — the matching MessageUpdated also rides the - // live tail, so an id-deduping client converges to current content - // (last-write-wins). Freezing content too would need an update/change-seq and - // a larger schema change; membership-only is the ratified scope (SEA-1333 OQ5). + // returns, by insert seq), not on content: a blocks update mutates m.blocks in + // place without bumping m.seq, so a row present at the boundary but edited + // mid-catch-up returns its post-boundary blocks. This is sufficient, not a + // lost update — the matching MessageUpdated also rides the live tail, so an + // id-deduping client converges to current content (last-write-wins). + // Freezing content too would need an update/change-seq and a larger schema + // change; membership-only is the ratified scope (SEA-1333 OQ5). const q = ` SELECT m.id, m.channel_id, m.author_account_id, m.at_unix_ms, m.blocks, COALESCE(m.parent_message_id, '') FROM messages m diff --git a/go/internal/store/messages_test.go b/go/internal/store/messages_test.go index 8badf5c7..3115cd84 100644 --- a/go/internal/store/messages_test.go +++ b/go/internal/store/messages_test.go @@ -5,8 +5,8 @@ package store // Message contracts: AppendMessage assigns id + timestamp and validates its // input; ListMessages pages newest-first with a working BeforeMessageID cursor // and a clamped limit; idempotency dedups on (author, non-empty request id) and -// not on an empty key; UpdateMessageBlocks replaces the block set; blocks (text -// and ask, with all ask fields) round-trip through JSONB unchanged; and +// not on an empty key; updateMessageBlocksExec replaces the block set; blocks +// (text and ask, with all ask fields) round-trip through JSONB unchanged; and // SearchMessages finds matches, scopes to the actor's visible channels, narrows // by scope, and tolerates punctuated queries. @@ -466,8 +466,8 @@ func TestUpdateMessageBlocksReplaces(t *testing.T) { } replacement := []MessageBlock{textBlock("rewritten"), askBlock()} - if err := s.UpdateMessageBlocks(ctx, msg.ID, replacement); err != nil { - t.Fatalf("UpdateMessageBlocks: %v", err) + if err := updateMessageBlocksExec(ctx, s.pool, msg.ID, replacement); err != nil { + t.Fatalf("updateMessageBlocksExec: %v", err) } got, err := s.ListMessages(ctx, author.ID, ContainerRef{ChannelID: ch.ID}, Page{}) if err != nil { @@ -496,7 +496,7 @@ func TestUpdateMessageBlocksRejectsEmptyAskID(t *testing.T) { // ask_id is assigned once at append and immutable thereafter. An update // carrying an ask block with an empty AskID must be rejected, NOT re-minted: // a fresh id would orphan any pending RespondToAsk against the original. - err = s.UpdateMessageBlocks(ctx, msg.ID, []MessageBlock{textBlock("rewritten"), askBlockID("")}) + err = updateMessageBlocksExec(ctx, s.pool, msg.ID, []MessageBlock{textBlock("rewritten"), askBlockID("")}) sentinelIs(t, err, ErrInvalidArgument, "update ask block with empty ask_id") // The rejected update must not have persisted: the message still reads back @@ -511,8 +511,8 @@ func TestUpdateMessageBlocksRejectsEmptyAskID(t *testing.T) { // the caller's existing id verbatim (the id the append minted is what an // update must carry back). replacement := []MessageBlock{textBlock("rewritten"), askBlockID("ask-existing-42")} - if err := s.UpdateMessageBlocks(ctx, msg.ID, replacement); err != nil { - t.Fatalf("UpdateMessageBlocks(non-empty ask id): %v", err) + if err := updateMessageBlocksExec(ctx, s.pool, msg.ID, replacement); err != nil { + t.Fatalf("updateMessageBlocksExec(non-empty ask id): %v", err) } after, err := s.ListMessages(ctx, author.ID, ContainerRef{ChannelID: ch.ID}, Page{}) if err != nil { @@ -525,7 +525,8 @@ func TestUpdateMessageBlocksRejectsEmptyAskID(t *testing.T) { } func TestUpdateMessageBlocksUnknownNotFound(t *testing.T) { - err := newTestStore(t).UpdateMessageBlocks(context.Background(), MessageID("ghost"), []MessageBlock{textBlock("x")}) + s := newTestStore(t) + err := updateMessageBlocksExec(context.Background(), s.pool, MessageID("ghost"), []MessageBlock{textBlock("x")}) sentinelIs(t, err, ErrNotFound, "unknown message") } @@ -1054,7 +1055,7 @@ func TestAnswerAskRejectsDuplicateOption(t *testing.T) { // TestAnswerAskConcurrentDistinctAsksSerialize is the SEA-1226 red-first // regression: two distinct asks on ONE message answered concurrently. AnswerAsk // reads the whole block set, records its answer on its own ask in that snapshot, -// and writes ALL blocks back (UpdateMessageBlocks) — so an unserialized +// and writes ALL blocks back (updateMessageBlocksExec) — so an unserialized // read-modify-write lets the second writer's stale snapshot clobber the first // writer's answer, silently losing it (last-writer-wins). The contract is that // both answers survive; this test SHOULD stay RED until the store serializes the