diff --git a/server/utils/repo-health.ts b/server/utils/repo-health.ts index 3d170ed..a5954f6 100644 --- a/server/utils/repo-health.ts +++ b/server/utils/repo-health.ts @@ -51,8 +51,12 @@ export interface RepoHealthInput { export function explainRepoError(raw: string): string | null { const err = raw.toLowerCase() - if (err.includes('sufficient access permissions') || err.includes('accesscontrol')) { - return 'Tangled rejected the mirror, usually because a repo with this name already exists on your account. Rename or remove the existing one, then resync.' + if ( + err.includes('a repo with this name already exists') + || err.includes('sufficient access permissions') + || err.includes('accesscontrol') + ) { + return 'A repo with this name already exists on your tangled account. Rename or remove the existing one, then resync.' } if (err.includes('invalid path sequence')) { return 'The repository name can\'t be used as a tangled repo name. This one can\'t be mirrored as-is.' diff --git a/server/utils/tangled-repo.ts b/server/utils/tangled-repo.ts index 56475cf..7cd1ee1 100644 --- a/server/utils/tangled-repo.ts +++ b/server/utils/tangled-repo.ts @@ -90,7 +90,52 @@ const DEFAULT_KNOT = 'knot1.tangled.sh' export interface EnrolResult { status: 'enrolled' | 'already' | 'skipped' - reason?: 'private' | 'fork' | 'no-identity' + reason?: 'private' | 'fork' | 'no-identity' | 'name-conflict' +} + +/** + * User-facing message stored in `repo_mapping.lastError` when enrolment is + * blocked by a name that already exists on the user's tangled account. Kept as + * a constant so `repo-health` and tests can match it exactly. Issue #4. + */ +export const NAME_CONFLICT_ERROR + = 'a repo with this name already exists on your tangled account; rename or remove it, then resync' + +/** + * True when the knot's `repo.create` rejection is a name collision: the user + * already has a `sh.tangled.repo` of that name, so the knot refuses to mint a + * second. The knot surfaces this as an `AccessControl` 400 ("DID does not have + * sufficient access permissions"). + */ +function isNameConflictResponse(status: number, body: string): boolean { + if (status !== 400) return false + const lc = body.toLowerCase() + return lc.includes('accesscontrol') || lc.includes('sufficient access permissions') +} + +/** + * List the names of the user's existing `sh.tangled.repo` records. Used to + * detect a collision before calling the knot so we fail fast with a clear + * status instead of a knot 400 (issue #4). + */ +export async function listExistingRepoNames(agent: Agent, did: string): Promise> { + const names = new Set() + let cursor: string | undefined + do { + // eslint-disable-next-line no-await-in-loop -- sequential pagination + const page = await agent.com.atproto.repo.listRecords({ + repo: did, + collection: REPO_LEXICON, + limit: LIST_RECORDS_PAGE_SIZE, + cursor, + }) + for (const rec of page.data.records) { + const value = rec.value as Record + if (typeof value.name === 'string') names.add(value.name) + } + cursor = page.data.cursor + } while (cursor) + return names } /** @@ -123,7 +168,7 @@ export async function enrollRepo(opts: { }): Promise { const db = useDb() - const existing = await db.select({ id: repoMapping.id, status: repoMapping.status }) + const existing = await db.select({ id: repoMapping.id, status: repoMapping.status, tangledFullName: repoMapping.tangledFullName }) .from(repoMapping) .where(sql`${repoMapping.installationId} = ${opts.installationId} AND ${repoMapping.githubRepoId} = ${opts.githubRepoId}`) if (existing.length > 0 && !opts.force) { @@ -150,6 +195,22 @@ export async function enrollRepo(opts: { // 3. Service-auth JWT for the knot procedure. const agent = new Agent(opts.oauthSession) + + // Fail fast on a name collision (issue #4): if the user already has a + // `sh.tangled.repo` of this name that isn't the one this mapping owns, the + // knot would reject `repo.create` with an opaque `AccessControl` 400 and the + // job would retry to exhaustion. Record a clear error status instead and + // stop, so the dashboard can explain it and the user can act. + const ownName = `${opts.oauthSession.did}/${name}` + const alreadyOurs = existing[0]?.tangledFullName === ownName + if (!alreadyOurs) { + const existingNames = await listExistingRepoNames(agent, opts.oauthSession.did) + if (existingNames.has(name)) { + await recordEnrolError(db, opts, repo.full_name, NAME_CONFLICT_ERROR) + return { status: 'skipped', reason: 'name-conflict' } + } + } + const aud = `did:web:${knot}` const exp = Math.floor(Date.now() / 1000) + 60 const { data: { token } } = await agent.com.atproto.server.getServiceAuth({ @@ -177,6 +238,13 @@ export async function enrollRepo(opts: { }) if (!knotResponse.ok) { const body = await knotResponse.text() + // A name collision can still race in between the pre-check and this call + // (or the pre-check's listRecords lagged the firehose). Treat it as the + // same terminal, user-actionable condition rather than a retryable throw. + if (isNameConflictResponse(knotResponse.status, body)) { + await recordEnrolError(db, opts, repo.full_name, NAME_CONFLICT_ERROR) + return { status: 'skipped', reason: 'name-conflict' } + } throw new Error(`knot ${knot} returned ${knotResponse.status}: ${body}`) } const knotJson: { repoDid?: string } = await knotResponse.json() @@ -236,6 +304,35 @@ export async function enrollRepo(opts: { return { status: 'enrolled' } } +/** + * Upsert a `repo_mapping` row in `error` state with a user-facing `lastError`. + * Used when enrolment can't complete but we still want the repo to show on the + * dashboard with an explanation rather than vanish silently. + */ +async function recordEnrolError( + db: ReturnType, + opts: { installationId: number, githubRepoId: number }, + githubFullName: string, + message: string, +): Promise { + const existing = await db.select({ id: repoMapping.id }) + .from(repoMapping) + .where(sql`${repoMapping.installationId} = ${opts.installationId} AND ${repoMapping.githubRepoId} = ${opts.githubRepoId}`) + if (existing.length > 0) { + await db.update(repoMapping) + .set({ status: 'error', lastError: message, updatedAt: new Date() }) + .where(sql`${repoMapping.id} = ${existing[0]!.id}`) + return + } + await db.insert(repoMapping).values({ + installationId: opts.installationId, + githubRepoId: opts.githubRepoId, + githubFullName, + status: 'error', + lastError: message, + }) +} + export interface SyncMetadataResult { status: 'synced' | 'skipped' reason?: 'no-mapping' | 'disabled' | 'private' | 'fork' | 'no-pds-record' diff --git a/test/unit/repo-health.spec.ts b/test/unit/repo-health.spec.ts index 6b5236f..bfee317 100644 --- a/test/unit/repo-health.spec.ts +++ b/test/unit/repo-health.spec.ts @@ -62,6 +62,8 @@ describe('repoHealth', () => { describe('explainRepoError', () => { it('maps the production error strings we actually emit', () => { + expect(explainRepoError('a repo with this name already exists on your tangled account; rename or remove it, then resync')).toMatch(/already exists/i) + expect(explainRepoError('knot knot1.tangled.sh returned 400: {"error":"AccessControl"}')).toMatch(/already exists/i) expect(explainRepoError('receive-pack: unpack error (pack signature mismatch detected)')).toMatch(/transient connection/i) expect(explainRepoError('receive-pack: no advertisement (stderr: ssh error: Timed out while waiting for handshake)')).toMatch(/transient connection/i) expect(explainRepoError('knot rejected our ssh key; stopping sync')).toMatch(/ssh key/i) diff --git a/test/unit/tangled-repo.spec.ts b/test/unit/tangled-repo.spec.ts index cc4049b..0f1af28 100644 --- a/test/unit/tangled-repo.spec.ts +++ b/test/unit/tangled-repo.spec.ts @@ -8,6 +8,7 @@ import { buildReadOnlyDescription, enrollRepo, mergeRepoRecord, + NAME_CONFLICT_ERROR, stripReadOnlyMarker, syncRepoMetadata, } from '../../server/utils/tangled-repo' @@ -96,6 +97,9 @@ describe('enrollRepo', () => { getServiceAuthMock.mockResolvedValue({ data: { token: 'service-auth-jwt' } }) putRecordMock.mockResolvedValue({ data: { uri: 'at://did:plc:abc/sh.tangled.repo/whatever', cid: 'bafy' } }) + // Default: the user has no existing tangled repos, so the name-conflict + // pre-check passes. Individual tests override this to force a collision. + listRecordsMock.mockResolvedValue({ data: { records: [] } }) }) afterEach(() => { @@ -187,6 +191,61 @@ describe('enrollRepo', () => { expect(fakeFetch).not.toHaveBeenCalled() }) + it('skips enrolment and records an error when the name already exists on tangled', async () => { + githubGet.mockResolvedValue({ data: ghRepo() }) + listRecordsMock.mockResolvedValue({ + data: { records: [{ uri: 'at://did:plc:abc/sh.tangled.repo/existing', cid: 'bafy', value: { name: 'my-project' } }] }, + }) + + const result = await enrollRepo({ + oauthSession: fakeOauthSession('did:plc:abc'), + installationId: 1, + githubRepoId: 9001, + }) + expect(result).toEqual({ status: 'skipped', reason: 'name-conflict' }) + // Knot procedure never called; PDS record never written. + expect(fakeFetch).not.toHaveBeenCalled() + expect(putRecordMock).not.toHaveBeenCalled() + + const rows = await useDb().select().from(repoMapping).where(sql`${repoMapping.installationId} = 1`) + expect(rows).toHaveLength(1) + expect(rows[0]!.status).toBe('error') + expect(rows[0]!.lastError).toBe(NAME_CONFLICT_ERROR) + expect(rows[0]!.tangledRepoDid).toBeNull() + }) + + it('classifies a knot AccessControl 400 as a name conflict rather than retrying', async () => { + githubGet.mockResolvedValue({ data: ghRepo() }) + // Pre-check sees no collision (firehose lag), but the knot rejects. + fakeFetch.mockResolvedValue(new Response( + JSON.stringify({ error: 'AccessControl', message: 'DID does not have sufficient access permissions for this operation' }), + { status: 400 }, + )) + + const result = await enrollRepo({ + oauthSession: fakeOauthSession('did:plc:abc'), + installationId: 1, + githubRepoId: 9001, + }) + expect(result).toEqual({ status: 'skipped', reason: 'name-conflict' }) + + const rows = await useDb().select().from(repoMapping).where(sql`${repoMapping.installationId} = 1`) + expect(rows).toHaveLength(1) + expect(rows[0]!.status).toBe('error') + expect(rows[0]!.lastError).toBe(NAME_CONFLICT_ERROR) + }) + + it('still throws on non-conflict knot errors so the queue retries', async () => { + githubGet.mockResolvedValue({ data: ghRepo() }) + fakeFetch.mockResolvedValue(new Response('upstream boom', { status: 502 })) + + await expect(enrollRepo({ + oauthSession: fakeOauthSession('did:plc:abc'), + installationId: 1, + githubRepoId: 9001, + })).rejects.toThrow(/returned 502/) + }) + it('no-ops if a mapping already exists', async () => { await useDb().insert(repoMapping).values({ installationId: 1,