Skip to content

fix(cli): reject . as a foreign --lockfile-dir importer; correct docs#442

Merged
jdx merged 1 commit intomainfrom
claude/lockfile-dir-foreign-fix
Apr 30, 2026
Merged

fix(cli): reject . as a foreign --lockfile-dir importer; correct docs#442
jdx merged 1 commit intomainfrom
claude/lockfile-dir-foreign-fix

Conversation

@jdx
Copy link
Copy Markdown
Contributor

@jdx jdx commented Apr 30, 2026

Follow-up to #431 addressing two greptile P1s left over after merge.

Summary

  • Foreign-importer guard previously exempted ".". When the current project's importer key is non-., a "." entry on disk is itself an unrelated project that ran aube install in the lockfile dir directly. Dropping it on write is the same data-loss class the guard exists to prevent. Drop the "." exemption from the filter; the caller already gates on importer_key != "." so the default install path stays untouched.
  • Replace stale lockfileDir docs. crates/aube-settings/settings.toml and the generated docs/settings/index.md still claimed "several unrelated projects can share a single committed lockfile", which is the exact configuration the guard rejects. Replaced with the correct scope: single-project relocation only, multi-project sharing requires a workspace.

Test plan

  • New: refuses to clobber a parent dir that is itself a project in test/lockfile_dir.bats — covers the . foreign-importer case the previous filter missed.
  • mise run test:bats test/lockfile_dir.bats (6/6).
  • mise run test:bats test/install.bats test/workspace.bats (79/79) — no regressions; workspace installs with multiple importers stay healthy because the guard is gated on a non-. importer key.
  • cargo clippy -p aube --all-targets -- -D warnings clean.

🤖 Generated with Claude Code


Note

Medium Risk
Changes aube install --lockfile-dir to fail in an additional scenario (when the target lockfile already contains a . importer), which could break previously-working but unsafe setups. Scope is small and covered by a new bats test, but it affects install-time behavior and lockfile safety checks.

Overview
Tightens the --lockfile-dir foreign-importer guard so a lockfile containing a . importer is treated as foreign when installing from a subproject, preventing silent loss of the parent project’s lockfile entries.

Updates lockfileDir documentation (settings registry + generated docs) to remove the claim that unrelated projects can share a lockfile, clarifying that multi-project sharing requires a pnpm-workspace.yaml workspace. Adds a bats regression test to ensure installs refuse to target a parent directory that is itself a project.

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

- Foreign-importer guard previously exempted `"."`. When the current
  project's importer key is non-`.`, a `"."` entry on disk is itself
  an unrelated project that ran `aube install` in the lockfile dir
  directly, and dropping it on write is the same data-loss class
  the guard exists to prevent. Drop the `"."` exemption from the
  filter; the caller already gates on `importer_key != "."` so the
  default install path stays untouched.
- Replace the "several unrelated projects can share a single
  committed lockfile" sentence in `settings.toml` and the generated
  `docs/settings/index.md` with the correct scope: single-project
  relocation only, multi-project sharing requires a workspace.

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

greptile-apps Bot commented Apr 30, 2026

Greptile Summary

This PR fixes a data-loss edge case in the --lockfile-dir foreign-importer guard and corrects the stale documentation that described the now-rejected multi-project sharing pattern. The one-line code change (dropping the "." exemption from the filter) is correct and well-reasoned: the caller already gates on importer_key != ".", so a "." entry in an existing lockfile can only belong to a project that installed directly in lockfileDir, making it a foreign importer by definition.

Confidence Score: 5/5

Safe to merge — the fix is minimal, logically sound, and covered by a new bats test; only finding is a P2 on test assertion strength.

The core logic change is a single-line removal with a clear invariant provided by the caller. All four files are consistent with each other. Only finding is a P2 suggestion.

No files require special attention.

Important Files Changed

Filename Overview
crates/aube/src/commands/install/mod.rs Removes the erroneous "." exemption from the foreign-importer filter; the one-line change is correct given that the caller already gates on importer_key != "."
test/lockfile_dir.bats Adds a correct end-to-end test for the . foreign-importer case; the assertion is slightly weaker than the parallel sibling test (no check on the specific importer key in the error message).
crates/aube-settings/settings.toml Documentation updated to match the actual enforced semantics (single-project relocation only); change is accurate and clear.
docs/settings/index.md Generated docs kept in sync with settings.toml; no issues.

Fix All in Claude Code

Reviews (1): Last reviewed commit: "fix(cli): reject `.` as a foreign import..." | Re-trigger Greptile

Comment thread test/lockfile_dir.bats
cd child || return
run aube install --lockfile-dir .. --no-frozen-lockfile
assert_failure
assert_output --partial "records importers from other projects"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Missing assertion on the specific foreign importer key

The sibling test at line 114 adds assert_output --partial "alpha" to confirm the error message names the specific importer that was rejected. This new test only checks the generic prefix, so a regression that reports the wrong importer key (e.g., still exempting .) would still pass. Because . is too short and ubiquitous to assert on safely, asserting on the parenthesised list form "(." would distinguish the case without false matches.

Fix in Claude Code

@jdx jdx enabled auto-merge (squash) April 30, 2026 23:41
@jdx jdx merged commit 435538c into main Apr 30, 2026
20 checks passed
@jdx jdx deleted the claude/lockfile-dir-foreign-fix branch April 30, 2026 23:46
@github-actions
Copy link
Copy Markdown

Benchmark changes

Versions:

  • aube: 1.5.1 -> 1.5.2
  • pnpm: 11.0.2 -> 11.0.3

Public ratios: warm installs vs Bun 4x -> 3x; warm installs vs pnpm 5x -> 7x.

Benchmark aube bun pnpm
Fresh install (warm cache) 1021ms -> 761ms (-25%) 4134ms -> 2378ms (-42%) 4717ms -> 5266ms (+12%)
CI install (warm cache, GVS disabled) 2920ms -> 2799ms (-4%) 3396ms -> 2628ms (-23%) 4864ms -> 5133ms (+6%)
CI install (cold cache, GVS disabled) 10801ms -> 7572ms (-30%) 10012ms -> 8832ms (-12%) 9722ms -> 8380ms (-14%)

ffa91ac vs 400c56a | aube/bun/pnpm | 3 scenarios | 3 runs | 500mbit/50ms | generated by Codex.

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.

1 participant