fix: preserve filenames for file uploads

[rendylong]

Summary

Fix generic --file uploads so multipart filenames are preserved for local files.

BuildFormdata previously opened a local file, read it into memory, and passed a bytes.Reader to the Lark SDK. The SDK only preserves multipart filenames when the form field is a concrete *os.File; anonymous readers fall back to unknown-file.

This change passes the opened local file directly to Formdata.AddFile, allowing the SDK to use filepath.Base(f.Name()) as the multipart filename.

Fixes #729

Tests

• go test ./internal/cmdutil
• go test ./cmd/api ./cmd/service
• make build

Summary by CodeRabbit

• Bug Fixes
  • Improved file upload performance by streamlining data processing, reducing memory consumption during uploads and enabling better handling of larger files.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

📝 Walkthrough

Walkthrough

The BuildFormdata function is refactored to pass file handles directly to the Lark SDK's multipart handler instead of reading file contents into memory with io.ReadAll and wrapping them in a bytes.Reader. This preserves filename metadata through the SDK's native file handling.

Changes

Cohort / File(s)	Summary
File Upload Implementation internal/cmdutil/fileupload.go	Simplified file addition logic: removed io.ReadAll memory read and bytes.Reader wrapper; now passes *os.File directly to fd.AddFile() for SDK-level multipart handling.
File Upload Tests internal/cmdutil/fileupload_test.go	Added reflection-based helper to verify that the "photo" field in Formdata contains exactly *os.File type at runtime, ensuring the SDK receives the file handle directly.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• feat: add --file flag for multipart/form-data file uploads #395 — Directly modifies the same cmdutil.BuildFormdata function to pass *os.File handles and updates corresponding tests to assert the concrete type.

Suggested reviewers

• liangshuo-1

Poem

🐰 A whisker twitch, a file in hand,
No memory heap, just as planned!
The SDK knows the filename's name—
Attachments bloom with rightful fame! 📎✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: preserve filenames for file uploads' directly describes the main change: using the file object directly to preserve multipart filenames instead of reading into a bytes.Reader.
Description check	✅ Passed	The description provides a clear summary, explains the technical change, mentions the fix reference (#729), and lists test commands, though it omits a formal Test Plan section from the template.
Linked Issues check	✅ Passed	The PR preserves uploaded filenames by passing the opened file directly to the SDK, addressing issue #729's primary objective to display correct filenames instead of 'unknown file'.
Out of Scope Changes check	✅ Passed	All changes are focused on preserving filenames during upload; the reflection-based test helper validates the runtime type change and is directly related to the implementation.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.}

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

internal/cmdutil/fileupload.go (1)

111-119: ⚠️ Potential issue | 🟠 Major

Add explicit cleanup for file handles opened in BuildFormdata.

The opened file from fileIO.Open(filePath) is passed directly to fd.AddFile() but never closed. The Lark SDK does not close files passed to Formdata.AddFile, so the caller must handle cleanup. Currently, the file handle escapes BuildFormdata via the returned Formdata and leaks until process exit.

Ensure file closure either by:

• Using a cleanup callback mechanism in the returned Formdata wrapper
• Having callers explicitly close the file after the request completes
• Documenting the ownership contract clearly if callers are responsible

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/cmdutil/fileupload.go` around lines 111 - 119, The BuildFormdata
function currently opens files with fileIO.Open and passes the *os.File to
fd.AddFile (fd.AddFile) but never closes them, leaking file descriptors; modify
BuildFormdata to track opened files and expose a cleanup mechanism on the
returned Formdata (e.g., a Close or Cleanup method on the Formdata wrapper) that
iterates over tracked files and calls Close, or alternatively document and
require callers to call a new CloseFiles helper; ensure the file-opening path in
BuildFormdata records each opened handle (from fileIO.Open) so the cleanup
method can close them after the HTTP request completes.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@internal/cmdutil/fileupload.go`:
- Around line 111-119: The BuildFormdata function currently opens files with
fileIO.Open and passes the *os.File to fd.AddFile (fd.AddFile) but never closes
them, leaking file descriptors; modify BuildFormdata to track opened files and
expose a cleanup mechanism on the returned Formdata (e.g., a Close or Cleanup
method on the Formdata wrapper) that iterates over tracked files and calls
Close, or alternatively document and require callers to call a new CloseFiles
helper; ensure the file-opening path in BuildFormdata records each opened handle
(from fileIO.Open) so the cleanup method can close them after the HTTP request
completes.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b9acdf76-0b9c-4e4a-ba46-2f0c939da26b

📥 Commits

Reviewing files that changed from the base of the PR and between f27b8fd and 929ee2c.

📒 Files selected for processing (2)

• internal/cmdutil/fileupload.go
• internal/cmdutil/fileupload_test.go

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.38%. Comparing base (f27b8fd) to head (929ee2c).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #730   +/-   ##
=======================================
  Coverage   64.38%   64.38%           
=======================================
  Files         512      512           
  Lines       45426    45422    -4     
=======================================
- Hits        29249    29247    -2     
+ Misses      13604    13603    -1     
+ Partials     2573     2572    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add rendylong/cli#fix-file-upload-filename -y -g