Validate pending state against service history

[Alex Villarreal (alexvy86)]

Description

Pending-state rehydration previously restored the last processed sequence number but discarded the corresponding saved message and its minimum sequence number. As a result, it could not use DeltaManager's existing overwritten-history validation and only validated future minimum sequence numbers against the older base snapshot.

Pass the last saved op into DeltaManager so it:

• re-fetches and validates that op before continuing from pending state;
• rejects a later op whose minimum sequence number moves backward; and
• includes the raw service checkpoint as diagnostic context only when either anomaly occurs.

This replaces the standalone checkpoint-regression telemetry because service checkpoints may legitimately lag and therefore are not independently actionable. The overlap validation remains deliberately asynchronous: pending-state rehydration does not wait for network access, but a mismatch closes the returned container once detected.

Tests cover matching and conflicting serialized saved-op anchors, raw checkpoint normalization with connection-before-attachment ordering, malformed saved-op minimum sequence numbers, and the production Container path that returns while validation is blocked and later closes on mismatch.

AB#82434

Reviewer Guidance

The review process is outlined in the pull request guidelines.

Please focus on using the last persisted op as the rehydration baseline for both overlap validation and minimum-sequence-number monotonicity, including the deliberately optimistic asynchronous validation behavior.

[github-actions]

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

