Skip to content

fix(deploy): upload Build Output modules under their manifest paths - #393

Merged
MattieTK merged 4 commits into
cloudflare:mainfrom
MattieTK:fix/python-vendored-module-names
Oct 23, 2026
Merged

MattieTK merged 4 commits into
cloudflare:mainfrom
MattieTK:fix/python-vendored-module-names

Conversation

@MattieTK

@MattieTK MattieTK commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

Fixes #392

What

assembleBuildResult() (packages/cli/src/lib/deploy-input.ts) prefixed every non-entrypoint Build Output module with ./ when turning the manifest into upload modules. This change uses the manifest path as-is, which is what Wrangler uploads.

Why

Python Workers use module names as file paths without normalising them, and import vendored packages from python_modules/. JavaScript module names are normalised, which is why the prefix went unnoticed. With the prefix, pywrangler sync's vendored workers SDK was uploaded as ./python_modules/workers/__init__.py, so cf deploy of any Python Worker that imports the SDK failed validation:

ModuleNotFoundError: No module named 'workers'
You need to update to workers-py >= 1.9 or to pass disable_python_external_sdk [code: 10021]

I captured both tools' uploads against a fake API: the multipart parts are identical in content and content type, and the part names are the only difference (python_modules/... from Wrangler, ./python_modules/... from cf). Full write-up in #392.

The prefix was also inconsistent within cf itself: the main module was already uploaded unprefixed (index.js), while siblings were ./lib.cjs, ./app.py and so on.

Testing

  • Updated the existing assertions in deploy-input.test.ts and commands/deploy/index.test.ts that expected ./-prefixed names.
  • Added keeps Python vendored module paths exactly as in the manifest to deploy-input.test.ts.
  • deploy-input, deploy/index, previews-deploy and workers-check tests pass. workers check runs Workers that import "./message.js" / require("./helper.cjs"), so relative imports still resolve with unprefixed module names.
  • Full run on Linux (WSL2 Ubuntu 22.04, Node 24.22.1):
    • CLI tests: 1948 passed, 1 failed. The failure is cf dev > happy path > reports missing project commands and restores signal listeners, which fails the same way on main (8e51747) in that environment.
    • check:lint, check:format and check:type pass.
  • End to end, against a real account, with the same one-line change applied to the published 1.0.0-beta.20 bundle:
    • Python hello world (vendored workers SDK): failed with 10021 before; now deploys and returns 200 Hello from a Python Worker.
    • Rust hello world (worker-build, Wasm + JS shim): still deploys and returns 200 Hello from a Rust Worker.

Includes a patch changeset.

🤖 Generated with Claude Code

assembleBuildResult() prefixed every non-entrypoint module with "./".
Python Workers only install vendored packages whose module names start
with "python_modules/", so `cf deploy` of a Python Worker importing the
Workers SDK failed with 10021 "No module named 'workers'". Use the
manifest path as-is, matching Wrangler.

Fixes cloudflare#392

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pkg-pr-new

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

Copy link
Copy Markdown
npm i https://pkg.pr.new/cf@393

commit: 461c643

@clanky-the-clanker

clanky-the-clanker Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

💬 Approved with comments — The one-line fix in deploy-input.ts is correct, matches how the main module is already uploaded, and comes with updated and new tests and a patch changeset. One INFO note about the unrelated AGENTS.md policy addition.

Findings: 1 info

The decision is advisory: this comment never approves, blocks or requests changes on the pull request.

🔧 Fix with your agent

Paste the following into your coding agent to investigate and fix these issues.

Fix the following code review issues found in pull request #393.

Before making changes, review each issue and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g., the fix would require disproportionate changes
  for minimal ROI, or the finding is factually incorrect), explain why
- If you need to clarify something before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order

## INFO (1)

### [Documentation] AGENTS.md adds a repository-wide "Module Workers only" policy that tells review not to flag service-worker regressions. This is unrelated to the Python module-name fix; consider confirming it with maintainers or moving it to its own pull request.
- **File:** AGENTS.md:418

📝 Documentation: 💬 1 info

Changeset is accurate; one note on the AGENTS.md addition

Recommendation: 💬 Optional Nits

  • [INFO] AGENTS.md adds a repository-wide "Module Workers only" policy that tells review not to flag service-worker regressions. This is unrelated to the Python module-name fix; consider confirming it with maintainers or moving it to its own pull request. (AGENTS.md:418)
View full review

The changeset ("cf": patch) has an imperative title. Its body correctly names the affected commands (cf deploy, cf workers versions create, cf previews deploy) and gives the user-visible error (10021).

The AGENTS.md addition ("Module Workers only") states a repository-wide support policy: service-worker format is unsupported, and service-worker regressions should not be flagged in review. That goes beyond this fix. It is reasonable if maintainers agree with it, but a policy change like this is easier to discuss and find later in its own pull request, or at least with an explicit mention in this description.


✅ All other checks passed

📋 Code Quality: ✅ LGTM

The change in packages/cli/src/lib/deploy-input.ts uploads each non-entrypoint module under its manifest key as-is, without adding a ./ prefix. The main module was already uploaded this way.

  • Path safety: validateModulePath still checks the unprefixed name before the file is read, so the change opens no path-traversal gap.
  • Key consistency: the sourcemap branch already used the raw name, so every module type now uses the manifest key.
  • Tests: the assertions that expected ./-prefixed names were updated in deploy-input.test.ts and commands/deploy/index.test.ts, and a regression test for python_modules/... paths was added. No other test expects ./-prefixed upload names.
  • Cross-platform: manifest keys are used as-is and never go through a path join before upload, so Windows path separators can't end up in module names.
  • Changeset: a patch changeset is included, as the repository requires for a user-facing fix.

Reviewed commit: 461c6435

@MattieTK
MattieTK marked this pull request as draft October 23, 2026 11:26
Python Workers use module names as file paths without normalising them,
so a `./python_modules/...` package is not on the import path. The
previous text described a prefix-based install step that does not exist,
and named only `cf deploy` although `cf workers versions create` and
`cf previews deploy` share the fix.
Reviews of the Build Output upload names flagged a regression for
service-worker format Workers. That format is deprecated and cf does
not support it, so record this to stop future reviews designing for it.
The comments described a prefix-based install step that does not exist.
The runtime normalises `./` in JavaScript module names but uses Python
module names as file paths, so a `./python_modules/...` package is not
on `sys.path`.
@petebacondarwin
petebacondarwin requested review from a team and removed request for dimitropoulos and srikrsna October 23, 2026 12:59
@MattieTK
MattieTK marked this pull request as ready for review October 23, 2026 13:17
@MattieTK
MattieTK added this pull request to the merge queue Oct 23, 2026
Merged via the queue into cloudflare:main with commit 068bde2 Oct 23, 2026
10 checks passed
@MattieTK
MattieTK deleted the fix/python-vendored-module-names branch October 23, 2026 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants