mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-01 02:01:24 +00:00
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 <cursoragent@cursor.com> * chore(autofix): apply prettier + eslint fixes via /autofix command --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This commit is contained in:
parent
dcb2eb5cb4
commit
ee2feb7a5b
4 changed files with 75 additions and 4 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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<void>;
|
||||
/**
|
||||
* 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<string> => {
|
||||
): Promise<RegisterRepoUnlockedResult> => {
|
||||
// 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<string> => withRegistryLock(() => registerRepoUnlocked(repoPath, meta, opts));
|
||||
): Promise<string> => {
|
||||
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.
|
||||
|
|
|
|||
|
|
@ -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' });
|
||||
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue