scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-11-10 08:27:27 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

fix(storage): preserve fresh registry entries (#3777)

Browse source 
Recheck prune candidates under the registry lock so stale inspections
cannot remove a completed re-registration. Preserve transient errors
and unreadable registry state.

Refs #3515

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
This commit is contained in:
Gergő Magyar 2026-10-30 20:47:29 +00:00 • committed by GitHub
parent 1736a1bfd8
commit 97ec905b5d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 246 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
gitnexus/src/storage/repo-manager.ts
View file

@ -63,6 +63,7 @@ import {
LIST_STORAGE_REQUIREMENTS,
requireDeletableStoragePath,
StorageDeletionError,
type StorageInspection,
validateConfiguredStoragePath,
} from './storage-resolver.js';
@ -1745,6 +1746,12 @@ export const listRegisteredRepos = async (opts?: {
const prunedSlots = new Set<string>();
const registrySlotKey = (entry: RegistryEntry): string =>
`${canonicalizePath(entry.path)}\0${canonicalizePath(entry.storagePath)}`;
const isSafelyPrunable = (inspection: StorageInspection): boolean =>
!isTransientStorageInspection(inspection) &&
(inspection.state === 'missing' ||
inspection.state === 'empty' ||
(inspection.state === 'unowned' &&
inspection.reason === 'Storage directory contains data but no valid ownership metadata.'));
const inspections = await mapPool(entries, inspectRegisteredStorage, 8);
for (const [entry, inspection] of entries.map(
(entry, i) => [entry, inspections[i]] as [RegistryEntry, (typeof inspections)[number]],
@ -1754,12 +1761,7 @@ export const listRegisteredRepos = async (opts?: {
(!LIST_STORAGE_REQUIREMENTS.requireCodeIndexDB || inspection.hasCodeIndexDB);
if (meetsRequirements) {
valid.push(entry);
} else if (
inspection.state === 'missing' ||
inspection.state === 'empty' ||
(inspection.state === 'unowned' &&
inspection.reason === 'Storage directory contains data but no valid ownership metadata.')
) {
} else if (isSafelyPrunable(inspection)) {
// A missing/empty directory or a registry slot with no ownership
// metadata is not a usable index. Removing only the registry row is
// safe; the storage directory itself is never deleted here.
@ -1786,15 +1788,22 @@ export const listRegisteredRepos = async (opts?: {
// only then. The validation walk above is read-only and can touch several
// files per entry, so holding the global lock across it would serialize
// every `gitnexus augment` behind unrelated registry work for no benefit.
// Re-read inside the lock and drop only the same provably-absent storage
// slots from that fresh snapshot, so a concurrent re-registration of the
// same repository path into another slot survives.
// Reinspect only deletion candidates under the lock: even the same slot
// with identical metadata may have been rebuilt and re-registered since
// the initial inspection (#3515). Other slots keep the lock-free read path.
if (prunedSlots.size > 0) {
try {
await withRegistryLock(async () => {
const fresh = await readRegistry();
const fresh = await readRegistryStrictIfPresent();
if (!fresh) return;
const candidates = fresh.filter((entry) => prunedSlots.has(registrySlotKey(entry)));
const rechecked = await mapPool(candidates, inspectRegisteredStorage, 8);
const confirmedSlots = new Set(
candidates.filter((_, i) => isSafelyPrunable(rechecked[i])).map(registrySlotKey),
);
if (confirmedSlots.size === 0) return;
await writeRegistry(
fresh.filter((entry) => !prunedSlots.has(registrySlotKey(entry))),
fresh.filter((entry) => !confirmedSlots.has(registrySlotKey(entry))),
1,
);
});

243
gitnexus/test/unit/repo-manager-registry-validation-race.test.ts
View file

@ -15,8 +15,10 @@ import {
listRegisteredRepos,
readRegistry,
registerRepo,
type RegistryEntry,
type RepoMeta,
} from '../../src/storage/repo-manager.js';
import type { StorageInspection } from '../../src/storage/storage-resolver.js';
import { createTempDir } from '../helpers/test-db.js';
describe('listRegisteredRepos validation persistence', () => {
@ -72,25 +74,232 @@ describe('listRegisteredRepos validation persistence', () => {
const releaseInspectionPromise = new Promise<void>((resolve) => {
releaseInspection = resolve;
});
storageResolverCtx.inspectRegisteredStorage.mockImplementationOnce(async (entry: any) => {
inspectionStarted();
await releaseInspectionPromise;
return {
repoPath: entry.path,
storagePath: entry.storagePath,
state: 'missing',
hasCodeIndexDB: false,
};
});
storageResolverCtx.inspectRegisteredStorage.mockImplementationOnce(
async (entry: RegistryEntry) => {
inspectionStarted();
await releaseInspectionPromise;
return {
repoPath: entry.path,
storagePath: entry.storagePath,
state: 'missing',
hasCodeIndexDB: false,
};
},
);
const validatingRead = listRegisteredRepos({ validate: true });
await inspectionStartedPromise;
await registerRepo(repoPath, meta, { name: 'repo', storagePath: replacementStoragePath });
releaseInspection();
try {
await inspectionStartedPromise;
await registerRepo(repoPath, meta, { name: 'repo', storagePath: replacementStoragePath });
releaseInspection();
await expect(validatingRead).resolves.toEqual([]);
await expect(readRegistry()).resolves.toMatchObject([
{ path: repoPath, storagePath: replacementStoragePath },
]);
await expect(validatingRead).resolves.toEqual([]);
await expect(readRegistry()).resolves.toMatchObject([
{ path: repoPath, storagePath: replacementStoragePath },
]);
} finally {
releaseInspection();
await validatingRead.catch(() => undefined);
}
});
it.each(['updated', 'unchanged'] as const)(
'keeps a concurrent same-slot re-registration with %s metadata',
async (metadata) => {
const repoPath = tmpRepo.dbPath;
const storagePath = path.join(repoPath, '.gitnexus');
const meta: RepoMeta = {
repoPath,
storagePath,
lastCommit: 'initial-commit',
indexedAt: '2026-09-05T00:00:00.000Z',
stats: { files: 1, nodes: 1 },
};
await registerRepo(repoPath, meta, { name: 'repo', storagePath });
let inspectionStarted!: () => void;
let releaseInspection!: () => void;
const started = new Promise<void>((resolve) => {
inspectionStarted = resolve;
});
const released = new Promise<void>((resolve) => {
releaseInspection = resolve;
});
storageResolverCtx.inspectRegisteredStorage
.mockResolvedValue({
repoPath,
storagePath,
state: 'owned',
hasCodeIndexDB: true,
} satisfies StorageInspection)
.mockImplementationOnce(async (): Promise<StorageInspection> => {
inspectionStarted();
await released;
return { repoPath, storagePath, state: 'missing', hasCodeIndexDB: false };
});
const validatingRead = listRegisteredRepos({ validate: true });
try {
await started;
const replacement =
metadata === 'updated'
? { ...meta, lastCommit: 'rebuilt-commit', indexedAt: '2026-09-06T00:00:00.000Z' }
: meta;
await registerRepo(repoPath, replacement, { name: 'repo', storagePath });
releaseInspection();
await expect(validatingRead).resolves.toEqual([]);
await expect(readRegistry()).resolves.toMatchObject([
{
path: repoPath,
storagePath,
lastCommit: replacement.lastCommit,
indexedAt: replacement.indexedAt,
},
]);
} finally {
releaseInspection();
await validatingRead.catch(() => undefined);
}
},
);
it('still persists pruning when the registered storage remains missing', async () => {
const repoPath = tmpRepo.dbPath;
const storagePath = path.join(repoPath, '.gitnexus');
await registerRepo(repoPath, {
repoPath,
lastCommit: 'initial-commit',
indexedAt: '2026-09-05T00:00:00.000Z',
stats: { files: 1, nodes: 1 },
});
storageResolverCtx.inspectRegisteredStorage.mockResolvedValue({
repoPath,
storagePath,
state: 'missing',
hasCodeIndexDB: false,
} satisfies StorageInspection);
await expect(listRegisteredRepos({ validate: true })).resolves.toEqual([]);
await expect(readRegistry()).resolves.toEqual([]);
});
it.each([
{ label: 'owned', state: 'owned', hasCodeIndexDB: true },
{
label: 'temporarily unreadable',
state: 'invalid_storage',
hasCodeIndexDB: false,
reason: 'EIO: Storage directory could not be inspected.',
},
{
label: 'empty with a transient database error',
state: 'empty',
hasCodeIndexDB: false,
reason: 'EIO: LadybugDB directory could not be inspected.',
},
] satisfies Array<
Pick<StorageInspection, 'state' | 'hasCodeIndexDB' | 'reason'> & { label: string }
>)(
'does not rewrite the registry when the candidate becomes $label',
async ({ label: _label, ...inspection }) => {
const repoPath = tmpRepo.dbPath;
const storagePath = path.join(repoPath, '.gitnexus');
const registryPath = path.join(tmpHome.dbPath, 'registry.json');
const original = JSON.stringify([{ name: 'repo', path: repoPath, storagePath }]);
// Compact formatting makes an otherwise identical atomic rewrite visible.
await fs.writeFile(registryPath, original);
storageResolverCtx.inspectRegisteredStorage
.mockResolvedValue({ repoPath, storagePath, ...inspection } satisfies StorageInspection)
.mockResolvedValueOnce({
repoPath,
storagePath,
state: 'missing',
hasCodeIndexDB: false,
} satisfies StorageInspection);
await expect(listRegisteredRepos({ validate: true })).resolves.toEqual([]);
expect(await fs.readFile(registryPath, 'utf8')).toBe(original);
},
);
it('keeps an initially empty slot when its database inspection failed transiently', async () => {
const repoPath = tmpRepo.dbPath;
const storagePath = path.join(repoPath, '.gitnexus');
const registryPath = path.join(tmpHome.dbPath, 'registry.json');
const entry = { name: 'repo', path: repoPath, storagePath };
const original = JSON.stringify([entry]);
await fs.writeFile(registryPath, original);
storageResolverCtx.inspectRegisteredStorage.mockResolvedValue({
repoPath,
storagePath,
state: 'empty',
hasCodeIndexDB: false,
reason: 'EIO: LadybugDB directory could not be inspected.',
} satisfies StorageInspection);
await expect(listRegisteredRepos({ validate: true })).resolves.toEqual([entry]);
expect(await fs.readFile(registryPath, 'utf8')).toBe(original);
});
it.each(['corrupt', 'deleted'] as const)(
'does not overwrite a registry that becomes %s during validation',
async (change) => {
const repoPath = tmpRepo.dbPath;
const storagePath = path.join(repoPath, '.gitnexus');
const registryPath = path.join(tmpHome.dbPath, 'registry.json');
const corrupt = '{"truncated":';
await fs.writeFile(
registryPath,
JSON.stringify([{ name: 'repo', path: repoPath, storagePath }]),
);
storageResolverCtx.inspectRegisteredStorage.mockImplementation(
async (): Promise<StorageInspection> => {
if (change === 'corrupt') await fs.writeFile(registryPath, corrupt);
else await fs.rm(registryPath, { force: true });
return { repoPath, storagePath, state: 'missing', hasCodeIndexDB: false };
},
);
await expect(listRegisteredRepos({ validate: true })).resolves.toEqual([]);
if (change === 'corrupt') expect(await fs.readFile(registryPath, 'utf8')).toBe(corrupt);
else await expect(fs.access(registryPath)).rejects.toMatchObject({ code: 'ENOENT' });
},
);
it('preserves an unrelated registration completed during validation', async () => {
const repoPath = tmpRepo.dbPath;
const storagePath = path.join(repoPath, '.gitnexus');
const addedRepoPath = path.join(tmpHome.dbPath, 'new-repo');
const registryPath = path.join(tmpHome.dbPath, 'registry.json');
await fs.writeFile(
registryPath,
JSON.stringify([{ name: 'gone', path: repoPath, storagePath }]),
);
storageResolverCtx.inspectRegisteredStorage
.mockResolvedValue({
repoPath,
storagePath,
state: 'missing',
hasCodeIndexDB: false,
} satisfies StorageInspection)
.mockImplementationOnce(async (): Promise<StorageInspection> => {
await registerRepo(
addedRepoPath,
{
repoPath: addedRepoPath,
lastCommit: 'new-commit',
indexedAt: '2026-09-06T00:00:00.000Z',
stats: { files: 1, nodes: 1 },
},
{ name: 'new-repo' },
);
return { repoPath, storagePath, state: 'missing', hasCodeIndexDB: false };
});
await expect(listRegisteredRepos({ validate: true })).resolves.toEqual([]);
const persisted = await readRegistry();
expect(persisted).toHaveLength(1);
expect(persisted[0]).toMatchObject({ name: 'new-repo', path: addedRepoPath });
});
});