fix(update): support global updates#840
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughImplements ChangesGlobal Package Update Feature
Sequence DiagramsequenceDiagram
participant User
participant run as run(args)
participant run_global as run_global()
participant select as select_global_updates()
participant run_inner as run(global=false...)
participant shimlink as BinShimLinker
User->>run: aube update -g [pkg@spec...]
run->>run_global: dispatch to global branch
run_global->>run_global: resolve global layout
run_global->>run_global: scan installed global packages
run_global->>select: select_global_updates(requested, packages)
select->>run_global: return selected packages
loop For each selected package
run_global->>run_global: chdir to package install dir
run_global->>run_inner: call run(global=false, latest=true, exact=true, workspace=false)
run_inner->>run_inner: perform package update
run_global->>shimlink: link new bin shims
run_global->>shimlink: unlink stale bin shims
end
run_global->>User: report update results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR wires up
Confidence Score: 5/5The change is safe to merge; it is scoped entirely to the global update path and does not touch existing project-update logic. The global update path is new, well-isolated, and backed by BATS regressions. The cwd-mutation pattern (retarget_cwd / finish_filtered_workspace) is established in this codebase, and the sequential loop avoids the concurrency hazards that pattern would otherwise risk. No existing behaviour is altered. No files require special attention; the only observation is the --depth warning ordering in update.rs. Important Files Changed
Reviews (3): Last reviewed commit: "test(update): align filter global behavi..." | Re-trigger Greptile |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/global_install.bats (1)
71-76: 💤 Low valueConsider explicitly asserting package.json absence for test clarity.
The test verifies that the output doesn't contain "no package.json found" (line 76), which confirms the command succeeds outside a project. For additional clarity and defensive testing, consider explicitly asserting that
package.jsondoesn't exist in theno-projectdirectory after line 72.🧪 Optional defensive assertion
mkdir -p "$TEST_TEMP_DIR/no-project" cd "$TEST_TEMP_DIR/no-project" + assert_file_not_exists "package.json" run aube update -g🤖 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 `@test/global_install.bats` around lines 71 - 76, Add an explicit assertion that package.json is absent immediately after creating and cd-ing into the no-project directory (after mkdir -p "$TEST_TEMP_DIR/no-project" and cd "$TEST_TEMP_DIR/no-project") so the test defensively verifies the precondition before running run aube update -g; use the test framework's file-existence assertion (e.g., a bats refute_path_exists or an equivalent check) to assert "package.json" does not exist in that directory.
🤖 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.
Nitpick comments:
In `@test/global_install.bats`:
- Around line 71-76: Add an explicit assertion that package.json is absent
immediately after creating and cd-ing into the no-project directory (after mkdir
-p "$TEST_TEMP_DIR/no-project" and cd "$TEST_TEMP_DIR/no-project") so the test
defensively verifies the precondition before running run aube update -g; use the
test framework's file-existence assertion (e.g., a bats refute_path_exists or an
equivalent check) to assert "package.json" does not exist in that directory.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 12f2e9ac-63e2-423f-926c-adf3a01f0e58
📒 Files selected for processing (2)
crates/aube/src/commands/update.rstest/global_install.bats
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fe076db. Configure here.
| .into_iter() | ||
| .filter(|name| !linked_set.contains(name.as_str())) | ||
| .collect(); | ||
| super::global::unlink_bins(&info.install_dir, &layout.bin_dir, &stale_bins); |
There was a problem hiding this comment.
Lockfile-only breaks global bin relink
Medium Severity
run_global clones UpdateArgs without clearing lockfile_only, so nested run can refresh the lockfile and package.json while skipping node_modules materialization, then still runs link_bins against the stale install tree—global CLI shims may not match the versions just written.
Reviewed by Cursor Bugbot for commit fe076db. Configure here.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/global_install.bats (1)
67-84: 💤 Low valueConsider asserting the new version more explicitly.
Lines 82-83 verify that the package is still listed and the old version (
1.0.0) is gone, which indirectly confirms an update occurred. However, the test doesn't explicitly verify what the new version is. While this is understandable given version uncertainty, consider adding an assertion that verifies the version format or that the version is greater than1.0.0if the test infrastructure supports version comparison.For example, you could parse the version from
aube list -goutput and verify it's not1.0.0.🤖 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 `@test/global_install.bats` around lines 67 - 84, Update the test "aube update -g updates global packages outside a project" to more explicitly assert the new version: after running `run aube list -g` (and before the existing refute_output for "is-positive 1.0.0"), parse the reported version for "is-positive" from the `aube list -g` output and assert it is not "1.0.0" (or assert it matches a semver pattern like X.Y.Z and/or is greater than 1.0.0 if your test harness supports comparison). Keep references to the existing commands (`run aube list -g`, `assert_output --partial "is-positive "`, `refute_output --partial "is-positive 1.0.0"`) and add a single explicit check for the new version string/format.
🤖 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 `@test/global_install.bats`:
- Line 78: Replace the brittle human-readable output assertion that uses
assert_output --partial "is-positive: 1.0.0 ->" with a deterministic check of
the installed package version: run the CLI listing command (aube list -g --json
or aube list -g --parseable) and assert that the installed version for the
is-positive manifest key is the newer version (and/or keep the existing refute
that "is-positive 1.0.0" is not present); update the test to parse the
JSON/parseable output and assert the exact installed version instead of matching
the human-readable eprintln line.
---
Nitpick comments:
In `@test/global_install.bats`:
- Around line 67-84: Update the test "aube update -g updates global packages
outside a project" to more explicitly assert the new version: after running `run
aube list -g` (and before the existing refute_output for "is-positive 1.0.0"),
parse the reported version for "is-positive" from the `aube list -g` output and
assert it is not "1.0.0" (or assert it matches a semver pattern like X.Y.Z
and/or is greater than 1.0.0 if your test harness supports comparison). Keep
references to the existing commands (`run aube list -g`, `assert_output
--partial "is-positive "`, `refute_output --partial "is-positive 1.0.0"`) and
add a single explicit check for the new version string/format.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0146d606-e13c-4c04-954e-2bf4dbc1e60c
📒 Files selected for processing (2)
crates/aube/src/commands/update.rstest/global_install.bats
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/aube/src/commands/update.rs
| run aube update -g | ||
| assert_success | ||
| refute_output --partial "no package.json found" | ||
| assert_output --partial "is-positive: 1.0.0 ->" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if is-positive has versions newer than 1.0.0 in the test registry
# Check test/test_helper/common_setup for registry configuration
rg -nP 'AUBE_TEST_REGISTRY|is-positive' test/test_helper/
# Look for test registry fixture or package listing that confirms available versions
fd -t f -e json . test/ --exec rg -l '"is-positive"' {}Repository: endevco/aube
Length of output: 466
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the is-positive registry fixture for available versions
if [ -f "test/registry/storage/is-positive/package.json" ]; then
echo "=== package.json ==="
cat -n test/registry/storage/is-positive/package.json
else
echo "Missing: test/registry/storage/is-positive/package.json" >&2
exit 1
fi
# Search for other version metadata under the fixture directory
echo "=== files under test/registry/storage/is-positive ==="
ls -la test/registry/storage/is-positive
echo "=== grep version-like strings ==="
rg -n '"(version|versions)"|1\.0\.0|is-positive@|tarball|dist' test/registry/storage/is-positive -S || true
# If there is a manifest/index, locate it and print minimal relevant parts
fd -t f . test/registry/storage/is-positive | xargs -I{} sh -lc 'echo "--- {} ---"; head -n 60 {}'Repository: endevco/aube
Length of output: 22809
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show context around AUBE_TEST_REGISTRY usage
echo "=== common_setup.bash (context) ==="
nl -ba test/test_helper/common_setup.bash | sed -n '40,120p'
# Find where registry is started/configured and AUBE_TEST_REGISTRY is set
echo "=== Search for AUBE_TEST_REGISTRY assignments ==="
rg -n 'AUBE_TEST_REGISTRY' test/test_helper test -S
# Inspect global_install.bats around the first test case (lines ~60-90)
echo "=== global_install.bats (context) ==="
nl -ba test/global_install.bats | sed -n '50,110p'Repository: endevco/aube
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== common_setup.bash (context) ==="
cat -n test/test_helper/common_setup.bash | sed -n '40,140p'
echo "=== Search for AUBE_TEST_REGISTRY assignments/usages ==="
rg -n 'AUBE_TEST_REGISTRY' test/test_helper test -S
echo "=== global_install.bats (context around first test) ==="
cat -n test/global_install.bats | sed -n '50,110p'Repository: endevco/aube
Length of output: 9699
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== test/setup_suite.bash ==="
cat -n test/setup_suite.bash | sed -n '1,120p'
echo "=== test/registry/start.bash ==="
cat -n test/registry/start.bash | sed -n '1,140p'
echo "=== test/global_install.bats header/setup ==="
cat -n test/global_install.bats | sed -n '1,120p'Repository: endevco/aube
Length of output: 7015
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate where global update prints output and the exact formatting
rg -n --hidden --no-ignore-vcs 'not globally installed' .
rg -n --hidden --no-ignore-vcs '->' src test -S || true
# Narrow to update command implementation
rg -n --hidden --no-ignore-vcs '\bupdate\b' src -S | head -n 50
# Search for the specific message fragment "is-positive:" or the pattern "name: .*->"
rg -n --hidden --no-ignore-vcs ':\s*[0-9]+\.[0-9]+\.[0-9]+\s*->' src -S || true
rg -n --hidden --no-ignore-vcs '->' src/commands src -S || trueRepository: endevco/aube
Length of output: 733
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="crates/aube/src/commands/update.rs"
echo "=== rg ' -> ' in update.rs ==="
rg -n ' -> ' "$FILE" || true
echo "=== rg update.rs for 'is-positive:' style formatting patterns ==="
rg -n ':.*->' "$FILE" || true
rg -n '->\s*\$|format!\(|println!\(|writeln!\(' "$FILE" || true
echo "=== Inspect around the global-missing error line ==="
sed -n '840,910p' "$FILE"
echo "=== Inspect broader region near where success output might be produced ==="
# (Find next occurrences of '->' to choose window; if none, just dump a bit around printing logic)
rg -n '->' "$FILE" | head -n 20 || trueRepository: endevco/aube
Length of output: 6773
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="crates/aube/src/commands/update.rs"
echo "=== Find run_global print/format sites ==="
rg -n "run_global|GlobalPackageInfo|global" "$FILE" | head -n 80
echo "=== Search for printing code around run_global ==="
# Get approximate region by finding the function start line
start_line=$(rg -n "async fn run_global" "$FILE" | head -n 1 | cut -d: -f1)
echo "run_global starts at line: $start_line"
# Dump a larger window around run_global for inspection
sed -n "$((start_line-80)),$((start_line+260))p" "$FILE"
echo "=== Search for output arrows in update.rs (use -- to avoid option confusion) ==="
rg -n -- "->" "$FILE" | head -n 80
echo "=== Search for strings that look like '{old} -> {new}' / ': {old}' ==="
rg -n "old.*->|->.*new|:.*->|->.*\\}" "$FILE" || trueRepository: endevco/aube
Length of output: 19068
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="crates/aube/src/commands/update.rs"
sed -n '620,690p' "$FILE"Repository: endevco/aube
Length of output: 3433
Drop the “no newer is-positive version” flakiness concern; only flag output-format coupling
The test runs against the seeded local Verdaccio registry (AUBE_TEST_REGISTRY → test/registry/storage/is-positive/package.json), which includes is-positive versions newer than 1.0.0 (e.g., 2.0.0, 3.0.0, 3.1.0), so aube update -g is not dependent on public npm registry state.
What remains brittle is the human-readable assertion: global update prints eprintln!(" {manifest_key}: {old} -> {new}"), so assert_output --partial "is-positive: 1.0.0 ->" will break if message text/format changes. Prefer asserting the updated installed version via aube list -g --json/--parseable (and/or the existing refute of is-positive 1.0.0).
🤖 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 `@test/global_install.bats` at line 78, Replace the brittle human-readable
output assertion that uses assert_output --partial "is-positive: 1.0.0 ->" with
a deterministic check of the installed package version: run the CLI listing
command (aube list -g --json or aube list -g --parseable) and assert that the
installed version for the is-positive manifest key is the newer version (and/or
keep the existing refute that "is-positive 1.0.0" is not present); update the
test to parse the JSON/parseable output and assert the exact installed version
instead of matching the human-readable eprintln line.
|
Addressed the review feedback and the failing BATS shard:
Latest PR checks are green. This comment was generated by Codex. |


Summary
aube update --globalthrough the global install directory instead of project-root discovery.Fixes Discussion #839: https://github.com/endevco/aube/discussions/839
Tests
cargo fmt --checkcargo test -p aube globalcargo clippy -p aube --all-targets -- -D warningstest/bats/bin/bats --filter 'aube update -g updates global packages outside a project' test/global_install.batsNote:
mise run test:bats test/global_install.batscould not start in this environment because GNUparallelis missing. Running the whole BATS file directly reached the new regression successfully, then later unrelated lifecycle-script tests failed becausenodeis currently a brokenmiseshim here.This PR was generated by Codex.
Note
Medium Risk
Touches global install layout, bin linking, and reuses the full update resolver inside each global directory; mistakes could break CLI tools on PATH but scope is limited to the global code path.
Overview
aube update --globalnow updates tools from the global install store instead of requiring a projectpackage.json, so it works from any directory (including outside a repo).When
--globalis set, the command scans installed globals, runs the normal update flow inside each global install directory with--latestand exact manifest pins, then relinks bin shims and removes stale executables after version changes. Named packages resolve by alias; missing names fail withnot globally installed. With a workspace--filter,--globaltakes precedence and does not mutate filtered workspace manifests.BATS coverage adds global update success outside a project and failure for unknown package names; the filter test expects global routing instead of a no-op.
Reviewed by Cursor Bugbot for commit 5e98c55. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
aube update --globalnow updates globally installed packages from anywhere, supporting updating all globals or specific packages by name, and ensures binary shims are linked and stale shims removed.