Add server timestamps to version mark resolution APIs

[Mark Fields (markfields)]

Description

Version mark consumers need the server timestamp associated with a resolved mark, not only its sequence number. This change carries the final op's timestamp through each resolution path:

• Resolved values from sealAndCaptureVersionMark() and resolve() include timestamp.
• onBatchSequenced() supplies the timestamp as its third listener argument when an incoming local batch resolves a pending mark.
• Live and historical resolution paths retain the timestamp alongside the batch sequence number.

API reports, type compatibility baselines, and a legacy API changeset are included.

Breaking Changes

None - the changes are all backwards compatible.

Reviewer Guidance

The review process is outlined in the pull request guidelines.

Please verify that each API receives the timestamp from the exact op represented by its resolved sequence number, particularly the final op of grouped and piecemeal batches.

[github-actions]

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

Based on the diff (446 lines, 10 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]

Pull request overview

This PR extends the container-runtime’s legacy/beta version mark resolution surface area to propagate the server timestamp alongside the resolved sequence number, ensuring consumers can associate a resolved mark with the exact server-ticketed op time.

Changes:

• Add timestamp to resolved results for ResolveResult and VersionMarkCapture, and thread it through live + historical resolution paths.
• Extend IVersionMarkResolver.onBatchSequenced() to provide (batchId, sequenceNumber, timestamp) and carry the timestamp through inbound batch processing.
• Update runtime wiring, API reports, type-compat baselines, tests, and publish a legacy changeset noting the breaking API change.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
packages/runtime/container-runtime/src/versionMarks/versionMarkResolver.ts	Adds timestamp to resolved types, tracks {sequenceNumber, timestamp} per batch, and extends listener + runtime hooks.
packages/runtime/container-runtime/src/versionMarks/inboundBatch.ts	Carries the last op’s timestamp when deriving sequenced batch completion updates.
packages/runtime/container-runtime/src/test/versionMarks/versionMarkResolver.spec.ts	Updates unit tests to validate timestamp propagation across capture/resolve/listener paths.
packages/runtime/container-runtime/src/test/versionMarks/inboundBatch.spec.ts	Updates helper tests to validate timestamp extraction from inbound messages.
packages/runtime/container-runtime/src/test/types/validateContainerRuntimePrevious.generated.ts	Marks expected forward-compat breaks for updated type aliases.
packages/runtime/container-runtime/src/test/containerRuntime.spec.ts	Updates runtime-level test expectations to include resolved timestamp.
packages/runtime/container-runtime/src/containerRuntime.ts	Wires deltaManager.lastMessage?.timestamp into resolver hooks and forwards inbound timestamps into processInboundBatch.
packages/runtime/container-runtime/package.json	Updates type validation “broken” config for the changed public type aliases.
packages/runtime/container-runtime/api-report/container-runtime.legacy.beta.api.md	Updates generated API report to reflect new timestamp-bearing signatures/types.
packages/runtime/container-runtime/api-report/container-runtime.legacy.alpha.api.md	Updates generated API report to reflect new timestamp-bearing signatures/types.
.changeset/lazy-teams-tan.md	Adds a minor legacy changeset describing the timestamp additions and callback signature change.

Suppressed comments (1)

packages/runtime/container-runtime/src/versionMarks/versionMarkResolver.ts:84

• The @returns docs state the resolved path always includes a "last processed op timestamp", but the returned timestamp can be undefined per the VersionMarkCapture type. Document that explicitly so consumers know to handle it.

	 * @returns The pending batch identity and exclusive sequence number lower bound, or the current sequence number
	 * and last processed op timestamp when there is no pending local batch.

---

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

[github-actions]

Fleet Review — In progress

Running reviewers: correctness, security, api-compatibility

View run

[Tony Murphy (anthony-murphy)]

Deep Review is feeling its way through the dark.

• ✅ Mechanical Review
• ✅ Historical / Contextual Review
• ✅ Solution / Problem Space Review
• 🪸 Adversarial Filter — identifying the colony…
• ⬜ Synthesis
• ⬜ Comments

[Tony Murphy (anthony-murphy)]

