feat: add --file flag for multipart/form-data file uploads#395
Merged
MaxHuang22 merged 7 commits intomainfrom Apr 10, 2026
Merged
feat: add --file flag for multipart/form-data file uploads#395MaxHuang22 merged 7 commits intomainfrom
MaxHuang22 merged 7 commits intomainfrom
Conversation
github-actions
bot
added
the
size/L
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR adds file upload functionality with a Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/CLI
participant Cmd as API/Service Command
participant Util as cmdutil (Validation & Build)
participant FS as File System
participant Server as Lark Server
User->>Cmd: Execute with --file field=path --data {...}
Cmd->>Util: ValidateFileFlag() check
Util-->>Cmd: ✓ valid or error
alt Validation Passed
Cmd->>Util: ParseFileFlag() → field, path, isStdin
Util-->>Cmd: field, path, isStdin
alt --dry-run mode
Cmd->>Util: PrintDryRunWithFile(file metadata)
Util-->>User: Display file field + path (no actual upload)
else Normal execution
Cmd->>Util: BuildFormdata(field, path, form fields)
alt Read from file
Util->>FS: Open and read file
FS-->>Util: file bytes
else Read from stdin
Util->>User: Read stdin
User-->>Util: stdin bytes
end
Util-->>Cmd: multipart/form-data payload
Cmd->>Server: POST/PUT/PATCH/DELETE with WithFileUpload()
Server-->>User: response
end
else Validation Failed
Util-->>User: error (conflicts, unsupported method, etc.)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
MaxHuang22
force-pushed
the
feat/meta_api_file_upload
branch
from
19fb716 to
e77148d
Compare
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@2bb4534ce4286d006d5f5908138307dad9344577🧩 Skill updatenpx skills add larksuite/cli#feat/meta_api_file_upload -y -g |
Greptile SummaryThis PR adds a Confidence Score: 5/5Safe to merge — all remaining findings are P2 style/performance suggestions with no correctness impact. The previously flagged dual-stdin guard issues and the empty dry-run path display are resolved. The two remaining comments (non-deterministic sort in DetectFileFields and double-buffering in BuildFormdata) are non-blocking: the sort issue only affects display order for APIs with multiple file fields (rare) and doesn't affect correctness, while the memory concern is only relevant for large uploads. No P0/P1 issues remain. internal/cmdutil/fileupload.go — DetectFileFields sort order and BuildFormdata memory usage. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["--file flag provided?"] -->|No| B["Normal JSON body path\nParseOptionalBody → request.Data"]
A -->|Yes| C["ValidateFileFlag\n(method, mutual exclusions)"]
C -->|Error| ERR["Return validation error"]
C -->|OK| D["ParseFileFlag\n→ fieldName, filePath, isStdin"]
D --> E["--data provided?"]
E -->|Yes| F["ParseOptionalBody\n→ dataFields (must be map)"]
E -->|No| G["dataFields = nil"]
F --> H{"DryRun?"}
G --> H
H -->|Yes| I["Return FileUploadMeta\n(no file read)"]
H -->|No| J["BuildFormdata\n(read file / stdin)"]
I --> K["PrintDryRunWithFile\n(show field + path metadata)"]
J --> L["request.Data = fd\nWithFileUpload()"]
L --> M["DoAPI"]
Reviews (3): Last reviewed commit: "fix: reject non-object --data with --fil..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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 (5)
cmd/schema/schema_test.go (1)
170-182: Assert the returned field names, not just their count.Right now
TestHasFileFieldswill still pass ifhasFileFieldsreturns the wrong names in a same-length slice. That leaves the default-field selection path under-tested. Comparenamesagainsttt.wantFieldsdirectly.Proposed assertion cleanup
+import "reflect" + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, names := hasFileFields(tt.method) if got != tt.wantBool { t.Errorf("hasFileFields() = %v, want %v", got, tt.wantBool) } - if tt.wantFields == nil && names != nil { - t.Errorf("expected nil names, got %v", names) - } - if tt.wantFields != nil && len(names) != len(tt.wantFields) { - t.Errorf("expected %d field names, got %d", len(tt.wantFields), len(names)) + if !reflect.DeepEqual(names, tt.wantFields) { + t.Errorf("hasFileFields() names = %v, want %v", names, tt.wantFields) } }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/schema/schema_test.go` around lines 170 - 182, The test only checks the length of the returned field names from hasFileFields but not their actual values; update TestHasFileFields to compare the slices directly by asserting names equals tt.wantFields (when tt.wantFields != nil) rather than only comparing lengths—use a slice equality check (e.g., reflect.DeepEqual or cmp.Equal) to fail the test if the values differ, and keep the existing nil checks for consistency; target the hasFileFields call and the local variables names and tt.wantFields in the test.docs/superpowers/specs/2026-04-10-meta-api-file-upload-design.md (3)
115-125: Add language specifier to fenced code block.This help output example should specify
textas the language.📝 Suggested fix
-``` +```text Parameters:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/superpowers/specs/2026-04-10-meta-api-file-upload-design.md` around lines 115 - 125, The fenced code block that contains the CLI help example (the block starting with ``` and the "Parameters:" section) needs a language specifier; change the opening fence from ``` to ```text so the snippet is explicitly marked as plain text (i.e., replace the opening ``` with ```text for the CLI help example).
9-14: Add language specifier to fenced code block.This CLI syntax example should specify a language for proper rendering. Use
textorbashdepending on context.📝 Suggested fix
-``` +```text --file <path> # field name defaults to "file" (raw API) or auto-detected from metadata (meta API) --file <field>=<path> # explicit field name --file - # read from stdin, field name "file" --file <field>=- # read from stdin, explicit field name -``` +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/superpowers/specs/2026-04-10-meta-api-file-upload-design.md` around lines 9 - 14, Update the fenced code block that contains the CLI examples (the block showing the lines beginning with --file <path>, --file <field>=<path>, --file -, --file <field>=-) to include a language specifier (e.g., change the opening ``` to ```text or ```bash) so the snippet renders with proper syntax highlighting.
131-136: Add language specifier to fenced code block.This
--helpoutput example should specifytextas the language.📝 Suggested fix
-``` +```text Flags:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/superpowers/specs/2026-04-10-meta-api-file-upload-design.md` around lines 131 - 136, The fenced code block showing the CLI `--help` example is missing a language specifier; update the Markdown snippet in the document (the triple-backtick block that contains "Flags:" and the `--file`/`--data` lines) to use ```text as the opening fence so the block becomes highlighted as plain text.internal/cmdutil/fileupload.go (1)
95-99: Path validation is already handled by the FileIO interface default implementation, but error context is discarded.The
fileio.FileIO.Open()method's default implementation (LocalFileIO) internally validates paths viaSafeInputPathbefore opening files, so the security concern is addressed. However, the current error message discards the original error details, which hinders debugging. Consider improving the error context:Improvement
} else { f, err := fileIO.Open(filePath) if err != nil { - return nil, output.ErrValidation("cannot open file: %s", filePath) + return nil, output.ErrValidation("cannot open file %q: %v", filePath, err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cmdutil/fileupload.go` around lines 95 - 99, The current branch discards the original error from fileIO.Open(filePath) which makes debugging harder; change the error return so it preserves the underlying error context by including err in the output.ErrValidation message (e.g., combine a descriptive string "cannot open file" with filePath and the original err), keeping the call site in internal/cmdutil/fileupload.go around the fileIO.Open/filePath handling and still returning the ErrValidation type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/api/api.go`:
- Around line 159-162: Before calling cmdutil.BuildFormdata in the upload flow,
validate the user-supplied filePath with validate.SafeInputPath and
reject/return an error for unsafe paths; specifically, in the code path that
prepares fd for cmdutil.BuildFormdata (where
opts.Factory.ResolveFileIO(opts.Ctx), fieldName, filePath, isStdin, stdin,
dataFields are passed), call validate.SafeInputPath(filePath) and only proceed
to invoke cmdutil.BuildFormdata if it succeeds, otherwise return a clear error
to the caller.
- Around line 145-151: When --file (opts.File) is used, reject non-object --data
values: after parsing dataFields via cmdutil.ParseOptionalBody (in the
cmd/api/api.go block handling opts.Data and opts.Method), check that dataFields
is either nil or a JSON object (type map[string]any / map[string]interface{});
if opts.File != "" and dataFields is non-nil but not a map[string]any, return an
error explaining that --data must be a JSON object when --file is provided (this
prevents BuildFormdata from silently dropping arrays/strings/numbers).
In `@cmd/schema/schema.go`:
- Around line 165-171: The help output incorrectly advertises a default upload
field when there are multiple file fields; update the isFileUpload branch that
uses fileFieldNames/defaultField and the fmt.Fprintf calls so that when
len(fileFieldNames) == 1 it keeps the existing "--file <[field=]path>" and
"Default field: %q" behavior, but when len(fileFieldNames) != 1 switch the usage
text to require an explicit field (e.g. "--file <field=path>") and print the
valid field names (from fileFieldNames) instead of claiming a default; apply the
same change to the other identical fmt.Fprintf block that handles file uploads
later in the file.
In `@docs/superpowers/plans/2026-04-10-meta-api-file-upload.md`:
- Line 13: The document has an invalid heading level jump where "### Task 1:
Shared file upload helpers (`internal/cmdutil/fileupload.go`)" and sibling task
headings use H3 under a top-level title; change that line and the other task
headings from "###" to "##" so they are H2 (e.g., update the headings containing
"Task 1: Shared file upload helpers (`internal/cmdutil/fileupload.go`)" and the
other "Task" headings) to resolve the markdownlint MD001 outline error.
---
Nitpick comments:
In `@cmd/schema/schema_test.go`:
- Around line 170-182: The test only checks the length of the returned field
names from hasFileFields but not their actual values; update TestHasFileFields
to compare the slices directly by asserting names equals tt.wantFields (when
tt.wantFields != nil) rather than only comparing lengths—use a slice equality
check (e.g., reflect.DeepEqual or cmp.Equal) to fail the test if the values
differ, and keep the existing nil checks for consistency; target the
hasFileFields call and the local variables names and tt.wantFields in the test.
In `@docs/superpowers/specs/2026-04-10-meta-api-file-upload-design.md`:
- Around line 115-125: The fenced code block that contains the CLI help example
(the block starting with ``` and the "Parameters:" section) needs a language
specifier; change the opening fence from ``` to ```text so the snippet is
explicitly marked as plain text (i.e., replace the opening ``` with ```text for
the CLI help example).
- Around line 9-14: Update the fenced code block that contains the CLI examples
(the block showing the lines beginning with --file <path>, --file
<field>=<path>, --file -, --file <field>=-) to include a language specifier
(e.g., change the opening ``` to ```text or ```bash) so the snippet renders with
proper syntax highlighting.
- Around line 131-136: The fenced code block showing the CLI `--help` example is
missing a language specifier; update the Markdown snippet in the document (the
triple-backtick block that contains "Flags:" and the `--file`/`--data` lines) to
use ```text as the opening fence so the block becomes highlighted as plain text.
In `@internal/cmdutil/fileupload.go`:
- Around line 95-99: The current branch discards the original error from
fileIO.Open(filePath) which makes debugging harder; change the error return so
it preserves the underlying error context by including err in the
output.ErrValidation message (e.g., combine a descriptive string "cannot open
file" with filePath and the original err), keeping the call site in
internal/cmdutil/fileupload.go around the fileIO.Open/filePath handling and
still returning the ErrValidation type.
🪄 Autofix (Beta)
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
Run ID: 0de2549e-57b2-40ac-b9f6-7c0c1982d672
📒 Files selected for processing (11)
cmd/api/api.gocmd/api/api_test.gocmd/schema/schema.gocmd/schema/schema_test.gocmd/service/service.gocmd/service/service_test.godocs/superpowers/plans/2026-04-10-meta-api-file-upload.mddocs/superpowers/specs/2026-04-10-meta-api-file-upload-design.mdinternal/cmdutil/dryrun.gointernal/cmdutil/fileupload.gointernal/cmdutil/fileupload_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
cmd/schema/schema.go (1)
158-165:⚠️ Potential issue | 🟡 MinorDon’t advertise an implicit upload field for multi-file methods.
When
len(fileFieldNames) != 1, there is no schema-derived default. The current help text and CLI example still suggest--file <path>and printDefault field: "file", which points users at a request shape the API may reject.Suggested fix
if isFileUpload { - defaultField := "file" - if len(fileFieldNames) == 1 { - defaultField = fileFieldNames[0] - } - fmt.Fprintf(w, "\n %s--file%s <[field=]path> %sfile upload%s\n", output.Cyan, output.Reset, output.Dim, output.Reset) - fmt.Fprintf(w, " Upload file as multipart/form-data. Default field: %q\n", defaultField) + if len(fileFieldNames) == 1 { + fmt.Fprintf(w, "\n %s--file%s <[field=]path> %sfile upload%s\n", output.Cyan, output.Reset, output.Dim, output.Reset) + fmt.Fprintf(w, " Upload file as multipart/form-data. Default field: %q\n", fileFieldNames[0]) + } else { + fmt.Fprintf(w, "\n %s--file%s <field=path> %sfile upload%s\n", output.Cyan, output.Reset, output.Dim, output.Reset) + fmt.Fprintf(w, " Upload file as multipart/form-data. Valid fields: %s\n", strings.Join(fileFieldNames, ", ")) + } } @@ - if isFileUpload { + if len(fileFieldNames) == 1 { fmt.Fprintf(w, "%sCLI:%s lark-cli %s %s %s --file <path>\n", output.Bold, output.Reset, specName, resName, methodName) + } else if isFileUpload { + fmt.Fprintf(w, "%sCLI:%s lark-cli %s %s %s --file <field=path>\n", output.Bold, output.Reset, specName, resName, methodName) } else { fmt.Fprintf(w, "%sCLI:%s lark-cli %s %s %s\n", output.Bold, output.Reset, specName, resName, methodName) }Also applies to: 207-208
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/schema/schema.go` around lines 158 - 165, The help output currently prints a "Default field" line and an example shape that imply an implicit upload field even for multi-file methods; update the isFileUpload branch so that when len(fileFieldNames) == 1 (use the existing fileFieldNames and defaultField variables) you print the concise example (e.g., "--file <path>") and the "Default field: ..." line, but when len(fileFieldNames) != 1 you print the generic example with the field qualifier (e.g., "--file <[field=]path>") and omit the "Default field" line; make the same change for the duplicate logic around the fmt.Fprintf calls at the later location (the other instance at lines 207-208).internal/cmdutil/fileupload.go (1)
123-127:⚠️ Potential issue | 🟠 MajorReject non-object multipart fields instead of dropping them.
If
dataJSONis an array, string, or number, this branch silently skips every form field and still builds the upload. That turns a valid-looking--datainput into a different request on the wire.Suggested fix
// Add top-level JSON keys as text form fields. - if m, ok := dataJSON.(map[string]any); ok { + if dataJSON != nil { + m, ok := dataJSON.(map[string]any) + if !ok { + return nil, output.ErrValidation("--data must be a JSON object when used with --file") + } for k, v := range m { fd.AddField(k, fmt.Sprintf("%v", v)) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cmdutil/fileupload.go` around lines 123 - 127, The code currently treats only map[string]any for multipart fields and silently ignores other JSON types; update the logic where dataJSON is inspected (variable dataJSON in internal/cmdutil/fileupload.go) to validate that it is an object and return an error if it is not (e.g., when dataJSON is an array, string, number, or nil) instead of continuing and building the upload; ensure the calling function surfaces a clear error message indicating that multipart/form-data --data must be a JSON object and reference the offending input so callers know why fd.AddField was not invoked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/cmdutil/fileupload.go`:
- Around line 109-114: Validate the user-supplied filePath with
validate.SafeInputPath before performing any file I/O: call
validate.SafeInputPath(filePath) and handle its error (returning an appropriate
output.ErrValidation) prior to the fileIO.Open call in this branch (the block
that currently calls fileIO.Open and defers f.Close). Ensure the validation is
applied here so both upload entry points are protected even if a caller forgets
to validate.
---
Duplicate comments:
In `@cmd/schema/schema.go`:
- Around line 158-165: The help output currently prints a "Default field" line
and an example shape that imply an implicit upload field even for multi-file
methods; update the isFileUpload branch so that when len(fileFieldNames) == 1
(use the existing fileFieldNames and defaultField variables) you print the
concise example (e.g., "--file <path>") and the "Default field: ..." line, but
when len(fileFieldNames) != 1 you print the generic example with the field
qualifier (e.g., "--file <[field=]path>") and omit the "Default field" line;
make the same change for the duplicate logic around the fmt.Fprintf calls at the
later location (the other instance at lines 207-208).
In `@internal/cmdutil/fileupload.go`:
- Around line 123-127: The code currently treats only map[string]any for
multipart fields and silently ignores other JSON types; update the logic where
dataJSON is inspected (variable dataJSON in internal/cmdutil/fileupload.go) to
validate that it is an object and return an error if it is not (e.g., when
dataJSON is an array, string, number, or nil) instead of continuing and building
the upload; ensure the calling function surfaces a clear error message
indicating that multipart/form-data --data must be a JSON object and reference
the offending input so callers know why fd.AddField was not invoked.
🪄 Autofix (Beta)
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
Run ID: 1ce91408-b690-44d7-aa06-ecacebc1fae8
📒 Files selected for processing (5)
cmd/api/api.gocmd/schema/schema.gocmd/service/service.gointernal/cmdutil/dryrun.gointernal/cmdutil/fileupload.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/cmdutil/dryrun.go
- cmd/service/service.go
Add ParseFileFlag, ValidateFileFlag, and BuildFormdata to support multipart file upload via --file flag across raw API and meta API commands. Change-Id: Ib724cf8b055b0b314af11d8d830f38559dac60eb
Add --file flag to `lark-cli api` command enabling file upload via multipart/form-data. The flag accepts [field=]path format and supports stdin (-). Includes mutual exclusion validation with --output, --page-all, and GET method. Dry-run mode shows file metadata instead of building actual formdata. Change-Id: Icf34aba5da3a558219a97a583e8f6aa951ded199
Add file upload support to meta API service method commands. The --file flag is conditionally registered only for methods whose metadata declares file-type fields (POST/PUT/PATCH/DELETE). The default field name is auto-detected from metadata when exactly one file field exists. Change-Id: Ibbf04eb42341ba11bb1fd9750e63bc1d0eacd08d
Add hasFileFields helper to detect file-type fields in requestBody metadata. Modify printMethodDetail to display [file upload] tag on --data line, --file flag description with default field name, and --file <path> in CLI example for methods that accept file uploads. Change-Id: Iae3bc14fe07e16a8b5f6a50a2b3592d6d8490ed9
- ParseFileFlag: change idx >= 0 to idx > 0 to prevent empty field name when input like "=photo.jpg" is passed - BuildFormdata: read file into bytes.Reader with defer Close to prevent file handle leak on later errors - BuildFormdata: remove unused ctx parameter from signature and callers - Eliminate duplicated dry-run logic by having buildAPIRequest and buildServiceRequest return FileUploadMeta when in dry-run mode, removing ~60 lines of copy-pasted URL building and validation code Change-Id: I27b9534fd0eaefce40390f6e723dd0c04a2cdf80
- Remove opts.File=="" guard on dual-stdin check so --file photo.jpg --params - --data - correctly reports an error instead of silently dropping --data content (P1 bug in both api.go and service.go) - Extract shared DetectFileFields into cmdutil, deduplicate detectFileFields (service.go) and hasFileFields (schema.go) - Show "<stdin>" instead of empty path in dry-run output for --file - Change-Id: Iccc5d879165ea6a3d04f0425ec6a5018a10e72e1
- --data with --file now requires a JSON object; arrays/strings/numbers are rejected with a clear error instead of being silently dropped - Schema display for multi-file methods shows explicit field=path syntax and lists valid field names instead of advertising a false default Change-Id: I0facdb3ad86f68cb125c7ea109a33714fd91dba0
MaxHuang22
force-pushed
the
feat/meta_api_file_upload
branch
from
85d3cf7 to
2bb4534
Compare
liangshuo-1
approved these changes
Apr 10, 2026
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
--fileflag to both rawapicommand and meta API service commands for multipart/form-data file uploadsimageforim.images.create)--file -) and explicit field prefix (--file image=photo.jpg)[file upload]indicator and--fileusage inschemadisplayinternal/cmdutil/fileupload.gofor parsing, validation, and form buildingTest plan
lark-cli schema im.images.createshows[file upload]tag and--filedescription--filein schema or--help--file+GET,--file+--output,--file+--page-all, dual stdincmd/api,cmd/service,internal/cmdutilSummary by CodeRabbit
Release Notes
New Features
--fileflag to upload files via multipart form-data in API and service commands, supporting both explicit field names and default field mapping.Tests