Skip to content

fix(miniflare): allow tcp connection when a outboundService is configured#13999

Merged
edmundhung merged 2 commits into
mainfrom
edmundhung/fix-miniflare-outbound
May 22, 2026
Merged

fix(miniflare): allow tcp connection when a outboundService is configured#13999
edmundhung merged 2 commits into
mainfrom
edmundhung/fix-miniflare-outbound

Conversation

@edmundhung
Copy link
Copy Markdown
Member

@edmundhung edmundhung commented May 21, 2026

Fixes n/a.


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because:
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: bugfix

A picture of a cute animal (not mandatory, but encouraged)


Open in Devin Review

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 21, 2026

🦋 Changeset detected

Latest commit: f935c02

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
miniflare Patch
@cloudflare/pages-shared Patch
@cloudflare/vite-plugin Patch
@cloudflare/vitest-pool-workers Patch
wrangler Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-project-automation github-project-automation Bot moved this to Untriaged in workers-sdk May 21, 2026
@workers-devprod workers-devprod requested review from a team and dario-piotrowicz and removed request for a team May 21, 2026 21:13
@workers-devprod
Copy link
Copy Markdown
Contributor

workers-devprod commented May 21, 2026

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

✅ All changesets look good

Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk Bot left a comment

Choose a reason for hiding this comment

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

I'm Bonk, and I've done a quick review of your PR.

This PR adds the connect_pass_through compatibility flag to the custom fetch service worker used for outboundService, so that TCP connect events pass through instead of failing when there's no connect handler.

  1. Missing the same fix for SCRIPT_CUSTOM_NODE_SERVICE (medium severity): The node-style custom service handler (line ~473) has the identical structure -- a service worker script with only a fetch listener, also constructed in maybeGetCustomServiceService. It would have the same TCP passthrough failure if used as an outbound service. It should also receive the connect_pass_through and experimental compatibility flags. Specifically, at line 474, the block:
serviceWorkerScript: SCRIPT_CUSTOM_NODE_SERVICE,
compatibilityDate: "2022-09-01",
bindings: [

should become:

serviceWorkerScript: SCRIPT_CUSTOM_NODE_SERVICE,
compatibilityDate: "2022-09-01",
compatibilityFlags: ["connect_pass_through", "experimental"],
bindings: [

Comment thread packages/miniflare/src/plugins/core/index.ts Outdated
@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented May 21, 2026

Review posted successfully on PR #13999.

Here's a summary of the review:

The fix itself is correct -- adding connect_pass_through and experimental compatibility flags to the custom fetch service worker allows TCP connections (via cloudflare:sockets) to pass through when an outboundService is configured. The test is well-structured and covers both fetch interception and TCP passthrough in the same scenario.

One actionable issue found: The SCRIPT_CUSTOM_NODE_SERVICE branch in the same maybeGetCustomServiceService function (line ~473) has an identical structure -- a service worker with only a fetch event listener -- and would suffer from the same TCP failure. It should also receive the connect_pass_through flag for consistency.

github run

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 2 additional findings in Devin Review.

Open in Devin Review

Comment thread packages/miniflare/src/plugins/core/index.ts
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 21, 2026

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@13999

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@13999

miniflare

npm i https://pkg.pr.new/miniflare@13999

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@13999

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@13999

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@13999

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@13999

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@13999

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@13999

wrangler

npm i https://pkg.pr.new/wrangler@13999

commit: f935c02

Copy link
Copy Markdown
Member

@dario-piotrowicz dario-piotrowicz left a comment

Choose a reason for hiding this comment

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

LGTM 🙂

Copy link
Copy Markdown
Contributor

@workers-devprod workers-devprod left a comment

Choose a reason for hiding this comment

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

Codeowners reviews satisfied

@github-project-automation github-project-automation Bot moved this from Untriaged to Approved in workers-sdk May 22, 2026
@edmundhung edmundhung merged commit 30657e1 into main May 22, 2026
63 checks passed
@edmundhung edmundhung deleted the edmundhung/fix-miniflare-outbound branch May 22, 2026 06:20
@github-project-automation github-project-automation Bot moved this from Approved to Done in workers-sdk May 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants