fix(cli): reject . as a foreign --lockfile-dir importer; correct docs#442
fix(cli): reject . as a foreign --lockfile-dir importer; correct docs#442
. as a foreign --lockfile-dir importer; correct docs#442Conversation
- 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 SummaryThis PR fixes a data-loss edge case in the Confidence Score: 5/5Safe 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
Reviews (1): Last reviewed commit: "fix(cli): reject `.` as a foreign import..." | Re-trigger Greptile |
| cd child || return | ||
| run aube install --lockfile-dir .. --no-frozen-lockfile | ||
| assert_failure | ||
| assert_output --partial "records importers from other projects" |
There was a problem hiding this comment.
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.
Benchmark changesVersions:
Public ratios: warm installs vs Bun 4x -> 3x; warm installs vs pnpm 5x -> 7x.
ffa91ac vs 400c56a | aube/bun/pnpm | 3 scenarios | 3 runs | 500mbit/50ms | generated by Codex. |
Follow-up to #431 addressing two greptile P1s left over after merge.
Summary
".". When the current project's importer key is non-., a"."entry on disk is itself an unrelated project that ranaube installin 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 onimporter_key != "."so the default install path stays untouched.lockfileDirdocs. 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
refuses to clobber a parent dir that is itself a projectin 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 warningsclean.🤖 Generated with Claude Code
Note
Medium Risk
Changes
aube install --lockfile-dirto 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-dirforeign-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
lockfileDirdocumentation (settings registry + generated docs) to remove the claim that unrelated projects can share a lockfile, clarifying that multi-project sharing requires apnpm-workspace.yamlworkspace. 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.