From bb3657ccecb96bd9900e12f4f095a10612ecbee4 Mon Sep 17 00:00:00 2001 From: lmjabreu Date: Sat, 23 May 2026 09:27:14 +0100 Subject: [PATCH] fix(refs): resolve unjoined-but-public channels by name resolveChannelRef's name branch called client.channels.getChannels({ workspaceId }), which is membership-scoped and excludes public channels the current user hasn't joined. Acting on such a channel by name failed with CHANNEL_NOT_FOUND while the id: and URL forms worked, because getChannel(id) isn't membership-filtered. Merge getChannels with workspaces.getPublicChannels and dedupe by id so joined and unjoined-but-public channels resolve consistently. Same shape as the --scope public path in src/commands/channel/list.ts. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/lib/refs.test.ts | 66 ++++++++++++++++++++++++++++++++++++++------ src/lib/refs.ts | 17 +++++++++++- 2 files changed, 74 insertions(+), 9 deletions(-) diff --git a/src/lib/refs.test.ts b/src/lib/refs.test.ts index 6023dca..7d6e531 100644 --- a/src/lib/refs.test.ts +++ b/src/lib/refs.test.ts @@ -216,6 +216,17 @@ describe('resolveChannelRef', () => { const mockGetChannel = vi.fn() const mockGetChannels = vi.fn() + const mockGetPublicChannels = vi.fn() + + /** + * For name refs, resolveChannelRef merges joined channels (getChannels — membership-scoped, + * includes both active + archived) with public channels (getPublicChannels — workspace-scoped, + * finds unjoined-but-public channels). Tests default both to empty unless overridden. + */ + function mockChannelLists(joined: unknown[] = [], publicChannels: unknown[] = []) { + mockGetChannels.mockResolvedValue(joined) + mockGetPublicChannels.mockResolvedValue(publicChannels) + } beforeEach(() => { vi.clearAllMocks() @@ -224,6 +235,9 @@ describe('resolveChannelRef', () => { getChannel: mockGetChannel, getChannels: mockGetChannels, }, + workspaces: { + getPublicChannels: mockGetPublicChannels, + }, }) }) @@ -265,19 +279,20 @@ describe('resolveChannelRef', () => { expect(mockGetChannel).not.toHaveBeenCalled() }) - it('resolves exact case-insensitive name match', async () => { + it('resolves exact case-insensitive name match against joined channels', async () => { const ch = createChannel(10, 'General') - mockGetChannels.mockResolvedValue([ch, createChannel(20, 'Leadership')]) + mockChannelLists([ch, createChannel(20, 'Leadership')]) const result = await resolveChannelRef('general', 1) expect(mockGetChannels).toHaveBeenCalledWith({ workspaceId: 1 }) + expect(mockGetPublicChannels).toHaveBeenCalledWith(1) expect(result).toEqual(ch) }) it('resolves unique substring name match', async () => { const ch = createChannel(30, 'Marketing') - mockGetChannels.mockResolvedValue([createChannel(10, 'General'), ch]) + mockChannelLists([createChannel(10, 'General'), ch]) const result = await resolveChannelRef('market', 1) @@ -285,10 +300,7 @@ describe('resolveChannelRef', () => { }) it('throws AMBIGUOUS_CHANNEL on multiple substring matches', async () => { - mockGetChannels.mockResolvedValue([ - createChannel(10, 'Engineering'), - createChannel(20, 'Engineering-Ops'), - ]) + mockChannelLists([createChannel(10, 'Engineering'), createChannel(20, 'Engineering-Ops')]) await expect(resolveChannelRef('eng', 1)).rejects.toHaveProperty( 'code', @@ -297,13 +309,51 @@ describe('resolveChannelRef', () => { }) it('throws CHANNEL_NOT_FOUND when no match', async () => { - mockGetChannels.mockResolvedValue([createChannel(10, 'General')]) + mockChannelLists([createChannel(10, 'General')]) await expect(resolveChannelRef('nope', 1)).rejects.toHaveProperty( 'code', 'CHANNEL_NOT_FOUND', ) }) + + it('resolves unjoined-but-public channel by name', async () => { + const publicCh = createChannel(50, 'Old Public Channel') + mockChannelLists([createChannel(10, 'General')], [publicCh]) + + const result = await resolveChannelRef('Old Public Channel', 1) + + expect(result).toEqual(publicCh) + }) + + it('resolves unjoined-but-public channel by substring', async () => { + const publicCh = createChannel(60, 'tw-cli-smoke-test-channel') + mockChannelLists([createChannel(10, 'General')], [publicCh]) + + const result = await resolveChannelRef('smoke-test', 1) + + expect(result).toEqual(publicCh) + }) + + it('deduplicates channels appearing in both joined and public lists', async () => { + // A public channel the user has joined would appear in both. Resolution must not + // throw AMBIGUOUS_CHANNEL just because the same channel id shows up twice. + const joinedPublic = createChannel(70, 'Engineering', { public: true }) + mockChannelLists([joinedPublic], [joinedPublic]) + + const result = await resolveChannelRef('Engineering', 1) + + expect(result).toEqual(joinedPublic) + }) + + it('throws AMBIGUOUS_CHANNEL on substring matches spanning joined and public lists', async () => { + mockChannelLists([createChannel(10, 'Engineering')], [createChannel(20, 'Engineering-Ops')]) + + await expect(resolveChannelRef('eng', 1)).rejects.toHaveProperty( + 'code', + 'AMBIGUOUS_CHANNEL', + ) + }) }) describe('resolveConversationId', () => { diff --git a/src/lib/refs.ts b/src/lib/refs.ts index 0786d5b..d9a59ea 100644 --- a/src/lib/refs.ts +++ b/src/lib/refs.ts @@ -230,7 +230,22 @@ export async function resolveChannelRef(ref: string, workspaceId: number): Promi } if (parsed.type === 'name') { - const channels = await client.channels.getChannels({ workspaceId }) + // getChannels is membership-scoped — it returns only channels the current user has + // joined (across active + archived). Public channels the user hasn't joined are not + // included, so name-resolving e.g. `tw channel archive "Old Public Channel"` would + // fail with CHANNEL_NOT_FOUND even though the channel is discoverable. Merge with + // getPublicChannels (workspace-scoped, returns all public channels regardless of + // membership) and dedupe by id so a joined-and-public channel doesn't match twice. + const [joined, publicChannels] = await Promise.all([ + client.channels.getChannels({ workspaceId }), + client.workspaces.getPublicChannels(workspaceId), + ]) + const byId = new Map() + for (const channel of joined) byId.set(channel.id, channel) + for (const channel of publicChannels) { + if (!byId.has(channel.id)) byId.set(channel.id, channel) + } + const channels = Array.from(byId.values()) return matchByName(channels, parsed.name, { ambiguousCode: 'AMBIGUOUS_CHANNEL', notFoundCode: 'CHANNEL_NOT_FOUND',