Based on the diff (688 lines, 6 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

[Copilot]

🟡 Changes recommended

Checkpoint normalization can prevent valid regression telemetry from being emitted.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds telemetry for service checkpoint sequence number regressions.

Changes:

• Emits CheckpointSequenceNumberRegression diagnostics.
• Adds regression telemetry coverage.

The telemetry currently reads a normalized checkpoint, which can conceal a lower service-provided checkpoint when initial messages advance it. Preserve the raw checkpoint or compare before normalization, and test this scenario.

File summaries

File	Description
packages/loader/container-loader/src/test/deltaManager.spec.ts	Tests checkpoint regression telemetry.
packages/loader/container-loader/src/deltaManager.ts	Detects and logs checkpoint regressions.

Review details

Suppressed comments (1)

packages/loader/container-loader/src/deltaManager.ts:530

• This comparison can run before sequence tracking is initialized. Container.load starts the delta connection before fetching the snapshot/pending state (container.ts:1640-1670), while attachOpHandler assigns lastProcessedSequenceNumber later. If connection wins that race, this compares checkpoint 5 with the default 0, then attachment sets the value to 13 without rechecking, so the regression is never logged. Run the check after handler initialization as well (or defer it until then), with per-connection deduplication.

		if (
			checkpointSequenceNumber !== undefined &&
			checkpointSequenceNumber < this.lastProcessedSequenceNumber

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Balanced

---

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

[Copilot]

🟡 Changes recommended

Two moderate test coverage gaps must be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Balanced

[Tony Murphy (anthony-murphy)]

packages/loader/container-loader/src/container.ts:1674

Deep Review: For pending state, load-mode setup forces opsBeforeReturn: undefined (container.ts:340-345), so production hits the case undefined branch that calls attachDeltaManagerOpHandler(..., "all", lastProcessedMessage) behind // eslint-disable-next-line @typescript-eslint/no-floating-promises — the validation fetch is deliberately not awaited. Both new tests drive startDeltaManager(..., savedOp, "all") and await it directly (deltaManager.spec.ts:201-280); neither exercises this Container fire-and-forget branch, which is the real production entry point, so a regression in the Container ordering would pass CI. Add a Container-level test that drives the real opsBeforeReturn === undefined branch and asserts the saved-op validation is kicked off (and closes the container on mismatch) without blocking Container.load. Whether the validation should remain a floating promise or become a synchronous load barrier is a design call for the invariant owners — flagged for human review.

[Alex Villarreal (alexvy86)]

Addressed the Container-level deep-review feedback in a0e39f5. The new local-server test drives the production pending-state load path, blocks the anchor fetch, verifies Container.load returns while validation is still pending, then releases a mismatched anchor and verifies the returned container closes. Validation remains deliberately asynchronous so pending-state rehydration does not acquire a network dependency before returning.

[Tony Murphy (anthony-murphy)]

Deep Review: Confirmed — the new local-server test (pendingStateHistoryValidation.spec.ts) drives the real opsBeforeReturn === undefined production branch: Container.load returns while the anchor fetch is blocked, then the returned container closes once a mismatched anchor is released. That closes the Container-level coverage gap raised in that comment. Keeping the validation asynchronous preserves the #5773 non-blocking-boot design; whether the optimistic model should stay fire-and-forget or become a synchronous load barrier remains a design call flagged for the invariant owners.

[Tony Murphy (anthony-murphy)]

packages/loader/container-loader/src/deltaManager.ts:754

Deep Review: A live socket op can abort the anchor-validation storage fetch before the persisted anchor is re-read, so the overwritten-history check silently does not run on the common online-reconnect path.

attachOpHandler seeds this.pendingStateAnchor = lastProcessedMessage and kicks off this.fetchMissingDeltasCore("DocumentOpen_all", false) before _inbound.resume(). Because the request is unbound (to === undefined), the cancellation predicate is cancelFetch = (op) => op.sequenceNumber >= this.lastObservedSeqNumber (deltaManager.ts:712-714). The opListener fires on every "push" to this._inbound; for a live socket op !opsFromFetch is true and cancelFetch(op) is true, so controller.abort("DeltaManager getDeltas fetch cancelled") terminates the storage fetch. DeltaQueue.push emits "push" while the queue is still paused (deltaQueue.ts:90-94), so a socket op enqueued during the pending fetch can abort it before the storage stream delivers the anchor message. When that happens pendingStateAnchor is never re-observed and the fileOverwrittenInStorage / payload-mismatch check does not run during normal online rehydration — defeating the feature's core data-integrity check in the happy path.

The new integration test (pendingStateHistoryValidation.spec.ts:116-141) delays and then releases storage validation but never races a live op against the fetch, so this abort-by-live-op scenario is untested.

Fix options: fetch the anchor with a bounded range (e.g. from-1 .. from) before starting the unbound stream catch-up, or shield the anchor-validation fetch from live-op cancellation until the anchor sequence number has been received and compared. Add a test that races a live socket op against the anchor fetch and asserts the overwrite check still runs (container closes on a mismatched anchor).

Question for the author: is the anchor-validation fetch expected to survive a concurrent live-op reconnect, or is the online case intentionally allowed to skip overwritten-history validation? If the latter, that tradeoff should be documented.

[Alex Villarreal (alexvy86)]

[Agent-generated]

Addressed in 53a7ab8. Live socket ops can cancel normal catch-up only after pendingStateAnchor has been re-read and compared, so the existing unbounded fetch remains cancellable without skipping the integrity check. The DeltaManager test now emits a live socket op while the anchor storage read is blocked, verifies the fetch signal remains active, then releases a mismatched anchor and verifies the overwrite error still closes the DeltaManager.

[Tony Murphy (anthony-murphy)]

Deep Review: Confirmed — the new this.pendingStateAnchor === undefined guard (deltaManager.ts:749) keeps a live socket op from cancelling catch-up until the anchor has been re-read and compared, and the DeltaManager test now emits a live op while the anchor storage read is blocked, verifies the fetch signal stays active, then releases a mismatched anchor and confirms the overwrite error still closes the DeltaManager. The online-reconnect skip is resolved. Two new data-integrity gaps are flagged in the current review: validation is bypassed on deltaConnection: "none" pending-state loads, and the anchor comparison excludes op contents.

[Tony Murphy (anthony-murphy)]

Deep Review: Confirmed — Container.connect() now requests storage ops while an unvalidated pending-state anchor is present, so a deltaConnection: "none" load validates its anchor on explicit connect, and the new integration test covers it. One residual on this same path: if the validation fetch runs but the service never returns the anchor op (history truncated below the saved edge), nothing validates or clears pendingStateAnchor — it stays set forever, permanently disabling cancelFetch and keeping fetchOpsFromStorage true on every deferred connect. I've flagged that as a separate inline thread on the anchor-clear branch (deltaManager.ts:1110-1111), since the fix here only forces the fetch to run, not to complete with the anchor observed.

[Tony Murphy (anthony-murphy)]

Deep Review: Confirmed — the anchor comparison now includes normalized contents, closing the content-only-rewrite false negative, and telemetry carries only a contentsDiffer boolean (not the payload). One new surface the fix introduces: comparableMessageContents falls back to JSON.stringify, which is not canonical across object key ordering, so a re-fetched op that serializes the same object with different key order can set contentsDiffer = true and fatally close the container. Flagged as a separate inline thread on the contentsDiffer gate (deltaManager.ts:1082-1086).

[github-actions]

Bundle size comparison

Base commit: cb0d7e6f7979287cbbe54effc632f853f46d6255
Head commit: ee89ecfdefc98e8a522c25329bdc848baf89dd6c

Pending — Build - client packages is running. Results will appear here when the build completes.

[Tony Murphy (anthony-murphy)]

Deep Review

Reviewed commit ee89ecf on 2026-09-03.

Readiness: 6/10 — MAKING PROGRESS

Not ready for sign-off, but genuine forward progress: two of the three correctness bugs from the last review are resolved at this commit — the never-returned-anchor cleanup-before-drain false negative, and the "cached"/"all" unhandled-rejection concern (the author rebutted it with concrete evidence that Container.load() rewrites opsBeforeReturn to undefined whenever pendingLocalState exists, container.ts:341-345). One contained concern remains: comparableMessageContents gates a fatal, non-retryable container close on a non-canonical JSON.stringify, so a key-order difference between the round-tripped saved anchor and the re-fetched service op can false-close a healthy container with no recovery. The design is sound and independently validated; the remaining item is localized with a cheap fix.

Path to Ready

• Resolve inline threads
• Confirm CI is green — Build - client packages is still pending; ensure it passes so the bundle-size and downstream gates run, then take the PR out of draft

Context for Reviewers

• Anchor mismatch reuses the Better handling when loader detects file changes (epoch change detected on client) #5882 fileOverwrittenInStorage / NonRetryableError machinery (originally from Add protection against data loss #3900), not DataCorruptionError — a deliberate choice to treat op-data mismatch as a legitimate file-overwrite/epoch change. This PR widens the trigger to also compare op contents; confirm the wider trigger does not false-signal for legitimately-lagging service checkpoints.
• The non-blocking load path follows the Implement more controls on how Container is loaded, including ability to apply cached ops #5773 floating-promise .catch rule: pending-state loads force opsBeforeReturn: undefined (container.ts:341-345), so the saved-op re-fetch/validate runs as a non-awaited promise guarded by .catch((error) => this.close(...)).
• Anomaly telemetry stays privacy-safe: the mismatch error records only scalar fields plus a contentsDiffer boolean, never raw op contents (preserving the Add protection against data loss #3900 PII boundary). serviceCheckpointSequenceNumber stays inside the DeltaManager: refactor out connectivity manager #8423 ConnectionManager boundary as diagnostic-only — keep it internal, do not expand into public API.
• Suggested reviewers: vladsud (data-loss/overwrite machinery) and anthony-murphy (pending-state/load-mode path) primary; ChumpChief/markfields secondary; tanviraumi/GaryWilber for anchor-mismatch semantics.

For human reviewer

• Needs human judgment — Fire-and-forget vs. synchronous-barrier validation: pending-state loads run the saved-op re-fetch/validate as a non-awaited promise before Container.load returns and accepts new local work. Confirm the optimistic model tolerates building ops on history a background check may later find overwritten, or decide it should be a synchronous load barrier. Owners: vladsud (Implement more controls on how Container is loaded, including ability to apply cached ops #5773), anthony-murphy, jcmoore.
• Needs human judgment — Scope of pending-state integrity validation: whether boundary-only (saved-op anchor) validation suffices, or the full savedOps chain warrants an interior monotonicity audit.
• Needs human judgment — Whether the anchor mismatch should remain routed through the Better handling when loader detects file changes (epoch change detected on client) #5882 fileOverwrittenInStorage semantics vs. a distinct pending-state error class now that the trigger also compares contents. Owner: vladsud.
• Cannot be assessed by the pipeline — Whether any targeted driver/service re-stamps an op timestamp between pending-state serialization and delta-storage refetch (comparableMessagePayload includes timestamp, so a re-stamp could false-positive the overlap check); this also bears on the key-order concern's likelihood.
• Cannot be assessed by the pipeline — Cross-driver determinism of JSON key ordering (ODSP, R11s, and any partner driver) between pending-state serialization and storage refetch — the exact assumption the author-declined resolution rests on.

Review history (6 prior reviews)

• 4031458 2026-09-03 · 5/10 — three contained close/validation-path bugs: key-order close, anchor-bypass false negative, unhandled rejection
• 636d18d 2026-09-03 · 5/10 — contents-comparison fix introduced a key-order false-positive close; never-returned-anchor gap remained
• 53a7ab8 2026-09-03 · 5/10 — deltaConnection:"none" validation skip and contents-excluded anchor gaps flagged inline
• 36dd7cb 2026-09-03 · 6/10 — online-reconnect anchor-validation fetch abortable by a live socket op
• a0e39f5 2026-09-03 · 8/10 — no blocking defect; all four inline threads resolved, Container-level test added
• a16cf2f 2026-09-02 · 8/10 — no blocking defect; two test-coverage gaps flagged inline plus one optional hardening assert