Skip to content

fix(deploy): bundle workspace siblings and file: deps; add --no-prod#507

Merged
jdx merged 7 commits intomainfrom
claude/magical-saha-130da5
May 5, 2026
Merged

fix(deploy): bundle workspace siblings and file: deps; add --no-prod#507
jdx merged 7 commits intomainfrom
claude/magical-saha-130da5

Conversation

@jdx
Copy link
Copy Markdown
Contributor

@jdx jdx commented May 3, 2026

Summary

Fixes three real bugs and one missing flag in aube deploy, all reported in discussion #345:

  • workspace:* deps tried to fetch from the registry. Deploy rewrote workspace:* → concrete version and asked install to resolve it — fine for published siblings, broken for the (very common) unpublished-sibling case. Now siblings reachable from the deployed package get bundled into <target>/.aube-deploy-injected/<id>/ and the manifest spec becomes a relative file: pointer at the staged copy. Recursion handles siblings whose own deps are workspace siblings.
  • file: deps resolved relative to the deploy output dir. A file:../local-vendor spec stays in the deployed manifest unchanged, so it now points at <target>/../local-vendor instead of the source workspace's local-vendor. Same bundling pipeline as workspace siblings — copy the target into the staging dir, rewrite the spec.
  • No way to opt out of --prod. Added --no-prod (mutually exclusive with --prod / --dev) for deploys that need devDependencies at runtime — test-harness deploys, build-step staging. Combine with --no-optional to drop optionals while keeping prod + dev.
  • Drift between manifest strip, lockfile subset, and install dep-selection. Three call-sites computed the same prod/dev/optional axis by hand. Consolidated into StripFields::for_args / dep_selection_for_args / keep_dep_for_args so they can't drift.

Bundled siblings always have devDependencies stripped (a deploy is a runtime artifact, not a dev environment for siblings). When bundling happens, the lockfile-subset path is skipped — the rewritten file: pointers don't appear in the source lockfile, so a frozen install would immediately read as drifted.

Out of scope: the no-symlinks deploy mode the comment also mentions — that's a substantial design conversation, not a bug fix.

Test plan

  • cargo test -p aube --bin aube commands::deploy — 18 unit tests covering strip/dep-selection axes, file-spec rewriting (directory, tarball, sibling-in-injected-dir), unique-id collision handling
  • mise run test:bats test/deploy.bats — 16 bats tests, including new coverage for sibling bundling, file: dep bundling, recursive sibling chains, --no-prod, and --prod default stripping
  • cargo clippy --all-targets -- -D warnings clean
  • cargo fmt --check clean
  • Manual smoke test: deploy a workspace package whose workspace:* sibling chain is two levels deep + a file: dep + a devDep, verified node resolves every dep at runtime

🤖 Generated with Claude Code


Note

Medium Risk
Changes deploy artifact composition by bundling workspace:/file:/link: dependencies and rewriting manifests, which could affect runtime resolution and lockfile behavior across monorepos. Risk is moderated by extensive new unit and bats coverage but touches core packaging/install flow.

Overview
aube deploy now bundles local dependencies instead of rewriting workspace:* to a version and relying on the registry: any reachable workspace: siblings and file:/link: targets are copied into <target>/.aube-deploy-injected/<id>/ and manifests are rewritten to relative file: specs (including recursive sibling chains and back-references to the deployed package).

Adds --no-prod to deploy all dependency kinds (prod+dev+optional) and centralizes dependency filtering so manifest stripping, install DepSelection, and lockfile subsetting stay consistent; when bundling occurs, lockfile subsetting is skipped to avoid drift. CLI docs/JSON and bats tests are updated/expanded to cover the new behavior.

Reviewed by Cursor Bugbot for commit cac5a84. Bugbot is set up for automated code reviews on this repo. Configure here.

jdx and others added 2 commits May 3, 2026 16:14
`aube deploy` previously rewrote `workspace:*` to a concrete version
and then asked install to resolve it from the registry — fine for
published siblings, broken for the (very common) unpublished-sibling
case. `file:` deps were left untouched, so a `file:../local-vendor`
spec resolved relative to the deploy output dir instead of the source
workspace. There was also no flag to opt out of the implicit `--prod`
default, so deployments that wanted devDependencies (test-harness
deploys, build-step staging) had no way to ask for them.

Now deploy bundles every `workspace:` / `file:` / `link:` ref the
deployed package reaches into `<target>/.aube-deploy-injected/<id>/`
and rewrites the manifest specs to relative `file:` pointers at the
staged copies. Recursion handles siblings whose own deps are workspace
siblings — a bundled sibling's manifest references other bundled
siblings as `file:../<id>` within the injected dir. `file:` directory
targets get the same treatment; `file:` tarballs ship as opaque
archives. Bundled siblings always have devDependencies stripped (a
deploy is a runtime artifact, not a dev environment for siblings).

