Two competing store seams in the workspace: StorageEngine and domain::Store

Источник: dev/fera-research/docs/oc3/004-two-competing-store-seams.md (read-only; сгенерировано scripts/sync-docs.mjs)

Two competing store seams in the workspace: `StorageEngine` and `domain::Store`

kind: problem
area: storage
severity: mid

Evidence

grep -rn "^pub trait" --include='*.rs' ~/dev/fera/fera finds five traits. Two of them define overlapping, incompatible persistence contracts over the same data:

Seam 1 — core/crates/storage/src/engine.rs:12:

pub trait StorageEngine {
    type Error;
    fn get_messages(&self, chat: ChatId, before: Option<MessageId>, limit: u32) -> Result<Vec<Message>, Self::Error>;   // :27-32
    fn put_messages(&mut self, chat: ChatId, msgs: &[Message]) -> Result<(), Self::Error>;                            // :39
    fn get_read_state(&self, chat: ChatId) -> Result<Option<ReadState>, Self::Error>;                                // :47
    fn put_read_state(&mut self, chat: ChatId, state: ReadState) -> Result<(), Self::Error>;                          // :54
    fn get_outbox(&self) -> Result<Vec<Message>, Self::Error>;                                                       // :62
    fn confirm_sent(&mut self, chat: ChatId, local_id: MessageId, server_id: MessageId) -> Result<(), Self::Error>;   // :71-76
    fn mark_failed(&mut self, chat: ChatId, local_id: MessageId, attempts: u32) -> Result<(), Self::Error>;            // :83-88
}

Seam 2 — core/crates/domain/src/chat.rs:114:

pub trait Store {                                                                                                     // :114
    type Error;
    fn put_messages(&mut self, chat: ChatId, msgs: &[Message]) -> Result<(), Self::Error>;                            // :123
}

plus a private state struct core/crates/domain/src/chat.rs:105-110:

messages: Vec<Message>,
outbox: Vec<Message>,

Both declare put_messages(&mut self, ChatId, &[Message]) -> Result<(), Self::Error> with the same name, same arity, same argument types, and — per docs/oc3/001-encrypted-t-dropped-from-storage-seam.md — the same plaintext-Message deviation from TECHSPEC §3.

The ChatActor state holds its own outbox: Vec<Message> (chat.rs:109) while StorageEngine::get_outbox (:62) is also an outbox. grep -rn "outbox" core/crates/domain/src/chat.rs shows the actor’s own outbox is managed locally; there is no bridge, adapter, or impl Store for MemoryEngine anywhere in the workspace.

Суть

There are two persistence abstractions with overlapping method names and no relationship between them. Neither one is a bug on its own — domain::Store is plausibly intended as the actor’s narrow write-side port (a hexagonal-architecture seam) — but the workspace currently has no statement of which one is authoritative, and ChatActor keeps a second, independent outbox in memory alongside the persisted one the storage trait describes.

TECHSPEC.md:122-123 requires the outbox to be “persisted in storage, not just in-memory — must survive app restart while offline”. ADR-010 makes that the core offline-write guarantee. The in-memory outbox: Vec<Message> at chat.rs:109 and the persisted StorageEngine::get_outbox/confirm_sent/mark_failed at engine.rs:62-88 are two implementations of the same concept, and only one of them can satisfy that requirement.

This compounds with the encryption finding: whichever seam is chosen, the Encrypted<T> change has to be made twice unless the duplication is collapsed first.

Note this is the observation oc4 referenced on the bus (“the two competing outboxes … your audit is the evidence, this is the fix”). I am reporting the defect and the evidence; oc4’s idea-07 is the proposal and I am deliberately not duplicating it.

Варианты решения (с tradeoffs)

Option A — delete domain::Store; ChatActor depends on StorageEngine.

Cost: domain currently does not depend on storage (core/crates/domain/Cargo.toml:13 lists only fera-protocol). This adds a dependency edge domain → storage.
Benefit: one seam, one place to make the Encrypted<T> change, one contract test to satisfy.

Option B — keep Store as a deliberate write-port, and add an explicit blanket impl impl<T: StorageEngine> Store for T.

Cost: near zero. One impl in storage (or a small impls module).
Benefit: keeps the actor’s dependency narrow (domain never names the storage crate), which is better layering, and the two traits stop being “two competing outboxes” — Store becomes a restricted view of StorageEngine.
Risk: two traits with the same method name still read as accidental duplication to a future maintainer unless documented.

Option C — rename/split so the overlap is explicit (OutboxStore, MessageSink).

Cost: churn for clarity only.
Benefit: removes the name collision that suggests they’re interchangeable when they are not.
Risk: cosmetic; does not remove the second outbox.

Предлагаемое решение

B, plus delete the duplicate in-memory outbox field.

Keep both traits but make the relationship explicit: impl<T: StorageEngine> Store for T so ChatActor can take any Store and get the full engine behind it. One line of authority, no new dependency edge.
Delete ChatActor::outbox (chat.rs:109) and route every outbox read/write through the Store port. An in-memory mirror of a persisted outbox is exactly the failure ADR-010 and TECHSPEC.md:122-123 are written against, and it is what makes the “survives app restart” requirement untestable from the actor.
Do this in the same commit as the Encrypted<T> seam change (docs/oc3/001), so the signature is corrected once rather than twice.

Cross-references

docs/oc3/001-encrypted-t-dropped-from-storage-seam.md — the deviation both seams share
Bus: oc4 idea-07-op-queue-effects-vs-intent.md proposes the fix for the outbox identity model; this finding is the structural precondition for it.