From f792c7337191a4c2345768ba0326f3782dc35569 Mon Sep 17 00:00:00 2001 From: Rory O'Keeffe Date: Fri, 23 Oct 2026 17:02:38 +0100 Subject: [PATCH 1/2] perf(inbox): look up channel names with one call tdc inbox asked the server for each inbox channel one at a time, just to show its name. With 24 channels in the inbox, a script running the command every few seconds used up the per-user rate limit for channels/getone, and the user's own apps started getting 429s too. The public-channel check already fetches the workspace's channel list, which has the names. I now keep that list instead of only the public ids, and read the names from it. A channel missing from the list (one the user has left) still gets looked up by itself. Co-Authored-By: Claude Opus 5.5 --- src/commands/inbox.test.ts | 87 +++++++++++++++++++++++++++++---- src/commands/inbox.ts | 16 ++++-- src/lib/public-channels.test.ts | 10 ++-- src/lib/public-channels.ts | 24 ++++----- 4 files changed, 105 insertions(+), 32 deletions(-) diff --git a/src/commands/inbox.test.ts b/src/commands/inbox.test.ts index c8b811f..fa82bdf 100644 --- a/src/commands/inbox.test.ts +++ b/src/commands/inbox.test.ts @@ -24,19 +24,22 @@ vi.mock('../lib/global-args.js', async (importOriginal) => ({ includePrivateChannels: vi.fn().mockReturnValue(true), })) -vi.mock('../lib/public-channels.js', () => ({ - getPublicChannelIds: vi.fn(), -})) - vi.mock('chalk') +import { includePrivateChannels } from '../lib/global-args.js' +import { clearChannelCache } from '../lib/public-channels.js' import { registerInboxCommand } from './inbox.js' const createProgram = () => createTestProgram(registerInboxCommand) +beforeEach(() => { + clearChannelCache() +}) + function mockClient(overrides: { inboxThreads?: unknown[] unreadData?: unknown[] + channels?: unknown[] getChannel?: ReturnType getInbox?: ReturnType getUnread?: ReturnType @@ -45,13 +48,14 @@ function mockClient(overrides: { const getUnread = overrides.getUnread ?? vi.fn().mockResolvedValue({ data: overrides.unreadData ?? [], version: 1 }) + const getChannels = vi.fn().mockResolvedValue(overrides.channels ?? []) const getChannel = overrides.getChannel ?? vi.fn() apiMocks.getCommsClient.mockResolvedValue({ inbox: { getInbox }, threads: { getUnread }, - channels: { getChannel }, + channels: { getChannels, getChannel }, }) - return { getInbox, getUnread, getChannel } + return { getInbox, getUnread, getChannels, getChannel } } describe('inbox --workspace conflict', () => { @@ -134,7 +138,7 @@ describe('inbox --ids-only', () => { it('outputs thread IDs without fetching channel names when no channel filter is set', async () => { vi.clearAllMocks() apiMocks.getCurrentWorkspaceId.mockResolvedValue(1) - const { getChannel } = mockClient({ + const { getChannels, getChannel } = mockClient({ inboxThreads: [ { id: 'thread-1', @@ -157,6 +161,7 @@ describe('inbox --ids-only', () => { await createProgram().parseAsync(['node', 'tdc', 'inbox', '--ids-only']) expect(consoleSpy).toHaveBeenCalledWith('thread-1\nthread-2') + expect(getChannels).not.toHaveBeenCalled() expect(getChannel).not.toHaveBeenCalled() }) }) @@ -176,7 +181,7 @@ describe('inbox empty output (channel filter)', () => { } mockClient({ inboxThreads: [thread], - getChannel: vi.fn().mockResolvedValue({ id: 'CH10', name: 'engineering' }), + channels: [{ id: 'CH10', name: 'engineering', public: true }], }) logSpy = captureConsole('log') }) @@ -190,6 +195,70 @@ describe('inbox empty output (channel filter)', () => { }) }) +describe('inbox channel names', () => { + const threadIn = (channelId: string) => ({ + id: `thread-${channelId}`, + channelId, + title: channelId, + posted: '2026-05-01T00:00:00Z', + url: `https://example.test/thread-${channelId}`, + }) + let logSpy: ReturnType + + beforeEach(() => { + vi.clearAllMocks() + apiMocks.getCurrentWorkspaceId.mockResolvedValue(1) + logSpy = captureConsole('log') + }) + + function channelNamesByThread(): Record { + const output = JSON.parse(logSpy.mock.calls[0]?.[0] as string) as Array<{ + id: string + channelName: unknown + }> + return Object.fromEntries(output.map((t) => [t.id, t.channelName])) + } + + it('reuses the public-channel list for names without per-channel lookups', async () => { + vi.mocked(includePrivateChannels).mockReturnValueOnce(false) + const { getChannels, getChannel } = mockClient({ + inboxThreads: [threadIn('CH1'), threadIn('CH2'), threadIn('CH3')], + channels: [ + { id: 'CH1', name: 'engineering', public: true }, + { id: 'CH2', name: 'design', public: true }, + { id: 'CH3', name: 'secret', public: false }, + ], + }) + + await createProgram().parseAsync(['node', 'tdc', 'inbox', '--json', '--full']) + + expect(channelNamesByThread()).toEqual({ + 'thread-CH1': 'engineering', + 'thread-CH2': 'design', + }) + expect(getChannels).toHaveBeenCalledTimes(1) + expect(getChannel).not.toHaveBeenCalled() + }) + + it('looks up only the channels missing from the list', async () => { + const { getChannels, getChannel } = mockClient({ + inboxThreads: [threadIn('CH1'), threadIn('CH-left')], + channels: [{ id: 'CH1', name: 'engineering', public: true }], + getChannel: vi.fn().mockResolvedValue({ id: 'CH-left', name: 'old-team' }), + }) + + await createProgram().parseAsync(['node', 'tdc', 'inbox', '--json', '--full']) + + expect(channelNamesByThread()).toEqual({ + 'thread-CH1': 'engineering', + 'thread-CH-left': 'old-team', + }) + expect(getChannels).toHaveBeenCalledTimes(1) + expect(getChannel).toHaveBeenCalledTimes(1) + expect(getChannel).toHaveBeenCalledWith('CH-left') + }) +}) + describe('inbox API errors', () => { it('propagates SDK rejections through the Promise.all that replaced batch', async () => { vi.clearAllMocks() @@ -249,7 +318,7 @@ describe('inbox unread mentions', () => { mockClient({ inboxThreads: threads, unreadData, - getChannel: vi.fn().mockResolvedValue({ id: 'CH1', name: 'engineering' }), + channels: [{ id: 'CH1', name: 'engineering', public: true }], }) logSpy = captureConsole('log') }) diff --git a/src/commands/inbox.ts b/src/commands/inbox.ts index f738270..30f0a12 100644 --- a/src/commands/inbox.ts +++ b/src/commands/inbox.ts @@ -9,7 +9,7 @@ import { CliError } from '../lib/errors.js' import { includePrivateChannels, isAccessible } from '../lib/global-args.js' import { toDate, type PaginatedViewOptions } from '../lib/options.js' import { colors, formatJson, formatNdjson } from '../lib/output.js' -import { getPublicChannelIds } from '../lib/public-channels.js' +import { getPublicChannelIds, getWorkspaceChannels } from '../lib/public-channels.js' import { resolveWorkspaceRef } from '../lib/refs.js' import { fetchUnreadThreads, unreadFlags } from '../lib/threads.js' @@ -80,11 +80,17 @@ async function showInbox(workspaceRef: string | undefined, options: InboxOptions let channelMap = new Map() if (outputMode !== 'ids-only' || options.channel) { - const channelIds = [...new Set(inboxThreads.map((t) => t.channelId))] - const channelEntries = await Promise.all( - channelIds.map(async (id) => [id, await client.channels.getChannel(id)] as const), + const channels = await getWorkspaceChannels(workspaceId) + channelMap = new Map(channels.map((ch) => [ch.id, ch.name])) + // The list only has channels the user is a member of, but an inbox thread can sit + // in one they've left, so look those up one by one. + const unlistedIds = [...new Set(inboxThreads.map((t) => t.channelId))].filter( + (id) => !channelMap.has(id), ) - channelMap = new Map(channelEntries.map(([id, channel]) => [id, channel.name])) + const unlisted = await Promise.all( + unlistedIds.map(async (id) => [id, await client.channels.getChannel(id)] as const), + ) + for (const [id, channel] of unlisted) channelMap.set(id, channel.name) } if (options.channel) { diff --git a/src/lib/public-channels.test.ts b/src/lib/public-channels.test.ts index 3222334..f4ff464 100644 --- a/src/lib/public-channels.test.ts +++ b/src/lib/public-channels.test.ts @@ -6,11 +6,7 @@ vi.mock('./api.js', () => ({ import { getCommsClient } from './api.js' import { includePrivateChannels, resetGlobalArgs } from './global-args.js' -import { - assertChannelIsPublic, - clearPublicChannelCache, - getPublicChannelIds, -} from './public-channels.js' +import { assertChannelIsPublic, clearChannelCache, getPublicChannelIds } from './public-channels.js' const mockGetCommsClient = vi.mocked(getCommsClient) @@ -78,7 +74,7 @@ describe('includePrivateChannels', () => { describe('getPublicChannelIds', () => { beforeEach(() => { - clearPublicChannelCache() + clearChannelCache() }) it('returns only public channel IDs', async () => { @@ -128,7 +124,7 @@ describe('assertChannelIsPublic', () => { const originalEnv = process.env.COMMS_INCLUDE_PRIVATE_CHANNELS beforeEach(() => { - clearPublicChannelCache() + clearChannelCache() resetGlobalArgs() process.argv = ['node', 'tdc'] delete process.env.COMMS_INCLUDE_PRIVATE_CHANNELS diff --git a/src/lib/public-channels.ts b/src/lib/public-channels.ts index f842f59..6618741 100644 --- a/src/lib/public-channels.ts +++ b/src/lib/public-channels.ts @@ -1,25 +1,27 @@ +import type { Channel } from '@doist/comms-sdk' import { getCommsClient } from './api.js' import { CliError } from './errors.js' import { includePrivateChannels } from './global-args.js' -const publicChannelCache = new Map>() +const channelCache = new Map() -export async function getPublicChannelIds(workspaceId: number): Promise> { - const cached = publicChannelCache.get(workspaceId) +export async function getWorkspaceChannels(workspaceId: number): Promise { + const cached = channelCache.get(workspaceId) if (cached) return cached const client = await getCommsClient() const channels = await client.channels.getChannels({ workspaceId }) - const publicIds = new Set() - for (const ch of channels) { - if (ch.public) publicIds.add(ch.id) - } - publicChannelCache.set(workspaceId, publicIds) - return publicIds + channelCache.set(workspaceId, channels) + return channels +} + +export async function getPublicChannelIds(workspaceId: number): Promise> { + const channels = await getWorkspaceChannels(workspaceId) + return new Set(channels.filter((ch) => ch.public).map((ch) => ch.id)) } -export function clearPublicChannelCache(): void { - publicChannelCache.clear() +export function clearChannelCache(): void { + channelCache.clear() } export async function assertChannelIsPublic(channelId: string, workspaceId: number): Promise { From e35fc8ed1c2521e637473f8bd2beb830fbe5436a Mon Sep 17 00:00:00 2001 From: Rory O'Keeffe Date: Fri, 23 Oct 2026 17:26:38 +0100 Subject: [PATCH 2/2] refactor(channels): keep public ids cached with the channel list Bulk thread read/unread check the private-channel rule once per thread. Caching the public ids next to the list means each check is a lookup again, instead of rebuilding the set every time. Co-Authored-By: Claude Opus 5.5 --- src/lib/public-channels.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/lib/public-channels.ts b/src/lib/public-channels.ts index 6618741..7571dc0 100644 --- a/src/lib/public-channels.ts +++ b/src/lib/public-channels.ts @@ -3,21 +3,28 @@ import { getCommsClient } from './api.js' import { CliError } from './errors.js' import { includePrivateChannels } from './global-args.js' -const channelCache = new Map() +type WorkspaceChannels = { channels: Channel[]; publicIds: Set } -export async function getWorkspaceChannels(workspaceId: number): Promise { +const channelCache = new Map() + +async function loadWorkspaceChannels(workspaceId: number): Promise { const cached = channelCache.get(workspaceId) if (cached) return cached const client = await getCommsClient() const channels = await client.channels.getChannels({ workspaceId }) - channelCache.set(workspaceId, channels) - return channels + const publicIds = new Set(channels.filter((ch) => ch.public).map((ch) => ch.id)) + const loaded = { channels, publicIds } + channelCache.set(workspaceId, loaded) + return loaded +} + +export async function getWorkspaceChannels(workspaceId: number): Promise { + return (await loadWorkspaceChannels(workspaceId)).channels } export async function getPublicChannelIds(workspaceId: number): Promise> { - const channels = await getWorkspaceChannels(workspaceId) - return new Set(channels.filter((ch) => ch.public).map((ch) => ch.id)) + return (await loadWorkspaceChannels(workspaceId)).publicIds } export function clearChannelCache(): void {