Deep Review: Update on the 0xd34 assert concern from the prior review (commit 8f8438a). On e584c59 the container hard-fault reading is resolved — the (sequenceNumber, timestamp) pair is immutable per sequenced op (both read from the same final ISequencedDocumentMessage), so a tolerated duplicate carries the same timestamp and cannot convert into a hard-fault. What remains is the stale assert message (it names only sequence-number remapping though the dedup key is now the (batchId, sequenceNumber, timestamp) triple) plus one open question for a DuplicateBatchDetector owner. Both are now tracked inline on the assert line.

[Tony Murphy (anthony-murphy)]

Deep Review: This JSDoc now describes sequenceNumberLowerBound as exclusive — line 85 (@returns ...exclusive sequence number lower bound...) and line 96 (@param sequenceNumberLowerBound - The exclusive lower bound...) — but line 92 in the same block still says inclusive, and the implementation is inclusive. sealAndCaptureVersionMark returns sequenceNumberLowerBound: referenceSequenceNumber + 1 with the comment "Store that as an inclusive lower bound so resolve() scans directly from it," and resolveFromHistory fetches from sequenceNumberLowerBound directly (commented "(inclusive)"). DEV.md and the container-runtime test both reaffirm an inclusive lower bound, and the PR's own edited test moves resolve("targetBatch", 11) → resolve("targetBatch", 10) and expects fetchMessages to start at exactly 10 — an exclusive bound would skip the boundary op.

Revert the two "exclusive" edits (lines 85 and 96) back to "inclusive" so the public @beta/@legacy JSDoc matches the implementation, line 92, DEV.md, and the edited test.

[Tony Murphy (anthony-murphy)]

Deep Review: The sealAndCaptureVersionMark JSDoc says the timestamp is undefined when "no op may have been processed yet (e.g. on a freshly loaded container)." But the PR's own test "captures the summary timestamp after load before any new ops arrive" (containerRuntime.spec.ts:2894-2938) shows it retrieves the timestamp (123456) from summary metadata on a freshly loaded container, because getCurrentReferenceTimestampMs() falls back to messageAtLastSummary?.timestamp (containerRuntime.ts:4477-4480). It is undefined only when neither a last message nor a last-summary message exists.

Separately, DEV.md (line 681, also lines 45/58) documents the hook as getCurrentTimestamp -> deltaManager.lastMessage?.timestamp, omitting the ?? messageAtLastSummary?.timestamp fallback the code actually wires.

Correct the JSDoc to reflect that a freshly loaded container does obtain a timestamp via summary metadata, and update DEV.md to state the hook is getCurrentReferenceTimestampMs() (lastMessage?.timestamp ?? messageAtLastSummary?.timestamp). While in DEV.md, add a one-sentence rationale that reusing getCurrentReferenceTimestampMs() keeps the seq/timestamp pairing aligned at load — on load messageAtLastSummary.sequenceNumber === initialSequenceNumber, so with no new ops the fallback timestamp and the reported sequence number come from the same op.

[Tony Murphy (anthony-murphy)]

packages/runtime/container-runtime/src/versionMarks/versionMarkResolver.ts:1025

Deep Review: Following up on the 0xd34 concern from the prior review. The early-return dedup is now existingResolution?.sequenceNumber === sequenceNumber && existingResolution.timestamp === timestamp, and the following assert(existingResolution === undefined, 0xd34 ...) fires when that early-return is skipped — so a (batchId, sequenceNumber) re-observed with a differing timestamp now reaches the assert, whose message still names only sequence-number remapping.

The hard-fault reading is resolved: the (sequenceNumber, timestamp) pair is immutable per sequenced op (both read from the same final ISequencedDocumentMessage — inboundBatch.ts:47-57, 64-74), so a legitimate re-delivery carries the same timestamp and the timestamp branch can't trip. What remains is stale text: update the 0xd34 assert message to name the timestamp-mismatch case now that the dedup key is the (batchId, sequenceNumber, timestamp) triple.

Open question for a DuplicateBatchDetector owner: if any redelivery path (resubmit, replay, or a batch surfaced by both the live and history paths) could present an identical (batchId, sequenceNumber) with a differing timestamp, narrow the dedup guard back to sequenceNumber alone rather than asserting; if not, the message update is sufficient.

