feat(install): persist unreviewed-builds warning across repeat installs by jdx · Pull Request #476 · endevco/aube

jdx · 2026-05-02T19:46:45Z

Summary

Repeat aube install runs short-circuit through the warm path, which returns before the unreviewed-builds warning fires. Users were silently losing the nudge to run aube approve-builds between installs — exactly the failure mode strictDepBuilds exists to backstop.
Persist the unreviewed-build spec keys in .aube-state (new unreviewed_builds: Vec<String> on both InstallState and FreshnessState) and re-emit the same tracing::warn! line from both warm-path branches (missing_lockfile_restore_eligible and warm_path_eligible) when the set is non-empty. The allowBuilds review-placeholder write stays on the full pipeline, so warm installs nudge without re-seeding.
Ports pnpm/test/install/lifecycleScripts.ts:245 ('warning is shown when an install with --no-frozen-lockfile reuses an existing node_modules with ignored build scripts'). Wording substituted to aube's canonical ignored build scripts for N package(s) per the existing divergence note.

Background: triaged as support in #471 (test/PNPM_TEST_IMPORT.md). Companion to #472, which landed the bare-shorthand parser fix.

Test plan

cargo build
cargo test --workspace — 362+239+169+... all green; the two new state round-trip tests (unreviewed_builds_roundtrip_persists_into_fresh_state, unreviewed_builds_default_when_field_missing_in_state) confirm legacy state files load via the serde default and the new field round-trips through the FreshnessState sidecar.
cargo clippy --all-targets -- -D warnings
cargo fmt --check
./test/bats/bin/bats test/lifecycle_scripts.bats — 34/34 ok including the new aube install re-emits ignored-build-scripts warning on repeat install
./test/bats/bin/bats test/approve_builds.bats — 10/10 ok (regression check on the existing warning emission path)

🤖 Generated with Claude Code

Note

Medium Risk
Touches install warm-path short-circuit logic and the persisted .aube-state schema; regressions could cause incorrect warning behavior or state incompatibilities on upgrade, though the change is additive and guarded with serde defaults.

Overview
Repeat aube install runs that short-circuit via the warm path (and the missing-lockfile-restore fast path) now re-emit the ignored build scripts warning by reading a persisted unreviewed_builds list from .aube-state.

The install pipeline now computes the unreviewed-build set once, stores it in InstallState/FreshnessState, and centralizes warning formatting in emit_unreviewed_builds_warning; new unit tests cover round-tripping/defaulting of the new state field, and a new Bats test asserts the warning appears on a repeat no-op install.

^{Reviewed by Cursor Bugbot for commit daf7bf0. Bugbot is set up for automated code reviews on this repo. Configure here.}

greptile-apps · 2026-05-02T19:48:42Z

Greptile Summary

Persists unreviewed_builds: Vec<String> in InstallState/FreshnessState after a full install and re-emits the ignored build scripts warning from both warm-path short-circuits (missing_lockfile_restore_eligible, warm_path_eligible) so users keep seeing the nudge to run aube approve-builds on every repeat install. The computation is correctly gated with the same !ignore_scripts && !strict_dep_builds_setting && !virtual_store_only guard as the original, #[serde(default)] handles legacy state files, and the From<&InstallState> impl propagates the field into the freshness sidecar cleanly.

Confidence Score: 5/5

Safe to merge — additive state field with serde default, no behaviour change on existing paths, and well-tested.

No P0 or P1 findings. The guard conditions are consistent between computation, persistence, and emission; the serde default and From impl handle backward compatibility correctly; unit tests cover round-trip and legacy-file defaulting; a Bats integration test covers the end-to-end warm-path re-emission.

No files require special attention.

Important Files Changed

Filename	Overview
crates/aube/src/commands/install/mod.rs	Extracts unreviewed_dep_builds into a shared variable, persists it via write_state, re-emits the warning from both warm-path branches, and refactors the warning into emit_unreviewed_builds_warning; logic is correct and guarded identically to the original
crates/aube/src/state.rs	Adds unreviewed_builds: Vec to InstallState, FreshnessState, and WriteStateInput with correct serde defaults/skip_serializing_if, From impl propagation, and a new read accessor; two new unit tests cover round-trip and legacy-file defaulting
test/lifecycle_scripts.bats	Adds Bats test asserting warning fires on both the first (full pipeline) and second (warm-path) installs; correctly uses assert_output --partial for both stdout and stderr assertions
test/PNPM_TEST_IMPORT.md	Updates tracking document to reflect lifecycleScripts.ts:245 as landed and removes the duplicate Support bullet, incrementing the ported-test count from 15/21 to 16/21

_{Reviews (3): Last reviewed commit: "feat(install): persist unreviewed-builds..." | Re-trigger Greptile}

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

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

^{Reviewed by Cursor Bugbot for commit 42ec34d. Configure here.}

jdx · 2026-05-02T19:50:27Z

Addressed Greptile P1: state-persistence guard at install/mod.rs:4108 now also checks !virtual_store_only, matching the warning emission at line 4235. Without it, a virtualStoreOnly=true full install would seed unreviewed_builds in state and trigger spurious warnings on every warm-path repeat.

cargo build/clippy/fmt clean. Force-pushed.

Written with Claude.

github-actions · 2026-05-02T20:03:16Z

Benchmark changes

Public ratios: warm installs vs Bun 7x -> 10x; warm installs vs pnpm 11x -> 15x.

Benchmark	aube	bun	pnpm
Fresh install (warm cache)	332ms -> 178ms (-46%)	2242ms -> 1757ms (-22%)	3500ms -> 2745ms (-22%)
CI install (warm cache, GVS disabled)	930ms -> 1018ms (+9%)	1447ms -> 1463ms (+1%)	2475ms -> 2885ms (+17%)
CI install (cold cache, GVS disabled)	4364ms -> 4079ms (-7%)	4083ms -> 4050ms (-1%)	5380ms -> 6125ms (+14%)

The warm-path short-circuit returned before the unreviewed-builds warning fired, so the second `aube install` with no manifest or lockfile changes went silent and users forgot pending build approvals — exactly the failure mode `strictDepBuilds` exists to backstop. Persist the unreviewed-build spec keys in `.aube-state` alongside the existing freshness fields, and re-emit the same `tracing::warn!` line from both warm-path branches when the set is non-empty. The allowBuilds review-placeholder write stays on the full pipeline, so warm installs nudge without re-seeding. Ports pnpm/test/install/lifecycleScripts.ts:245 (16/21 in the pnpm test import map) — wording substituted to aube's canonical `ignored build scripts for N package(s)` per the existing divergence note. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

jdx · 2026-05-02T20:27:49Z

Addressed Cursor's low-severity finding: unreviewed_dep_builds was being called twice with identical args (state persistence + warning emission). Lifted the computation once before the state-write block; the state writer reuses a clone, the warning emission uses the cached value directly. The walk does a stat per package, so collapsing the two callers into one cuts the linker-tail cost on large graphs roughly in half.

Confirmed: cargo build/clippy/fmt clean, cargo test -p aube state 9/9, direct bats run on test/lifecycle_scripts.bats 34/34 (exit 0).

Written with Claude.

greptile-apps Bot reviewed May 2, 2026

View reviewed changes

Comment thread crates/aube/src/commands/install/mod.rs Outdated

cursor Bot reviewed May 2, 2026

View reviewed changes

Comment thread crates/aube/src/commands/install/mod.rs Outdated

Comment thread crates/aube/src/commands/install/mod.rs Outdated

jdx force-pushed the feat/persist-unreviewed-builds-warning branch from 42ec34d to 85ac681 Compare May 2, 2026 19:50

jdx force-pushed the feat/persist-unreviewed-builds-warning branch from 85ac681 to daf7bf0 Compare May 2, 2026 20:27

jdx enabled auto-merge (squash) May 2, 2026 20:36

jdx merged commit 3876bd0 into main May 2, 2026
18 checks passed

jdx deleted the feat/persist-unreviewed-builds-warning branch May 2, 2026 20:36

jdx mentioned this pull request May 2, 2026

feat(cli): aube add accepts git specs #482

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(install): persist unreviewed-builds warning across repeat installs#476

feat(install): persist unreviewed-builds warning across repeat installs#476
jdx merged 1 commit intomainfrom
feat/persist-unreviewed-builds-warning

jdx commented May 2, 2026 •

edited by cursor Bot

Loading

Uh oh!

greptile-apps Bot commented May 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

Uh oh!

jdx commented May 2, 2026

Uh oh!

github-actions Bot commented May 2, 2026 •

edited

Loading

Uh oh!

jdx commented May 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

jdx commented May 2, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

greptile-apps Bot commented May 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jdx commented May 2, 2026

Uh oh!

github-actions Bot commented May 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmark changes

Uh oh!

jdx commented May 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jdx commented May 2, 2026 •

edited by cursor Bot

Loading

greptile-apps Bot commented May 2, 2026 •

edited

Loading

github-actions Bot commented May 2, 2026 •

edited

Loading