From 4489d7425588867fbe3071d0cd5a7be1406ca2af Mon Sep 17 00:00:00 2001 From: Mark Bennett Date: Wed, 11 Feb 2026 14:23:04 +0000 Subject: [PATCH] fix: normalize JSON output fields across all issue commands All issue commands now return the full common field set (number, title, body, state, author, createdAt, uri, cid) in --json mode: - create: adds state:'open' (always open on creation) - view: adds number via getCompleteIssueData (replaces separate getIssue + getIssueState calls) - edit: adds number + state via parallel resolveSequentialNumber + getIssueState after updateIssue (avoids re-fetching the updated record) - close/reopen: adds body, author, createdAt via getCompleteIssueData with stateOverride; replaces separate getIssue + resolveSequentialNumber calls - delete: adds body, author, createdAt, state via getCompleteIssueData; replaces scattered local variables Update command tests to mock getCompleteIssueData and assert on the full field set. Co-Authored-By: Claude Sonnet 4.5 --- src/commands/issue.ts | 176 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------------------------------------------------- tests/commands/issue.test.ts | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------ 2 file(s) changed, 178 insertion(s)(+), 137 deletion(s)(-) diff --git a/src/commands/issue.ts b/src/commands/issue.ts --- a/src/commands/issue.ts +++ b/src/commands/issue.ts @@ -7,12 +7,14 @@ closeIssue, createIssue, deleteIssue, - getIssue, + getCompleteIssueData, getIssueState, listIssues, reopenIssue, + resolveSequentialNumber, updateIssue, } from '../lib/issues-api.js'; +import type { IssueData } from '../lib/issues-api.js'; import { buildRepoAtUri } from '../utils/at-uri.js'; import { requireAuth } from '../utils/auth-helpers.js'; import { readBodyInput } from '../utils/body-input.js'; @@ -87,38 +89,25 @@ } /** - * Resolve a sequential issue number from a displayId or by scanning the issue list. - * Fast path: if displayId is "#N", return N directly. - * Fallback: fetch all issues, sort oldest-first, return 1-based position. + * A custom subclass of Command with support for adding the common issue JSON flag. */ -async function resolveSequentialNumber( - displayId: string, - issueUri: string, - client: TangledApiClient, - repoAtUri: string -): Promise { - const match = displayId.match(/^#(\d+)$/); - if (match) return Number.parseInt(match[1], 10); - - const { issues } = await listIssues({ client, repoAtUri, limit: 100 }); - const sorted = issues.sort( - (a, b) => new Date(a.createdAt).getTime() - new Date(b.createdAt).getTime() - ); - const idx = sorted.findIndex((i) => i.uri === issueUri); - return idx >= 0 ? idx + 1 : undefined; +class IssueCommand extends Command { + addIssueJsonOption() { + return this.option( + '--json [fields]', + 'Output JSON; optionally specify comma-separated fields (number, title, body, state, author, createdAt, uri, cid)' + ); + } } /** * Issue view subcommand */ function createViewCommand(): Command { - return new Command('view') + return new IssueCommand('view') .description('View details of a specific issue') .argument('', 'Issue number (e.g., 1, #2) or rkey') - .option( - '--json [fields]', - 'Output JSON; optionally specify comma-separated fields (title, body, state, author, createdAt, uri, cid)' - ) + .addIssueJsonOption() .action(async (issueId: string, options: { json?: string | true }) => { try { // 1. Validate auth @@ -143,37 +132,25 @@ // 4. Resolve issue ID to URI const { uri: issueUri, displayId } = await resolveIssueUri(issueId, client, repoAtUri); - // 5. Fetch issue details - const issue = await getIssue({ client, issueUri }); + // 5. Fetch complete issue data (record, sequential number, state) + const issueData = await getCompleteIssueData(client, issueUri, displayId, repoAtUri); - // 6. Fetch issue state - const state = await getIssueState({ client, issueUri: issue.uri }); - - // 7. Output result + // 6. Output result if (options.json !== undefined) { - const issueData = { - title: issue.title, - body: issue.body, - state, - author: issue.author, - createdAt: issue.createdAt, - uri: issue.uri, - cid: issue.cid, - }; outputJson(issueData, typeof options.json === 'string' ? options.json : undefined); return; } - console.log(`\nIssue ${displayId} ${formatIssueState(state)}`); - console.log(`Title: ${issue.title}`); - console.log(`Author: ${issue.author}`); - console.log(`Created: ${formatDate(issue.createdAt)}`); + console.log(`\nIssue ${displayId} ${formatIssueState(issueData.state)}`); + console.log(`Title: ${issueData.title}`); + console.log(`Author: ${issueData.author}`); + console.log(`Created: ${formatDate(issueData.createdAt)}`); console.log(`Repo: ${context.name}`); - console.log(`URI: ${issue.uri}`); + console.log(`URI: ${issueData.uri}`); - if (issue.body) { + if (issueData.body) { console.log('\nBody:'); - console.log(issue.body); + console.log(issueData.body); } console.log(); // Empty line at end @@ -190,16 +167,13 @@ * Issue edit subcommand */ function createEditCommand(): Command { - return new Command('edit') + return new IssueCommand('edit') .description('Edit an issue title and/or body') .argument('', 'Issue number or rkey') .option('-t, --title ', 'New issue title') .option('-b, --body ', 'New issue body text') .option('-F, --body-file ', 'Read body from file (- for stdin)') - .option( - '--json [fields]', - 'Output JSON of the updated issue; optionally specify comma-separated fields (title, body, author, createdAt, uri, cid)' - ) + .addIssueJsonOption() .action( async ( issueId: string, @@ -251,9 +225,15 @@ // 9. Output result if (options.json !== undefined) { - const issueData = { + const [number, state] = await Promise.all([ + resolveSequentialNumber(displayId, updatedIssue.uri, client, repoAtUri), + getIssueState({ client, issueUri: updatedIssue.uri }), + ]); + const issueData: IssueData = { + number, title: updatedIssue.title, body: updatedIssue.body, + state, author: updatedIssue.author, createdAt: updatedIssue.createdAt, uri: updatedIssue.uri, @@ -283,13 +263,10 @@ * Issue close subcommand */ function createCloseCommand(): Command { - return new Command('close') + return new IssueCommand('close') .description('Close an issue') .argument('', 'Issue number or rkey') - .option( - '--json [fields]', - 'Output JSON; optionally specify comma-separated fields (number, title, uri, state, cid)' - ) + .addIssueJsonOption() .action(async (issueId: string, options: { json?: string | true }) => { try { // 1. Validate auth @@ -314,22 +291,24 @@ // 4. Resolve issue ID to URI const { uri: issueUri, displayId } = await resolveIssueUri(issueId, client, repoAtUri); - // 5. Fetch issue details and sequential number - const issue = await getIssue({ client, issueUri }); - const number = await resolveSequentialNumber(displayId, issueUri, client, repoAtUri); + // 5. Fetch complete issue data (state will be 'closed' after operation) + const issueData = await getCompleteIssueData( + client, + issueUri, + displayId, + repoAtUri, + 'closed' + ); // 6. Close issue await closeIssue({ client, issueUri }); // 7. Display success if (options.json !== undefined) { - outputJson( - { number, title: issue.title, uri: issueUri, state: 'closed', cid: issue.cid }, - typeof options.json === 'string' ? options.json : undefined - ); + outputJson(issueData, typeof options.json === 'string' ? options.json : undefined); } else { console.log(`✓ Issue ${displayId} closed`); - console.log(` Title: ${issue.title}`); + console.log(` Title: ${issueData.title}`); } } catch (error) { console.error( @@ -344,13 +323,10 @@ * Issue reopen subcommand */ function createReopenCommand(): Command { - return new Command('reopen') + return new IssueCommand('reopen') .description('Reopen a closed issue') .argument('', 'Issue number or rkey') - .option( - '--json [fields]', - 'Output JSON; optionally specify comma-separated fields (number, title, uri, state, cid)' - ) + .addIssueJsonOption() .action(async (issueId: string, options: { json?: string | true }) => { try { // 1. Validate auth @@ -375,22 +351,24 @@ // 4. Resolve issue ID to URI const { uri: issueUri, displayId } = await resolveIssueUri(issueId, client, repoAtUri); - // 5. Fetch issue details and sequential number - const issue = await getIssue({ client, issueUri }); - const number = await resolveSequentialNumber(displayId, issueUri, client, repoAtUri); + // 5. Fetch complete issue data (state will be 'open' after operation) + const issueData = await getCompleteIssueData( + client, + issueUri, + displayId, + repoAtUri, + 'open' + ); // 6. Reopen issue await reopenIssue({ client, issueUri }); // 7. Display success if (options.json !== undefined) { - outputJson( - { number, title: issue.title, uri: issueUri, state: 'open', cid: issue.cid }, - typeof options.json === 'string' ? options.json : undefined - ); + outputJson(issueData, typeof options.json === 'string' ? options.json : undefined); } else { console.log(`✓ Issue ${displayId} reopened`); - console.log(` Title: ${issue.title}`); + console.log(` Title: ${issueData.title}`); } } catch (error) { console.error( @@ -405,14 +383,11 @@ * Issue delete subcommand */ function createDeleteCommand(): Command { - return new Command('delete') + return new IssueCommand('delete') .description('Delete an issue permanently') .argument('', 'Issue number or rkey') .option('-f, --force', 'Skip confirmation prompt') - .option( - '--json [fields]', - 'Output JSON; optionally specify comma-separated fields (number, title, uri, cid)' - ) + .addIssueJsonOption() .action(async (issueId: string, options: { force?: boolean; json?: string | true }) => { // 1. Validate auth const client = createApiClient(); @@ -433,16 +408,11 @@ // 3. Build repo AT-URI, resolve issue ID, and fetch issue details let issueUri: string; let displayId: string; - let issueTitle: string; - let issueCid: string; - let issueNumber: number | undefined; + let issueData: IssueData; try { const repoAtUri = await buildRepoAtUri(context.owner, context.name, client); ({ uri: issueUri, displayId } = await resolveIssueUri(issueId, client, repoAtUri)); - const issue = await getIssue({ client, issueUri }); - issueTitle = issue.title; - issueCid = issue.cid; - issueNumber = await resolveSequentialNumber(displayId, issueUri, client, repoAtUri); + issueData = await getCompleteIssueData(client, issueUri, displayId, repoAtUri); } catch (error) { console.error( `✗ Failed to delete issue: ${error instanceof Error ? error.message : 'Unknown error'}` @@ -453,7 +423,7 @@ // 4. Confirm deletion if not --force (outside try so process.exit(0) propagates cleanly) if (!options.force) { const confirmed = await confirm({ - message: `Are you sure you want to delete issue ${displayId} "${issueTitle}"? This cannot be undone.`, + message: `Are you sure you want to delete issue ${displayId} "${issueData.title}"? This cannot be undone.`, default: false, }); @@ -467,13 +437,10 @@ try { await deleteIssue({ client, issueUri }); if (options.json !== undefined) { - outputJson( - { number: issueNumber, title: issueTitle, uri: issueUri, cid: issueCid }, - typeof options.json === 'string' ? options.json : undefined - ); + outputJson(issueData, typeof options.json === 'string' ? options.json : undefined); } else { console.log(`✓ Issue ${displayId} deleted`); - console.log(` Title: ${issueTitle}`); + console.log(` Title: ${issueData.title}`); } } catch (error) { console.error( @@ -506,15 +473,12 @@ * Issue create subcommand */ function createCreateCommand(): Command { - return new Command('create') + return new IssueCommand('create') .description('Create a new issue') .argument('', 'Issue title') .option('-b, --body <string>', 'Issue body text') .option('-F, --body-file <path>', 'Read body from file (- for stdin)') - .option( - '--json [fields]', - 'Output JSON; optionally specify comma-separated fields (number, title, body, author, createdAt, uri, cid)' - ) + .addIssueJsonOption() .action( async ( title: string, @@ -570,10 +534,11 @@ // 8. Output result if (options.json !== undefined) { - const issueData = { + const issueData: IssueData = { number, title: issue.title, body: issue.body, + state: 'open', author: issue.author, createdAt: issue.createdAt, uri: issue.uri, @@ -601,13 +566,10 @@ * Issue list subcommand */ function createListCommand(): Command { - return new Command('list') + return new IssueCommand('list') .description('List issues for the current repository') .option('-l, --limit <number>', 'Maximum number of issues to fetch', '50') - .option( - '--json [fields]', - 'Output JSON; optionally specify comma-separated fields (number, title, body, state, author, createdAt, uri, cid)' - ) + .addIssueJsonOption() .action(async (options: { limit: string; json?: string | true }) => { try { // 1. Validate auth diff --git a/tests/commands/issue.test.ts b/tests/commands/issue.test.ts --- a/tests/commands/issue.test.ts +++ b/tests/commands/issue.test.ts @@ -615,20 +615,26 @@ issues: [mockIssue], cursor: undefined, }); - vi.mocked(issuesApi.getIssue).mockResolvedValue(mockIssue); - vi.mocked(issuesApi.getIssueState).mockResolvedValue('open'); + vi.mocked(issuesApi.getCompleteIssueData).mockResolvedValue({ + number: 1, + title: mockIssue.title, + body: mockIssue.body, + state: 'open', + author: mockIssue.author, + createdAt: mockIssue.createdAt, + uri: mockIssue.uri, + cid: mockIssue.cid, + }); const command = createIssueCommand(); await command.parseAsync(['node', 'test', 'view', '1']); - expect(issuesApi.getIssue).toHaveBeenCalledWith({ - client: mockClient, - issueUri: mockIssue.uri, - }); - expect(issuesApi.getIssueState).toHaveBeenCalledWith({ - client: mockClient, - issueUri: mockIssue.uri, - }); + expect(issuesApi.getCompleteIssueData).toHaveBeenCalledWith( + mockClient, + mockIssue.uri, + '#1', + 'at://did:plc:abc123/sh.tangled.repo/xyz789' + ); expect(consoleLogSpy).toHaveBeenCalledWith('\nIssue #1 [OPEN]'); expect(consoleLogSpy).toHaveBeenCalledWith('Title: Test Issue'); expect(consoleLogSpy).toHaveBeenCalledWith('\nBody:'); @@ -636,27 +642,44 @@ }); it('should view issue by rkey', async () => { - vi.mocked(issuesApi.getIssue).mockResolvedValue(mockIssue); - vi.mocked(issuesApi.getIssueState).mockResolvedValue('closed'); + vi.mocked(issuesApi.getCompleteIssueData).mockResolvedValue({ + number: undefined, + title: mockIssue.title, + body: mockIssue.body, + state: 'closed', + author: mockIssue.author, + createdAt: mockIssue.createdAt, + uri: mockIssue.uri, + cid: mockIssue.cid, + }); const command = createIssueCommand(); await command.parseAsync(['node', 'test', 'view', 'issue1']); - expect(issuesApi.getIssue).toHaveBeenCalledWith({ - client: mockClient, - issueUri: 'at://did:plc:abc123/sh.tangled.repo.issue/issue1', - }); + expect(issuesApi.getCompleteIssueData).toHaveBeenCalledWith( + mockClient, + 'at://did:plc:abc123/sh.tangled.repo.issue/issue1', + 'issue1', + 'at://did:plc:abc123/sh.tangled.repo/xyz789' + ); expect(consoleLogSpy).toHaveBeenCalledWith('\nIssue issue1 [CLOSED]'); }); it('should show issue without body', async () => { - const issueWithoutBody = { ...mockIssue, body: undefined }; vi.mocked(issuesApi.listIssues).mockResolvedValue({ - issues: [issueWithoutBody], + issues: [mockIssue], cursor: undefined, }); - vi.mocked(issuesApi.getIssue).mockResolvedValue(issueWithoutBody); - vi.mocked(issuesApi.getIssueState).mockResolvedValue('open'); + vi.mocked(issuesApi.getCompleteIssueData).mockResolvedValue({ + number: 1, + title: mockIssue.title, + body: undefined, + state: 'open', + author: mockIssue.author, + createdAt: mockIssue.createdAt, + uri: mockIssue.uri, + cid: mockIssue.cid, + }); const command = createIssueCommand(); await command.parseAsync(['node', 'test', 'view', '1']); @@ -709,14 +732,23 @@ issues: [mockIssue], cursor: undefined, }); - vi.mocked(issuesApi.getIssue).mockResolvedValue(mockIssue); - vi.mocked(issuesApi.getIssueState).mockResolvedValue('open'); + vi.mocked(issuesApi.getCompleteIssueData).mockResolvedValue({ + number: 1, + title: mockIssue.title, + body: mockIssue.body, + state: 'open', + author: mockIssue.author, + createdAt: mockIssue.createdAt, + uri: mockIssue.uri, + cid: mockIssue.cid, + }); const command = createIssueCommand(); await command.parseAsync(['node', 'test', 'view', '1', '--json']); const jsonOutput = JSON.parse(consoleLogSpy.mock.calls[0][0] as string); expect(jsonOutput).toMatchObject({ + number: 1, title: 'Test Issue', body: 'Issue body', state: 'open', @@ -731,8 +763,16 @@ issues: [mockIssue], cursor: undefined, }); - vi.mocked(issuesApi.getIssue).mockResolvedValue(mockIssue); - vi.mocked(issuesApi.getIssueState).mockResolvedValue('closed'); + vi.mocked(issuesApi.getCompleteIssueData).mockResolvedValue({ + number: 1, + title: mockIssue.title, + body: mockIssue.body, + state: 'closed', + author: mockIssue.author, + createdAt: mockIssue.createdAt, + uri: mockIssue.uri, + cid: mockIssue.cid, + }); const command = createIssueCommand(); await command.parseAsync(['node', 'test', 'view', '1', '--json', 'title,state']); @@ -866,13 +906,17 @@ cursor: undefined, }); vi.mocked(issuesApi.updateIssue).mockResolvedValue(updatedIssue); + vi.mocked(issuesApi.resolveSequentialNumber).mockResolvedValue(1); + vi.mocked(issuesApi.getIssueState).mockResolvedValue('open'); const command = createIssueCommand(); await command.parseAsync(['node', 'test', 'edit', '1', '--title', 'New Title', '--json']); const jsonOutput = JSON.parse(consoleLogSpy.mock.calls[0][0] as string); expect(jsonOutput).toMatchObject({ + number: 1, title: 'New Title', + state: 'open', author: 'did:plc:abc123', uri: mockIssue.uri, cid: mockIssue.cid, @@ -888,6 +932,8 @@ cursor: undefined, }); vi.mocked(issuesApi.updateIssue).mockResolvedValue(updatedIssue); + vi.mocked(issuesApi.resolveSequentialNumber).mockResolvedValue(1); + vi.mocked(issuesApi.getIssueState).mockResolvedValue('open'); const command = createIssueCommand(); await command.parseAsync([ @@ -947,7 +993,16 @@ }); vi.mocked(atUri.buildRepoAtUri).mockResolvedValue('at://did:plc:abc123/sh.tangled.repo/xyz789'); - vi.mocked(issuesApi.getIssue).mockResolvedValue(mockIssue); + vi.mocked(issuesApi.getCompleteIssueData).mockResolvedValue({ + number: 1, + title: mockIssue.title, + body: undefined, + state: 'closed', + author: mockIssue.author, + createdAt: mockIssue.createdAt, + uri: mockIssue.uri, + cid: mockIssue.cid, + }); }); afterEach(() => { @@ -993,8 +1048,10 @@ expect(jsonOutput).toEqual({ number: 1, title: 'Test Issue', - uri: mockIssue.uri, state: 'closed', + author: mockIssue.author, + createdAt: mockIssue.createdAt, + uri: mockIssue.uri, cid: mockIssue.cid, }); }); @@ -1050,7 +1107,16 @@ }); vi.mocked(atUri.buildRepoAtUri).mockResolvedValue('at://did:plc:abc123/sh.tangled.repo/xyz789'); - vi.mocked(issuesApi.getIssue).mockResolvedValue(mockIssue); + vi.mocked(issuesApi.getCompleteIssueData).mockResolvedValue({ + number: 1, + title: mockIssue.title, + body: undefined, + state: 'open', + author: mockIssue.author, + createdAt: mockIssue.createdAt, + uri: mockIssue.uri, + cid: mockIssue.cid, + }); }); afterEach(() => { @@ -1096,8 +1162,10 @@ expect(jsonOutput).toEqual({ number: 1, title: 'Test Issue', - uri: mockIssue.uri, state: 'open', + author: mockIssue.author, + createdAt: mockIssue.createdAt, + uri: mockIssue.uri, cid: mockIssue.cid, }); }); @@ -1154,7 +1222,16 @@ }); vi.mocked(atUri.buildRepoAtUri).mockResolvedValue('at://did:plc:abc123/sh.tangled.repo/xyz789'); - vi.mocked(issuesApi.getIssue).mockResolvedValue(mockIssue); + vi.mocked(issuesApi.getCompleteIssueData).mockResolvedValue({ + number: 1, + title: mockIssue.title, + body: undefined, + state: 'open', + author: mockIssue.author, + createdAt: mockIssue.createdAt, + uri: mockIssue.uri, + cid: mockIssue.cid, + }); }); afterEach(() => { @@ -1241,10 +1318,12 @@ expect(jsonOutput).toEqual({ number: 1, title: 'Test Issue', + state: 'open', + author: mockIssue.author, + createdAt: mockIssue.createdAt, uri: mockIssue.uri, cid: mockIssue.cid, }); - expect(jsonOutput).not.toHaveProperty('state'); }); it('should output filtered JSON when --json with fields is passed', async () => { -- tangled.sh