refactor: migrate drive/doc/sheets shortcuts to FileIO#339
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR migrates shortcut file I/O to the runtime FileIO abstraction, adds Changes
Sequence Diagram(s)sequenceDiagram
participant Shortcut as Shortcut
participant Remote as Remote API
participant Runtime as runtime.FileIO()
participant Output as CLI Output
Shortcut->>Remote: request file / upload endpoint
Remote-->>Shortcut: response (body, content-type, length)
Shortcut->>Runtime: ResolveSavePath(outputPath)
alt resolve success
Runtime-->>Shortcut: resolvedPath
Shortcut->>Runtime: Stat(resolvedPath)
alt exists & !overwrite
Runtime-->>Shortcut: ErrExists
Shortcut->>Output: return validation error (use --overwrite)
else
Shortcut->>Runtime: Save(resolvedPath, SaveOptions, resp.Body)
Runtime-->>Shortcut: SaveResult (size, path)
Shortcut->>Output: report saved_path, size_bytes
end
else resolve failed
Runtime-->>Shortcut: ErrValidation
Shortcut->>Output: return validation error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
github-actions
Bot
added
domain/ccm
Greptile SummaryThis PR completes the FileIO migration for Confidence Score: 4/5Safe to merge after fixing the missing One P1 finding: shortcuts/doc/doc_media_preview.go — missing savedPath fallback at line 118.
|
| Filename | Overview |
|---|---|
| shortcuts/doc/doc_media_preview.go | Migrated to FileIO.Save; missing savedPath fallback that all sibling callers include, causing potential empty saved_path output field. |
| shortcuts/drive/drive_download.go | Migrated to FileIO.Stat/Save; early overwrite check uses Stat (SafeInputPath semantics) on an output path — previously flagged, Save re-validates via SafeOutputPath so no safety hole. |
| shortcuts/drive/drive_upload.go | Multipart upload replaced manual seek+close pattern with io.NewSectionReader; file re-opened per block with close after DoAPI (already flagged in previous review). |
| shortcuts/drive/drive_export_common.go | Migrated saveContentToOutputDir to accept fileio.FileIO, removed vfs/validate calls; includes resolvedPath fallback correctly. |
| shortcuts/common/runner.go | Adds WrapInputStatError helper that cleanly maps path-validation vs I/O errors into ErrValidation; implementation is correct. |
| shortcuts/common/validate.go | ValidateSafeOutputDir simplified to delegate to FileIO.ResolvePath; all callers updated to pass the new fio parameter. |
| extension/fileio/types.go | Adds Mode() fs.FileMode to FileInfo interface, enabling IsRegular() checks across all callers. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User invokes shortcut] --> B[runtime.ResolveSavePath - early validation]
B -->|error| C[Return ErrValidation]
B -->|ok| D[FileIO.Stat - overwrite check]
D -->|exists & !overwrite| E[Return ErrValidation]
D -->|not exists or overwrite| F[API call]
F --> G[FileIO.Save]
G -->|error| H[WrapSaveErrorByCategory]
G -->|ok| I[runtime.ResolveSavePath - get absolute path]
I -->|empty string| J[fallback to original path ✓ most shortcuts]
I -->|empty string, no fallback| K[saved_path: empty string ✗ doc_media_preview]
I -->|resolved| L[runtime.Out saved_path + size_bytes]
Reviews (6): Last reviewed commit: "Revert "fix: preserve --output-dir flag ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@1f1c9186632a35132b920cdf7f56ca6eb1c458aa🧩 Skill updatenpx skills add larksuite/cli#feat/fileio-migrate-drive-doc-sheets -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/doc/doc_media_download.go`:
- Around line 69-70: Validate the actual file that will be written (finalPath)
instead of only the raw --output (outputPath): after computing finalPath
(including any auto-appended extension) call runtime.ValidatePath(finalPath) and
return a validation error on failure; also update the overwrite gate around file
Stat to not swallow non-ENOENT errors—propagate Stat errors (other than
os.IsNotExist) so Save won't proceed silently, and use finalPath in any
subsequent existence/overwrite checks and error messages (referencing
runtime.ValidatePath, finalPath, outputPath, Save, and the overwrite gate/Stat
logic).
In `@shortcuts/drive/drive_download.go`:
- Around line 73-79: The failure path after calling runtime.FileIO().Save (the
Save call on the FileIO obtained via runtime.FileIO()) incorrectly wraps the
error as "api_error"; change the category passed to
common.WrapSaveErrorByCategory to the I/O category (e.g., "io_error" or the
project constant for I/O errors) so disk/save failures are classified correctly;
update the call site where result, err := runtime.FileIO().Save(...) is checked
and replace the "api_error" argument with the I/O error category.
- Around line 55-60: The current check conflates output-path safety and
overwrite probing by calling runtime.FileIO().Stat directly; instead first
resolve/validate the intended write path (e.g., normalize/clean/resolve
outputPath and ensure its parent directory is writable and not a directory
target) and only then call runtime.FileIO().Stat to determine existence/type for
overwrite logic. Update the block in drive_download.go around
runtime.FileIO().Stat so you: (1) resolve/clean outputPath and verify its parent
exists and is a directory and writable; (2) call
runtime.FileIO().Stat(outputPath) solely to check if the target already exists
and whether it is a file vs directory; and (3) if statErr == nil and target is a
directory, return a validation error, otherwise enforce overwrite only when a
regular file exists and --overwrite is set. Ensure error messages reference
outputPath and preserve existing output.ErrValidation usage.
In `@shortcuts/drive/drive_upload.go`:
- Around line 61-65: After calling runtime.FileIO().Stat(filePath) and before
using info.Size(), validate that the file is a regular file by checking
info.Mode().IsRegular(); if it is not regular return an input validation error
(use the existing common.WrapInputStatError or the project's input-error helper)
referencing filePath so directories, FIFOs, and device files are rejected early
and Size()/Open/upload are not attempted on non-regular inputs.
In `@shortcuts/sheets/sheet_export.go`:
- Around line 131-136: The error returned from runtime.FileIO().Save (called
with outputPath, fileio.SaveOptions and resp.Body) represents local filesystem
failures but is being wrapped with common.WrapSaveErrorByCategory(err,
"api_error"); change the category to the IO category (e.g., "io") so
disk/mkdir/write errors are classified correctly—update the call site that wraps
the error from runtime.FileIO().Save to use the proper "io" category when
returning the wrapped error.
- Around line 65-68: The code incorrectly uses runtime.ValidatePath on the
output target (outputPath), which runs FileIO.Stat and can misclassify ordinary
stat failures and bypass the write-path resolver; replace the ValidatePath
preflight with runtime.ResolveSavePath(outputPath) and return the same
validation error (e.g., via output.ErrValidation) if ResolveSavePath returns an
error so the write-path resolver is exercised before export (update the block
that references outputPath, runtime.ValidatePath, and output.ErrValidation
accordingly).
🪄 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: 83f6ef71-a15a-4477-935c-d7a3b817dd6e
📒 Files selected for processing (14)
extension/fileio/types.goshortcuts/common/runner.goshortcuts/doc/doc_media_download.goshortcuts/doc/doc_media_insert.goshortcuts/doc/doc_media_upload.goshortcuts/drive/drive_download.goshortcuts/drive/drive_export.goshortcuts/drive/drive_export_common.goshortcuts/drive/drive_export_test.goshortcuts/drive/drive_import.goshortcuts/drive/drive_import_common.goshortcuts/drive/drive_import_test.goshortcuts/drive/drive_upload.goshortcuts/sheets/sheet_export.go
This was referenced Apr 8, 2026
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/sheets/sheet_export.go (1)
114-119:⚠️ Potential issue | 🔴 CriticalMissing
returnstatement causes unintended download when--output-pathis omitted.When
outputPathis empty, the code outputsfile_token/ticketbut then falls through to the download and save logic, which will fail becauseFileIO.Save("")rejects the empty path. The user sees a successful export immediately followed by an "unsafe output path" error.🐛 Proposed fix to return early when no output path is specified
if outputPath == "" { runtime.Out(map[string]interface{}{ "file_token": fileToken, "ticket": ticket, }, nil) + return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/sheets/sheet_export.go` around lines 114 - 119, The branch that handles an empty outputPath currently calls runtime.Out(...) but doesn't return, so execution falls through into the download/save logic and triggers FileIO.Save("") errors; modify the export function in sheet_export.go to return immediately after emitting runtime.Out when outputPath == "" (i.e., add a return following the runtime.Out call) so that the subsequent download code (the FileIO.Save and related download logic) is skipped when no --output-path is provided.
🤖 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 `@shortcuts/sheets/sheet_export.go`:
- Around line 114-119: The branch that handles an empty outputPath currently
calls runtime.Out(...) but doesn't return, so execution falls through into the
download/save logic and triggers FileIO.Save("") errors; modify the export
function in sheet_export.go to return immediately after emitting runtime.Out
when outputPath == "" (i.e., add a return following the runtime.Out call) so
that the subsequent download code (the FileIO.Save and related download logic)
is skipped when no --output-path is provided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4821a3b-f2e5-496b-9494-35e86e3cb690
📒 Files selected for processing (3)
shortcuts/doc/doc_media_download.goshortcuts/drive/drive_download.goshortcuts/sheets/sheet_export.go
🚧 Files skipped from review as they are similar to previous changes (2)
- shortcuts/doc/doc_media_download.go
- shortcuts/drive/drive_download.go
- drive_download/upload/import/export: SafeInputPath/SafeOutputPath + vfs.Stat/Open/MkdirAll + AtomicWrite → FileIO.Stat/Open/Save - doc_media_download/insert/upload: same migration pattern - sheet_export: same migration pattern - Add Mode() fs.FileMode to fileio.FileInfo for IsRegular() checks - Add WrapInputStatError helper to preserve error message fidelity - Add WrapSaveErrorByCategory for standardized save error mapping Change-Id: I1d986ffea55b1ec63b136a8a037addc51821a4c3
…/sheets migration Change-Id: Ic06ee36b7cfba6be4e9dfef76cff18a7fd4c9290
ResolveSavePath returns a resolved absolute path, but FileIO.Stat internally applies SafeInputPath which re-resolves differently, causing the overwrite check to miss existing files. Change-Id: Icf9f9991fc89fc80e472a41eb17ae1d25784346f
- drive_media_upload.go: replace vfs.Open with runtime.FileIO().Open() - helpers.go: remove EnsureWritableFile (no remaining callers) - validate.go: replace ValidateSafeOutputDir internals with FileIO.ResolvePath - vc_notes.go: pass FileIO to ValidateSafeOutputDir Change-Id: I998f4009a294a440bb66c42bc8f4744e1bf55257
There was a problem hiding this comment.
♻️ Duplicate comments (1)
shortcuts/drive/drive_download.go (1)
58-60:⚠️ Potential issue | 🟡 MinorReject directory targets explicitly before save when
--overwriteis true.Current logic only blocks existing paths when overwrite is false. If the target exists and is a directory, the flow falls through to
Saveand returns an internal I/O-style error instead of a validation error for bad user input.🔧 Proposed fix
- if _, statErr := runtime.FileIO().Stat(outputPath); statErr == nil && !overwrite { - return output.ErrValidation("output file already exists: %s (use --overwrite to replace)", outputPath) - } + if fi, statErr := runtime.FileIO().Stat(outputPath); statErr == nil { + if fi.Mode().IsDir() { + return output.ErrValidation("output path is a directory: %s", outputPath) + } + if !overwrite { + return output.ErrValidation("output file already exists: %s (use --overwrite to replace)", outputPath) + } + }#!/bin/bash set -euo pipefail echo "1) Inspect overwrite gate in shortcuts/drive/drive_download.go" rg -n -C3 'ResolveSavePath|FileIO\(\)\.Stat\(outputPath\)|overwrite|Mode\(' shortcuts/drive/drive_download.go echo echo "2) Inspect FileInfo API for Mode() availability" rg -n -C3 'type FileInfo interface|Mode\(\)' extension/fileio/types.go echo echo "3) Inspect LocalFileIO Save implementation to confirm directory-target handling path" fd -i localfileio.go internal/vfs | xargs -I{} rg -n -C4 'func \(.*\) Save\(|IsDir|OpenFile|create file|atomic' {}Expected verification result: the current overwrite gate does not branch on
fi.Mode().IsDir(), whileFileInfo.Mode()is available for this pre-check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_download.go` around lines 58 - 60, The existing check uses runtime.FileIO().Stat(outputPath) and only respects overwrite, so if outputPath exists as a directory and overwrite is true the code falls through to Save and causes an internal IO error; update the pre-save validation to call runtime.FileIO().Stat(outputPath), inspect the returned FileInfo.Mode().IsDir() (or FileInfo interface) and if the path is a directory return output.ErrValidation("output path is a directory: %s", outputPath) regardless of the overwrite flag; keep the existing duplicate-file check when !overwrite but add this explicit IsDir check before calling Save to reject directory targets up-front.
🧹 Nitpick comments (1)
shortcuts/common/drive_media_upload.go (1)
170-172: Optional: extract the repeated input-open pattern into a helper.
Open(...)+WrapInputStatError(...)appears in both upload paths; a tiny helper would reduce duplication and keep future error-message tweaks in one place.Refactor sketch
+func openInputFile(runtime *RuntimeContext, path string) (interface{ Close() error }, error) { + f, err := runtime.FileIO().Open(path) + if err != nil { + return nil, WrapInputStatError(err) + } + return f, nil +} + func UploadDriveMediaAll(runtime *RuntimeContext, cfg DriveMediaUploadAllConfig) (string, error) { - f, err := runtime.FileIO().Open(cfg.FilePath) + f, err := openInputFile(runtime, cfg.FilePath) if err != nil { - return "", WrapInputStatError(err) + return "", err } defer f.Close() @@ func uploadDriveMediaMultipartParts(runtime *RuntimeContext, filePath string, fileSize int64, session DriveMediaMultipartUploadSession) error { - f, err := runtime.FileIO().Open(filePath) + f, err := openInputFile(runtime, filePath) if err != nil { - return WrapInputStatError(err) + return err } defer f.Close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/common/drive_media_upload.go` around lines 170 - 172, The code repeats the pattern runtime.FileIO().Open(filePath) followed by returning WrapInputStatError(err); extract this into a small helper (e.g., openInputFile or openAndWrapInputError) that accepts the filePath, calls runtime.FileIO().Open, and returns the opened file (and any wrapped error) so both upload paths call that helper instead of duplicating runtime.FileIO().Open and WrapInputStatError; update callers (the two upload paths) to use the new helper and preserve existing error wrapping behavior and return types.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@shortcuts/drive/drive_download.go`:
- Around line 58-60: The existing check uses runtime.FileIO().Stat(outputPath)
and only respects overwrite, so if outputPath exists as a directory and
overwrite is true the code falls through to Save and causes an internal IO
error; update the pre-save validation to call runtime.FileIO().Stat(outputPath),
inspect the returned FileInfo.Mode().IsDir() (or FileInfo interface) and if the
path is a directory return output.ErrValidation("output path is a directory:
%s", outputPath) regardless of the overwrite flag; keep the existing
duplicate-file check when !overwrite but add this explicit IsDir check before
calling Save to reject directory targets up-front.
---
Nitpick comments:
In `@shortcuts/common/drive_media_upload.go`:
- Around line 170-172: The code repeats the pattern
runtime.FileIO().Open(filePath) followed by returning WrapInputStatError(err);
extract this into a small helper (e.g., openInputFile or openAndWrapInputError)
that accepts the filePath, calls runtime.FileIO().Open, and returns the opened
file (and any wrapped error) so both upload paths call that helper instead of
duplicating runtime.FileIO().Open and WrapInputStatError; update callers (the
two upload paths) to use the new helper and preserve existing error wrapping
behavior and return types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 807b838f-f91d-4ab7-9d19-c1f7914427de
📒 Files selected for processing (7)
shortcuts/common/common_test.goshortcuts/common/drive_media_upload.goshortcuts/common/helpers.goshortcuts/common/validate.goshortcuts/common/validate_test.goshortcuts/drive/drive_download.goshortcuts/vc/vc_notes.go
💤 Files with no reviewable changes (2)
- shortcuts/common/common_test.go
- shortcuts/common/helpers.go
Align with other doc shortcuts: replace manual SafeOutputPath + EnsureWritableFile + vfs.MkdirAll + AtomicWriteFromReader with FileIO.Save/Stat/ResolveSavePath. Change-Id: Iad2e71fce2574ffe7027f3194acd776612b0cc85
tuxedomm
force-pushed
the
feat/fileio-migrate-drive-doc-sheets
branch
from
4610d2a to
87d86f5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/sheets/sheet_export.go (1)
114-119:⚠️ Potential issue | 🟠 MajorReturn immediately when
--output-pathis omitted.Line 114 treats the download as optional, but that branch never returns. The new save block at Line 131 still runs with
outputPath == "", so a successful export can fail locally instead of returning thefile_token/ticketmetadata.🔧 Proposed fix
if outputPath == "" { runtime.Out(map[string]interface{}{ "file_token": fileToken, "ticket": ticket, }, nil) + return nil } // Download resp, err := runtime.DoAPIStream(ctx, &larkcore.ApiReq{Also applies to: 131-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/sheets/sheet_export.go` around lines 114 - 119, The branch that handles the case when outputPath == "" writes metadata via runtime.Out (fileToken, ticket) but does not return, so the subsequent local save logic still runs and can fail; update the function in sheet_export.go to return immediately after the runtime.Out call when outputPath == "" (i.e., guard the save/export block so it only executes when outputPath != ""), ensuring the fileToken/ticket path exits early and the later save logic (the block around the save/export at lines ~131-145) is skipped.
♻️ Duplicate comments (1)
shortcuts/doc/doc_media_download.go (1)
114-118:⚠️ Potential issue | 🟠 MajorDon't swallow non-
ErrNotExistfailures in the overwrite check.Line 116 has the same gap: any
Staterror other than nil is treated as "file missing". That can bypass--overwrite=falseon unreadable existing files and defers the real error until after the download completes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/doc/doc_media_download.go` around lines 114 - 118, The overwrite check around runtime.FileIO().Stat(finalPath) incorrectly treats any Stat error as "missing"; update the logic in that block (the overwrite conditional using runtime.FileIO().Stat and finalPath) to: if statErr == nil -> return ErrValidation that file exists; else if !os.IsNotExist(statErr) -> return the Stat error (wrap or propagate a descriptive error) so unreadable or other filesystem errors aren't swallowed; keep the existing behavior only when os.IsNotExist(statErr).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/drive/drive_export_common.go`:
- Around line 267-275: The overwrite guard incorrectly treats any non-nil error
from fio.Stat as "file missing"; update the check around fio.Stat(target) (used
in the overwrite conditional with the overwrite variable and target) to only
treat fs.ErrNotExist as the missing-file case and return output.ErrValidation in
that case, while for any other stat error return or wrap that error (not fall
through to fio.Save) so callers see the real failure (use
common.WrapSaveErrorByCategory or a suitable wrapper for non-existence errors
where appropriate); ensure fio.Save's existing error handling
(common.WrapSaveErrorByCategory) remains unchanged.
---
Outside diff comments:
In `@shortcuts/sheets/sheet_export.go`:
- Around line 114-119: The branch that handles the case when outputPath == ""
writes metadata via runtime.Out (fileToken, ticket) but does not return, so the
subsequent local save logic still runs and can fail; update the function in
sheet_export.go to return immediately after the runtime.Out call when outputPath
== "" (i.e., guard the save/export block so it only executes when outputPath !=
""), ensuring the fileToken/ticket path exits early and the later save logic
(the block around the save/export at lines ~131-145) is skipped.
---
Duplicate comments:
In `@shortcuts/doc/doc_media_download.go`:
- Around line 114-118: The overwrite check around
runtime.FileIO().Stat(finalPath) incorrectly treats any Stat error as "missing";
update the logic in that block (the overwrite conditional using
runtime.FileIO().Stat and finalPath) to: if statErr == nil -> return
ErrValidation that file exists; else if !os.IsNotExist(statErr) -> return the
Stat error (wrap or propagate a descriptive error) so unreadable or other
filesystem errors aren't swallowed; keep the existing behavior only when
os.IsNotExist(statErr).
🪄 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: a088d3a7-599d-4643-a09d-ef1fa162d022
📒 Files selected for processing (21)
extension/fileio/types.goshortcuts/common/common_test.goshortcuts/common/drive_media_upload.goshortcuts/common/helpers.goshortcuts/common/runner.goshortcuts/common/validate.goshortcuts/common/validate_test.goshortcuts/doc/doc_media_download.goshortcuts/doc/doc_media_insert.goshortcuts/doc/doc_media_preview.goshortcuts/doc/doc_media_upload.goshortcuts/drive/drive_download.goshortcuts/drive/drive_export.goshortcuts/drive/drive_export_common.goshortcuts/drive/drive_export_test.goshortcuts/drive/drive_import.goshortcuts/drive/drive_import_common.goshortcuts/drive/drive_import_test.goshortcuts/drive/drive_upload.goshortcuts/sheets/sheet_export.goshortcuts/vc/vc_notes.go
💤 Files with no reviewable changes (2)
- shortcuts/common/common_test.go
- shortcuts/common/helpers.go
✅ Files skipped from review due to trivial changes (5)
- shortcuts/drive/drive_import_common.go
- shortcuts/drive/drive_import_test.go
- shortcuts/drive/drive_export_test.go
- shortcuts/common/drive_media_upload.go
- shortcuts/drive/drive_download.go
🚧 Files skipped from review as they are similar to previous changes (5)
- extension/fileio/types.go
- shortcuts/drive/drive_upload.go
- shortcuts/doc/doc_media_insert.go
- shortcuts/vc/vc_notes.go
- shortcuts/common/runner.go
FileIO.ResolvePath reports errors with --output prefix; rewrite to --output-dir to match the actual flag name exposed to users. Change-Id: If7f55d0f8a633e1ccb188ecc44a676c25cce5efd
… errors" This reverts commit 2e09e00.
fangshuyu-768
approved these changes
Apr 9, 2026
This was referenced Apr 9, 2026
3 tasks
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
drive_download/upload/import/export:SafeInputPath/SafeOutputPath+vfs.Stat/Open/MkdirAll+AtomicWrite→FileIO.Stat/Open/Savedoc_media_download/insert/upload: same migration patternsheet_export: same migration patternextension/fileio/types.go: addMode() fs.FileModetoFileInfoforIsRegular()checksshortcuts/common/runner.go: addWrapInputStatErrorhelper to preserve error message fidelityPart of the FileIO shortcut migration series:
Test plan
go test ./shortcuts/drive/...— all passgo test ./shortcuts/doc/...— all passgo test ./shortcuts/sheets/...— all passgo test ./shortcuts/common/...— all passgolangci-lint run— no new issueslark-cli drive +download/+upload/+import/+exportlark-cli docs +media-download/+media-insert/+media-uploadlark-cli sheets +exportSummary by CodeRabbit
Refactor
Bug Fixes
Chores
Tests
Manual Test Checklist
This PR migrates file IO in drive/doc/sheets/common/vc from direct vfs calls to the FileIO abstraction. Below are the functional paths that need manual verification:
Drive
drive +upload <file>— small file (<20MB) uploaddrive +upload <file>— large file (>20MB) multipart uploaddrive +download --file-token <token> --output <path>— download to specified pathdrive +download— without--overwrite, should error if target file existsdrive +import --file <path>— import document (small file)drive +import --file <path>— import document (>20MB large file): multipart upload path verified (6 chunks uploaded successfully); server rejected fake .docx content as expecteddrive +export --file-token <token>— export to default directorydrive +export --file-token <token> --output-dir <dir>— export to specified subdirectoryDoc
docs +media-upload --file <path> --parent-type <type>— small file media upload: verified via+media-insert(which calls the same upload code path internally)docs +media-upload --file <path>— large file (>20MB) multipart media upload: verified via+media-insertwith 21MB file (6-chunk multipart upload succeeded)docs +media-download --token <token> --output <path>— media downloaddocs +media-download— without--overwrite, repeated download should errordocs +media-preview --token <token> --output <path>— media preview downloaddocs +media-insert --doc-token <token> --file <path>— insert image into documentSheets
sheets +export --spreadsheet-token <token> --output <path>— export spreadsheetsheets +export— without--overwrite, repeated export succeeds (note: no--overwriteflag exists; this is pre-existing behavior on main, not a regression)Common (media upload)
UploadDriveMediaAll/UploadDriveMediaMultipart)VC
vc +notes --output-dir ../outside— path traversal should be rejectedGeneral boundary checks
--output ../../etc/passwd) should be rejected22/22 items verified. 0 regressions found. Compared key operations against main branch v1.0.3 — behavior is identical.