Skip to content

fix(ai-isolate-cloudflare): port worker from unsafe_eval to worker_loader#523

Open
Sriketk wants to merge 6 commits intoTanStack:mainfrom
Sriketk:fix/ai-isolate-cloudflare-worker-loader
Open

fix(ai-isolate-cloudflare): port worker from unsafe_eval to worker_loader#523
Sriketk wants to merge 6 commits intoTanStack:mainfrom
Sriketk:fix/ai-isolate-cloudflare-worker-loader

Conversation

@Sriketk
Copy link
Copy Markdown

@Sriketk Sriketk commented May 3, 2026

🎯 Changes

Port @tanstack/ai-isolate-cloudflare worker from the unsafe_eval binding to worker_loader (Dynamic Workers).

Closes #522.

Why

unsafe_eval is workerd-internal and gated by Cloudflare for all customer accounts — there is no public entitlement and no path to enable it. The current driver is unusable in production and broken in wrangler dev on Wrangler 4.x:

Attempt Result
wrangler deploy binding UNSAFE_EVAL has an unknown type eval [code: 10021]
wrangler dev binding mode remote, runtime returns UnsafeEvalNotAvailable
wrangler dev --local (3.114) Fatal: setsocketopt TCP_NODELAY crash

Cloudflare's supported replacement is the worker_loader (Dynamic Workers) binding, GA-beta'd 2026-03-24.

What changed

  • src/worker/index.ts: swap env.UNSAFE_EVAL.eval(wrappedCode) for env.LOADER.load({modules}).getEntrypoint().fetch(...). Wraps the existing IIFE-returning string in an ES module that exposes a fetch handler returning the structured result as JSON.
  • wrangler.toml: [[unsafe.bindings]][[worker_loaders]] binding = "LOADER". Compat date bumped to 2026-05-01.
  • Error renamed: UnsafeEvalNotAvailableWorkerLoaderNotAvailable.
  • Tests updated to mock the new binding shape.
  • dev-server.mjs removed: wrangler dev binds worker_loader natively, so the custom Miniflare bootstrap is no longer needed. dev:worker script now runs wrangler dev.
  • README updated for the new plan / binding requirements.

The HTTP tool-callback protocol, driver code, and public API are unchanged. ~120 LOC change.

Validation

  • pnpm test:lib — 36/36 pass
  • pnpm test:types — clean
  • pnpm test:eslint — clean
  • Validated end-to-end on a real CF deployment (a bun patch of the published 0.1.8 with the same swap, deployed at cobalt-sandbox.<account>.workers.dev, e2e calling tool-callback round-trip against a Drizzle-backed driver).

Plan / cost

worker_loader is Workers Paid plan only. The CF API rejects deploys and --remote on Free with code: 10195"In order to use Dynamic Workers, you must switch to a paid plan." Local wrangler dev accepts the binding on the Free plan, so inner-loop iteration stays free.

This is a tier requirement Cloudflare enforces, not a regression in this PR — unsafe_eval would have required Paid in any account that ever managed to enable it. The README now states the plan requirement explicitly.

Breaking

Yes — minor bump. Consumers must update wrangler.toml:

# before
[[unsafe.bindings]]
name = "UNSAFE_EVAL"
type = "unsafe_eval"

# after
[[worker_loaders]]
binding = "LOADER"

✅ Checklist

  • I have followed the steps in the Contributing guide.
  • I have tested this code locally with pnpm run test:pr (package-scoped tests pass).

🚀 Release Impact

  • This change affects published code, and I have generated a changeset.
  • This change is docs/CI/dev-only (no release).

Summary by CodeRabbit

  • Breaking Changes

    • Worker deployments now require a LOADER (worker_loader) binding instead of the previous unsafe-eval binding.
  • Documentation

    • Setup and deployment docs updated for the wrangler dev / worker_loader workflow, including Free vs Paid plan considerations and guidance for the WorkerLoaderNotAvailable failure mode.
  • Chores

    • Local development switched to wrangler dev; compatibility date updated to 2026-05-01; local ignore rules updated.
  • Tests

    • Tests and harnesses updated to exercise the new LOADER-based execution and error cases.

…ader

Cloudflare gates the `unsafe_eval` binding for all customer prod
accounts (no public entitlement); the previous driver was unusable in
production and broken in `wrangler dev` on current Wrangler 4.x.

Swap `env.UNSAFE_EVAL.eval(code)` for the supported `worker_loader`
(Dynamic Workers) binding — load the wrapped code as an ES module into
a fresh child Worker isolate via `env.LOADER.load({...}).getEntrypoint()
.fetch(...)` and read the structured result back as JSON.

The HTTP tool-callback protocol, driver, and public API are unchanged.
~120 LOC change in worker; tests + wrangler.toml + README updated.

Workers Paid plan is required for any edge usage (deploy or
`wrangler dev --remote`); local `wrangler dev` works on the Free plan.
The custom Miniflare `dev-server.mjs` is removed since `wrangler dev`
now binds `worker_loader` natively.

Closes TanStack#522.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 3, 2026

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2513632a-e248-44de-89a6-8af473c40780

📥 Commits

Reviewing files that changed from the base of the PR and between a266c41 and 4d4fb19.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (1)
  • packages/typescript/ai-isolate-cloudflare/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/typescript/ai-isolate-cloudflare/package.json

📝 Walkthrough

Walkthrough

This PR ports the Cloudflare worker in @tanstack/ai-isolate-cloudflare from the gated unsafe_eval binding to the worker_loader (Dynamic Workers) binding, updating configuration, implementation, dev scripts, tests, and docs while keeping the public API and tool-callback protocol unchanged.

Changes

Worker Loader Migration

Layer / File(s) Summary
Configuration & Breaking Change
packages/typescript/ai-isolate-cloudflare/wrangler.toml, .changeset/worker-loader-port.md, .gitignore
compatibility_date pinned to 2026-05-01; removed unsafe_eval binding and added [[worker_loaders]] binding = "LOADER"; changeset documents breaking binding change and Workers plan requirements; .wrangler ignored.
Public Types / Env
packages/typescript/ai-isolate-cloudflare/src/worker/index.ts
Introduces SANDBOX_COMPAT_DATE, WorkerLoader/LoadedWorker types and Env with optional LOADER?: WorkerLoader.
Code Wrapping
packages/typescript/ai-isolate-cloudflare/src/worker/index.ts, .../wrap-code.ts
Adds wrapAsSandboxModule(wrappedCode) to emit an ES module exporting fetch; updates module comment to reference worker_loader testability.
Core Execution Flow
packages/typescript/ai-isolate-cloudflare/src/worker/index.ts
executeCode(...) now errors with WorkerLoaderNotAvailable when env.LOADER is missing, uses env.LOADER.load(...) to create a child Worker, calls getEntrypoint().fetch, races against an AbortController timeout sentinel, maps timeouts to TimeoutError, maps child execution failures to structured done with success: false, preserves need_tools continuation behavior.
Worker Handler Wiring
packages/typescript/ai-isolate-cloudflare/src/worker/index.ts
Fetch handler retains CORS/POST/JSON validation and response shapes; now operates with the LOADER-based Env.
Development Workflow
packages/typescript/ai-isolate-cloudflare/package.json, packages/typescript/ai-isolate-cloudflare/dev-server.mjs (removed)
Removed Miniflare-based dev-server.mjs; dev:worker script changed to wrangler dev; wrangler devDependency bumped.
Documentation & Setup Guide
packages/typescript/ai-isolate-cloudflare/README.md, .changeset/worker-loader-port.md
Replaced Miniflare/unsafe_eval instructions with wrangler dev/worker_loader guidance; documents Free-plan rejection behavior and WorkerLoaderNotAvailable failure mode; security guidance retained.
Tests & Validation
packages/typescript/ai-isolate-cloudflare/tests/*
Updated tests to mock/use env.LOADER instead of env.UNSAFE_EVAL; assertions changed to WorkerLoaderNotAvailable; added LOADER happy-path and need_tools forwarding tests; timeout test updated to assert AbortSignal behavior.

Sequence Diagram

sequenceDiagram
    actor Client
    participant MainWorker as Main Worker (ai-isolate)
    participant LoaderBinding as LOADER Binding
    participant ChildWorker as Child Worker (Sandbox)

    Client->>MainWorker: POST /execute (code + context)
    MainWorker->>MainWorker: wrapAsSandboxModule(code) -> module source
    MainWorker->>LoaderBinding: load({ mainModule, modules, compatibilityDate })
    LoaderBinding->>ChildWorker: initialize isolate with module
    MainWorker->>ChildWorker: getEntrypoint().fetch(Request, { signal })
    ChildWorker->>ChildWorker: run wrapped IIFE, produce JSON result
    ChildWorker-->>MainWorker: Response with result JSON
    MainWorker->>MainWorker: parse result, handle need_tools/done/errors/timeouts
    MainWorker-->>Client: JSON { status, value/error, toolCalls?, continuationId? }
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I swapped the old Unsafe for LOADER's light,
Spawned a child to sandbox each midnight.
Fetch calls hum, timeouts softly chime,
Tests mock the loader and pass in good time.
Hopping forward—one patch, one brave byte.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change: porting the Cloudflare worker from unsafe_eval to worker_loader, which is the primary objective of the PR.
Description check ✅ Passed The PR description comprehensively covers the changes, rationale, validation steps, and breaking change notice. It follows the template structure with clear sections for changes, checklist completion, and release impact.
Linked Issues check ✅ Passed The PR meets all requirements from issue #522: replaces unsafe_eval with worker_loader binding, preserves HTTP protocol/driver/API, includes minimal focused changes (~120 LOC), documents Paid plan requirement, and validates with unit tests and end-to-end deployment.
Out of Scope Changes check ✅ Passed All changes are directly aligned with the migration objective: worker binding swap, configuration updates, test updates for new binding shape, dev-server removal, and documentation updates. No unrelated changes detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value).


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 and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/typescript/ai-isolate-cloudflare/tests/worker.test.ts (1)

191-209: ⚡ Quick win

Please add one success-path LOADER test here as well.

Right now this file only locks down the missing-binding branch. The new LOADER.load(...).getEntrypoint().fetch(...) flow is the real regression surface in this PR, and a small mocked happy-path test would catch contract drift in mainModule, modules, entrypoint resolution, and JSON handling.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/typescript/ai-isolate-cloudflare/tests/worker.test.ts` around lines
191 - 209, Add a happy-path test that supplies a mocked LOADER binding to the
worker invocation and asserts the full load→getEntrypoint→fetch flow: create an
env object with LOADER having a load() method that returns an object whose
getEntrypoint() returns an object with an async fetch(request) that returns a
Response (e.g., JSON { status: 'ok', result: ... }), call worker.fetch(request,
env, mockExecutionContext) using the same request shape as the existing test,
then assert response.status === 200 and the parsed JSON matches the expected
success payload (e.g., json.status === 'ok' and contains the returned result);
this will exercise the LOADER.load, getEntrypoint, and entrypoint.fetch contract
used in the worker.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/typescript/ai-isolate-cloudflare/src/worker/index.ts`:
- Around line 111-131: The Promise.race approach returns a timeout error but
leaves entrypoint.fetch running; replace the race with a cancellation-aware
flow: create an AbortController before calling
env.LOADER.load()/loaded.getEntrypoint() and pass controller.signal into
entrypoint.fetch if the loader supports AbortSignal, set timeoutId to call
controller.abort() (and reject with TIMEOUT_SENTINEL) on timeout, and clear the
timeout when fetch resolves; if the loader/entrypoint does not support
AbortSignal, instead call the loader/entrypoint termination/dispose API (if
available) when the timeout fires or else update docs to state this is a
response-time limit only—adjust handling around timeoutPromise,
TIMEOUT_SENTINEL, timeoutId, entrypoint.fetch, env.LOADER.load, and
loaded.getEntrypoint accordingly.

---

Nitpick comments:
In `@packages/typescript/ai-isolate-cloudflare/tests/worker.test.ts`:
- Around line 191-209: Add a happy-path test that supplies a mocked LOADER
binding to the worker invocation and asserts the full load→getEntrypoint→fetch
flow: create an env object with LOADER having a load() method that returns an
object whose getEntrypoint() returns an object with an async fetch(request) that
returns a Response (e.g., JSON { status: 'ok', result: ... }), call
worker.fetch(request, env, mockExecutionContext) using the same request shape as
the existing test, then assert response.status === 200 and the parsed JSON
matches the expected success payload (e.g., json.status === 'ok' and contains
the returned result); this will exercise the LOADER.load, getEntrypoint, and
entrypoint.fetch contract used in the worker.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ee905ac4-2230-4b85-8d36-afc9224989c3

📥 Commits

Reviewing files that changed from the base of the PR and between ff33855 and 167d052.

📒 Files selected for processing (10)
  • .changeset/worker-loader-port.md
  • packages/typescript/ai-isolate-cloudflare/README.md
  • packages/typescript/ai-isolate-cloudflare/dev-server.mjs
  • packages/typescript/ai-isolate-cloudflare/package.json
  • packages/typescript/ai-isolate-cloudflare/src/worker/index.ts
  • packages/typescript/ai-isolate-cloudflare/src/worker/wrap-code.ts
  • packages/typescript/ai-isolate-cloudflare/tests/escape-attempts.test.ts
  • packages/typescript/ai-isolate-cloudflare/tests/isolate-driver.test.ts
  • packages/typescript/ai-isolate-cloudflare/tests/worker.test.ts
  • packages/typescript/ai-isolate-cloudflare/wrangler.toml
💤 Files with no reviewable changes (1)
  • packages/typescript/ai-isolate-cloudflare/dev-server.mjs

Comment thread packages/typescript/ai-isolate-cloudflare/src/worker/index.ts
…-path tests

Address CodeRabbit review on TanStack#523:

1. Promise.race timeout left `entrypoint.fetch` running, leaking the loaded
   child Worker isolate. Add an AbortController whose signal flows into the
   Request passed to entrypoint.fetch — the timeout now actually cancels the
   in-flight request. Promise.race remains as a belt-and-suspenders guard.

2. Add three integration tests against a mocked LOADER binding:
   - happy path: full load → getEntrypoint → fetch chain, asserts the
     load() arguments (mainModule, modules, globalOutbound) and that the
     Request carries an AbortSignal
   - need_tools: forwards toolCalls + continuationId from sandbox
   - TimeoutError: AbortSignal-driven cancellation triggers the right
     error shape

Tests: 39/39 pass.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
packages/typescript/ai-isolate-cloudflare/tests/worker.test.ts (2)

223-228: ⚡ Quick win

Assertions inside load() mock may be silently swallowed by the outer worker's error handling.

load() is a synchronous function called inside the outer worker's implementation. If any expect(...) inside it throws (e.g., the toBeNull() on globalOutbound), the worker's try/catch will intercept that AssertionError and return a generic error response. The test then fails at a later downstream assertion (such as expect(json.status).toBe('done')) rather than at the actual failing line, making failures hard to diagnose.

Capture the options outside the mock and assert after worker.fetch() resolves:

♻️ Proposed refactor
+    let loadCalled = false
+    let capturedOptions: Parameters<NonNullable<typeof env['LOADER']>['load']>[0] | null = null
     let receivedSignal: AbortSignal | null = null
     const env = {
       LOADER: {
         load: (options: { ... }) => {
           loadCalled = true
-          // Sanity-check the load() arguments the worker passes.
-          expect(options.mainModule).toBe('main.js')
-          expect(options.modules).toHaveProperty('main.js')
-          expect(options.modules['main.js']).toContain('export default')
-          expect(options.globalOutbound).toBeNull()
+          capturedOptions = options
           return {
             getEntrypoint: () => ({
               fetch: async (req: Request) => { ... },
             }),
           }
         },
       },
     }

     // ... worker.fetch() call ...

     expect(loadCalled).toBe(true)
+    expect(capturedOptions!.mainModule).toBe('main.js')
+    expect(capturedOptions!.modules).toHaveProperty('main.js')
+    expect(capturedOptions!.modules['main.js']).toContain('export default')
+    expect(capturedOptions!.globalOutbound).toBeNull()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/typescript/ai-isolate-cloudflare/tests/worker.test.ts` around lines
223 - 228, The assertions currently inside the synchronous load() mock can be
swallowed by the worker's try/catch; instead capture the options object and
loadCalled flag from inside the mock (e.g., set loadCalled = true and
savedOptions = options) and remove all expect(...) calls from inside load();
then after awaiting worker.fetch(...) resolve, assert on savedOptions
(expect(savedOptions.mainModule).toBe('main.js'),
expect(savedOptions.modules).toHaveProperty('main.js'),
expect(savedOptions.modules['main.js']).toContain('export default'),
expect(savedOptions.globalOutbound).toBeNull()) and on loadCalled and
json.status to surface assertion failures at their true origin.

232-232: ⚡ Quick win

expect(receivedSignal).not.toBeNull() is trivially true; the timeout test doesn't definitively exercise the AbortSignal path.

Per the WHATWG Fetch spec and Cloudflare Workers docs, Request.signal is always a non-null AbortSignal — a never-aborted one is synthesised if no signal is provided in RequestInit. So line 257 passes regardless of whether the outer worker constructs and threads an AbortController signal.

The timeout test's mock comment says "Never resolves on its own; relies on AbortSignal," but because the outer worker also has a Promise.race sentinel (per the PR description), the test passes via that sentinel even if the AbortController is never properly connected. Neither test therefore proves that the signal is the outer worker's own timed AbortController signal.

Fix: drop the weak not.toBeNull() assertion; instead, capture receivedSignal in the timeout test and assert receivedSignal?.aborted === true after worker.fetch() returns. This confirms the outer worker actually aborts the signal — not just that a signal exists.

♻️ Proposed refactor for the timeout test
  it('returns TimeoutError when entrypoint.fetch exceeds timeout', async () => {
+   let receivedSignal: AbortSignal | null = null
    const env = {
      LOADER: {
        load: () => ({
          getEntrypoint: () => ({
            fetch: (req: Request) =>
              new Promise<Response>((_resolve, reject) => {
+               receivedSignal = req.signal
                req.signal.addEventListener('abort', () => {
                  reject(new Error('aborted'))
                })
                // Never resolves on its own; relies on AbortSignal.
              }),
          }),
        }),
      },
    }
    // ...
    const response = await worker.fetch(request, env, mockExecutionContext)

+   // Confirms the outer worker aborts its own AbortController signal on timeout,
+   // not merely that Promise.race fired.
+   expect(receivedSignal?.aborted).toBe(true)

    expect(response.status).toBe(200)
    const json = await response.json()
    expect(json.status).toBe('error')
    expect(json.error.name).toBe('TimeoutError')
    expect(json.error.message).toContain('50ms')
  })

Also applies to: 257-257, 303-336

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/typescript/ai-isolate-cloudflare/tests/worker.test.ts` at line 232,
The current timeout test only asserts expect(receivedSignal).not.toBeNull(),
which is ineffective because Request.signal is always non-null; modify the
timeout test to capture the receivedSignal from the mocked fetch handler (the
same variable currently set by receivedSignal = req.signal) and after calling
worker.fetch(...) assert that receivedSignal?.aborted === true to prove the
outer worker's AbortController actually aborted the signal; remove the weak
not.toBeNull() assertion and add the aborted check in the timeout test that
exercises the timeout-path used by the worker code (ensure the assertion runs
after the worker.fetch promise resolves).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/typescript/ai-isolate-cloudflare/tests/worker.test.ts`:
- Around line 223-228: The assertions currently inside the synchronous load()
mock can be swallowed by the worker's try/catch; instead capture the options
object and loadCalled flag from inside the mock (e.g., set loadCalled = true and
savedOptions = options) and remove all expect(...) calls from inside load();
then after awaiting worker.fetch(...) resolve, assert on savedOptions
(expect(savedOptions.mainModule).toBe('main.js'),
expect(savedOptions.modules).toHaveProperty('main.js'),
expect(savedOptions.modules['main.js']).toContain('export default'),
expect(savedOptions.globalOutbound).toBeNull()) and on loadCalled and
json.status to surface assertion failures at their true origin.
- Line 232: The current timeout test only asserts
expect(receivedSignal).not.toBeNull(), which is ineffective because
Request.signal is always non-null; modify the timeout test to capture the
receivedSignal from the mocked fetch handler (the same variable currently set by
receivedSignal = req.signal) and after calling worker.fetch(...) assert that
receivedSignal?.aborted === true to prove the outer worker's AbortController
actually aborted the signal; remove the weak not.toBeNull() assertion and add
the aborted check in the timeout test that exercises the timeout-path used by
the worker code (ensure the assertion runs after the worker.fetch promise
resolves).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5e527ef4-c182-4ea4-a978-f32123c8d0a7

📥 Commits

Reviewing files that changed from the base of the PR and between 167d052 and 669a2aa.

📒 Files selected for processing (2)
  • packages/typescript/ai-isolate-cloudflare/src/worker/index.ts
  • packages/typescript/ai-isolate-cloudflare/tests/worker.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/typescript/ai-isolate-cloudflare/src/worker/index.ts

Address CodeRabbit second-pass review:

1. happy-path test: hoist load() argument assertions out of the synchronous
   mock. Inside load() they get swallowed by the worker's outer try/catch and
   surface as a generic 500. Capture options into a local + assert after
   worker.fetch() resolves.

2. timeout test: `expect(receivedSignal).not.toBeNull()` is trivially true
   per the Fetch spec (Request.signal is always present). Drop it from the
   happy-path test and instead assert `signal.aborted === true` in the
   timeout test, which actually proves the outer worker's AbortController
   fired.

3. worker fix: when the AbortController fires first, fetchPromise rejects
   before timeoutPromise. Detect the timeout via either TIMEOUT_SENTINEL or
   `controller.signal.aborted` so the right error surfaces regardless of
   which branch of the race wins.

Tests: 39/39 pass.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/typescript/ai-isolate-cloudflare/src/worker/index.ts`:
- Around line 140-147: The code directly assigns await response.json() to the
local result variable and then branches on its status; instead validate this
boundary payload with Zod before using it. Define a Zod schema that matches the
expected ExecuteResponse shape (fields: status, success, value, error
{name,message,stack?}, logs: string[], toolCalls?: ToolCallRequest[]), call
resultRaw = await response.json() and then parse it with schema.parse or
schema.safeParse, and handle a failed parse by throwing or returning a clear
protocol error; update the code paths that currently reference result (the
status branching and subsequent use of value/logs/error/toolCalls) to use the
validated parsed object instead.
- Around line 71-79: The child worker currently assumes the wrapped result is
JSON-serializable and lets JSON.stringify throw (losing structured error info);
update wrapAsSandboxModule (and the code path produced by wrapCode) to catch
serialization failures: after awaiting __result, attempt to JSON.stringify
inside a try/catch and, on error, construct a deterministic serializable
fallback (e.g., { ok: false, errorType: 'SerializeError', message: String(err),
valueType: typeof __result }) or attempt safe conversions (e.g., BigInt ->
string) then return that JSON in the Response instead of letting JSON.stringify
propagate; ensure the module exports the same envelope shape for success (e.g.,
{ ok: true, value: ... }) and failure so the parent can reliably parse
structured sandbox results.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4f4ebfd5-c892-4c94-8625-c266567b8e89

📥 Commits

Reviewing files that changed from the base of the PR and between 669a2aa and b2ce85f.

📒 Files selected for processing (2)
  • packages/typescript/ai-isolate-cloudflare/src/worker/index.ts
  • packages/typescript/ai-isolate-cloudflare/tests/worker.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/typescript/ai-isolate-cloudflare/tests/worker.test.ts

Comment on lines +71 to +79
function wrapAsSandboxModule(wrappedCode: string): string {
return `
export default {
async fetch() {
const __result = await ${wrappedCode};
return new Response(JSON.stringify(__result), {
headers: { 'Content-Type': 'application/json' },
});
}
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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Catch serialization failures inside the child worker.

wrapCode() can produce value: unknown, but this path assumes the full result is JSON-serializable. Values like BigInt or circular objects will throw here, outside the wrapped IIFE, so the parent only sees an opaque fetch/JSON failure and loses the structured sandbox result.

Suggested fix
 export default {
   async fetch() {
     const __result = await ${wrappedCode};
-    return new Response(JSON.stringify(__result), {
-      headers: { 'Content-Type': 'application/json' },
-    });
+    try {
+      return new Response(JSON.stringify(__result), {
+        headers: { 'Content-Type': 'application/json' },
+      });
+    } catch (__error) {
+      return new Response(
+        JSON.stringify({
+          status: 'done',
+          success: false,
+          error: {
+            name: __error?.name ?? 'SerializationError',
+            message: __error?.message ?? String(__error),
+          },
+          logs: __result?.logs ?? [],
+        }),
+        {
+          headers: { 'Content-Type': 'application/json' },
+        },
+      );
+    }
   }
 };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/typescript/ai-isolate-cloudflare/src/worker/index.ts` around lines
71 - 79, The child worker currently assumes the wrapped result is
JSON-serializable and lets JSON.stringify throw (losing structured error info);
update wrapAsSandboxModule (and the code path produced by wrapCode) to catch
serialization failures: after awaiting __result, attempt to JSON.stringify
inside a try/catch and, on error, construct a deterministic serializable
fallback (e.g., { ok: false, errorType: 'SerializeError', message: String(err),
valueType: typeof __result }) or attempt safe conversions (e.g., BigInt ->
string) then return that JSON in the Response instead of letting JSON.stringify
propagate; ensure the module exports the same envelope shape for success (e.g.,
{ ok: true, value: ... }) and failure so the parent can reliably parse
structured sandbox results.

Comment thread packages/typescript/ai-isolate-cloudflare/src/worker/index.ts
@tombeckenham tombeckenham requested a review from a team May 7, 2026 07:06
@tombeckenham tombeckenham self-assigned this May 7, 2026
@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented May 7, 2026

View your CI Pipeline Execution ↗ for commit 4d4fb19

Command Status Duration Result
nx run-many --targets=build --exclude=examples/** ✅ Succeeded 1m 49s View ↗

☁️ Nx Cloud last updated this comment at 2026-05-07 07:45:19 UTC

@tombeckenham tombeckenham self-requested a review May 7, 2026 07:11
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 7, 2026

Open in StackBlitz

@tanstack/ai

npm i https://pkg.pr.new/@tanstack/ai@523

@tanstack/ai-anthropic

npm i https://pkg.pr.new/@tanstack/ai-anthropic@523

@tanstack/ai-client

npm i https://pkg.pr.new/@tanstack/ai-client@523

@tanstack/ai-code-mode

npm i https://pkg.pr.new/@tanstack/ai-code-mode@523

@tanstack/ai-code-mode-skills

npm i https://pkg.pr.new/@tanstack/ai-code-mode-skills@523

@tanstack/ai-devtools-core

npm i https://pkg.pr.new/@tanstack/ai-devtools-core@523

@tanstack/ai-elevenlabs

npm i https://pkg.pr.new/@tanstack/ai-elevenlabs@523

@tanstack/ai-event-client

npm i https://pkg.pr.new/@tanstack/ai-event-client@523

@tanstack/ai-fal

npm i https://pkg.pr.new/@tanstack/ai-fal@523

@tanstack/ai-gemini

npm i https://pkg.pr.new/@tanstack/ai-gemini@523

@tanstack/ai-grok

npm i https://pkg.pr.new/@tanstack/ai-grok@523

@tanstack/ai-groq

npm i https://pkg.pr.new/@tanstack/ai-groq@523

@tanstack/ai-isolate-cloudflare

npm i https://pkg.pr.new/@tanstack/ai-isolate-cloudflare@523

@tanstack/ai-isolate-node

npm i https://pkg.pr.new/@tanstack/ai-isolate-node@523

@tanstack/ai-isolate-quickjs

npm i https://pkg.pr.new/@tanstack/ai-isolate-quickjs@523

@tanstack/ai-ollama

npm i https://pkg.pr.new/@tanstack/ai-ollama@523

@tanstack/ai-openai

npm i https://pkg.pr.new/@tanstack/ai-openai@523

@tanstack/ai-openrouter

npm i https://pkg.pr.new/@tanstack/ai-openrouter@523

@tanstack/ai-preact

npm i https://pkg.pr.new/@tanstack/ai-preact@523

@tanstack/ai-react

npm i https://pkg.pr.new/@tanstack/ai-react@523

@tanstack/ai-react-ui

npm i https://pkg.pr.new/@tanstack/ai-react-ui@523

@tanstack/ai-solid

npm i https://pkg.pr.new/@tanstack/ai-solid@523

@tanstack/ai-solid-ui

npm i https://pkg.pr.new/@tanstack/ai-solid-ui@523

@tanstack/ai-svelte

npm i https://pkg.pr.new/@tanstack/ai-svelte@523

@tanstack/ai-vue

npm i https://pkg.pr.new/@tanstack/ai-vue@523

@tanstack/ai-vue-ui

npm i https://pkg.pr.new/@tanstack/ai-vue-ui@523

@tanstack/preact-ai-devtools

npm i https://pkg.pr.new/@tanstack/preact-ai-devtools@523

@tanstack/react-ai-devtools

npm i https://pkg.pr.new/@tanstack/react-ai-devtools@523

@tanstack/solid-ai-devtools

npm i https://pkg.pr.new/@tanstack/solid-ai-devtools@523

commit: 4d4fb19

@tombeckenham tombeckenham removed the request for review from a team May 7, 2026 07:11
These were only used by the deleted dev-server.mjs, which existed to
work around wrangler dev not surfacing the old unsafe_eval binding.
With the worker_loader port, wrangler dev handles dev natively.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

ai-isolate-cloudflare: unsafe_eval is gated by Cloudflare; port to worker_loader

2 participants