docs(storage): name the checks that answer recurring review findings (#3374)

Comment-only. The review bot re-derives findings from the code on every
push, so put the refuting fact next to each flagged line: the pristine
check's hidden-path coverage, sparse checkouts being skip-worktree, the
lock-safe unregister-then-delete order, the hook parity test for device
names, and why the stores/ lstat is not a race guard.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Gergo Magyar 2026-09-25 09:51:43 +00:00
parent b433e818fc
commit e2f3702684
4 changed files with 12 additions and 2 deletions

View file

@ -176,7 +176,9 @@ const collectSharedStores = async (force: boolean): Promise<void> => {
});
// Stray files (`.DS_Store`) are not stores, and a symlink is not followed:
// reclaim deletes under the root it is given. A store another collector
// removed meanwhile is simply gone.
// removed meanwhile is simply gone. The lstat skips a stray link; it is not
// a race guard, since stores/ belongs to the user running clean and anyone
// able to swap an entry there can already delete the store itself.
const roots: string[] = [];
for (const name of names) {
const root = path.join(storesDir, name);
@ -388,6 +390,9 @@ export const cleanCommand = async (options?: {
for (const entry of entries) {
try {
const storagePath = await requireDeletableStoragePath(entry);
// A shared slot is unregistered before it is deleted: its lock file
// lives inside it, so it must go last. A failed delete throws with the
// `clean --gc --force` recovery (shared-store-clean.test.ts).
await removeCheckoutStorage(storagePath, () => unregisterRepo(entry.path), entry.path);
console.log(t('clean.deletedRepo', { name: entry.name, storagePath }));
reportReclaim(await reclaimAfterSlotRemoval(storagePath));

View file

@ -391,6 +391,8 @@ export const publishSharedGraph = async (
// commit graph never changes, so a shared copy would stay short for good.
!meta.embeddingCheckpoint &&
// A sparse or partial checkout builds a graph missing the files it hides.
// Every sparse mode marks those entries skip-worktree, which this rejects
// (git-utils.test.ts covers no-cone, cone and sparse-index checkouts).
isWorkingTreePristine(repoPath);
// Every pointer change and the reclaim that follows run under one publish
// lock, so a concurrent reclaim never sees a half-recorded reference.

View file

@ -129,6 +129,8 @@ export const listWorkingTreeDirtyPaths = (repoPath: string): string[] | null =>
* git failure.
*/
export const isWorkingTreePristine = (repoPath: string): boolean => {
// Includes every skip-worktree and assume-unchanged path (listHiddenIndexPaths,
// `git ls-files -v`), so the `--stage` pass below only has to find gitlinks.
if (listWorkingTreeDirtyPaths(repoPath)?.length !== 0) return false;
try {
const out = execFileSync('git', ['ls-files', '--stage', '-z', '--'], {

View file

@ -31,7 +31,8 @@ export const sanitizeSlotBasename = (
const candidate = sanitized.slice(0, end) || 'repository';
// Exact device names were always prefixed. Windows also reserves them with
// an extension (`CON.txt`); apply that only there, so existing POSIX slot
// names stay stable.
// names stay stable. All four registry-query.cjs hook copies mirror this;
// hooks-shared-store.test.ts checks hook-vs-TS parity on both platforms.
const reserved =
platform === 'win32'
? /^(con|prn|aux|nul|com[1-9]|lpt[1-9])(\..*)?$/i