From 475b428180a7beea14107ec0efd71b29bd227285 Mon Sep 17 00:00:00 2001 From: Rory O'Keeffe Date: Fri, 23 Oct 2026 17:02:43 +0100 Subject: [PATCH 1/2] fix(thread): show the thread when the channel lookup is rate limited Channel lookups are the only call in `thread view` with a per-user rate limit, so a bulk read of threads hits it first. The thread and comments have already loaded by then, and the channel name is only a label. On a 429, thread view now shows the channel id instead of failing, and --json / --ndjson output sets channelName to null. Any other error still fails as before. The 429 hint now says to wait a minute or two, and that retrying sooner keeps the limit in place. The old "Wait a moment" read as "retry now". Co-Authored-By: Claude Opus 5.5 --- src/commands/thread/thread.test.ts | 70 ++++++++++++++++++++++++++---- src/commands/thread/view.ts | 44 +++++++++++++------ src/lib/api.test.ts | 5 ++- src/lib/api.ts | 6 ++- 4 files changed, 102 insertions(+), 23 deletions(-) diff --git a/src/commands/thread/thread.test.ts b/src/commands/thread/thread.test.ts index 8de0b3f..30351d3 100644 --- a/src/commands/thread/thread.test.ts +++ b/src/commands/thread/thread.test.ts @@ -47,6 +47,7 @@ vi.mock('../../lib/input.js', () => ({ vi.mock('chalk') import { clearWorkspaceUserCache } from '../../lib/api.js' +import { CliError } from '../../lib/errors.js' import { openEditor, readStdin, readStdinToEnd } from '../../lib/input.js' import { registerThreadCommand } from './index.js' @@ -186,6 +187,14 @@ function createClient({ const createProgram = () => createTestProgram(registerThreadCommand) +async function viewOutput(client: ReturnType, args: string[]) { + apiMocks.getCommsClient.mockResolvedValue(client) + const program = createProgram() + const consoleSpy = captureConsole('log') + await program.parseAsync(['node', 'tdc', 'thread', 'view', '500', ...args]) + return consoleSpy.mock.calls.map((c) => c[0]).join('\n') +} + // Shared setup for the --file suites: a fresh mock client wired into getCommsClient // plus a program. Tests asserting on output call captureConsole('log') themselves. function setupFileTest() { @@ -736,14 +745,6 @@ describe('thread view comment count header', () => { const users = { 1: { id: 1, fullName: 'Alice' }, 2: { id: 2, fullName: 'Bob' } } - async function viewOutput(client: ReturnType, args: string[]) { - apiMocks.getCommsClient.mockResolvedValue(client) - const program = createProgram() - const consoleSpy = captureConsole('log') - await program.parseAsync(['node', 'tdc', 'thread', 'view', '500', ...args]) - return consoleSpy.mock.calls.map((c) => c[0]).join('\n') - } - it('shows the plain count when every comment came back', async () => { const client = createClient({ comments: [createComment(1, 1), createComment(2, 2), createComment(3, 3)], @@ -850,6 +851,59 @@ describe('thread view error propagation', () => { }) }) +describe('thread view when the channel lookup fails', () => { + beforeEach(() => { + clearWorkspaceUserCache() + vi.clearAllMocks() + configMocks.getConfig.mockResolvedValue({}) + groupsMock.getWorkspaceGroups.mockResolvedValue([]) + groupsMock.getWorkspaceUsers.mockResolvedValue([]) + }) + + const rateLimited = new CliError( + 'RATE_LIMITED', + 'Comms is rate limiting requests: 429 Too Many Requests.', + ) + + function clientWithChannelError(error: CliError) { + const client = createClient({ comments: [createComment(1, 1)] }) + client.channels.getChannel.mockRejectedValue(error) + return client + } + + it('shows the thread with the channel id when rate limited', async () => { + const output = await viewOutput(clientWithChannelError(rateLimited), []) + + expect(output).toContain('Test Thread') + expect(output).toContain('[channel:CH100]') + expect(output).toContain('Comment 1') + }) + + it('sets channelName to null in --json output when rate limited', async () => { + const output = JSON.parse(await viewOutput(clientWithChannelError(rateLimited), ['--json'])) + + expect(output.thread).toMatchObject({ channelId: 'CH100', channelName: null }) + expect(output.comments).toHaveLength(1) + }) + + it('sets channelName to null for a single comment when rate limited', async () => { + const output = JSON.parse( + await viewOutput(clientWithChannelError(rateLimited), ['--comment', '1', '--json']), + ) + + expect(output).toMatchObject({ id: '1', channelName: null }) + }) + + it('still fails on any other error', async () => { + const notFound = new CliError('NOT_FOUND', 'Comms could not find that resource: 404.') + + await expect(viewOutput(clientWithChannelError(notFound), [])).rejects.toHaveProperty( + 'code', + 'NOT_FOUND', + ) + }) +}) + describe('thread create', () => { beforeEach(() => { clearWorkspaceUserCache() diff --git a/src/commands/thread/view.ts b/src/commands/thread/view.ts index f677198..e04ef92 100644 --- a/src/commands/thread/view.ts +++ b/src/commands/thread/view.ts @@ -2,6 +2,7 @@ import type { CommsApi } from '@doist/comms-sdk' import chalk from 'chalk' import { buildUserNameMap, getCommsClient } from '../../lib/api.js' import { formatRelativeDate } from '../../lib/dates.js' +import { isCliErrorCode } from '../../lib/errors.js' import { renderMarkdown } from '../../lib/markdown.js' import { toDate, type PaginatedViewOptions } from '../../lib/options.js' import { colors, formatJson, pluralize } from '../../lib/output.js' @@ -33,6 +34,27 @@ function formatCommentsHeader( return `--- ${label} ---` } +// Comms rate limits channel lookups per user, so a bulk read of threads hits that limit first. +// The name is only a label, so a rate-limited lookup returns null instead of failing the view. +async function getChannelName(client: CommsApi, channelId: string): Promise { + try { + const channel = await client.channels.getChannel(channelId) + return channel.name + } catch (error) { + if (isCliErrorCode(error, 'RATE_LIMITED')) return null + throw error + } +} + +function printThreadHeader( + thread: { title: string; channelId: string }, + channelName: string | null, +) { + console.log(chalk.bold(thread.title)) + console.log(colors.channel(`[${channelName ?? `channel:${thread.channelId}`}]`)) + console.log('') +} + async function viewSingleComment( client: CommsApi, threadId: string, @@ -44,8 +66,8 @@ async function viewSingleComment( client.comments.getComment(commentId), ]) - const [channel, userMap] = await Promise.all([ - client.channels.getChannel(thread.channelId), + const [channelName, userMap] = await Promise.all([ + getChannelName(client, thread.channelId), buildUserNameMap(thread.workspaceId, client), ]) @@ -53,7 +75,7 @@ async function viewSingleComment( const output = { ...comment, creatorName: userMap.get(comment.creator), - channelName: channel.name, + channelName, threadTitle: thread.title, } console.log(formatJson(output, undefined, options.full)) @@ -71,9 +93,7 @@ async function viewSingleComment( return } - console.log(chalk.bold(thread.title)) - console.log(colors.channel(`[${channel.name}]`)) - console.log('') + printThreadHeader(thread, channelName) await printComment(comment, userMap, options.raw ?? false) } @@ -133,8 +153,8 @@ export async function viewThread(ref: string, options: ViewOptions): Promise ({ @@ -158,7 +178,7 @@ export async function viewThread(ref: string, options: ViewOptions): Promise { await expect(client.channels.deleteChannel('CH429')).rejects.toMatchObject({ code: 'RATE_LIMITED', message: 'Comms is rate limiting requests: 429 Too Many Requests.', - hints: ['Wait a moment, then retry'], + hints: [ + 'Wait a minute or two before retrying', + 'Retrying sooner keeps the limit in place, because rejected calls count against it too', + ], }) }) diff --git a/src/lib/api.ts b/src/lib/api.ts index cdfd1f5..c77b7df 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -263,10 +263,14 @@ function wrapResult( ) } if (isRateLimited(error)) { + // Agents follow this hint literally, so it says how long to wait. throw new CliError( 'RATE_LIMITED', 'Comms is rate limiting requests: 429 Too Many Requests.', - ['Wait a moment, then retry'], + [ + 'Wait a minute or two before retrying', + 'Retrying sooner keeps the limit in place, because rejected calls count against it too', + ], ) } if (isServerError(error)) { From b3d3e097d2201413e5b11105d7a30476494eb799 Mon Sep 17 00:00:00 2001 From: Rory O'Keeffe Date: Fri, 23 Oct 2026 17:13:43 +0100 Subject: [PATCH 2/2] fix(thread): match the inbox's ch: fallback and test ndjson The inbox already shows `ch:` for a channel it can't name, so thread view now uses the same prefix instead of adding `channel:`. Also covers the --ndjson shape when the channel lookup is rate limited. Co-Authored-By: Claude Opus 5.5 --- src/commands/thread/thread.test.ts | 10 +++++++++- src/commands/thread/view.ts | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/commands/thread/thread.test.ts b/src/commands/thread/thread.test.ts index 30351d3..3c071ca 100644 --- a/src/commands/thread/thread.test.ts +++ b/src/commands/thread/thread.test.ts @@ -875,7 +875,7 @@ describe('thread view when the channel lookup fails', () => { const output = await viewOutput(clientWithChannelError(rateLimited), []) expect(output).toContain('Test Thread') - expect(output).toContain('[channel:CH100]') + expect(output).toContain('[ch:CH100]') expect(output).toContain('Comment 1') }) @@ -886,6 +886,14 @@ describe('thread view when the channel lookup fails', () => { expect(output.comments).toHaveLength(1) }) + it('sets channelName to null in --ndjson output when rate limited', async () => { + const output = await viewOutput(clientWithChannelError(rateLimited), ['--ndjson']) + const [threadLine, commentLine] = output.split('\n').map((line) => JSON.parse(line)) + + expect(threadLine).toMatchObject({ type: 'thread', channelId: 'CH100', channelName: null }) + expect(commentLine).toMatchObject({ type: 'comment', id: '1' }) + }) + it('sets channelName to null for a single comment when rate limited', async () => { const output = JSON.parse( await viewOutput(clientWithChannelError(rateLimited), ['--comment', '1', '--json']), diff --git a/src/commands/thread/view.ts b/src/commands/thread/view.ts index e04ef92..a83208b 100644 --- a/src/commands/thread/view.ts +++ b/src/commands/thread/view.ts @@ -51,7 +51,7 @@ function printThreadHeader( channelName: string | null, ) { console.log(chalk.bold(thread.title)) - console.log(colors.channel(`[${channelName ?? `channel:${thread.channelId}`}]`)) + console.log(colors.channel(`[${channelName ?? `ch:${thread.channelId}`}]`)) console.log('') }