From ee2feb7a5b1e7c4a66acb0dd70dee6aad88c56e5 Mon Sep 17 00:00:00 2001 From: Parafee41 Date: Sun, 20 Sep 2026 15:28:28 +0800 Subject: [PATCH] fix(cli): announce explicit registry alias changes (#3334) * fix(cli): announce explicit registry alias changes * fix: handle async rename observer failures * Address PR review feedback (#3334) - Invoke onRename after withRegistryLock releases so observer I/O cannot stall the registry - Spy the rejecting observer and prove lock release via re-entrant registerRepo Co-authored-by: Cursor * chore(autofix): apply prettier + eslint fixes via /autofix command --------- Co-authored-by: Gergo Magyar Co-authored-by: Cursor Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- gitnexus/src/core/run-analyze.ts | 6 ++++ gitnexus/src/storage/repo-manager.ts | 32 +++++++++++++++++++-- gitnexus/test/unit/repo-manager.test.ts | 37 +++++++++++++++++++++++++ gitnexus/test/unit/run-analyze.test.ts | 4 ++- 4 files changed, 75 insertions(+), 4 deletions(-) diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index c2d7b485d..f0c20e737 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -1597,6 +1597,8 @@ async function runFullAnalysisInner( if (options.registryName) { await registerRepo(repoPath, existingMeta, { name: options.registryName, + onRename: (previousName, nextName) => + log(`Registry name changed: "${previousName}" -> "${nextName}".`), allowDuplicateName: options.allowDuplicateName, branch: placement.branch, storagePath, @@ -2166,6 +2168,8 @@ async function runFullAnalysisInner( if (options.registryName) { await registerRepo(repoPath, existingMeta, { name: options.registryName, + onRename: (previousName, nextName) => + log(`Registry name changed: "${previousName}" -> "${nextName}".`), allowDuplicateName: options.allowDuplicateName, branch: placement.branch, storagePath, @@ -4644,6 +4648,8 @@ async function runFullAnalysisInner( // will look up (#979). const projectName = await registerRepo(repoPath, meta, { name: options.registryName, + onRename: (previousName, nextName) => + log(`Registry name changed: "${previousName}" -> "${nextName}".`), allowDuplicateName: options.allowDuplicateName, // Non-primary branch runs upsert into the entry's branches[]; the // primary/flat run (placement.branch === undefined) refreshes the diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index a4db22b46..aee05358f 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -838,6 +838,13 @@ export interface RegisterRepoOptions { * re-analyses of the same path without `--name` preserve the alias. */ name?: string; + /** + * Best-effort notification after an explicit alias change has been committed + * to the registry. Invoked after the registry lock is released. Callback + * failures are ignored: reporting must not turn a successful registry write + * into an apparent transaction failure. + */ + onRename?: (previousName: string, nextName: string) => void | Promise; /** * Allow two DIFFERENT repo paths to register under the same alias * (#829). Mapped from the `--allow-duplicate-name` CLI flag. @@ -926,6 +933,11 @@ const hasCustomAlias = (entry: RegistryEntry, inferredName: string | null): bool return true; }; +type RegisterRepoUnlockedResult = { + name: string; + rename?: { previousName: string; nextName: string }; +}; + /** * Register (add or update) a repo in the global registry. * Called after `gitnexus analyze` completes. @@ -954,7 +966,7 @@ const registerRepoUnlocked = async ( repoPath: string, meta: RepoMeta, opts?: RegisterRepoOptions, -): Promise => { +): Promise => { // Preserve the caller's chosen path form in the registry — don't // canonicalise at write time. This matters for two reasons: // 1. `list` and error messages show the path the user actually @@ -1151,14 +1163,28 @@ const registerRepoUnlocked = async ( } await writeRegistry(fresh); - return name; + const rename = + opts?.name !== undefined && freshExisting && freshExisting.name !== name + ? { previousName: freshExisting.name, nextName: name } + : undefined; + return { name, ...(rename ? { rename } : {}) }; }; export const registerRepo = async ( repoPath: string, meta: RepoMeta, opts?: RegisterRepoOptions, -): Promise => withRegistryLock(() => registerRepoUnlocked(repoPath, meta, opts)); +): Promise => { + const { name, rename } = await withRegistryLock(() => registerRepoUnlocked(repoPath, meta, opts)); + if (rename) { + try { + await opts?.onRename?.(rename.previousName, rename.nextName); + } catch { + // The rename is already durable; observer failures cannot roll it back. + } + } + return name; +}; /** * Remove a repo from the global registry. diff --git a/gitnexus/test/unit/repo-manager.test.ts b/gitnexus/test/unit/repo-manager.test.ts index cdf1d6fa2..fddafbe39 100644 --- a/gitnexus/test/unit/repo-manager.test.ts +++ b/gitnexus/test/unit/repo-manager.test.ts @@ -963,6 +963,43 @@ describe('registerRepo name override + collision guard (#829)', () => { expect(entries[0].name).toBe('new-alias'); }); + it('keeps an alias rename committed when an async observer rejects', async () => { + await registerRepo(tmpRepoA.dbPath, meta, { name: 'old-alias' }); + + const onRename = vi.fn(async () => { + throw new Error('observer failed'); + }); + + await expect( + registerRepo(tmpRepoA.dbPath, meta, { + name: 'new-alias', + onRename, + }), + ).resolves.toBe('new-alias'); + + expect(onRename).toHaveBeenCalledTimes(1); + expect(onRename).toHaveBeenCalledWith('old-alias', 'new-alias'); + expect(await listRegisteredRepos()).toMatchObject([{ name: 'new-alias' }]); + }); + + it('releases the registry lock before invoking onRename', async () => { + await registerRepo(tmpRepoA.dbPath, meta, { name: 'old-alias' }); + + await registerRepo(tmpRepoA.dbPath, meta, { + name: 'new-alias', + onRename: async () => { + await registerRepo(tmpRepoB.dbPath, meta, { name: 'observer-reentry' }); + }, + }); + + expect(await listRegisteredRepos()).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: 'new-alias' }), + expect.objectContaining({ name: 'observer-reentry' }), + ]), + ); + }); + it('registerRepo throws RegistryNameCollisionError when another path uses the name', async () => { await registerRepo(tmpRepoA.dbPath, meta, { name: 'shared' }); diff --git a/gitnexus/test/unit/run-analyze.test.ts b/gitnexus/test/unit/run-analyze.test.ts index fe1dc8660..7b6ac9552 100644 --- a/gitnexus/test/unit/run-analyze.test.ts +++ b/gitnexus/test/unit/run-analyze.test.ts @@ -188,10 +188,11 @@ describe('run-analyze module', () => { await registerRepo(tmpRepo.dbPath, meta, { name: 'old' }); const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const logs: string[] = []; const result = await runFullAnalysis( tmpRepo.dbPath, { registryName: 'new' }, - { onProgress: () => {} }, + { onProgress: () => {}, onLog: (message) => logs.push(message) }, ); expect(result.alreadyUpToDate).toBe(true); @@ -199,6 +200,7 @@ describe('run-analyze module', () => { const entries = await readRegistry(); expect(entries).toHaveLength(1); expect(entries[0].name).toBe('new'); + expect(logs).toContain('Registry name changed: "old" -> "new".'); const agents = await fs.readFile(path.join(tmpRepo.dbPath, 'AGENTS.md'), 'utf-8'); expect(agents).toContain('**new**'); } finally {