feat(attachments): support uploading files on replies and thread creation (#13)

* feat(attachments): support uploading files on replies and thread creation

Ports the twist-cli attachment feature to comms-cli, backed by the
`attachments` client in @doist/comms-sdk 0.4.1.

- Bump @doist/comms-sdk 0.3.0 -> 0.4.1
- `src/lib/local-file.ts` (`openLocalFileAsBlob`) reads a path into a
  file-backed Blob with structured FILE_NOT_FOUND / FILE_READ_ERROR errors
- `src/lib/attachments.ts` (`uploadAttachments`) validates every path up
  front, then uploads each concurrently (order preserved)
- `tdc thread reply`, `tdc conversation reply`, and `tdc thread create`
  gain a repeatable `--file`; a file-only post (no text) is allowed
- `conversation reply` preflights the conversation before uploading so an
  invalid/forbidden target fails before orphaning an upload
- `--file` + `--close`/`--reopen` on thread reply -> CONFLICTING_OPTIONS
- Add an `attachments.upload` spinner entry so uploads route through
  `wrapResult` for the 403 -> INSUFFICIENT_SCOPE re-login prompt
- Add `attachments:read` / `attachments:write` to the OAuth scopes
- Skill content + regenerated SKILL.md; tests for all of the above

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

* fix(attachments): address review feedback on #13

- thread reply: build a `satisfies`-typed createComment request so the
  `attachments` contract is compiler-checked again; only `recipients`
  (EVERYONE sentinels) keeps the assertion
- local-file: try `openAsBlob` first (happy path = one open, no TOCTOU);
  fall back to an `fs.open` probe only on failure to recover the real
  errno (openAsBlob masks it as ERR_INVALID_ARG_VALUE)
- attachments: cap upload concurrency (4) while preserving input order
- options: `collect` defaults `previous` to `[]` so the contract holds
  even if a caller forgets the Commander default
- dry-run now validates attachment paths (without uploading) so the
  preview fails on a bad path exactly as a real run would
- tests: attachments.upload exercised through the mutating write-guard
  (incl. read-only block), local-file FILE_READ_ERROR branch, and a
  conversation preflight-failure-skips-upload case

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

* chore: avoid new Array() in concurrency helper (oxlint no-new-array)

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

---------

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

Author: scottlovegrove

package-lock.json

@@ -51,7 +51,7 @@
     ],
     "dependencies": {
         "@doist/cli-core": "0.24.0",
-        "@doist/comms-sdk": "0.3.0",
+        "@doist/comms-sdk": "0.4.1",
         "@pnpm/tabtab": "0.5.4",
         "chalk": "5.6.2",
         "commander": "14.0.3",

package.json

@@ -91,13 +91,15 @@ tdc thread create <channel-ref> "Title" "content" --notify 123,456  # Notify spe
 tdc thread create <channel-ref> "Title" "content" --unarchive  # Land thread in author's Inbox (overrides default Comms auto-archive)
 tdc thread create <channel-ref> "Title" "content" --no-unarchive  # Force archive even when userSettings.unarchiveNewThreads=true
 tdc thread create <channel-ref> "Title" "content" --dry-run  # Preview without posting
+tdc thread create <channel-ref> "Title" --file ./a.png  # Attach a file (repeatable; content optional)
 tdc thread reply <ref> "content"  # Post a comment (notifies EVERYONE_IN_THREAD by default)
 tdc thread reply <ref> "content" --notify EVERYONE  # Notify all workspace members
 tdc thread reply <ref> "content" --notify 123,id:456   # Notify specific user IDs
 tdc thread reply <ref> "content" --json  # Post and return comment as JSON
 tdc thread reply <ref> "content" --json --full  # Include all comment fields
 tdc thread reply <ref> "content" --close       # Reply and close the thread
 tdc thread reply <ref> "content" --reopen      # Reply and reopen a closed thread
+tdc thread reply <ref> "content" --file ./a.png  # Attach a file (repeatable; content optional)
 tdc thread done <ref>                 # Preview thread archive (requires --yes to execute)
 tdc thread done <ref> --yes           # Archive thread (mark done)
 tdc thread done <ref> --yes --json    # Archive and return status as JSON
@@ -153,6 +155,7 @@ tdc conversation with <user-ref> --include-groups # List any conversations with
 tdc conversation reply <ref> "content"     # Send a message
 tdc conversation reply <ref> "content" --json  # Send and return message as JSON
 tdc conversation reply <ref> "content" --json --full  # Include all message fields
+tdc conversation reply <ref> "content" --file ./a.png  # Attach a file (repeatable; content optional)
 tdc conversation done <ref>                    # Preview conversation archive (requires --yes to execute)
 tdc conversation done <ref> --yes              # Archive conversation
 tdc conversation done <ref> --yes --json       # Archive and return status as JSON

skills/comms-cli/SKILL.md

@@ -1,9 +1,12 @@
+import { mkdtemp, rm, writeFile } from 'node:fs/promises'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
 import {
     captureConsole,
     createTestProgram,
     describeEmptyMachineOutput,
 } from '@doist/cli-core/testing'
-import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
 import { clearWorkspaceUserCache } from '../../lib/api.js'
 import { CliError } from '../../lib/errors.js'
 
@@ -32,6 +35,12 @@ vi.mock('../../lib/markdown.js', () => ({
 
 vi.mock('chalk')
 
+vi.mock('../../lib/input.js', () => ({
+    readStdin: vi.fn().mockResolvedValue(''),
+    openEditor: vi.fn().mockResolvedValue(''),
+}))
+
+import { openEditor } from '../../lib/input.js'
 import { registerConversationCommand } from './index.js'
 
 type TestConversation = {
@@ -115,7 +124,25 @@ function createClient({
                 async ({ conversationId }: { conversationId: string; limit?: number }) =>
                     messagesByConversation[conversationId] ?? [],
             ),
-            createMessage: vi.fn(),
+            createMessage: vi.fn(
+                async (args: {
+                    conversationId: string
+                    content: string
+                    attachments?: Array<{ fileName?: string | null }>
+                }) => ({
+                    id: '999',
+                    conversationId: args.conversationId,
+                    content: args.content,
+                    url: 'https://comms.todoist.com/a/1/msg/999',
+                }),
+            ),
+        },
+        attachments: {
+            upload: vi.fn(async (args: { file: Blob; fileName: string }) => ({
+                attachmentId: `att-${args.fileName}`,
+                urlType: 'file',
+                fileName: args.fileName,
+            })),
         },
         workspaceUsers: {
             getUserById: vi.fn(
@@ -128,6 +155,30 @@ function createClient({
 
 const createProgram = () => createTestProgram(registerConversationCommand)
 
+// Shared setup for the --file suite: a fresh mock client wired into getCommsClient
+// plus a program. Tests asserting on output call captureConsole('log') themselves.
+function setupFileTest() {
+    const client = createClient({})
+    apiMocks.getCommsClient.mockResolvedValue(client)
+    return { client, program: createProgram() }
+}
+
+// Registers a temp dir with two files for a --file suite, cleaned up afterwards.
+function useFileFixtures(prefix: string, png: string, pdf: string) {
+    const paths = { dir: '', png: '', pdf: '' }
+    beforeAll(async () => {
+        paths.dir = await mkdtemp(join(tmpdir(), prefix))
+        paths.png = join(paths.dir, png)
+        paths.pdf = join(paths.dir, pdf)
+        await writeFile(paths.png, 'png-bytes')
+        await writeFile(paths.pdf, 'pdf-bytes')
+    })
+    afterAll(async () => {
+        await rm(paths.dir, { recursive: true, force: true })
+    })
+    return paths
+}
+
 // Cache lives in api.ts module scope, so reset between tests.
 beforeEach(() => {
     clearWorkspaceUserCache()
@@ -689,3 +740,147 @@ describe('conversation done', () => {
         expect(client.conversations.archiveConversation).not.toHaveBeenCalled()
     })
 })
+
+describe('conversation reply --file', () => {
+    const files = useFileFixtures('tdc-convo-reply-', 'photo.png', 'doc.pdf')
+
+    beforeEach(() => {
+        vi.clearAllMocks()
+    })
+
+    it('uploads the file and attaches it to the message', async () => {
+        const { client, program } = setupFileTest()
+        const consoleSpy = captureConsole('log')
+
+        await program.parseAsync([
+            'node',
+            'tdc',
+            'conversation',
+            'reply',
+            '42',
+            'See attached',
+            '--file',
+            files.png,
+        ])
+
+        expect(client.attachments.upload).toHaveBeenCalledTimes(1)
+        expect(client.attachments.upload).toHaveBeenCalledWith(
+            expect.objectContaining({ fileName: 'photo.png' }),
+        )
+        expect(client.conversationMessages.createMessage).toHaveBeenCalledWith(
+            expect.objectContaining({
+                conversationId: '42',
+                content: 'See attached',
+                attachments: [expect.objectContaining({ fileName: 'photo.png' })],
+            }),
+        )
+        expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Attached: photo.png'))
+    })
+
+    it('attaches multiple repeated --file values', async () => {
+        const { client, program } = setupFileTest()
+
+        await program.parseAsync([
+            'node',
+            'tdc',
+            'conversation',
+            'reply',
+            '42',
+            'two files',
+            '--file',
+            files.png,
+            '--file',
+            files.pdf,
+        ])
+
+        expect(client.attachments.upload).toHaveBeenCalledTimes(2)
+        const args = client.conversationMessages.createMessage.mock.calls[0][0] as {
+            attachments: Array<{ fileName?: string }>
+        }
+        expect(args.attachments.map((a) => a.fileName)).toEqual(['photo.png', 'doc.pdf'])
+    })
+
+    it('allows a file-only message with no text content', async () => {
+        const { client, program } = setupFileTest()
+
+        await program.parseAsync([
+            'node',
+            'tdc',
+            'conversation',
+            'reply',
+            '42',
+            '--file',
+            files.png,
+        ])
+
+        expect(client.conversationMessages.createMessage).toHaveBeenCalledWith(
+            expect.objectContaining({ content: '', attachments: expect.any(Array) }),
+        )
+        // A file-only message must not block on the editor.
+        expect(openEditor).not.toHaveBeenCalled()
+    })
+
+    it('errors with FILE_NOT_FOUND for a missing path and does not send', async () => {
+        const { client, program } = setupFileTest()
+
+        await expect(
+            program.parseAsync([
+                'node',
+                'tdc',
+                'conversation',
+                'reply',
+                '42',
+                'x',
+                '--file',
+                join(files.dir, 'missing.png'),
+            ]),
+        ).rejects.toMatchObject({ code: 'FILE_NOT_FOUND' })
+
+        expect(client.attachments.upload).not.toHaveBeenCalled()
+        expect(client.conversationMessages.createMessage).not.toHaveBeenCalled()
+    })
+
+    it('skips the upload when the conversation preflight fails (no orphaned upload)', async () => {
+        const { client, program } = setupFileTest()
+        client.conversations.getConversation.mockRejectedValueOnce(
+            new CliError('NOT_FOUND', 'Conversation not found'),
+        )
+
+        await expect(
+            program.parseAsync([
+                'node',
+                'tdc',
+                'conversation',
+                'reply',
+                '42',
+                'See attached',
+                '--file',
+                files.png,
+            ]),
+        ).rejects.toMatchObject({ code: 'NOT_FOUND' })
+
+        expect(client.attachments.upload).not.toHaveBeenCalled()
+        expect(client.conversationMessages.createMessage).not.toHaveBeenCalled()
+    })
+
+    it('does not upload during --dry-run but lists the attachment', async () => {
+        const { client, program } = setupFileTest()
+        const consoleSpy = captureConsole('log')
+
+        await program.parseAsync([
+            'node',
+            'tdc',
+            'conversation',
+            'reply',
+            '42',
+            'preview',
+            '--file',
+            files.png,
+            '--dry-run',
+        ])
+
+        expect(client.attachments.upload).not.toHaveBeenCalled()
+        expect(client.conversationMessages.createMessage).not.toHaveBeenCalled()
+        expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining(files.png))
+    })
+})

src/commands/conversation/conversation.test.ts

@@ -18,7 +18,7 @@ export type ConversationWithOptions = PaginatedViewOptions & {
     snippet?: boolean
 }
 
-export type ReplyOptions = MutationOptions
+export type ReplyOptions = MutationOptions & { file?: string[] }
 
 export type MuteOptions = MutationOptions & { minutes?: string }
 

src/commands/conversation/helpers.ts

@@ -1,4 +1,5 @@
 import { Command } from 'commander'
+import { collect } from '../../lib/options.js'
 import { markConversationDone } from './done.js'
 import { muteConversation } from './mute.js'
 import { replyToConversation } from './reply.js'
@@ -77,6 +78,7 @@ Examples:
     conversation
         .command('reply <conversation-ref> [content]')
         .description('Send a message in a conversation')
+        .option('--file <path>', 'Attach a file (repeatable; content optional)', collect, [])
         .option('--dry-run', 'Show what would be sent without sending')
         .option('--json', 'Output sent message as JSON')
         .option('--full', 'Include all fields in JSON output')
@@ -86,7 +88,8 @@ Examples:
 Examples:
   tdc conversation reply 12345 "Hello!"
   echo "Message body" | tdc conversation reply 12345
-  tdc conversation reply 12345 "Update" --json`,
+  tdc conversation reply 12345 "Update" --json
+  tdc conversation reply 12345 "See attached" --file ./photo.jpg`,
         )
         .action(replyToConversation)
 

src/commands/conversation/index.ts

@@ -1,4 +1,6 @@
+import chalk from 'chalk'
 import { getCommsClient } from '../../lib/api.js'
+import { uploadAttachments, validateAttachmentFiles } from '../../lib/attachments.js'
 import { CliError } from '../../lib/errors.js'
 import { openEditor, readStdin } from '../../lib/input.js'
 import { formatJson, printDryRun } from '../../lib/output.js'
@@ -12,34 +14,54 @@ export async function replyToConversation(
 ): Promise<void> {
     const conversationId = resolveConversationId(ref)
 
+    const files = options.file ?? []
+    const hasFiles = files.length > 0
+
     let replyContent = await readStdin()
     if (!replyContent && content) {
         replyContent = content
     }
-    if (!replyContent) {
+    // A file-only message is allowed: skip the editor prompt and the empty-content guard.
+    if (!replyContent && !hasFiles) {
         replyContent = await openEditor()
     }
-    if (!replyContent || replyContent.trim() === '') {
+    if ((!replyContent || replyContent.trim() === '') && !hasFiles) {
         throw new CliError(
             'MISSING_CONTENT',
-            'No content provided. Pass content as an argument or pipe via stdin.',
+            'No content provided. Pass content as an argument, pipe via stdin, or attach a file.',
         )
     }
+    const messageContent = replyContent ?? ''
 
     if (options.dryRun) {
+        // Validate attachment paths so the preview fails on a bad path exactly
+        // as a real run would (no upload happens in dry-run).
+        if (hasFiles) {
+            await validateAttachmentFiles(files)
+        }
         const preview =
-            replyContent.length > 200 ? `${replyContent.slice(0, 200)}...` : replyContent
+            messageContent.length > 200 ? `${messageContent.slice(0, 200)}...` : messageContent
         printDryRun('send message to conversation', {
             Conversation: String(conversationId),
-            Content: preview,
+            Attach: hasFiles ? files.join(', ') : undefined,
+            Content: preview || undefined,
         })
         return
     }
 
     const client = await getCommsClient()
+
+    // Preflight the target before uploading so an invalid or forbidden
+    // conversation fails fast instead of leaving an orphaned upload behind.
+    if (hasFiles) {
+        await client.conversations.getConversation(conversationId)
+    }
+
+    const attachments = hasFiles ? await uploadAttachments(files) : undefined
     const message = await client.conversationMessages.createMessage({
         conversationId,
-        content: replyContent,
+        content: messageContent,
+        attachments,
     })
 
     if (options.json) {
@@ -48,4 +70,8 @@ export async function replyToConversation(
     }
 
     console.log(`Message sent: ${message.url}`)
+    if (attachments && attachments.length > 0) {
+        const names = attachments.map((a) => a.fileName ?? 'file').join(', ')
+        console.log(chalk.dim(`Attached: ${names}`))
+    }
 }