Skip to content

Fix stableStringify treating shared references as circular#327398

Open
dsavy4 wants to merge 3 commits into
microsoft:mainfrom
dsavy4:fix/stablestringify-shared-refs
Open

Fix stableStringify treating shared references as circular#327398
dsavy4 wants to merge 3 commits into
microsoft:mainfrom
dsavy4:fix/stablestringify-shared-refs

Conversation

@dsavy4

@dsavy4 dsavy4 commented Jul 24, 2026

Copy link
Copy Markdown

_stableStringify guards against circular references with a WeakSet, but it never removes a value from the set after serializing its subtree. So a value that is shared across sibling branches, which is a DAG and not a cycle, gets reported as "[Circular]" on every occurrence after the first.

Repro:

const shared = { a: 1 };
stableStringify({ x: shared, y: shared });
// before: {"x":{"a":1},"y":"[Circular]"}
// after:  {"x":{"a":1},"y":{"a":1}}

This matters because stableStringify is used to compare objects for change detection (for example in the chat voice client, stableStringify(current[key]) !== stableStringify(prev[key])), where a shared reference can make two structurally equal objects compare as different.

The fix tracks only the current ancestor path by removing the value from the set once its subtree is done, so true cycles are still detected while shared references serialize in full. Added tests for shared references in both object and array positions, and the existing circular-reference test still passes.

_stableStringify uses a WeakSet to guard against circular references, but
it never removed a value after serializing its subtree. As a result a
value that is shared across sibling branches (a DAG, not a cycle) was
reported as "[Circular]" on every occurrence after the first.

For example stableStringify({ x: shared, y: shared }) returned
{"x":{"a":1},"y":"[Circular]"} instead of
{"x":{"a":1},"y":{"a":1}}.

Track only the current ancestor path by removing the value from the set
once its subtree is done. True cycles are still detected. Added tests for
shared references in both object and array positions.
Copilot AI review requested due to automatic review settings July 24, 2026 21:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes stableStringify to distinguish shared references from circular references.

Changes:

  • Tracks only the active ancestor path during serialization.
  • Adds object and array shared-reference regression tests.

Reviewed changes

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

File Description
src/vs/base/common/objects.ts Removes serialized values from cycle tracking.
src/vs/base/test/common/objects.test.ts Tests repeated shared references.

Comment thread src/vs/base/common/objects.ts Outdated
Comment thread src/vs/base/test/common/objects.test.ts
@dsavy4

dsavy4 commented Jul 24, 2026

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants