Skip to content

fix(config): don't exclude workspace members from deploy file patterns#33562

Merged
bartlomieju merged 5 commits intomainfrom
fix/deploy-config-workspace-files
Apr 28, 2026
Merged

fix(config): don't exclude workspace members from deploy file patterns#33562
bartlomieju merged 5 commits intomainfrom
fix/deploy-config-workspace-files

Conversation

@avocet-bot
Copy link
Copy Markdown
Contributor

@avocet-bot avocet-bot commented Apr 27, 2026

Problem

When deno deploy runs from a workspace root, WorkspaceDirectory::to_deploy_config() calls append_workspace_members_to_exclude() which adds all workspace member directories to the exclude list. This causes the deploy CLI to upload zero workspace member files (empty manifest), resulting in errors like:

Working directory at 'packages/backend' not found

Root Cause

The append_workspace_members_to_exclude() call was added in deploy-cli#75 (shipped in @deno/deploy@0.0.93) when file collection moved from TypeScript walk() to the Rust FileCollector backed by deno_config::to_deploy_config(). This exclusion pattern is correct for lint/fmt/test/bench/publish configs (each workspace member handles its own config), but NOT for deploy — the entire workspace needs to be uploaded.

Fix

Remove the append_workspace_members_to_exclude() call from to_deploy_config() so workspace member files are included in the deploy upload manifest.

Test Plan

Added 4 new tests in libs/config/workspace/mod.rs:

  1. test_deploy_config_in_root_does_not_exclude_members — deploy config in workspace root with multiple members; verifies member directories and files are not excluded
  2. test_deploy_config_in_both_root_and_member — deploy config in both root and member; verifies member files are included and member config takes precedence
  3. test_deploy_config_root_only_no_member_deploy — deploy:{org,app} in root only, no deploy in any member; verifies all 3 member directories are accessible
  4. test_deploy_config_with_exclude — deploy config with top-level exclude still works, but workspace members are not additionally excluded

All 140 existing deno_config tests continue to pass.

When `deno deploy` runs from a workspace root, `to_deploy_config()` was
calling `append_workspace_members_to_exclude()` which excluded all workspace
member directories from the file patterns. This caused the CLI to upload
zero workspace member files (empty manifest), resulting in 'Working directory
not found' errors for monorepo deployments.

This exclusion makes sense for lint/fmt/test/bench/publish configs (each
member handles its own), but NOT for deploy — the entire workspace needs to
be uploaded.

The fix removes the `append_workspace_members_to_exclude` call from
`to_deploy_config`, so workspace member files are included in the upload.

Regression introduced in deploy-cli 0.0.93 (commit 92610a9, 2026-03-13)
which moved file collection from TypeScript `walk()` to the Rust
`FileCollector` backed by `deno_config::to_deploy_config()`.

Fixes denoland/deploy-cli#XX
Ref: TICKET-20260417-55060
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 27, 2026

CLA assistant check
All committers have signed the CLA.

Comment thread libs/config/workspace/mod.rs Outdated
Comment thread libs/config/workspace/mod.rs Outdated
Signed-off-by: Yusuke Tanaka <wing0920@gmail.com>
Signed-off-by: Yusuke Tanaka <wing0920@gmail.com>
Copy link
Copy Markdown
Contributor

@fibibot fibibot left a comment

Choose a reason for hiding this comment

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

The substantive change is right: to_deploy_config is the only one of the per-config builders where excluding workspace member directories is wrong, because deploy collects the whole monorepo for upload (lint/fmt/test/bench/publish each scope to one member at a time and consult the member's own config). Removing append_workspace_members_to_exclude(...) from line 2426 is the minimal, correct fix. The four regression tests cover the main cases — root-only deploy, root+member with member-takes-precedence, root-only-no-member-deploy, and exclude interaction.

Two things before this is mergeable:

  1. CI lint/format is failing on the test code formatting — the assert-blocks where you wrote deploy_config\n .files\n .matches_path(...) need to be on a single chained call (deploy_config.files.matches_path(...)). The format check at https://github.com/denoland/deno/actions/runs/24982179189/job/73146771467 lists each one. A fresh ./tools/format.js (or dprint fmt) should fix all of them in one pass. The latest commits removed the XXXXX placeholder but didn't re-run formatting.

  2. One logic case I want you to confirm — see inline.

Comment thread libs/config/workspace/mod.rs Outdated
self.exclude_includes_with_member_for_base_for_root(&mut config.files);
combine_files_config_with_cli_args(&mut config.files, cli_args);
self.append_workspace_members_to_exclude(&mut config.files);
// NOTE: Unlike lint/fmt/test/bench/publish configs, deploy should NOT
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worth covering: when deno deploy is run from the workspace root and a workspace member has its own deploy: { ... } block, what's the expected behavior?

  • to_deploy_config_inner takes member_config from self.deno_json.member.to_deploy_config(). From root, that's the root deno.json itself, so member_config.files is the root's file patterns (which after this fix include all members).
  • The combined result will include every member's files in one upload.
  • A member that explicitly declared its own deploy config (e.g. deploy: { app: "frontend" }) gets its files swept into the root's deploy upload anyway.

Is that the intended behavior, or should a member that opts into its own deploy config be excluded from a root-level deploy (so it can be deployed separately)? The existing test test_deploy_config_in_both_root_and_member only runs from the member directory, so it doesn't exercise this path. The pre-PR behavior happened to exclude all members unconditionally (broken in the simple case, but accidentally also handled this case). Now the simple case works; this edge case may need its own handling.

Not a blocker — could be a follow-up if you confirm it's intended that root-level deno deploy always sweeps everything in.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good callout. Yes, this is intentional — it matches pre-0.0.93 behavior where TypeScript walk() uploaded everything under workspace root.

The member-with-its-own-deploy-config case is real but: (1) root-level deploy doesn't know about member-specific deploy targets, (2) uploading extra files is wasteful but not incorrect since the server uses entrypoint + working directory, (3) users can scope with include.

Handling this properly (exclude members that declare their own deploy config from root-level deploys) is a good follow-up but shouldn't block this regression fix.

Comment thread libs/config/workspace/mod.rs Outdated
);
assert!(
deploy_config
.files
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This (and the analogous blocks at ~6688, ~6764, ~6790, ~6798) is the formatting CI is rejecting. dprint wants the .files.matches_path(...) chain folded onto a single call. Re-running ./tools/format.js from the repo root should auto-fix all of them.

@avocet-bot
Copy link
Copy Markdown
Contributor Author

Addressed all review feedback in ea3e9ab:

  1. Removed verbose comment block (magurotuna's suggestions were already applied via GitHub UI — I removed the remaining lines)
  2. Fixed formatting — ran rustfmt which reformatted the assert chains that CI was complaining about
  3. Replied to fibibot's edge case question inline — the root-level-deploy-sweeps-everything behavior is intentional (matches pre-0.0.93), and scoping via member-specific deploy exclusion is a good follow-up

All 140 deno_config tests pass.

@magurotuna magurotuna requested a review from fibibot April 28, 2026 05:13
Copy link
Copy Markdown
Contributor

@fibibot fibibot left a comment

Choose a reason for hiding this comment

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

LGTM after the cleanup. The substantive fix is unchanged from my prior review (removing append_workspace_members_to_exclude from to_deploy_config so workspace members aren't excluded from the upload manifest), and the four regression tests still cover the four key cases (root-only deploy, root+member with member precedence, root-only with no member deploy, exclude-with-deploy-config).

Both my prior concerns are addressed:

  • Format failures resolvedlint debug now green on all 3 platforms (was red before with the multi-line .files.matches_path(...) chains that dprint wanted folded). Author ran tools/format.js.
  • XXXXX placeholder comment removed from the source-level comment in to_deploy_config.

CI satisfies the gate for the change's relevant areas: 95/n passing, zero fail. test libs debug (the cargo test job that exercises libs/config/workspace/mod.rs's new tests) green on linux-aarch64 and macos-aarch64, windows-x86_64 still pending. Lint debug clean on all 3 platforms. The remaining pending Windows tests aren't gating for this Rust-only change.

The cross-member-deploy edge case from my prior review (running root-level deploy when a member has its own deploy block) is still uncovered, but that's pre-existing scope and a reasonable follow-up.

@bartlomieju bartlomieju merged commit 304a644 into main Apr 28, 2026
136 checks passed
@bartlomieju bartlomieju deleted the fix/deploy-config-workspace-files branch April 28, 2026 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants