From d67d5a9f5de94363bebe593a7769055d0c24c930 Mon Sep 17 00:00:00 2001 From: lmjabreu Date: Thu, 17 Sep 2026 19:32:12 +0100 Subject: [PATCH 1/5] feat: add thread undone, conversation undone and thread mark-unread Give `tdc` the inverses it lacked so a caller that archives or marks read can record an undo step. `thread undone` and `conversation undone` mirror their `done` siblings (`--yes`, `--dry-run`, `--json`). `thread mark-unread` mirrors `mark-read`, including bulk refs on stdin, and takes `--from ` to mark unread from one comment onward: the API's objIndex is the last comment that stays read, so a comment at index N is sent as N-1 (-1 marks the whole thread). The bulk-ref and unread-lookup helpers `mark-read` kept private move to thread/helpers.ts so both verbs share them. Co-Authored-By: Claude Opus 5 --- README.md | 6 + skills/comms-cli/SKILL.md | 10 +- .../conversation/conversation.test.ts | 100 ++++++ src/commands/conversation/index.ts | 16 + src/commands/conversation/undone.ts | 41 +++ src/commands/thread/helpers.ts | 95 +++++- src/commands/thread/index.ts | 46 ++- src/commands/thread/mutate.ts | 37 +++ src/commands/thread/read.ts | 95 +----- src/commands/thread/thread.test.ts | 284 ++++++++++++++++++ src/commands/thread/unread.ts | 144 +++++++++ src/lib/api.ts | 1 + src/lib/skills/content.ts | 10 +- 13 files changed, 800 insertions(+), 85 deletions(-) create mode 100644 src/commands/conversation/undone.ts create mode 100644 src/commands/thread/unread.ts diff --git a/README.md b/README.md index 00c3769..ecc80bf 100644 --- a/README.md +++ b/README.md @@ -128,9 +128,15 @@ tdc thread reply # reply to a thread tdc thread reply "Update" --notify NONE # reply without notifying anyone tdc thread rename "New title" # rename a thread tdc thread update "New body" # edit a thread's body (first post) +tdc thread done --yes # archive a thread (mark done) +tdc thread undone --yes # move it back to your inbox +tdc thread mark-read # mark a thread read +tdc thread mark-unread # mark it unread again (--from for part of it) tdc conversation unread # list unread conversations tdc conversation list # list conversations (--kind, --participant, --name, --state) tdc conversation view # view conversation messages +tdc conversation done --yes # archive a conversation +tdc conversation undone --yes # unarchive it tdc msg view # view a conversation message tdc search "keyword" # search across workspace tdc search "keyword" --all # fetch all result pages diff --git a/skills/comms-cli/SKILL.md b/skills/comms-cli/SKILL.md index 2471f9f..a92190f 100644 --- a/skills/comms-cli/SKILL.md +++ b/skills/comms-cli/SKILL.md @@ -118,9 +118,14 @@ tdc thread reply "content" --file ./a.png # Attach a file (repeatable; co tdc thread done # Preview thread archive (requires --yes to execute) tdc thread done --yes # Archive thread (mark done) tdc thread done --yes --json # Archive and return status as JSON +tdc thread undone --yes # Unarchive thread (move it back to your inbox); inverse of done +tdc thread undone --yes --json # Unarchive and return status as JSON tdc thread mark-read # Mark a thread read tdc thread mark-read --yes # Mark multiple threads read printf "id:CbT8n2Kp4Qx6Rz9Lm3Va\nid:CbT9m4Qr7Vz2Nx8Lp5Sa\n" | tdc thread mark-read --dry-run # Preview bulk mark-read from stdin +tdc thread mark-unread # Mark a whole thread unread; inverse of mark-read +tdc thread mark-unread --from # Mark unread from that comment onward (single thread only) +tdc thread mark-unread --yes # Mark multiple threads unread (also accepts refs on stdin) tdc thread mute # Mute thread for 60 minutes (default) tdc thread mute --minutes 480 # Mute for custom duration tdc thread mute --json # Mute and return { id, mutedUntil } as JSON @@ -189,6 +194,8 @@ tdc conversation reply "content" --file ./a.png # Attach a file (repeatab tdc conversation done # Preview conversation archive (requires --yes to execute) tdc conversation done --yes # Archive conversation tdc conversation done --yes --json # Archive and return status as JSON +tdc conversation undone --yes # Unarchive conversation; inverse of done +tdc conversation undone --yes --json # Unarchive and return status as JSON tdc conversation mute # Mute conversation for 60 minutes (default) tdc conversation mute --minutes 480 # Mute for custom duration tdc conversation mute --json # Mute and return { id, mutedUntil } as JSON @@ -446,7 +453,7 @@ echo "Quick reply" | tdc conversation reply If no content argument is provided and no stdin is piped, the CLI opens `$EDITOR` for interactive input. In non-TTY environments (e.g. when called by an agent or in a pipeline), the editor is automatically skipped and the command fails fast with an actionable error message. Use `--non-interactive` to force this behavior even in a TTY, or `--interactive` to override auto-detection. -`tdc thread mark-read` also accepts thread refs from stdin, one per line: +`tdc thread mark-read` and `tdc thread mark-unread` also accept thread refs from stdin, one per line: ```bash printf "id:CbT8n2Kp4Qx6Rz9Lm3Va\nid:CbT9m4Qr7Vz2Nx8Lp5Sa\n" | tdc thread mark-read --yes @@ -469,6 +476,7 @@ tdc inbox --unread --json tdc thread view --unread tdc thread reply "Thanks, I'll look into this." tdc thread done --yes +tdc thread undone --yes # Changed your mind: back to the inbox ``` **Search and review:** diff --git a/src/commands/conversation/conversation.test.ts b/src/commands/conversation/conversation.test.ts index 16dfc80..e244cc1 100644 --- a/src/commands/conversation/conversation.test.ts +++ b/src/commands/conversation/conversation.test.ts @@ -114,6 +114,7 @@ function createClient({ getUnread: vi.fn().mockResolvedValue({ data: [], version: 1 }), getConversation: vi.fn(async (id: string) => conversationsById.get(id)), archiveConversation: vi.fn(), + unarchiveConversation: vi.fn(), muteConversation: vi.fn(async ({ id, minutes }: { id: string; minutes: number }) => ({ ...conversationsById.get(id), mutedUntil: new Date(Date.now() + minutes * 60000), @@ -1225,6 +1226,105 @@ describe('conversation done', () => { }) }) +describe('conversation undone', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('unarchives a conversation with --yes', async () => { + const conversation = { + ...createConversation(42, [1, 2], '2026-03-08T10:00:00.000Z'), + archived: true, + } + const client = createClient({ archivedConversations: [conversation] }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'conversation', 'undone', '42', '--yes']) + + expect(client.conversations.unarchiveConversation).toHaveBeenCalledWith('42') + expect(client.conversations.archiveConversation).not.toHaveBeenCalled() + expect(consoleSpy).toHaveBeenCalledWith('Conversation 42 unarchived.') + }) + + it('prompts for confirmation without --yes', async () => { + const conversation = createConversation(42, [1, 2], '2026-03-08T10:00:00.000Z') + const client = createClient({ archivedConversations: [conversation] }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'conversation', 'undone', '42']) + + expect(consoleSpy).toHaveBeenCalledWith('Would unarchive: conversation 42') + expect(consoleSpy).toHaveBeenCalledWith('Use --yes to confirm.') + expect(client.conversations.unarchiveConversation).not.toHaveBeenCalled() + }) + + it('outputs JSON with --json --yes', async () => { + const conversation = createConversation(42, [1, 2], '2026-03-08T10:00:00.000Z') + const client = createClient({ archivedConversations: [conversation] }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'conversation', 'undone', '42', '--json', '--yes']) + + const jsonOutput = JSON.parse(consoleSpy.mock.calls[0][0]) + expect(jsonOutput).toEqual({ id: '42', archived: false }) + }) + + it('errors when --json is used without --yes', async () => { + const conversation = createConversation(42, [1, 2], '2026-03-08T10:00:00.000Z') + const client = createClient({ archivedConversations: [conversation] }) + apiMocks.getCommsClient.mockResolvedValue(client) + const program = createProgram() + + await expect( + program.parseAsync(['node', 'tdc', 'conversation', 'undone', '42', '--json']), + ).rejects.toHaveProperty('code', 'MISSING_YES_FLAG') + + expect(client.conversations.unarchiveConversation).not.toHaveBeenCalled() + }) + + it('shows dry run output and flags a conversation that is not archived', async () => { + const conversation = createConversation(42, [1, 2], '2026-03-08T10:00:00.000Z') + const client = createClient({ activeConversations: [conversation] }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'conversation', 'undone', '42', '--dry-run']) + + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining('Would unarchive conversation'), + ) + expect(consoleSpy).toHaveBeenCalledWith(' Conversation: conversation 42') + expect(consoleSpy).toHaveBeenCalledWith(' Status: not archived') + expect(client.conversations.unarchiveConversation).not.toHaveBeenCalled() + }) + + it('runs validation in dry-run mode', async () => { + const client = createClient({ activeConversations: [] }) + client.conversations.getConversation.mockRejectedValueOnce( + new Error('conversation not found'), + ) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + + await expect( + program.parseAsync(['node', 'tdc', 'conversation', 'undone', '42', '--dry-run']), + ).rejects.toThrow('conversation not found') + expect(client.conversations.unarchiveConversation).not.toHaveBeenCalled() + }) +}) + describe('conversation reply --file', () => { const files = useFileFixtures('tdc-convo-reply-', 'photo.png', 'doc.pdf') diff --git a/src/commands/conversation/index.ts b/src/commands/conversation/index.ts index 339e702..2365030 100644 --- a/src/commands/conversation/index.ts +++ b/src/commands/conversation/index.ts @@ -5,6 +5,7 @@ import { markConversationDone } from './done.js' import { listConversations } from './list.js' import { muteConversation } from './mute.js' import { replyToConversation } from './reply.js' +import { markConversationUndone } from './undone.js' import { unmuteConversation } from './unmute.js' import { showUnread } from './unread.js' import { viewConversation } from './view.js' @@ -160,6 +161,21 @@ Examples: ) .action(markConversationDone) + conversation + .command('undone ') + .description('Unarchive a conversation; inverse of done') + .option('--yes', 'Confirm unarchive') + .option('--dry-run', 'Show what would happen without executing') + .option('--json', 'Output result as JSON') + .addHelpText( + 'after', + ` +Examples: + tdc conversation undone id:CbV8n2Kp4Qx6Rz9Lm3Va --yes + tdc conversation undone id:CbV8n2Kp4Qx6Rz9Lm3Va --dry-run`, + ) + .action(markConversationUndone) + conversation .command('mute ') .description('Mute a conversation (stop notifications)') diff --git a/src/commands/conversation/undone.ts b/src/commands/conversation/undone.ts new file mode 100644 index 0000000..caf1cdd --- /dev/null +++ b/src/commands/conversation/undone.ts @@ -0,0 +1,41 @@ +import { getCommsClient } from '../../lib/api.js' +import { CliError } from '../../lib/errors.js' +import { formatJson, printDryRun } from '../../lib/output.js' +import { resolveConversationId } from '../../lib/refs.js' +import { conversationLabel, type DoneOptions } from './helpers.js' + +export async function markConversationUndone(ref: string, options: DoneOptions): Promise { + const conversationId = resolveConversationId(ref) + + const client = await getCommsClient() + const conversation = await client.conversations.getConversation(conversationId) + + if (options.dryRun) { + printDryRun('unarchive conversation', { + Conversation: conversationLabel(conversation), + Status: conversation.archived ? undefined : 'not archived', + }) + return + } + + if (!options.yes) { + if (options.json) { + throw new CliError( + 'MISSING_YES_FLAG', + '--yes is required to execute unarchive in --json mode.', + ) + } + console.log(`Would unarchive: ${conversationLabel(conversation)}`) + console.log('Use --yes to confirm.') + return + } + + await client.conversations.unarchiveConversation(conversationId) + + if (options.json) { + console.log(formatJson({ id: conversationId, archived: false })) + return + } + + console.log(`Conversation ${conversationId} unarchived.`) +} diff --git a/src/commands/thread/helpers.ts b/src/commands/thread/helpers.ts index e39c44f..fe22506 100644 --- a/src/commands/thread/helpers.ts +++ b/src/commands/thread/helpers.ts @@ -1,9 +1,12 @@ +import type { CommsApi, Thread } from '@doist/comms-sdk' import chalk from 'chalk' import { getWorkspaceGroups, getWorkspaceUsers } from '../../lib/api.js' import { formatRelativeDate } from '../../lib/dates.js' import { isAccessible } from '../../lib/global-args.js' +import { readStdinToEnd } from '../../lib/input.js' import { renderMarkdown } from '../../lib/markdown.js' -import { colors } from '../../lib/output.js' +import { colors, pluralize } from '../../lib/output.js' +import { assertChannelIsPublic } from '../../lib/public-channels.js' import { partitionNotifyIds } from '../../lib/refs.js' export function printSeparator(label: string): void { @@ -75,3 +78,93 @@ export async function resolveNotifyIds( export function formatNotifyLabel(items: NamedEntity[]): string { return items.map((i) => `${i.name} (${i.id})`).join(', ') } + +// Shared by `mark-read` and `mark-unread`: bulk ref collection, the per-workspace +// unread lookup, and the text summary. + +export type ReadStateTextStatus = 'changed' | 'preview' | 'unchanged' + +export type ThreadReadState = { + thread: Thread + /** + * Object index of the last comment the user has read, or `null` when the + * thread is fully read (absent from the workspace's unread list). `-1` + * means nothing has been read, including the thread body. + */ + lastReadObjIndex: number | null +} + +export async function collectThreadRefs(refs: string[]): Promise { + const inlineRefs = refs.map((ref) => ref.trim()).filter(Boolean) + + const stdinContent = await readStdinToEnd() + if (!stdinContent) return inlineRefs + + const stdinRefs = stdinContent + .split(/\r?\n/) + .map((line) => line.trim()) + .filter((line) => line !== '' && !line.startsWith('#')) + + return [...inlineRefs, ...stdinRefs] +} + +/** + * Loads a thread and its unread position. `unreadCache` maps a workspace id to + * its unread threads (`threadId` -> last read `objIndex`) so bulk runs fetch + * the unread list once per workspace; callers update it after mutating. + */ +export async function loadThreadReadState( + client: CommsApi, + unreadCache: Map>, + threadId: string, +): Promise { + const thread = await client.threads.getThread(threadId) + await assertChannelIsPublic(thread.channelId, thread.workspaceId) + + let unreadByThread = unreadCache.get(thread.workspaceId) + if (!unreadByThread) { + const unread = await client.threads.getUnread(thread.workspaceId) + unreadByThread = new Map( + unread.data.map((unreadThread) => [unreadThread.threadId, unreadThread.objIndex]), + ) + unreadCache.set(thread.workspaceId, unreadByThread) + } + + return { thread, lastReadObjIndex: unreadByThread.get(thread.id) ?? null } +} + +export function getLatestObjIndex(thread: Thread): number { + return Math.max( + ...[thread.lastComment?.objIndex, thread.lastObjIndex, thread.commentCount, 0] + .filter((value): value is number => typeof value === 'number') + .map((value) => Math.max(value, 0)), + ) +} + +export function threadLabel(thread: Thread): string { + return `${thread.title} (${thread.id})` +} + +export function printReadStateSummary(statuses: ReadStateTextStatus[]): void { + const summary = [ + summarizeStatus(statuses, 'changed'), + summarizeStatus(statuses, 'unchanged'), + summarizeStatus(statuses, 'preview'), + ].filter(Boolean) + + console.log('') + console.log(`Summary: ${summary.join(', ')}`) +} + +function summarizeStatus( + statuses: ReadStateTextStatus[], + status: ReadStateTextStatus, +): string | null { + const count = statuses.filter((value) => value === status).length + if (count === 0) { + return null + } + + const noun = status === 'preview' ? pluralize(count, 'preview') : pluralize(count, 'thread') + return status === 'preview' ? `${count} ${noun}` : `${count} ${status} ${noun}` +} diff --git a/src/commands/thread/index.ts b/src/commands/thread/index.ts index dc17c9c..c4d4c7f 100644 --- a/src/commands/thread/index.ts +++ b/src/commands/thread/index.ts @@ -3,11 +3,12 @@ import { withUnvalidatedChoices } from '../../lib/completion.js' import { collect } from '../../lib/options.js' import { createThread } from './create.js' import { deleteThread } from './delete.js' -import { markThreadDone } from './mutate.js' +import { markThreadDone, markThreadUndone } from './mutate.js' import { muteThread, unmuteThread } from './mute.js' import { markThreadRead } from './read.js' import { renameThread } from './rename.js' import { replyToThread } from './reply.js' +import { markThreadUnread } from './unread.js' import { updateThread } from './update.js' import { viewThread } from './view.js' @@ -117,6 +118,22 @@ Examples: ) .action(markThreadDone) + thread + .command('undone ') + .description('Unarchive a thread (move it back to your inbox); inverse of done') + .option('--yes', 'Confirm unarchive') + .option('--dry-run', 'Show what would happen without executing') + .option('--json', 'Output result as JSON') + .addHelpText( + 'after', + ` +Examples: + tdc thread undone id:CbT8n2Kp4Qx6Rz9Lm3Va --yes + tdc thread undone id:CbT8n2Kp4Qx6Rz9Lm3Va --dry-run + tdc thread undone id:CbT8n2Kp4Qx6Rz9Lm3Va --json --yes`, + ) + .action(markThreadUndone) + const markReadCmd = thread .command('mark-read [thread-refs...]') .description('Mark a thread read for the current user') @@ -139,6 +156,33 @@ Examples: return markThreadRead(refs, options) }) + const markUnreadCmd = thread + .command('mark-unread [thread-refs...]') + .description('Mark a thread unread for the current user; inverse of mark-read') + .option( + '--from ', + 'Mark unread from this comment onward (single thread only; default: whole thread)', + ) + .option('--yes', 'Skip confirmation for bulk operations') + .option('--dry-run', 'Show what would happen without executing') + .option('--json', 'Output result as JSON') + .addHelpText( + 'after', + ` +Examples: + tdc thread mark-unread id:CbT8n2Kp4Qx6Rz9Lm3Va + tdc thread mark-unread id:CbT8n2Kp4Qx6Rz9Lm3Va --from id:CbM8n2Kp4Qx6Rz9Lm3Va + tdc thread mark-unread id:CbT8n2Kp4Qx6Rz9Lm3Va id:CbT9m4Qr7Vz2Nx8Lp5Sa --yes + printf "id:CbT8n2Kp4Qx6Rz9Lm3Va\\nid:CbT9m4Qr7Vz2Nx8Lp5Sa\\n" | tdc thread mark-unread --yes`, + ) + .action((refs, options) => { + if (refs.length === 0 && process.stdin.isTTY) { + markUnreadCmd.help() + return + } + return markThreadUnread(refs, options) + }) + thread .command('delete ') .description('Permanently delete a thread') diff --git a/src/commands/thread/mutate.ts b/src/commands/thread/mutate.ts index 3ef90e8..3733e4d 100644 --- a/src/commands/thread/mutate.ts +++ b/src/commands/thread/mutate.ts @@ -40,3 +40,40 @@ export async function markThreadDone(ref: string, options: MutationOptions): Pro console.log(`Thread ${threadId} archived.`) } + +export async function markThreadUndone(ref: string, options: MutationOptions): Promise { + const threadId = resolveThreadId(ref) + + const client = await getCommsClient() + const thread = await client.threads.getThread(threadId) + await assertChannelIsPublic(thread.channelId, thread.workspaceId) + + if (options.dryRun) { + printDryRun('unarchive thread', { + Thread: `${thread.title} (${threadId})`, + Status: thread.isArchived ? undefined : 'already in inbox', + }) + return + } + + if (!options.yes) { + if (options.json) { + throw new CliError( + 'MISSING_YES_FLAG', + '--yes is required to execute unarchive in --json mode.', + ) + } + console.log(`Would unarchive: ${thread.title}`) + console.log('Use --yes to confirm.') + return + } + + await client.inbox.unarchiveThread(threadId) + + if (options.json) { + console.log(formatJson({ id: threadId, isArchived: false })) + return + } + + console.log(`Thread ${threadId} unarchived.`) +} diff --git a/src/commands/thread/read.ts b/src/commands/thread/read.ts index b1df3a4..28c5ab3 100644 --- a/src/commands/thread/read.ts +++ b/src/commands/thread/read.ts @@ -1,26 +1,24 @@ -import type { CommsApi, Thread } from '@doist/comms-sdk' import { getCommsClient } from '../../lib/api.js' import { CliError } from '../../lib/errors.js' -import { readStdinToEnd } from '../../lib/input.js' import type { MutationOptions } from '../../lib/options.js' -import { formatJson, pluralize } from '../../lib/output.js' -import { assertChannelIsPublic } from '../../lib/public-channels.js' +import { formatJson } from '../../lib/output.js' import { resolveThreadId } from '../../lib/refs.js' +import { + collectThreadRefs, + getLatestObjIndex, + loadThreadReadState, + printReadStateSummary, + type ReadStateTextStatus, + threadLabel, +} from './helpers.js' export type MarkThreadReadOptions = MutationOptions -type LoadedThread = { - thread: Thread - isUnread: boolean -} - type MarkReadStatus = { id: string isRead: true } -type TextStatus = 'changed' | 'preview' | 'unchanged' - export async function markThreadRead( refs: string[], options: MarkThreadReadOptions, @@ -42,15 +40,15 @@ export async function markThreadRead( } const client = await getCommsClient() - const unreadCache = new Map>() + const unreadCache = new Map>() const jsonStatuses: MarkReadStatus[] = [] - const textStatuses: TextStatus[] = [] + const textStatuses: ReadStateTextStatus[] = [] for (const rawRef of rawRefs) { const threadId = resolveThreadId(rawRef) - const loaded = await loadThread(client, unreadCache, threadId) + const loaded = await loadThreadReadState(client, unreadCache, threadId) - if (!loaded.isUnread) { + if (loaded.lastReadObjIndex === null) { jsonStatuses.push({ id: threadId, isRead: true }) textStatuses.push('unchanged') if (!options.json) { @@ -88,75 +86,10 @@ export async function markThreadRead( } if (!options.json && rawRefs.length > 1) { - printSummary(textStatuses) + printReadStateSummary(textStatuses) } if (!options.json && needsConfirmation) { console.log('Use --yes to confirm.') } } - -async function collectThreadRefs(refs: string[]): Promise { - const inlineRefs = refs.map((ref) => ref.trim()).filter(Boolean) - - const stdinContent = await readStdinToEnd() - if (!stdinContent) return inlineRefs - - const stdinRefs = stdinContent - .split(/\r?\n/) - .map((line) => line.trim()) - .filter((line) => line !== '' && !line.startsWith('#')) - - return [...inlineRefs, ...stdinRefs] -} - -async function loadThread( - client: CommsApi, - unreadCache: Map>, - threadId: string, -): Promise { - const thread = await client.threads.getThread(threadId) - await assertChannelIsPublic(thread.channelId, thread.workspaceId) - - let unreadIds = unreadCache.get(thread.workspaceId) - if (!unreadIds) { - const unread = await client.threads.getUnread(thread.workspaceId) - unreadIds = new Set(unread.data.map((unreadThread) => unreadThread.threadId)) - unreadCache.set(thread.workspaceId, unreadIds) - } - - return { thread, isUnread: unreadIds.has(thread.id) } -} - -function getLatestObjIndex(thread: Thread): number { - return Math.max( - ...[thread.lastComment?.objIndex, thread.lastObjIndex, thread.commentCount, 0] - .filter((value): value is number => typeof value === 'number') - .map((value) => Math.max(value, 0)), - ) -} - -function threadLabel(thread: Thread): string { - return `${thread.title} (${thread.id})` -} - -function printSummary(statuses: TextStatus[]): void { - const summary = [ - summarizeStatus(statuses, 'changed'), - summarizeStatus(statuses, 'unchanged'), - summarizeStatus(statuses, 'preview'), - ].filter(Boolean) - - console.log('') - console.log(`Summary: ${summary.join(', ')}`) -} - -function summarizeStatus(statuses: TextStatus[], status: TextStatus): string | null { - const count = statuses.filter((value) => value === status).length - if (count === 0) { - return null - } - - const noun = status === 'preview' ? pluralize(count, 'preview') : pluralize(count, 'thread') - return status === 'preview' ? `${count} ${noun}` : `${count} ${status} ${noun}` -} diff --git a/src/commands/thread/thread.test.ts b/src/commands/thread/thread.test.ts index 9b81f65..0e06d92 100644 --- a/src/commands/thread/thread.test.ts +++ b/src/commands/thread/thread.test.ts @@ -122,6 +122,12 @@ function createClient({ markRead: vi.fn(async ({ id }: { id: string; objIndex: number }) => { unreadState = unreadState.filter((unread) => unread.threadId !== id) }), + markUnread: vi.fn(async ({ id, objIndex }: { id: string; objIndex: number }) => { + unreadState = [ + ...unreadState.filter((unread) => unread.threadId !== id), + { threadId: id, channelId: 'CH100', objIndex, directMention: false }, + ] + }), muteThread: vi.fn(async (_args: { id: string; minutes: number }) => ({ ...thread, mutedUntil: new Date(Date.now() + _args.minutes * 60000), @@ -1829,6 +1835,284 @@ describe('thread done', () => { }) }) +describe('thread undone', () => { + beforeEach(() => { + clearWorkspaceUserCache() + vi.clearAllMocks() + }) + + it('unarchives a thread with --yes', async () => { + const client = createClient({ thread: { ...createThreadFixture(500), isArchived: true } }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'thread', 'undone', '500', '--yes']) + + expect(client.inbox.unarchiveThread).toHaveBeenCalledWith('500') + expect(client.inbox.archiveThread).not.toHaveBeenCalled() + expect(consoleSpy).toHaveBeenCalledWith('Thread 500 unarchived.') + }) + + it('prompts for confirmation without --yes', async () => { + const client = createClient() + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'thread', 'undone', '500']) + + expect(consoleSpy).toHaveBeenCalledWith('Would unarchive: Test Thread') + expect(consoleSpy).toHaveBeenCalledWith('Use --yes to confirm.') + expect(client.inbox.unarchiveThread).not.toHaveBeenCalled() + }) + + it('outputs JSON with --json --yes', async () => { + const client = createClient() + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'thread', 'undone', '500', '--json', '--yes']) + + const jsonOutput = JSON.parse(consoleSpy.mock.calls[0][0]) + expect(jsonOutput).toEqual({ id: '500', isArchived: false }) + }) + + it('errors when --json is used without --yes', async () => { + const client = createClient() + apiMocks.getCommsClient.mockResolvedValue(client) + const program = createProgram() + + await expect( + program.parseAsync(['node', 'tdc', 'thread', 'undone', '500', '--json']), + ).rejects.toHaveProperty('code', 'MISSING_YES_FLAG') + + expect(client.inbox.unarchiveThread).not.toHaveBeenCalled() + }) + + it('shows dry run output and flags a thread already in the inbox', async () => { + const client = createClient() + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'thread', 'undone', '500', '--dry-run']) + + expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Would unarchive thread')) + expect(consoleSpy).toHaveBeenCalledWith(' Thread: Test Thread (500)') + expect(consoleSpy).toHaveBeenCalledWith(' Status: already in inbox') + expect(client.inbox.unarchiveThread).not.toHaveBeenCalled() + }) + + it('runs validation in dry-run mode', async () => { + const client = createClient() + apiMocks.getCommsClient.mockResolvedValue(client) + client.threads.getThread.mockRejectedValueOnce(new Error('thread not found')) + + const program = createProgram() + + await expect( + program.parseAsync(['node', 'tdc', 'thread', 'undone', '500', '--dry-run']), + ).rejects.toThrow('thread not found') + expect(client.inbox.unarchiveThread).not.toHaveBeenCalled() + }) +}) + +describe('thread mark-unread', () => { + beforeEach(() => { + clearWorkspaceUserCache() + vi.clearAllMocks() + vi.mocked(readStdinToEnd).mockResolvedValue('') + }) + + it('marks a read thread unread from the start without requiring --yes', async () => { + const client = createClient() + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'thread', 'mark-unread', '500']) + + expect(client.threads.markUnread).toHaveBeenCalledWith({ id: '500', objIndex: -1 }) + expect(consoleSpy).toHaveBeenCalledWith('Thread Test Thread (500) marked unread.') + }) + + it('marks unread from a comment by passing the previous object index', async () => { + const client = createClient({ comments: [createComment(10, 0), createComment(11, 1)] }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'thread', 'mark-unread', '500', '--from', '11']) + + expect(client.comments.getComment).toHaveBeenCalledWith('11') + expect(client.threads.markUnread).toHaveBeenCalledWith({ id: '500', objIndex: 0 }) + expect(consoleSpy).toHaveBeenCalledWith( + 'Thread Test Thread (500) marked unread from comment 11.', + ) + }) + + it('rejects a --from comment that belongs to another thread', async () => { + const client = createClient({ + comments: [{ ...createComment(11, 1), threadId: '999' }], + }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + + await expect( + program.parseAsync(['node', 'tdc', 'thread', 'mark-unread', '500', '--from', '11']), + ).rejects.toHaveProperty('code', 'INVALID_REF') + expect(client.threads.markUnread).not.toHaveBeenCalled() + }) + + it('rejects --from with more than one thread ref', async () => { + const client = createClient({ comments: [createComment(11, 1)] }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + + await expect( + program.parseAsync([ + 'node', + 'tdc', + 'thread', + 'mark-unread', + '500', + '501', + '--from', + '11', + ]), + ).rejects.toHaveProperty('code', 'CONFLICTING_OPTIONS') + expect(client.threads.markUnread).not.toHaveBeenCalled() + }) + + it('leaves a thread that is already unread at or before the target unchanged', async () => { + const client = createClient({ + comments: [createComment(11, 1)], + unreadThreads: [ + { threadId: '500', channelId: 'CH100', objIndex: 0, directMention: false }, + ], + }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'thread', 'mark-unread', '500', '--from', '11']) + + expect(client.threads.markUnread).not.toHaveBeenCalled() + expect(consoleSpy).toHaveBeenCalledWith( + 'Thread Test Thread (500) is already unread from comment 11.', + ) + }) + + it('still moves an unread thread further back when the target is earlier', async () => { + const client = createClient({ + unreadThreads: [ + { threadId: '500', channelId: 'CH100', objIndex: 1, directMention: false }, + ], + }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'thread', 'mark-unread', '500']) + + expect(client.threads.markUnread).toHaveBeenCalledWith({ id: '500', objIndex: -1 }) + }) + + it('shows dry run output', async () => { + const client = createClient() + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'thread', 'mark-unread', '500', '--dry-run']) + + expect(consoleSpy).toHaveBeenCalledWith( + 'Dry run: would mark unread thread Test Thread (500).', + ) + expect(client.threads.markUnread).not.toHaveBeenCalled() + }) + + it('outputs JSON with --json', async () => { + const client = createClient() + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'thread', 'mark-unread', '500', '--json']) + + const jsonOutput = JSON.parse(consoleSpy.mock.calls[0][0]) + expect(jsonOutput).toEqual([{ id: '500', isRead: false, lastReadObjIndex: -1 }]) + }) + + it('previews bulk refs from stdin and asks for --yes', async () => { + const client = createClient() + apiMocks.getCommsClient.mockResolvedValue(client) + vi.mocked(readStdinToEnd).mockResolvedValueOnce('# refs\n500\n501\n') + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'thread', 'mark-unread']) + + expect(client.threads.markUnread).not.toHaveBeenCalled() + expect(consoleSpy).toHaveBeenCalledWith('Would mark unread thread Test Thread (500).') + expect(consoleSpy).toHaveBeenCalledWith('Summary: 2 previews') + expect(consoleSpy).toHaveBeenCalledWith('Use --yes to confirm.') + }) + + it('marks bulk refs unread with --yes', async () => { + const client = createClient() + client.threads.getThread.mockImplementation(async (id: string) => createThreadFixture(id)) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'thread', 'mark-unread', '500', '501', '--yes']) + + expect(client.threads.markUnread).toHaveBeenCalledTimes(2) + expect(consoleSpy).toHaveBeenCalledWith('Summary: 2 changed threads') + }) + + it('errors when --json is used for bulk refs without --yes', async () => { + const client = createClient() + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + + await expect( + program.parseAsync(['node', 'tdc', 'thread', 'mark-unread', '500', '501', '--json']), + ).rejects.toHaveProperty('code', 'MISSING_YES_FLAG') + expect(client.threads.markUnread).not.toHaveBeenCalled() + }) + + it('surfaces markUnread failures through the shared error path', async () => { + const client = createClient() + client.threads.markUnread.mockRejectedValueOnce(new Error('mark failed')) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + + await expect( + program.parseAsync(['node', 'tdc', 'thread', 'mark-unread', '500']), + ).rejects.toThrow('mark failed') + }) +}) + describe('thread reply --file', () => { const files = useFileFixtures('tdc-reply-', 'diagram.png', 'report.pdf') diff --git a/src/commands/thread/unread.ts b/src/commands/thread/unread.ts new file mode 100644 index 0000000..4b6a183 --- /dev/null +++ b/src/commands/thread/unread.ts @@ -0,0 +1,144 @@ +import type { CommsApi } from '@doist/comms-sdk' +import { getCommsClient } from '../../lib/api.js' +import { CliError } from '../../lib/errors.js' +import type { MutationOptions } from '../../lib/options.js' +import { formatJson } from '../../lib/output.js' +import { resolveCommentId, resolveThreadId } from '../../lib/refs.js' +import { + collectThreadRefs, + loadThreadReadState, + printReadStateSummary, + type ReadStateTextStatus, + threadLabel, +} from './helpers.js' + +export type MarkThreadUnreadOptions = MutationOptions & { from?: string } + +type MarkUnreadStatus = { + id: string + isRead: false + /** Object index of the last comment still read; `-1` when the whole thread is unread. */ + lastReadObjIndex: number +} + +/** The API marks the thread body and every comment unread from `-1`. */ +const WHOLE_THREAD = -1 + +export async function markThreadUnread( + refs: string[], + options: MarkThreadUnreadOptions, +): Promise { + const rawRefs = await collectThreadRefs(refs) + if (rawRefs.length === 0) { + throw new CliError( + 'INVALID_REF', + 'No thread references provided. Pass refs as arguments or pipe them via stdin.', + ) + } + + if (options.from !== undefined && rawRefs.length > 1) { + throw new CliError( + 'CONFLICTING_OPTIONS', + '--from applies to a single thread; pass one thread ref when using it.', + ) + } + + const needsConfirmation = rawRefs.length > 1 && !options.yes && !options.dryRun + if (options.json && needsConfirmation) { + throw new CliError( + 'MISSING_YES_FLAG', + '--yes is required to execute bulk mark-unread in --json mode.', + ) + } + + const client = await getCommsClient() + const unreadCache = new Map>() + const jsonStatuses: MarkUnreadStatus[] = [] + const textStatuses: ReadStateTextStatus[] = [] + + for (const rawRef of rawRefs) { + const threadId = resolveThreadId(rawRef) + const loaded = await loadThreadReadState(client, unreadCache, threadId) + const target = + options.from === undefined + ? WHOLE_THREAD + : await resolveFromObjIndex(client, threadId, options.from) + const label = threadLabel(loaded.thread) + const scope = options.from === undefined ? '' : ` from comment ${options.from}` + + // Already unread at or before the target: nothing to move. + if (loaded.lastReadObjIndex !== null && loaded.lastReadObjIndex <= target) { + jsonStatuses.push({ + id: threadId, + isRead: false, + lastReadObjIndex: loaded.lastReadObjIndex, + }) + textStatuses.push('unchanged') + if (!options.json) { + console.log(`Thread ${label} is already unread${scope}.`) + } + continue + } + + if (needsConfirmation || options.dryRun) { + jsonStatuses.push({ id: threadId, isRead: false, lastReadObjIndex: target }) + textStatuses.push('preview') + if (!options.json) { + const prefix = options.dryRun ? 'Dry run: would' : 'Would' + console.log(`${prefix} mark unread thread ${label}${scope}.`) + } + continue + } + + await client.threads.markUnread({ id: threadId, objIndex: target }) + unreadCache.get(loaded.thread.workspaceId)?.set(threadId, target) + + jsonStatuses.push({ id: threadId, isRead: false, lastReadObjIndex: target }) + textStatuses.push('changed') + if (!options.json) { + console.log(`Thread ${label} marked unread${scope}.`) + } + } + + if (options.json && !options.dryRun) { + console.log(formatJson(jsonStatuses)) + return + } + + if (!options.json && rawRefs.length > 1) { + printReadStateSummary(textStatuses) + } + + if (!options.json && needsConfirmation) { + console.log('Use --yes to confirm.') + } +} + +/** + * Turns a comment ref into the `objIndex` to hand `markUnread`: the API takes + * the last comment that stays READ, so a comment at index N becomes the first + * unread one when the thread is marked unread from N - 1. + */ +async function resolveFromObjIndex( + client: CommsApi, + threadId: string, + commentRef: string, +): Promise { + const commentId = resolveCommentId(commentRef) + const comment = await client.comments.getComment(commentId) + + if (comment.threadId !== threadId) { + throw new CliError( + 'INVALID_REF', + `Comment ${commentId} belongs to thread ${comment.threadId}, not ${threadId}.`, + ) + } + if (typeof comment.objIndex !== 'number') { + throw new CliError( + 'INVALID_REF', + `Comment ${commentId} has no object index; cannot mark unread from it.`, + ) + } + + return comment.objIndex - 1 +} diff --git a/src/lib/api.ts b/src/lib/api.ts index c66aa71..c2aa04d 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -36,6 +36,7 @@ const API_SPINNER_MESSAGES: Record "content" --file ./a.png # Attach a file (repeatable; co tdc thread done # Preview thread archive (requires --yes to execute) tdc thread done --yes # Archive thread (mark done) tdc thread done --yes --json # Archive and return status as JSON +tdc thread undone --yes # Unarchive thread (move it back to your inbox); inverse of done +tdc thread undone --yes --json # Unarchive and return status as JSON tdc thread mark-read # Mark a thread read tdc thread mark-read --yes # Mark multiple threads read printf "id:CbT8n2Kp4Qx6Rz9Lm3Va\\nid:CbT9m4Qr7Vz2Nx8Lp5Sa\\n" | tdc thread mark-read --dry-run # Preview bulk mark-read from stdin +tdc thread mark-unread # Mark a whole thread unread; inverse of mark-read +tdc thread mark-unread --from # Mark unread from that comment onward (single thread only) +tdc thread mark-unread --yes # Mark multiple threads unread (also accepts refs on stdin) tdc thread mute # Mute thread for 60 minutes (default) tdc thread mute --minutes 480 # Mute for custom duration tdc thread mute --json # Mute and return { id, mutedUntil } as JSON @@ -193,6 +198,8 @@ tdc conversation reply "content" --file ./a.png # Attach a file (repeatab tdc conversation done # Preview conversation archive (requires --yes to execute) tdc conversation done --yes # Archive conversation tdc conversation done --yes --json # Archive and return status as JSON +tdc conversation undone --yes # Unarchive conversation; inverse of done +tdc conversation undone --yes --json # Unarchive and return status as JSON tdc conversation mute # Mute conversation for 60 minutes (default) tdc conversation mute --minutes 480 # Mute for custom duration tdc conversation mute --json # Mute and return { id, mutedUntil } as JSON @@ -450,7 +457,7 @@ echo "Quick reply" | tdc conversation reply If no content argument is provided and no stdin is piped, the CLI opens \`$EDITOR\` for interactive input. In non-TTY environments (e.g. when called by an agent or in a pipeline), the editor is automatically skipped and the command fails fast with an actionable error message. Use \`--non-interactive\` to force this behavior even in a TTY, or \`--interactive\` to override auto-detection. -\`tdc thread mark-read\` also accepts thread refs from stdin, one per line: +\`tdc thread mark-read\` and \`tdc thread mark-unread\` also accept thread refs from stdin, one per line: \`\`\`bash printf "id:CbT8n2Kp4Qx6Rz9Lm3Va\\nid:CbT9m4Qr7Vz2Nx8Lp5Sa\\n" | tdc thread mark-read --yes @@ -473,6 +480,7 @@ tdc inbox --unread --json tdc thread view --unread tdc thread reply "Thanks, I'll look into this." tdc thread done --yes +tdc thread undone --yes # Changed your mind: back to the inbox \`\`\` **Search and review:** From 9959a58eaf8e8d100b9fa9eda070cd60e7e45a1c Mon Sep 17 00:00:00 2001 From: lmjabreu Date: Thu, 17 Sep 2026 19:46:05 +0100 Subject: [PATCH 2/5] refactor: share the mark-read/mark-unread loop and emit JSON on dry run Address the doistbot pass on #64. The bulk-ref loop now lives once in thread/helpers.ts as runThreadReadStateMutation, with each verb supplying a per-thread plan; the plan runs before the unread lookup so a bad --from ref fails without a workspace-wide request. Both verbs now print their statuses under --json --dry-run instead of nothing (mark-read had the same gap). getLatestObjIndex goes back to being private to read.ts, done/undone use threadLabel, and the bulk tests assert per-ref calls with the stdin case pinning isTTY. Co-Authored-By: Claude Opus 5 --- src/commands/thread/helpers.ts | 135 +++++++++++++++++++++++++---- src/commands/thread/mutate.ts | 5 +- src/commands/thread/read.ts | 104 +++++----------------- src/commands/thread/thread.test.ts | 102 ++++++++++++++++++++-- src/commands/thread/unread.ts | 115 ++++++------------------ 5 files changed, 265 insertions(+), 196 deletions(-) diff --git a/src/commands/thread/helpers.ts b/src/commands/thread/helpers.ts index fe22506..d879de2 100644 --- a/src/commands/thread/helpers.ts +++ b/src/commands/thread/helpers.ts @@ -1,13 +1,15 @@ import type { CommsApi, Thread } from '@doist/comms-sdk' import chalk from 'chalk' -import { getWorkspaceGroups, getWorkspaceUsers } from '../../lib/api.js' +import { getCommsClient, getWorkspaceGroups, getWorkspaceUsers } from '../../lib/api.js' import { formatRelativeDate } from '../../lib/dates.js' +import { CliError } from '../../lib/errors.js' import { isAccessible } from '../../lib/global-args.js' import { readStdinToEnd } from '../../lib/input.js' import { renderMarkdown } from '../../lib/markdown.js' -import { colors, pluralize } from '../../lib/output.js' +import type { MutationOptions } from '../../lib/options.js' +import { colors, formatJson, pluralize } from '../../lib/output.js' import { assertChannelIsPublic } from '../../lib/public-channels.js' -import { partitionNotifyIds } from '../../lib/refs.js' +import { partitionNotifyIds, resolveThreadId } from '../../lib/refs.js' export function printSeparator(label: string): void { const totalWidth = 60 @@ -79,8 +81,8 @@ export function formatNotifyLabel(items: NamedEntity[]): string { return items.map((i) => `${i.name} (${i.id})`).join(', ') } -// Shared by `mark-read` and `mark-unread`: bulk ref collection, the per-workspace -// unread lookup, and the text summary. +// Shared by `mark-read` and `mark-unread`: the bulk-ref loop, the per-workspace +// unread lookup, confirmation, and output. Each verb supplies a per-thread plan. export type ReadStateTextStatus = 'changed' | 'preview' | 'unchanged' @@ -94,7 +96,112 @@ export type ThreadReadState = { lastReadObjIndex: number | null } -export async function collectThreadRefs(refs: string[]): Promise { +export type ReadStatePlan = { + /** Appended to messages, e.g. `' from comment X'`; empty for the whole thread. */ + scope: string + isUnchanged(state: ThreadReadState): boolean + /** JSON row; `outcome` is `'unchanged'` when nothing needs to move. */ + status(state: ThreadReadState, outcome: 'planned' | 'unchanged'): Status + /** Performs the mutation and returns the thread's new `lastReadObjIndex`. */ + apply(client: CommsApi, state: ThreadReadState): Promise +} + +export type ReadStateMutation = { + verb: 'read' | 'unread' + /** + * Builds the per-thread plan. Runs before the unread lookup so an invalid + * option (a bad `--from` ref) fails without a workspace-wide request. + */ + plan(client: CommsApi, threadId: string): Promise> +} + +export async function runThreadReadStateMutation( + refs: string[], + options: MutationOptions, + mutation: ReadStateMutation, +): Promise { + const rawRefs = await collectThreadRefs(refs) + if (rawRefs.length === 0) { + throw new CliError( + 'INVALID_REF', + 'No thread references provided. Pass refs as arguments or pipe them via stdin.', + ) + } + + const needsConfirmation = rawRefs.length > 1 && !options.yes && !options.dryRun + if (options.json && needsConfirmation) { + throw new CliError( + 'MISSING_YES_FLAG', + `--yes is required to execute bulk mark-${mutation.verb} in --json mode.`, + ) + } + + const client = await getCommsClient() + const unreadCache = new Map>() + const jsonStatuses: Status[] = [] + const textStatuses: ReadStateTextStatus[] = [] + + for (const rawRef of rawRefs) { + const threadId = resolveThreadId(rawRef) + const plan = await mutation.plan(client, threadId) + const state = await loadThreadReadState(client, unreadCache, threadId) + const label = threadLabel(state.thread) + + if (plan.isUnchanged(state)) { + jsonStatuses.push(plan.status(state, 'unchanged')) + textStatuses.push('unchanged') + if (!options.json) { + console.log(`Thread ${label} is already ${mutation.verb}${plan.scope}.`) + } + continue + } + + if (needsConfirmation || options.dryRun) { + jsonStatuses.push(plan.status(state, 'planned')) + textStatuses.push('preview') + if (!options.json) { + const prefix = options.dryRun ? 'Dry run: would' : 'Would' + console.log(`${prefix} mark ${mutation.verb} thread ${label}${plan.scope}.`) + } + continue + } + + const lastReadObjIndex = await plan.apply(client, state) + const unreadByThread = unreadCache.get(state.thread.workspaceId) + if (lastReadObjIndex === null) { + unreadByThread?.delete(threadId) + } else { + unreadByThread?.set(threadId, lastReadObjIndex) + } + + jsonStatuses.push(plan.status(state, 'planned')) + textStatuses.push('changed') + if (!options.json) { + console.log(`Thread ${label} marked ${mutation.verb}${plan.scope}.`) + } + } + + if (options.json) { + console.log( + formatJson( + options.dryRun + ? jsonStatuses.map((status) => ({ ...status, dryRun: true })) + : jsonStatuses, + ), + ) + return + } + + if (rawRefs.length > 1) { + printReadStateSummary(textStatuses) + } + + if (needsConfirmation) { + console.log('Use --yes to confirm.') + } +} + +async function collectThreadRefs(refs: string[]): Promise { const inlineRefs = refs.map((ref) => ref.trim()).filter(Boolean) const stdinContent = await readStdinToEnd() @@ -111,9 +218,9 @@ export async function collectThreadRefs(refs: string[]): Promise { /** * Loads a thread and its unread position. `unreadCache` maps a workspace id to * its unread threads (`threadId` -> last read `objIndex`) so bulk runs fetch - * the unread list once per workspace; callers update it after mutating. + * the unread list once per workspace. */ -export async function loadThreadReadState( +async function loadThreadReadState( client: CommsApi, unreadCache: Map>, threadId: string, @@ -133,19 +240,11 @@ export async function loadThreadReadState( return { thread, lastReadObjIndex: unreadByThread.get(thread.id) ?? null } } -export function getLatestObjIndex(thread: Thread): number { - return Math.max( - ...[thread.lastComment?.objIndex, thread.lastObjIndex, thread.commentCount, 0] - .filter((value): value is number => typeof value === 'number') - .map((value) => Math.max(value, 0)), - ) -} - -export function threadLabel(thread: Thread): string { +export function threadLabel(thread: Pick): string { return `${thread.title} (${thread.id})` } -export function printReadStateSummary(statuses: ReadStateTextStatus[]): void { +function printReadStateSummary(statuses: ReadStateTextStatus[]): void { const summary = [ summarizeStatus(statuses, 'changed'), summarizeStatus(statuses, 'unchanged'), diff --git a/src/commands/thread/mutate.ts b/src/commands/thread/mutate.ts index 3733e4d..0729a74 100644 --- a/src/commands/thread/mutate.ts +++ b/src/commands/thread/mutate.ts @@ -4,6 +4,7 @@ import type { MutationOptions } from '../../lib/options.js' import { formatJson, printDryRun } from '../../lib/output.js' import { assertChannelIsPublic } from '../../lib/public-channels.js' import { resolveThreadId } from '../../lib/refs.js' +import { threadLabel } from './helpers.js' export async function markThreadDone(ref: string, options: MutationOptions): Promise { const threadId = resolveThreadId(ref) @@ -14,7 +15,7 @@ export async function markThreadDone(ref: string, options: MutationOptions): Pro if (options.dryRun) { printDryRun('archive thread', { - Thread: `${thread.title} (${threadId})`, + Thread: threadLabel(thread), }) return } @@ -50,7 +51,7 @@ export async function markThreadUndone(ref: string, options: MutationOptions): P if (options.dryRun) { printDryRun('unarchive thread', { - Thread: `${thread.title} (${threadId})`, + Thread: threadLabel(thread), Status: thread.isArchived ? undefined : 'already in inbox', }) return diff --git a/src/commands/thread/read.ts b/src/commands/thread/read.ts index 28c5ab3..5648192 100644 --- a/src/commands/thread/read.ts +++ b/src/commands/thread/read.ts @@ -1,16 +1,6 @@ -import { getCommsClient } from '../../lib/api.js' -import { CliError } from '../../lib/errors.js' +import type { Thread } from '@doist/comms-sdk' import type { MutationOptions } from '../../lib/options.js' -import { formatJson } from '../../lib/output.js' -import { resolveThreadId } from '../../lib/refs.js' -import { - collectThreadRefs, - getLatestObjIndex, - loadThreadReadState, - printReadStateSummary, - type ReadStateTextStatus, - threadLabel, -} from './helpers.js' +import { runThreadReadStateMutation } from './helpers.js' export type MarkThreadReadOptions = MutationOptions @@ -23,73 +13,27 @@ export async function markThreadRead( refs: string[], options: MarkThreadReadOptions, ): Promise { - const rawRefs = await collectThreadRefs(refs) - if (rawRefs.length === 0) { - throw new CliError( - 'INVALID_REF', - 'No thread references provided. Pass refs as arguments or pipe them via stdin.', - ) - } - - const needsConfirmation = rawRefs.length > 1 && !options.yes && !options.dryRun - if (options.json && needsConfirmation) { - throw new CliError( - 'MISSING_YES_FLAG', - '--yes is required to execute bulk mark-read in --json mode.', - ) - } - - const client = await getCommsClient() - const unreadCache = new Map>() - const jsonStatuses: MarkReadStatus[] = [] - const textStatuses: ReadStateTextStatus[] = [] - - for (const rawRef of rawRefs) { - const threadId = resolveThreadId(rawRef) - const loaded = await loadThreadReadState(client, unreadCache, threadId) - - if (loaded.lastReadObjIndex === null) { - jsonStatuses.push({ id: threadId, isRead: true }) - textStatuses.push('unchanged') - if (!options.json) { - console.log(`Thread ${threadLabel(loaded.thread)} is already read.`) - } - continue - } - - if (needsConfirmation || options.dryRun) { - jsonStatuses.push({ id: threadId, isRead: true }) - textStatuses.push('preview') - if (!options.json) { - const prefix = options.dryRun ? 'Dry run: would' : 'Would' - console.log(`${prefix} mark read thread ${threadLabel(loaded.thread)}.`) - } - continue - } - - await client.threads.markRead({ - id: threadId, - objIndex: getLatestObjIndex(loaded.thread), - }) - unreadCache.get(loaded.thread.workspaceId)?.delete(threadId) - - jsonStatuses.push({ id: threadId, isRead: true }) - textStatuses.push('changed') - if (!options.json) { - console.log(`Thread ${threadLabel(loaded.thread)} marked read.`) - } - } - - if (options.json && !options.dryRun) { - console.log(formatJson(jsonStatuses)) - return - } - - if (!options.json && rawRefs.length > 1) { - printReadStateSummary(textStatuses) - } + await runThreadReadStateMutation(refs, options, { + verb: 'read', + plan: async (_client, threadId) => ({ + scope: '', + isUnchanged: (state) => state.lastReadObjIndex === null, + status: () => ({ id: threadId, isRead: true }), + apply: async (client, state) => { + await client.threads.markRead({ + id: threadId, + objIndex: getLatestObjIndex(state.thread), + }) + return null + }, + }), + }) +} - if (!options.json && needsConfirmation) { - console.log('Use --yes to confirm.') - } +function getLatestObjIndex(thread: Thread): number { + return Math.max( + ...[thread.lastComment?.objIndex, thread.lastObjIndex, thread.commentCount, 0] + .filter((value): value is number => typeof value === 'number') + .map((value) => Math.max(value, 0)), + ) } diff --git a/src/commands/thread/thread.test.ts b/src/commands/thread/thread.test.ts index 0e06d92..5d4d635 100644 --- a/src/commands/thread/thread.test.ts +++ b/src/commands/thread/thread.test.ts @@ -1279,6 +1279,32 @@ describe('thread read', () => { expect(jsonOutput).toEqual([{ id: '500', isRead: true }]) }) + it('emits the planned statuses with --json --dry-run', async () => { + const client = createClient({ + unreadThreads: [ + { threadId: '500', channelId: 'CH100', objIndex: 1, directMention: false }, + ], + }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync([ + 'node', + 'tdc', + 'thread', + 'mark-read', + '500', + '--json', + '--dry-run', + ]) + + expect(client.threads.markRead).not.toHaveBeenCalled() + const jsonOutput = JSON.parse(consoleSpy.mock.calls[0][0]) + expect(jsonOutput).toEqual([{ id: '500', isRead: true, dryRun: true }]) + }) + it('runs validation in dry-run mode', async () => { const client = createClient() apiMocks.getCommsClient.mockResolvedValue(client) @@ -2059,19 +2085,34 @@ describe('thread mark-unread', () => { }) it('previews bulk refs from stdin and asks for --yes', async () => { - const client = createClient() - apiMocks.getCommsClient.mockResolvedValue(client) vi.mocked(readStdinToEnd).mockResolvedValueOnce('# refs\n500\n501\n') + const originalIsTTY = process.stdin.isTTY + Object.defineProperty(process.stdin, 'isTTY', { value: false, configurable: true }) - const program = createProgram() - const consoleSpy = captureConsole('log') + try { + const client = createClient() + client.threads.getThread.mockImplementation(async (id: string) => + createThreadFixture(id), + ) + apiMocks.getCommsClient.mockResolvedValue(client) - await program.parseAsync(['node', 'tdc', 'thread', 'mark-unread']) + const program = createProgram() + const consoleSpy = captureConsole('log') - expect(client.threads.markUnread).not.toHaveBeenCalled() - expect(consoleSpy).toHaveBeenCalledWith('Would mark unread thread Test Thread (500).') - expect(consoleSpy).toHaveBeenCalledWith('Summary: 2 previews') - expect(consoleSpy).toHaveBeenCalledWith('Use --yes to confirm.') + await program.parseAsync(['node', 'tdc', 'thread', 'mark-unread']) + + expect(readStdinToEnd).toHaveBeenCalled() + expect(client.threads.markUnread).not.toHaveBeenCalled() + expect(consoleSpy).toHaveBeenCalledWith('Would mark unread thread Test Thread (500).') + expect(consoleSpy).toHaveBeenCalledWith('Would mark unread thread Test Thread (501).') + expect(consoleSpy).toHaveBeenCalledWith('Summary: 2 previews') + expect(consoleSpy).toHaveBeenCalledWith('Use --yes to confirm.') + } finally { + Object.defineProperty(process.stdin, 'isTTY', { + value: originalIsTTY, + configurable: true, + }) + } }) it('marks bulk refs unread with --yes', async () => { @@ -2084,10 +2125,53 @@ describe('thread mark-unread', () => { await program.parseAsync(['node', 'tdc', 'thread', 'mark-unread', '500', '501', '--yes']) + expect(client.threads.markUnread).toHaveBeenCalledWith({ id: '500', objIndex: -1 }) + expect(client.threads.markUnread).toHaveBeenCalledWith({ id: '501', objIndex: -1 }) expect(client.threads.markUnread).toHaveBeenCalledTimes(2) + expect(consoleSpy).toHaveBeenCalledWith('Thread Test Thread (500) marked unread.') + expect(consoleSpy).toHaveBeenCalledWith('Thread Test Thread (501) marked unread.') expect(consoleSpy).toHaveBeenCalledWith('Summary: 2 changed threads') }) + it('emits the planned statuses with --json --dry-run', async () => { + const client = createClient() + client.threads.getThread.mockImplementation(async (id: string) => createThreadFixture(id)) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync([ + 'node', + 'tdc', + 'thread', + 'mark-unread', + '500', + '501', + '--json', + '--dry-run', + ]) + + expect(client.threads.markUnread).not.toHaveBeenCalled() + const jsonOutput = JSON.parse(consoleSpy.mock.calls[0][0]) + expect(jsonOutput).toEqual([ + { id: '500', isRead: false, lastReadObjIndex: -1, dryRun: true }, + { id: '501', isRead: false, lastReadObjIndex: -1, dryRun: true }, + ]) + }) + + it('rejects an invalid --from comment before loading the unread list', async () => { + const client = createClient({ comments: [{ ...createComment(11, 1), threadId: '999' }] }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + + await expect( + program.parseAsync(['node', 'tdc', 'thread', 'mark-unread', '500', '--from', '11']), + ).rejects.toHaveProperty('code', 'INVALID_REF') + expect(client.threads.getUnread).not.toHaveBeenCalled() + }) + it('errors when --json is used for bulk refs without --yes', async () => { const client = createClient() apiMocks.getCommsClient.mockResolvedValue(client) diff --git a/src/commands/thread/unread.ts b/src/commands/thread/unread.ts index 4b6a183..c601899 100644 --- a/src/commands/thread/unread.ts +++ b/src/commands/thread/unread.ts @@ -1,16 +1,8 @@ import type { CommsApi } from '@doist/comms-sdk' -import { getCommsClient } from '../../lib/api.js' import { CliError } from '../../lib/errors.js' import type { MutationOptions } from '../../lib/options.js' -import { formatJson } from '../../lib/output.js' -import { resolveCommentId, resolveThreadId } from '../../lib/refs.js' -import { - collectThreadRefs, - loadThreadReadState, - printReadStateSummary, - type ReadStateTextStatus, - threadLabel, -} from './helpers.js' +import { resolveCommentId } from '../../lib/refs.js' +import { runThreadReadStateMutation } from './helpers.js' export type MarkThreadUnreadOptions = MutationOptions & { from?: string } @@ -28,90 +20,39 @@ export async function markThreadUnread( refs: string[], options: MarkThreadUnreadOptions, ): Promise { - const rawRefs = await collectThreadRefs(refs) - if (rawRefs.length === 0) { - throw new CliError( - 'INVALID_REF', - 'No thread references provided. Pass refs as arguments or pipe them via stdin.', - ) - } - - if (options.from !== undefined && rawRefs.length > 1) { + const from = options.from + if (from !== undefined && refs.length > 1) { throw new CliError( 'CONFLICTING_OPTIONS', '--from applies to a single thread; pass one thread ref when using it.', ) } - const needsConfirmation = rawRefs.length > 1 && !options.yes && !options.dryRun - if (options.json && needsConfirmation) { - throw new CliError( - 'MISSING_YES_FLAG', - '--yes is required to execute bulk mark-unread in --json mode.', - ) - } - - const client = await getCommsClient() - const unreadCache = new Map>() - const jsonStatuses: MarkUnreadStatus[] = [] - const textStatuses: ReadStateTextStatus[] = [] - - for (const rawRef of rawRefs) { - const threadId = resolveThreadId(rawRef) - const loaded = await loadThreadReadState(client, unreadCache, threadId) - const target = - options.from === undefined - ? WHOLE_THREAD - : await resolveFromObjIndex(client, threadId, options.from) - const label = threadLabel(loaded.thread) - const scope = options.from === undefined ? '' : ` from comment ${options.from}` - - // Already unread at or before the target: nothing to move. - if (loaded.lastReadObjIndex !== null && loaded.lastReadObjIndex <= target) { - jsonStatuses.push({ - id: threadId, - isRead: false, - lastReadObjIndex: loaded.lastReadObjIndex, - }) - textStatuses.push('unchanged') - if (!options.json) { - console.log(`Thread ${label} is already unread${scope}.`) + await runThreadReadStateMutation(refs, options, { + verb: 'unread', + plan: async (client, threadId) => { + const target = + from === undefined + ? WHOLE_THREAD + : await resolveFromObjIndex(client, threadId, from) + return { + scope: from === undefined ? '' : ` from comment ${from}`, + // Already unread at or before the target: nothing to move. + isUnchanged: (state) => + state.lastReadObjIndex !== null && state.lastReadObjIndex <= target, + status: (state, outcome) => ({ + id: threadId, + isRead: false, + lastReadObjIndex: + outcome === 'unchanged' ? (state.lastReadObjIndex ?? target) : target, + }), + apply: async (client) => { + await client.threads.markUnread({ id: threadId, objIndex: target }) + return target + }, } - continue - } - - if (needsConfirmation || options.dryRun) { - jsonStatuses.push({ id: threadId, isRead: false, lastReadObjIndex: target }) - textStatuses.push('preview') - if (!options.json) { - const prefix = options.dryRun ? 'Dry run: would' : 'Would' - console.log(`${prefix} mark unread thread ${label}${scope}.`) - } - continue - } - - await client.threads.markUnread({ id: threadId, objIndex: target }) - unreadCache.get(loaded.thread.workspaceId)?.set(threadId, target) - - jsonStatuses.push({ id: threadId, isRead: false, lastReadObjIndex: target }) - textStatuses.push('changed') - if (!options.json) { - console.log(`Thread ${label} marked unread${scope}.`) - } - } - - if (options.json && !options.dryRun) { - console.log(formatJson(jsonStatuses)) - return - } - - if (!options.json && rawRefs.length > 1) { - printReadStateSummary(textStatuses) - } - - if (!options.json && needsConfirmation) { - console.log('Use --yes to confirm.') - } + }, + }) } /** From 92c38d38a3bd5d71dbce159e9d5ae8e1f04678d4 Mon Sep 17 00:00:00 2001 From: lmjabreu Date: Thu, 17 Sep 2026 20:48:55 +0100 Subject: [PATCH 3/5] fix: count stdin refs in the mark-unread --from guard The single-thread check ran on positional refs only, so refs piped on stdin bypassed it and the first thread was mutated before the second failed. The driver now takes a validateRefs hook that sees the merged list. Also from the second doistbot pass: done/undone share one setThreadArchiveState / setConversationArchiveState like the channel pair, the confirmation line uses the same Title (id) label as dry run, and the tests cover the earlier-unread no-op and the unchanged JSON row. Co-Authored-By: Claude Opus 5 --- src/commands/conversation/archive.ts | 59 ++++++++++++++++++++ src/commands/conversation/done.ts | 41 -------------- src/commands/conversation/index.ts | 3 +- src/commands/conversation/undone.ts | 41 -------------- src/commands/thread/helpers.ts | 3 ++ src/commands/thread/mutate.ts | 63 ++++++++-------------- src/commands/thread/thread.test.ts | 80 ++++++++++++++++++++++------ src/commands/thread/unread.ts | 14 ++--- 8 files changed, 157 insertions(+), 147 deletions(-) create mode 100644 src/commands/conversation/archive.ts delete mode 100644 src/commands/conversation/done.ts delete mode 100644 src/commands/conversation/undone.ts diff --git a/src/commands/conversation/archive.ts b/src/commands/conversation/archive.ts new file mode 100644 index 0000000..1af7a3c --- /dev/null +++ b/src/commands/conversation/archive.ts @@ -0,0 +1,59 @@ +import { getCommsClient } from '../../lib/api.js' +import { CliError } from '../../lib/errors.js' +import { formatJson, printDryRun } from '../../lib/output.js' +import { resolveConversationId } from '../../lib/refs.js' +import { conversationLabel, type DoneOptions } from './helpers.js' + +async function setConversationArchiveState( + ref: string, + options: DoneOptions, + archive: boolean, +): Promise { + const action = archive ? 'archive' : 'unarchive' + const conversationId = resolveConversationId(ref) + + const client = await getCommsClient() + const conversation = await client.conversations.getConversation(conversationId) + + if (options.dryRun) { + const noop = conversation.archived === archive + printDryRun(`${action} conversation`, { + Conversation: conversationLabel(conversation), + Status: noop ? (archive ? 'already archived' : 'not archived') : undefined, + }) + return + } + + if (!options.yes) { + if (options.json) { + throw new CliError( + 'MISSING_YES_FLAG', + `--yes is required to execute ${action} in --json mode.`, + ) + } + console.log(`Would ${action}: ${conversationLabel(conversation)}`) + console.log('Use --yes to confirm.') + return + } + + if (archive) { + await client.conversations.archiveConversation(conversationId) + } else { + await client.conversations.unarchiveConversation(conversationId) + } + + if (options.json) { + console.log(formatJson({ id: conversationId, archived: archive })) + return + } + + console.log(`Conversation ${conversationId} ${action}d.`) +} + +export async function markConversationDone(ref: string, options: DoneOptions): Promise { + await setConversationArchiveState(ref, options, true) +} + +export async function markConversationUndone(ref: string, options: DoneOptions): Promise { + await setConversationArchiveState(ref, options, false) +} diff --git a/src/commands/conversation/done.ts b/src/commands/conversation/done.ts deleted file mode 100644 index 648bafd..0000000 --- a/src/commands/conversation/done.ts +++ /dev/null @@ -1,41 +0,0 @@ -import { getCommsClient } from '../../lib/api.js' -import { CliError } from '../../lib/errors.js' -import { formatJson, printDryRun } from '../../lib/output.js' -import { resolveConversationId } from '../../lib/refs.js' -import { conversationLabel, type DoneOptions } from './helpers.js' - -export async function markConversationDone(ref: string, options: DoneOptions): Promise { - const conversationId = resolveConversationId(ref) - - const client = await getCommsClient() - const conversation = await client.conversations.getConversation(conversationId) - - if (options.dryRun) { - printDryRun('archive conversation', { - Conversation: conversationLabel(conversation), - Status: conversation.archived ? 'already archived' : undefined, - }) - return - } - - if (!options.yes) { - if (options.json) { - throw new CliError( - 'MISSING_YES_FLAG', - '--yes is required to execute archive in --json mode.', - ) - } - console.log(`Would archive: ${conversationLabel(conversation)}`) - console.log('Use --yes to confirm.') - return - } - - await client.conversations.archiveConversation(conversationId) - - if (options.json) { - console.log(formatJson({ id: conversationId, archived: true })) - return - } - - console.log(`Conversation ${conversationId} archived.`) -} diff --git a/src/commands/conversation/index.ts b/src/commands/conversation/index.ts index 2365030..86f0a59 100644 --- a/src/commands/conversation/index.ts +++ b/src/commands/conversation/index.ts @@ -1,11 +1,10 @@ import { Command, Option } from 'commander' import { withCaseInsensitiveChoices } from '../../lib/completion.js' import { collect } from '../../lib/options.js' -import { markConversationDone } from './done.js' +import { markConversationDone, markConversationUndone } from './archive.js' import { listConversations } from './list.js' import { muteConversation } from './mute.js' import { replyToConversation } from './reply.js' -import { markConversationUndone } from './undone.js' import { unmuteConversation } from './unmute.js' import { showUnread } from './unread.js' import { viewConversation } from './view.js' diff --git a/src/commands/conversation/undone.ts b/src/commands/conversation/undone.ts deleted file mode 100644 index caf1cdd..0000000 --- a/src/commands/conversation/undone.ts +++ /dev/null @@ -1,41 +0,0 @@ -import { getCommsClient } from '../../lib/api.js' -import { CliError } from '../../lib/errors.js' -import { formatJson, printDryRun } from '../../lib/output.js' -import { resolveConversationId } from '../../lib/refs.js' -import { conversationLabel, type DoneOptions } from './helpers.js' - -export async function markConversationUndone(ref: string, options: DoneOptions): Promise { - const conversationId = resolveConversationId(ref) - - const client = await getCommsClient() - const conversation = await client.conversations.getConversation(conversationId) - - if (options.dryRun) { - printDryRun('unarchive conversation', { - Conversation: conversationLabel(conversation), - Status: conversation.archived ? undefined : 'not archived', - }) - return - } - - if (!options.yes) { - if (options.json) { - throw new CliError( - 'MISSING_YES_FLAG', - '--yes is required to execute unarchive in --json mode.', - ) - } - console.log(`Would unarchive: ${conversationLabel(conversation)}`) - console.log('Use --yes to confirm.') - return - } - - await client.conversations.unarchiveConversation(conversationId) - - if (options.json) { - console.log(formatJson({ id: conversationId, archived: false })) - return - } - - console.log(`Conversation ${conversationId} unarchived.`) -} diff --git a/src/commands/thread/helpers.ts b/src/commands/thread/helpers.ts index d879de2..89a986a 100644 --- a/src/commands/thread/helpers.ts +++ b/src/commands/thread/helpers.ts @@ -108,6 +108,8 @@ export type ReadStatePlan = { export type ReadStateMutation = { verb: 'read' | 'unread' + /** Runs on the full ref list (positional plus stdin) before anything is loaded. */ + validateRefs?(rawRefs: string[]): void /** * Builds the per-thread plan. Runs before the unread lookup so an invalid * option (a bad `--from` ref) fails without a workspace-wide request. @@ -127,6 +129,7 @@ export async function runThreadReadStateMutation( 'No thread references provided. Pass refs as arguments or pipe them via stdin.', ) } + mutation.validateRefs?.(rawRefs) const needsConfirmation = rawRefs.length > 1 && !options.yes && !options.dryRun if (options.json && needsConfirmation) { diff --git a/src/commands/thread/mutate.ts b/src/commands/thread/mutate.ts index 0729a74..240ae42 100644 --- a/src/commands/thread/mutate.ts +++ b/src/commands/thread/mutate.ts @@ -6,7 +6,12 @@ import { assertChannelIsPublic } from '../../lib/public-channels.js' import { resolveThreadId } from '../../lib/refs.js' import { threadLabel } from './helpers.js' -export async function markThreadDone(ref: string, options: MutationOptions): Promise { +async function setThreadArchiveState( + ref: string, + options: MutationOptions, + archive: boolean, +): Promise { + const action = archive ? 'archive' : 'unarchive' const threadId = resolveThreadId(ref) const client = await getCommsClient() @@ -14,8 +19,9 @@ export async function markThreadDone(ref: string, options: MutationOptions): Pro await assertChannelIsPublic(thread.channelId, thread.workspaceId) if (options.dryRun) { - printDryRun('archive thread', { + printDryRun(`${action} thread`, { Thread: threadLabel(thread), + Status: !archive && !thread.isArchived ? 'already in inbox' : undefined, }) return } @@ -24,57 +30,32 @@ export async function markThreadDone(ref: string, options: MutationOptions): Pro if (options.json) { throw new CliError( 'MISSING_YES_FLAG', - '--yes is required to execute archive in --json mode.', + `--yes is required to execute ${action} in --json mode.`, ) } - console.log(`Would archive: ${thread.title}`) + console.log(`Would ${action}: ${threadLabel(thread)}`) console.log('Use --yes to confirm.') return } - await client.inbox.archiveThread(threadId) + if (archive) { + await client.inbox.archiveThread(threadId) + } else { + await client.inbox.unarchiveThread(threadId) + } if (options.json) { - console.log(formatJson({ id: threadId, isArchived: true })) + console.log(formatJson({ id: threadId, isArchived: archive })) return } - console.log(`Thread ${threadId} archived.`) + console.log(`Thread ${threadId} ${action}d.`) } -export async function markThreadUndone(ref: string, options: MutationOptions): Promise { - const threadId = resolveThreadId(ref) - - const client = await getCommsClient() - const thread = await client.threads.getThread(threadId) - await assertChannelIsPublic(thread.channelId, thread.workspaceId) - - if (options.dryRun) { - printDryRun('unarchive thread', { - Thread: threadLabel(thread), - Status: thread.isArchived ? undefined : 'already in inbox', - }) - return - } - - if (!options.yes) { - if (options.json) { - throw new CliError( - 'MISSING_YES_FLAG', - '--yes is required to execute unarchive in --json mode.', - ) - } - console.log(`Would unarchive: ${thread.title}`) - console.log('Use --yes to confirm.') - return - } - - await client.inbox.unarchiveThread(threadId) - - if (options.json) { - console.log(formatJson({ id: threadId, isArchived: false })) - return - } +export async function markThreadDone(ref: string, options: MutationOptions): Promise { + await setThreadArchiveState(ref, options, true) +} - console.log(`Thread ${threadId} unarchived.`) +export async function markThreadUndone(ref: string, options: MutationOptions): Promise { + await setThreadArchiveState(ref, options, false) } diff --git a/src/commands/thread/thread.test.ts b/src/commands/thread/thread.test.ts index 5d4d635..47d26a9 100644 --- a/src/commands/thread/thread.test.ts +++ b/src/commands/thread/thread.test.ts @@ -1803,7 +1803,7 @@ describe('thread done', () => { await program.parseAsync(['node', 'tdc', 'thread', 'done', '500']) - expect(consoleSpy).toHaveBeenCalledWith('Would archive: Test Thread') + expect(consoleSpy).toHaveBeenCalledWith('Would archive: Test Thread (500)') expect(consoleSpy).toHaveBeenCalledWith('Use --yes to confirm.') expect(client.inbox.archiveThread).not.toHaveBeenCalled() }) @@ -1890,7 +1890,7 @@ describe('thread undone', () => { await program.parseAsync(['node', 'tdc', 'thread', 'undone', '500']) - expect(consoleSpy).toHaveBeenCalledWith('Would unarchive: Test Thread') + expect(consoleSpy).toHaveBeenCalledWith('Would unarchive: Test Thread (500)') expect(consoleSpy).toHaveBeenCalledWith('Use --yes to confirm.') expect(client.inbox.unarchiveThread).not.toHaveBeenCalled() }) @@ -1997,6 +1997,8 @@ describe('thread mark-unread', () => { program.parseAsync(['node', 'tdc', 'thread', 'mark-unread', '500', '--from', '11']), ).rejects.toHaveProperty('code', 'INVALID_REF') expect(client.threads.markUnread).not.toHaveBeenCalled() + // The comment is validated before the workspace-wide unread lookup. + expect(client.threads.getUnread).not.toHaveBeenCalled() }) it('rejects --from with more than one thread ref', async () => { @@ -2020,11 +2022,41 @@ describe('thread mark-unread', () => { expect(client.threads.markUnread).not.toHaveBeenCalled() }) - it('leaves a thread that is already unread at or before the target unchanged', async () => { + it('rejects --from when stdin adds a second thread ref', async () => { + vi.mocked(readStdinToEnd).mockResolvedValueOnce('501\n') + const client = createClient({ comments: [createComment(11, 1)] }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + + await expect( + program.parseAsync([ + 'node', + 'tdc', + 'thread', + 'mark-unread', + '500', + '--from', + '11', + '--yes', + ]), + ).rejects.toHaveProperty('code', 'CONFLICTING_OPTIONS') + expect(client.threads.markUnread).not.toHaveBeenCalled() + }) + + it.each([ + ['at the target', 0], + ['before the target', -1], + ])('leaves a thread already unread %s unchanged', async (_case, lastReadObjIndex) => { const client = createClient({ comments: [createComment(11, 1)], unreadThreads: [ - { threadId: '500', channelId: 'CH100', objIndex: 0, directMention: false }, + { + threadId: '500', + channelId: 'CH100', + objIndex: lastReadObjIndex, + directMention: false, + }, ], }) apiMocks.getCommsClient.mockResolvedValue(client) @@ -2040,6 +2072,34 @@ describe('thread mark-unread', () => { ) }) + it('reports the current read position in JSON when nothing changes', async () => { + const client = createClient({ + comments: [createComment(12, 2)], + unreadThreads: [ + { threadId: '500', channelId: 'CH100', objIndex: 0, directMention: false }, + ], + }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync([ + 'node', + 'tdc', + 'thread', + 'mark-unread', + '500', + '--from', + '12', + '--json', + ]) + + expect(client.threads.markUnread).not.toHaveBeenCalled() + const jsonOutput = JSON.parse(consoleSpy.mock.calls[0][0]) + expect(jsonOutput).toEqual([{ id: '500', isRead: false, lastReadObjIndex: 0 }]) + }) + it('still moves an unread thread further back when the target is earlier', async () => { const client = createClient({ unreadThreads: [ @@ -2160,18 +2220,6 @@ describe('thread mark-unread', () => { ]) }) - it('rejects an invalid --from comment before loading the unread list', async () => { - const client = createClient({ comments: [{ ...createComment(11, 1), threadId: '999' }] }) - apiMocks.getCommsClient.mockResolvedValue(client) - - const program = createProgram() - - await expect( - program.parseAsync(['node', 'tdc', 'thread', 'mark-unread', '500', '--from', '11']), - ).rejects.toHaveProperty('code', 'INVALID_REF') - expect(client.threads.getUnread).not.toHaveBeenCalled() - }) - it('errors when --json is used for bulk refs without --yes', async () => { const client = createClient() apiMocks.getCommsClient.mockResolvedValue(client) diff --git a/src/commands/thread/unread.ts b/src/commands/thread/unread.ts index c601899..cb42d22 100644 --- a/src/commands/thread/unread.ts +++ b/src/commands/thread/unread.ts @@ -21,15 +21,17 @@ export async function markThreadUnread( options: MarkThreadUnreadOptions, ): Promise { const from = options.from - if (from !== undefined && refs.length > 1) { - throw new CliError( - 'CONFLICTING_OPTIONS', - '--from applies to a single thread; pass one thread ref when using it.', - ) - } await runThreadReadStateMutation(refs, options, { verb: 'unread', + validateRefs: (rawRefs) => { + if (from !== undefined && rawRefs.length > 1) { + throw new CliError( + 'CONFLICTING_OPTIONS', + '--from applies to a single thread; pass one thread ref when using it.', + ) + } + }, plan: async (client, threadId) => { const target = from === undefined From e50bbbba59e04d75f5d311f5832582cfccdfe3ec Mon Sep 17 00:00:00 2001 From: lmjabreu Date: Thu, 17 Sep 2026 21:59:35 +0100 Subject: [PATCH 4/5] fix: make done and undone no-ops when already in the target state Mirror the channel archive helper: skip the write when the thread or conversation is already where the verb would put it, and say so in the text output, so a repeated undo is idempotent. Also cover the archived dry-run preview and the missing-objIndex guard from the third doistbot pass. Co-Authored-By: Claude Opus 5 --- src/commands/conversation/archive.ts | 17 ++++--- .../conversation/conversation.test.ts | 22 ++++++++- src/commands/thread/mutate.ts | 13 +++--- src/commands/thread/thread.test.ts | 45 ++++++++++++++++++- 4 files changed, 84 insertions(+), 13 deletions(-) diff --git a/src/commands/conversation/archive.ts b/src/commands/conversation/archive.ts index 1af7a3c..f43b4e3 100644 --- a/src/commands/conversation/archive.ts +++ b/src/commands/conversation/archive.ts @@ -15,8 +15,9 @@ async function setConversationArchiveState( const client = await getCommsClient() const conversation = await client.conversations.getConversation(conversationId) + const noop = conversation.archived === archive + if (options.dryRun) { - const noop = conversation.archived === archive printDryRun(`${action} conversation`, { Conversation: conversationLabel(conversation), Status: noop ? (archive ? 'already archived' : 'not archived') : undefined, @@ -36,10 +37,12 @@ async function setConversationArchiveState( return } - if (archive) { - await client.conversations.archiveConversation(conversationId) - } else { - await client.conversations.unarchiveConversation(conversationId) + if (!noop) { + if (archive) { + await client.conversations.archiveConversation(conversationId) + } else { + await client.conversations.unarchiveConversation(conversationId) + } } if (options.json) { @@ -47,7 +50,9 @@ async function setConversationArchiveState( return } - console.log(`Conversation ${conversationId} ${action}d.`) + console.log( + `Conversation ${conversationId} ${action}d${noop ? ' (already in target state)' : ''}.`, + ) } export async function markConversationDone(ref: string, options: DoneOptions): Promise { diff --git a/src/commands/conversation/conversation.test.ts b/src/commands/conversation/conversation.test.ts index e244cc1..52d28f8 100644 --- a/src/commands/conversation/conversation.test.ts +++ b/src/commands/conversation/conversation.test.ts @@ -1249,6 +1249,22 @@ describe('conversation undone', () => { expect(consoleSpy).toHaveBeenCalledWith('Conversation 42 unarchived.') }) + it('skips the write when the conversation is not archived', async () => { + const conversation = createConversation(42, [1, 2], '2026-03-08T10:00:00.000Z') + const client = createClient({ activeConversations: [conversation] }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'conversation', 'undone', '42', '--yes']) + + expect(client.conversations.unarchiveConversation).not.toHaveBeenCalled() + expect(consoleSpy).toHaveBeenCalledWith( + 'Conversation 42 unarchived (already in target state).', + ) + }) + it('prompts for confirmation without --yes', async () => { const conversation = createConversation(42, [1, 2], '2026-03-08T10:00:00.000Z') const client = createClient({ archivedConversations: [conversation] }) @@ -1265,7 +1281,10 @@ describe('conversation undone', () => { }) it('outputs JSON with --json --yes', async () => { - const conversation = createConversation(42, [1, 2], '2026-03-08T10:00:00.000Z') + const conversation = { + ...createConversation(42, [1, 2], '2026-03-08T10:00:00.000Z'), + archived: true, + } const client = createClient({ archivedConversations: [conversation] }) apiMocks.getCommsClient.mockResolvedValue(client) @@ -1274,6 +1293,7 @@ describe('conversation undone', () => { await program.parseAsync(['node', 'tdc', 'conversation', 'undone', '42', '--json', '--yes']) + expect(client.conversations.unarchiveConversation).toHaveBeenCalledWith('42') const jsonOutput = JSON.parse(consoleSpy.mock.calls[0][0]) expect(jsonOutput).toEqual({ id: '42', archived: false }) }) diff --git a/src/commands/thread/mutate.ts b/src/commands/thread/mutate.ts index 240ae42..e46b3c6 100644 --- a/src/commands/thread/mutate.ts +++ b/src/commands/thread/mutate.ts @@ -38,10 +38,13 @@ async function setThreadArchiveState( return } - if (archive) { - await client.inbox.archiveThread(threadId) - } else { - await client.inbox.unarchiveThread(threadId) + const noop = thread.isArchived === archive + if (!noop) { + if (archive) { + await client.inbox.archiveThread(threadId) + } else { + await client.inbox.unarchiveThread(threadId) + } } if (options.json) { @@ -49,7 +52,7 @@ async function setThreadArchiveState( return } - console.log(`Thread ${threadId} ${action}d.`) + console.log(`Thread ${threadId} ${action}d${noop ? ' (already in target state)' : ''}.`) } export async function markThreadDone(ref: string, options: MutationOptions): Promise { diff --git a/src/commands/thread/thread.test.ts b/src/commands/thread/thread.test.ts index 47d26a9..82a1b68 100644 --- a/src/commands/thread/thread.test.ts +++ b/src/commands/thread/thread.test.ts @@ -1881,6 +1881,19 @@ describe('thread undone', () => { expect(consoleSpy).toHaveBeenCalledWith('Thread 500 unarchived.') }) + it('skips the write when the thread is already in the inbox', async () => { + const client = createClient() + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'thread', 'undone', '500', '--yes']) + + expect(client.inbox.unarchiveThread).not.toHaveBeenCalled() + expect(consoleSpy).toHaveBeenCalledWith('Thread 500 unarchived (already in target state).') + }) + it('prompts for confirmation without --yes', async () => { const client = createClient() apiMocks.getCommsClient.mockResolvedValue(client) @@ -1896,7 +1909,7 @@ describe('thread undone', () => { }) it('outputs JSON with --json --yes', async () => { - const client = createClient() + const client = createClient({ thread: { ...createThreadFixture(500), isArchived: true } }) apiMocks.getCommsClient.mockResolvedValue(client) const program = createProgram() @@ -1904,6 +1917,7 @@ describe('thread undone', () => { await program.parseAsync(['node', 'tdc', 'thread', 'undone', '500', '--json', '--yes']) + expect(client.inbox.unarchiveThread).toHaveBeenCalledWith('500') const jsonOutput = JSON.parse(consoleSpy.mock.calls[0][0]) expect(jsonOutput).toEqual({ id: '500', isArchived: false }) }) @@ -1920,6 +1934,21 @@ describe('thread undone', () => { expect(client.inbox.unarchiveThread).not.toHaveBeenCalled() }) + it('shows dry run output for an archived thread without a status line', async () => { + const client = createClient({ thread: { ...createThreadFixture(500), isArchived: true } }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'thread', 'undone', '500', '--dry-run']) + + expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Would unarchive thread')) + expect(consoleSpy).toHaveBeenCalledWith(' Thread: Test Thread (500)') + expect(consoleSpy).not.toHaveBeenCalledWith(expect.stringContaining('Status:')) + expect(client.inbox.unarchiveThread).not.toHaveBeenCalled() + }) + it('shows dry run output and flags a thread already in the inbox', async () => { const client = createClient() apiMocks.getCommsClient.mockResolvedValue(client) @@ -2001,6 +2030,20 @@ describe('thread mark-unread', () => { expect(client.threads.getUnread).not.toHaveBeenCalled() }) + it('rejects a --from comment with no object index', async () => { + const client = createClient({ + comments: [{ ...createComment(11, 1), objIndex: undefined as unknown as number }], + }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + + await expect( + program.parseAsync(['node', 'tdc', 'thread', 'mark-unread', '500', '--from', '11']), + ).rejects.toHaveProperty('code', 'INVALID_REF') + expect(client.threads.markUnread).not.toHaveBeenCalled() + }) + it('rejects --from with more than one thread ref', async () => { const client = createClient({ comments: [createComment(11, 1)] }) apiMocks.getCommsClient.mockResolvedValue(client) From 687f52e24350632fa7956f445f68cbe3a3e7cf37 Mon Sep 17 00:00:00 2001 From: lmjabreu Date: Fri, 18 Sep 2026 09:13:00 +0100 Subject: [PATCH 5/5] test: report already-archived in thread done dry run, cover archived conversation preview Co-Authored-By: Claude Opus 5 --- .../conversation/conversation.test.ts | 21 +++++++++++++++++++ src/commands/thread/mutate.ts | 5 +++-- src/commands/thread/thread.test.ts | 13 ++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/commands/conversation/conversation.test.ts b/src/commands/conversation/conversation.test.ts index 52d28f8..3910189 100644 --- a/src/commands/conversation/conversation.test.ts +++ b/src/commands/conversation/conversation.test.ts @@ -1311,6 +1311,27 @@ describe('conversation undone', () => { expect(client.conversations.unarchiveConversation).not.toHaveBeenCalled() }) + it('shows dry run output for an archived conversation without a status line', async () => { + const conversation = { + ...createConversation(42, [1, 2], '2026-03-08T10:00:00.000Z'), + archived: true, + } + const client = createClient({ archivedConversations: [conversation] }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'conversation', 'undone', '42', '--dry-run']) + + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining('Would unarchive conversation'), + ) + expect(consoleSpy).toHaveBeenCalledWith(' Conversation: conversation 42') + expect(consoleSpy).not.toHaveBeenCalledWith(expect.stringContaining('Status:')) + expect(client.conversations.unarchiveConversation).not.toHaveBeenCalled() + }) + it('shows dry run output and flags a conversation that is not archived', async () => { const conversation = createConversation(42, [1, 2], '2026-03-08T10:00:00.000Z') const client = createClient({ activeConversations: [conversation] }) diff --git a/src/commands/thread/mutate.ts b/src/commands/thread/mutate.ts index e46b3c6..4264d32 100644 --- a/src/commands/thread/mutate.ts +++ b/src/commands/thread/mutate.ts @@ -18,10 +18,12 @@ async function setThreadArchiveState( const thread = await client.threads.getThread(threadId) await assertChannelIsPublic(thread.channelId, thread.workspaceId) + const noop = thread.isArchived === archive + if (options.dryRun) { printDryRun(`${action} thread`, { Thread: threadLabel(thread), - Status: !archive && !thread.isArchived ? 'already in inbox' : undefined, + Status: noop ? (archive ? 'already archived' : 'already in inbox') : undefined, }) return } @@ -38,7 +40,6 @@ async function setThreadArchiveState( return } - const noop = thread.isArchived === archive if (!noop) { if (archive) { await client.inbox.archiveThread(threadId) diff --git a/src/commands/thread/thread.test.ts b/src/commands/thread/thread.test.ts index 82a1b68..fb65919 100644 --- a/src/commands/thread/thread.test.ts +++ b/src/commands/thread/thread.test.ts @@ -1847,6 +1847,19 @@ describe('thread done', () => { expect(client.inbox.archiveThread).not.toHaveBeenCalled() }) + it('flags an already archived thread in dry run', async () => { + const client = createClient({ thread: { ...createThreadFixture(500), isArchived: true } }) + apiMocks.getCommsClient.mockResolvedValue(client) + + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'thread', 'done', '500', '--dry-run']) + + expect(consoleSpy).toHaveBeenCalledWith(' Status: already archived') + expect(client.inbox.archiveThread).not.toHaveBeenCalled() + }) + it('runs validation in dry-run mode', async () => { const client = createClient() apiMocks.getCommsClient.mockResolvedValue(client)