refactor(container-runtime): structured return types for serializeOp and getLocalState by anthony-murphy-agent · Pull Request #26541 · microsoft/FluidFramework

anthony-murphy-agent · 2026-02-25T19:00:25Z

Summary

Change serializeOp() to return { content: string } instead of a raw string
Change PendingStateManager.getLocalState() to return { pending: IPendingLocalState } instead of a bare IPendingLocalState
Update all callers and tests to use the new structured return types

These structured return types make it straightforward to extend these functions with additional metadata in the future without further signature changes. Specifically, the staging mode pending state work in #26475 needs to add stagedHandleCache to both return types — by landing the structural change first, that PR can focus on the substantive feature logic without a large mechanical diff touching many test files.

Test plan

build:esnext passes for @fluidframework/container-runtime and all dependencies
CI validation

🤖 Generated with Claude Code

…Op and getLocalState Change `serializeOp()` to return `{ content: string }` instead of a raw string, and `PendingStateManager.getLocalState()` to return `{ pending: IPendingLocalState }` instead of a bare `IPendingLocalState`. These structured return types make it straightforward to extend these functions with additional metadata in the future without further signature changes. Co-Authored-By: anthony-murphy <anthony.murphy@microsoft.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

anthony-murphy-agent · 2026-02-25T19:03:12Z

/azp run Build - api-markdown-documenter, Build - benchmark-tool, Build - build-common, Build - build-tools, Build - client packages, Build - common-utils, Build - eslint-config-fluid, Build - eslint-plugin-fluid, Build - protocol-definitions, Build site

anthony-murphy-agent · 2026-02-25T19:03:14Z

/azp run Build - test-tools, repo-policy-check, server-gitrest, server-gitssh, server-historian, server-routerlicious

azure-pipelines · 2026-02-25T19:03:19Z

Commenter does not have sufficient privileges for PR 26541 in repo microsoft/FluidFramework

azure-pipelines · 2026-02-25T19:03:21Z

Commenter does not have sufficient privileges for PR 26541 in repo microsoft/FluidFramework

anthony-murphy · 2026-02-25T19:04:07Z

/azp run Build - api-markdown-documenter, Build - benchmark-tool, Build - build-common, Build - build-tools, Build - client packages, Build - common-utils, Build - eslint-config-fluid, Build - eslint-plugin-fluid, Build - protocol-definitions, Build site

anthony-murphy · 2026-02-25T19:04:12Z

/azp run Build - test-tools, repo-policy-check, server-gitrest, server-gitssh, server-historian, server-routerlicious

azure-pipelines · 2026-02-25T19:04:27Z

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines · 2026-02-25T19:04:29Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Pull request overview

Refactors container-runtime op serialization and pending state plumbing to return structured objects instead of raw values, enabling future extension (e.g., adding staging-mode metadata) without another broad mechanical signature change.

Changes:

Updated serializeOp() to return { content: string } and adjusted all call sites/tests to use .content.
Updated PendingStateManager.getLocalState() to return { pending: IPendingLocalState } and adjusted all call sites/tests to use .pending.
Updated runtime pending-state assembly to consume the new getLocalState() shape.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
packages/runtime/container-runtime/src/opLifecycle/opSerialization.ts	Changes `serializeOp()` to return a structured object `{ content }`.
packages/runtime/container-runtime/src/opLifecycle/outbox.ts	Adapts outbound message creation to use `serializeOp(...).content`.
packages/runtime/container-runtime/src/opLifecycle/batchManager.ts	Updates rollback telemetry payload to use `serializeOp(...).content`.
packages/runtime/container-runtime/src/pendingStateManager.ts	Wraps local pending state return value as `{ pending: ... }` and updates stored op content to use `.content`.
packages/runtime/container-runtime/src/containerRuntime.ts	Destructures `{ pending }` from `getLocalState()` when building `IPendingRuntimeState`.
packages/runtime/container-runtime/src/test/opLifecycle/outbox.spec.ts	Updates test helper serialization usage to `.content`.
packages/runtime/container-runtime/src/test/opLifecycle/opSerialization.spec.ts	Updates assertions for the new `{ content }` return shape.
packages/runtime/container-runtime/src/test/pendingStateManager.spec.ts	Updates pending state access paths to `.pending.pendingStates` / `.pending`.
packages/runtime/container-runtime/src/test/containerRuntime.spec.ts	Updates pending state assertions to `.pending.pendingStates`.