`--no-prod` opts out of the production default and deploys every dep
kind; combine with `--no-optional` to drop optionals while keeping prod
+ dev. `StripFields::for_args`, `dep_selection_for_args`, and
`keep_dep_for_args` share one axis so the manifest rewrite, the
lockfile subset, and the install dep-selection agree on which dep
types survive.

Reported in #345 (comment).
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 3, 2026

Greptile Summary

  • Replaces the previous "rewrite workspace:* to concrete semver" approach with a full local-dep bundling pipeline: plan_injections BFS-discovers all reachable workspace:/file:/link: targets, materialize_injections copies them under <target>/.aube-deploy-injected/<id>/, and rewrite_local_refs rewrites each manifest to relative file: specs — fixing the three reported deploy bugs.
  • Adds --no-prod flag and consolidates the prod/dev/optional axis into DepAxis::for_args, with StripFields::for_args and keep_dep_for_args deriving from it; dep_selection_for_args is intentionally not derived and relies on from_flags(false,false,false) == All for the --no-prod case (previously flagged risk).
  • The --no-prod bats test checks manifest rewriting and sibling copy but is missing an assert_dir_exists out/node_modules/@test/app assertion, so an install-side regression in --no-prod dep selection would only be caught by unit tests, not the integration suite.

Confidence Score: 4/5

Safe to merge; the only new finding is a missing node_modules assertion in one bats test (P2).

No new P0 or P1 issues found beyond those already under discussion. The implementation is well-structured and the previously-flagged bugs are addressed. The sole new finding is a P2 test coverage gap in the --no-prod bats test.

crates/aube/src/commands/deploy.rs — dep_selection_for_args fragility (previously flagged); test/deploy.bats — --no-prod test missing node_modules assertion.

Important Files Changed

Filename Overview
crates/aube/src/commands/deploy.rs Core deploy logic: adds plan_injections/materialize_injections/rewrite_local_refs pipeline to bundle workspace/file:/link: deps, centralises prod/dev/optional axis into DepAxis, adds --no-prod flag; previously-reviewed bugs fixed (tarball guard, back-ref guard, peer file: error); remaining concern is dep_selection_for_args relying on implicit from_flags(false,false,false)==All semantic for --no-prod.
test/deploy.bats Adds 6 new bats tests covering sibling bundling, file: bundling, recursive chains, --no-prod, --prod default stripping; --no-prod test is missing a node_modules assertion that would catch install-side dep-selection regressions.
aube.usage.kdl Adds --no-prod flag definition with correct mutual-exclusion description and long_help.
docs/cli/commands.json Generated docs entry for --no-prod; matches aube.usage.kdl definition.
docs/cli/deploy.md Adds --no-prod flag documentation section with accurate description and flag semantics.

Fix All in Claude Code

Reviews (6): Last reviewed commit: "fix(deploy): extend back-ref guard to fi..." | Re-trigger Greptile

Comment thread crates/aube/src/commands/deploy.rs
Comment thread test/deploy.bats Outdated
Comment thread crates/aube/src/commands/deploy.rs
… refute

Three review fixups on the deploy bundling PR:

- workspace-sibling injections stored the raw `sibling_dir` from
  ws_index in `Injection.source_dir`, but `materialize_injections`
  compared it canonicalized — under symlinks the equality never
  held, so `deployAllFiles=true` silently fell back to pack's
  selection for siblings. Store + enqueue the canonical path so
  the workspace-sibling case mirrors the file:/link: case.
- the `--prod default` bats test had a no-op `[ ! -d ... ] || run cat`
  that always succeeded. Replace with an explicit version refutation
  that only asserts is-number@7.x is absent (transitive 3.x is fine).
- drop the unused `_canonical` arg from `unique_id` — disambiguation
  is by counter, not by path, and the parameter just advertised
  intent that wasn't implemented.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread crates/aube/src/commands/deploy.rs
The three deploy helpers — `StripFields::for_args`, `dep_selection_for_args`,
and `keep_dep_for_args` — claimed to "stay in lockstep" but each rederived
the prod/dev/optional axis with a slightly different formula. They happened
to produce equivalent observable output today (the manifest strip drops
optionals under `--dev`, so install never sees them), but the formulas
were a drift hazard.

Extract a single `DepAxis::for_args` source of truth and have all three
consumers destructure from it. As a side effect, `--dev` alone now maps
to `DepSelection::DevNoOptional` rather than `Dev` — observationally
identical because the stripped manifest has no optionals to walk, but
the install state and tracing label now match what actually happens.

Lock the (dev, no_prod, no_optional) -> DepSelection table down with a
matrix test so future drift is caught at compile/test time.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread crates/aube/src/commands/deploy.rs
Comment thread crates/aube/src/commands/deploy.rs
Two bugs surfaced by review on PR #507:

- Tarball injections crashed the rewrite loop. `materialize_injections`
  copies tarballs as opaque archives — no `package.json` lands under
  `inj.target_dir` — but the follow-up `rewrite_local_refs` loop ran
  unconditionally, failing with "No such file or directory" the moment
  any `file:./pkg.tgz` dep appeared. Skip `inj.is_tarball` entries to
  match the materialize side's `continue`.

- Workspace `peerDependencies` shipped raw `workspace:*` to the install
  layer. The previous rewrite path resolved every `workspace:` spec
  (including peers) to a concrete semver via `resolve_workspace_spec`;
  the new bundling pipeline only rewrites peer specs whose sibling is
  in the injection plan, and peers don't get bundled (bundling walks
  deps/devDeps/optionalDeps only). The fall-through left a raw
  `workspace:*` string the standalone install layer cannot parse.
  Restore `resolve_workspace_spec` and call it on the peer-not-bundled
  branch so peer specs become a parseable `1.2.3` / `^1.2.3` / `~1.2.3` /
  literal-suffix range.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread crates/aube/src/commands/deploy.rs
Comment thread crates/aube/src/commands/deploy.rs
Comment thread crates/aube/src/commands/deploy.rs
Three issues surfaced by review on PR #507, each fixed independently:

1. (P1) Sibling back-deps on the deployed package duplicated it.
   When sibling B declared `"@deployed-pkg": "workspace:*"`, BFS bundling
   would inject the deployed package's own source under
   `.aube-deploy-injected/@deployed-pkg/`, yielding two copies at
   runtime and breaking module-singleton assumptions
   (`require('@deployed-pkg')` from root vs from B returned distinct
   instances). Skip the deployed package's canonical path in
   `plan_injections`, and rewrite back-refs in the manifest rewriter
   to a relative `file:` pointer at the deploy root.

2. (P1) `file:`/`link:` peer specs were silently left unrewritten.
   The plan-miss arm `continue`d for peerDependencies file:/link:
   refs, leaving relative paths that resolve nowhere under the
   deploy target. Hard-fail with a clear message instead — peer
   file: refs aren't a sane shape for a self-contained deploy.

3. (Medium) `--dev` install dep_selection regressed to DevNoOptional.
   The earlier dep-axis unification folded `--dev` into the install-side
   optional axis, contradicting the explicit warning at lines 252–258:
   stripping transitive optional sub-deps of devDependencies (e.g.
   optional sub-deps of `jest`) breaks dev tooling. Decouple
   `dep_selection_for_args` from `DepAxis` — the manifest strip and
   lockfile subset legitimately drop direct optionalDependencies
   under `--dev` (matches pnpm), but install's resolved-graph filter
   must not. Restore `Dev` for `--dev` alone, document the asymmetry,
   and update the matrix test.

Bundles `deployed_canonical` + `target_root` into a `DeployRoot` to
keep `rewrite_local_refs`'s arg count under clippy's 7-arg limit, and
adds three unit tests (back-ref to deploy root, file: peer error,
the corrected --dev cell).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4384a59. Configure here.

Comment thread crates/aube/src/commands/deploy.rs
Cursor caught the missing twin to the previous back-ref fix: the
`workspace:*` branch in `plan_injections` and `rewrite_local_refs`
both check `canonical == deployed_canonical` to avoid duplicating the
deployed package, but the `file:`/`link:` Directory/Link branches
didn't. A sibling reaching the deployed package via
`file:../deployed-pkg` (instead of `workspace:*`) would still inject a
duplicate copy under `.aube-deploy-injected/`, breaking the same
runtime singleton invariant.

Mirror the `workspace:` guards on the file:/link: branches:
- plan_injections: skip the deployed canonical path before inserting.
- rewrite_local_refs: emit `file:<rel-to-target-root>` for back-refs
  before falling through to the `plan.get` lookup.

Adds a unit test covering the file:-based back-ref shape.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jdx jdx merged commit ea99ae6 into main May 5, 2026
18 checks passed
@jdx jdx deleted the claude/magical-saha-130da5 branch May 5, 2026 17:11
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.

1 participant