Skip to content

fix(add): forward --allow-build to global installs#620

Merged
jdx merged 1 commit into
mainfrom
claude/bold-bassi-f680c2
May 11, 2026
Merged

fix(add): forward --allow-build to global installs#620
jdx merged 1 commit into
mainfrom
claude/bold-bassi-f680c2

Conversation

@jdx

@jdx jdx commented May 11, 2026 •
edited by cursor Bot
Loading

Copy link
Copy Markdown
Owner

Summary

  • aube add --global built a synthetic AddArgs with allow_build: Vec::new(), silently dropping the outer flag. Under strictDepBuilds=true the install then errored with "must be reviewed before install" even when the user explicitly approved the dep on the CLI.
  • Plumb allow_build through run_global / run_global_inner so the inner run() writes approvals into the throwaway install dir's package.json#aube.allowBuilds before lifecycle scripts run. No workspace yaml exists in that throwaway dir, so the writer falls through to the aube.allowBuilds namespace in package.json — the same shape aube approve-builds -g already produces (covered by the existing aube approve-builds -g --all test).
  • Added a bats regression in test/global_install.bats that runs under AUBE_STRICT_DEP_BUILDS=true and asserts the build actually fires (the marker dep's postinstall writes aube-builds-marker.txt).

Closes https://github.com/endevco/aube/discussions/617.

Test plan

  • cargo build -p aube
  • cargo clippy --all-targets -- -D warnings
  • cargo fmt --check
  • cargo test -p aube --bin aube (473 passed)
  • mise run test:bats test/global_install.bats (14/14 passed, including the new regression)

This PR was generated by Claude.

Note

Medium Risk
Changes global install argument plumbing for lifecycle-script approvals; a bug here could unintentionally allow/deny dependency build scripts during aube add -g under strictDepBuilds.

Overview
Fixes aube add -g to preserve and forward any --allow-build=<pkg> flags into the inner synthetic AddArgs, so global installs correctly pre-approve dependency lifecycle scripts instead of dropping the approval.

Adds a Bats regression test covering AUBE_STRICT_DEP_BUILDS=true to ensure aube add -g --allow-build=... both succeeds without review errors and actually runs the approved package’s build (marker file is created and approval is written to the global install package.json).

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

@jdx @claude
fix(add): forward --allow-build to global installs
d532e95 
`aube add --global` built a synthetic `AddArgs` with
`allow_build: Vec::new()`, silently dropping the outer flag. Under
`strictDepBuilds=true` the install then errored with "must be reviewed
before install" even when the user explicitly approved the dep on the
CLI. Plumb `allow_build` through `run_global` / `run_global_inner` so
the inner `run()` writes approvals into the throwaway install dir's
`package.json#aube.allowBuilds` before lifecycle scripts run.

Closes #617.

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

greptile-apps Bot commented May 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Fixes a silent parameter drop in aube add --global: the synthetic AddArgs built inside run_global_inner hardcoded allow_build: Vec::new(), discarding any --allow-build=<pkg> flags the user passed on the CLI. Under strictDepBuilds=true this caused an immediate "must be reviewed before install" error despite the user having explicitly approved the package.

  • The fix plumbs allow_build through run_global → run_global_inner → AddArgs with a straightforward ownership-move chain, matching how lockfile, network, and virtual_store are already forwarded.
  • A Bats regression test under AUBE_STRICT_DEP_BUILDS=true verifies that the build runs (marker file is created) and that the approval is written to the throwaway install dir's package.json.

Confidence Score: 5/5

Safe to merge — the change is a minimal, targeted plumbing fix with no side effects on non-global install paths.

The only code touched is the parameter-forwarding path for --allow-build in global installs. The fix correctly moves ownership of the Vec<String> through run_global → run_global_inner → AddArgs, consistent with how lockfile, network, and virtual_store are already threaded through. All other AddArgs fields remain unchanged, and a dedicated regression test under AUBE_STRICT_DEP_BUILDS=true covers the exact failure mode described in the bug report.

No files require special attention.

Important Files Changed

Filename Overview
crates/aube/src/commands/add.rs Adds allow_build: Vec<String> parameter to run_global and run_global_inner, forwarding it into the synthetic AddArgs instead of hard-coding Vec::new(); minimal, correct ownership-move change.
test/global_install.bats New Bats regression test for the --allow-build forwarding fix; consistent with the existing test helper patterns in the file and properly isolated via per-test AUBE_HOME.

Reviews (1): Last reviewed commit: "fix(add): forward --allow-build to globa..." | Re-trigger Greptile

@jdx jdx merged commit 05d21f6 into main May 11, 2026
19 checks passed
@jdx jdx deleted the claude/bold-bassi-f680c2 branch May 11, 2026 22:36
@greptile-apps greptile-apps Bot mentioned this pull request May 11, 2026
Merged
@chenrui333 chenrui333 mentioned this pull request May 12, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jdx