Beyond the message outbox: split pending effects from pending intent, with per-kind coalescing policy

Источник: dev/fera-research/docs/oc4/idea-07-op-queue-effects-vs-intent.md (read-only; сгенерировано scripts/sync-docs.mjs)

Beyond the message outbox: split pending effects from pending intent, with per-kind coalescing policy

• kind: idea
• area: storage / protocol / offline-first
• evidence:
  • ~/dev/fera/fera/core/crates/storage/src/engine.rs:56-88 — the outbox is Vec<Message>; get_outbox, confirm_sent{local_id,server_id}, mark_failed. The queue’s payload type is Message. It can express exactly one operation: send this message.
  • ~/dev/fera/fera/core/crates/domain/src/outbox.rs:17-24,99-110 — RetryEntry { local_id: MessageId, attempts, not_before } and rows_to_entries(rows: &[Message]): the retry scheduler is message-shaped throughout.
  • ~/dev/fera/fera/core/crates/protocol/src/contract.rs:9-11,18-59 — ChatRepository is 4 methods: get_chats, send_message, paginate_history, mark_read. There is no operation for edit, delete, react, pin, forward, media, or draft.
  • ~/dev/fera/fera/core/crates/protocol/src/types.rs:88-100 — Message { id, chat, text, status, timestamp }: no author, no media, no edit time, no reply/forward metadata, no reactions. Media messages have no representation at all.
  • ~/dev/fera/project-design/context/ADR.md:307-315 — ADR-010: “Outbound writes (new messages, read-receipts) are persisted locally first, with a pending → sent → failed status”
  • ~/dev/fera/project-design/context/ADR.md:346-349 — ADR-011: “Read-receipt sync reuses the same outbox mechanism as message sending”
  • ~/dev/fera/project-design/context/RFC.md:24 — “Offline-first is structural, not a fallback path: the UI has no code path that reads from the network directly”
  • ~/dev/fera/project-design/context/TECHSPEC.md:115-116 — Chat actor responsibilities: “message state (optimistic-update aware), pagination, ReadState enum (ADR-011), outbox integration”
  • ~/dev/fera/project-design/context/TECHSPEC.md:172-208 — the media pipeline (§4) specifies fetch/decode/cache, but never specifies a durable, offline-capable upload path
  • ~/dev/fera/project-design/context/QA.md §Domain — the only outbox gate is “Outbox entries persist across a simulated app restart while in Pending state, and are retried on reconnect”
  • ~/dev/fera/project-design/context/ADR.md:489-492 — ADR-025 already flags one bespoke offline policy as unsettled: “pending secret messages that outlive an offline restart (drop vs. keep-with-warning)” — i.e. the need for per-kind policy is already acknowledged once
  • ~/dev/fera-research/docs/oc5/local-id-reuse-ack-misdelivery.md (kind: risk, severity: high, oc5) — next_local_id() scans only self.messages; SendConfirmed rewrites a local id to a positive server id, so the counter resets and reissues -1. With no attempt id and no status guard, a retried or stale ACK marks the wrong message Sent and removes the wrong outbox entry — silent message loss, in exactly the offline/ack-loss case the outbox exists to handle. This is the strongest available argument for the design below, and it is a defect, not a hypothetical.

Суть / Thesis

Offline-first is the project’s headline feature (RFC §1.2, RFC §8: “structural, not a fallback path”). The outbox is its write-path mechanism. And the outbox is typed to exactly one operation: its payload is Message, its retry entry is keyed by local_id: MessageId, its scheduler counts send attempts.

That is correct for what it covers. The problem is that a Telegram client has roughly a dozen user-initiated mutations, they all want the same durability guarantee, and they do not share a merge policy:

Operation	Offline behaviour	Coalescing	Why it differs from “send”
Send text/media	queue, retry ladder, assign server id	never — 3 messages are 3 messages	the baseline
Send media	upload is resumable and has its own lifecycle	never	upload ≠ send; needs a separate resumable-upload queue
Edit message	apply optimistically, sync later	yes — 3 offline edits = 1	collides with remote edits; needs a conflict rule
Delete message	apply optimistically	yes	idempotent; “delete for me” ≠ “delete for everyone”
React	set a value	yes — toggled 3× = 1	per-message set/map semantics, not a queue entry
Pin / unpin	set absolute state	yes	an assignment, not a delta
Set chat title / photo / mute	set absolute state	yes	assignment
Draft sync	set absolute state	yes	must never be accidentally sent
Join / leave chat	set membership	yes	assignment
Add / rename / delete contact	set absolute state	yes	assignment
Mark read	set a watermark	yes	already modelled as ReadState (ADR-011)
Forward	queue, retry	never	an effect

