feat: support wiki node targets in drive +upload#611
Open
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:
📝 WalkthroughWalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
actor CLI
end
rect rgba(200,255,200,0.5)
participant Runtime
end
rect rgba(255,200,200,0.5)
participant DriveAPI
end
CLI->>Runtime: parse flags (--file, --folder-token, --wiki-token, --name, --dry-run)
Runtime->>Runtime: build driveUploadSpec → derive driveUploadTarget {file_name, parent_type, parent_node}
Runtime->>DriveAPI: upload_prepare (send parent_type, parent_node, file_name)
DriveAPI-->>Runtime: upload_prepare response (upload_all / part URLs)
Runtime->>DriveAPI: upload_all / upload_part(s) (multipart or small upload with file bytes and parent fields)
DriveAPI-->>Runtime: upload_finish
Runtime->>CLI: display progress and result (includes destination label)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
github-actions
Bot
added
domain/ccm
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/cli_e2e/drive/drive_upload_dryrun_test.go (1)
39-40: Tighten the"wiki"assertion.
assert.Contains(t, output, "wiki")is already satisfied by the wiki token itself (wikcnDryRunUploadTarget) and by the command surface strings, so it does not actually proveparent_type="wiki"was emitted. Consider asserting on a more specific substring, e.g. the pair"parent_type": "wiki"(or whatever shape the dry-run JSON uses), so the test catches regressions whereparent_typesilently becomesexplorer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/drive/drive_upload_dryrun_test.go` around lines 39 - 40, The test currently checks output for the generic substring "wiki" which can be satisfied by tokens like "wikcnDryRunUploadTarget"; update the assertion to assert the exact dry-run JSON key/value so it proves parent_type was emitted as wiki—for example replace or add an assertion that output contains the more specific substring `"parent_type": "wiki"` (matching the dry-run JSON format emitted by the upload command) and keep the existing check for "wikcnDryRunUploadTarget" if needed for other coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/cli_e2e/drive/drive_upload_dryrun_test.go`:
- Around line 43-48: Update setDriveDryRunConfigEnv to also isolate config state
by setting LARKSUITE_CLI_CONFIG_DIR (use t.Setenv inside
setDriveDryRunConfigEnv) so the dry-run test does not read real disk configs,
and strengthen the assertion in the drive dry-run test (where assert.Contains(t,
output, "wiki") is used) to check the explicit parent_type value (e.g., assert
that the output contains the JSON key/value for parent_type:"wiki" or equivalent
exact substring) to avoid false positives from tokens like
wikcnDryRunUploadTarget.
---
Nitpick comments:
In `@tests/cli_e2e/drive/drive_upload_dryrun_test.go`:
- Around line 39-40: The test currently checks output for the generic substring
"wiki" which can be satisfied by tokens like "wikcnDryRunUploadTarget"; update
the assertion to assert the exact dry-run JSON key/value so it proves
parent_type was emitted as wiki—for example replace or add an assertion that
output contains the more specific substring `"parent_type": "wiki"` (matching
the dry-run JSON format emitted by the upload command) and keep the existing
check for "wikcnDryRunUploadTarget" if needed for other coverage.
🪄 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: e6d67fd5-345f-41d5-afbf-2ae2cc3243fc
📒 Files selected for processing (7)
shortcuts/drive/drive_io_test.goshortcuts/drive/drive_upload.goskill-template/domains/drive.mdskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-upload.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_upload_dryrun_test.go
wittam-01
force-pushed
the
feat/drive-upload-wiki-target
branch
from
66bc689 to
44caba2
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #611 +/- ##
==========================================
+ Coverage 59.94% 61.17% +1.23%
==========================================
Files 405 428 +23
Lines 42673 45248 +2575
==========================================
+ Hits 25580 27681 +2101
- Misses 15084 15455 +371
- Partials 2009 2112 +103 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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_upload.go`:
- Around line 40-46: The newDriveUploadSpec currently trims all inputs; remove
strings.TrimSpace for FilePath and Name so that newDriveUploadSpec returns
FilePath: runtime.Str("file") and Name: runtime.Str("name") verbatim, while
keeping TrimSpace for FolderToken and WikiToken (i.e., FolderToken:
strings.TrimSpace(runtime.Str("folder-token")) and WikiToken:
strings.TrimSpace(runtime.Str("wiki-token"))) so tokens stay normalized but
user-supplied file paths and explicit upload names are preserved; update the
function body in newDriveUploadSpec and ensure driveUploadSpec consumers expect
raw values.
🪄 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: 1aab9b83-769c-42db-93cc-f7d121a88184
📒 Files selected for processing (6)
shortcuts/drive/drive_io_test.goshortcuts/drive/drive_upload.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-upload.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_upload_dryrun_test.go
✅ Files skipped from review due to trivial changes (1)
- shortcuts/drive/drive_io_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- skills/lark-drive/references/lark-drive-upload.md
- tests/cli_e2e/drive/coverage.md
- skills/lark-drive/SKILL.md
- tests/cli_e2e/drive/drive_upload_dryrun_test.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@07052faeda8fd4231d0225bd909113d7c459598f🧩 Skill updatenpx skills add larksuite/cli#feat/drive-upload-wiki-target -y -g |
wittam-01
force-pushed
the
feat/drive-upload-wiki-target
branch
2 times, most recently
from
50e8cf2 to
84becfc
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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_io_test.go`:
- Around line 956-983: The helper decodeDriveMultipartBody currently treats any
error from reader.NextPart() or buf.ReadFrom(part) as EOF and silently ignores
failures; update decodeDriveMultipartBody to treat non-EOF errors as test
failures by calling t.Fatalf (or t.Fatalf-like) with the error context, i.e.
check if err from reader.NextPart() is io.EOF then break else fail the test, and
check the error returned by buf.ReadFrom(part) and fail the test with details if
non-nil; reference the decodeDriveMultipartBody function and the calls to
reader.NextPart() and buf.ReadFrom(part) when making this change.
In `@tests/cli_e2e/drive/coverage.md`:
- Around line 4-6: The coverage metrics are inconsistent with the command table:
the table lists 29 leaf commands but the metrics show "Denominator: 28", and
dry-run coverage is marked for "drive +apply-permission" while "drive +upload"
has dry-run coverage but remains marked ✕; choose one semantics and make the
counts consistent. Either (A) treat dry-run as covered: update "Denominator" to
29, increment "Covered" to include "drive +upload" so both "drive
+apply-permission" and "drive +upload" are counted, and recalc "Coverage"
percentage; or (B) treat dry-run as not covered: remove dry-run coverage marks
from the table entries (or from "drive +apply-permission"), keep "Denominator:
28", adjust "Covered" and "Coverage" accordingly; apply the same rule
consistently across the file.
🪄 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: b74c4d45-21e2-4476-b883-1801dee27ec3
📒 Files selected for processing (6)
shortcuts/drive/drive_io_test.goshortcuts/drive/drive_upload.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-upload.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_upload_dryrun_test.go
✅ Files skipped from review due to trivial changes (1)
- tests/cli_e2e/drive/drive_upload_dryrun_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- skills/lark-drive/SKILL.md
- skills/lark-drive/references/lark-drive-upload.md
wittam-01
changed the title
fangshuyu-768
previously approved these changes
Apr 23, 2026
wittam-01
force-pushed
the
feat/drive-upload-wiki-target
branch
from
84becfc to
07052fa
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
shortcuts/drive/drive_io_test.go (1)
1018-1030:⚠️ Potential issue | 🟡 MinorHelper still swallows non-EOF multipart errors.
decodeDriveMultipartBodytreats anyNextPart()error as end-of-stream and discardsReadFrom's error via_, _. A malformed boundary or a truncated part will produce a partially decoded body that silently passes the new wiki-target assertions (e.g.,parent_type/parent_nodemissing could look like an assertion failure even though the real problem is a decode error). This is the same issue raised on the previous commit and still applies.🧪 Proposed helper hardening
+ "io" @@ for { part, err := reader.NextPart() + if errors.Is(err, io.EOF) { + break + } if err != nil { - break + t.Fatalf("read multipart part: %v", err) } buf := new(bytes.Buffer) - _, _ = buf.ReadFrom(part) + if _, err := buf.ReadFrom(part); err != nil { + t.Fatalf("read multipart field %q: %v", part.FormName(), err) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_io_test.go` around lines 1018 - 1030, The loop in decodeDriveMultipartBody incorrectly treats any reader.NextPart() error as EOF and swallows ReadFrom errors, producing silently partial decodes; change the NextPart() handling to check for io.EOF and break only on EOF, otherwise return the error from NextPart(), and propagate any error returned by buf.ReadFrom(part) instead of discarding it; ensure you still populate body.Files and body.Fields only after successful ReadFrom and reference the reader.NextPart(), buf.ReadFrom, and body.Files/body.Fields symbols when making the change.tests/cli_e2e/drive/coverage.md (1)
4-28:⚠️ Potential issue | 🟡 MinorMetrics and command table still inconsistent.
The table has 29 rows and 2 ✓ entries (
drive +apply-permissionon line 20 anddrive files create_folderon line 39), but the metrics sayDenominator: 28/Covered: 1. Additionally,drive +uploadnow has dry-run coverage viaTestDriveUploadDryRun_WikiTargetbut remains marked ✕ on line 28 whiledrive +apply-permission(also dry-run only) is marked ✓ — pick one semantic for "dry-run counts" and apply it consistently across table + metrics.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli_e2e/drive/coverage.md` around lines 4 - 28, Update the coverage metrics and command table in coverage.md so the "dry-run counts" policy is applied consistently: decide whether dry-run tests (e.g., TestDrive_ApplyPermissionDryRun, TestDriveUploadDryRun_WikiTarget) should be considered covered or not, then adjust the Denominator, Covered, Coverage percentage and the ✓/✕ marks in the Command Table accordingly; ensure the entries referencing TestDrive_FilesCreateFolderWorkflow, TestDrive_ApplyPermissionDryRun / TestDrive_ApplyPermissionDryRunRejectsFullAccess, and TestDriveUploadDryRun_WikiTarget are updated to reflect that chosen semantic so the table and summary metrics match.
🤖 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_io_test.go`:
- Around line 1018-1030: The loop in decodeDriveMultipartBody incorrectly treats
any reader.NextPart() error as EOF and swallows ReadFrom errors, producing
silently partial decodes; change the NextPart() handling to check for io.EOF and
break only on EOF, otherwise return the error from NextPart(), and propagate any
error returned by buf.ReadFrom(part) instead of discarding it; ensure you still
populate body.Files and body.Fields only after successful ReadFrom and reference
the reader.NextPart(), buf.ReadFrom, and body.Files/body.Fields symbols when
making the change.
In `@tests/cli_e2e/drive/coverage.md`:
- Around line 4-28: Update the coverage metrics and command table in coverage.md
so the "dry-run counts" policy is applied consistently: decide whether dry-run
tests (e.g., TestDrive_ApplyPermissionDryRun, TestDriveUploadDryRun_WikiTarget)
should be considered covered or not, then adjust the Denominator, Covered,
Coverage percentage and the ✓/✕ marks in the Command Table accordingly; ensure
the entries referencing TestDrive_FilesCreateFolderWorkflow,
TestDrive_ApplyPermissionDryRun /
TestDrive_ApplyPermissionDryRunRejectsFullAccess, and
TestDriveUploadDryRun_WikiTarget are updated to reflect that chosen semantic so
the table and summary metrics match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7173356d-377a-4aa5-b44a-85171d9c4ab4
📒 Files selected for processing (6)
shortcuts/drive/drive_io_test.goshortcuts/drive/drive_upload.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-upload.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_upload_dryrun_test.go
✅ Files skipped from review due to trivial changes (1)
- skills/lark-drive/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/cli_e2e/drive/drive_upload_dryrun_test.go
- skills/lark-drive/references/lark-drive-upload.md
fangshuyu-768
approved these changes
Apr 23, 2026
Change-Id: Iaf94270c0a2a2ac02af81c234553ac5850c0668b
wittam-01
force-pushed
the
feat/drive-upload-wiki-target
branch
from
07052fa to
22e22fb
Compare
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 wiki-target upload support to drive +upload by introducing --wiki-token and mapping uploads to parent_type=wiki / parent_node=<wiki_token>. This keeps existing folder/root behavior unchanged while making wiki uploads available through the same shortcut.
Changes
Test Plan
Related Issues
Summary by CodeRabbit
New Features
Improvements
Tests
Documentation