Skip to content

fix(cli): skip verify-deps inside lifecycle scripts#538

Merged
jdx merged 3 commits intomainfrom
fix/lifecycle-verify-deps-skip
May 7, 2026
Merged

fix(cli): skip verify-deps inside lifecycle scripts#538
jdx merged 3 commits intomainfrom
fix/lifecycle-verify-deps-skip

Conversation

@jdx
Copy link
Copy Markdown
Contributor

@jdx jdx commented May 7, 2026

Summary

  • Real bug fix: closes the verify-deps-before-run deadlock/hard-fail in nested aube run from inside lifecycle scripts. With verifyDepsBeforeRun=error, the inner call hard-failed on missing install state (parent preinstall runs before state is written); with verifyDepsBeforeRun=install, it deadlocked on the project lock the outer install holds. ensure_installed now returns early when npm_lifecycle_event is set in the env, matching npm/pnpm's "no verify-deps inside lifecycle scripts" contract.
  • Test ports: lifecycleScripts.ts:179 (install scripts that run other scripts can cause an infinite loop with verify-deps-before-run=install pnpm/pnpm#8954, --config.verify-deps-before-run=error), :200 (infinite loop with a postinstall task and verifyDepsBeforeRun: install pnpm/pnpm#10060, verifyDepsBeforeRun: install via workspace yaml, with a 60s timeout guard in case the deadlock returns), :282 (selective aube rebuild <pkg> preserves unreviewed_builds for un-approved siblings, asserted via the warm-path repeat-install warning), and :336 (git-dep prepare under dangerouslyAllowAllBuilds, network-gated slow port pinned to pnpm/test-git-fetch.git@8b333f12, registry overridden to npmjs.org for the typescript devDep fetch).
  • Bookkeeping: pnpm/test/install/lifecycleScripts.ts now ticked 21/21 in test/PNPM_TEST_IMPORT.md. Cleaned up a stale "fix needed before port" entry for update.ts:51, 95 (the feature landed and both tests are ported).

Test plan

  • cargo clippy --all-targets -- -D warnings
  • cargo fmt --check
  • bats test/lifecycle_scripts.bats test/rebuild.bats (56 tests, all green)
  • AUBE_NETWORK_TESTS=1 bats test/lifecycle_scripts_slow.bats (1 test, green)

🤖 Generated with Claude Code


Note

Medium Risk
Changes ensure_installed behavior for any aube run/exec/related command executed with npm_lifecycle_event set, which could mask real staleness if that env var is present unexpectedly. Added tests reduce regression risk, but this touches core install-validation flow.

Overview
Prevents nested aube invocations from re-entering dependency verification during lifecycle scripts by making ensure_installed return early when npm_lifecycle_event is present, matching npm/pnpm behavior and avoiding both lock deadlocks and missing .aube-state hard-failures.

Ports the corresponding pnpm lifecycle-script tests: two new cases in test/lifecycle_scripts.bats covering verifyDepsBeforeRun=error and verifyDepsBeforeRun=install (with a 60s timeout guard), a new network-gated test/lifecycle_scripts_slow.bats for git deps with prepare under dangerouslyAllowAllBuilds, and a test/rebuild.bats assertion that selective aube rebuild <pkg> preserves the unreviewed-builds warning for other packages. Updates test/PNPM_TEST_IMPORT.md to mark lifecycleScripts.ts as fully ported.

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

Empirically, with `verifyDepsBeforeRun=error` and `preinstall: aube run
sayHello`, the inner `aube run` fired the verify-deps check (state
isn't written yet, root preinstall runs before linking) and exited
with `dependencies need install before run: install state not found`,
failing the parent install. With `verifyDepsBeforeRun=install`, the
inner `aube run` triggered `ensure_installed` -> `install::run`, which
deadlocked on the project lock the outer install holds.

`ensure_installed` now returns early when `npm_lifecycle_event` is
set in the env, matching npm/pnpm's "no verify-deps inside lifecycle
scripts" contract.

Ports the two pnpm regression guards (lifecycleScripts.ts:179, 200)
covering pnpm/pnpm#8954 and pnpm/pnpm#10060, plus the previously-
landed selective rebuild test (lifecycleScripts.ts:282) asserting
that `aube rebuild <pkg>` preserves `unreviewed_builds` for the
un-approved sibling. Also tidies a stale "fix needed" entry in
PNPM_TEST_IMPORT.md for update.ts:51, 95 (the feature landed and
both tests are ported).

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

greptile-apps Bot commented May 7, 2026

Greptile Summary

Fixes a deadlock/hard-fail that occurred when aube run or aube exec was invoked from inside a lifecycle script: ensure_installed now returns early whenever npm_lifecycle_event is set in the environment, matching the npm/pnpm "no verify-deps inside lifecycle scripts" contract. Accompanying bats tests port the four remaining pnpm lifecycle coverage gaps.

  • crates/aube/src/commands/mod.rs: Ten-line guard added to ensure_installed — checks npm_lifecycle_event before the freshness/lock logic, preventing the two known failure modes (verifyDepsBeforeRun=error hard-fails on missing state; verifyDepsBeforeRun=install deadlocks on the parent install's project lock).
  • test/lifecycle_scripts.bats / test/lifecycle_scripts_slow.bats / test/rebuild.bats: Four pnpm test ports added — two offline deadlock/hard-fail regression guards (one with a portable 60s timeout fallback), one network-gated git-dep prepare test, and one selective-rebuild unreviewed_builds preservation test.
  • test/PNPM_TEST_IMPORT.md: Bookkeeping updated to reflect 21/21 lifecycle tests ported and stale "fix needed before port" entries resolved.

Confidence Score: 5/5

Safe to merge — the change is a targeted early-return behind a well-defined environment variable, and the behaviour matches the published npm/pnpm contract.

The core change is small and surgical: a single is_some() check on npm_lifecycle_event before any lock or state-file access. The comment precisely documents both failure modes being closed. Tests cover both the error and install variants of verifyDepsBeforeRun, including a timeout guard to catch deadlock regressions in CI. No existing call sites of ensure_installed are affected outside the lifecycle-script context.

No files require special attention.

Important Files Changed

Filename Overview
crates/aube/src/commands/mod.rs Adds early-return in ensure_installed when npm_lifecycle_event is set, preventing deadlocks and hard-fails on nested aube run/exec inside lifecycle scripts.
test/lifecycle_scripts.bats Two new tests ported from pnpm: one asserting --config.verify-deps-before-run=error doesn't fire during preinstall, one asserting verifyDepsBeforeRun: install doesn't deadlock (with a portable timeout guard for the deadlock regression).
test/lifecycle_scripts_slow.bats New network-gated test for git-dep prepare under dangerouslyAllowAllBuilds, pinned to pnpm/test-git-fetch.git@8b333f12, registry overridden to npmjs.org for the duration.
test/rebuild.bats Adds pnpm port asserting that aube rebuild <pkg> does not clear unreviewed_builds for packages not being rebuilt; verified via the warm-path repeat-install warning.
test/PNPM_TEST_IMPORT.md Bookkeeping update: marks lifecycleScripts.ts 21/21 ported, cleans up stale "fix needed before port" entries for update.ts and the lifecycle/rebuild items.

Reviews (3): Last reviewed commit: "test(scripts): use timeout fallback for ..." | Re-trigger Greptile

Comment thread test/lifecycle_scripts.bats
jdx and others added 2 commits May 7, 2026 14:13
Network-gated port of pnpm/test/install/lifecycleScripts.ts:336
('git dependencies with preparation scripts should be installed
when dangerouslyAllowAllBuilds is true'). Pins pnpm's own fixture
`pnpm/test-git-fetch.git@8b333f12` (aube already exercises pnpm
fixtures in pnpm_install_misc_slow.bats), gates behind
AUBE_NETWORK_TESTS=1, and overrides the bats registry to npmjs.org
for the duration of the test so the git-dep prepare bootstrap can
fetch typescript@4.2.4 (the fixture's devDep, not in offline
Verdaccio).

Closes the last unported test in pnpm/test/install/lifecycleScripts.ts
(21/21 now covered across lifecycle_scripts.bats, rebuild.bats, and
lifecycle_scripts_slow.bats).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`timeout(1)` is GNU coreutils — Linux ships it as `timeout`,
macOS only ships it as `gtimeout` after `brew install coreutils`,
and not at all on a stock install. The verifyDepsBeforeRun=install
deadlock guard now picks `timeout` / `gtimeout` / direct invocation
in that order. The bats wall-clock cap in CI catches a deadlock
regression on stock-macOS dev machines that hit neither tool.

Addresses Greptile review on PR #538.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jdx jdx merged commit ee5774b into main May 7, 2026
18 checks passed
@jdx jdx deleted the fix/lifecycle-verify-deps-skip branch May 7, 2026 16:49
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