feat: support file attachments on Slack-native drafts - #135
Open
andromedarabbit wants to merge 3 commits into
Open
feat: support file attachments on Slack-native drafts#135andromedarabbit wants to merge 3 commits into
andromedarabbit wants to merge 3 commits into
Conversation
Slack-native drafts (message draft create/update) accepted no --attach: createDraft's input lacked fileIds, the CLI exposed no option, and the upload helper was message-only (completeUploadExternal bound the file to a channel, so it could not serve a draft that has no message yet). - Add uploadFileForDraft to src/slack/upload.ts: shares upload staging with uploadLocalFileToSlack but completes without channel_id, returning the file id (files.completeUploadExternal's channel_id is optional). - Wire fileIds through createDraft (updateDraft already supported it). - Add a repeatable --attach to `message draft create` / `update`; update merges new uploads into the draft's existing file_ids. The safe-mode compose editor path remains intentionally out of scope: draft-editor.html has no attachment UI, so redirectSendToDraft still rejects --attach there. Co-Authored-By: Claude <noreply@anthropic.com>
Address ce-code-review findings on PR stablyai#135: - Extract normalizeAttachPaths + collectOptionValue into src/cli/options.ts. The two helpers were duplicated byte-for-byte across message-actions.ts, message-command.ts, and the message-draft-* modules (review stablyai#4, stablyai#6). - Replace the per-file draft upload loop with uploadFilesForDraft: stage every file first, then a single completeUploadExternal. A staging failure leaves nothing completed, and Slack discards staged-but-uncompleted uploads, so a later --attach failure can no longer orphan private files (review stablyai#1). - Add a mergeFileIds de-duplication test for overlapping file ids, and update the mock/assertions for the batched completion order (review stablyai#5). Co-Authored-By: Claude <noreply@anthropic.com>
리뷰 stablyai#2/stablyai#3 대응. draft에 파일을 첨부할 때 뒤따르는 draft API 호출이 실패하면 이미 올린 파일이 비공개로 orphan 되는 문제를 막는다. stablyai#2 (auth 재시도 재실행): createDraft/updateDraftAction에서 uploadedFileIds를 withAutoRefresh의 work() 바깥 클로저에 메모이제이션한다. drafts.create/update가 invalid_auth로 실패해 work()가 재실행될 때 upload를 다시 돌리지 않고 올린 file id를 재사용한다. 첫 업로드 파일이 두 번째 업로드에 밀려 orphan/중복되던 문제를 해결한다. stablyai#3 (stale 실패 orphan): createDraft/updateDraft에 ensureDraftOk를 추가해 ok:false(stale conflict, denied 등)를 throw하게 했다(기존엔 null로 삼킴). cleanupUploadedDraftFiles(files.delete best-effort) 헬퍼를 추가하고, action 전체를 try/catch로 감싸 최종 실패 시 업로드한 새 파일만 정리 후 rethrow한다. update는 existing file_ids는 건드리지 않고 새로 올린 파일만 정리한다. 테스트: createContext에 failOnce/failWith/retryOnAuth 주입 옵션을 추가하고 stablyai#2 create/update 재시도, stablyai#3 create/update 실패 정리, 첨부 없으면 files.delete 미호출 시나리오 5개를 추가했다. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Slack-native drafts (
message draft create/update) couldn't carry file attachments — onlymessage send --attachcould. This adds a repeatable--attachto draft create/update.Root cause
createDraft's input lackedfileIds, the CLI exposed no--attachoption, and the only upload helper (uploadLocalFileToSlack) was message-only —files.completeUploadExternalbound the file to achannel_id, so it couldn't serve a draft that has no message yet. Per the Slack docs,channel_idis optional onfiles.completeUploadExternal, so a file can be staged and its id returned without binding it to a message. (updateDraft already plumbedfileIds, and tests already verifiedfile_idsround-trip — only the create pipe was severed.)Changes
src/slack/upload.ts— extract sharedstageFileUpload; adduploadFileForDraftthat completes the upload withoutchannel_idand returns the file id.src/slack/drafts.ts—createDraftacceptsfileIds(updateDraft already did).src/cli/message-draft-actions.ts— upload--attachfiles and passfileIds;updatemerges new uploads into the draft's existingfile_ids(de-duplicated).src/cli/message-draft-command.ts— repeatable--attach <path>oncreate/update.Tests
test/upload.test.ts(new):uploadFileForDrafthappy path (asserts nochannel_id/thread_ts/initial_commenton the completion call), missing path, directory, getUploadURL failure, byte-POST failure, completeUpload failure;uploadLocalFileToSlackregression after the shared-helper extraction.test/drafts.test.ts—createDraftpropagatesfileIds.test/message-draft-actions.test.ts— create multi-attach (ordered file_ids), missing-path abort (no upload, nodrafts.create), update merge into existingfile_ids, commander--attach a --attach bwiring.bun test357 pass / 0 fail ·tsc --noEmitclean ·oxlint0 errors.Out of scope
The safe-mode compose editor path (
redirectSendToDraft→draft-editor.html) still rejects--attach; that path needs editor UI work and is left for a follow-up.🤖 Generated with Claude Code