Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion crates/aube/src/commands/dlx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,15 @@ pub async fn run(args: DlxArgs) -> miette::Result<()> {
// restores the original on drop — so the exec path below, any error
// diagnostic rendering, and even a panic unwinding past this frame all
// observe the user's real cwd instead of a dir that's about to vanish.
// Pin `project_dir` to the scratch path explicitly so install::run
// does not walk upward looking for a workspace root — the scratch
// dir lives under TMPDIR, which inside a parent workspace would
// otherwise resolve to that workspace and install the wrong tree.
let prev_cwd = {
let _cwd_guard = CwdGuard::switch_to(&project_dir)?;
let install_result = super::install::run(dlx_install_options()).await;
let mut opts = dlx_install_options();
opts.project_dir = Some(project_dir.clone());
let install_result = super::install::run(opts).await;
let prev = _cwd_guard.original.clone();
install_result.wrap_err("dlx install failed")?;
prev
Expand Down
21 changes: 16 additions & 5 deletions crates/aube/src/commands/install/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1709,11 +1709,22 @@ pub async fn run(opts: InstallOptions) -> miette::Result<()> {
project_dir.clone()
} else {
let initial_cwd = crate::dirs::cwd()?;
// Walk upward to the nearest `package.json` so `aube install` run
// from a subdirectory (e.g. `repo/docs`) installs against the
// project root instead of erroring with "package.json not found".
// Matches pnpm's behavior.
match crate::dirs::find_project_root(&initial_cwd) {
// Prefer the workspace root so `aube install` from inside a
// workspace member installs against the workspace, not the
// member as a standalone project — otherwise the member gets
// its own `aube-lock.yaml`, its own `.aube/` virtual store,
// and re-downloads anything not already in the global cache.
// The workspace root must have its own `package.json` —
// `find_workspace_root` returns yaml-only roots too, but
// `install::run` reads the root manifest unconditionally and
// would error on a yaml-only workspace. Fall through to the
// nearest `package.json` (the member itself, or the cwd for
// non-workspace subdirectory installs like `repo/docs`).
// Mirrors the anchor logic in `ensure_installed`.
match crate::dirs::find_workspace_root(&initial_cwd)
.filter(|root| root.join("package.json").is_file())
.or_else(|| crate::dirs::find_project_root(&initial_cwd))
{

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Production fix accidentally included in test-only PR

Medium Severity

The PR title (test(install): pin failing repro …) and description explicitly state this is a test-only change, with the actual fix being "out of scope for this PR." The unchecked test-plan item confirms the test is supposed to fail right now. However, the diff includes the production code change in install/mod.rs that swaps find_project_root for find_workspace_root().or_else(|| find_project_root()) — which is the fix itself. With the fix present the new bats test will pass, contradicting the stated intent of landing a pinned-failing regression test first.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c4b89ad. Configure here.

Some(root) => root,
None => {
return Err(miette!(
Expand Down
24 changes: 17 additions & 7 deletions mise-tasks/ci/bats/nonserial
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,23 @@ printf "Running non-serial BATS shard %s/%s with %s files\n" "$((shard_index + 1
printf " %s\n" "${test_files[@]}"
mkdir -p "$report_dir"
status=0
BATS_REPORT_FILENAME="$report_name" ./test/bats/bin/bats \
--report-formatter junit \
--output "$report_dir" \
--jobs "$jobs" \
--parallel-binary-name parallel \
--filter-tags "!serial" \
"${test_files[@]}" || status=$?
# DEBUG (TEMP): run files one at a time with a 90s per-file timeout so
# any hanging test surfaces as a SIGTERM with the file name visible in
# the log. Revert this change once the hang is identified.
for tf in "${test_files[@]}"; do
echo "::group::bats $tf"
if ! timeout --signal=KILL --verbose 90 ./test/bats/bin/bats \
--filter-tags "!serial" \
--timing \
"$tf"; then
rc=$?
echo "::endgroup::"
echo "BATS file failed or timed out: $tf (exit $rc)"
status=$rc

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Exit code always zero after negated if condition

High Severity

rc=$? inside the then block of if ! timeout ... bats ...; then always captures 0 because ! negates the pipeline's exit status before the if evaluates it. This means status is never set to a nonzero value from a test failure, the log message always says (exit 0), and the script may exit successfully even when tests fail — CI could silently pass with broken tests depending on whether the report upload also fails.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4436eee. Configure here.

break
fi
echo "::endgroup::"
done

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Temporary debug CI code committed in unrelated PR

Medium Severity

The mise-tasks/ci/bats/nonserial changes are explicitly marked DEBUG (TEMP) and Revert this change once the hang is identified. This removes parallel test execution, drops JUnit report generation (so the upload on line 47 targets a nonexistent file), and adds break on first failure — all unmentioned in the PR description which is solely about workspace member install anchoring.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4436eee. Configure here.


upload_status=0
mise run ci:test-engine:upload -- "$report_path" || upload_status=$?
Expand Down
97 changes: 97 additions & 0 deletions test/workspace_member_install_walks_up.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
#!/usr/bin/env bats
#
# Regression: `aube install` run from inside a workspace member must
# resolve up to the workspace root (pnpm parity) instead of treating the
# member as a standalone project. The user-visible bug is that a member
# install runs a fresh resolve/fetch/link cycle and writes a private
# lockfile, virtual store, and install state next to the member's own
# package.json — duplicating work the prior workspace-root install
# already did, and re-downloading anything not already in the global
# cache.
#
# Root cause: install/mod.rs's `find_project_root` walks up to the
# *nearest* package.json, which inside a workspace is the member's own.
# A workspace-aware walk should resolve to the workspace root when the
# member's ancestor is a workspace declaration.
#
# bats file_tags=serial

# shellcheck disable=SC2034
BATS_NO_PARALLELIZE_WITHIN_FILE=1

setup() {
load 'test_helper/common_setup'
_common_setup
}

teardown() {
_common_teardown
}

_setup_minimal_workspace() {
cat >package.json <<-'JSON'
{
"name": "ws-root",
"version": "0.0.0",
"private": true
}
JSON
cat >pnpm-workspace.yaml <<-'YAML'
packages:
- packages/*
YAML
mkdir -p packages/a
cat >packages/a/package.json <<-'JSON'
{
"name": "@ws/a",
"version": "1.0.0",
"dependencies": {
"is-odd": "3.0.1"
}
}
JSON
}

@test "aube install from a workspace member resolves up to the workspace root" {
_setup_minimal_workspace

# Prime the workspace at the root. After this, the workspace root
# owns the lockfile, the virtual store, and the install state.
run aube install
assert_success
assert_file_exists aube-lock.yaml
assert_dir_exists node_modules/.aube
assert_file_exists node_modules/.aube-state
assert_link_exists packages/a/node_modules/is-odd

# Snapshot the root install state. A correct member install must
# resolve back up to this same root and short-circuit on the warm
# fast path — the root state must be byte-identical afterward.
local root_state_before
root_state_before="$(cat node_modules/.aube-state)"

cd packages/a
run aube install
assert_success
cd ../..

# No standalone artifacts inside the workspace member. Each
# assertion below proves a distinct symptom of the bug:
# - own lockfile -> member ran its own resolve
# - own .aube -> member ran its own linker pass
# - own .aube-state -> member ran the full install pipeline
# Any single failure means the member install ignored the
# workspace root.
run test -e packages/a/aube-lock.yaml
assert_failure
run test -d packages/a/node_modules/.aube
assert_failure
run test -e packages/a/node_modules/.aube-state
assert_failure

# The root install state is byte-identical — the only correct
# warm path is "do nothing", and "do nothing" can't write any
# state file. `assert_equal` (not `[`) so a regression prints
# both snapshots and we can diff what changed.
assert_equal "$(cat node_modules/.aube-state)" "$root_state_before"
}
Loading