CBG-5764 Keep _mou previous values describing one mutation

[torcolvin]

_mou records that Sync Gateway changed a document's metadata without touching its body: _mou.cas names the mutation just made, and pCas/pRev name the mutation that last wrote the body. pRev did not keep up with pCas:

• computeMetadataOnlyUpdate carried pCas forward when replacing a metadata-only update but always wrote the document's current revision sequence number, so chained metadata-only updates left pCas naming the body write and pRev naming the update before them.

• restampVersionCAS wrote pRev as 0, because updateAndReturnDoc reads the document without the revSeqNo virtual xattr and so never populates doc.RevSeqNo in the write path. The re-stamp is now a read-modify-write that takes the revision sequence number the server assigned the write it is correcting, declining the correction when the document has moved on since - previously a CAS mismatch, now ErrUpdateCancel. Both mean the write being corrected has been superseded, so both are skipped, behind a shared isSupersededWriteError predicate.

• MigrateAttachmentMetadata hand-built its _mou and never read the existing one, so it could not carry anything forward. It now uses computeMetadataOnlyUpdate like the other metadata-only writers.

Only pRev was ever wrong. Running the tests below against the parent commit places the damage, where a chained update is one landing on a document whose previous mutation was already a metadata-only update:

metadata-only write path	pRev, first update	pRev, chained
on-demand import for get	correct	wrong
on-demand import for write	correct	wrong
attachment metadata migration	correct	wrong
channel history compaction	correct	wrong
resync	correct	wrong
version-CAS correction	wrong, always 0	wrong

Nothing else about these paths was wrong:

behaviour	before this commit
_mou.cas naming the mutation just made	correct
pCas, chained updates included	correct
re-stamping a tombstone	already worked
declining a superseded re-stamp	already worked

Re-stamping a tombstone already worked because updateXattrs sets SubdocDocFlagAccessDeleted, and WriteTombstoneWithXattrs reaches that same call for an existing tombstone. A re-stamp superseded by a concurrent write was already declined, on a CAS mismatch.

TestMetadataOnlyUpdateWritePaths covers the paths whose metadata-only write lands on a document some earlier write left behind - both on-demand import paths, attachment metadata migration, channel history compaction and resync - asserting that _mou.cas names the mutation just made, that pCas and pRev name the last write to the body, and that both survive a following metadata-only write from a different path.

TestRestampVersionCASMou covers the version-CAS correction separately, over a live document and a tombstone. It is the one path whose metadata-only write follows a body write it makes itself, so _mou names that write rather than anything observable before it, and both previous values are pinned by sandwiching them between the mutation preceding the write under test and the re-stamp that follows it. Channel history compaction stands in for resync as the second metadata-only write there, as resync declines a tombstone.

TestAttachmentMigrationMouCarriedForward covers migration of a document whose previous mutation was already a metadata-only update, as arrives by mobile XDCR from a cluster that had not migrated its attachment metadata. TestRestampVersionCASSkipsConcurrentWrite covers the declined correction, asserting that the re-stamp is refused in a way correctVersionAheadOfCAS skips on and that the concurrent write is left untouched.

TestWritePathRepairMouChain, which covers the rev tree repair on the write path, pinned pRev to the mutation immediately before the repair - the import the repair chains back past. That contradicted its own pCas assertion, which already required the chain to reach the SDK write, so the pRev assertion now names the same write pCas does.

Pre-review checklist

• Logging sensitive data? Make sure it's tagged (e.g. base.UD(docID), base.MD(dbName))
• Updated relevant information in the API specifications (such as endpoint descriptions, schemas, ...) in docs/api

Integration Tests

• https://jenkins.sgwdev.com/job/SyncGatewayIntegration/1140/

[Copilot]

🔵 Needs a closer look

The changes touch low-level metadata/xattr write semantics and concurrency-sensitive correction logic where subtle regressions can be costly, so a final human review is warranted.

Pull request overview

This PR fixes _mou (metadata-only update) bookkeeping so that _mou.pRev stays consistent with _mou.pCas across chained metadata-only updates, and corrects the version/CAS re-stamp path to record the correct rev-sequence number while safely skipping superseded corrections.

Changes:

• Fix computeMetadataOnlyUpdate to carry forward both prior mutation identifiers (pCas and pRev) when replacing an existing metadata-only update.
• Update attachment metadata migration and version/CAS re-stamping to compute _mou from the current _mou/revSeqNo state (and skip re-stamps when the target write has been superseded).
• Add/adjust targeted tests to pin _mou behavior across all metadata-only write paths, chained updates, and concurrent-write skip scenarios.

File summaries

File	Description
db/document.go	Fix _mou computation to keep pRev aligned with pCas when chaining/replacing metadata-only updates.
db/crud.go	Update attachment migration to reuse _mou chaining logic; refactor version/CAS re-stamp to read revSeqNo and skip superseded corrections via a shared predicate.
db/import_test.go	Add comprehensive tests validating _mou invariants across multiple metadata-only write paths and chained updates; add revSeqNo helper.
db/hybrid_logical_vector_test.go	Add tests for re-stamp _mou correctness and for skipping re-stamp when a concurrent write has moved the document forward.
db/crud_test.go	Adjust write-path repair test to assert pRev chains back to the same body mutation as pCas.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[factory-droid]

Droid finished @torcolvin's task —— View job

---

Review complete.

LGTM — I did not find any high-confidence, actionable issues in the changed production code or the added/updated tests.

[adamcfraser]

Changes seem good, one suggestion about error handling.

[adamcfraser]

Are all ErrUpdateCancel cases definitely due to a superseding write? I guess they are for the specific usage of ErrUpdateCancel in the callback used by restampVersionCAS. Given that, it feels like this check should be embedded inside restampVersionCAS, so that someone doesn't come along and use isSupersededWriteError more generically.

[torcolvin]

Since I changed restampVersionCAS to be WriteUpdateWithXattrs, it can not return a cas error anymore, only ErrUpdateCancel. Good catch!