Skip to content

fix(core): convert worker results like the other adapters - #100

Merged
frontegg-david merged 1 commit into
ci/browser-e2e-playwrightfrom
fix/core-worker-result-conversion
Oct 4, 2026
Merged

frontegg-david merged 1 commit into
ci/browser-e2e-playwrightfrom
fix/core-worker-result-conversion

Conversation

@frontegg-david

@frontegg-david frontegg-david commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Stack 8/10: main ← #92 ← #93 ← #94 ← #95 ← #96 ← #98 ← #99 ← this ← #101 ← #102

Follow-up to #94 and #98. The double VM and single VM now return a script's Date as a Date; the worker_threads adapter had its own result path and didn't.

Root cause

worker-script.ts copied the result with sanitizeObject (safe-deserialize.ts) and sent it as a JSON string. The protocol is JSON-only on purpose (no structured clone). On worker_threads, before this change:

Script returns double / single VM worker before worker after
{ date: new Date(0) } Date {} Date
{ big: 10n } '10' run fails: Do not know how to serialize a BigInt '10'
{ nan: NaN, inf: Infinity, undef: undefined } as is { nan: null, inf: null } as is
circular a.self = a '[Circular]' copied 50 levels deep, then undefined '[Circular]'
more than maxSanitizeProperties / deeper than maxSanitizeDepth fails succeeds fails

Fix

  • Worker: runs core's sanitizeValue on the result with the configured maxSanitizeDepth / maxSanitizeProperties, so the conversion is the same as on the in-process adapters.

  • Transport stays JSON. New result-encoding.ts: encodeResult replaces each value JSON can't carry (Date, NaN, ±Infinity, undefined) with null and lists it in specialValues as { path, kind, time? }. decodeResult on the main thread puts them back.

  • The decoder treats the message as untrusted:

    • specialValues must be an array of known kinds with string or non-negative integer path segments;
    • each path walks only own properties of arrays and null-prototype objects (the only containers safeDeserialize produces), never __proto__/constructor/prototype;
    • each path must end on a null placeholder.

    Anything else throws MessageValidationError, which fails the run.

  • Keys the transport strips (prototype, ...) are left out of the encoding, so their paths never dangle.

  • ExecutionResultMessage and its zod schema gain the optional specialValues field.

Behavior change

Worker results are now bounded by maxSanitizeDepth / maxSanitizeProperties (STANDARD: 20 / 500), like the other adapters. A larger result now fails the run. Before, only the transport's 50-level depth cap applied.

Docs: worker-pool.mdx explains how results are converted. Its basic example now sets doubleVm: { enabled: false }: without it, adapter: 'worker_threads' has no effect (see #96).

Red → green

Suite Before After
enclave.result-sanitization.spec.ts, worker_threads cases (new) 2 failed ({} for the Date; Do not know how to serialize a BigInt) pass
worker-result-encoding.spec.ts (new, 14 tests: round trip through the real transport, root values, invalid Date, stripped keys, 8 malformed lists refused) n/a (new module) pass

Verification

  • core: jest -c libs/core/jest.config.ts --testPathIgnorePatterns /perf/: 67 suites, 2376 passed, 1 skipped (core:build-worker-script rebuilt first; the worker tests load it from dist)
  • core perf: 48 passed
  • nx lint core, nx build core, prettier: pass

🤖 Generated with Claude Code

The worker_threads adapter copied the script result with its own
sanitizeObject and sent it as JSON: a Date came back as {}, a BigInt
failed the run ("Do not know how to serialize a BigInt"), NaN and
Infinity became null, and maxSanitizeDepth and maxSanitizeProperties
did not apply.

The worker now runs core's sanitizeValue on the result, with the
configured limits, so it converts exactly as the double and single VM
do ('[Circular]', BigInt to string, Map to object, Set to array, limits
and functions fail the run). The transport stays JSON: the values JSON
cannot carry (Date, NaN, +/-Infinity, undefined) are sent next to the
result as [path, kind] entries and restored by the pool. The decoder
refuses an entry that does not point at a null placeholder inside the
result's arrays and null-prototype objects, so a malformed message
fails the run instead of writing elsewhere.

The worker pool docs example now disables the double VM, without
which adapter: 'worker_threads' has no effect.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

Next included review available in 8 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 3 included reviews currently available. Your 43 included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: agentfront/enclave/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 72a63e74-f6b9-4f05-9032-07c7a82b52cb
📥 Commits

Reviewing files that changed from the base of the PR and between 495e5af and ce8371b.

📒 Files selected for processing (7)
  • docs/enclave/core-libraries/enclave-vm/worker-pool.mdx
  • libs/core/src/__tests__/enclave.result-sanitization.spec.ts
  • libs/core/src/__tests__/worker-result-encoding.spec.ts
  • libs/core/src/adapters/worker-pool/protocol.ts
  • libs/core/src/adapters/worker-pool/result-encoding.ts
  • libs/core/src/adapters/worker-pool/worker-pool-adapter.ts
  • libs/core/src/adapters/worker-pool/worker-script.ts
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@frontegg-david
frontegg-david merged commit 1c7054e into main Oct 4, 2026
9 checks passed
@frontegg-david
frontegg-david deleted the fix/core-worker-result-conversion branch October 4, 2026 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants