Skip to content

[wrangler] Remove redundant dev-registry filtering #12974

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ask-bonk wants to merge 3 commits into
base: main
Choose a base branch
from
+17 −64
Open

[wrangler] Remove redundant dev-registry filtering #12974

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
861e5a8
[wrangler] Remove redundant dev-registry filtering in unstable_getMin…
ask-bonk[bot] Mar 19, 2026
9171d57
Fix CI: remove unused import and add PR checklist to AGENTS.md
ask-bonk[bot] Mar 20, 2026
c11533c
Remove redundant unused-imports line from AGENTS.md
ask-bonk[bot] Mar 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/remove-dev-registry-filtering.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": patch
---

Remove redundant dev-registry filtering in `unstable_getMiniflareWorkerOptions`

The code that rewrote `serviceBindings` and `durableObjects` in `unstable_getMiniflareWorkerOptions` was originally needed to avoid relying on the dev registry for the Workers Vitest pool. Since the dev registry is now entirely defined in Miniflare, this rewriting is no longer necessary — `buildMiniflareBindingOptions` already produces the correct bindings. Removing this code also means `props` on service bindings are no longer dropped.
10 changes: 10 additions & 0 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,16 @@ This is the **Cloudflare Workers SDK** monorepo containing tools and libraries f
- Keep all checkboxes in the template (don't delete unchecked ones)
- PR title format: `[package name] description` (e.g. `[wrangler] Fix bug in dev command`)
- If the change doesn't require a changeset, add the `no-changeset-required` label
- CI validates the PR description (see `tools/deployments/validate-pr-description.ts`). The description **must** include:
- A checked (`[x]`) test checkbox — either "Tests included/updated", or one of the justification checkboxes with a non-empty explanation
- A checked (`[x]`) documentation checkbox — either a Cloudflare docs PR/issue link, or "Documentation not necessary because:" with a non-empty explanation
- A changeset file (or the `no-changeset-required` label)

**Pre-Submission Checklist:**

- Run `pnpm check` (lint + type-check + format) locally before pushing — do not rely on CI to catch lint errors
- When removing code, verify that all imports used only by the removed code are also removed (the linter enforces no unused imports)
Copy link
Copy Markdown
Contributor

@penalosa penalosa Mar 20, 2026

Choose a reason for hiding this comment

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

/bonk this line is redundant

Copy link
Copy Markdown
Contributor Author

@ask-bonk ask-bonk bot Mar 20, 2026

Choose a reason for hiding this comment

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

Agreed — the linter catches this via pnpm check which is already listed. Removed in c11533c.

- Run `pnpm prettify` to ensure formatting is correct

## Key Locations

Expand Down
65 changes: 1 addition & 64 deletions packages/wrangler/src/api/integrations/platform/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
getLocalWorkerdCompatibilityDate,
getRegistryPath,
} from "@cloudflare/workers-utils";
import { kCurrentWorker, Miniflare } from "miniflare";
import { Miniflare } from "miniflare";
import { getAssetsOptions, NonExistentAssetsDirError } from "../../../assets";
import { readConfig } from "../../../config";
import { partitionDurableObjectBindings } from "../../../deployment-bundle/entry";
Expand All @@ -15,7 +15,6 @@ import {
buildAssetOptions,
buildMiniflareBindingOptions,
buildSitesOptions,
getImageNameFromDOClassName,
} from "../../../dev/miniflare";
import { logger } from "../../../logger";
import { getSiteAssetPaths } from "../../../sites";
Expand Down Expand Up @@ -435,68 +434,6 @@ export function unstable_getMiniflareWorkerOptions(
options?.remoteProxyConnectionString
);

// This function is currently only exported for the Workers Vitest pool.
// In tests, we don't want to rely on the dev registry, as we can't guarantee
// which sessions will be running. Instead, we rewrite `serviceBindings` and
// `durableObjects` to use more traditional Miniflare config expecting the
// user to define workers with the required names in the `workers` array.
// These will run the same `workerd` processes as tests.
const serviceBindings = extractBindingsOfType("service", bindings);
if (serviceBindings.length > 0) {
bindingOptions.serviceBindings = Object.fromEntries(
serviceBindings.map((binding) => {
const name =
binding.service === config.name ? kCurrentWorker : binding.service;
if (options?.remoteProxyConnectionString && binding.remote) {
return [
binding.binding,
{
name,
entrypoint: binding.entrypoint,
remoteProxyConnectionString: options.remoteProxyConnectionString,
},
];
}
return [binding.binding, { name, entrypoint: binding.entrypoint }];
})
);
}
const durableObjectBindings = extractBindingsOfType(
"durable_object_namespace",
bindings
);
if (durableObjectBindings.length > 0) {
type DurableObjectDefinition = NonNullable<
typeof bindingOptions.durableObjects
>[string];

const classNameToUseSQLite = getDurableObjectClassNameToUseSQLiteMap(
config.migrations
);

bindingOptions.durableObjects = Object.fromEntries(
durableObjectBindings.map((binding) => {
const useSQLite = classNameToUseSQLite.get(binding.class_name);
return [
binding.name,
{
className: binding.class_name,
scriptName: binding.script_name,
useSQLite,
container:
enableContainers && config.containers?.length
? getImageNameFromDOClassName({
doClassName: binding.class_name,
containerDOClassNames,
containerBuildId: options?.containerBuildId,
})
: undefined,
} satisfies DurableObjectDefinition,
];
})
);
}

const sitesAssetPaths = getSiteAssetPaths(config);
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot Mar 20, 2026

Choose a reason for hiding this comment

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

🔴 Self-referencing service bindings broken in vitest-pool-workers due to removed kCurrentWorker mapping

The removed code mapped self-referencing service bindings (binding.service === config.name) to kCurrentWorker, a special Miniflare symbol that routes requests back to the current worker regardless of its registered name. Without this, buildMiniflareBindingOptions (packages/wrangler/src/dev/miniflare/index.ts:559-575) uses the literal service.service value (e.g., "my-worker").

In the vitest-pool-workers context, the worker's name is overridden to vitest-pool-workers-runner-<project> at packages/vitest-pool-workers/src/pool/index.ts:285, which does NOT match config.name. So a self-referencing service binding now points to a worker name that doesn't exist in the Miniflare configuration, causing the binding to fail at runtime.

How the old code handled this

The removed code at the old lines 446-462 did:

const name = binding.service === config.name ? kCurrentWorker : binding.service;

This ensured that when a worker had a service binding to itself, Miniflare would route it to the current worker via the kCurrentWorker symbol (packages/miniflare/src/plugins/core/index.ts:382), regardless of what name the worker was registered under.

Prompt for agents 
In packages/wrangler/src/api/integrations/platform/index.ts, after the call to buildMiniflareBindingOptions (around line 435), re-add the kCurrentWorker mapping for self-referencing service bindings. This is needed because vitest-pool-workers renames the worker (packages/vitest-pool-workers/src/pool/index.ts:285) so service bindings that reference config.name won't resolve correctly.

Specifically:
1. Re-add the import of kCurrentWorker from miniflare at line 7
2. After line 435, add code that iterates over bindingOptions.serviceBindings and replaces any entry where the name matches config.name with kCurrentWorker, similar to what was previously done:

  if (bindingOptions.serviceBindings) {
    for (const [key, value] of Object.entries(bindingOptions.serviceBindings)) {
      if (typeof value === 'object' && value !== null && 'name' in value && value.name === config.name) {
        bindingOptions.serviceBindings[key] = { ...value, name: kCurrentWorker };
      }
    }
  }

This preserves the props field (which the old code was dropping) while still correctly handling self-references.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

const sitesOptions = buildSitesOptions({ legacyAssetPaths: sitesAssetPaths });
const processedAssetOptions = getAssetsOptions(
Expand Down
Loading