Skip to content

fix: support 2GB base attachment uploads#441

Merged
kongenpei merged 1 commit intomainfrom
codex/base-attachment-multipart-upload
Apr 13, 2026
Merged

fix: support 2GB base attachment uploads#441
kongenpei merged 1 commit intomainfrom
codex/base-attachment-multipart-upload

Conversation

@kongenpei
Copy link
Copy Markdown
Collaborator

@kongenpei kongenpei commented Apr 13, 2026

Summary

Support base +record-upload-attachment for larger files by switching to Drive multipart upload above 20MB and keeping validation/output explicit for AI-agent use.

Changes

  • use the shared Drive upload helpers so Base attachment uploads support multipart transfer and enforce the 2GB API limit
  • add coverage for multipart uploads and over-limit validation, and update the lark-base reference doc

Test Plan

  • Unit tests pass (make unit-test)
  • Manual local verification confirms the lark-cli base +record-upload-attachment command works as expected (multipart upload succeeds for 20MB + 1 byte and exact 2GB; 2GB + 1 byte fails fast with exceeds 2GB limit)
  • go mod tidy
  • go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev=origin/main

Related Issues

  • None

Summary by CodeRabbit

Release Notes

  • New Features

    • Record attachment uploads now support files up to 2GB (increased from 20MB limit)
  • Documentation

    • Updated documentation with new 2GB file size limit for record attachments
    • Added guidance on proper media file download procedures

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 13, 2026

📝 Walkthrough

Walkthrough

This PR extends base record attachment upload functionality to support files up to 2GB by integrating multipart upload capabilities. For files exceeding 20MB, the code now uses a multipart upload sequence (upload_prepareupload_partupload_finish) instead of the single-part endpoint, while maintaining backward compatibility with smaller files.

Changes

Cohort / File(s) Summary
Multipart Upload Integration
shortcuts/base/record_upload_attachment.go
Refactored attachment upload to conditionally use multipart sequence for files >20MB via shared helpers; increased max file size from 20MB to 2GB; added 2GB validation; delegated upload logic to common.UploadDriveMediaAll and common.UploadDriveMediaMultipart.
Test Coverage Expansion
shortcuts/base/base_execute_test.go
Added test cases validating multipart upload behavior (multiple upload_part calls) for files exceeding 20MB threshold and 2GB validation boundary condition.
Documentation Update
skills/lark-base/references/lark-base-record-upload-attachment.md
Updated file size limit documentation from 20MB to 2GB and added clarification that record attachment file tokens require docs +media-download instead of drive +download.

Sequence Diagram

sequenceDiagram
    participant Client
    participant BaseRecordUpload
    participant UploadHelper as Upload Helper
    participant DriveAPI as Drive API

    Client->>BaseRecordUpload: Upload attachment (fileSize, filePath)
    BaseRecordUpload->>BaseRecordUpload: Validate fileSize ≤ 2GB
    
    alt fileSize ≤ 20MB
        BaseRecordUpload->>UploadHelper: UploadDriveMediaAll()
        UploadHelper->>DriveAPI: POST /upload_all
        DriveAPI-->>UploadHelper: fileToken
    else fileSize > 20MB
        BaseRecordUpload->>UploadHelper: UploadDriveMediaMultipart()
        UploadHelper->>DriveAPI: POST /upload_prepare
        DriveAPI-->>UploadHelper: uploadId
        loop for each part
            UploadHelper->>DriveAPI: POST /upload_part
            DriveAPI-->>UploadHelper: partToken
        end
        UploadHelper->>DriveAPI: POST /upload_finish
        DriveAPI-->>UploadHelper: fileToken
    end
    
    UploadHelper-->>BaseRecordUpload: fileToken
    BaseRecordUpload->>Client: Updated record with attachment
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • fangshuyu-768
  • zgz2048

Poem

🐰 A rabbit hops with files so grand,
From twenty megs to two-gig land,
In pieces small they fly so fast,
Upload by upload, at last, at last! ✨📤

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description follows the required template with all main sections completed: Summary, Changes, Test Plan, and Related Issues.
Title check ✅ Passed The title clearly and concisely captures the main change: enabling support for 2GB base attachment uploads through multipart upload functionality.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/base-attachment-multipart-upload

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added domain/base PR touches the base domain size/M Single-domain feat or fix with limited business impact labels Apr 13, 2026
@kongenpei kongenpei marked this pull request as ready for review April 13, 2026 08:21
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kongenpei has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@kongenpei kongenpei requested a review from zgz2048 April 13, 2026 08:22
@github-actions
Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@aae49d5d64bae61e16661945547480af3c8820cb

🧩 Skill update

npx skills add larksuite/cli#codex/base-attachment-multipart-upload -y -g

@kongenpei kongenpei changed the title [codex] fix: support large base attachment uploads fix: support large base attachment uploads Apr 13, 2026
@kongenpei kongenpei changed the title fix: support large base attachment uploads fix: support 2GB base attachment uploads Apr 13, 2026
@kongenpei kongenpei merged commit c70c3fd into main Apr 13, 2026
21 checks passed
@kongenpei kongenpei deleted the codex/base-attachment-multipart-upload branch April 13, 2026 11:32
yxzhaao pushed a commit to yxzhaao/cli that referenced this pull request Apr 14, 2026
Co-authored-by: kongenpei <kongenpei@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/base PR touches the base domain size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants