Skip to content

feat(global)!: keep aube's global dirs under its own data root - #1231

Open
jdx wants to merge 3 commits into
mainfrom
claude/github-discussion-1219-96f2e4
Open

feat(global)!: keep aube's global dirs under its own data root#1231
jdx wants to merge 3 commits into
mainfrom
claude/github-discussion-1219-96f2e4

Conversation

@jdx

@jdx jdx commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Aube's global install layout lived in pnpm's directories. The default root was $XDG_DATA_HOME/pnpm, ~/Library/pnpm on macOS, or %LOCALAPPDATA%\pnpm on Windows, and PNPM_HOME was consulted ahead of the platform default — so aube add -g linked bins into a directory another package manager owns. The hardcoded pnpm leaf also ignored the embedder's data_namespace, meaning a tool shipping under its own brand still installed into .../pnpm.

Status: not scheduled. Per @jdx, it's unclear when or if this lands — it would require a v2. Opening it so the shape of the change is reviewable, not to merge it as-is.

New layout

Globals now hang off the same data root the store, Node runtimes, and shims already use:

<data_root>/bin           # globalBinDir — the directory you put on PATH
<data_root>/global-aube   # globalDir — physical installs + hash pointers

<data_root> is $XDG_DATA_HOME/<data_namespace>, falling back to ~/.local/share/<ns> (%LOCALAPPDATA%\<ns> on Windows) — the same resolution aube_store::dirs::store_dir uses. aube prefix -g prints <data_root>; the bin dir is a bin/ child of it so the PATH entry holds executables and nothing else, and package installs are a sibling rather than nested inside a PATH directory.

AUBE_HOME keeps its existing meaning — when set it is the bin dir, with installs in a global-aube/ subdir. Only the default moved, so anyone who already opted into AUBE_HOME sees no change.

Dropping the macOS ~/Library/pnpm special case also means an explicit XDG_DATA_HOME is finally honored there. That was the one place aube ignored XDG on macOS — the second half of Discussion #1219.

Breaking

  • aube add -g installs into <data_root>/global-aube and links bins into <data_root>/bin.
  • PNPM_HOME is no longer read.
  • Packages installed globally by an earlier version are not migrated. They stay on disk and their bins keep working while the old directory is on PATH, but aube list -g / aube remove -g no longer see them. Recovery is aube add -g <pkg> after putting the new bin dir on PATH, or AUBE_HOME=<old dir> to pin the previous layout.

Both failure modes are otherwise silent, so each gets a warning code:

Code Fires when
WARN_AUBE_GLOBAL_DIR_LEGACY_LOCATION Globals exist in a pre-2.0 pnpm-named location and none exist in the new one. The old directory is only ever read — never written to, never deleted.
WARN_AUBE_GLOBAL_BIN_DIR_NOT_ON_PATH add -g linked a bin into a directory absent from $PATH. This gap predates the PR (nothing warned, so a relocated default would silently produce "command not found"), and it's independently droppable if you'd rather not take it here.

Scope

Reading pnpm's files is untouched — pnpm-lock.yaml, pnpm-workspace.yaml, and ~/.config/pnpm/auth.ini are compatibility surfaces aube reads, not directories it owns. I audited every other aube-owned path (aube-store::dirs, aube-runtime::paths, tool_shims, adaptive, config); global.rs was the only one using a pnpm name.

Also included

The first commit is an unrelated-but-adjacent bugfix from the same discussion. It is now also open standalone as #1232 — review it there, since it shouldn't wait on a v2. This branch keeps the identical commit so it stays testable on its own; whichever merges first, the other rebases cleanly. The bug: with the global virtual store on (the default outside CI), remove -g left every global bin behind as a dangling symlink. unlink_bins decided ownership by canonicalizing the bin's target, which resolves through .aube/<dep_path> into <cacheDir>/virtual-store/... — never under the install dir, so every bin looked like it belonged to another install. Reproduced on Linux, not macOS-specific. The existing test missed it because assert_file_not_exists is [ -f ], which follows symlinks and passes for a dangling one.

Testing

  • test/global_install.bats: 30 pass, including 6 new tests covering the default paths, the ~/.local/share fallback, PNPM_HOME being ignored, both warnings, and that the legacy directory is left intact.
  • cargo test --workspace, cargo clippy --all-targets -- -D warnings, cargo fmt --check: clean.
  • Commit 1 verified building and passing (24/24 bats) in isolation, so the split is bisectable.

Closes the XDG half of #1219.

AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable.


Note

High Risk
Breaking default paths and PATH expectations for all global installs; also changes bin unlink semantics on remove, though covered by new bats tests.

Overview
Breaking: Default global layout moves off pnpm-owned paths (PNPM_HOME, …/pnpm) to the same data root as the store (<data_root>/bin for bins, <data_root>/global-aube for installs). AUBE_HOME behavior is unchanged; PNPM_HOME is no longer used for resolution.

Adds WARN_AUBE_GLOBAL_DIR_LEGACY_LOCATION when globals exist only under old pnpm-era homes (read-only detection; suggests reinstall or AUBE_HOME), and WARN_AUBE_GLOBAL_BIN_DIR_NOT_ON_PATH after add -g when the bin dir is not on PATH.

Fixes remove -g leaving dangling global bin symlinks when the global virtual store is on: unlink_bins now treats ownership with lex-normalized paths under the install dir instead of relying on canonicalization into the shared virtual store.

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

Summary by CodeRabbit

  • New Features

    • Global installations now use Aube-managed directories with platform-aware defaults.
    • Added warnings for legacy installation locations and global binaries missing from PATH.
    • Improved cleanup of globally installed binaries, including symlinked entries.
  • Bug Fixes

    • Global installation migration and removal now handle legacy paths more reliably.
  • Documentation

    • Clarified global directory defaults, AUBE_HOME behavior, and PNPM_HOME handling.

jdx and others added 2 commits August 4, 2026 21:48
`unlink_bins` decided whether a global bin belonged to the install being
removed by canonicalizing the symlink target and requiring it to live
under the install dir. With the global virtual store enabled — the
default outside CI — `node_modules/<alias>` resolves through
`.aube/<dep_path>` into `<cacheDir>/virtual-store/<dep>-<hash>`, so the
canonical target is never under the install dir: every global bin was
read as owned by some other install and left behind, dangling once
`remove -g` deleted the install dir.

Check the lexically-normalized target first, the way the regular-file
shim branch already did, and keep canonicalization as a fallback for
bins linked by older versions. A bin overwritten by a later `add -g`
still points at that install's path, so ownership semantics hold.

The existing test missed this because `assert_file_not_exists` is
`[ -f ]`, which follows symlinks and therefore passes for a dangling
one; it now also asserts `[ ! -L ]`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
aube's global install layout lived in pnpm's directories: the default
root was `$XDG_DATA_HOME/pnpm`, `~/Library/pnpm` on macOS, or
`%LOCALAPPDATA%\pnpm` on Windows, and `PNPM_HOME` was honored ahead of
the platform default. Global bins were therefore linked into a directory
another package manager owns, and the hardcoded `pnpm` leaf ignored the
embedder's `data_namespace` — an embedder shipping under its own brand
still installed into `.../pnpm`.

Globals now hang off the same data root the store, Node runtimes, and
shims already use:

    <data_root>/bin           # globalBinDir — the dir you put on PATH
    <data_root>/global-aube   # globalDir — physical installs + pointers

where `<data_root>` is `$XDG_DATA_HOME/<data_namespace>`, falling back
to `~/.local/share/<ns>` (`%LOCALAPPDATA%\<ns>` on Windows). `PNPM_HOME`
is no longer read. `AUBE_HOME` keeps its meaning — when set it is the
bin dir, with installs in a `global-aube/` subdir of it.

Dropping the macOS `~/Library/pnpm` special case also means an explicit
`XDG_DATA_HOME` is now honored there, which was the one place aube
ignored it on macOS (Discussion #1219).

Two warnings cover the migration, since both failure modes are
otherwise silent:

- `WARN_AUBE_GLOBAL_DIR_LEGACY_LOCATION` fires when globals are found
  in a pre-2.0 pnpm-named location and none exist in the new one. The
  old directory is only read, never written to or deleted.
- `WARN_AUBE_GLOBAL_BIN_DIR_NOT_ON_PATH` fires when `add -g` links a
  bin into a directory absent from `$PATH` — previously that reported
  success and produced a command not found.

Reading pnpm's *files* is untouched: `pnpm-lock.yaml`,
`pnpm-workspace.yaml`, and `~/.config/pnpm/auth.ini` are compat
surfaces, not directories aube owns.

BREAKING CHANGE: `aube add -g` installs into `<data_root>/global-aube`
and links bins into `<data_root>/bin` instead of pnpm's directories, and
`PNPM_HOME` is no longer consulted. Packages installed globally by an
earlier version are not migrated: they stay on disk, their bins keep
working if the old directory is still on `PATH`, but `aube list -g` and
`aube remove -g` no longer see them. Reinstall them with
`aube add -g <pkg>` after putting the new bin dir on `PATH`, or set
`AUBE_HOME` to the old location to keep the previous layout.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jdx jdx added the breaking label Aug 4, 2026
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

Global installation layout

Layer / File(s) Summary
Aube-owned global layout resolution
crates/aube/src/commands/global.rs, crates/aube-settings/settings.toml, test/global_install.bats
Global bins and packages now use Aube-owned data roots, branded home overrides, and embedder-specific package directories. Tests cover AUBE_HOME, XDG defaults, fallback paths, and ignored PNPM_HOME.
Legacy location warning registration
crates/aube-codes/src/warnings.rs, crates/aube/src/commands/global.rs, docs/error-codes.data.json, test/global_install.bats
Legacy pnpm-named directories are detected and reported with registered global-install warning codes. Tests cover warning suppression after migration.
PATH reporting and binary cleanup
crates/aube/src/commands/global.rs, crates/aube/src/commands/add/global.rs, test/global_install.bats
Global commands warn when the bin directory is outside PATH. Symlink cleanup now checks lexical and canonical ownership, with tests for dangling links.
Global directory documentation
docs/settings/index.md
The settings documentation describes AUBE-specific paths, platform fallbacks, and PNPM_HOME exclusion.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AubeAddGlobal
  participant GlobalLayout
  participant FileSystem
  participant WarningCodes
  AubeAddGlobal->>GlobalLayout: resolve global package and bin directories
  GlobalLayout->>FileSystem: inspect current and legacy locations
  FileSystem-->>GlobalLayout: return directory and package state
  GlobalLayout->>WarningCodes: emit legacy-location warning when needed
  AubeAddGlobal->>GlobalLayout: link global binaries
  GlobalLayout->>WarningCodes: emit PATH warning when bin directory is absent from PATH
Loading

Possibly related PRs

  • jdx/aube#1193: Adds warning codes and registers them in ALL, but covers a different warning category.

Poem

A bunny hops through paths anew,
With bins in roots that Aube grew.
Old pnpm trails now raise a sign,
Missing PATHs get warned in time.
Clean links vanish, neat and bright.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: moving Aube's global directories under its own data root.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cursor cursor 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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

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

Reviewed by Cursor Bugbot for commit 4cd56fd. Configure here.

Comment thread crates/aube/src/commands/global.rs
@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR relocates global installations beneath aube’s own data root while preserving AUBE_HOME behavior and adds migration and PATH diagnostics.

  • Changes the default global package and binary directories and stops consulting PNPM_HOME.
  • Warns when legacy global installs are stranded or newly linked binaries are absent from PATH.
  • Corrects global-bin cleanup when virtual-store symlinks resolve outside the installation directory.
  • Adds matching warning-code documentation and global-install regression coverage.

Confidence Score: 5/5

The PR appears safe to merge with respect to the previously reported documentation issue.

No blocking failure remains; the PATH warning is now included in the documentation data with metadata matching the runtime warning registry.

Important Files Changed

Filename Overview
crates/aube/src/commands/global.rs Implements the new global layout, legacy-location and PATH warnings, and lexical ownership checks for bin cleanup.
crates/aube/src/commands/add/global.rs Emits the PATH diagnostic after a global installation links binaries.
crates/aube-codes/src/warnings.rs Registers both global-install warning codes and their user-facing descriptions.
docs/error-codes.data.json Adds the previously missing PATH warning entry and matches the runtime warning registry.
crates/aube-settings/settings.toml Documents the new global directory defaults and removal of PNPM_HOME precedence.
test/global_install.bats Covers directory resolution, migration warnings, PATH warnings, and dangling-symlink cleanup.

Reviews (2): Last reviewed commit: "fix(global): regenerate error-code docs ..." | Re-trigger Greptile

Comment thread crates/aube-codes/src/warnings.rs

@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: 4

🤖 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/global.rs`:
- Around line 226-230: Update warn_if_bin_dir_not_on_path to warn when PATH is
unset instead of returning early. Treat the missing environment variable as an
absent bin_dir entry, while preserving the existing path-checking behavior when
PATH is present.
- Around line 67-68: Update the package-directory resolution around setting_pkg
and default_pkg_dir so an explicitly configured globalDir is used directly
without appending pkg_subdir, while retaining the default_pkg_dir(&pkg_subdir)
behavior when no globalDir is configured. Ensure global list and remove
operations resolve existing installations under the configured directory.
- Around line 98-114: Update data_root so a configured XDG_DATA_HOME is checked
and returned before entering the Windows-specific LOCALAPPDATA fallback.
Preserve the existing platform-specific fallback behavior when no XDG data
directory is configured, including the current error handling and non-Windows
home-directory path.

In `@docs/error-codes.data.json`:
- Around line 911-916: Update docs/error-codes.data.json to add the missing
WARN_AUBE_GLOBAL_BIN_DIR_NOT_ON_PATH entry, matching its public registration in
crates/aube-codes/src/warnings.rs and the existing global-install warning
schema. Ensure the generated warning documentation includes both global-install
warnings.
🪄 Autofix

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: fe89fe63-c2ad-43eb-9b79-cb2bca8156bc

📥 Commits

Reviewing files that changed from the base of the PR and between 6233612 and 4cd56fd.

📒 Files selected for processing (7)
  • crates/aube-codes/src/warnings.rs
  • crates/aube-settings/settings.toml
  • crates/aube/src/commands/add/global.rs
  • crates/aube/src/commands/global.rs
  • docs/error-codes.data.json
  • docs/settings/index.md
  • test/global_install.bats

Comment thread crates/aube/src/commands/global.rs
Comment thread crates/aube/src/commands/global.rs Outdated
Comment thread crates/aube/src/commands/global.rs Outdated
Comment thread docs/error-codes.data.json
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Instruction counts

benchmark trend instructions Δ wall (min) Δ
graph ▃▁█▁▁ 17,531,105 → 17,533,453 +0.01% 4.65 → 4.58ms -1.50%
install ▆▆█▆▁ 134,034,601 → 124,928,311 -6.79% 31.98 → 30.07ms -5.96%
startup ▁██▄▆ 7,460,031 → 7,462,204 +0.03% 3.50 → 3.19ms -8.92%
tree ▁▆▆▄█ 17,714,593 → 17,723,245 +0.05% 4.60 → 4.15ms -9.84%

No instruction-count regression above 1%.

Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run.

Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.

ac22be64e64e vs c27198b1dc51 · measured on this runner, not pushed to the history.

Review follow-ups on the global-directory relocation:

- `docs/error-codes.data.json` was generated before
  `WARN_AUBE_GLOBAL_BIN_DIR_NOT_ON_PATH` was added, so the published
  code table was missing it and CI's `assert render produces no diff`
  gate failed. Regenerated via `mise run render`.
- An unset `PATH` made the not-on-PATH check return early instead of
  warning, even though nothing is reachable in that state. Extracted
  `bin_dir_on_path`, which treats a missing `PATH` as an empty search
  list, and unit-tested the listed / absent / unset cases.
- `data_root` now falls back to `XDG_DATA_HOME` (then `~/.local/share`)
  on Windows when `%LOCALAPPDATA%` is unset, rather than erroring —
  matching `aube_store::dirs::store_dir`. `%LOCALAPPDATA%` still takes
  precedence there so the global dir and the content store can't end up
  under different roots.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

jdx commented Aug 4, 2026

Copy link
Copy Markdown
Owner Author

The linux-x64-musl / linux-arm64-musl failures here are inherited from main, not caused by this PR.

#1226 bumped decmpfs to =0.1.2, which is the exact version the comment directly above that pin says fails to compile for *-unknown-linux-musl (its FICLONE path types the ioctl request as libc::c_ulonglibc::Ioctl on glibc, c_int on musl). This PR only surfaces it: it touches crates/aube-codes/**, which is one of the few path filters that triggers the ffi and node-addon workflows. #1226 touched only Cargo.toml and Cargo.lock, and neither workflow watches those files, so the musl matrix never ran for it.

Fix is in #1233, which re-pins to 0.1.0 and adds Cargo.toml / Cargo.lock to both workflows' path filters so dependency bumps can't skip the musl jobs again. Worth noting the musl packages are published from those jobs, so main would currently release without them.

This PR needs a rebase on top of that before its checks can go green. test-linux was a genuine failure of mine — stale docs/error-codes.data.json — and is fixed in ac22be6.

AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant