Node-carrier receipt writes replace the whole record, so a concurrent field is erased

Context

Split out of PR #17482 review 4995131088 by @neo-gpt-emmy (RA-1/RA-2). That PR's own new defect — a seenAt cache that was not rolled back on a failed persist, so the write-once guard poisoned itself and never retried — is fixed in the PR. This ticket carries the part that genuinely pre-exists it, so #17321 can close without an uncited residual.

Filed because a review asked me to name a surviving owner rather than write "filed separately" and leave it unowned. That claim was in the PR body before this ticket existed, which is the thing being corrected.

Live latest-open sweep: checked latest 20 open issues at 2026-08-21T16:1x UTC; gh search issues for persistReceipt OR whole-record OR clobber returned zero. No equivalent exists.

The Problem

persistReceiptNode writes the entire node record:

async function persistReceiptNode(node) {
    const db = GraphService.db;
    if (!db?.storage) return false;
    await db.storage.addNodes([node]);
    db.acknowledgeLocalMutations?.();
    return true;
}

Every node-carrier receipt writer goes through it — setMessageNodeReadAt, setMessageNodeArchivedAt, setMessageNodeSeenAt. A field committed to storage by another process between this process's read and its persist is overwritten with the older in-memory value. Nothing detects it: the write succeeds, and the lost field is simply absent afterwards.

The edge carrier already has its fix. PR #17483 introduced getDeliveryEdgePropertiesForWrite, which overlays storage-owned mutable state (including explicit nulls) before a whole-edge write. That closes the same class on DELIVERED_TO edges. The node carrier has no equivalent.

Scope honesty — the two carriers are not equally exposed, and the difference matters for the fix. The edge writers rebuilt state from a possibly-stale index-Set member, which is an in-process hazard and the one #17483 targets. The node writers mutate getRecordProperties(node) in place on the canonical db.nodes.get() record, so they are not exposed to that shape. What remains on the node path is the genuine cross-process race: two writers, one SQLite row, last-write-wins over the whole document. Verified by reading both writers at ce33c868b4, not inferred from the shape.

The Architectural Reality

• ai/services/memory-core/MailboxService.mjs — persistReceiptNode, and the three setMessageNode* writers above it
• ai/services/memory-core/MailboxService.mjs — getDeliveryEdgePropertiesForWrite, the existing edge-side precedent from #17483
• Storage is SQLite; Nodes.data holds the serialized record, so a partial update means a JSON-path write rather than a document replace

The receipt fields are storage-owned mutable state: readAt, archivedAt, seenAt. They are exactly the fields two processes can legitimately write concurrently, since a listing, a drain and an archive are independent operations on the same row.

The Fix

Give the node carrier the same authority the edge carrier now has. Two candidate shapes, and the choice is the substance of this ticket rather than a detail:

1. Overlay before write — mirror getDeliveryEdgePropertiesForWrite on the node path. Cheapest, consistent with the merged precedent, still a read-modify-write and therefore still racy in a narrower window.
2. Conditional JSON-path update — modify only $.properties.<field> in SQLite so no unrelated field can be clobbered at all. Strictly stronger, and what the original #17321 review prescribed; larger change, and it should then be applied to the edge carrier too rather than leaving two different mechanisms.

Recommendation: (2), with (1) as the fallback if the storage layer cannot express a scoped update. Two carriers with two different durability mechanisms is the outcome to avoid — that asymmetry is what makes this a ticket instead of a patch.

Acceptance Criteria

• A node-carrier receipt write preserves an unrelated storage-only field committed between read and persist — RED first, using an interposed write, not a mocked storage double alone
• The same control passes for the edge carrier, so the two carriers demonstrably share one mechanism
• readAt, archivedAt and seenAt are all covered; a fix to one field must not leave the others on the old path
• Mutation control: reverting the node-side change reddens the node arm and leaves the edge arm green
• The chosen mechanism is recorded with its rejected alternative, so the next reader does not re-derive the overlay-vs-conditional-update decision

Out of Scope

• The seenAt cache-rollback defect from #17321 — already fixed in PR #17482, and it is a different failure (non-retry within one process, not cross-process loss)
• Any change to which carrier holds which receipt; node-vs-edge routing is settled

Avoided Traps

Treating this as "the seenAt race". It is not seenAt-specific and fixing it only there would create exactly the asymmetry #17482 declined to introduce — a stronger durability guarantee for the newest field than for the two that have carried receipts all along.

Proving it with a storage double only. A mock that never actually races green-lights an overlay that still loses the write under real concurrency. The AC asks for an interposed committed write.

Related

• PR #17482 — where the residual was surfaced and the seenAt half fixed
• PR #17483 — the edge-carrier overlay precedent this should converge with
• #16965, #17321

Retrieval Hint: persistReceiptNode whole record clobber receipt carrier overlay conditional update

Origin Session ID: 752da6ac-a6c3-447f-8847-1da4ce49deb8

Decision Record impact: none — implementation-level durability within an existing owning service; no ADR authority touched. Structure-map gate: N/A, no new file or placement.