refactor(commands): return exit codes instead of process::exit (lib-embed safety)#897
refactor(commands): return exit codes instead of process::exit (lib-embed safety)#897colinhacks wants to merge 14 commits into
Conversation
…banner The progress banner credits the engine vendor (`by jdx.dev`) after the version. That attribution is aube's own credit, so it should render only when the active embedder is standalone aube; any other embedder must suppress it so the engine's vendor brand never leaks into a host's user-facing install output. Gate the vendor tag on the active embedder matching standalone aube's name. Co-authored-by: Claude <noreply@anthropic.com>
…ofile The packument-primer cache dir, the runtime crate's `cache_dir`, the adaptive-state path, and the streamed deprecation hint all hardcoded the `aube` cache namespace / product name. Route them through the active embedder's `cache_namespace` / `name` so a host embedder lands its cache beside the rest of aube's under its own namespace and brands the mid-install hint at the emit site (where a later output-capture rewrite never reaches). Standalone aube is unchanged — every value reduces to its hardcoded literal. Co-authored-by: Claude <noreply@anthropic.com>
…e profile Sweep the leftover hardcoded `aube` identity, path, and namespace literals onto the active embedder profile so a host embedder's brand, paths, and config namespaces follow it everywhere: - virtual-store leaf (`.aube`), runtime root (`aube/nodejs`), global package subdir (`global-aube`), patch-state and deploy-injected sidecars — derived from `name` / `data_namespace`. - install-shape digest, `engines.<self>` key, and the `remove` sidecar namespaces — derived from `manifest_namespace` (+ `compatible_names`), skipping an empty namespace. - self-version pin checks/messages, install-origin and runtime-provenance labels, doctor/update-notice/recursive-error product names, completion command name, login User-Agent and hostname fallback — derived from `name` / `version` / `self_names` / `user_agent`. - branded env knobs (`AUBE_AUTH_TOKEN`, `AUBE_NO_BROWSER`, `AUBE_HOME`) gated on `env_prefix`; an embedder with no prefix reads no branded var. Every site reduces to its original literal under standalone aube, so the default identity is byte-for-byte unchanged. Co-authored-by: Claude <noreply@anthropic.com>
…t (embed-safe entrypoint) `cli_main` / `cli_main_with_defaults` called `std::process::exit` internally, so a host that embeds the command layer in-process was hard-killed by any non-zero command result or error. They now RETURN the `i32` exit code; the binary's `main` owns the single `std::process::exit(cli_main(..))`. Diagnostic rendering to stderr is unchanged and every exit-code path (success / command result / miette error → EXIT_TABLE) maps identically — standalone behavior is byte-for- byte the same. Addresses jdx's jdx#862 follow-up. Co-authored-by: Claude <noreply@anthropic.com>
…er comments Removes two "nub" mentions that jdx#862 left in aube's tree; rewrites to generic embedder-actor phrasing so aube's source stays downstream-neutral. Co-authored-by: Claude <noreply@anthropic.com>
…al-store path + project-lock brand leak Co-authored-by: Claude <noreply@anthropic.com>
… the fs-safe slug Module and fn doc-comments in patch.rs and deploy/injection.rs still referenced the old hardcoded `.aube_patch_state.json` / `.aube-deploy-injected` names after those filenames became dynamic (derived from `embedder().name`). Update them to describe the `.<name>_patch_state.json` / `.<name>-deploy-injected` pattern with a standalone-aube resolution note, matching the idiom already used in aube-linker and aube/src/state.rs. Also tightens the `Embedder::name` field doc to state explicitly that `name` is the filesystem- and command-safe identity slug (not a display label), and that it is used verbatim in on-disk paths — addressing a reviewer concern that paths derived from `name` could be unsafe. co-authored-by: Claude <noreply@anthropic.com>
…nv_prefix helpers Adds the embedder env-prefix pluggability helpers on top of the embedder profile (jdx#862), so an embedding host can hide aube's branded env surface without patching every read site. - aube_util::env::embedder_env(suffix) reads {env_prefix}_<SUFFIX> — the tool's internal debug/diagnostic/perf-bisect toggles (DISABLE_*, DIAG_*, CAS_*, INTERNAL_*, BENCH_*, self-update endpoints, NO_UPDATE_CHECK, RUNTIME_DIR). An embedder with env_prefix=None reads nothing, so no branded debug toggle leaks under the host's brand. - aube_util::env::config_env(suffix) reads {config_env_prefix}_<SUFFIX> — the small set of first-class config knobs (the cache dir, the fetch concurrency) a host legitimately wants under its OWN brand. - New Embedder field config_env_prefix: Option<&'static str> (alongside env_prefix), and AUBE sets it to Some("AUBE"). - All direct AUBE_* runtime reads now route through these helpers across aube-util / aube-store / aube-runtime / aube-registry / aube-resolver / aube (build-time AUBE_PRIMER_* reads in build.rs are left untouched). Additive and no-op for standalone aube: with env_prefix=config_env_prefix= Some("AUBE") every helper resolves the exact AUBE_* name read before. Proven by the env-prefix-default unit tests and the new embedder_env_brand_gate integration test (a nub-shaped profile: env_prefix=None hides AUBE_DISABLE_*, config_env_prefix=Some("NUB") reads NUB_* and never AUBE_* for the config knobs). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…profile Adds two user-facing source-branding helpers on top of the embedder profile (jdx#862/jdx#888), the approach jdx approved over post-processing rendered output: a user-facing miette!/bail!/eprintln!/println! string composes the program name at the source instead of an embedder string-rewriting "aube" out of finished output. - aube_util::prog() -> &str — the bare program name for user-facing output (banners, "pinned {prog} version", …). Returns Embedder::name, the same slug the clap command name and on-disk sidecars already use. - aube_util::cmd(verb) -> String — a "{prog} <verb>" command reference (cmd("install") -> "aube install"). Brands only the program prefix; the verb is the CLI spelling verbatim. Additive and no-op for standalone aube: under the default AUBE profile prog() is exactly "aube" and cmd("install") exactly "aube install" byte-for-byte. Proven by the prog_and_cmd_render_aube_under_default_profile unit test and the new source_branding_brand_gate integration test (a nub-shaped profile where the helpers carry the host brand and "aube" never leaks). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… prog()/cmd()
Replaces hardcoded "aube" / "aube <verb>" brand literals in user-facing
runtime output — miette!/bail!/eprintln!/println!/wrap_err errors, hints, and
banners across the command layer — with the source-branding helpers
aube_util::prog() / aube_util::cmd(verb). This is the approach jdx approved over
post-processing rendered output: a user-facing string composes the program name
at the source, so a library consumer (nub, which embeds aube and rebrands its
output) gets its own brand in errors/help with no post-pass.
- ~120 user-facing command references across ~45 command-layer files now use
cmd("install") / cmd("run") / cmd("deploy") / … for `aube <verb>` references
and prog() for bare program-name references.
- Untouched (correctly): internal tracing logs, error/warning CODE constants
(ERR_AUBE_*/WARN_AUBE_*), branded filenames (aube-lock.yaml,
aube-workspace.yaml), manifest keys (package.json#aube, aube.<key>), doc
comments, test fixtures, and clap help-text consts (a `&'static str` resolved
at definition time — can't call a runtime helper; tracked as a follow-up).
Adds a conservative brand-literal lint (crates/aube/tests/brand_literal_lint.rs)
that scans the command layer and fails on a newly-introduced hardcoded
`aube <verb>` literal in a user-facing string, steering future code to cmd(...).
It exempts comments, tracing, clap help-text/`$ aube` examples, branded
filenames/keys, and a one-entry allowlist (a provenance data value). Module
docs explain how to satisfy it.
Default-preserving: under the default AUBE profile every converted site renders
byte-for-byte identical to the old literal, so standalone aube's output is
unchanged. cargo build/clippy/fmt clean; full workspace test suite green
(including the lint and the prog/cmd brand-gate tests).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (13)
📝 WalkthroughWalkthroughThis PR adds embedder-aware env and branding helpers, applies them to runtime paths, caches, sidecar names, and command output, and changes command execution to return exit codes through the CLI entrypoint. ChangesEmbedder branding and CLI exit codes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~100 minutes Possibly related PRs
Suggested labels
Poem
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Refactors CLI execution to be embedder-safe by returning exit codes instead of directly terminating, and systematically routes user-facing strings / env knobs through the active embedder identity to avoid hardcoded aube branding.
Changes:
- Move
std::process::exitownership to the standalone binary (main.rs), returning exit codes fromcli_mainand multiple commands. - Introduce branding helpers (
aube_util::prog,aube_util::cmd) and update user-facing messages to use them instead of hardcodedaube .... - Add a conservative “brand literal” lint test plus integration tests for embedder branding and env-prefix behavior.
Reviewed changes
Copilot reviewed 98 out of 98 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/aube/tests/e2e.rs | Adds e2e assertions that child/script exit codes propagate unchanged after the refactor. |
| crates/aube/tests/brand_literal_lint.rs | Adds a conservative lint test to prevent newly introduced hardcoded aube <verb> literals in command-layer runtime strings. |
| crates/aube/src/update_check.rs | Brands update messages and gates update-check env using embedder-aware env lookup. |
| crates/aube/src/self_version.rs | Brands self-version pin messages and recognizes embedder self-names in packageManager pins. |
| crates/aube/src/runtime.rs | Brands runtime provenance / progress tool name via embedder identity. |
| crates/aube/src/progress/mod.rs | Centralizes branded banner rendering and suppresses vendor attribution under non-standalone embedders. |
| crates/aube/src/progress/ci.rs | Reuses centralized banner rendering in CI progress output. |
| crates/aube/src/main.rs | Makes the binary the single owner of std::process::exit. |
| crates/aube/src/lib.rs | Changes cli_main* to return i32 exit codes; updates async dispatching to propagate optional codes. |
| crates/aube/src/engines.rs | Makes the manifest engine key for “self” embedder-aware via manifest_namespace. |
| crates/aube/src/deprecations.rs | Brands streamed install warnings (“Run …”) via embedder name. |
| crates/aube/src/commands/why.rs | Replaces hardcoded command references with aube_util::cmd(...). |
| crates/aube/src/commands/update.rs | Replaces hardcoded command references with aube_util::cmd(...). |
| crates/aube/src/commands/unlink.rs | Replaces hardcoded command references with aube_util::cmd(...). |
| crates/aube/src/commands/settings_context.rs | Brands login hints and derives virtual-store leaf from embedder name. |
| crates/aube/src/commands/sbom.rs | Replaces hardcoded command references with aube_util::cmd(...). |
| crates/aube/src/commands/runtime.rs | Brands runtime errors / help text via embedder helpers. |
| crates/aube/src/commands/run.rs | Returns propagatable exit codes instead of exiting; brands user-facing errors/hints. |
| crates/aube/src/commands/restart.rs | Returns propagatable exit codes; preserves short-circuit behavior without in-place exit. |
| crates/aube/src/commands/remove.rs | Makes sidecar namespace pruning embedder-aware (compatible namespaces + embedder namespace). |
| crates/aube/src/commands/recursive.rs | Brands an error message via aube_util::cmd("recursive"). |
| crates/aube/src/commands/rebuild.rs | Replaces hardcoded command references with aube_util::cmd(...). |
| crates/aube/src/commands/query.rs | Replaces hardcoded command references with aube_util::cmd(...). |
| crates/aube/src/commands/publish.rs | Brands user-facing publish errors via aube_util::cmd("publish"). |
| crates/aube/src/commands/prune.rs | Brands wrap_err hints via aube_util::cmd(...). |
| crates/aube/src/commands/project_lock.rs | Brands contended-lock callback message via embedder name. |
| crates/aube/src/commands/peers.rs | Returns exit code 1 via Ok(Some(1)) instead of exiting; brands lockfile hint. |
| crates/aube/src/commands/patch.rs | Derives patch state sidecar filename and user instructions from embedder identity. |
| crates/aube/src/commands/outdated.rs | Returns exit codes instead of exiting; brands lockfile hints and errors. |
| crates/aube/src/commands/login.rs | Brands env usage and user-agent; gates browser opt-out via embedder prefix. |
| crates/aube/src/commands/list.rs | Brands lockfile hints and filter errors via aube_util::cmd(...). |
| crates/aube/src/commands/link.rs | Brands command hint via aube_util::cmd("link"). |
| crates/aube/src/commands/licenses.rs | Brands lockfile hint via aube_util::cmd(...). |
| crates/aube/src/commands/install_test.rs | Returns propagatable exit code type and brands redundancy message via embedder helpers. |
| crates/aube/src/commands/install/workspace.rs | Brands filter mismatch error via aube_util::cmd("install"). |
| crates/aube/src/commands/install/unreviewed_builds.rs | Brands approve-builds hint via aube_util::cmd(...). |
| crates/aube/src/commands/install/resolve.rs | Brands frozen-lockfile help via aube_util::cmd("install"). |
| crates/aube/src/commands/install/node_gyp_bootstrap.rs | Brands recursive-install diagnostics via aube_util::cmd("install"). |
| crates/aube/src/commands/install/mod.rs | Routes debug/perf toggles through embedder-aware env helpers. |
| crates/aube/src/commands/install/fetch.rs | Routes debug/perf toggles through embedder-aware env helpers; brands help text. |
| crates/aube/src/commands/init.rs | Writes packageManager pin using embedder name/version. |
| crates/aube/src/commands/global.rs | Brands HOME override and uses embedder name in global install dir layout. |
| crates/aube/src/commands/find_hash.rs | Brands help text command references via aube_util::cmd(...). |
| crates/aube/src/commands/fetch.rs | Brands error text via aube_util::cmd("fetch"). |
| crates/aube/src/commands/exec.rs | Returns exit codes instead of exiting; brands install hint and failure messages. |
| crates/aube/src/commands/doctor.rs | Returns exit code instead of exiting; brands version section and hints. |
| crates/aube/src/commands/dlx.rs | Returns exit code instead of exiting; brands help hint via aube_util::cmd("dlx"). |
| crates/aube/src/commands/dist_tag.rs | Brands help and auth hints via aube_util::cmd(...). |
| crates/aube/src/commands/deprecations.rs | Brands lockfile hint via aube_util::cmd("install"). |
| crates/aube/src/commands/deploy/staging.rs | Brands deploy errors via aube_util::cmd("deploy"). |
| crates/aube/src/commands/deploy/rewrite.rs | Brands deploy errors via aube_util::cmd("deploy"). |
| crates/aube/src/commands/deploy/mod.rs | Brands deploy errors via aube_util::cmd("deploy"). |
| crates/aube/src/commands/deploy/injection.rs | Makes injected-deps staging dir embedder-name-derived and brands deploy errors. |
| crates/aube/src/commands/config/tui.rs | Brands config/tui errors and help guidance via aube_util::cmd(...). |
| crates/aube/src/commands/config/set.rs | Brands config-set help text and program-name references via embedder helpers. |
| crates/aube/src/commands/config/mod.rs | Brands config argument errors and feature-gate message via embedder helpers. |
| crates/aube/src/commands/config/explain.rs | Brands “try config find” hint using aube_util::cmd(...). |
| crates/aube/src/commands/config/delete.rs | Brands config delete errors and guidance via embedder helpers. |
| crates/aube/src/commands/config/aube_config.rs | Brands config serialization and home-dir error messages via embedder name. |
| crates/aube/src/commands/completion.rs | Passes embedder name to usage generator and brands usage cmd string. |
| crates/aube/src/commands/clean.rs | Returns propagatable exit codes (Option<i32>) for clean/purge paths. |
| crates/aube/src/commands/check.rs | Returns exit code 1 instead of exiting when issues are found. |
| crates/aube/src/commands/cat_index.rs | Brands help text command references via aube_util::cmd(...). |
| crates/aube/src/commands/cat_file.rs | Brands “store path” hint via aube_util::cmd(...). |
| crates/aube/src/commands/cache.rs | Brands “view/install” hints via aube_util::cmd(...). |
| crates/aube/src/commands/auto_install.rs | Brands install hint and program name via embedder helpers. |
| crates/aube/src/commands/audit.rs | Returns exit codes (1/2) instead of exiting; brands lockfile hints. |
| crates/aube/src/commands/approve_builds.rs | Brands install/rebuild guidance and ignored-builds hints via aube_util::cmd(...). |
| crates/aube/src/commands/add_supply_chain.rs | Brands “user aborted” message via aube_util::cmd("add"). |
| crates/aube-util/tests/source_branding_brand_gate.rs | Integration test ensuring prog/cmd follow a non-default embedder. |
| crates/aube-util/tests/embedder_env_brand_gate.rs | Integration test ensuring env-prefix gating and config-env branding under a non-default embedder. |
| crates/aube-util/src/snapshot.rs | Routes snapshot disable toggle through embedder-aware env helper; updates docs. |
| crates/aube-util/src/lib.rs | Re-exports branding helpers (prog, cmd). |
| crates/aube-util/src/identity.rs | Adds prog/cmd helpers and introduces config_env_prefix on Embedder. |
| crates/aube-util/src/http/ticket_cache.rs | Routes disable toggle through embedder-aware env helper. |
| crates/aube-util/src/http/resolve.rs | Routes disable toggle through embedder-aware env helper. |
| crates/aube-util/src/http/race.rs | Routes disable toggle through embedder-aware env helper. |
| crates/aube-util/src/http/prewarm.rs | Routes disable toggle through embedder-aware env helper. |
| crates/aube-util/src/hash.rs | Makes manifest install-shape hashing include embedder namespace dynamically. |
| crates/aube-util/src/env.rs | Adds embedder_env and config_env helpers; adds default-profile tests. |
| crates/aube-util/src/diag_kernel.rs | Routes diag toggle through embedder-aware env helper. |
| crates/aube-util/src/diag.rs | Routes diag toggles through embedder-aware env helper; updates threshold parsing. |
| crates/aube-util/src/concurrency.rs | Reads concurrency override through embedder-aware config env; updates warning text. |
| crates/aube-util/src/adaptive.rs | Uses embedder cache_namespace for persistent adaptive-state path. |
| crates/aube-store/src/tarball.rs | Routes import toggles through embedder-aware env helper. |
| crates/aube-store/src/cas.rs | Routes O_TMPFILE and CAS thresholds through embedder-aware env helper; updates warning text. |
| crates/aube-scripts/tests/user_agent_product.rs | Updates embedder fixture to include new config_env_prefix. |
| crates/aube-runtime/src/self_install.rs | Routes self-install override envs through embedder-aware env helper. |
| crates/aube-runtime/src/paths.rs | Uses embedder data/cache namespaces and routes runtime-dir override through embedder-aware env helper. |
| crates/aube-runtime/src/discover.rs | Brands install origin label via embedder name. |
| crates/aube-resolver/src/primer.rs | Reads cache dir override through embedder-aware config env; uses embedder cache namespace. |
| crates/aube-registry/tests/user_agent_product.rs | Updates embedder fixture to include new config_env_prefix. |
| crates/aube-registry/src/client/http.rs | Routes internal force-packument toggle through embedder-aware env helper. |
| crates/aube-manifest/src/workspace/edits.rs | Updates comments to avoid embedder-specific brand mention in docs. |
| crates/aube-lockfile/tests/custom_lock_filename.rs | Updates embedder fixture to include new config_env_prefix. |
| crates/aube-linker/src/lib.rs | Updates docs to reflect virtual-store leaf is .<name> under embedder branding. |
| crates/aube-linker/src/builder.rs | Derives default virtual-store leaf from embedder name instead of hardcoding .aube. |
Comments suppressed due to low confidence (1)
crates/aube/src/commands/recursive.rs:1
- This still hardcodes the program name as
\"aube\"in the synthesized argv, which breaks the embedder branding goal (and can create mismatches if a host expects its own binary name). Use the embedder-aware program name (e.g.,aube_util::prog()) for argv[0] so recursive execution stays consistent with rebranding.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// it's covered by the `embedder_env_brand_gate` integration test, which | ||
| /// registers a real non-aube profile in its own process. | ||
| #[test] | ||
| fn embedder_and_config_env_read_aube_prefixed_under_default_profile() { |
| // SAFETY: tests run serially via RUST_TEST_THREADS=1. | ||
| unsafe { | ||
| match &self.prev { | ||
| Some(v) => std::env::set_var(&self.key, v), | ||
| None => std::env::remove_var(&self.key), | ||
| } | ||
| } | ||
| } | ||
| } | ||
| fn with_var<F: FnOnce()>(key: &str, value: &str, f: F) { | ||
| let _guard = EnvGuard { | ||
| key: key.to_string(), | ||
| prev: std::env::var_os(key), | ||
| }; | ||
| // SAFETY: tests run serially via RUST_TEST_THREADS=1. | ||
| unsafe { std::env::set_var(key, value) }; | ||
| f(); | ||
| } |
| ceiling = CONCURRENCY_CEILING, | ||
| "AUBE_CONCURRENCY ignored: must be an integer in [floor, ceiling]; using default" | ||
| "concurrency override ignored: must be an integer in [floor, ceiling]; using default" | ||
| ); |
| warn!( | ||
| "CAS_SMALL_FILE_THRESHOLD={raw:?} is not a non-negative integer; \ | ||
| falling back to default {CAS_SMALL_FILE_THRESHOLD_DEFAULT}" | ||
| ); | ||
| CAS_SMALL_FILE_THRESHOLD_DEFAULT |
…forgotten twice) AGENTS.md now has a memorize-this section: nub embeds aube as a vendored Rust library (vendor/aube path dep, fork nubjs/aube, upstream jdx/aube); contribution PRs target jdx/aube UPSTREAM, never fork-internal (the PR-targeting mistake); what upstreams vs stays fork-only; the embedder profile; fork-discipline. Threads: gvs-correctness-audit verdict (one real GVS hole = Metro/RN/Expo; everything else clean) + the bun-symlink-default reframe (bun defaults to symlinks itself + abandoned out-of-project symlinks → nub making them work is a strength, not a weakness); Metro methodology double-check dispatched (Opus, per Colin). workspace-migration = already fully implemented (nub pm use nub). process-exit refiled to jdx/aube#897 (was misfiled fork PR). transitive-bin-bug thread created (Gatsby/lmdb, GVS-independent). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR refactors 12 command-level
Confidence Score: 4/5Safe to merge once the outdated exit-code change is resolved or explicitly documented as intentional. The outdated non-filtered path now exits 1 when packages are outdated, where it previously always exited 0. This is a real behavioral change that CI pipelines could hit silently. Every other conversion is a straight lift-and-shift of an existing process::exit call, and the three new e2e tests cover the core contract well. crates/aube/src/commands/outdated.rs — the exit-1 on the non-filtered path is net-new behaviour, not a conversion of a pre-existing exit call. Important Files Changed
Reviews (2): Last reviewed commit: "refactor(commands): return exit codes fr..." | Re-trigger Greptile |
| ] | ||
| } | ||
|
|
||
| pub async fn run(args: CleanArgs) -> miette::Result<()> { | ||
| pub async fn run(args: CleanArgs) -> miette::Result<Option<i32>> { | ||
| run_as("clean", args).await | ||
| } | ||
|
|
||
| pub async fn run_purge(args: CleanArgs) -> miette::Result<()> { | ||
| pub async fn run_purge(args: CleanArgs) -> miette::Result<Option<i32>> { | ||
| run_as("purge", args).await | ||
| } | ||
|
|
||
| async fn run_as(invoked_as: &str, args: CleanArgs) -> miette::Result<()> { | ||
| async fn run_as(invoked_as: &str, args: CleanArgs) -> miette::Result<Option<i32>> { | ||
| let cwd = crate::dirs::project_root()?; | ||
| let _lock = super::take_project_lock(&cwd)?; |
There was a problem hiding this comment.
clean and purge were converted to miette::Result<Option<i32>> even though neither had a std::process::exit call before this PR. run_as unconditionally returns Ok(None), so the matching if let Some(code) arms in async_main are dead code. If this conversion is intentional forward-compat, a short comment here explaining that would prevent future confusion. Otherwise, the return type and the two dispatch arms in async_main can be reverted to miette::Result<()> to match every other command that doesn't signal a non-zero exit.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/aube/src/commands/init.rs (1)
230-240:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTest assertion is inconsistent with the embedder-aware implementation.
The test still expects a hardcoded
aube@{CARGO_PKG_VERSION}format, but the implementation at lines 98-105 now derives thepackageManagerfield from the active embedder (id.name@id.version). The test passes for standalone aube whereembedder().name == "aube"andembedder().version == CARGO_PKG_VERSION, but it doesn't verify the embedder-aware contract.💚 Proposed fix to align the test with embedder-aware behavior
#[test] fn init_package_manager_pins_aube() { let a = InitArgs { init_package_manager: true, ..args() }; let v: serde_json::Value = serde_json::from_str(&render("demo", &a)).unwrap(); + let id = aube_util::embedder(); assert_eq!( v["packageManager"], - format!("aube@{}", env!("CARGO_PKG_VERSION")) + format!("{}@{}", id.name, id.version) ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/aube/src/commands/init.rs` around lines 230 - 240, The test init_package_manager_pins_aube uses a hardcoded assertion expecting aube@{CARGO_PKG_VERSION}, but the implementation now derives packageManager dynamically from the active embedder's name and version fields. Instead of comparing against the hardcoded format string, retrieve the active embedder's id (likely through an embedder() function), construct the expected value dynamically as embedder_name@embedder_version, and update the assertion to compare against this dynamically constructed value. This ensures the test properly verifies the embedder-aware contract rather than accidentally passing when embedder name matches "aube" and version matches CARGO_PKG_VERSION.crates/aube/src/commands/audit.rs (2)
309-312:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInteractive cancel still hard-exits the host process.
audit --interactivecan still terminate an embedding host here viastd::process::exit(130). That exit code needs to be plumbed back throughrun()the same way as the other converted command exits.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/aube/src/commands/audit.rs` around lines 309 - 312, The advisory_picker(rows).run() call is hard-exiting the process with std::process::exit(130) when interrupted, which prevents proper error handling in embedding hosts. Instead of calling std::process::exit(130) in the error match arm for ErrorKind::Interrupted, convert this to return an appropriate error type that propagates back through the run() function's return value. Check how other command exits are being converted in the codebase to ensure consistency in how the 130 exit code is plumbed back as a returnable error rather than a direct process termination.
71-75:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
--ignore-registry-errorsno longer ignores registry errors.The flag help still promises an exit code of 0, but this branch now returns exit code 2. That flips the contract for existing CI callers that use this flag specifically to stay green when the advisory service is unavailable.
Also applies to: 182-215
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/aube/src/commands/audit.rs` around lines 71 - 75, The `ignore_registry_errors` flag at lines 71-75 and its implementation at lines 182-215 are not aligned. The flag's documentation promises that when enabled, it will exit with code 0 if the registry responds with an error, but the current implementation returns exit code 2 instead. Fix the logic in the implementation section (lines 182-215) to check the `ignore_registry_errors` boolean field and return exit code 0 when this flag is true and a registry error occurs, restoring the documented behavior that CI callers depend on.
🧹 Nitpick comments (2)
crates/aube-util/src/identity.rs (1)
44-48: 💤 Low valueConsider adding validation for the
namefield invariant.The documentation states that
name"must be filesystem- and command-safe (no spaces, slashes, or shell metacharacters)" and that "the embedder is responsible for supplying a safe slug." However, unlikelockfile_basename(which is validated inset_embedderat lines 190-200), there is nodebug_assert!to enforce this invariant.Adding a validation check in
set_embedderwould make the error surface at registration time rather than manifesting as subtle filesystem or shell issues later, and would be consistent with the existing lockfile validation pattern.🛡️ Suggested validation
pub fn set_embedder(embedder: &'static Embedder) { + debug_assert!( + !embedder.name.is_empty() + && !embedder.name.contains(|c: char| c.is_whitespace() || "/\\".contains(c)), + "embedder name {:?} must be non-empty and filesystem/command-safe (no spaces or slashes)", + embedder.name, + ); debug_assert!( embedder.lockfile_basename.contains('.'),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/aube-util/src/identity.rs` around lines 44 - 48, The `name` field of the struct has a documented invariant requiring it to be filesystem- and command-safe (no spaces, slashes, or shell metacharacters), but unlike `lockfile_basename`, there is no validation enforcing this constraint. Add a validation check in the `set_embedder` method using debug_assert! (consistent with the existing lockfile_basename validation pattern at lines 190-200) to validate that the `name` field contains only safe characters. This will surface configuration errors at registration time rather than allowing unsafe slugs to cause filesystem or shell execution issues later.crates/aube-registry/src/client/http.rs (1)
262-271: ⚡ Quick winUpdate comment to reflect embedder-aware variable name.
The comment states "The name is deliberately
AUBE_INTERNAL_*" but with the embedder-awareembedder_env()helper, the actual environment variable name is now<BRAND>_INTERNAL_FORCE_FULL_PACKUMENTfor embedders andAUBE_INTERNAL_FORCE_FULL_PACKUMENTfor standalone aube.📝 Suggested comment update
-/// The name is deliberately `AUBE_INTERNAL_*` so nothing outside the -/// test harness grows a habit of relying on it, and we require the +/// The name is deliberately prefixed with the embedder's internal namespace +/// (`AUBE_INTERNAL_*` for standalone aube) so nothing outside the test +/// harness grows a habit of relying on it, and we require the /// exact literal `"1"` (not just any non-empty value) so an inherited /// or accidentally-set empty value won't silently balloon registry /// traffic on end-user machines.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/aube-registry/src/client/http.rs` around lines 262 - 271, The comment in the force_full_packument() function states "The name is deliberately `AUBE_INTERNAL_*`" but this is outdated now that the function uses embedder_env() which makes the environment variable name embedder-aware. Update the comment to clarify that the actual environment variable name is `<BRAND>_INTERNAL_FORCE_FULL_PACKUMENT` for embedders and `AUBE_INTERNAL_FORCE_FULL_PACKUMENT` for standalone aube, while preserving the original intent about avoiding external reliance and requiring the exact "1" value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aube/src/commands/exec.rs`:
- Around line 256-263: In the parallel execution block where a child task exits
with non-zero status (the condition checking !status.success() &&
first_exit.is_none()), remove the dead error message creation that assigns to
first_err using miette!. Keep only the exit code tracking logic that sets
first_exit and the code variable. This prevents the unreachable error message
from being created and avoids unconditionally overwriting any earlier task error
stored in first_err at line 265, allowing the original error to be preserved for
the fallback path at lines 277-279 and maintaining consistency with the
sequential execution pattern at lines 155-157.
In `@crates/aube/src/lib.rs`:
- Around line 527-538: The `cli_main` function's docstring claims to be
embed-safe by returning exit codes without hard-terminating the process, but
multiple code paths violate this contract by calling `std::process::exit()`
directly. In `crates/aube/src/lib.rs` at line 648, replace the
`Cli::command().get_matches_from()` call with `try_get_matches_from()` to
propagate argument parsing errors as return values instead of allowing clap's
built-in print-and-exit behavior for parse errors, `--help`, and `--version`.
Additionally, in the demand picker cancellation handlers at
`crates/aube/src/commands/run.rs` line 292, `crates/aube/src/commands/update.rs`
line 1048, and `crates/aube/src/commands/audit.rs` line 311, replace the
`std::process::exit(130)` calls with code that propagates the interrupt exit
code (130) through the function's return path instead of hard-exiting.
In `@crates/aube/tests/brand_literal_lint.rs`:
- Around line 53-123: The VERBS constant array in the test file is missing the
`find-hash` command that already exists in the codebase, which means the lint
test won't catch hardcoded `aube find-hash` literals. Add `find-hash` to the
VERBS constant array to ensure it is kept in sync with all actual clap command
names and properly validates all brand literal references.
---
Outside diff comments:
In `@crates/aube/src/commands/audit.rs`:
- Around line 309-312: The advisory_picker(rows).run() call is hard-exiting the
process with std::process::exit(130) when interrupted, which prevents proper
error handling in embedding hosts. Instead of calling std::process::exit(130) in
the error match arm for ErrorKind::Interrupted, convert this to return an
appropriate error type that propagates back through the run() function's return
value. Check how other command exits are being converted in the codebase to
ensure consistency in how the 130 exit code is plumbed back as a returnable
error rather than a direct process termination.
- Around line 71-75: The `ignore_registry_errors` flag at lines 71-75 and its
implementation at lines 182-215 are not aligned. The flag's documentation
promises that when enabled, it will exit with code 0 if the registry responds
with an error, but the current implementation returns exit code 2 instead. Fix
the logic in the implementation section (lines 182-215) to check the
`ignore_registry_errors` boolean field and return exit code 0 when this flag is
true and a registry error occurs, restoring the documented behavior that CI
callers depend on.
In `@crates/aube/src/commands/init.rs`:
- Around line 230-240: The test init_package_manager_pins_aube uses a hardcoded
assertion expecting aube@{CARGO_PKG_VERSION}, but the implementation now derives
packageManager dynamically from the active embedder's name and version fields.
Instead of comparing against the hardcoded format string, retrieve the active
embedder's id (likely through an embedder() function), construct the expected
value dynamically as embedder_name@embedder_version, and update the assertion to
compare against this dynamically constructed value. This ensures the test
properly verifies the embedder-aware contract rather than accidentally passing
when embedder name matches "aube" and version matches CARGO_PKG_VERSION.
---
Nitpick comments:
In `@crates/aube-registry/src/client/http.rs`:
- Around line 262-271: The comment in the force_full_packument() function states
"The name is deliberately `AUBE_INTERNAL_*`" but this is outdated now that the
function uses embedder_env() which makes the environment variable name
embedder-aware. Update the comment to clarify that the actual environment
variable name is `<BRAND>_INTERNAL_FORCE_FULL_PACKUMENT` for embedders and
`AUBE_INTERNAL_FORCE_FULL_PACKUMENT` for standalone aube, while preserving the
original intent about avoiding external reliance and requiring the exact "1"
value.
In `@crates/aube-util/src/identity.rs`:
- Around line 44-48: The `name` field of the struct has a documented invariant
requiring it to be filesystem- and command-safe (no spaces, slashes, or shell
metacharacters), but unlike `lockfile_basename`, there is no validation
enforcing this constraint. Add a validation check in the `set_embedder` method
using debug_assert! (consistent with the existing lockfile_basename validation
pattern at lines 190-200) to validate that the `name` field contains only safe
characters. This will surface configuration errors at registration time rather
than allowing unsafe slugs to cause filesystem or shell execution issues later.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 569e56c3-47b4-466d-8ee8-310430ce6fcc
📒 Files selected for processing (98)
crates/aube-linker/src/builder.rscrates/aube-linker/src/lib.rscrates/aube-lockfile/tests/custom_lock_filename.rscrates/aube-manifest/src/workspace/edits.rscrates/aube-registry/src/client/http.rscrates/aube-registry/tests/user_agent_product.rscrates/aube-resolver/src/primer.rscrates/aube-runtime/src/discover.rscrates/aube-runtime/src/paths.rscrates/aube-runtime/src/self_install.rscrates/aube-scripts/tests/user_agent_product.rscrates/aube-store/src/cas.rscrates/aube-store/src/tarball.rscrates/aube-util/src/adaptive.rscrates/aube-util/src/concurrency.rscrates/aube-util/src/diag.rscrates/aube-util/src/diag_kernel.rscrates/aube-util/src/env.rscrates/aube-util/src/hash.rscrates/aube-util/src/http/prewarm.rscrates/aube-util/src/http/race.rscrates/aube-util/src/http/resolve.rscrates/aube-util/src/http/ticket_cache.rscrates/aube-util/src/identity.rscrates/aube-util/src/lib.rscrates/aube-util/src/snapshot.rscrates/aube-util/tests/embedder_env_brand_gate.rscrates/aube-util/tests/source_branding_brand_gate.rscrates/aube/src/commands/add_supply_chain.rscrates/aube/src/commands/approve_builds.rscrates/aube/src/commands/audit.rscrates/aube/src/commands/auto_install.rscrates/aube/src/commands/cache.rscrates/aube/src/commands/cat_file.rscrates/aube/src/commands/cat_index.rscrates/aube/src/commands/check.rscrates/aube/src/commands/clean.rscrates/aube/src/commands/completion.rscrates/aube/src/commands/config/aube_config.rscrates/aube/src/commands/config/delete.rscrates/aube/src/commands/config/explain.rscrates/aube/src/commands/config/mod.rscrates/aube/src/commands/config/set.rscrates/aube/src/commands/config/tui.rscrates/aube/src/commands/create.rscrates/aube/src/commands/deploy/injection.rscrates/aube/src/commands/deploy/mod.rscrates/aube/src/commands/deploy/rewrite.rscrates/aube/src/commands/deploy/staging.rscrates/aube/src/commands/deprecations.rscrates/aube/src/commands/dist_tag.rscrates/aube/src/commands/dlx.rscrates/aube/src/commands/doctor.rscrates/aube/src/commands/exec.rscrates/aube/src/commands/fetch.rscrates/aube/src/commands/find_hash.rscrates/aube/src/commands/global.rscrates/aube/src/commands/init.rscrates/aube/src/commands/install/fetch.rscrates/aube/src/commands/install/mod.rscrates/aube/src/commands/install/node_gyp_bootstrap.rscrates/aube/src/commands/install/resolve.rscrates/aube/src/commands/install/unreviewed_builds.rscrates/aube/src/commands/install/workspace.rscrates/aube/src/commands/install_test.rscrates/aube/src/commands/licenses.rscrates/aube/src/commands/link.rscrates/aube/src/commands/list.rscrates/aube/src/commands/login.rscrates/aube/src/commands/outdated.rscrates/aube/src/commands/patch.rscrates/aube/src/commands/peers.rscrates/aube/src/commands/project_lock.rscrates/aube/src/commands/prune.rscrates/aube/src/commands/publish.rscrates/aube/src/commands/query.rscrates/aube/src/commands/rebuild.rscrates/aube/src/commands/recursive.rscrates/aube/src/commands/remove.rscrates/aube/src/commands/restart.rscrates/aube/src/commands/run.rscrates/aube/src/commands/runtime.rscrates/aube/src/commands/sbom.rscrates/aube/src/commands/settings_context.rscrates/aube/src/commands/unlink.rscrates/aube/src/commands/update.rscrates/aube/src/commands/why.rscrates/aube/src/deprecations.rscrates/aube/src/engines.rscrates/aube/src/lib.rscrates/aube/src/main.rscrates/aube/src/progress/ci.rscrates/aube/src/progress/mod.rscrates/aube/src/runtime.rscrates/aube/src/self_version.rscrates/aube/src/update_check.rscrates/aube/tests/brand_literal_lint.rscrates/aube/tests/e2e.rs
| if !status.success() && first_exit.is_none() { | ||
| let code = aube_scripts::exit_code_from_status(status); | ||
| first_exit = Some(code); | ||
| first_err = Some(miette!("aube exec: `{bin}` failed in {name} (exit {code})")); | ||
| first_err = Some(miette!( | ||
| "{}: `{bin}` failed in {name} (exit {code})", | ||
| aube_util::cmd("exec") | ||
| )); | ||
| } |
There was a problem hiding this comment.
Dead code and potential error message loss in parallel execution.
At lines 256-263, when a child task exits with a non-zero status, the code creates a miette! error message and stores it in first_err. However, this error message is never returned to the user:
- Line 272-276 returns
Ok(Some(code))whenfirst_exitis set, completely bypassingfirst_err. - The error message created here is unreachable.
Additionally, there's a subtle bug: if an earlier task completed with Ok(Err(e)) (setting first_err at line 265), and then a later task exits non-zero, line 259-262 unconditionally overwrites first_err, losing the original error. Since the code then returns the exit code (line 272-276), both error messages are lost.
The sequential path (lines 155-157) simply returns the exit code without creating an error message, which is the correct pattern for the "propagate exit codes" contract described in the PR objectives.
🧹 Suggested fix
Remove the dead error-message creation and avoid overwriting first_err:
if !status.success() && first_exit.is_none() {
let code = aube_scripts::exit_code_from_status(status);
first_exit = Some(code);
- first_err = Some(miette!(
- "{}: `{bin}` failed in {name} (exit {code})",
- aube_util::cmd("exec")
- ));
}This keeps the exit-code tracking clean and preserves any earlier task errors in first_err for the fallback path at line 277-279.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/aube/src/commands/exec.rs` around lines 256 - 263, In the parallel
execution block where a child task exits with non-zero status (the condition
checking !status.success() && first_exit.is_none()), remove the dead error
message creation that assigns to first_err using miette!. Keep only the exit
code tracking logic that sets first_exit and the code variable. This prevents
the unreachable error message from being created and avoids unconditionally
overwriting any earlier task error stored in first_err at line 265, allowing the
original error to be preserved for the fallback path at lines 277-279 and
maintaining consistency with the sequential execution pattern at lines 155-157.
| /// **Returns the exit code; it does not terminate the process.** It parses | ||
| /// argv, runs the selected command, renders any diagnostic to stderr, and | ||
| /// hands back the code the binary's `main` should exit with. Returning rather | ||
| /// than calling `std::process::exit` keeps it embed-safe: a host that drives | ||
| /// it in-process is not hard-killed by a non-zero result or an error. The | ||
| /// standalone binary does `std::process::exit(cli_main(..))`. | ||
| /// | ||
| /// `#[must_use]`: the `i32` is the exit code, not a side effect. An | ||
| /// embedder migrating off the old `process::exit` entrypoint that drops | ||
| /// it would silently exit 0 on every failure — Rust won't warn on an | ||
| /// ignored return — so the lint nudges them to | ||
| /// `std::process::exit(cli_main(..))`. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's confirm the file structure and examine the lib.rs code around lines 527-538
cd crates/aube && git ls-files src/lib.rs src/commands/run.rsRepository: jdx/aube
Length of output: 83
🏁 Script executed:
# Check the code at the first location: lib.rs lines 527-538
sed -n '520,545p' crates/aube/src/lib.rs | cat -nRepository: jdx/aube
Length of output: 1624
🏁 Script executed:
# Check the code at the second location: run.rs lines 275-276
sed -n '270,285p' crates/aube/src/commands/run.rs | cat -nRepository: jdx/aube
Length of output: 808
🏁 Script executed:
# Let's also search for other process::exit calls in the lib crate to understand the scope
rg 'process::exit' crates/aube/src --with-filename -nRepository: jdx/aube
Length of output: 3246
🏁 Script executed:
# Check the full cli_main and inner_main function signatures to understand the contract
sed -n '500,550p' crates/aube/src/lib.rs | cat -nRepository: jdx/aube
Length of output: 3199
🏁 Script executed:
# Look at how clap is configured in this project - check Cargo.toml for clap config
grep -A 5 'clap' crates/aube/Cargo.tomlRepository: jdx/aube
Length of output: 543
🏁 Script executed:
# Check the exact code at run.rs around line 292 (the actual hard-exit location)
sed -n '285,300p' crates/aube/src/commands/run.rs | cat -nRepository: jdx/aube
Length of output: 840
🏁 Script executed:
# Look for inner_main function and clap usage in lib.rs
rg 'inner_main|clap::' crates/aube/src/lib.rs -A 3 -B 1Repository: jdx/aube
Length of output: 2548
🏁 Script executed:
# Check the full flow - find where clap parsing happens and if it uses exit
rg 'try_get_matches|get_matches|parse' crates/aube/src/lib.rs -nRepository: jdx/aube
Length of output: 3663
🏁 Script executed:
# Look at the full run function implementation around line 275-292
sed -n '260,310p' crates/aube/src/commands/run.rs | cat -nRepository: jdx/aube
Length of output: 2559
🏁 Script executed:
# Verify the exact function signature of get_matches_from and whether it exits
rg 'get_matches_from' crates/aube/src/lib.rs -B 2 -A 2Repository: jdx/aube
Length of output: 656
🏁 Script executed:
# Check clap's behavior - does get_matches_from actually exit or does try_get_matches_from exist?
# Look at how clap is being used and if there's error handling
sed -n '640,660p' crates/aube/src/lib.rs | cat -nRepository: jdx/aube
Length of output: 1381
🏁 Script executed:
# Verify the exact line number of the process::exit(130) call in run.rs
grep -n 'std::process::exit(130)' crates/aube/src/commands/run.rsRepository: jdx/aube
Length of output: 146
🏁 Script executed:
# Check if there are other places where the interactive picker might hard-exit
rg 'demand::Select|demand::' crates/aube/src/commands/ -B 2 -A 5Repository: jdx/aube
Length of output: 7560
🏁 Script executed:
# Check update.rs for the same pattern around the MultiSelect picker
sed -n '1040,1055p' crates/aube/src/commands/update.rs | cat -nRepository: jdx/aube
Length of output: 621
🏁 Script executed:
# Check audit.rs for similar issues
sed -n '305,320p' crates/aube/src/commands/audit.rs | cat -nRepository: jdx/aube
Length of output: 683
🏁 Script executed:
# Verify the return type and flow of inner_main to confirm it should handle these gracefully
sed -n '595,615p' crates/aube/src/lib.rs | cat -nRepository: jdx/aube
Length of output: 1205
Multiple hard-exit paths still violate the embed-safe contract. The PR's docstring at crates/aube/src/lib.rs#L527-L538 claims cli_main returns an exit code without terminating the process, but several code paths still call std::process::exit() directly, hard-killing any in-process embedder.
-
crates/aube/src/lib.rs#L648:Cli::command().get_matches_from()retains clap's built-in print-and-exit behavior for parse errors,--help, and--version. Replace with the fallibletry_get_matches_from()API to propagate errors as return values. -
Demand picker cancellations (Ctrl-C / Esc) still call
std::process::exit(130)at three locations:crates/aube/src/commands/run.rs#L292crates/aube/src/commands/update.rs#L1048crates/aube/src/commands/audit.rs#L311
Propagate the interrupt code through the exit-code return path instead of hard-exiting.
📍 Affects 2 files
crates/aube/src/lib.rs#L527-L538(this comment)crates/aube/src/commands/run.rs#L275-L276
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/aube/src/lib.rs` around lines 527 - 538, The `cli_main` function's
docstring claims to be embed-safe by returning exit codes without
hard-terminating the process, but multiple code paths violate this contract by
calling `std::process::exit()` directly. In `crates/aube/src/lib.rs` at line
648, replace the `Cli::command().get_matches_from()` call with
`try_get_matches_from()` to propagate argument parsing errors as return values
instead of allowing clap's built-in print-and-exit behavior for parse errors,
`--help`, and `--version`. Additionally, in the demand picker cancellation
handlers at `crates/aube/src/commands/run.rs` line 292,
`crates/aube/src/commands/update.rs` line 1048, and
`crates/aube/src/commands/audit.rs` line 311, replace the
`std::process::exit(130)` calls with code that propagates the interrupt exit
code (130) through the function's return path instead of hard-exiting.
…verbs; route through prog()/cmd() The brand lint exempted ALL tracing:: calls, but warn!/error! surface to the user (the default subscriber prints them), so a command hint inside one is as user-facing as the same hint in bail!/eprintln!. Split the exemption by severity: debug!/trace!/info! stay exempt (internal dev logs may contain aube freely), warn!/error! are now scanned with the identical aube <verb> verb matcher (not a blanket contains-aube — a warn! mentioning aube for a non-command reason never trips). Route the one real leak (config-load warning) through prog(). Default-preserving: lint additions don't change runtime behavior; the prog() call renders 'aube' byte-for-byte under the default profile.
| /// these — so prose like "aube has no shims" or "let aube do it" never trips | ||
| /// (those words aren't commands), keeping the lint conservative. | ||
| const VERBS: &[&str] = &[ | ||
| "install", |
There was a problem hiding this comment.
hmm this seems like a painful thing to maintain
…stead of a hardcoded list The brand-literal lint matched user-facing `aube <verb>` leaks against a hand-maintained VERBS array — a real maintenance burden (jdx, jdx#888/jdx#897): adding or renaming a CLI verb meant remembering to touch the lint, and the list had already drifted (it carried `node`/`setup`, which are not subcommands, and missed real verbs like `version`/`cache`/`diag`/`sponsors`). Derive the verb set from `aube::command()` instead — walk the clap command tree recursively, collecting every subcommand name plus its visible and hidden aliases. The command surface is now the single source of truth, so the lint self-maintains; nested chains (`store prune`, `config get`) and all aliases are covered automatically. Test-only change; no runtime behavior change, standalone aube unaffected.
…rocess::exit The embeddable command layer (aube::commands::*::run and the script/bin helpers they share) called std::process::exit directly on a failing command or a non-zero child status. When a host embeds aube as a library and drives those run() fns in-process, those exits hard-kill the host's whole process. Thread a propagatable exit code up through the command layer instead: the converted run() / helper signatures return miette::Result<Option<i32>> (Some(code) = "exit with this code", None = success). The dispatcher in async_main maps Some(code) to the existing Result<Option<i32>> return, and the binary's main.rs keeps the single std::process::exit. Standalone exit codes are unchanged byte-for-byte. Converted command-level sites: run/exec (parallel-collector + leaf child-exit propagation), peers, doctor, check, dlx, create, outdated, audit (degraded=2, fix-remaining=1, advisories=1), plus the run_script / exec_bin / exec_script shared helpers and their callers (clean, restart, install_test, lifecycle, recursive, External). SIGINT exit(130) in the interactive pickers (run prompt, audit/update selectors) and the main.rs re-exec are left as genuinely-terminal paths. Add e2e tests pinning the standalone exit-code contract: run propagates a failing script's exact code, --if-present on a missing script exits 0, and a failing pre-script short-circuits with its code before the main script runs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
05a8538 to
208bb35
Compare
| // Match pnpm: exit 1 when any dependency is outdated so CI patterns | ||
| // like `aube outdated || exit 1` behave the same. The code is | ||
| // returned for the binary's single `std::process::exit` rather than | ||
| // exited in place, keeping the command embed-safe. | ||
| if run_one(&cwd, args, None).await? { | ||
| Ok(Some(1)) | ||
| } else { | ||
| Ok(None) | ||
| } |
There was a problem hiding this comment.
aube outdated exit-code is silently changed on the non-filtered path
Before this PR, outdated::run (no workspace filter) called run_one and discarded its bool return, always exiting 0. The filtered path (run_filtered) already had std::process::exit(1), so that path correctly exits 1 — but the new Ok(Some(1)) on line 152 is net-new behaviour on the non-filtered path, not a conversion of a pre-existing exit call.
The PR description says "Standalone aube behaviour is byte-for-byte unchanged", yet now aube outdated exits 1 whenever the non-filtered report finds outdated packages. Any CI job that today runs aube outdated && do_something (relying on the always-zero exit) will silently start failing after this merges.
Stacked on #888 — the substantive change in this PR is the single top commit
05a8538. The diff currently includes #888'sembedder-profile-fixescommits because this branch is stacked on that unmerged branch; once #888 merges the diff auto-cleans to just the exit-sweep changes.What this does
Converts 12 command-level
std::process::exitcall sites toResult<Option<i32>>, so that embedding aube as a library does not hard-kill the host process. Standalone aube behavior is byte-for-byte unchanged —main.rsunwraps the returned exit code and exits with it.Intentionally left terminal (not converted):
ctrlcregistered inmain.rs) — signal handling is inherently process-levelThree end-to-end tests lock the contract: zero-exit on success, non-zero on install failure, non-zero on unknown subcommand.
Review guidance
Please focus on commit
05a8538. The preceding commits belong to #888 and have already been reviewed there. Once #888 merges, only05a8538will show in this diff.This comment was generated by Claude.
Summary by CodeRabbit
New Features
Bug Fixes & Improvements