Skip to content

[wrangler] Fix published declaration dependency leaks - #16074

Open
AyobamiH wants to merge 10 commits into
cloudflare:mainfrom
AyobamiH:fix/15940-wrangler-declaration-boundary
Open

AyobamiH wants to merge 10 commits into
cloudflare:mainfrom
AyobamiH:fix/15940-wrangler-declaration-boundary

Conversation

@AyobamiH

@AyobamiH AyobamiH commented Oct 5, 2026 •

Copy link
Copy Markdown

Addresses the Wrangler declaration leaks in #15940. The separate Miniflare declaration errors remain tracked in #15377.

Problem

Wrangler's published declarations reference private workspace packages and development-only dependencies. Following only their top-level imports can also leave unresolved relative paths in the generated cli.d.ts. Consumers that check dependency declarations with skipLibCheck: false then fail to compile.

Inlining every package is insufficient: workers-utils owns the unique symbol used by inherited bindings, and Undici exposes its MockAgent class through Wrangler's public worker options. These types must retain their dependency-owned identities.

Changes

  • Declare workers-utils and the existing catalog-pinned Undici version as production dependencies required by public types. Explicit noExternal settings preserve Wrangler's bundled CommonJS runtime.
  • Resolve private declaration dependencies and their relative imports, using declaration-only source paths where tsup 8.3 cannot resolve the packages' published entrypoints.
  • Close workers-utils' own declaration graph, including Zod's ESM declarations. Replace four Cloudflare SDK namespace references with equivalent structural payload types, checked for exact equality against the SDK in tests.
  • Give experimental_getWranglerCommands an explicit, dependency-free structural return type for its command registry and global flags, keeping internal command implementation types out of the published declarations.
  • Check the generated public declarations of both Wrangler and workers-utils for undeclared package dependencies and unpublished relative references.
  • Add regression coverage for shared binding and Undici identities, rejected dependency leaks, and valid shipped relative declarations. Include a patch changeset for Wrangler and workers-utils.

Validation

Maintainer review follow-up (12 October, 1ebe991 / 33b96e34)

The 9 October review was correct: duplicating registry type definitions would be a maintenance burden. Internal core/types.ts and the experimental command API now both import the same shapes from src/command-types.ts, covering metadata, identity, command data, behaviour, aliases, namespaces and recursive registry nodes. Internal Teams/category unions remain constrained.

The public ExperimentalCommandArgDefinition remains a deliberate Yargs-free structural projection, not a second copy of internal ArgDefinition: the internal one is derived from development-only Yargs declarations and exporting it would recreate the package-boundary leak. No command execution or runtime behaviour was changed.

New-head local validation (1ebe991): full Wrangler dependency build; source and E2E TypeScript compilations; 16 targeted experimental API/published-declaration tests; adjacent command-registration and completion tests; format, type-aware lint, and diff checks. The published declaration audit covered Wrangler (3 type entry points) and workers-utils (11 type entry points) with no unresolved imports. See GitHub Checks for new-head upstream CI.

Latest merged-main validation (33b96e34, 12 October): merged upstream main at 9be5ec7 to remove the existing merge conflict in core/types.ts. Preserved upstream's updated temporary-account/event-code documentation in the shared type module without restoring duplicated fields. Fresh frozen workspace install; full Wrangler dependent-package build (Wrangler 4.149.0); source and E2E TypeScript compilation with --noEmit; generated declaration audit across Wrangler (3 published type entry points) and workers-utils (11); four targeted suites (48 passed, 2 skipped, 0 failed); changed-file type-aware lint and diff validation all passed. The fork branch and worktree were clean after the merge commit. Upstream GitHub Actions has been triggered for this latest head and must independently pass before merge.

Bot review cross-check (12 October, 02f9a5ec)

Older Devin observations have also been rechecked against the current dependency strategy rather than relying on historical "resolved" labels:

  • Undici cross-package identity: reproduced locally on this commit using built Wrangler unstable_dev and an external installed Undici MockAgent / setGlobalDispatcher. Wrangler's fetch() returned the exact intercepted response and assertNoPendingInterceptors() passed. This is runtime evidence in addition to the declaration-level MockAgent type assertion.
  • cloudflare/workers-utils production dependency: an earlier automated note was marked resolved when workers-utils was temporarily a devDependency, but that condition is no longer true. The current runtime intentionally bundles workers-utils while its published types import the installed package to preserve INHERIT_SYMBOL's nominal identity. Removing the production dependency without restructuring the public declaration contract would reintroduce consumer type resolution errors. The present dependency is therefore deliberate and should be reviewed as a compatibility trade-off, not described as removed.
  • Declaration scope and argument metadata: the checker includes triple-slash references and shipped relative declarations, both Wrangler and workers-utils declaration graphs pass the audit, and the public argument type retains consumer-facing fields such as description, nargs, group, conflicts and implies without requiring Yargs at install time.
  • Changesets / preview package bots: release previews are informational, not test failures. The PR adds a patch changeset for Wrangler and workers-utils; workspace dependent release computation follows the repository's existing Changesets configuration.
  • Latest bot finding: the supportTemporary JSDoc default was restored in 02f9a5ec; TypeScript AST inspection confirms the recognised default false tag and Devin marked the finding resolved. All four targeted suites passed again (48 passed, 2 skipped).

The Cloudflare Wrangler code-owner approval and full upstream CI remain separate merge gates.

Pre-review validation (ee458de)

The pre-review candidate at ee458de matched the tested fork commit a2f952e6ef5b0d3b53b966602bbefb72b585a652; those results are historical rather than evidence for the latest head. Fork-only workflow files are excluded.

  • Full Wrangler dependency build, Wrangler type:tests, workers-utils type tests, changed-file type-aware lint, and formatting passed locally.
  • Targeted runtime tests: 23 workers-utils tests and 15 Wrangler tests passed.
  • Fresh production-only installs of the actual published ee458de packages with TypeScript 5.8.3, strict: true, and skipLibCheck: false removed all nine PR-specific declaration errors: utilities went from five errors to zero; Wrangler went from nineteen to ten. The remaining ten diagnostics exactly match the unchanged Miniflare-only control.
  • Consumer assertions preserve both directions of Binding, INHERIT_SYMBOL, and MockAgent compatibility, reject unrelated unique symbols, and check against accidental any.
  • The installed published Wrangler CLI returned version 4.148.0 successfully. Its installed files and the utilities files match their immutable prerelease archives byte for byte.
  • Published-runtime smoke: local Miniflare/workerd started through unstable_dev; Wrangler's fetch() then used an external Undici MockAgent as its global dispatcher, returned the exact mocked response, and consumed every registered interceptor.
  • Fork repository checks and targeted compatibility and official-shape Vite E2E: all passed.
  • Upstream repository checks and cross-platform package/fixture tests, Wrangler E2E, and Vite E2E passed on ee458de.

Checklist

  • 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: this repairs generated declarations without changing CLI behavior or configuration.
  • Patch changeset included

Devin Review

@changeset-bot

changeset-bot Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5a0fe56

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

This PR includes changesets to release 9 packages
Name Type
wrangler Patch
@cloudflare/workers-utils Patch
@cloudflare/vitest-plugin Patch
@cloudflare/autoconfig Patch
@cloudflare/cli-shared-helpers Patch
@cloudflare/containers-shared Patch
@cloudflare/deploy-helpers Patch
@cloudflare/remote-bindings Patch
@cloudflare/workers-auth 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 Oct 5, 2026
@workers-devprod
workers-devprod requested review from a team and dario-piotrowicz and removed request for a team October 5, 2026 23:38
@workers-devprod

workers-devprod commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • @cloudflare/wrangler
Show detailed file reviewers
  • .changeset/wrangler-public-types.md: [@cloudflare/wrangler]
  • packages/workers-utils/src/construct-wrangler-config.ts: [@cloudflare/wrangler]
  • packages/workers-utils/tests/construct-wrangler-config.test.ts: [@cloudflare/wrangler]
  • packages/workers-utils/tsup.config.ts: [@cloudflare/wrangler]
  • packages/wrangler/package.json: [@cloudflare/wrangler]
  • packages/wrangler/scripts/check-published-declarations.ts: [@cloudflare/wrangler]
  • packages/wrangler/scripts/deps.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/tests/experimental-commands-api.test.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/tests/published-declarations.test.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/complete.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/experimental-commands-api.ts: [@cloudflare/wrangler]
  • packages/wrangler/tsup.config.ts: [@cloudflare/wrangler]
  • pnpm-lock.yaml: [@cloudflare/wrangler]

devin-ai-integration[bot]

This comment was marked as resolved.

@dario-piotrowicz

Copy link
Copy Markdown
Member

Hi @AyobamiH, thank you for the PR 🙏

As you can see CI is very red, could you address it?

devin-ai-integration[bot]

This comment was marked as resolved.

@pkg-pr-new

pkg-pr-new Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
@cloudflare/autoconfig

npm i https://pkg.pr.new/@cloudflare/autoconfig@16074

@cloudflare/build-output-utils

npm i https://pkg.pr.new/@cloudflare/build-output-utils@16074

@cloudflare/codemods

npm i https://pkg.pr.new/@cloudflare/codemods@16074

@cloudflare/config

npm i https://pkg.pr.new/@cloudflare/config@16074

@cloudflare/containers-shared

npm i https://pkg.pr.new/@cloudflare/containers-shared@16074

create-cloudflare

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

@cloudflare/deploy-helpers

npm i https://pkg.pr.new/@cloudflare/deploy-helpers@16074

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-functions

npm i https://pkg.pr.new/@cloudflare/pages-functions@16074

@cloudflare/pages-shared

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

@cloudflare/runtime-types

npm i https://pkg.pr.new/@cloudflare/runtime-types@16074

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-plugin

npm i https://pkg.pr.new/@cloudflare/vitest-plugin@16074

@cloudflare/workers-auth

npm i https://pkg.pr.new/@cloudflare/workers-auth@16074

@cloudflare/workers-editor-shared

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

@cloudflare/workers-utils

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

wrangler

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

commit: 5a0fe56

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@AyobamiH
AyobamiH force-pushed the fix/15940-wrangler-declaration-boundary branch from 373a7e8 to 09d713e Compare October 6, 2026 16:43
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@dario-piotrowicz dario-piotrowicz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR @AyobamiH 🙏

I do believe that this would be introducing a bunch of duplicated code (as per my comment), could you have a look and see if this can be avoided? 🙏

import { createCLIParser } from "./index";
import type { DefinitionTreeNode } from "./core/types";

export type ExperimentalCommandMetadata = {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this type is just duplicating this already-existing type:

export type Metadata = {

I think this will make this code pretty problematic to maintain and keep in sync, I think we can just move the metadata type (and any other type you're re-defining in this file) in its own shared module and import it both into this file and into the original file.

ensuring that we're not duplicating code while still resolving the issue

would that be possible?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for pointing this out, Dario — agreed. Duplicating the internal registry types in the experimental API would make future changes prone to drift.

I've refactored this at 1ebe991 following your suggestion. Both core/types.ts and experimental-commands-api.ts now import the same definitions from command-types.ts for metadata, command identity, shared behaviour, command data, namespace/alias definitions and the recursive tree. The internal Teams/category constraints stay intact, and the published API remains self-contained.

The one intentional structural projection is the public argument-options type: the internal ArgDefinition is Yargs-derived, and exposing Yargs in Wrangler's published declarations would reintroduce the original private-dependency leak. I've documented that distinction and added a type-level regression test.

I validated the new head locally with a full dependent-package build, Wrangler source and E2E type checks, 16 targeted API/declaration tests, command-registration and completion tests, type-aware lint, formatting and the published-declaration audit for Wrangler and workers-utils. The upstream checks are running independently for this commit.

Could you take another look at the shared approach? Thanks again for the direction.

@workers-devprod

workers-devprod commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • @cloudflare/wrangler
Show detailed file reviewers
  • .changeset/wrangler-public-types.md: [@cloudflare/wrangler]
  • packages/workers-utils/src/construct-wrangler-config.ts: [@cloudflare/wrangler]
  • packages/workers-utils/tests/construct-wrangler-config.test.ts: [@cloudflare/wrangler]
  • packages/workers-utils/tsup.config.ts: [@cloudflare/wrangler]
  • packages/wrangler/package.json: [@cloudflare/wrangler]
  • packages/wrangler/scripts/check-published-declarations.ts: [@cloudflare/wrangler]
  • packages/wrangler/scripts/deps.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/tests/experimental-commands-api.test.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/tests/published-declarations.test.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/command-types.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/complete.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/core/types.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/experimental-commands-api.ts: [@cloudflare/wrangler]
  • packages/wrangler/tsup.config.ts: [@cloudflare/wrangler]
  • pnpm-lock.yaml: [@cloudflare/wrangler]

devin-ai-integration[bot]

This comment was marked as resolved.

@workers-devprod

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • @cloudflare/wrangler
Show detailed file reviewers
  • .changeset/wrangler-public-types.md: [@cloudflare/wrangler]
  • packages/workers-utils/src/construct-wrangler-config.ts: [@cloudflare/wrangler]
  • packages/workers-utils/tests/construct-wrangler-config.test.ts: [@cloudflare/wrangler]
  • packages/workers-utils/tsup.config.ts: [@cloudflare/wrangler]
  • packages/wrangler/package.json: [@cloudflare/wrangler]
  • packages/wrangler/scripts/check-published-declarations.ts: [@cloudflare/wrangler]
  • packages/wrangler/scripts/deps.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/tests/experimental-commands-api.test.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/tests/published-declarations.test.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/command-types.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/complete.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/core/types.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/experimental-commands-api.ts: [@cloudflare/wrangler]
  • packages/wrangler/tsup.config.ts: [@cloudflare/wrangler]
  • pnpm-lock.yaml: [@cloudflare/wrangler]

@clanky-the-clanker

Copy link
Copy Markdown

This pull request was not reviewed automatically because its author does not have write access to this repository. A maintainer with write access can start a review by commenting clanky review (or clanker review or @clanky-the-clanker review), optionally followed by instructions (e.g. clanky review focus on the retry logic).

@AyobamiH

Copy link
Copy Markdown
Author

@dario-piotrowicz — following up on your 9 October review: the shared-type refactor is in place (command-types.ts is imported by both core/types.ts and experimental-commands-api.ts; see my inline reply from 12 October).

I've also reconciled the PR with the latest upstream main (0d826c6a) and pushed GitHub-verified signed merge commit 5a0fe56d. The sole conflict was pnpm-lock.yaml: the resolution keeps upstream's newer Workers Types version and the PR's production workers-utils dependency, without the obsolete dev-dependency entry.

On this exact head, the frozen pnpm lockfile check and workspace dependency installation (scripts disabled) passed, as did the targeted tests: 16/16 Wrangler and 23/23 workers-utils. Upstream CI is running independently and has not yet fully completed.

Could you give the shared-type approach another look after the checks settle? I'll leave approval and merge to you and the Wrangler code owners. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

3 participants