[github-actions]

🔗 Found some broken links! 💔

Run a link check locally to find them. See Checking for Broken Links for more information.

linkcheck output

$ start-server-and-test "npm run serve -- --host 127.0.0.1 --no-open" http://127.0.0.1:3000 check-links
1: starting server using command "npm run serve -- --host 127.0.0.1 --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-website@0.0.0 serve
> docusaurus serve --host 127.0.0.1 --no-open

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

> fluid-framework-website@0.0.0 check-links
> linkcheck http://127.0.0.1:3000 --skip-file skipped-urls.txt

Crawling...

http://127.0.0.1:3000/docs/data-structures/tree/schema-evolution/feature-flag-schema-upgrades
- (72:12) 'isStaged..' => http://127.0.0.1:3000/docs/api/fluid-framework/treeviewalpha-interface#isstagedupgradeenabled-methodsignature (HTTP 200 but missing anchor)


Stats:
  338443 links
    2041 destination URLs
    2297 URLs ignored
       1 warnings
       0 errors

Error: Command failed with exit code 1: npm run check-links
    at makeError (/home/runner/work/FluidFramework/FluidFramework/website/node_modules/.pnpm/execa@5.1.1/node_modules/execa/lib/error.js:60:11)
    at handlePromise (/home/runner/work/FluidFramework/FluidFramework/website/node_modules/.pnpm/execa@5.1.1/node_modules/execa/index.js:118:26)
    at process.processTicksAndRejections (node:internal/process/task_queues:103:5) {
  shortMessage: 'Command failed with exit code 1: npm run check-links',
  command: 'npm run check-links',
  escapedCommand: '"npm run check-links"',
  exitCode: 1,
  signal: undefined,
  signalDescription: undefined,
  stdout: undefined,
  stderr: undefined,
  failed: true,
  timedOut: false,
  isCanceled: false,
  killed: false
}
[ELIFECYCLE] Command failed with exit code 1.

[github-actions]

Bundle size comparison

Base commit: unresolved
Head commit: ff6808da36e5ad6029944597e28136670a1d1cd9

⚠️ Comparison unavailable.

The PR's CI build failed — fix the build and the comment will update once the next run succeeds.

[Tony Murphy (anthony-murphy)]

Deep Review

Reviewed commit ff6808d on 2026-08-24.

Readiness: 8/10 — ALMOST READY

Not signed off, but close. The resolved op's server timestamp is threaded additively through the live/session, inbound, and historical resolution paths, and the @legacy @beta surface stays optional and source-compatible. Both prior-run inline doc threads are now fixed in code (the exclusive/inclusive JSDoc contradiction reverted to "inclusive"; the freshly-loaded-container timestamp docstring corrected; plus a real referenceSequenceNumber return-value bug fixed). No correctness blockers remain — what's left is a cluster of four contained polish/hardening items: a changeset disclosure gap, a stale assert message, an optional-listener-parameter decision, and a capture-pairing question. Fixing them moves this to READY.

Path to Ready

• Resolve inline threads
• Add a sentence to .changeset/lazy-teams-tan.md and the PR description disclosing the internal assert→tolerate behavior change — the resolver no longer faults on a re-broadcasted no-explicit-batch single-op batch that remaps to a new sequence number; it keeps the first-landed resolution.
• Update the stale 0xd34 assert message to name the timestamp-mismatch case now that the dedup key is the (batchId, sequenceNumber, timestamp) triple — or restore a non-fatal telemetry/debug-only assert when an incoming resolution disagrees with the stored sequenceNumber (detail in this note).

Context for Reviewers

• versionMarks is a brand-new, single-author area that deliberately mirrors markfields' offline duplicate-batch patterns (Offline: Only enable DuplicateBatchDetector if Offline Load is enabled #22497 DuplicateBatchDetector gating, ContainerRuntime: Process incoming batches op-by-op instead of waiting for the whole batch #22508 op-by-op inbound-batch model). Evaluate it against those precedents rather than an accumulated review culture of its own.
• This extends the @legacy @beta resolved shape ~6 days after beta promotion (feat(container-runtime): add internal version mark resolver #27834 introduced the resolver, Move marker and load to sequence number API to legacy alpha #27891 → @legacy @alpha, Promote point-in-time APIs to legacy beta #27976 → @legacy @beta). The new timestamp being optional is the source-compatible, non-breaking choice.
• The new timestamp inherits the "first sequenced batch wins" duplicate-batch behavior: for a re-broadcast batch the first observed op's timestamp is kept, matching the sequence-number choice (versionMarkResolver.ts:352-365) — the one deliberate case where a stored timestamp is not re-derived on a later duplicate.

For human reviewer

• Domain-expert judgment on the widened dedup path — an owner of DuplicateBatchDetector should confirm whether any redelivery path (resubmit, replay, or a batch surfaced by both the live and history paths) can present an identical (batchId, sequenceNumber) with a differing timestamp, or a same-batchId→different-sequenceNumber corruption that slips past the upstream detector. This drives whether the 0xd34 item is a message fix or a guard change.
• Beta contract owners — lindsnguyen (original resolver author) and sonalideshpandemsft (beta-promotion owner) should confirm the optional-timestamp compatibility story matches external Bohemia/Loop consumption expectations, given this extends the resolved shape ~6 days after beta promotion.
• Cross-module load-time invariant (cannot be assessed by the pipeline) — the capture-path pairing depends on messageAtLastSummary corresponding to deltaManager.initialSequenceNumber at load across module boundaries; confirm this holds on all load/recovery paths, not just the tested fresh-load case.

Review history (2 prior reviews)

• e584c59 2026-08-24 · 7/10 — 0xd34 hard-fault reading resolved; four contained doc/polish items remain
• 8f8438a 2026-08-21 · 5/10 — 0xd34 assert flagged as a gating hard-fault concern requiring domain-expert sign-off

[Tony Murphy (anthony-murphy)]

Deep Review: The @legacy @beta listener signature types timestamp as optional — listener: (batchId: string, sequenceNumber: number, timestamp?: number) => void — but the sole runtime caller always supplies a concrete value: processInboundBatch(batchId, sequenceNumber, timestamp: number) takes a required timestamp and invokes listener(batchId, sequenceNumber, timestamp). The doc comment itself describes the listener firing with "the final op's server timestamp," implying no optionality.

Since the only producer never omits the argument, tightening the parameter to timestamp: number removes defensive dead code with no runtime change. If the optionality is intentional (source-compatibility with existing two-arg listeners), keep it and note that rationale at the declaration so the width is a documented decision rather than an apparent oversight on a release-blocking public surface.

Suggested change

	): () => void;
	onBatchSequenced(
	listener: (batchId: string, sequenceNumber: number, timestamp: number) => void,
	): () => void;

[Tony Murphy (anthony-murphy)]

Deep Review: The two capture hooks read different DeltaManager members — getCurrentSequenceNumber: () => this.deltaManager.lastSequenceNumber (line 1945) and getCurrentTimestamp: () => this.getCurrentReferenceTimestampMs() (line 1946), where getCurrentReferenceTimestampMs() returns this.deltaManager.lastMessage?.timestamp ?? this.messageAtLastSummary?.timestamp. sealAndCaptureVersionMark() pairs these two independently-read values into one { sequenceNumber, timestamp } capture.

#22508's stated motivation was that lastSequenceNumber can advance ahead of the last surfaced message, so the pairing is a caller discipline rather than a structural guarantee. Load-time consistency is verified (messageAtLastSummary?.sequenceNumber vs initialSequenceNumber) and the added test covers the load/summary fallback, but the mid-batch skew case is not covered, and the PR's own Reviewer Guidance asks to confirm each API receives the timestamp from the exact op represented by its resolved sequence number.

Confirm no synchronous window exists where deltaManager.lastSequenceNumber refers to an op whose timestamp is not lastMessage.timestamp at capture time (validate across fullBatch vs piecemeal inbound shapes). If the invariant holds, optionally harden by deriving both values from a single combined hook returning { sequenceNumber, timestamp }, making the pairing atomic by construction rather than a documented caller invariant.