Skip to content

refactor(commands): return exit codes instead of process::exit (lib-embed safety)#897

Open
colinhacks wants to merge 14 commits into
jdx:mainfrom
nubjs:process-exit-sweep
Open

refactor(commands): return exit codes instead of process::exit (lib-embed safety)#897
colinhacks wants to merge 14 commits into
jdx:mainfrom
nubjs:process-exit-sweep

Conversation

@colinhacks

@colinhacks colinhacks commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Stacked on #888 — the substantive change in this PR is the single top commit 05a8538. The diff currently includes #888's embedder-profile-fixes commits 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::exit call sites to Result<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.rs unwraps the returned exit code and exits with it.

Intentionally left terminal (not converted):

  • SIGINT handler (ctrlc registered in main.rs) — signal handling is inherently process-level
  • Re-exec paths — the re-exec mechanism replaces the current process by design

Three 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, only 05a8538 will show in this diff.

This comment was generated by Claude.

Summary by CodeRabbit

  • New Features

    • Added embedder-aware branding/configuration, including user-facing program/command name rendering.
    • Added embedder-scoped environment-variable support for toggles and configuration knobs.
  • Bug Fixes & Improvements

    • Improved exit-code handling across CLI commands and script lifecycles for reliable scripting/automation.
    • Updated virtual store, cache, runtime, and adaptive-state paths to use embedder-derived namespaces/names.
    • Refreshed user messages to consistently reference the active tool name and command suggestions.
    • Extended integration coverage for branded identity and env-prefix behavior.

colinhacks and others added 11 commits June 13, 2026 17:44
…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>
@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 7bdfc0e0-b91a-42df-bdea-fadf30a7dfdb

📥 Commits

Reviewing files that changed from the base of the PR and between 05a8538 and 208bb35.

📒 Files selected for processing (16)
  • crates/aube/src/commands/audit.rs
  • crates/aube/src/commands/check.rs
  • crates/aube/src/commands/clean.rs
  • crates/aube/src/commands/config/aube_config.rs
  • crates/aube/src/commands/create.rs
  • crates/aube/src/commands/dlx.rs
  • crates/aube/src/commands/doctor.rs
  • crates/aube/src/commands/exec.rs
  • crates/aube/src/commands/install_test.rs
  • crates/aube/src/commands/outdated.rs
  • crates/aube/src/commands/peers.rs
  • crates/aube/src/commands/restart.rs
  • crates/aube/src/commands/run.rs
  • crates/aube/src/lib.rs
  • crates/aube/tests/brand_literal_lint.rs
  • crates/aube/tests/e2e.rs
✅ Files skipped from review due to trivial changes (1)
  • crates/aube/src/commands/config/aube_config.rs
🚧 Files skipped from review as they are similar to previous changes (13)
  • crates/aube/tests/e2e.rs
  • crates/aube/src/commands/create.rs
  • crates/aube/src/commands/dlx.rs
  • crates/aube/src/commands/check.rs
  • crates/aube/src/commands/install_test.rs
  • crates/aube/src/commands/peers.rs
  • crates/aube/src/commands/clean.rs
  • crates/aube/src/commands/audit.rs
  • crates/aube/src/commands/doctor.rs
  • crates/aube/src/commands/restart.rs
  • crates/aube/src/commands/outdated.rs
  • crates/aube/src/commands/exec.rs
  • crates/aube/src/commands/run.rs

📝 Walkthrough

Walkthrough

This 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.

Changes

Embedder branding and CLI exit codes

Layer / File(s) Summary
Embedder identity and env contracts
crates/aube-util/src/identity.rs, crates/aube-util/src/env.rs, crates/aube-util/src/lib.rs, crates/aube-util/tests/*, crates/aube-*/tests/*
Embedder gains config_env_prefix, aube-util adds embedder_env/config_env plus prog()/cmd(), and tests/fixtures cover default and branded profiles.
Embedder-aware paths and env lookup adoption
crates/aube-linker/..., crates/aube-runtime/..., crates/aube-store/..., crates/aube-util/src/{adaptive,concurrency,diag,hash,http/*,snapshot}.rs, crates/aube/src/commands/{install/*,login,settings_context}.rs, crates/aube/src/update_check.rs
Path derivation, cache namespaces, runtime directories, feature toggles, and override env vars now resolve through the active embedder’s prefixes and namespaces.
Embedder-branded names, paths, and namespaces
crates/aube/src/commands/{global,deploy/injection,patch,doctor}.rs, crates/aube/src/{progress/*,runtime.rs,self_version.rs,deprecations.rs,engines.rs}
Filesystem sidecars, injected directories, manifest/runtime labels, progress banners, engine keys, and version-related output now use embedder-derived names and namespaces.
CLI branding sweep and lint coverage
crates/aube/src/commands/*, crates/aube/tests/brand_literal_lint.rs
User-facing command strings across command modules now format command names with aube_util::cmd()/prog(), and a test scans command sources for hardcoded aube <verb> literals.
Exit-code propagation to main
crates/aube/src/commands/{audit,check,clean,create,dlx,doctor,exec,install_test,outdated,peers,restart,run}.rs, crates/aube/src/lib.rs, crates/aube/src/main.rs, crates/aube/tests/e2e.rs
Command runners return Option<i32>, dispatcher paths propagate explicit exit codes through cli_main, and e2e tests cover run exit-code and lifecycle short-circuit behavior.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~100 minutes

Possibly related PRs

  • jdx/aube#848: Touches crates/aube/src/commands/dlx.rs, which is part of the same command-exit and install/error-message area.
  • jdx/aube#862: Introduces the embedder identity and env-prefix plumbing that this PR extends across paths, branding, and env lookup.

Suggested labels

keep-open

Poem

🐇 I hopped through paths once named the same,
and painted each with embedder flame.
I tucked exit codes in a careful pack,
so commands return and wander back.
No hardcoded burrows left in sight—
just branded trails by moonlit night.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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::exit ownership to the standalone binary (main.rs), returning exit codes from cli_main and multiple commands.
  • Introduce branding helpers (aube_util::prog, aube_util::cmd) and update user-facing messages to use them instead of hardcoded aube ....
  • 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() {
Comment on lines +186 to +203
// 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();
}
Comment on lines 43 to 45
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"
);
Comment on lines +541 to +545
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
colinhacks added a commit to nubjs/nub that referenced this pull request Jun 14, 2026
…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-apps

greptile-apps Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR refactors 12 command-level std::process::exit call sites to return miette::Result<Option<i32>>, so that embedding aube as a library does not hard-kill the host process. main.rs now owns the single std::process::exit(aube::cli_main(embedder)) call, and cli_main/cli_main_with_defaults are annotated #[must_use] to prevent embedders from silently discarding the exit code.

  • run.rs, exec.rs, dlx.rs, audit.rs, check.rs, doctor.rs, peers.rs, and lifecycle commands all cleanly swap std::process::exit(code) for return Ok(Some(code)), with async_main in lib.rs propagating each via if let Some(code) dispatch arms.
  • outdated.rs's run_filtered path is correctly converted, but the non-filtered run() path adds a new Ok(Some(1)) where none existed before — the old code silently discarded run_one's bool and always returned Ok(()). This is a real exit-code change, not a conversion of an existing process::exit.
  • Three new e2e tests pin exact exit-code propagation for a failing script, --if-present no-op, and pre-script short-circuit.

Confidence Score: 4/5

Safe 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

Filename Overview
crates/aube/src/main.rs Correctly moves the single std::process::exit here, calling cli_main for the exit code.
crates/aube/src/lib.rs Converts cli_main/cli_main_with_defaults to return i32, adds #[must_use], and routes async_main's Option<i32> to unwrap_or(0) in inner_main. All dispatch arms in async_main properly handle Option<i32> returns.
crates/aube/src/commands/run.rs Converts all std::process::exit sites to return Ok(Some(code)). The exec_optional return type is now Option<Option<i32>> — already flagged in a prior review thread.
crates/aube/src/commands/exec.rs Converts two std::process::exit sites (sequential and parallel fanout) to Ok(Some(code)). Sequential bail-on-first-nonzero is preserved.
crates/aube/src/commands/outdated.rs Converts run_filtered's std::process::exit(1) correctly, but also adds a new exit-1 on the non-filtered run path where none existed before — a silent behavior change contradicting the PR's stated "byte-for-byte unchanged" goal.
crates/aube/src/commands/dlx.rs Converts the single std::process::exit call (failed dlx binary) to Ok(Some(code)); scratch dir cleanup (drop(tmp)) is correctly placed before the return.
crates/aube/src/commands/audit.rs All three std::process::exit sites (degraded status exit 2, remaining-fixes exit 1, advisory-threshold exit 1) cleanly converted to Ok(Some(code)).
crates/aube/src/commands/clean.rs Converted to Option<i32> but always returns Ok(None) — this had no std::process::exit before. Previously flagged; not repeated here.
crates/aube/tests/e2e.rs Adds three new end-to-end tests pinning exact exit-code propagation for failing scripts, --if-present no-op, and pre-script short-circuit.
crates/aube/src/commands/restart.rs Converts to Option<i32>, correctly short-circuits on a failing stop before start runs.
crates/aube/src/commands/install_test.rs Correctly propagates run_script's Option<i32> as the function's last expression.

Reviews (2): Last reviewed commit: "refactor(commands): return exit codes fr..." | Re-trigger Greptile

Comment thread crates/aube/src/commands/run.rs
Comment on lines 44 to 57
]
}

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)?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 win

Test 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 the packageManager field from the active embedder (id.name@id.version). The test passes for standalone aube where embedder().name == "aube" and embedder().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 win

Interactive cancel still hard-exits the host process.

audit --interactive can still terminate an embedding host here via std::process::exit(130). That exit code needs to be plumbed back through run() 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-errors no 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 value

Consider adding validation for the name field 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, unlike lockfile_basename (which is validated in set_embedder at lines 190-200), there is no debug_assert! to enforce this invariant.

Adding a validation check in set_embedder would 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 win

Update comment to reflect embedder-aware variable name.

The comment states "The name is deliberately AUBE_INTERNAL_*" but with the embedder-aware embedder_env() helper, the actual environment variable name is now <BRAND>_INTERNAL_FORCE_FULL_PACKUMENT for embedders and AUBE_INTERNAL_FORCE_FULL_PACKUMENT for 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

📥 Commits

Reviewing files that changed from the base of the PR and between 41ef43e and 05a8538.

📒 Files selected for processing (98)
  • crates/aube-linker/src/builder.rs
  • crates/aube-linker/src/lib.rs
  • crates/aube-lockfile/tests/custom_lock_filename.rs
  • crates/aube-manifest/src/workspace/edits.rs
  • crates/aube-registry/src/client/http.rs
  • crates/aube-registry/tests/user_agent_product.rs
  • crates/aube-resolver/src/primer.rs
  • crates/aube-runtime/src/discover.rs
  • crates/aube-runtime/src/paths.rs
  • crates/aube-runtime/src/self_install.rs
  • crates/aube-scripts/tests/user_agent_product.rs
  • crates/aube-store/src/cas.rs
  • crates/aube-store/src/tarball.rs
  • crates/aube-util/src/adaptive.rs
  • crates/aube-util/src/concurrency.rs
  • crates/aube-util/src/diag.rs
  • crates/aube-util/src/diag_kernel.rs
  • crates/aube-util/src/env.rs
  • crates/aube-util/src/hash.rs
  • crates/aube-util/src/http/prewarm.rs
  • crates/aube-util/src/http/race.rs
  • crates/aube-util/src/http/resolve.rs
  • crates/aube-util/src/http/ticket_cache.rs
  • crates/aube-util/src/identity.rs
  • crates/aube-util/src/lib.rs
  • crates/aube-util/src/snapshot.rs
  • crates/aube-util/tests/embedder_env_brand_gate.rs
  • crates/aube-util/tests/source_branding_brand_gate.rs
  • crates/aube/src/commands/add_supply_chain.rs
  • crates/aube/src/commands/approve_builds.rs
  • crates/aube/src/commands/audit.rs
  • crates/aube/src/commands/auto_install.rs
  • crates/aube/src/commands/cache.rs
  • crates/aube/src/commands/cat_file.rs
  • crates/aube/src/commands/cat_index.rs
  • crates/aube/src/commands/check.rs
  • crates/aube/src/commands/clean.rs
  • crates/aube/src/commands/completion.rs
  • crates/aube/src/commands/config/aube_config.rs
  • crates/aube/src/commands/config/delete.rs
  • crates/aube/src/commands/config/explain.rs
  • crates/aube/src/commands/config/mod.rs
  • crates/aube/src/commands/config/set.rs
  • crates/aube/src/commands/config/tui.rs
  • crates/aube/src/commands/create.rs
  • crates/aube/src/commands/deploy/injection.rs
  • crates/aube/src/commands/deploy/mod.rs
  • crates/aube/src/commands/deploy/rewrite.rs
  • crates/aube/src/commands/deploy/staging.rs
  • crates/aube/src/commands/deprecations.rs
  • crates/aube/src/commands/dist_tag.rs
  • crates/aube/src/commands/dlx.rs
  • crates/aube/src/commands/doctor.rs
  • crates/aube/src/commands/exec.rs
  • crates/aube/src/commands/fetch.rs
  • crates/aube/src/commands/find_hash.rs
  • crates/aube/src/commands/global.rs
  • crates/aube/src/commands/init.rs
  • crates/aube/src/commands/install/fetch.rs
  • crates/aube/src/commands/install/mod.rs
  • crates/aube/src/commands/install/node_gyp_bootstrap.rs
  • crates/aube/src/commands/install/resolve.rs
  • crates/aube/src/commands/install/unreviewed_builds.rs
  • crates/aube/src/commands/install/workspace.rs
  • crates/aube/src/commands/install_test.rs
  • crates/aube/src/commands/licenses.rs
  • crates/aube/src/commands/link.rs
  • crates/aube/src/commands/list.rs
  • crates/aube/src/commands/login.rs
  • crates/aube/src/commands/outdated.rs
  • crates/aube/src/commands/patch.rs
  • crates/aube/src/commands/peers.rs
  • crates/aube/src/commands/project_lock.rs
  • crates/aube/src/commands/prune.rs
  • crates/aube/src/commands/publish.rs
  • crates/aube/src/commands/query.rs
  • crates/aube/src/commands/rebuild.rs
  • crates/aube/src/commands/recursive.rs
  • crates/aube/src/commands/remove.rs
  • crates/aube/src/commands/restart.rs
  • crates/aube/src/commands/run.rs
  • crates/aube/src/commands/runtime.rs
  • crates/aube/src/commands/sbom.rs
  • crates/aube/src/commands/settings_context.rs
  • crates/aube/src/commands/unlink.rs
  • crates/aube/src/commands/update.rs
  • crates/aube/src/commands/why.rs
  • crates/aube/src/deprecations.rs
  • crates/aube/src/engines.rs
  • crates/aube/src/lib.rs
  • crates/aube/src/main.rs
  • crates/aube/src/progress/ci.rs
  • crates/aube/src/progress/mod.rs
  • crates/aube/src/runtime.rs
  • crates/aube/src/self_version.rs
  • crates/aube/src/update_check.rs
  • crates/aube/tests/brand_literal_lint.rs
  • crates/aube/tests/e2e.rs

Comment on lines 256 to 263
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")
));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

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)) when first_exit is set, completely bypassing first_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.

Comment thread crates/aube/src/lib.rs
Comment on lines +527 to +538
/// **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(..))`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 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.rs

Repository: 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 -n

Repository: 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 -n

Repository: 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 -n

Repository: 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 -n

Repository: 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.toml

Repository: 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 -n

Repository: 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 1

Repository: 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 -n

Repository: 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 -n

Repository: 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 2

Repository: 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 -n

Repository: 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.rs

Repository: 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 5

Repository: 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 -n

Repository: 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 -n

Repository: 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 -n

Repository: 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.

  1. 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 fallible try_get_matches_from() API to propagate errors as return values.

  2. Demand picker cancellations (Ctrl-C / Esc) still call std::process::exit(130) at three locations:

    • crates/aube/src/commands/run.rs#L292
    • crates/aube/src/commands/update.rs#L1048
    • crates/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.

Comment thread crates/aube/tests/brand_literal_lint.rs Outdated
…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.
Comment thread crates/aube/tests/brand_literal_lint.rs Outdated
/// 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",

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

hmm this seems like a painful thing to maintain

@colinhacks

Copy link
Copy Markdown
Contributor Author

Same answer as on #888 (this PR is stacked on it, so it inherits the fix): making the brand-lint VERBS derive from the clap command tree — it walks the subcommands + aliases instead of a hand-maintained list, so it self-maintains. Landing it on the #888 base; it flows into this PR automatically.

colinhacks and others added 2 commits June 15, 2026 18:44
…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>
@colinhacks colinhacks force-pushed the process-exit-sweep branch from 05a8538 to 208bb35 Compare June 16, 2026 01:50
@colinhacks

Copy link
Copy Markdown
Contributor Author

Rebased this on the updated #888 tip, so it now carries the dynamic-VERBS change too — the brand lint here derives the verb set by walking aube::command() (top-level verbs + aliases + nested chains) instead of the hardcoded array. Same fix you flagged on #888, now reflected here.

Comment on lines +147 to +155
// 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)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 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.

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.

4 participants