Skip to content

fix(cli): honor AUBE_VIRTUAL_STORE_DIR env var + port 5 more pnpm/misc tests#456

Merged
jdx merged 2 commits intomainfrom
claude/hungry-elbakyan-b59bd0
May 1, 2026
Merged

fix(cli): honor AUBE_VIRTUAL_STORE_DIR env var + port 5 more pnpm/misc tests#456
jdx merged 2 commits intomainfrom
claude/hungry-elbakyan-b59bd0

Conversation

@jdx
Copy link
Copy Markdown
Contributor

@jdx jdx commented May 1, 2026 •
edited by cursor Bot
Loading

Summary

  • Fix: resolve_virtual_store_dir only checked npm_config_virtual_store_dir and NPM_CONFIG_VIRTUAL_STORE_DIR, even though settings.toml declares AUBE_VIRTUAL_STORE_DIR as a third alias. Setting only the AUBE_-prefixed variant silently fell through to the default node_modules/.aube. Now all three env-var aliases trigger the explicit-override path.
  • Tests: brings test/pnpm_install_misc.bats to 27/36 ported (was 22/37 — corrected count; pnpm has 36 test() calls, not 37). Adds 5 new ports + 6 documented divergences + 2 already-equivalent.

New ports

pnpm misc.ts aube translation
63 --no-lockfile AUBE_LOCKFILE=false env (no CLI flag in aube)
254 pnpm -r add bare aube -r add reuses single-project validator
405 --virtual-store-dir AUBE_VIRTUAL_STORE_DIR env (now works thanks to the fix above)
427 CI mode drift fails CI=1 + drifted manifest → frozen-mode hard-fail; --no-frozen-lockfile bypasses
457 CI env-var override AUBE_PREFER_FROZEN_LOCKFILE=false maps to FrozenMode::No

Documented divergences (test/PNPM_TEST_IMPORT.md)

  • 136 package.yaml manifest — aube reads JSON only
  • 303 engines.pnpm — aube checks engines.node only
  • 337/371 workspace per-project engines.node — aube checks root manifest only
  • 479 updateConfig pnpmfile hook — aube supports readPackage/afterAllResolved/preResolution only
  • 567 git URL with #branch/with-slash — aube has no git resolver

Already-equivalent

  • 271/287 root engine-strict warn/fail — covered by test/engines.bats:12,32

Test plan

  • 3 new unit tests in crates/aube/src/commands/mod.rs (resolve_virtual_store_dir_tests)
  • All 27 tests in test/pnpm_install_misc.bats pass
  • All 348 unit tests in cargo test --bin aube pass
  • cargo clippy --all-targets -- -D warnings clean
  • cargo fmt --check clean

🤖 Generated with Claude Code

Note

Medium Risk
Changes how the install/linker chooses the virtualStoreDir when only environment variables are set, which can affect on-disk layout and downstream commands that read the virtual store path. Risk is mitigated by added unit and integration tests covering default and override cases.

Overview
Fixes resolve_virtual_store_dir to treat AUBE_VIRTUAL_STORE_DIR as an explicit override (alongside the npm_config_* aliases), so setting only the aube-prefixed env var no longer falls back to the default node_modules/.aube.

Adds regression unit tests for virtual-store resolution and ports five additional pnpm/test/install/misc.ts cases into test/pnpm_install_misc.bats (env-based no-lockfile, recursive bare add validation, virtual-store relocation, CI frozen-lockfile drift behavior, and CI env-var override). Updates PNPM_TEST_IMPORT.md to reflect the new ported count and document additional divergences/equivalent coverage.

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

jdx and others added 2 commits May 1, 2026 10:33
@jdx @claude
fix(cli): honor AUBE_VIRTUAL_STORE_DIR env var
0d951b5 
`resolve_virtual_store_dir` only checked `npm_config_virtual_store_dir`
and `NPM_CONFIG_VIRTUAL_STORE_DIR` for the explicit-override gate, even
though settings.toml declares `AUBE_VIRTUAL_STORE_DIR` alongside both.
Setting only the AUBE_-prefixed variant silently fell through to the
default `node_modules/.aube`, contradicting the documented setting.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jdx @claude
test: port 5 more pnpm install/misc.ts cases
a3ba2f6 
Brings test/pnpm_install_misc.bats to 27/36 ported + 9 documented
divergences/already-equivalent. New ports: --no-lockfile (63),
recursive bare add (254), virtual-store-dir relocation (405), CI mode
auto-frozen drift (427), CI env-var override (457).

Skipped with documentation in test/PNPM_TEST_IMPORT.md:
- 271/287 already covered by test/engines.bats
- 136 (package.yaml manifest, aube reads JSON only)
- 303 (engines.pnpm, aube checks engines.node only)
- 337/371 (workspace per-project engines, aube checks root only)
- 479 (updateConfig pnpmfile hook, not implemented)
- 567 (git URL with hash, no git resolver)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 1, 2026

Greptile Summary

This PR fixes a silent bug in resolve_virtual_store_dir where setting AUBE_VIRTUAL_STORE_DIR alone was not recognized as an explicit override, causing the function to return the default node_modules/.aube path even when aube_settings::resolved::virtual_store_dir would have honored the env value. The fix adds the missing alias to the env-key detection closure, backed by three targeted unit tests and a new bats end-to-end test that validates both the relocation and the absence of writes to the default location. The five additional bats ports are straightforward and well-commented, with clear rationale for each divergence from pnpm's original test steps.

Confidence Score: 5/5

Safe to merge — fix is minimal, well-scoped, and fully covered by new unit and integration tests.

The core fix is a one-liner addition to a detection predicate with no broader behavioral surface, all existing tests pass, and the regression is explicitly guarded by both a unit test and an end-to-end bats test. No security, logic, or compatibility concerns were found.

No files require special attention.

Important Files Changed

Filename Overview
crates/aube/src/commands/mod.rs Adds AUBE_VIRTUAL_STORE_DIR to the explicit-env detection list in resolve_virtual_store_dir, fixing the silent fallback to default; includes 3 unit tests covering the regression, existing npm_config_* path, and the default case.
test/pnpm_install_misc.bats Adds 5 new bats test ports covering no-lockfile env var, recursive bare-add failure, virtual-store relocation, CI frozen mode with drift, and CI frozen mode env-var override.
test/PNPM_TEST_IMPORT.md Updates tracking table to reflect 27/36 ports (corrected count from 22/37), adds newly ported tests, documents 6 architectural divergences, and marks 2 already-equivalent cases.

Reviews (1): Last reviewed commit: "test: port 5 more pnpm install/misc.ts c..." | Re-trigger Greptile

@jdx jdx enabled auto-merge (squash) May 1, 2026 15:38
@jdx jdx merged commit e9ad213 into main May 1, 2026
20 checks passed
@jdx jdx deleted the claude/hungry-elbakyan-b59bd0 branch May 1, 2026 15:40
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

Benchmark changes

Public ratios: warm installs vs Bun 5x -> 9x; warm installs vs pnpm 6x -> 12x.

Benchmark aube bun pnpm
Fresh install (warm cache) 288ms -> 229ms (-20%) 1504ms -> 2026ms (+35%) 1849ms -> 2826ms (+53%)
CI install (warm cache, GVS disabled) 416ms -> 692ms (+66%) 966ms -> 2129ms (+120%) 2237ms -> 2426ms (+8%)
CI install (cold cache, GVS disabled) 5053ms -> 4053ms (-20%) 3918ms -> 4212ms (+8%) 5491ms -> 5457ms (-1%)

a3ba2f6 vs 8bd5012 | aube/bun/pnpm | 3 scenarios | 3 runs | 500mbit/50ms | generated by Codex.

@chenrui333 chenrui333 mentioned this pull request May 1, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jdx