feat(task): add attachment download shortcut - #2274
Open
zhouyijian wants to merge 3 commits into
Open
Conversation
|
|
github-actions
Bot
added
domain/task
📝 WalkthroughWalkthroughThe PR adds the ChangesTask attachment download
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant TaskAPI
participant TemporaryURL
CLI->>TaskAPI: Request attachment metadata
TaskAPI-->>CLI: Return metadata and temporary URL
CLI->>TemporaryURL: Download attachment stream
TemporaryURL-->>CLI: Return file bytes or authorization error
CLI->>TaskAPI: Refresh metadata after authorization error
TaskAPI-->>CLI: Return refreshed temporary URL
CLI->>TemporaryURL: Retry download with refreshed URL
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
zhouyijian
force-pushed
the
codex/task-attachment-download
branch
from
August 10, 2026 14:29
abadce5 to
04b5e61
Compare
zhouyijian
marked this pull request as ready for review
August 10, 2026 14:33
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
internal/affordance/task_source_test.go (1)
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
internal/vfsfor filesystem access in this internal test.The test uses
os.DirFSandos.ReadFile. Route repository filesystem access throughinternal/vfs, or adapt the source injection helper to accept the repository VFS.As per coding guidelines, “Use
internal/vfsfilesystem APIs instead ofosfilesystem APIs.”Also applies to: 39-42
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/affordance/task_source_test.go` at line 17, Update the test’s filesystem setup and reads around SetSource to use the repository’s internal/vfs APIs instead of os.DirFS and os.ReadFile. Reuse the existing repository VFS or adapt the source injection helper to accept it, while preserving the test’s current paths and behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/download/transport_test.go`:
- Around line 137-175: Extend the OpaqueURL error tests with cases for malformed
signed URLs and CheckRedirect failures, alongside
TestOpaqueURLDoesNotLeakSignedURLOrResponseBody and
TestOpaqueURLDoesNotLeakSignedURLOnTransportFailure. For each case, assert the
error omits both the signed URL material and query key, and verify
errs.ProblemOf reports the expected retryable network/transport metadata.
- Around line 153-155: Update both transport failure tests to assert
problem.Category is CategoryNetwork and problem.Subtype is
SubtypeNetworkTransport alongside the existing HTTP 403 check. In
TestOpaqueURLDoesNotLeakSignedURLOnTransportFailure, retain the original source
error and verify errors.Is(err, cause) after extracting metadata with
errs.ProblemOf.
In `@shortcuts/task/task_download_attachment.go`:
- Around line 185-191: Update the output-path inspection logic around
runtime.FileIO().Stat in the download flow: keep path-validation failures as the
existing invalid-argument validation error, but classify non-fs.ErrNotExist
permission and filesystem failures with the prescribed typed file-I/O error
constructor while preserving the original cause. Apply the same classification
to the related error branch around the output handling at the referenced second
location.
- Around line 173-175: Update the attachment response handling around
metadata.GUID to reject responses whose server-provided GUID is empty or
whitespace by returning errs.NewInternalError(errs.SubtypeInvalidResponse, ...);
remove the fallback assignment from the requested guid, and add a test covering
this invalid response shape.
---
Nitpick comments:
In `@internal/affordance/task_source_test.go`:
- Line 17: Update the test’s filesystem setup and reads around SetSource to use
the repository’s internal/vfs APIs instead of os.DirFS and os.ReadFile. Reuse
the existing repository VFS or adapt the source injection helper to accept it,
while preserving the test’s current paths and behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0d874e90-1c5a-4217-9a31-1bbdaa752ccd
📒 Files selected for processing (12)
affordance/task.mdinternal/affordance/task_source_test.gointernal/download/transport.gointernal/download/transport_test.goshortcuts/task/shortcuts.goshortcuts/task/task_download_attachment.goshortcuts/task/task_download_attachment_test.goskills/lark-task/SKILL.mdskills/lark-task/references/lark-task-download-attachment.mdtests/cli_e2e/task/coverage.mdtests/cli_e2e/task/task_download_attachment_dryrun_test.gotests/cli_e2e/task/task_download_attachment_workflow_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/task/task_download_attachment_test.go`:
- Around line 212-231: Update the test around DownloadAttachmentTask to track
requests to the temporary download URL with an OnMatch counter on downloadStub,
then assert that the counter remains zero after runMountedTaskShortcut returns.
Replace the CapturedBodies assertion, which does not reliably indicate whether
this non-reusable stub was called.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a61455e8-af3a-400a-9089-5e7c800180d0
📒 Files selected for processing (3)
internal/download/transport_test.goshortcuts/task/task_download_attachment.goshortcuts/task/task_download_attachment_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/task/task_download_attachment.go
Author
|
Hi @liangshuo-1, all review threads have been addressed and the relevant local checks pass ( |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@8072aee8ace9fb659554a7f6138f934d5cba5533🧩 Skill updatenpx skills add zhouyijian/cli#codex/task-attachment-download -y -g |
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
2 participants
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
Add a first-class Task attachment download workflow that resolves an attachment GUID to a short-lived URL and streams the file through the CLI FileIO provider without exposing signed URL material.
Changes
task +download-attachmentwith safe output-path handling, overwrite protection, URL refresh, and structured outputTest Plan
make unit-testmake vetmake fmt-checkgo mod tidyleavesgo.modandgo.sumunchangedQUALITY_GATE_CHANGED_FROM=upstream/main make quality-gateupstream/main--helpand dry-run verification confirms the two-step Task API and external download flowRelated Issues
Summary by CodeRabbit
New Features
task +download-attachmentfor downloading task attachments by ID.Security
Documentation
Tests