feat(dds): squash-on-resubmit for non-tree DDSes

[anthony-murphy]

Summary

Implements reSubmitSquashed across all non-tree DDSes so staging-mode commits (commitChanges({ squash: true })) drop intermediate values before they reach the wire. Values written and removed within a single staging session — e.g. a sensitive string set and then deleted before commit — are no longer transmitted as part of the squashed batch. With this change, the listed DDSes no longer depend on the Fluid.SharedObject.AllowStagingModeWithoutSquashing config flag fallback.

Per-DDS treatment

DDS	Treatment
SharedCell, SharedCounter, SharedMap, SharedDirectory, SharedMatrix	Content-aware per-cell / per-key LWW squash
SharedTaskManager	Explicit override delegating to reSubmitCore (already collapses volunteer/abandon pairs by disconnect semantics)
SharedSequence / intervals	Unchanged — squash was already wired end-to-end via merge-tree's regeneratePendingOp(squash)
Ink, ConsensusRegisterCollection, ConsensusOrderedCollection, PactMap, legacy SharedArray, legacy SharedSignal	Identity squash with documented rationale (append-only, order-preserving, or consensus-bound semantics)

The mixed-lifetime case

For Map/Directory, PendingKeyLifetime groups consecutive sets on the same key with no intervening delete or clear. A pre-staging set(k, A) followed by a staging set(k, B) lands in one lifetime, with keySet A pre-staging and keySet B staging.

replayPendingStates({ committingStagedBatches: true }) only replays staged batches — pre-staging messages remain in flight at the runtime layer. The naive approach of wiping the entire lifetime during squash would lose pre-staging keySet A from kernel state, so when its ACK eventually arrives, the kernel finds the squashed lifetime instead and asserts 0xc07: Got a local set message we weren't expecting.

The fix in MapKernel.squashPendingDataForBatch / SubDirectory.squashPendingStorageForBatch:

1. Locate the staging boundary — either a standalone entry, or a (lifetimeIdx, keySetIdx) pair inside a lifetime.
2. Compute finalKeyOps first, including the mixed lifetime's staging suffix (its last keySet provides the candidate for that key, unless a later clear in the staging slice nullifies it).
3. Then mutate: truncate the mixed lifetime to its pre-staging prefix (splice the lifetime entirely if its prefix is empty); drop the pure-staging entries.
4. Submit the squashed ops.

Order matters — computing before mutating preserves the staging-suffix value needed for LWW.

The same shape (boundary detection, suffix-aware finalKeyOps, mutate-after) is used in both MapKernel and SubDirectory.

Test coverage

• Unit tests: new squash.spec.ts (Map, Directory) and matrix.squash.spec.ts (Matrix), plus inline blocks in cell.spec.ts and counter.spec.ts. Cover set-then-delete, set-then-set chains, clear-in-staging, delete-after-clear, single-pending pass-through.
• Stress: local-server-stress-tests now plumbs squash: random.bool() through ExitStagingMode ops, so the 200-seed default suite exercises end-to-end squash across all wired DDSes. The stress run is what caught the mixed-lifetime bug above — both the disposed-subdirectory guard and the keySet-boundary split were added in response to seeds it generated.
• Test helper: reconnectAndSquash is now exported from @fluid-private/test-dds-utils for per-DDS unit-test use. It was previously internal to the fuzz harness.

Result: 4,440 unit tests across 12 DDS packages pass, plus 200/200 stress seeds in the local-server-stress default workload.

Known scope limitations (intentional)

• SharedDirectory subdir create/delete lifecycle: not squashed (the create/delete ops carry no user content, so this is a future optimization rather than a leak fix). Existing reSubmitCore handles them.
• SharedTaskManager: in-staging abandon for a task volunteered pre-staging isn't propagated by this squash path — reSubmitCore assumes the server has already removed the client from the queue, which is consistent with TaskManager's overall connection-bound design.
• legacy SharedArray: identity squash. The insertEntry op carries the entry's user-supplied value, so an inserted-then-deleted entry within one staging session can still surface its value on commit. A full per-entry squash would mirror SharedMap's complexity in a DDS marked "not intended for use in new code" — left as a future task with a documented caveat in the code.
• ConsensusRegisterCollection: writes participate in version history exposed via readVersions(). Collapsing pending writes would change observable semantics; intentional no-squash.

Test plan

• pnpm install + fluid-build for every touched package
• Unit tests pass for cell, counter, task-manager, map (Map + Directory), matrix, sequence, ink, register-collection, ordered-collection, pact-map, legacy-dds
• local-server-stress-tests 200-seed default workload passes with randomized squash on every staging commit
• CI typecheck / lint / API-extractor across all affected packages

🤖 Generated with Claude Code

[github-actions]

Hi! Thank you for opening this PR. Want me to review it?

Based on the diff (3785 lines, 41 files), I've queued these reviewers:

• Correctness — logic errors, race conditions, lifecycle issues
• Security — vulnerabilities, secret exposure, injection
• API Compatibility — breaking changes, release tags, type design
• Performance — algorithmic regressions, memory leaks
• Testing — coverage gaps, hollow tests

How this works

• Adjust the reviewer set by ticking/unticking boxes above. Reviewer toggles alone don't trigger anything.

• Tick Start review below to dispatch the review fleet.

• After review finishes, tick Start review again to request another run — it auto-resets after each dispatch.

• This comment updates as new commits land; your reviewer selections are preserved.

• Start review

[anthony-murphy]

Claude here, following up on the algorithmic-complexity concern.

Inner-walk audit per DDS

DDS	Inner work per reSubmitSquashed call	Degenerate input	Per call	Total
SharedCounter	identity — delegates to reSubmitCore	—	O(1)	O(N) ✓
SharedCell	pendingMessageIds.indexOf + splice	many staged sets on one cell	O(N)	O(N²)
SharedMap / SharedDirectory	walk pendingData to locate metadata, walk again for subsumption, nested keySets.indexOf inside the locate	many staged sets on one key (all in one PendingKeyLifetime)	O(N) — and O(K²) inside the lifetime	O(N²)
SharedMatrix	per-cell local.findIndex + splice	many staged sets on one cell	O(K)	O(K²) per cell
SharedTaskManager	per-task latestPendingOps[].findIndex	many staged volunteer/abandon on one task	O(P)	O(P²) per task
SharedSequence	merge-tree internal	—	already amortized via merge-tree	—

Small change in 0d5957c

PendingKeySet now has a lifetime: PendingKeyLifetime back-pointer. dropIfSubsumed jumps to the containing lifetime in O(1) via metadata.lifetime and does the tip check by reference comparison (metadata === lifetime.keySets[length - 1]). No pendingData walk and no nested keySets.indexOf in the common non-tip case.

That collapses the locate phase for the "many sets on one key" pattern (everything in one lifetime) from O(K²) to O(K). The tip case and standalone clear / delete metadata still scan pendingData once (a single indexOf), and the subsumption walk for "any later entry affecting this key" remains O(N-i).

Remaining O(N²) risk and possible follow-ups (intentionally not in this PR)

The current code is still O(N²) in two narrower worst cases:

1. Many staged ops with mixed types in pendingData — the pendingData.indexOf for standalone metadata and the subsumption walk for "later entry affecting this key" are still O(N) per call. The fix would be either generation tracking (lastEntryGenerationByKey: Map<string, number> + lastClearGeneration: number, updated on push) or a per-key doubly-linked chain through pendingData entries. Both add maintenance overhead on every set/delete/clear submit, so I'd want signal that it's worth taking before doing it.

2. Many sets on one key — splice within lifetime.keySets — even with the back-pointer, dropping a non-tip keySet is still keySets.splice(idx, 1), which is O(K) for the shift. Total O(K²) for K drops in a row. The fix is either a linked-list keysets or a head-offset trick (keySetsHead: number + lazy compaction). Both are more invasive than the back-pointer because the existing iterator API also reads from keySets.

SharedCell, SharedMatrix, SharedTaskManager have analogous O(N) inner walks but their N is bounded per cell / per task in practice — happy to look at any of them if you'd like the same treatment.

The headline guarantee — staged inserted-then-deleted secrets don't reach the wire — is unchanged by these complexity considerations. Pushed in 0d5957c; 932/932 SharedMap+SharedDirectory unit tests pass, 199/199 stress seeds pass.

[anthony-murphy]

Claude here — preemptive cross-DDS audit while deep review runs. Ran an op-type-by-op-type sweep across every DDS in scope: enumerate each operation interface, find every submit / process / resubmit site, and verify the squash decision against each op type. Pushed in 542ed4d.

Verdicts per DDS

DDS	Op types audited	Verdict
SharedCell	setCell, deleteCell	✓ correct; both op types covered by LWW tip-check
SharedCounter	increment	✓ correct; only op type; identity squash sound
SharedMap	set, delete, clear	✓ correct; subsumption rules cover all three
SharedDirectory storage	set, delete, clear	✓ correct; same as Map plus subdir-reachability guard
SharedDirectory subdir lifecycle	createSubDirectory, deleteSubDirectory	✓ no leak (only subdirName string is on the wire); intentionally not squashed (idempotent server-side)
SharedMatrix	setCell, row/col vector ops	✓ correct; setCell per-cell LWW; vector ops delegate to merge-tree's squash=true
SharedTaskManager	volunteer, abandon, complete	✓ correct; reSubmitCore already collapses pairs by design
PactMap	set, accept	✓ safe via refSeq filtering in reSubmitCore
Ink	createStroke, stylus, clear	✓ documented intentional leak
legacy SharedArray	insertEntry, deleteEntry, moveEntry, toggle, toggleMove	✓ insertEntry leak documented; the other four carry only ids
legacy SharedSignal	signal	✓ documented intentional leak
SharedSequence text / intervals (text + endpoint channels)	INSERT, REMOVE, ANNOTATE, OBLITERATE, GROUP, interval ADD/CHANGE/DELETE	✓ correct for text and endpoints via merge-tree's regeneratePendingOp(squash)

Actionable findings, addressed in 542ed4d

