fix: stop treating unjoined public channels as private (#102)

* fix: stop treating unjoined public channels as private

The private-channel check only knew about the channels you've joined,
so a thread in a public channel you hadn't joined failed with "This
thread belongs to a private channel" and needed
--include-private-channels.

When a channel isn't one you've joined, the check now looks for it
among the workspace's public channels before failing. Joined channels
work as before with no extra call, and joined private channels still
need the flag.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* perf: use the channel's own public flag when the caller has it

channel threads and thread create already have the channel they
resolved, so the private-channel check now takes its public flag
instead of looking it up. For an unjoined channel resolved by name,
the lookup was fetching the workspace's public channels a second time.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Author: craigcarlyle

src/commands/channel/threads.test.ts

@@ -437,7 +437,7 @@ describe('channel threads', () => {
 
         await program.parseAsync(['node', 'tdc', 'channel', 'threads', '12345', '--json'])
 
-        expect(vi.mocked(assertChannelIsPublic)).toHaveBeenCalledWith('CH100', 1)
+        expect(vi.mocked(assertChannelIsPublic)).toHaveBeenCalledWith('CH100', 1, true)
     })
 
     it('propagates the error when the channel is private and the guard rejects', async () => {

src/commands/channel/threads.ts

@@ -156,7 +156,7 @@ export async function showChannelThreads(
     }
 
     const channel = await resolveChannelRef(channelRef, workspaceId)
-    await assertChannelIsPublic(channel.id, workspaceId)
+    await assertChannelIsPublic(channel.id, workspaceId, channel.public)
 
     const limitNum = options.limit ? parseInt(options.limit, 10) : 50
     const limit = Number.isFinite(limitNum) && limitNum > 0 ? limitNum : 50

src/commands/thread/create.ts

@@ -47,7 +47,7 @@ export async function createThread(
 
     const client = await getCommsClient()
     const channel = await client.channels.getChannel(channelId)
-    await assertChannelIsPublic(channelId, channel.workspaceId)
+    await assertChannelIsPublic(channelId, channel.workspaceId, channel.public)
 
     let resolved: ResolvedNotify | undefined
     if (allIds) {

src/lib/public-channels.test.ts

@@ -16,11 +16,15 @@ const mockGetCommsClient = vi.mocked(getCommsClient)
 
 function makeMockChannels(
     channels: Array<{ id: string; public: boolean }>,
+    workspacePublicChannels: Array<{ id: string }> = [],
 ): ReturnType<typeof getCommsClient> {
     return Promise.resolve({
         channels: {
             getChannels: vi.fn().mockResolvedValue(channels),
         },
+        workspaces: {
+            getPublicChannels: vi.fn().mockResolvedValue(workspacePublicChannels),
+        },
     }) as unknown as ReturnType<typeof getCommsClient>
 }
 
@@ -161,6 +165,58 @@ describe('assertChannelIsPublic', () => {
         await expect(assertChannelIsPublic('CH5', 100)).resolves.toBeUndefined()
     })
 
+    it("allows a public channel the user hasn't joined", async () => {
+        mockGetCommsClient.mockImplementation(() =>
+            makeMockChannels([{ id: 'CH5', public: true }], [{ id: 'CH5' }, { id: 'CH7' }]),
+        )
+
+        await expect(assertChannelIsPublic('CH7', 100)).resolves.toBeUndefined()
+    })
+
+    it('throws for a channel that is neither joined nor public', async () => {
+        mockGetCommsClient.mockImplementation(() =>
+            makeMockChannels([{ id: 'CH5', public: true }], [{ id: 'CH5' }, { id: 'CH7' }]),
+        )
+
+        await expect(assertChannelIsPublic('CH9', 100)).rejects.toThrow('private channel')
+    })
+
+    it("only lists the workspace's public channels for a channel the user hasn't joined", async () => {
+        const getPublicChannels = vi.fn().mockResolvedValue([{ id: 'CH5' }, { id: 'CH7' }])
+        mockGetCommsClient.mockResolvedValue({
+            channels: {
+                getChannels: vi.fn().mockResolvedValue([
+                    { id: 'CH5', public: true },
+                    { id: 'CH6', public: false },
+                ]),
+            },
+            workspaces: { getPublicChannels },
+        } as unknown as Awaited<ReturnType<typeof getCommsClient>>)
+
+        await assertChannelIsPublic('CH5', 100)
+        await expect(assertChannelIsPublic('CH6', 100)).rejects.toThrow('private channel')
+        expect(getPublicChannels).not.toHaveBeenCalled()
+
+        await assertChannelIsPublic('CH7', 100)
+        await assertChannelIsPublic('CH7', 100)
+        expect(getPublicChannels).toHaveBeenCalledTimes(1)
+        expect(getPublicChannels).toHaveBeenCalledWith(100)
+    })
+
+    it('uses the public flag a caller passes instead of looking the channel up', async () => {
+        const getChannels = vi.fn()
+        const getPublicChannels = vi.fn()
+        mockGetCommsClient.mockResolvedValue({
+            channels: { getChannels },
+            workspaces: { getPublicChannels },
+        } as unknown as Awaited<ReturnType<typeof getCommsClient>>)
+
+        await expect(assertChannelIsPublic('CH7', 100, true)).resolves.toBeUndefined()
+        await expect(assertChannelIsPublic('CH6', 100, false)).rejects.toThrow('private channel')
+        expect(getChannels).not.toHaveBeenCalled()
+        expect(getPublicChannels).not.toHaveBeenCalled()
+    })
+
     it('allows private channels when --include-private-channels is set', async () => {
         process.argv = ['node', 'tdc', '--include-private-channels']
         resetGlobalArgs()

src/lib/public-channels.ts

@@ -2,30 +2,65 @@ import { getCommsClient } from './api.js'
 import { CliError } from './errors.js'
 import { includePrivateChannels } from './global-args.js'
 
-const publicChannelCache = new Map<number, Set<string>>()
+// Channel ID to whether it's public, for the channels the user has joined.
+const joinedChannelCache = new Map<number, Map<string, boolean>>()
+const workspacePublicChannelCache = new Map<number, Set<string>>()
 
-export async function getPublicChannelIds(workspaceId: number): Promise<Set<string>> {
-    const cached = publicChannelCache.get(workspaceId)
+async function getJoinedChannels(workspaceId: number): Promise<Map<string, boolean>> {
+    const cached = joinedChannelCache.get(workspaceId)
     if (cached) return cached
 
     const client = await getCommsClient()
     const channels = await client.channels.getChannels({ workspaceId })
-    const publicIds = new Set<string>()
-    for (const ch of channels) {
-        if (ch.public) publicIds.add(ch.id)
-    }
-    publicChannelCache.set(workspaceId, publicIds)
+    const joined = new Map(channels.map((ch) => [ch.id, ch.public]))
+    joinedChannelCache.set(workspaceId, joined)
+    return joined
+}
+
+/** IDs of the public channels the user has joined. */
+export async function getPublicChannelIds(workspaceId: number): Promise<Set<string>> {
+    const joined = await getJoinedChannels(workspaceId)
+    return new Set([...joined].filter(([, isPublic]) => isPublic).map(([id]) => id))
+}
+
+// Every public channel in the workspace, joined or not. Much bigger than the joined list (all
+// members of every public channel) and empty for guests, so it's only used for unjoined channels.
+async function getWorkspacePublicChannelIds(workspaceId: number): Promise<Set<string>> {
+    const cached = workspacePublicChannelCache.get(workspaceId)
+    if (cached) return cached
+
+    const client = await getCommsClient()
+    const channels = await client.workspaces.getPublicChannels(workspaceId)
+    const publicIds = new Set(channels.map((ch) => ch.id))
+    workspacePublicChannelCache.set(workspaceId, publicIds)
     return publicIds
 }
 
 export function clearPublicChannelCache(): void {
-    publicChannelCache.clear()
+    joinedChannelCache.clear()
+    workspacePublicChannelCache.clear()
+}
+
+async function isPublicChannel(channelId: string, workspaceId: number): Promise<boolean> {
+    const joined = await getJoinedChannels(workspaceId)
+    return (
+        joined.get(channelId) ??
+        // getChannels only returns joined channels, so an unjoined one could still be public.
+        (await getWorkspacePublicChannelIds(workspaceId)).has(channelId)
+    )
 }
 
-export async function assertChannelIsPublic(channelId: string, workspaceId: number): Promise<void> {
+/**
+ * Throws unless the channel is public or --include-private-channels is set. Callers that
+ * already have the channel pass its `public` flag, which skips the lookups.
+ */
+export async function assertChannelIsPublic(
+    channelId: string,
+    workspaceId: number,
+    isPublic?: boolean,
+): Promise<void> {
     if (includePrivateChannels()) return
-    const publicIds = await getPublicChannelIds(workspaceId)
-    if (!publicIds.has(channelId)) {
+    if (!(isPublic ?? (await isPublicChannel(channelId, workspaceId)))) {
         throw new CliError('NOT_FOUND', 'This thread belongs to a private channel.', [
             'Use --include-private-channels to access it',
         ])