[miniflare] Native dev registry review feedback by penalosa · Pull Request #13569 · cloudflare/workers-sdk

penalosa · 2026-04-16T15:11:52Z

Follow-up to #12600.

Summary

Distinguish "worker not running" from "incompatible wrangler version" in proxy error messages — when a registry entry exists but lacks debugPortAddress (old format), the proxy now returns a specific message instead of the generic "not found"
Use debug port RPC for explorer cross-instance aggregation instead of HTTP loopback, removing loopbackAddress from WorkerDefinition

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: internal dev tooling change

…on' in proxy errors When a registry entry exists but lacks debugPortAddress (written by an older wrangler), the proxy now returns a specific message asking the user to update all Wrangler instances, instead of the generic 'not found' message.

changeset-bot · 2026-04-16T15:11:59Z

⚠️ No Changeset found

Latest commit: 16c8442

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

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

pkg-pr-new · 2026-04-16T15:16:52Z

create-cloudflare

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-editor-shared

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

wrangler

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

commit: 16c8442

ask-bonk

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

This PR improves proxy error messages to distinguish "worker not running" from "incompatible wrangler version", and switches explorer cross-instance aggregation from HTTP loopback to debug port RPC.

I found two issues with the test updates:

Wrong service name in test assertion (high): The test at fixtures/dev-registry/tests/dev-registry.test.ts:937 expects an error message about "service-worker", but the binding WORKER_ENTRYPOINT_WITH_ASSETS maps to service "worker-entrypoint-with-assets" (see wrangler.worker-entrypoint.jsonc line 20-22). The proxy uses this.ctx.props.service in the error message, which would be "worker-entrypoint-with-assets". Additionally, since "worker-entrypoint-with-assets" has no registry entry at all at that point (only "worker-entrypoint" is registered by getPlatformProxy), hasRegistryEntry returns false and the message should be the "not found" variant, not "not compatible". See inline suggestion.
Missing test update for stale registry (medium): The existing "gracefully handles a stale registry entry" test at fixtures/dev-registry/tests/dev-registry.test.ts:1228-1229 expects Worker "service-worker" not found. Make sure it is running locally., but now that an old-format entry IS in the registry Map (written at line 1206), hasRegistryEntry("service-worker") returns true, so workerNotFoundMessage should return the "not compatible" message instead. This line wasn't changed in the PR but should be updated to: `Worker "service-worker" is not compatible with this version of the dev server. Please update all Worker instances to the same version.`
Stale JSDoc on fetchFromPeer (low): The @param peerUrl doc at aggregation.ts:56 still says "Base URL of the peer instance (e.g., http://127.0.0.1:8788)" but the parameter now receives a debug port address like "127.0.0.1:12345". See inline suggestion.

ask-bonk · 2026-04-16T15:25:37Z

The review has been posted successfully. Here's a summary of what I found:

Wrong service name in test assertion (high severity): fixtures/dev-registry/tests/dev-registry.test.ts:937 - The test expects an error about "service-worker" but the binding actually maps to "worker-entrypoint-with-assets". The assertion should have remained unchanged from before the PR. Posted an inline suggestion to revert to the original.
Missing test update for stale registry (medium severity): fixtures/dev-registry/tests/dev-registry.test.ts:1228-1229 - The "stale registry entry" test expects the "not found" message, but with the new hasRegistryEntry/workerNotFoundMessage logic, an old-format entry in the registry would trigger the "not compatible" message instead. This test needs updating but was missed.
Stale JSDoc (low severity): aggregation.ts:56 - The @param peerUrl doc still describes an HTTP URL but the parameter now receives a debug port address.

github run

workers-devprod · 2026-04-16T15:35:17Z

Codeowners approval required for this PR:

✅ @cloudflare/wrangler

Show detailed file reviewers

Replace HTTP loopback aggregation with debug port RPC. Remove loopbackAddress from WorkerDefinition since the explorer now connects to peers via the debug port directly. Add DEV_REGISTRY_DEBUG_PORT binding to the explorer worker.

…ssage The test was passing by accident — vi.waitFor succeeded before the file watcher picked up the old-format entry, so it matched the generic 'not found' message. Now waits specifically for the incompatible version message to verify the detection works end-to-end.

…egation Entries from older wrangler versions lack debugPortAddress. Filter them out in getPeerDebugPortAddresses() to avoid passing undefined to connect(). Also rename peerUrl parameter to peerDebugPortAddress to match the new semantics.

dario-piotrowicz

LGTM 🙂

workers-devprod

Codeowners reviews satisfied

workers-devprod added this to workers-sdk Apr 16, 2026

github-project-automation bot moved this to Untriaged in workers-sdk Apr 16, 2026

ask-bonk bot reviewed Apr 16, 2026

View reviewed changes

Comment thread fixtures/dev-registry/tests/dev-registry.test.ts Outdated

Comment thread packages/miniflare/src/workers/local-explorer/aggregation.ts Outdated

penalosa marked this pull request as ready for review April 16, 2026 15:34

penalosa requested a review from workers-devprod as a code owner April 16, 2026 15:34

workers-devprod requested review from a team and edmundhung and removed request for a team April 16, 2026 15:35

This comment was marked as resolved.

Sign in to view

penalosa force-pushed the penalosa/native-registry-review-feedback branch from 3a5c508 to ed45595 Compare April 16, 2026 16:09

penalosa added the no-changeset-required label Apr 16, 2026

penalosa added 2 commits April 16, 2026 17:57

dario-piotrowicz approved these changes Apr 16, 2026

View reviewed changes

Comment thread packages/miniflare/src/workers/core/dev-registry-proxy-shared.worker.ts

workers-devprod approved these changes Apr 16, 2026

View reviewed changes

github-project-automation bot moved this from Untriaged to Approved in workers-sdk Apr 16, 2026

penalosa merged commit 4d16ba9 into main Apr 17, 2026
63 of 64 checks passed

penalosa deleted the penalosa/native-registry-review-feedback branch April 17, 2026 10:00

github-project-automation bot moved this from Approved to Done in workers-sdk Apr 17, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[miniflare] Native dev registry review feedback#13569

[miniflare] Native dev registry review feedback#13569
penalosa merged 4 commits intomainfrom
penalosa/native-registry-review-feedback

penalosa commented Apr 16, 2026

Uh oh!

changeset-bot bot commented Apr 16, 2026 •

edited

Loading

Uh oh!

pkg-pr-new bot commented Apr 16, 2026 •

edited

Loading

Uh oh!

ask-bonk bot left a comment

Uh oh!

Uh oh!

Uh oh!

ask-bonk bot commented Apr 16, 2026

Uh oh!

workers-devprod commented Apr 16, 2026 •

edited

Loading

Uh oh!

This comment was marked as resolved.

Uh oh!

dario-piotrowicz left a comment

Uh oh!

Uh oh!

workers-devprod left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

penalosa commented Apr 16, 2026

Summary

Uh oh!

changeset-bot bot commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

pkg-pr-new bot commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ask-bonk bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ask-bonk bot commented Apr 16, 2026

Uh oh!

workers-devprod commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

dario-piotrowicz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

workers-devprod left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

changeset-bot bot commented Apr 16, 2026 •

edited

Loading

pkg-pr-new bot commented Apr 16, 2026 •

edited

Loading

workers-devprod commented Apr 16, 2026 •

edited

Loading