1. SharedCell lacked a pre-staging-in-flight regression test (Counter/Map/Directory had one). Added: pre-staging set(\"pre\") → disconnect → set(\"secret\") + delete during staging → commit-squash → assert \"secret\" never reaches the peer. Per-op squash logic was already correct; the test closes a coverage gap.

2. ConsensusOrderedCollection.add leak was undocumented. Identity squash transmits the add op on commit, so add(secret) → acquire → complete inside a staging session sends secret. Comment updated to call this out alongside the SharedSignal / SharedArray / RegisterCollection callouts. No code change (the leak is intentional given the queue's order-dependent semantics — same logic as the others).

Known limitations, documented as not addressed in this PR

3. SharedSequence property channel is not squashed by merge-tree. Text content + interval endpoints squash correctly: insertText(0, \"secret\") → removeRange(0, ...) → squash drops the insert via squashInsertion() (merge-tree client.ts:1423-1442). But the property channel does not get the same treatment:

   • annotateRange(0, 1, {foo: \"secret\"}) → annotateRange(0, 1, {foo: \"public\"}) — both ops resubmit; the squash flag is unused inside merge-tree's annotate handling.
   • insertText(0, \"x\", {handle: poison}) with no later removal — the insert's properties resubmit verbatim (correct — the user committed them); but the inserted-then-removed case where squashInsertion() fires does suppress the segment data including its properties (no leak).
   • Interval CHANGE ops with property-only changes bypass squash logic entirely (endpointChangesNode === undefined short-circuits in rebaseLocalInterval).

   This is a pre-existing merge-tree gap, not introduced by this PR. The existing sharedString.squash.fuzz.spec.ts already notes "once we support squashing property changes, we should record additional information in poisonedHandleLocations" (line 156-158), confirming this was already known as future work. Adding it to the changeset's known-limitations list so it's explicit at release.

4. Subdir create+delete collapse: a staged deleteSubDirectory(x) → createSubDirectory(x) (same name) sends both ops. They're idempotent server-side and the wire ops only carry subdirName — no data-leak surface. Left as a future optimization rather than a correctness fix.

Test posture after this commit

• 932 + 13 + 217 + 791 + 23 + 217 + 234 + 14 + 94 + 260 + 1664 = 4,459 unit tests across 12 DDS packages, all green
• 199/199 stress seeds pass
• All three deep-review threads resolved

[anthony-murphy]

Correction to my own audit comment above. @anthony-murphy pointed out that subdirName is a leak, and they're right — user-supplied content can absolutely live in subdirectory names (user ids, tenant slugs, etc.). My earlier "no leak (only subdirName string is on the wire)" framing was wrong.

Fixed in d2bb799. New SubDirectory.dropIfSubsumedSubdirOp(subdirName, opType):

• A staged createSubDirectory(name) is subsumed if any later pending entry for the same name exists. If the subsumer is a deleteSubDirectory(name), both entries are spliced so the matching delete call also drops itself — neither op reaches the wire.
• A deleteSubDirectory(name) whose entry was already spliced (as part of a paired create) drops itself.
• A deleteSubDirectory(name) whose entry is still present (the pre-existing-subdir case) falls through to reSubmitCore; the name was already on the wire pre-staging, so no new leak.

Three regression tests in squash.spec.ts:

✔ drops a staged createSubDirectory + deleteSubDirectory pair so the subdir name doesn't leak
✔ keeps the final create when staged ops are create+delete+create on the same name
✔ preserves a delete of a pre-existing subdirectory (no leak, no false subsumption)

The first test sets up a peer listener on subDirectoryCreated / subDirectoryDeleted and asserts zero events fire for the squashed pair on a name like "secret-id".

935/935 SharedMap+SharedDirectory unit tests pass (was 932 before this fix). Stress re-run in progress.

[anthony-murphy]

Closing the property-channel gap the audit identified.

What changed

merge-tree (40760b2)

SegmentGroupCollection.keysAnnotatedLaterThan(localSeq) returns the keys touched by later annotates on a given segment — derived from each later `SegmentGroup`'s `previousProps[i]` (whose keys are exactly the keys the annotate touched, by construction of `propertyManager.handleProperties`).

In `Client.resetPendingDeltaToOps` under `squash=true`:

• ANNOTATE case: `resetOp.props` is filtered against `keysAnnotatedLaterThan(segmentGroup.localSeq)`. If every key is overridden, the op is dropped entirely (no `createAnnotateRangeOp` call). Adjust ops (`resetOp.adjust`) are unchanged — different semantics.
• INSERT case: `segInsertOp.properties` is filtered the same way. The segment text still flows; only the per-key values later overwritten by a staged annotate are stripped. If the filter leaves no keys, `properties` becomes `undefined`.

sequence (interval collection) (8c68884)

`IntervalAddLocalMetadata` and `IntervalChangeLocalMetadata` now carry a `propertyKeys?: ReadonlySet` populated at submit time from the `props` arg.

In `IntervalCollection.resubmitMessage` under `squash=true` for ADD or CHANGE:

• If a later pending op for the same interval id is a DELETE, the entire ADD/CHANGE is dropped.
• Otherwise `value.properties` is filtered against the union of `propertyKeys` from later pending ADD/CHANGE ops on the same interval. Filtering is non-destructive (spread, not mutation). If the filter empties the property set, `properties` becomes `undefined`.

The prior `endpointChangesNode === undefined` bypass that skipped `rebaseLocalInterval` entirely for property-only CHANGE ops is still in place for the rebase path, but the property filter now runs before that branch, so property-only changes get squash treatment.

Regression tests

Four new tests in the existing `squash property channel` describe block:

```
✔ drops a staged annotate value overridden by a later staged annotate on the same range
✔ drops a staged insert's property value overridden by a later staged annotate
✔ drops a staged interval add subsumed by a later staged delete
✔ drops a staged interval change's property value overridden by a later staged change
```

Each test sets up a peer listener (segment `sequenceDelta` or interval `addInterval`/`propertyChanged`) and asserts the secret value never appears in the captured stream while the final value lands correctly.

Test posture

• 1555/1555 merge-tree tests pass
• 1668/1668 sequence tests pass (4 new)
• 935/935 SharedMap+SharedDirectory tests pass
• 200/200 stress seeds pass

Remaining audit gaps (documented, not in this PR)

The audit also flagged user-content leaks on identity-squashed DDSes where the op itself carries the leaked value and the DDS's semantics make true squash structurally harder than per-key filtering:

• `ConsensusOrderedCollection.add(value)`: the queue's `acquireId` is server-assigned, so a client can't tell at squash time whether a staged `add → acquire → complete` chain actually resolves to its own add. Documented in the override.
• legacy `SharedArray.insertEntry(value)`: insert-then-delete on the same `entryId` could be squashed but the DDS's internal pending state (counters per entry + a doubly-linked skip list) makes the change non-trivial. Documented.
• `ConsensusRegisterCollection` writes participate in `readVersions()` history; intentional.
• `Ink` and legacy `SharedSignal` ops are fire-and-forget user-event content; intentional.

If any of these blocks ship, I can prioritize them next.

[anthony-murphy]

Deep Review

Reviewed commit 10ace64 on 2026-05-19.

Readiness: 5/10 — MAKING PROGRESS

Not ready for sign-off. Both Tier 2 issues flagged on 1e3232f are resolved on this head — stripUserPropertiesFromDeletePayload scrubs the delete-op payload and stagingBoundaryIdx scopes SharedArray.computeSquashPlan to the staged slice. A fresh Tier 2 surfaces in the pair-cancel branch the IntervalCollection fix added: hasEarlierAddForInterval (intervalCollection.ts:3870-3883) walks this.pending[id].local with no staging-batch tag, so a pre-staging add still in flight plus a staged delete makes the staged delete drop itself via clearEmptyPendingEntry — peers retain an interval the local view shows deleted. Flagged inline on intervalCollection.ts. Held Tier 3 backlog (PR-description drift, three duplicate shortcode strings, Counter/Ink/CRC/PactMap design tradeoffs, fallback default) is unchanged.

Path to Ready

• Resolve inline threads
• After the IntervalCollection fix lands, refresh the PR body's "mixed-lifetime case" section to describe the per-op tryResubmitSquashedMessage / dropIfSubsumed design using the PendingKeySet.lifetime back-pointer (mapKernel.ts:1820, directory.ts:1421) — the current text still describes a squashPendingDataForBatch / finalKeyOps compute-then-mutate algorithm that does not exist in the diff
• Move legacy SharedArray out of the per-DDS table's "Identity squash" row; document the chain-walker, the stagingBoundaryIdx-scoped wireBlacklist lifetime, and the residual insertEntry.value leak for non-subsumed inserts
• Add a row for the new sequence property-channel filter (SegmentGroupCollection.keysAnnotatedLaterThan + IntervalCollection property-key filtering) — the current "SharedSequence/intervals — Unchanged" line is wrong
• Differentiate the three duplicate-message shortcodes (0xcf9 / 0xcfd / 0xcfe, 0xcfb / 0xcff, 0xcfc / 0xd02) in assertionShortCodesMap.ts:4377-4382
• Complete the final CI box in the test plan (typecheck / lint / API-extractor) and re-confirm 200/200 stress seeds on the IntervalCollection-fix head
• Un-draft once the above land

Context for Reviewers

• Lands the squash-rebase work explicitly deferred from staging-mode introduction in #23783 ("squash rebase, where content that is added then removed is not included in the final set of ops sent over the wire"). reSubmitSquashed mirrors the cross-DDS uniform-method template as applyStashedOp from this author's #19518 — useful as a reading template.
• mapKernel.ts / directory.ts are in a recently-unstable regime: #25070 (ChumpChief, 2025-07-22) re-landed PendingKeyLifetime/PendingKeySet after #24961 was reverted via #25066 over stress-test failures. This PR extends the same data model with a lifetime back-pointer plus tryResubmitSquashedMessage / dropIfSubsumed / dropIfSubsumedByLaterStorageOp. Two earlier rounds of this review already caught a disposed-subdir guard hole (fixed in 2149ab7) and a name-only dropIfSubsumedSubdirOp matching that caused wrong-pair splicing on delete→create→delete (fixed in d2bb799). Treat the 200/200 stress-seed result as load-bearing.
• Counter identity-squash is deliberate: #25771 (ChumpChief, Counter rollback) warned that comparing only increment properties is ambiguous when many ops increment by the same value. Tests at counter.spec.ts:180-210 assert two remote events with amounts 10 and -3, not a single net 7 — coalescing would change observable behavior.
• Suggested reviewers from prior ownership:
  • ChumpChief — map.ts / mapKernel.ts / directory.ts / counter.ts (#25070 PendingKeyLifetime, Counter rollback #25771)
  • Abe27342 — test-dds-utils, matrix, merge-tree, all *.squash.fuzz.spec.ts (merge-tree squash #24424, DDS squash fuzz framework #24425; same-name create/delete cycles on #15493)
  • markfields — local-server-stress-tests and the runtime ↔ DDS reSubmitSquashed contract (staging-mode #23783)
  • scarlettjlee — cell.ts (Cell rollback #12273)
  • alexvy86 — assertion shortcode policy (#10488)

For human reviewer

• Runtime ↔ DDS contract (markfields) — Does the runtime guarantee that pre-staging ops still in flight are never passed to reSubmitSquashed, and that they are not present in per-DDS pending lists at squash time? The new IntervalCollection finding, the SharedArray stagingBoundaryIdx fix, and the MapKernel lifetime back-pointer all hinge on the answer. Worth an explicit ack rather than inferring from the harnesses.
• Staging-boundary plumbing (Abe27342 / ChumpChief) — Per-DDS implementations diverge: Map/Directory use a per-op lifetime back-pointer, SharedArray uses a single boundary index, test-dds-utils/utils.ts:23-28 exposes firstStagedIndex directly, and IntervalCollection (post-fix) will need its own answer. Standardize one mechanism, or keep per-DDS shape?
• Counter identity-squash semantics (ChumpChief) — Sanity check on preserving per-op "incremented" events rather than algebraic coalescing. Both blind design reviewers expected coalescing; the choice is defensible but worth confirming as the final position.
• Ink stroke/clear semantics (ChumpChief) — Identity squash transmits stroke payloads when a staged createStroke → clear chain occurs. Documented as a limitation; stroke data is user content. Final position or follow-up?
• AllowStagingModeWithoutSquashing default (markfields) — Permissive default contradicts the PR's stated privacy goal for out-of-tree DDSes. Default flip vs. one-time telemetry/warning is a deprecation-policy call.
• ConsensusRegisterCollection / PactMap intermediate-value question (markfields / ChumpChief) — Whether to drop locally-superseded same-key writes is a consensus-semantics design call.
• Stress runtime characteristics — Author cites 200/200 stress seeds passing. The perf cost of dropIfSubsumedByLaterStorageOp over long lifetimes, and of any staged-slice scan in the IntervalCollection fix, cannot be assessed by static review.
• Cross-fork compatibility — Whether out-of-tree DDS implementers should be notified of the reSubmitSquashed contract ahead of any future AllowStagingModeWithoutSquashing default flip.

Review history (5 prior reviews)

• 1e3232f 2026-05-19 · 5/10 — IntervalCollection delete-gate omits property scrub; SharedArray cross-staging walk flagged
• 9a28475 2026-05-18 · 5/10 — dropIfSubsumedSubdirOp name+opType matching and reconnectAndSquash harness over-marking flagged
• 887e66b 2026-05-15 · 5/10 — SubDirectory.dropIfSubsumedSubdirOp wrong-pair splicing on same-name delete→create→delete and pre-staging-create + staged cycle
• 600ec6d 2026-05-15 · 5/10 — Counter and MapKernel fixes resolved prior threads; SharedDirectory disposed-subdir set + deleteSubDirectory leak flagged
• 65c8a84 2026-05-15 · 4/10 — Counter and MapKernel correctness bugs flagged inline

[github-actions]

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output

> fluid-framework-docs-site@0.0.0 ci:check-links /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test "npm run serve -- --no-open" 3000 check-links

1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> fluid-framework-docs-site@0.0.0 serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/

> fluid-framework-docs-site@0.0.0 check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  288859 links
    1925 destination URLs
    2175 URLs ignored
       0 warnings
       0 errors