Two families, not one queue. And the distinction is not cosmetic — it is the difference between correct and incorrect:

• Effects (Send, Forward, SendMediaUpload) are irreversible events. Each must be replayed exactly once, in order, each needs its own retry identity, and losing one loses user data. Current model. Correct.
• Intent (Rename, Pin, React, Delete, Mute, Draft, Join) is an absolute assignment of state. Replaying three queued renames is wasteful and wrong — it should coalesce to the last one, and — critically — it may be already satisfied by the time the connection returns, in which case replaying it can overwrite a newer server-side change. A queue that only knows “retry until success” cannot express “make the server’s value equal X, but only if the server hasn’t moved since I last saw it.”

Today there is nowhere to put this distinction, so each new operation will be invented ad hoc — and given the project’s own track record (ADR-025 already needing a special-case decision for secret messages, ADR-021 needing a downgrade-protection story for a manifest), ad hoc is where the interesting design decisions land without a record.

Варианты / Options (with tradeoffs)

A. Keep Vec<Message>; widen the payload to an enum OutboxOp as needed.

• Pros: smallest change; one queue, one retry ladder.
• Cons: the retry ladder is wrong for coalescing ops. Retrying a queued rename 5 times with exponential backoff is meaningless — a rename either applies or is superseded. And 3 offline renames become 3 network calls with 3 chances to conflict. Correct-looking, quietly inefficient, and it reintroduces ordering hazards between ops that are genuinely independent (pinning a message need not wait behind renaming a chat).

B. Per-operation bespoke paths (what will happen by default).

• Pros: each is locally simple; each can be optimal for its kind.
• Cons: N retry ladders, N persistence formats, N conflict rules, N “pending” UI surfaces. ADR-025’s secret-chat drop-vs-keep question is the visible symptom of exactly this. Compounds every release.

C. Typed op queue with per-kind merge policy (proposed).

Proposed solution

1. One persisted queue, two families, one policy trait

// The queue's payload stops being Message.
enum Op {
    // ——— effects: replay each, exactly once, in order ———
    SendMessage   { chat: ChatId, text: MessageText, reply_to: Option<MessageId> },
    Forward       { chat: ChatId, from: MessageRef },
    UploadMedia   { chat: ChatId, blob: BlobRef, caption: Option<MessageText> },

    // ——— intent: coalesce to the latest, reconcile on apply ———
    EditMessage   { chat: ChatId, id: MessageId, text: MessageText },
    DeleteMessage { chat: ChatId, id: MessageId, scope: DeleteScope },
    SetReaction   { chat: ChatId, id: MessageId, reaction: Reaction },  // set semantics
    SetPinned     { chat: ChatId, id: MessageId, pinned: bool },
    SetChatTitle  { chat: ChatId, title: String },
    SetChatPhoto  { chat: ChatId, blob: BlobRef },
    SetMuted      { chat: ChatId, muted: bool },
    SetDraft      { chat: ChatId, text: MessageText },
    SetMembership { chat: ChatId, member: bool },
    SetContact    { id: UserId, contact: ContactPatch },
    MarkRead      { chat: ChatId, up_to: MessageId },   // already modelled
}

enum DeleteScope { ForMe, ForEveryone }

2. The merge policy is the design content

trait MergePolicy {
    /// Two ops of the same kind coalesce into at most one.
    fn coalesce(a: Op, b: Op) -> Option<Op>;
    /// What to do when the server's current value differs from what we last saw.
    fn reconcile(local: Op, server: Value, base: ObservedAt) -> Resolution;
}

Resolution is where the real semantics live and where ADR-025’s unsettled question gets a home:

• Send → Always — always push, never coalesce, never reconcile.
• EditMessage → IfUnchangedSince(base) — the assignment carries the edit time we last observed; if the server moved since, surface a conflict to the user instead of clobbering.
• SetReaction → MergeSet — per-message map; multiple reactions coexist.
• DeleteMessage{ForMe} → Always (idempotent).
• DeleteMessage{ForEveryone} → IfUnchangedSince(base) — deleting an already-edited message is a different act than deleting the one you saw.
• SetDraft → Always, and never dispatched by the retry loop — a draft is a local-first, server-second assignment with a distinct lifetime.
• Secret-chat Send → DropOnRestart | KeepWithWarning — ADR-025’s open question becomes a policy value rather than a bespoke branch, chosen by the same mechanism as everything else.

That is the payoff: the project’s next three hard offline questions become data in a table instead of three bespoke code paths.

3. What stays exactly as it is

OutboxRetry (outbox.rs:28-88) is correct and should not be thrown away. It becomes the policy for the effect family unchanged — 5 attempts, 1s→×2→ 5 min, park at 5, user-retry resets the ladder (a genuinely nice touch: “the user’s intent outranks the schedule”). Intent ops do not need a ladder; they need a reconciliation pass, which is cheaper — one round trip that resolves N coalesced assignments instead of N-N exponential round trips.

4. Three concrete defects this removes rather than defers

• Local-id reuse → silent message loss (oc5’s HIGH finding, now the strongest motivation for this design). next_local_id() derives its counter from self.messages; once SendConfirmed rewrites a local id to a positive server id, the scan finds no negative ids left and reissues -1. With no attempt id and no status guard on the ack path, a stale or duplicated ACK therefore marks the wrong message Sent and drops the wrong outbox entry. The root cause is structural: the outbox entry’s identity is positional, not stable. An Op queue with a stable, persisted, per-op OpId (plus an idempotency key the server ack echoes) fixes it at the source — and the fix has to live in the queue’s identity model, which is why the queue should be generalised now rather than after the first ack-loss incident report.
• RetryEntry.not_before: Option<Instant> (outbox.rs:23) is not persistable. Instant is process-local. On restart every entry deserializes with not_before: None → every entry is immediately eligible → retry storm — precisely the thing the ladder exists to prevent. The fix is a persisted absolute deadline (wall-clock SystemTime, or monotonic-since-boot paired with wall clock). This belongs in the queue format rather than bolted on, which is a reason to design the format now. (Also flagged to oc5 as a live defect.)
• The two competing outboxes. domain::chat::ChatActor::outbox is a plain in-memory Vec<Message> (domain/src/chat.rs:109) while StorageEngine has its own (engine.rs:56-88), with overlapping signatures and no bridge. A single typed queue collapses two concepts into one.

5. The UI surface, which is the user-visible dividend

One honest “pending changes” affordance, per chat, which is only possible because intents coalesce:

• “3 messages waiting to send” → the effect family, with per-item retry state.
• “Title change pending” → the intent family — one line, because three offline renames became one.
• A conflict, when IfUnchangedSince rejects: “This chat’s name was changed on another device. Keep yours or use theirs?”

That is a materially better offline experience than the official clients provide, and it is the same promise ADR-010 makes for reads — extended to writes, honestly.

6. Honest costs and the honest counter-argument

• This is a real design effort, not a refactor: a merge-policy table is design work that must be reviewed per kind, and getting IfUnchangedSince wrong for edits is worse than not having it.
• Counter-argument to take seriously: MTProto’s own update stream already carries most of these changes from other devices, so much of the reconciliation may resolve itself — and a smaller design that leans on the update stream might be enough. The honest response: that is exactly why the first version should be the effect family only (already correct, just widen the payload), with intent ops added per kind as each UI surface is actually built — carrying the policy table forward as the place they land, rather than each inventing its own queue. The proposal is the seam, not the complete table.

7. Sequencing

1. Now: persist retry deadlines as absolute wall-clock; fix the Instant-on-restart retry storm. Small, independent, real.
2. Now: collapse the two competing outboxes into one type.
3. Then: widen the payload to enum Op, keeping only the effect family.
4. Per UI surface: add the relevant intent kinds + their policy. First candidate is EditMessage (most common user mutation) and DeleteMessage{ForEveryone} (the one with a real conflict rule).