diff --git a/layers/registry/server/utils/discovery-candidates.ts b/layers/registry/server/utils/discovery-candidates.ts index dcb2c708..9e43411a 100644 --- a/layers/registry/server/utils/discovery-candidates.ts +++ b/layers/registry/server/utils/discovery-candidates.ts @@ -115,6 +115,7 @@ function isPermanentSyncFailure(error: string): boolean { return error === 'tree_truncated' || error === 'repo fetch 404' || error === 'repo fetch 410' + || error.startsWith('move_refused:') } export function discoveryOutcomeFromSyncStats(stats: SyncRepoStats): DiscoveryAttemptOutcome { diff --git a/layers/registry/server/utils/sync-prefetch.ts b/layers/registry/server/utils/sync-prefetch.ts index a871bd2b..daf73b9f 100644 --- a/layers/registry/server/utils/sync-prefetch.ts +++ b/layers/registry/server/utils/sync-prefetch.ts @@ -5,7 +5,7 @@ import type { ExistingRepo } from './sync-repo' import { getRepoSummariesBatch } from './github-client' import { resolveRepoSourceIdentityFromRow } from './repo-source-identity' import { sameRepositoryName } from './repository-move' -import { repoUnchangedStatements, unchangedRepoStatus } from './sync-repo' +import { repositoryIdentityFailure, repoUnchangedStatements, unchangedRepoStatus } from './sync-repo' /** One repository the hourly sync is about to queue. */ export interface SyncCandidate { @@ -85,7 +85,7 @@ export async function prefetchUnchangedRepos( eligible.forEach((candidate, index) => { const row = rows.get(repoKey(candidate)) const summary = batch.summaries[index] - if (!row || !summary || row.in_progress === 1 || isMoved(candidate, summary) || !isUnchanged(row, summary)) + if (!row || !summary || row.in_progress === 1 || repositoryIdentityFailure(row, candidate, summary.repositoryId) || isMoved(candidate, summary) || !isUnchanged(row, summary)) return unchanged.add(repoKey(candidate)) writes.push(repoUnchangedStatements(dependencies.db, { @@ -138,7 +138,7 @@ async function loadPrefetchRows( SELECT json_extract(value, '$[0]'), json_extract(value, '$[1]') FROM json_each(?1) ) - SELECT r.owner, r.repo, r.last_tree_sha, r.pushed_at, r.source_owner, r.source_repo, + SELECT r.owner, r.repo, r.repository_id, r.last_tree_sha, r.pushed_at, r.source_owner, r.source_repo, EXISTS (SELECT 1 FROM skills s WHERE s.owner = r.owner AND s.repo = r.repo) AS has_skills, EXISTS ( SELECT 1 FROM repo_sync_progress p diff --git a/layers/registry/server/utils/sync-repo.ts b/layers/registry/server/utils/sync-repo.ts index 95eb5d50..691ee700 100644 --- a/layers/registry/server/utils/sync-repo.ts +++ b/layers/registry/server/utils/sync-repo.ts @@ -65,12 +65,19 @@ interface ExistingSkillAssets { } export interface ExistingRepo { + repository_id: number | null last_tree_sha: string | null pushed_at: number | null source_owner: string | null source_repo: string | null } +export function repositoryIdentityFailure(existing: Pick | null, name: RepositoryName, repositoryId: number): string | null { + if (existing?.repository_id == null || existing.repository_id === repositoryId) + return null + return `move_refused: ${name.owner}/${name.repo} is Repository ${repositoryId} on GitHub, and the registry holds that name for Repository ${existing.repository_id}` +} + export type RefreshRepoAssetsResult = | { _tag: 'refreshed' @@ -365,6 +372,11 @@ export async function refreshRepoAssets( } } + const identityFailure = repositoryIdentityFailure(existingRepo, { owner, repo }, repoRes.data.repositoryId) + if (identityFailure) { + return { _tag: 'failed', owner, repo, reason: identityFailure, retryable: false, rateLimited: false, unauthorized: false, ...rate } + } + const branch = repoRes.data.meta.default_branch || 'main' const sourceOwner = repoRes.data.meta.owner.login const sourceRepo = repoRes.data.meta.name @@ -489,7 +501,7 @@ async function followRepositoryMove( async function loadExistingRepo(db: D1Database, owner: string, repo: string): Promise { return await db .prepare(` - SELECT last_tree_sha, pushed_at, source_owner, source_repo + SELECT repository_id, last_tree_sha, pushed_at, source_owner, source_repo FROM repos WHERE owner = ? AND repo = ? `) @@ -559,7 +571,7 @@ async function repoHasAdmittedSkills(db: D1Database, owner: string, repo: string * still being discovered, so its tree must be read whatever the cursor says. */ export function unchangedRepoStatus(input: { - existing: ExistingRepo | null + existing: Pick | null hasAdmittedSkills: boolean headTreeSha: string | null repoPushedAt: number | null @@ -863,6 +875,12 @@ export async function syncRepo( // The rows follow it there, and the sync goes on under the new name, so a // caller keyed by the old name still gets an outcome (ADR-0015). const current: RepositoryName = { owner: sourceOwner, repo: sourceRepo } + const identityFailure = repositoryIdentityFailure(requestedRepoRow, requested, repositoryId) + if (identityFailure) { + stats.status = 'failed' + stats.reason = identityFailure + return stats + } const movePlan = sameRepositoryName(requested, current) ? null : await followRepositoryMove(db, requested, current, repositoryId) diff --git a/server/jobs/registry/repo-maintenance.ts b/server/jobs/registry/repo-maintenance.ts index f21b1e02..ee380957 100644 --- a/server/jobs/registry/repo-maintenance.ts +++ b/server/jobs/registry/repo-maintenance.ts @@ -513,5 +513,6 @@ function isPermanentRepoFailure(reason: string | undefined): boolean { || reason === 'repo fetch 410' || reason === 'tree_truncated' || reason === 'repo_too_large_to_index' + || reason?.startsWith('move_refused:') === true || reason?.startsWith('skill_parse_rejected:') === true } diff --git a/test/unit/discovery-candidates.test.ts b/test/unit/discovery-candidates.test.ts index 93d316fb..8e48b1d3 100644 --- a/test/unit/discovery-candidates.test.ts +++ b/test/unit/discovery-candidates.test.ts @@ -473,6 +473,12 @@ describe('sync failure permanence', () => { } as never)).toEqual({ _tag: 'rejected', reason: 'tree_truncated' }) }) + it('rejects an identity conflict without another discovery attempt', () => { + const reason = 'move_refused: acme/skills is Repository 1 on GitHub, and the registry holds that name for Repository 111' + expect(discoveryOutcomeFromSyncStats({ status: 'failed', reason } as never)) + .toEqual({ _tag: 'rejected', reason }) + }) + it('keeps retrying throttled and server-side failures', () => { for (const status of [403, 429, 500, 502]) { expect(discoveryOutcomeFromSyncStats({ diff --git a/test/unit/repo-maintenance-continuation.test.ts b/test/unit/repo-maintenance-continuation.test.ts index 0a5eef88..289e707b 100644 --- a/test/unit/repo-maintenance-continuation.test.ts +++ b/test/unit/repo-maintenance-continuation.test.ts @@ -215,6 +215,19 @@ describe('permanent repository failures on the sync path', () => { .toMatchObject({ count: 0 }) }) + it('ends an identity conflict without retrying or retaining continuation progress', async () => { + const reason = 'move_refused: acme/skills is Repository 1 on GitHub, and the registry holds that name for Repository 111' + syncRepo.mockResolvedValue(failedWith(reason)) + const { ctx, control } = jobContext(db) + + await handleRegistryRepoJob({ ...syncPayload }, ctx as never) + + expect(control.action).toBe('failed') + expect(control.error).toBe(reason) + expect(sqlite.prepare('SELECT COUNT(*) AS count FROM repo_sync_progress').get()) + .toMatchObject({ count: 0 }) + }) + it('records the bare reason the health check compares against', async () => { syncRepo.mockResolvedValue(failedWith('repo fetch 410')) const { ctx, control } = jobContext(db) diff --git a/test/unit/sync-prefetch.test.ts b/test/unit/sync-prefetch.test.ts index a0b6b4b6..26952ee8 100644 --- a/test/unit/sync-prefetch.test.ts +++ b/test/unit/sync-prefetch.test.ts @@ -116,6 +116,17 @@ describe('prefetchUnchangedRepos', () => { .toEqual({ repo_meta_synced_at: null }) }) + it('queues a reused name without replacing the stored Repository ID or freshness', async () => { + harness.raw.exec(`UPDATE repos SET repository_id=111 WHERE owner='acme' AND repo='unchanged'`) + vi.stubGlobal('fetch', vi.fn(graphqlAnswer)) + + const result = await prefetchUnchangedRepos({ db: harness.db, bindings, now: NOW }, [candidate('unchanged')]) + + expect(result).toMatchObject({ _tag: 'prefetched', unchanged: 0, queue: [candidate('unchanged')] }) + expect(harness.raw.prepare(`SELECT repository_id,stars,repo_meta_synced_at FROM repos WHERE owner='acme' AND repo='unchanged'`).get()) + .toEqual({ repository_id: 111, stars: 1, repo_meta_synced_at: null }) + }) + it('queues a changed tree even when the push timestamp has not advanced', async () => { harness.raw.prepare(`UPDATE repos SET pushed_at = ? WHERE owner = 'acme' AND repo = 'changed'`) .run(Date.parse('2026-10-01T00:00:00Z') / 1000) diff --git a/test/unit/sync-repo-freshness.test.ts b/test/unit/sync-repo-freshness.test.ts index dd9cd692..060da180 100644 --- a/test/unit/sync-repo-freshness.test.ts +++ b/test/unit/sync-repo-freshness.test.ts @@ -1,6 +1,6 @@ import Database from 'better-sqlite3' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { syncRepo } from '../../layers/registry/server/utils/sync-repo' +import { refreshRepoAssets, syncRepo } from '../../layers/registry/server/utils/sync-repo' const github = vi.hoisted(() => ({ getBlobsBatch: vi.fn(), @@ -123,7 +123,35 @@ describe('syncRepo freshness cursor', () => { sqlite.close() }) - it('advances repo metadata freshness on the GraphQL tree-SHA skip path', async () => { + it.each(['skills', 'renamed'])('refuses a reused name before updating metadata or following it to %s', async (canonicalName) => { + sqlite.exec(`UPDATE repos SET repository_id=111 WHERE owner='acme' AND repo='skills'`) + const summary = repoSummary() + summary.data.meta.name = canonicalName + github.getRepoSummary.mockResolvedValue(summary) + + const result = await syncRepo('acme', 'skills', {}, db) + + expect(result).toMatchObject({ status: 'failed', reason: 'move_refused: acme/skills is Repository 1 on GitHub, and the registry holds that name for Repository 111' }) + expect(sqlite.prepare(`SELECT repository_id,stars,repo_meta_synced_at FROM repos WHERE owner='acme' AND repo='skills'`).get()) + .toEqual({ repository_id: 111, stars: 1, repo_meta_synced_at: 75 }) + expect(github.getTree).not.toHaveBeenCalled() + expect(github.getBlobsBatch).not.toHaveBeenCalled() + }) + + it('refuses assets from a different Repository at the stored name', async () => { + sqlite.exec(`UPDATE repos SET repository_id=111 WHERE owner='acme' AND repo='skills'`) + github.getTree.mockResolvedValue({ status: 503, data: null, rateLimit: null }) + + const result = await refreshRepoAssets('acme', 'skills', {}, db) + + expect(result).toMatchObject({ _tag: 'failed', retryable: false, rateLimited: false, unauthorized: false }) + expect(github.getTree).not.toHaveBeenCalled() + expect(sqlite.prepare(`SELECT source_owner,source_repo FROM repos WHERE owner='acme' AND repo='skills'`).get()) + .toEqual({ source_owner: null, source_repo: null }) + }) + + it.each([null, 1])('advances repo metadata freshness when the stored ID is %s', async (repositoryId) => { + sqlite.prepare(`UPDATE repos SET repository_id=? WHERE owner='acme' AND repo='skills'`).run(repositoryId) const result = await syncRepo('acme', 'skills', {}, db) const row = sqlite.prepare(` SELECT default_branch, stars, forks, pushed_at, repo_created_at,