Repository navigation
Conversation
|
Hi! I'm the It looks like you correctly set up a CI job that uses the autofix.ci GitHub Action, but the autofix.ci GitHub App has not been installed for this repository. This means that autofix.ci unfortunately does not have the permissions to fix this pull request. If you are the repository owner, please install the app and then restart the CI workflow! 😃 |
autofix.ci
Co-authored-by: Jason (Json) <263060202+fuller-stack-dev@users.noreply.github.com>
Co-authored-by: Jason (Json) <263060202+fuller-stack-dev@users.noreply.github.com>
Co-authored-by: Jason (Json) <263060202+fuller-stack-dev@users.noreply.github.com>
Co-authored-by: Jason (Json) <263060202+fuller-stack-dev@users.noreply.github.com>
Retain exact v14 bytes for cause-free values and upgrade to v15 only when a cause edge is serialized. Advertise serializer support in the existing IPC hello and omit causes until the peer advertises support, preserving mixed-version process communication. Keep legacy Buffer ordering for cause-free graphs, collect cause Buffers from the native walk, and cover Node parity, graph identity, legacy byte fixtures, workers, and negotiated IPC. Co-authored-by: Jason (Json) <263060202+fuller-stack-dev@users.noreply.github.com>
ee601d0 to
7a50bb3
Compare
steipete
changed the title
Retain both Unreleased entries while refreshing the runtime fix against the current fork. Runtime changes merge automatically; release tooling remains in its separate draft PR. Co-authored-by: Jason (Json) <263060202+fuller-stack-dev@users.noreply.github.com>
Pass the explicit non-Node arguments to the nested CryptoKey serializer and use the engine's uncheckedDowncast/asArray helpers. Apply the v8 regression test import ordering requested by CI. Co-authored-by: Jason (Json) <263060202+fuller-stack-dev@users.noreply.github.com>
Keep the five-byte and protocol-version checks exact while expecting the Error.cause capability advertised by the compatible serializer handshake. Co-authored-by: Jason (Json) <263060202+fuller-stack-dev@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb391eaf21
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| writeIndex(reference.poolSize + 1, reference.index + 1); | ||
| cursor = reference.offset + indexWidth(reference.poolSize); | ||
| } | ||
| write(valueBytes.span().data() + cursor, valueBytes.size() - cursor); |
There was a problem hiding this comment.
For node:v8.serialize() and advanced IPC messages containing a Buffer, finishBufferEnvelope() retains the complete first serialization in valueBytes and then rewrites all of those bytes into a fresh m_buffer (the reference loop plus this final write). The previous [value, buffers] path serialized the envelope in one pass, so large Buffer payloads now incur another O(payload) copy and roughly double peak serialized storage; near memory limits, messages that previously succeeded can fail, while hot IPC pays the extra pass on every send. Build the outer envelope during the original walk or reuse the original allocation instead of reconstructing it.
Useful? React with 👍 / 👎.
…ral design (#196) This reverts b40c6ae (#194), then cb98044 (#192), using git revert. The pooled median Worker session-message ratio across three machines was 1.078019079 against the predeclared maximum 1.05. Restore the S3 v14 wire format and Error cause omission pending a performance-neutral design. Keep #195's pending-container-terminator reservation unchanged. The source tree differs from S3 only by that serializer patch and the changelog.
Problem and root cause
structuredClone(new Error("outer", { cause }))discardedcause, including nested errors, shared references, cycles, and shared memory hidden behind that edge. Error records were terminal metadata records in JavaScriptCore's clone codec, so the graph walker never visited the cause.This change records each Error in the existing object pool before traversing its own data-descriptor cause. The cause is captured and rooted before error metadata can run user code. Existing graph traversal, back-references, transfer handling, and the separate bounded worker-error metadata path retain ownership of their respective behavior.
Compatibility contract
node:v8serialization retains v14 bytes without causes and emits v15 for cause-bearing graphs. Older Bun builds cannot read those v15 values. Bun and Node already use different wire formats; cross-engine byte interchange remains unsupported.The existing Error tag (
0x37) is unchanged. Version 15 adds a one-byte cause-presence marker and optional normal Value; v14 omits both.The Buffer pre-scan is retained for byte compatibility. Native collection adds Buffer identities needed by cause-bearing graphs without changing legacy cause-free getter behavior.
Scope and prior art
The unrelated release tooling has been split out unchanged into draft #193. It has not been reviewed or approved as part of this runtime fix. Original contributor commits and human credit are preserved.
Prior art: oven-sh#31525. That discussion requested portable
node:testcoverage, distinguished behavioral parity from byte interchange, and highlighted graph pooling, GC lifetime, and one-way format compatibility. This change adds portable Node coverage and explicitly handles mixed-version process compatibility.Qualification
Reviewed head:
fb391eaf21de1df17ed96bc8620b44ebeb864875. The native CI run builds merge commit0495b1748fcdc69d13beaa26a93c700d1037609d, whose tree is identical to that head.The OpenClaw runs used a scratch checkout,
OPENCLAW_BUN_PATHpointing to the verified CI binary, and the launcher's supported local override mode. The released pin was unchanged. Repository suites used pinned Node 26.3.0; the explicit Node comparison used 26.12.0. No test assertions, timeouts, heap limits or worker counts were relaxed.