Skip to content

upgrade-webkit: check upstream structured clone changes against Bun's copy - #46773

Open
robobun wants to merge 1 commit into
mainfrom
robobun/973fa8ba/upgrade-webkit-structured-clone-check
Open

robobun wants to merge 1 commit into
mainfrom
robobun/973fa8ba/upgrade-webkit-structured-clone-check

Conversation

@robobun

@robobun robobun commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Related to #45739

Problem

  • A WebKit upgrade does not update Bun's structured clone serializer. src/jsc/bindings/webcore/SerializedScriptValue.cpp is Bun's own copy, with its own CloneBase, CloneSerializer and CloneDeserializer classes.
  • The upgrade checklist in .claude/commands/upgrade-webkit.md does not mention this copy. An upstream change to the clone code stays unported until a user reports the difference.
  • Example: upstream corrected the address type of a cloned shared Memory64 WebAssembly.Memory on 2026-08-08 (WebKit/WebKit@3e351c7849f3). Bun's copy still hard-codes Wasm::AddressType::I32 (SerializedScriptValue.cpp:3779), and Shared WebAssembly.Memory with address: 'i64' (Memory64) is received as i32 memory in a worker_threads Worker #45739 reported it on 2026-10-17.

Fix

  • Add one item to "Things to check for a successful upgrade". The item names the upstream files that hold the clone code. It tells the upgrader to port each behavior change into Bun's copy by hand.
  • The item follows the JSType.h item. Both items cover a Bun file that mirrors upstream source by hand.
  • Verified: prettier --check passes on the file. The PR has no code change, so it has no test.

Background

  • Structured clone is the serializer behind structuredClone() and postMessage().
  • Upstream keeps the shared part in JavaScriptCore headers: CloneBase.h, CloneSerializerBase.h, CloneDeserializerBase.h and StructuredCloneTags.h. WebCore's SerializedScriptValue.cpp derives its serializer from them.
  • Bun receives those headers with each WebKit upgrade but does not include them. The pinned CloneSerializerBase.h:589 and CloneDeserializerBase.h:719-725 already handle Memory64.
  • build(android): pin NDK r30 together with the prebuilt WebKit #46623 (draft) appends a different item to the end of the same list. A test merge of the two branches is clean.

Behaviour change: none

Notes

no test proof · iteration 0 · docs-only change; test-proof not applicable

… copy

Bun's structured clone serializer in
src/jsc/bindings/webcore/SerializedScriptValue.cpp is its own copy. It
does not derive from the JSC clone base classes, so a WebKit upgrade
does not update it. Add a checklist item that names the upstream files
to read and tells the upgrader to port each behavior change by hand.
@robobun

robobun commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Status

This is a docs-only change: one new item in the checklist of .claude/commands/upgrade-webkit.md.

Checked on main (179664e) and on the pinned WebKit (oven-sh/WebKit@680ef47cec2c):

  • Bun's src/jsc/bindings/webcore/SerializedScriptValue.cpp declares its own CloneBase, CloneSerializer and CloneDeserializer. No file under src/ includes a JSC clone header.
  • Each upstream file that the item names exists at the pinned commit.
  • The pinned CloneSerializerBase.h:589 writes the Memory64 address type. Bun's copy does not (SerializedScriptValue.cpp:3779).

No open PR makes the same edit. #46623 (draft) appends a different item to the same list, and a test merge with this branch is clean.

PR: #46773

@robobun

robobun commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 1:08 PM PT - Oct 24th, 2026

✅ @robobun, your commit 84c0a775bbf694a3b8b50c1d977fe173cc176f92 passed in Build #132134! 🎉


🧪   To try this PR locally:

bunx bun-pr 46773

That installs a local version of the PR into your bun-46773 executable, so you can run:

bun-46773 --bun

@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 179664e and 84c0a77.

Files selected for processing: 1
  • .claude/commands/upgrade-webkit.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

Walkthrough

The WebKit upgrade checklist now includes a structured-clone check. It directs maintainers to port behavior changes manually to Bun’s SerializedScriptValue.cpp.

Changes

WebKit upgrade checklist

Layer / File(s) Summary
Structured-clone upgrade check
.claude/commands/upgrade-webkit.md
The checklist identifies JavaScriptCore and WebCore structured-clone files to check. It directs maintainers to port behavior changes to Bun’s SerializedScriptValue.cpp, which does not derive from the JSC clone base classes.

Priority: Low

Merge Risk: Minimal · up to 84c0a

This documentation-only change is mergeable.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly identifies the checklist change and its purpose: checking upstream structured-clone changes against Bun's copy.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The pull request description is complete. It explains the problem, the fix, the scope, verification, and why no test is required for this documentation-only change. It does not use the exact template …
Full details: Description check

Explanation

The pull request description is complete. It explains the problem, the fix, the scope, verification, and why no test is required for this documentation-only change. It does not use the exact template headings, but it provides the required information in equivalent sections.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude claude Bot 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.

LGTM — a one-line addition to the WebKit upgrade checklist, no code change.

What was reviewed:

  • Confirmed the in-tree claim: src/jsc/bindings/webcore/SerializedScriptValue.cpp defines its own CloneBase (line 621) with CloneSerializer/CloneDeserializer deriving from it (lines 714, 2252), and nothing under src/ includes a JSC clone header, so a merge does not update it.
  • The upstream file list could not be confirmed from this checkout (vendor/WebKit is not populated); a wrong name there would only make the checklist item harder to follow, not affect the build.
  • Placement next to the JSType.h item matches the existing pattern of hand-mirrored files.
Extended reasoning...

The diff adds one bullet to the "Things to check for a successful upgrade" list in .claude/commands/upgrade-webkit.md, a Claude slash-command prompt; no source, build, test, or CI files change. It touches no security-sensitive surface. The checkable factual claim (Bun's SerializedScriptValue.cpp has its own CloneBase hierarchy and does not include JSC's clone base headers) was verified against the current tree. The path is not covered by CODEOWNERS and the change is small and self-contained, which decided approve.

@robobun

robobun commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

The review could not confirm the upstream file names, because vendor/WebKit is not populated in its checkout. Each file exists in upstream WebKit at f59345e04a8d. That is the latest upstream commit in the pinned WEBKIT_VERSION (oven-sh/WebKit@680ef47cec2c):

The pinned fork commit carries the same six files with the same blob hashes. Upstream's serializer and deserializer derive from the JSC base classes at SerializedScriptValue.cpp:487 and SerializedScriptValue.cpp:1790.

This branch has not been deployed

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant