Skip to content

fix: excessive cloning in runSubscribersWithMutation #899

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
daohoangson wants to merge 2 commits into
base: main
Choose a base branch
from
+9 −11
Open

fix: excessive cloning in runSubscribersWithMutation #899

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf6dd97
refactor(client): Optimize structuredClone usages in runSubscribersWi…
daohoangson Dec 30, 2025
c381b62
refactor(client): Optimize stringify usages in runSubscribersWithMuta…
daohoangson Dec 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 9 additions & 11 deletions sdks/typescript/packages/client/src/agent/subscriber.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,18 +178,16 @@ export async function runSubscribersWithMutation(
state: State,
) => MaybePromise<AgentStateMutation | void>,
): Promise<AgentStateMutation> {
let messages: Message[] = initialMessages;
let state: State = initialState;
const messages0 = structuredClone_(initialMessages);
const state0 = structuredClone_(initialState);
let messages: Message[] = messages0;
let state: State = state0;

let stopPropagation: boolean | undefined = undefined;

for (const subscriber of subscribers) {
try {
const mutation = await executor(
subscriber,
structuredClone_(messages),
structuredClone_(state),
);
const mutation = await executor(subscriber, messages, state);
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This optimization removes the defensive cloning that protects against subscribers mutating the input parameters directly. While subscribers should only mutate via return values, passing the same reference to multiple subscribers means that if a subscriber mutates the messages or state parameters in-place (even accidentally), those mutations will leak to subsequent subscribers.

The old approach cloned the inputs for each subscriber, providing isolation. Consider whether this risk is acceptable, or if there should be documentation/runtime checks to enforce that subscribers don't mutate inputs directly.

Suggested change
const mutation = await executor(subscriber, messages, state);
const messagesForSubscriber = structuredClone_(messages);
const stateForSubscriber = structuredClone_(state);
const mutation = await executor(
subscriber,
messagesForSubscriber,
stateForSubscriber,
);

Copilot uses AI. Check for mistakes.
Copy link
Author

@daohoangson daohoangson Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentional.


if (mutation === undefined) {
// Nothing returned – keep going
Expand All @@ -198,11 +196,11 @@ export async function runSubscribersWithMutation(

// Merge messages/state so next subscriber sees latest view
if (mutation.messages !== undefined) {
messages = mutation.messages;
messages = structuredClone_(mutation.messages);
}

if (mutation.state !== undefined) {
state = mutation.state;
state = structuredClone_(mutation.state);
}

stopPropagation = mutation.stopPropagation;
Expand All @@ -224,8 +222,8 @@ export async function runSubscribersWithMutation(
}

return {
...(JSON.stringify(messages) !== JSON.stringify(initialMessages) ? { messages } : {}),
...(JSON.stringify(state) !== JSON.stringify(initialState) ? { state } : {}),
...(messages !== messages0 ? { messages } : {}),
...(state !== state0 ? { state } : {}),
...(stopPropagation !== undefined ? { stopPropagation } : {}),
};
}