Fix line-length formatting issues flagged by biome check. Co-Authored-By: anthony-murphy <anthony.murphy@microsoft.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

anthony-murphy-agent · 2026-02-25T20:35:13Z

After the latest push (formatting fixes for biome), the following Azure Pipelines need to be triggered. Please copy and post these commands:

/azp run Build - client packages
/azp run Build - api-markdown-documenter
/azp run Build - benchmark-tool
/azp run Build - build-common
/azp run Build - build-tools
/azp run Build - common-utils
/azp run Build - eslint-config-fluid
/azp run Build - eslint-plugin-fluid
/azp run Build - protocol-definitions
/azp run Build - test-tools

/azp run repo-policy-check
/azp run server-gitrest
/azp run server-gitssh
/azp run server-historian
/azp run server-routerlicious

anthony-murphy · 2026-02-25T20:36:27Z

/azp run Build - client packages
/azp run Build - api-markdown-documenter
/azp run Build - benchmark-tool
/azp run Build - build-common
/azp run Build - build-tools
/azp run Build - common-utils
/azp run Build - eslint-config-fluid
/azp run Build - eslint-plugin-fluid
/azp run Build - protocol-definitions
/azp run Build - test-tools

azure-pipelines · 2026-02-25T20:36:34Z

No pipelines are associated with this pull request.

anthony-murphy · 2026-02-25T20:36:34Z

/azp run repo-policy-check
/azp run server-gitrest
/azp run server-gitssh
/azp run server-historian
/azp run server-routerlicious

azure-pipelines · 2026-02-25T20:36:40Z

No pipelines are associated with this pull request.

anthony-murphy · 2026-02-25T20:37:12Z

/azp run Build - api-markdown-documenter, Build - benchmark-tool, Build - build-common, Build - build-tools, Build - client packages, Build - common-utils, Build - eslint-config-fluid, Build - eslint-plugin-fluid, Build - protocol-definitions, Build site

azure-pipelines · 2026-02-25T20:37:31Z

Azure Pipelines successfully started running 1 pipeline(s).

anthony-murphy · 2026-02-25T20:39:03Z

/azp run Build - test-tools, repo-policy-check, server-gitrest, server-gitssh, server-historian, server-routerlicious

azure-pipelines · 2026-02-25T20:39:14Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

…te shape Update three proxy mocks in containerRuntime.spec.ts to return `{ pending: { pendingStates } }` instead of the old `IPendingLocalState` shape. Remove unused IPendingLocalState import. Co-Authored-By: anthony-murphy <anthony.murphy@microsoft.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

anthony-murphy-agent · 2026-02-25T20:45:48Z

After the latest push (mock fixes for review feedback), the following Azure Pipelines need to be triggered. Please copy and post these commands:

/azp run Build - client packages
/azp run Build - api-markdown-documenter
/azp run Build - benchmark-tool
/azp run Build - build-common
/azp run Build - build-tools
/azp run Build - common-utils
/azp run Build - eslint-config-fluid
/azp run Build - eslint-plugin-fluid
/azp run Build - protocol-definitions
/azp run Build - test-tools

/azp run repo-policy-check
/azp run server-gitrest
/azp run server-gitssh
/azp run server-historian
/azp run server-routerlicious

anthony-murphy · 2026-02-25T22:31:07Z

/azp run Build - api-markdown-documenter, Build - benchmark-tool, Build - build-common, Build - build-tools, Build - client packages, Build - common-utils, Build - eslint-config-fluid, Build - eslint-plugin-fluid, Build - protocol-definitions, Build site

anthony-murphy · 2026-02-25T22:31:16Z

/azp run Build - test-tools, repo-policy-check, server-gitrest, server-gitssh, server-historian, server-routerlicious

azure-pipelines · 2026-02-25T22:31:24Z

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines · 2026-02-25T22:31:28Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

anthony-murphy · 2026-02-26T01:25:15Z

/azp run Build - api-markdown-documenter, Build - benchmark-tool, Build - build-common, Build - build-tools, Build - client packages, Build - common-utils, Build - eslint-config-fluid, Build - eslint-plugin-fluid, Build - protocol-definitions, Build site

anthony-murphy · 2026-02-26T01:25:20Z

/azp run Build - test-tools, repo-policy-check, server-gitrest, server-gitssh, server-historian, server-routerlicious

azure-pipelines · 2026-02-26T01:25:35Z

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines · 2026-02-26T01:25:36Z

Azure Pipelines successfully started running 1 pipeline(s).

markfields · 2026-02-26T23:29:12Z


-	public getLocalState(snapshotSequenceNumber?: number): IPendingLocalState {
+	public getLocalState(snapshotSequenceNumber?: number): {
+		pending: IPendingLocalState;


Do we really need the extra hierarchy here? Why not just add the next thing to IPendingLocalState?

IPendingLocalState is a serializable object. i want to pass non-serializable state along with it, specifically the list of handles referenced from the pending state for un-attached things

markfields · 2026-02-26T23:32:43Z

 	const outboundMessages = localBatch.messages.map<OutboundBatchMessage>(
 		({ runtimeOp, ...message }) => ({
-			contents: serializeOp(runtimeOp),
+			contents: serializeOp(runtimeOp).content,


Bugs me that we are so inconsistent between content and contents`. That ship has sailed though, you just gotta pick one I guess.

Although, most usages I see in this PR are contents, I think all except for in IPendingMessage. Might be worth asking Claude to evaluate how close we are to converging on contents without making breaking changes.

Have we shipped anything using IPendingMessage as a durable/code format yet?

nothing durable yet AFAIK

It's mostly principled, not random:

contents = the payload field of a protocol message or envelope (wire format)

IDocumentMessage.contents, ISequencedDocumentMessage.contents — the wire protocol origin

IEnvelope.contents, ISignalEnvelope.contents, TypedContainerRuntimeMessage.contents, IChunkedOp.contents

Rule: if it's a wrapper/envelope carrying something, use contents

content = leaf data (blob bytes) or typed payload within a TypedMessage

ISummaryBlob.content, IBlob.content (git), IOdspSummaryBlob.content — raw bytes

TypedMessage.content, ISignalMessageBase.content, INack.content — typed inner payload

Rule: if it's the actual data or a strongly-typed leaf field, use content

i have no idea how true this is

anthony-murphy · 2026-03-15T04:45:04Z

PRISM Review

Readiness: 9/10 — READY

Clean mechanical refactor wrapping serializeOp() and PendingStateManager.getLocalState() return types in structured objects. All call sites and tests are correctly updated — 7 serializeOp() callers access .content, all 8 getLocalState() callers access .pending. No behavioral changes, no durable format changes. No code defects found.

Findings

None. Style and naming preferences (e.g., anonymous vs named return types, content vs contents) were evaluated and excluded — these are internal API decisions already under active discussion in existing threads.

Path to Ready

Resolve the wrapper design discussion with Mark Fields — the open thread on pendingStateManager.ts asks whether { pending: IPendingLocalState } is needed vs extending IPendingLocalState directly. Your justification (separating serializable from non-serializable state for PR feat: staging mode pending state with handle-aware datastore rehydration #26475) is clear; this needs Mark's sign-off or an alternative proposal.
Verify CI is green — Azure Pipelines runs have been triggered; confirm all checks pass.
Get sign-off from dannimad — requested reviewer who owns the getLocalState() snapshot-filtering logic.

Context for Reviewers

This is the first half of a two-part change. PR #26475 will add stagedHandleCache to both return types — the wrapper exists to separate serializable state (IPendingLocalState) from non-serializable state (handle cache for unattached things). The pending local state area has a history of careful multi-step migrations (e.g., #16930 format migration, #24779 staging mode crash fix), though this PR doesn't alter any serialized format.

…and getLocalState (microsoft#26541) ## Summary - Change `serializeOp()` to return `{ content: string }` instead of a raw string - Change `PendingStateManager.getLocalState()` to return `{ pending: IPendingLocalState }` instead of a bare `IPendingLocalState` - Update all callers and tests to use the new structured return types These structured return types make it straightforward to extend these functions with additional metadata in the future without further signature changes. Specifically, the staging mode pending state work in microsoft#26475 needs to add `stagedHandleCache` to both return types — by landing the structural change first, that PR can focus on the substantive feature logic without a large mechanical diff touching many test files. ## Test plan - [x] `build:esnext` passes for `@fluidframework/container-runtime` and all dependencies - [ ] CI validation 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: anthony-murphy-agent <anthony-murphy-agent@users.noreply.github.com> Co-authored-by: anthony-murphy <anthony.murphy@microsoft.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

anthony-murphy requested a review from Copilot February 25, 2026 19:03

Copilot started reviewing on behalf of anthony-murphy February 25, 2026 19:04 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

fix: apply biome formatting to containerRuntime.ts and outbox.spec.ts

b6257d5

Fix line-length formatting issues flagged by biome check. Co-Authored-By: anthony-murphy <anthony.murphy@microsoft.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

anthony-murphy requested a review from Copilot February 25, 2026 20:38

Copilot started reviewing on behalf of anthony-murphy February 25, 2026 20:39 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

Comment thread packages/runtime/container-runtime/src/test/containerRuntime.spec.ts

anthony-murphy requested a review from Copilot February 25, 2026 22:31

Copilot started reviewing on behalf of anthony-murphy February 25, 2026 22:32 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

anthony-murphy requested review from dannimad and markfields February 26, 2026 23:05

markfields reviewed Feb 26, 2026

View reviewed changes

sonalideshpandemsft approved these changes Mar 19, 2026

View reviewed changes

anthony-murphy merged commit 5903c74 into microsoft:main Mar 23, 2026
34 checks passed

anthony-murphy deleted the refactor/structured-return-types branch March 23, 2026 20:59

Conversation

anthony-murphy-agent commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

anthony-murphy-agent commented Feb 25, 2026

Uh oh!

anthony-murphy-agent commented Feb 25, 2026

Uh oh!

azure-pipelines Bot commented Feb 25, 2026

Uh oh!

azure-pipelines Bot commented Feb 25, 2026

Uh oh!

anthony-murphy commented Feb 25, 2026

Uh oh!

anthony-murphy commented Feb 25, 2026

Uh oh!

azure-pipelines Bot commented Feb 25, 2026

Uh oh!

azure-pipelines Bot commented Feb 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

anthony-murphy-agent commented Feb 25, 2026

Uh oh!

anthony-murphy commented Feb 25, 2026

Uh oh!

azure-pipelines Bot commented Feb 25, 2026

Uh oh!

anthony-murphy commented Feb 25, 2026

Uh oh!

azure-pipelines Bot commented Feb 25, 2026

Uh oh!

anthony-murphy commented Feb 25, 2026

Uh oh!

azure-pipelines Bot commented Feb 25, 2026

Uh oh!

anthony-murphy commented Feb 25, 2026

Uh oh!

azure-pipelines Bot commented Feb 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

anthony-murphy-agent commented Feb 25, 2026

Uh oh!

anthony-murphy commented Feb 25, 2026

Uh oh!

anthony-murphy commented Feb 25, 2026

Uh oh!

azure-pipelines Bot commented Feb 25, 2026

Uh oh!

azure-pipelines Bot commented Feb 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

anthony-murphy commented Feb 26, 2026

Uh oh!

anthony-murphy commented Feb 26, 2026

Uh oh!

azure-pipelines Bot commented Feb 26, 2026

Uh oh!

azure-pipelines Bot commented Feb 26, 2026

Uh oh!

markfields Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

anthony-murphy Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

markfields Feb 26, 2026

Choose a reason for hiding this comment

anthony-murphy-agent commented Feb 25, 2026 •

edited

Loading