From da67dcd5cc9ce2a0e1b5e6888ca19dc95558da27 Mon Sep 17 00:00:00 2001 From: Ken-ichi Ueda Date: Tue, 26 May 2026 17:48:14 -0700 Subject: [PATCH] Fix issue listing for cross-PDS repositories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Rename buildRepoAtUri → resolveRepoId; return repoDid from the sh.tangled.repo record instead of the record's AT-URI, since issue records reference repoDid in their .repo field * Fix repo name matching to check both rkey and value.name, handling older tangled.org records that use an opaque TID as rkey Co-authored-by: Claude (claude-sonnet-4-6) --- src/commands/issue.ts | 14 +- src/utils/at-uri.ts | 67 +++++---- tests/commands/issue.test.ts | 12 +- tests/utils/at-uri.test.ts | 281 +++++++++++++---------------------- 4 files changed, 153 insertions(+), 221 deletions(-) diff --git a/src/commands/issue.ts b/src/commands/issue.ts index db82339..15d61d6 100644 --- a/src/commands/issue.ts +++ b/src/commands/issue.ts @@ -13,7 +13,7 @@ import { resolveSequentialNumber, updateIssue, } from '../lib/issues-api.js'; -import { buildRepoAtUri } from '../utils/at-uri.js'; +import { resolveRepoId } from '../utils/at-uri.js'; import { ensureAuthenticated, requireAuth } from '../utils/auth-helpers.js'; import { readBodyInput } from '../utils/body-input.js'; import { formatDate, formatIssueState, outputJson } from '../utils/formatting.js'; @@ -122,7 +122,7 @@ function createViewCommand(): Command { } // 3. Build repo AT-URI - const repoAtUri = await buildRepoAtUri(context.owner, context.name, client); + const repoAtUri = await resolveRepoId(context.owner, context.name); // 4. Resolve issue ID to URI const { uri: issueUri, displayId } = await resolveIssueUri(issueId, client, repoAtUri); @@ -195,7 +195,7 @@ function createEditCommand(): Command { } // 4. Build repo AT-URI - const repoAtUri = await buildRepoAtUri(context.owner, context.name, client); + const repoAtUri = await resolveRepoId(context.owner, context.name); // 5. Resolve issue ID to URI const { uri: issueUri, displayId } = await resolveIssueUri(issueId, client, repoAtUri); @@ -275,7 +275,7 @@ function createCloseCommand(): Command { } // 3. Build repo AT-URI - const repoAtUri = await buildRepoAtUri(context.owner, context.name, client); + const repoAtUri = await resolveRepoId(context.owner, context.name); // 4. Resolve issue ID to URI const { uri: issueUri, displayId } = await resolveIssueUri(issueId, client, repoAtUri); @@ -332,7 +332,7 @@ function createReopenCommand(): Command { } // 3. Build repo AT-URI - const repoAtUri = await buildRepoAtUri(context.owner, context.name, client); + const repoAtUri = await resolveRepoId(context.owner, context.name); // 4. Resolve issue ID to URI const { uri: issueUri, displayId } = await resolveIssueUri(issueId, client, repoAtUri); @@ -421,7 +421,7 @@ function createCreateCommand(): Command { } // 5. Build repo AT-URI - const repoAtUri = await buildRepoAtUri(context.owner, context.name, client); + const repoAtUri = await resolveRepoId(context.owner, context.name); // 6. Create issue (suppress progress message in JSON mode) if (options.json === undefined) { @@ -496,7 +496,7 @@ function createListCommand(): Command { } // 3. Build repo AT-URI - const repoAtUri = await buildRepoAtUri(context.owner, context.name, client); + const repoAtUri = await resolveRepoId(context.owner, context.name); // 4. Fetch issues const limit = Number.parseInt(options.limit, 10); diff --git a/src/utils/at-uri.ts b/src/utils/at-uri.ts index da4a81f..320c6a5 100644 --- a/src/utils/at-uri.ts +++ b/src/utils/at-uri.ts @@ -1,4 +1,6 @@ -import type { TangledApiClient } from '../lib/api-client.js'; +import { AtpAgent } from '@atproto/api'; +import { IdResolver } from '@atproto/identity'; +import { resolvePdsFromIdentifier } from './pds-resolver.js'; /** * Parse an AT-URI into its components @@ -28,30 +30,31 @@ export function parseAtUri(uri: string): { } /** - * Resolve a handle to a DID using the AT Protocol identity resolution + * Resolve a handle to a DID using AT Protocol identity resolution. + * Uses @atproto/identity directly rather than routing through the authenticated + * agent, so the result is independent of which PDS the caller is logged into. + * * @param handle - Handle string (e.g., "mark.bsky.social" or "@mark.bsky.social") - * @param client - Authenticated API client * @returns DID string (e.g., "did:plc:abc123") * @throws Error if handle cannot be resolved */ -export async function resolveHandleToDid( - handle: string, - client: TangledApiClient -): Promise { +export async function resolveHandleToDid(handle: string): Promise { // Strip leading @ if present const cleanHandle = handle.startsWith('@') ? handle.slice(1) : handle; try { - const response = await client.getAgent().com.atproto.identity.resolveHandle({ - handle: cleanHandle, - }); + const resolver = new IdResolver(); + const did = await resolver.handle.resolve(cleanHandle); - if (!response.data.did) { + if (!did) { throw new Error(`No DID found for handle: ${cleanHandle}`); } - return response.data.did; + return did; } catch (error) { + if (error instanceof Error && error.message.startsWith('No DID found')) { + throw error; + } if (error instanceof Error) { throw new Error(`Failed to resolve handle '${cleanHandle}': ${error.message}`); } @@ -60,42 +63,52 @@ export async function resolveHandleToDid( } /** - * Build a repository AT-URI from owner and repository name + * Resolve a repository identifier to its repo DID (the identifier used in issue records). + * Resolves the owner's PDS independently of the caller's authenticated session, + * so cross-PDS lookups (e.g., a tngl.sh user looking up a bsky.social repo) work. + * + * Issue records store `repoDid` in their `.repo` field, not the AT-URI of the + * `sh.tangled.repo` record. Falls back to the record's AT-URI if `repoDid` is absent. + * * @param ownerDidOrHandle - DID (e.g., "did:plc:abc") or handle (e.g., "mark.bsky.social") * @param repoName - Repository name - * @param client - Authenticated API client - * @returns AT-URI string (e.g., "at://did:plc:abc/sh.tangled.repo/3mef23waqwq22") + * @returns repoDid string, or record AT-URI as fallback * @throws Error if repository not found */ -export async function buildRepoAtUri( - ownerDidOrHandle: string, - repoName: string, - client: TangledApiClient -): Promise { +export async function resolveRepoId(ownerDidOrHandle: string, repoName: string): Promise { // Resolve owner to DID const isDid = ownerDidOrHandle.startsWith('did:'); - const did = isDid ? ownerDidOrHandle : await resolveHandleToDid(ownerDidOrHandle, client); + const did = isDid ? ownerDidOrHandle : await resolveHandleToDid(ownerDidOrHandle); try { + // Resolve the owner's PDS and query their records directly, + // bypassing the caller's authenticated agent entirely + const ownerPds = await resolvePdsFromIdentifier(did); + const ownerAgent = new AtpAgent({ service: ownerPds }); + // Query for sh.tangled.repo records - const response = await client.getAgent().com.atproto.repo.listRecords({ + const response = await ownerAgent.com.atproto.repo.listRecords({ repo: did, collection: 'sh.tangled.repo', limit: 100, // Reasonable limit for most users }); - // Find the record matching the repo name + // Find the record matching the repo name. + // Newer records use the repo name as the rkey; older records store the name + // in value.name with an opaque tid as the rkey. Check both. const repoRecord = response.data.records.find((record) => { - const recordData = record.value as { name?: string }; - return recordData.name === repoName; + const rkey = record.uri.split('/').pop(); + const recordName = (record.value as { name?: string }).name; + return rkey === repoName || recordName === repoName; }); if (!repoRecord) { throw new Error(`Repository '${repoName}' not found for ${ownerDidOrHandle}`); } - // Return the record's URI (which includes the correct rkey) - return repoRecord.uri; + // Return repoDid if present; fall back to record URI + const repoDid = (repoRecord.value as { repoDid?: string }).repoDid; + return repoDid ?? repoRecord.uri; } catch (error) { if (error instanceof Error) { throw new Error(`Failed to resolve repository AT-URI: ${error.message}`); diff --git a/tests/commands/issue.test.ts b/tests/commands/issue.test.ts index c305428..79e431c 100644 --- a/tests/commands/issue.test.ts +++ b/tests/commands/issue.test.ts @@ -49,7 +49,7 @@ describe('issue create command', () => { }); // Mock AT-URI builder - vi.mocked(atUri.buildRepoAtUri).mockResolvedValue( + vi.mocked(atUri.resolveRepoId).mockResolvedValue( 'at://did:plc:abc123/sh.tangled.repo/test-repo' ); @@ -341,7 +341,7 @@ describe('issue list command', () => { }); // Mock AT-URI builder - vi.mocked(atUri.buildRepoAtUri).mockResolvedValue('at://did:plc:abc123/sh.tangled.repo/xyz789'); + vi.mocked(atUri.resolveRepoId).mockResolvedValue('at://did:plc:abc123/sh.tangled.repo/xyz789'); }); afterEach(() => { @@ -604,7 +604,7 @@ describe('issue view command', () => { protocol: 'ssh', }); - vi.mocked(atUri.buildRepoAtUri).mockResolvedValue('at://did:plc:abc123/sh.tangled.repo/xyz789'); + vi.mocked(atUri.resolveRepoId).mockResolvedValue('at://did:plc:abc123/sh.tangled.repo/xyz789'); vi.mocked(authHelpers.requireAuth).mockResolvedValue({ did: 'did:plc:abc123', @@ -830,7 +830,7 @@ describe('issue edit command', () => { protocol: 'ssh', }); - vi.mocked(atUri.buildRepoAtUri).mockResolvedValue('at://did:plc:abc123/sh.tangled.repo/xyz789'); + vi.mocked(atUri.resolveRepoId).mockResolvedValue('at://did:plc:abc123/sh.tangled.repo/xyz789'); vi.mocked(bodyInput.readBodyInput).mockResolvedValue(undefined); vi.mocked(authHelpers.requireAuth).mockResolvedValue({ @@ -1004,7 +1004,7 @@ describe('issue close command', () => { protocol: 'ssh', }); - vi.mocked(atUri.buildRepoAtUri).mockResolvedValue('at://did:plc:abc123/sh.tangled.repo/xyz789'); + vi.mocked(atUri.resolveRepoId).mockResolvedValue('at://did:plc:abc123/sh.tangled.repo/xyz789'); vi.mocked(issuesApi.getCompleteIssueData).mockResolvedValue({ number: 1, title: mockIssue.title, @@ -1121,7 +1121,7 @@ describe('issue reopen command', () => { protocol: 'ssh', }); - vi.mocked(atUri.buildRepoAtUri).mockResolvedValue('at://did:plc:abc123/sh.tangled.repo/xyz789'); + vi.mocked(atUri.resolveRepoId).mockResolvedValue('at://did:plc:abc123/sh.tangled.repo/xyz789'); vi.mocked(issuesApi.getCompleteIssueData).mockResolvedValue({ number: 1, title: mockIssue.title, diff --git a/tests/utils/at-uri.test.ts b/tests/utils/at-uri.test.ts index 2b3f920..f38dde0 100644 --- a/tests/utils/at-uri.test.ts +++ b/tests/utils/at-uri.test.ts @@ -1,21 +1,27 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -import type { TangledApiClient } from '../../src/lib/api-client.js'; -import { buildRepoAtUri, parseAtUri, resolveHandleToDid } from '../../src/utils/at-uri.js'; - -// Mock API client -const createMockClient = (): TangledApiClient => { - return { - getAgent: vi.fn(() => ({ - com: { - atproto: { - identity: { - resolveHandle: vi.fn(), - }, - }, - }, - })), - } as unknown as TangledApiClient; -}; +import { parseAtUri, resolveHandleToDid, resolveRepoId } from '../../src/utils/at-uri.js'; + +const { mockHandleResolve, mockListRecords, mockResolvePds } = vi.hoisted(() => ({ + mockHandleResolve: vi.fn(), + mockListRecords: vi.fn(), + mockResolvePds: vi.fn().mockResolvedValue('https://pds.example.com'), +})); + +vi.mock('@atproto/identity', () => ({ + IdResolver: vi.fn().mockImplementation(() => ({ + handle: { resolve: mockHandleResolve }, + })), +})); + +vi.mock('../../src/utils/pds-resolver.js', () => ({ + resolvePdsFromIdentifier: mockResolvePds, +})); + +vi.mock('@atproto/api', () => ({ + AtpAgent: vi.fn().mockImplementation(() => ({ + com: { atproto: { repo: { listRecords: mockListRecords } } }, + })), +})); describe('parseAtUri', () => { it('should parse AT-URI with rkey', () => { @@ -70,265 +76,178 @@ describe('parseAtUri', () => { }); describe('resolveHandleToDid', () => { - let mockClient: TangledApiClient; - beforeEach(() => { - mockClient = createMockClient(); + vi.clearAllMocks(); }); it('should resolve handle to DID', async () => { - const mockResolve = vi.fn().mockResolvedValue({ - data: { did: 'did:plc:abc123' }, - }); - - vi.mocked(mockClient.getAgent).mockReturnValue({ - com: { - atproto: { - identity: { - resolveHandle: mockResolve, - }, - }, - }, - } as never); + mockHandleResolve.mockResolvedValue('did:plc:abc123'); - const result = await resolveHandleToDid('mark.bsky.social', mockClient); + const result = await resolveHandleToDid('mark.bsky.social'); expect(result).toBe('did:plc:abc123'); - expect(mockResolve).toHaveBeenCalledWith({ handle: 'mark.bsky.social' }); + expect(mockHandleResolve).toHaveBeenCalledWith('mark.bsky.social'); }); it('should strip leading @ from handle', async () => { - const mockResolve = vi.fn().mockResolvedValue({ - data: { did: 'did:plc:abc123' }, - }); - - vi.mocked(mockClient.getAgent).mockReturnValue({ - com: { - atproto: { - identity: { - resolveHandle: mockResolve, - }, - }, - }, - } as never); + mockHandleResolve.mockResolvedValue('did:plc:abc123'); - await resolveHandleToDid('@mark.bsky.social', mockClient); + await resolveHandleToDid('@mark.bsky.social'); - expect(mockResolve).toHaveBeenCalledWith({ handle: 'mark.bsky.social' }); + expect(mockHandleResolve).toHaveBeenCalledWith('mark.bsky.social'); }); - it('should throw error when handle not found', async () => { - const mockResolve = vi.fn().mockResolvedValue({ - data: { did: null }, - }); - - vi.mocked(mockClient.getAgent).mockReturnValue({ - com: { - atproto: { - identity: { - resolveHandle: mockResolve, - }, - }, - }, - } as never); + it('should throw error when handle resolves to nothing', async () => { + mockHandleResolve.mockResolvedValue(undefined); - await expect(resolveHandleToDid('nonexistent.bsky.social', mockClient)).rejects.toThrow( + await expect(resolveHandleToDid('nonexistent.bsky.social')).rejects.toThrow( 'No DID found for handle: nonexistent.bsky.social' ); }); - it('should throw error on network failure', async () => { - const mockResolve = vi.fn().mockRejectedValue(new Error('Network error')); - - vi.mocked(mockClient.getAgent).mockReturnValue({ - com: { - atproto: { - identity: { - resolveHandle: mockResolve, - }, - }, - }, - } as never); + it('should throw error on resolution failure', async () => { + mockHandleResolve.mockRejectedValue(new Error('Network error')); - await expect(resolveHandleToDid('mark.bsky.social', mockClient)).rejects.toThrow( + await expect(resolveHandleToDid('mark.bsky.social')).rejects.toThrow( "Failed to resolve handle 'mark.bsky.social': Network error" ); }); }); -describe('buildRepoAtUri', () => { - let mockClient: TangledApiClient; - +describe('resolveRepoId', () => { beforeEach(() => { - mockClient = createMockClient(); + vi.clearAllMocks(); + mockResolvePds.mockResolvedValue('https://pds.example.com'); }); - it('should query PDS and use repo record rkey', async () => { - const mockListRecords = vi.fn().mockResolvedValue({ + it('should return repoDid from the repo record', async () => { + mockListRecords.mockResolvedValue({ data: { records: [ { - uri: 'at://did:plc:abc123/sh.tangled.repo/3mef23waqwq22', - value: { name: 'my-repo', description: 'Test repo' }, + uri: 'at://did:plc:abc123/sh.tangled.repo/my-repo', + value: { repoDid: 'did:plc:repodid111', knot: 'knot1.tangled.sh' }, }, ], }, }); - vi.mocked(mockClient.getAgent).mockReturnValue({ - com: { - atproto: { - repo: { - listRecords: mockListRecords, - }, - }, - }, - } as never); - - const result = await buildRepoAtUri('did:plc:abc123', 'my-repo', mockClient); + const result = await resolveRepoId('did:plc:abc123', 'my-repo'); - expect(result).toBe('at://did:plc:abc123/sh.tangled.repo/3mef23waqwq22'); + expect(mockResolvePds).toHaveBeenCalledWith('did:plc:abc123'); expect(mockListRecords).toHaveBeenCalledWith({ repo: 'did:plc:abc123', collection: 'sh.tangled.repo', limit: 100, }); + expect(result).toBe('did:plc:repodid111'); }); - it('should resolve handle then query for repo record', async () => { - const mockResolve = vi.fn().mockResolvedValue({ - data: { did: 'did:plc:abc123' }, - }); - - const mockListRecords = vi.fn().mockResolvedValue({ + it('should resolve handle to DID then return repoDid', async () => { + mockHandleResolve.mockResolvedValue('did:plc:abc123'); + mockListRecords.mockResolvedValue({ data: { records: [ { - uri: 'at://did:plc:abc123/sh.tangled.repo/xyz789', - value: { name: 'my-repo' }, + uri: 'at://did:plc:abc123/sh.tangled.repo/my-repo', + value: { repoDid: 'did:plc:repodid111', knot: 'knot1.tangled.sh' }, }, ], }, }); - vi.mocked(mockClient.getAgent).mockReturnValue({ - com: { - atproto: { - identity: { - resolveHandle: mockResolve, - }, - repo: { - listRecords: mockListRecords, - }, - }, - }, - } as never); - - const result = await buildRepoAtUri('mark.bsky.social', 'my-repo', mockClient); + const result = await resolveRepoId('mark.bsky.social', 'my-repo'); - expect(result).toBe('at://did:plc:abc123/sh.tangled.repo/xyz789'); - expect(mockResolve).toHaveBeenCalledWith({ handle: 'mark.bsky.social' }); - expect(mockListRecords).toHaveBeenCalledWith({ - repo: 'did:plc:abc123', - collection: 'sh.tangled.repo', - limit: 100, - }); + expect(mockHandleResolve).toHaveBeenCalledWith('mark.bsky.social'); + expect(mockResolvePds).toHaveBeenCalledWith('did:plc:abc123'); + expect(result).toBe('did:plc:repodid111'); }); - it('should find correct repo among multiple records', async () => { - const mockListRecords = vi.fn().mockResolvedValue({ + it('should find correct repo among multiple records by rkey', async () => { + mockListRecords.mockResolvedValue({ data: { records: [ { - uri: 'at://did:plc:abc123/sh.tangled.repo/aaa111', - value: { name: 'other-repo' }, + uri: 'at://did:plc:abc123/sh.tangled.repo/other-repo', + value: { repoDid: 'did:plc:other' }, }, { - uri: 'at://did:plc:abc123/sh.tangled.repo/bbb222', - value: { name: 'target-repo' }, + uri: 'at://did:plc:abc123/sh.tangled.repo/target-repo', + value: { repoDid: 'did:plc:target' }, }, { - uri: 'at://did:plc:abc123/sh.tangled.repo/ccc333', - value: { name: 'another-repo' }, + uri: 'at://did:plc:abc123/sh.tangled.repo/another-repo', + value: { repoDid: 'did:plc:another' }, }, ], }, }); - vi.mocked(mockClient.getAgent).mockReturnValue({ - com: { - atproto: { - repo: { - listRecords: mockListRecords, + const result = await resolveRepoId('did:plc:abc123', 'target-repo'); + + expect(result).toBe('did:plc:target'); + }); + + it('should match by value.name when rkey is an opaque tid', async () => { + // Older tangled.org records use an opaque tid as rkey and store the repo + // name in value.name instead + mockListRecords.mockResolvedValue({ + data: { + records: [ + { + uri: 'at://did:plc:abc123/sh.tangled.repo/3mksu5zaint22', + value: { name: 'my-repo', repoDid: 'did:plc:repodid222', knot: 'knot1.tangled.sh' }, }, - }, + ], }, - } as never); + }); - const result = await buildRepoAtUri('did:plc:abc123', 'target-repo', mockClient); + const result = await resolveRepoId('did:plc:abc123', 'my-repo'); - expect(result).toBe('at://did:plc:abc123/sh.tangled.repo/bbb222'); + expect(result).toBe('did:plc:repodid222'); }); - it('should throw error when repository not found', async () => { - const mockListRecords = vi.fn().mockResolvedValue({ + it('should fall back to record URI when repoDid is absent', async () => { + mockListRecords.mockResolvedValue({ data: { records: [ { - uri: 'at://did:plc:abc123/sh.tangled.repo/xyz789', - value: { name: 'different-repo' }, + uri: 'at://did:plc:abc123/sh.tangled.repo/my-repo', + value: { knot: 'knot1.tangled.sh' }, }, ], }, }); - vi.mocked(mockClient.getAgent).mockReturnValue({ - com: { - atproto: { - repo: { - listRecords: mockListRecords, - }, - }, + const result = await resolveRepoId('did:plc:abc123', 'my-repo'); + + expect(result).toBe('at://did:plc:abc123/sh.tangled.repo/my-repo'); + }); + + it('should throw error when repository not found', async () => { + mockListRecords.mockResolvedValue({ + data: { + records: [{ uri: 'at://did:plc:abc123/sh.tangled.repo/different-repo', value: {} }], }, - } as never); + }); - await expect(buildRepoAtUri('did:plc:abc123', 'nonexistent-repo', mockClient)).rejects.toThrow( + await expect(resolveRepoId('did:plc:abc123', 'nonexistent-repo')).rejects.toThrow( "Repository 'nonexistent-repo' not found for did:plc:abc123" ); }); it('should throw error when handle resolution fails', async () => { - const mockResolve = vi.fn().mockRejectedValue(new Error('Resolution failed')); + mockHandleResolve.mockRejectedValue(new Error('Resolution failed')); - vi.mocked(mockClient.getAgent).mockReturnValue({ - com: { - atproto: { - identity: { - resolveHandle: mockResolve, - }, - }, - }, - } as never); - - await expect(buildRepoAtUri('mark.bsky.social', 'my-repo', mockClient)).rejects.toThrow( + await expect(resolveRepoId('mark.bsky.social', 'my-repo')).rejects.toThrow( "Failed to resolve handle 'mark.bsky.social': Resolution failed" ); }); it('should throw error when listRecords fails', async () => { - const mockListRecords = vi.fn().mockRejectedValue(new Error('API error')); - - vi.mocked(mockClient.getAgent).mockReturnValue({ - com: { - atproto: { - repo: { - listRecords: mockListRecords, - }, - }, - }, - } as never); + mockListRecords.mockRejectedValue(new Error('API error')); - await expect(buildRepoAtUri('did:plc:abc123', 'my-repo', mockClient)).rejects.toThrow( + await expect(resolveRepoId('did:plc:abc123', 'my-repo')).rejects.toThrow( 'Failed to resolve repository AT-URI: API error' ); }); -- knotmirror 0.0.1