mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-20 00:11:37 +00:00
* feat(cli): gitnexus remove <target> to unindex a registered repo by name or path (#664)
Add a `remove` CLI command that deletes the `.gitnexus/` index AND
unregisters a repo from the global registry (~/.gitnexus/registry.json),
addressing the lifecycle gap flagged in #664: previously users had to
cd into the repo to run `clean`, and there was no path-based or
alias-based remove for an already-deleted working tree.
- New command `gitnexus remove <target> [-f|--force]`. `<target>` is
alias / basename-derived name / remote-inferred name / absolute path.
- New helper `resolveRegistryEntry(entries, target)` in repo-manager.ts
with path > name precedence; throws RegistryNotFoundError or
RegistryAmbiguousTargetError (typed, `kind`-discriminated).
- Atomicity mirrors `clean`: fs.rm first, then unregisterRepo; partial
failures self-heal on next `listRegisteredRepos({ validate: true })`.
- Idempotent on unknown targets (exit 0 with warning) per the #664
spec: "behave atomically and idempotently so retries are safe".
- `--force` uses `clean`-style confirmation-skip semantics — distinct
from `analyze --force` (pipeline re-index); here there is no pipeline
so no conflation.
- 7 new unit tests cover resolver precedence, case sensitivity,
ambiguity, and not-found hints; 2 integration tests cover the real
CLI -> registry -> filesystem chain including the --allow-duplicate-name
(#829) ambiguity case.
* fix(cli): canonicalize repo paths so remove/register match across platforms (#1003 review)
Address review feedback from @evander-wang and @magyargergo on PR #1003
plus the Windows + macOS CI failure (same root cause).
Problem:
- macOS: /var is a symlink to /private/var. `path.resolve` does NOT
follow symlinks, so a child running analyze in /var/folders/X stores
/private/var/folders/X (realpath from OS cwd) but an outer caller
passing the symlink form misses.
- Windows: GitHub runners surface tmpdirs in 8.3 short-name form
(RUNNERA~1) while process.cwd() returns the long form (runneradmin).
Same divergence.
Fix: new `canonicalizePath(p)` helper wraps `path.resolve` plus
`fs.realpathSync.native`, falling back to `path.resolve` when the path
doesn't exist (preserves idempotent-on-missing semantics needed by
`remove <unknown>`). Applied at 3 call-sites — registerRepo,
unregisterRepo, resolveRegistryEntry — canonicalising BOTH the input
and each stored `entry.path` at compare time. That last bit is the
backward-compat story: registries written by older versions
(pre-canonicalisation) still match correctly, so we don't need a
migration script.
Test side: the ambiguous-target integration test now reads the path
from the registry snapshot rather than passing the outer `repoA`
variable directly, so it exercises the registry contract regardless of
which path form the platform stores. 4 new unit tests cover the helper
(idempotent, fallback-on-missing, absolute-for-relative) plus the
backward-compat resolver path.
* fix(cli): store resolved (non-canonical) path, compare via canonicalizePath (#1003 CI)
Follow-up to c5eceba0. The previous commit canonicalised the repo path
at BOTH write-time AND compare-time in registerRepo — that expanded
Windows 8.3 short names (RUNNER~1) to long names (runneradmin) when
storing `entry.path`. Pre-existing #829 unit tests that assert
`path.resolve(err.existingPath) === path.resolve(tmpPath)` then broke
because `tmpPath` is still short-form (path.resolve doesn't expand
8.3) while `entry.path` was long-form (canonicalizePath does).
Fix: split storage from comparison.
- entry.path stores `path.resolve(repoPath)` — whatever form the
caller passed. `list` output and error messages show the path the
user typed.
- All compare points (existing-entry lookup in registerRepo, the
collision guard, unregisterRepo, resolveRegistryEntry path tier)
canonicalise BOTH sides via `canonicalizePath`. That is where the
/var ↔ /private/var and RUNNER~1 ↔ runneradmin divergence actually
matters.
Net effect: storage is tolerant (preserves user input), matching is
strict (canonical-vs-canonical). Pre-existing #829 tests stay green
because `err.existingPath` is unchanged from what `path.resolve` gives
back; the cross-platform CI failure from #1003 stays fixed because
every comparison path goes through `canonicalizePath`.
* fix(cli): refuse destructive fs.rm when registry storagePath isn't <repo>/.gitnexus (#1003 review)
Address @magyargergo's inline review finding on remove.ts:89 and the
sibling vulnerability in clean.ts --all (caught during a pre-commit
safety audit). ~/.gitnexus/registry.json is a user-writable plain-text
file, so a corrupted or hand-edited entry could point storagePath at
the repo root (catastrophic: rm the working tree), an empty string
(→ cwd), a parent dir, or anywhere else. fs.rm(recursive: true,
force: true) on any of those is a runtime disaster.
- New UnsafeStoragePathError + exported assertSafeStoragePath() in
repo-manager.ts. Pure lexical string check (Windows-case-
insensitive) asserting entry.storagePath === path.join(entry.path,
'.gitnexus').
- Guard wired into BOTH destructive registry-trusting sites:
- remove.ts: exit 1 with actionable hint
- clean.ts --all: skip the poisoned entry with a warning and
continue (preserves existing per-repo error tolerance — one bad
entry doesn't halt the batch)
- clean.ts default path and server/api.ts are safe-by-construction
(they recompute storagePath from findRepo / getStoragePath rather
than trusting the registry field).
- 8 unit tests cover the guard (valid, repo-root, parent, empty,
unrelated, sibling, error payload, Windows case).
- 2 integration tests prove the full CLI path: remove-poisoned exits
1 without touching the working tree; clean --all with a poisoned
sibling entry cleans the good entry, skips the bad one, and leaves
the poisoned repo intact.
* test(cli): assert full remove dry-run + success output shape (#1003 NIT)
Address the one NIT from the senior-reviewer pass on PR #1003: the
integration test was only checking for the "Run with --force" hint in
dry-run output, not verifying that the three actual console.log lines
(alias, repo path, storage path) appear. Same weak check on the
success-branch "Removed" output.
Tighten both assertions to toContain(alias), toContain(entry.path),
toContain(storagePath). Catches silent format regressions — e.g. a
future refactor that drops a console.log line or swaps
entry.name/entry.path in the output.
No code change; +20 test lines. All assertions in the happy-path
integration test now fire for a meaningful reason.
110 lines
4.4 KiB
TypeScript
110 lines
4.4 KiB
TypeScript
/**
|
|
* Remove Command (#664)
|
|
*
|
|
* Delete the `.gitnexus/` index for a registered repo and unregister it
|
|
* from the global registry (~/.gitnexus/registry.json). The target is
|
|
* identified by alias / basename-derived name / remote-inferred name /
|
|
* absolute path — no `--repo` flag, just a positional argument so the
|
|
* destructive-command ergonomics match `clean` (which is also
|
|
* destructive but scoped to `process.cwd()`).
|
|
*
|
|
* Compared to `clean`:
|
|
* - `clean` acts on the repo discovered by walking up from cwd.
|
|
* - `remove` acts on any registered repo identified by name or path.
|
|
*
|
|
* Behaviour notes:
|
|
* - Idempotent on unknown targets: exits 0 with a warning so that
|
|
* `remove X && analyze Y` keeps working in scripts. Per #664:
|
|
* "behave atomically and idempotently so retries are safe".
|
|
* - Atomic order mirrors `clean`: fs.rm FIRST, then unregister. A
|
|
* partial failure leaves the registry pointing at a missing dir
|
|
* (recoverable by `listRegisteredRepos({ validate: true })` on
|
|
* next read) rather than the opposite, which would orphan
|
|
* .gitnexus/ directories on disk.
|
|
* - `-f` / `--force` matches the confirmation-skip semantics of
|
|
* `clean -f`. (Distinct from `analyze --force`, which re-indexes;
|
|
* here there is no pipeline, so no conflation.)
|
|
*/
|
|
|
|
import fs from 'fs/promises';
|
|
import {
|
|
readRegistry,
|
|
resolveRegistryEntry,
|
|
assertSafeStoragePath,
|
|
unregisterRepo,
|
|
RegistryNotFoundError,
|
|
RegistryAmbiguousTargetError,
|
|
UnsafeStoragePathError,
|
|
} from '../storage/repo-manager.js';
|
|
|
|
export const removeCommand = async (target: string, options?: { force?: boolean }) => {
|
|
// Read the registry snapshot once and pass it to the resolver — this
|
|
// lets us render the "before" state in the dry-run path without a
|
|
// second disk read.
|
|
const entries = await readRegistry();
|
|
|
|
let entry;
|
|
try {
|
|
entry = resolveRegistryEntry(entries, target);
|
|
} catch (err) {
|
|
if (err instanceof RegistryNotFoundError) {
|
|
// Idempotent: missing target is a no-op warning, not an error.
|
|
// The `availableNames` hint comes from the error itself so users
|
|
// can see what they might have meant.
|
|
console.warn(`Nothing to remove: ${err.message}`);
|
|
return;
|
|
}
|
|
if (err instanceof RegistryAmbiguousTargetError) {
|
|
// Duplicate aliases are allowed via --allow-duplicate-name (#829);
|
|
// refuse to guess which one the user meant — surface the full list
|
|
// and exit non-zero so scripts don't silently pick the wrong repo.
|
|
console.error(`Error: ${err.message}`);
|
|
process.exit(1);
|
|
}
|
|
throw err;
|
|
}
|
|
|
|
// Confirmation gate — same shape as `clean`. Default is a dry-run
|
|
// that describes what would be deleted; `--force` actually deletes.
|
|
if (!options?.force) {
|
|
console.log(`This will delete the GitNexus index for: ${entry.name}`);
|
|
console.log(` Path: ${entry.path}`);
|
|
console.log(` Storage: ${entry.storagePath}`);
|
|
console.log('\nRun with --force to confirm deletion.');
|
|
return;
|
|
}
|
|
|
|
// Safety guard (#1003 review — @magyargergo): refuse to proceed if
|
|
// the registry entry's `storagePath` isn't the canonical
|
|
// `<entry.path>/.gitnexus` subfolder. `~/.gitnexus/registry.json` is
|
|
// user-writable, so a corrupted or hand-edited entry could point
|
|
// storagePath at the repo root, an empty string (→ cwd), a parent
|
|
// dir, or anywhere else; `fs.rm(recursive: true, force: true)` on
|
|
// any of those would be a runtime disaster. Bail before touching
|
|
// disk, with an actionable hint for recovering a broken registry.
|
|
try {
|
|
assertSafeStoragePath(entry);
|
|
} catch (err) {
|
|
if (err instanceof UnsafeStoragePathError) {
|
|
console.error(`Error: ${err.message}`);
|
|
process.exit(1);
|
|
}
|
|
throw err;
|
|
}
|
|
|
|
// Deletion order: fs.rm first, then unregister. If fs.rm fails mid-way,
|
|
// the registry entry stays so the user can retry. If fs.rm succeeds but
|
|
// unregister throws (e.g. ENOSPC on registry write), the entry becomes
|
|
// orphaned — `listRegisteredRepos({ validate: true })` prunes those on
|
|
// next read, so the failure is self-healing.
|
|
try {
|
|
await fs.rm(entry.storagePath, { recursive: true, force: true });
|
|
await unregisterRepo(entry.path);
|
|
console.log(`Removed: ${entry.name}`);
|
|
console.log(` Path: ${entry.path}`);
|
|
console.log(` Storage: ${entry.storagePath}`);
|
|
} catch (err) {
|
|
console.error(`Failed to remove ${entry.name}:`, err);
|
|
process.exit(1);
|
|
}
|
|
};
|