Skip to content

feat(sheets): add sheet management shortcuts#722

Open
caojie0621 wants to merge 2 commits intomainfrom
feat/sheet
Open

feat(sheets): add sheet management shortcuts#722
caojie0621 wants to merge 2 commits intomainfrom
feat/sheet

Conversation

@caojie0621
Copy link
Copy Markdown
Collaborator

@caojie0621 caojie0621 commented Apr 29, 2026

Summary

This PR adds sheet management shortcuts for Sheets so agents can create, copy,
delete, and update sheets without hand-writing raw API payloads. It also fixes
the related validation gaps and consolidates the lark-sheets reference docs
from per-command files into grouped capability-based docs.

Changes

  • Add +create-sheet, +copy-sheet, +delete-sheet, and +update-sheet in shortcuts/sheets/sheet_manage.go
  • Add unit tests and dry-run E2E coverage for request shapes and validation behavior
  • Consolidate skills/lark-sheets/references from 43 per-command files into 10 grouped docs:
    • lark-sheets-spreadsheet-management.md
    • lark-sheets-sheet-management.md
    • lark-sheets-cell-data.md
    • lark-sheets-cell-style-and-merge.md
    • lark-sheets-cell-images.md
    • lark-sheets-row-column-management.md
    • lark-sheets-filter-views.md
    • lark-sheets-dropdown.md
    • lark-sheets-float-images.md
    • lark-sheets-formula.md

Test Plan

  • make unit-test
  • go test ./shortcuts/sheets

Summary by CodeRabbit

  • New Features

    • Added sheet-level commands: create, copy, update, delete sheets
    • Expanded cell commands: read, write, append, find, replace
    • Added cell styling & merge: set-style, batch-style, merge/unmerge cells
    • Row/column management: add/insert/update/move/delete dimensions
    • Media support: upload images for floating/cell image workflows
  • Documentation

    • Reorganized and consolidated Sheets reference docs; bumped skill version to 1.2.0

@github-actions github-actions Bot added domain/ccm PR touches the ccm domain size/L Large or sensitive change across domains or core paths labels Apr 29, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 29, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Consolidates Sheets CLI shortcuts into domain modules (cell data, styles/merge, sheet management, filter views, float images, row/column management), adds sheet-level CRUD commands and many tests, removes many legacy per-command files, and adds/updates grouped reference documentation and E2E coverage entries.

Changes

Cohort / File(s) Summary
Shortcut registry
shortcuts/sheets/shortcuts.go
Reordered and re-grouped shortcuts; added SheetCreateSheet, SheetCopySheet, SheetDeleteSheet, SheetUpdateSheet and category comments.
Sheet management (new)
shortcuts/sheets/lark_sheets_sheet_management.go, shortcuts/sheets/lark_sheets_sheet_manage_test.go
Implements +create-sheet, +copy-sheet, +delete-sheet, +update-sheet with validation, batch-update request builders, response normalization, and tests (dry-run + execute flows).
Spreadsheet management (new)
shortcuts/sheets/lark_sheets_spreadsheet_management.go
Implements +info, +create, +export with token/url handling, optional header/data append, export task polling and optional download.
Cell data (new)
shortcuts/sheets/lark_sheets_cell_data.go, skills/lark-sheets/references/lark-sheets-cell-data.md
Adds +read, +write, +append, +find, +replace implementations and a consolidated reference doc; includes range normalization, find/replace condition construction, dry-run support.
Cell style & merge (new)
shortcuts/sheets/lark_sheets_cell_style_and_merge.go, skills/lark-sheets/references/lark-sheets-cell-style-and-merge.md
Adds +set-style, +batch-set-style, +merge-cells, +unmerge-cells with JSON parsing, batch support, and docs.
Filter views (new)
shortcuts/sheets/lark_sheets_filter_views.go, skills/lark-sheets/references/lark-sheets-filter-views.md
Adds create/update/list/get/delete filter-view shortcuts with path/token helpers and validation; new reference doc.
Float images / media-upload (new)
shortcuts/sheets/lark_sheets_float_images.go, skills/lark-sheets/references/lark-sheets-float-images.md
Adds +media-upload with single-part vs multipart routing (20MB threshold), file validation, dry-run, and docs.
Row/column (new)
shortcuts/sheets/lark_sheets_row_column_management.go, skills/lark-sheets/references/lark-sheets-row-column-management.md
Implements +add-dimension, +insert-dimension, +update-dimension, +move-dimension, +delete-dimension with index/constraint validation and dry-run/execute flows.
New tests / E2E
tests/cli_e2e/sheets/sheets_sheet_shortcuts_dryrun_test.go, tests/cli_e2e/sheets/sheets_sheet_shortcuts_workflow_test.go, multiple unit tests under shortcuts/sheets/*_test.go
Adds dry-run and live workflow E2E tests for sheet shortcuts, and unit tests tightening validation for styles/filter views/media upload/range handling.
References — created
skills/lark-sheets/references/* (e.g., lark-sheets-cell-images.md, lark-sheets-sheet-management.md, etc.)
Adds grouped, consolidated reference docs for cell-data, cell-images, cell-style-and-merge, dropdowns, filter-views, float-images, row-column-management, sheet-management, spreadsheet-management, and updates SKILL.md.
Legacy implementations — deleted
shortcuts/sheets/sheet_*.go (many files removed, e.g., sheet_create.go, sheet_read.go, sheet_write.go, sheet_* others)
Removes numerous per-command legacy files; their functionality moved into the new consolidated modules.
Legacy references — deleted
skills/lark-sheets/references/lark-sheets-*.md (many per-command docs removed)
Removes many individual command reference pages; content consolidated into new grouped docs.
Coverage report
tests/cli_e2e/sheets/coverage.md
Updates CLI E2E coverage metrics and documents added dry-run and live workflow suites and newly covered sheet commands.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as CLI (user)
    participant Runtime as CLI runtime
    participant SheetsAPI as Sheets API
    participant Drive as Drive / Permission service
    rect rgba(200,200,255,0.5)
    CLI->>Runtime: invoke `sheets +create-sheet --title ... [--data]`
    Runtime->>SheetsAPI: POST /open-apis/sheets/v3/spreadsheets (create)
    SheetsAPI-->>Runtime: 201 {spreadsheet_token, sheets[...] }
    Runtime->>SheetsAPI: POST /open-apis/sheets/v2/spreadsheets/{token}/values_append (optional header/data)
    SheetsAPI-->>Runtime: 200 {append result}
    Runtime->>Drive: (optional) grant permission / record permission_grant (bot-mode)
    Drive-->>Runtime: permission result
    Runtime-->>CLI: print normalized output (spreadsheet_token, url, permission_grant, appended info)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

size/L

Suggested reviewers

  • fangshuyu-768
  • kongenpei

Poem

🐰 Hopping through files with nimble feet,

New shortcuts gathered, tidy and neat,
Sheets and styles, images take flight,
Tests keep watch by day and night,
A rabbit cheers: the CLI's now complete! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 2.52% 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 clearly and concisely summarizes the main change: adding sheet management shortcuts for the Sheets service. It directly reflects the primary objective of the PR.
Description check ✅ Passed The PR description includes all required sections from the template (Summary, Changes, Test Plan, Related Issues). It provides comprehensive details about the sheet management shortcuts added, documentation consolidation, and verification steps completed.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/sheet

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

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

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 29, 2026

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add larksuite/cli#feat/sheet -y -g

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
tests/cli_e2e/sheets/sheets_sheet_shortcuts_workflow_test.go (1)

28-30: Clarify the purpose of parentT parameter.

The parentT variable is captured and passed to createSpreadsheet but it's unclear from this file whether the helper uses it to register cleanup. Consider adding a brief comment explaining this pattern, or if cleanup is not handled internally, add an explicit cleanup step.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/cli_e2e/sheets/sheets_sheet_shortcuts_workflow_test.go` around lines 28
- 30, The test captures and passes parentT into createSpreadsheet (in the t.Run
block that sets spreadsheetToken) but its purpose is unclear; either document
that parentT is used by createSpreadsheet to register deferred cleanup or, if
createSpreadsheet does not handle cleanup, add an explicit cleanup/defer in this
test to remove the created sheet. Update the test near the t.Run invocation to
include a brief comment stating whether parentT is provided so createSpreadsheet
registers cleanup, or add a defer/cleanup call referencing spreadsheetToken
after createSpreadsheet returns to ensure resources are cleaned up.
shortcuts/sheets/sheet_manage.go (1)

66-77: Redundant backslash check.

Line 73 has a redundant check: strings.ContainsAny(title, /?*[]:) already includes the backslash character \, so the subsequent strings.Contains(title, `)` is unnecessary.

♻️ Proposed simplification
 func validateSheetTitle(flagName, title string) error {
 	if err := validate.RejectControlChars(title, flagName); err != nil {
 		return common.FlagErrorf("%v", err)
 	}
 	if len([]rune(title)) > 100 {
 		return common.FlagErrorf("--%s must be <= 100 characters", flagName)
 	}
-	if strings.ContainsAny(title, `/\?*[]:`) || strings.Contains(title, `\`) {
+	if strings.ContainsAny(title, `/\?*[]:`) {
 		return common.FlagErrorf("--%s must not contain any of / \\ ? * [ ] :", flagName)
 	}
 	return nil
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/sheets/sheet_manage.go` around lines 66 - 77, The validation in
validateSheetTitle contains a redundant backslash check: remove the second
condition using strings.Contains(title, `\`) and rely solely on
strings.ContainsAny(title, `/\?*[]:`) to detect backslashes and the other
forbidden characters; ensure the error returned by common.FlagErrorf in that
branch still references flagName and the same human-readable forbidden-character
list, and keep the function signature and surrounding logic unchanged.
🤖 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/sheets/sheet_manage.go`:
- Around line 309-320: The Validate function currently returns early when
validating "title" which skips validating "index" if both flags are provided;
change the logic so both flags are validated independently: call
validateSheetManageToken as before, then for "title" do if
runtime.Changed("title") { if err := validateSheetTitle("title",
runtime.Str("title")); err != nil { return err } } and for "index" do if
runtime.Changed("index") { if err := validateNonNegativeInt("index",
runtime.Int("index")); err != nil { return err } }, ensuring neither check uses
an early return that prevents the other from running.
- Around line 353-369: The Validate function currently returns directly from the
index check which causes index validation to be skipped when other checks (like
title) short-circuit; change the index branch so it performs validation but does
not unconditionally return — call validateNonNegativeInt("index",
runtime.Int("index")) and if it returns an error then return that error,
otherwise continue; adjust the runtime.Changed("index") block in the same
Validate function (alongside validateSheetManageToken, validateSheetID,
validateSheetTitle) to use the conditional error-handling pattern instead of a
bare return so both title and index validations run independently.

---

Nitpick comments:
In `@shortcuts/sheets/sheet_manage.go`:
- Around line 66-77: The validation in validateSheetTitle contains a redundant
backslash check: remove the second condition using strings.Contains(title, `\`)
and rely solely on strings.ContainsAny(title, `/\?*[]:`) to detect backslashes
and the other forbidden characters; ensure the error returned by
common.FlagErrorf in that branch still references flagName and the same
human-readable forbidden-character list, and keep the function signature and
surrounding logic unchanged.

In `@tests/cli_e2e/sheets/sheets_sheet_shortcuts_workflow_test.go`:
- Around line 28-30: The test captures and passes parentT into createSpreadsheet
(in the t.Run block that sets spreadsheetToken) but its purpose is unclear;
either document that parentT is used by createSpreadsheet to register deferred
cleanup or, if createSpreadsheet does not handle cleanup, add an explicit
cleanup/defer in this test to remove the created sheet. Update the test near the
t.Run invocation to include a brief comment stating whether parentT is provided
so createSpreadsheet registers cleanup, or add a defer/cleanup call referencing
spreadsheetToken after createSpreadsheet returns to ensure resources are cleaned
up.
🪄 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: d799db54-a98b-4808-a920-021d8aa63075

📥 Commits

Reviewing files that changed from the base of the PR and between b37adfd and 1a0ab4e.

📒 Files selected for processing (12)
  • shortcuts/sheets/sheet_manage.go
  • shortcuts/sheets/sheet_manage_test.go
  • shortcuts/sheets/shortcuts.go
  • skill-template/domains/sheets.md
  • skills/lark-sheets/SKILL.md
  • skills/lark-sheets/references/lark-sheets-copy-sheet.md
  • skills/lark-sheets/references/lark-sheets-create-sheet.md
  • skills/lark-sheets/references/lark-sheets-delete-sheet.md
  • skills/lark-sheets/references/lark-sheets-update-sheet.md
  • tests/cli_e2e/sheets/coverage.md
  • tests/cli_e2e/sheets/sheets_sheet_shortcuts_dryrun_test.go
  • tests/cli_e2e/sheets/sheets_sheet_shortcuts_workflow_test.go

Comment thread shortcuts/sheets/lark_sheets_sheet_management.go
Comment thread shortcuts/sheets/lark_sheets_sheet_management.go
Comment thread shortcuts/sheets/sheet_manage.go Outdated
Comment thread shortcuts/sheets/sheet_manage.go Outdated
Comment thread shortcuts/sheets/lark_sheets_sheet_management.go
Comment thread shortcuts/sheets/lark_sheets_sheet_management.go
- add +create-sheet, +copy-sheet, +delete-sheet, and +update-sheet
- cover request-shape dry-run and sheet workflow tests
- document new sheet management shortcuts in lark-sheets skill
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@skills/lark-sheets/references/lark-sheets-copy-sheet.md`:
- Around line 10-13: Remove the empty line breaking the blockquote so the
`[!CAUTION]` admonition stays inside the same blockquote (fixes
MD028/no-blanks-blockquote); in the lark-sheets-copy-sheet.md block that
contains "底层封装的是官方“操作工作表(operate-sheets)”接口..." and the `[!CAUTION]` lines,
delete the blank line between them and ensure both lines start with a single `>`
and a space so the admonition is treated as part of the same blockquote.

In `@skills/lark-sheets/references/lark-sheets-create-sheet.md`:
- Around line 10-13: Remove the blank line inside the blockquote: merge the
lines so the quoted sentence "底层封装的是官方“操作工作表(operate-sheets)”接口。" is immediately
followed by the admonition marker "[!CAUTION]" and its content "这是**写入操作**。可以先用
`--dry-run` 预览。" with no intervening empty line to satisfy the
no-blanks-blockquote MD028 rule.

In `@skills/lark-sheets/references/lark-sheets-delete-sheet.md`:
- Around line 10-13: The blockquote contains an extra blank line before the
admonition causing MD028; remove the blank line so the admonition marker
"[!CAUTION]" sits directly after the preceding blockquote text (the line
starting with "底层封装的是官方“操作工作表(operate-sheets)”接口。"), ensuring the entire
blockquote is contiguous and the markdownlint MD028 warning is resolved.

In `@tests/cli_e2e/sheets/sheets_sheet_shortcuts_dryrun_test.go`:
- Around line 87-89: The test currently asserts validation errors by only
checking result.Stderr; update both assertions that reference result.Stderr to
instead assert the process exited non-zero and that
strings.Contains(result.Stdout+result.Stderr, "mutually exclusive") (or the
expected validation text) is true so Validate-stage dry-run failures surfaced in
stdout are caught; locate the two occurrences using the test variable name
result and replace the stderr-only check with the combined-output check and a
non-zero exit assertion.
🪄 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: 86d17d4f-ce12-488a-a0fb-07da1212da3d

📥 Commits

Reviewing files that changed from the base of the PR and between 1a0ab4e and 7c77963.

📒 Files selected for processing (12)
  • shortcuts/sheets/sheet_manage.go
  • shortcuts/sheets/sheet_manage_test.go
  • shortcuts/sheets/shortcuts.go
  • skill-template/domains/sheets.md
  • skills/lark-sheets/SKILL.md
  • skills/lark-sheets/references/lark-sheets-copy-sheet.md
  • skills/lark-sheets/references/lark-sheets-create-sheet.md
  • skills/lark-sheets/references/lark-sheets-delete-sheet.md
  • skills/lark-sheets/references/lark-sheets-update-sheet.md
  • tests/cli_e2e/sheets/coverage.md
  • tests/cli_e2e/sheets/sheets_sheet_shortcuts_dryrun_test.go
  • tests/cli_e2e/sheets/sheets_sheet_shortcuts_workflow_test.go
✅ Files skipped from review due to trivial changes (4)
  • shortcuts/sheets/shortcuts.go
  • skill-template/domains/sheets.md
  • skills/lark-sheets/references/lark-sheets-update-sheet.md
  • skills/lark-sheets/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • shortcuts/sheets/sheet_manage_test.go
  • tests/cli_e2e/sheets/sheets_sheet_shortcuts_workflow_test.go
  • shortcuts/sheets/sheet_manage.go

Comment thread skills/lark-sheets/references/lark-sheets-copy-sheet.md Outdated
Comment thread skills/lark-sheets/references/lark-sheets-create-sheet.md Outdated
Comment thread skills/lark-sheets/references/lark-sheets-delete-sheet.md Outdated
Comment thread tests/cli_e2e/sheets/sheets_sheet_shortcuts_dryrun_test.go Outdated
@github-actions github-actions Bot added size/XL Architecture-level or global-impact change and removed size/L Large or sensitive change across domains or core paths labels Apr 30, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 13

♻️ Duplicate comments (1)
shortcuts/sheets/lark_sheets_sheet_management.go (1)

422-432: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Require --title and keep --index validation reachable in +create-sheet.

This validator still allows +create-sheet with no --title, so buildCreateSheetBody can send an empty addSheet.properties object and defer a required-field failure to the backend. Also, the early return on Line 427 skips the --index check when both flags are passed.

Suggested fix
 	Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
 		if _, err := validateSheetManageToken(runtime); err != nil {
 			return err
 		}
-		if runtime.Changed("title") {
-			return validateSheetTitle("title", runtime.Str("title"))
+		if err := validateSheetTitle("title", runtime.Str("title")); err != nil {
+			return err
 		}
 		if runtime.Changed("index") {
-			return validateNonNegativeInt("index", runtime.Int("index"))
+			if err := validateNonNegativeInt("index", runtime.Int("index")); err != nil {
+				return err
+			}
 		}
 		return nil
 	},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/sheets/lark_sheets_sheet_management.go` around lines 422 - 432, The
validator currently returns early and allows +create-sheet without --title;
update Validate (which calls validateSheetManageToken, validateSheetTitle,
validateNonNegativeInt) to (1) always run the index check (remove the
early-return pattern) and (2) enforce that for the "+create-sheet" command the
"title" flag must be present by returning an error if runtime.CmdName() ==
"+create-sheet" and !runtime.Changed("title"); afterwards, if
runtime.Changed("title") call validateSheetTitle("title", runtime.Str("title")),
and if runtime.Changed("index") call validateNonNegativeInt("index",
runtime.Int("index")), so both validations can run and buildCreateSheetBody will
never be allowed to send an empty addSheet.properties.
🤖 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/sheets/lark_sheets_cell_data.go`:
- Around line 114-121: The code currently only json.Unmarshal's
runtime.Str("values") into an interface{} (variable values) which accepts non-2D
inputs; change the validation to unmarshal (or assert) that values is a slice of
slices ([][]interface{}) after json.Unmarshal or attempt to directly unmarshal
into [][]interface{} and return common.FlagErrorf("--values must be a 2D JSON
array") if that assertion/unmarshal fails; update the same validation in the
other occurrence (around lines 200-207) so both places use the
runtime.Str("values") -> json.Unmarshal/typed unmarshal into [][]interface{} (or
type-assert interface{} to []interface{} and then ensure each element is
[]interface{}) before calling validateSheetRangeInput.
- Around line 29-40: Replace the hand-rolled token extraction/empty-check in
each Validate (and in DryRun/Execute blocks) with the shared sheet-token
validator to enforce consistent URL parsing and reject invalid combined flags;
specifically, remove the manual token := ... / extractSpreadsheetToken(...) /
empty check and instead call the existing validator (validateSheetManageToken or
the package's shared sheet-token validation helper) and return its error
directly from the Validate, DryRun, and Execute functions (locations: the
Validate func shown and the other Validate/DryRun/Execute blocks referenced) so
the stronger validation logic is reused.

In `@shortcuts/sheets/lark_sheets_cell_style_and_merge.go`:
- Around line 97-188: Update Validate to fully parse runtime.Str("data") into a
[]interface{} and reject any item that is not an object with a "ranges" array of
strings where each range string contains a sheetId prefix (contains a '!'
character); return common.FlagErrorf describing the bad item instead of allowing
it. Use the same normalization helper normalizePointRange when
checking/normalizing ranges (call it during Validate or call a new
validateAndNormalizeBatchStyleRanges that wraps normalizeBatchStyleRanges), and
keep normalizeBatchStyleRanges behavior in DryRun/Execute but assume Validate
has guaranteed all entries are well-formed so malformed items will no longer
reach runtime.CallAPI or DryRun API payloads. Ensure you reference the symbols
Validate, normalizeBatchStyleRanges, normalizePointRange, DryRun and Execute
when making these changes.

In `@shortcuts/sheets/lark_sheets_filter_views.go`:
- Around line 50-74: The Validate/DryRun flow currently allows an empty string
for the required "range" flag to proceed; add a check in the Validate function
(alongside the existing validateFilterViewToken call) to call
runtime.Str("range") and return a descriptive error if it is empty or only
whitespace so the command fails fast; keep DryRun unchanged except it can assume
validateFilterViewToken and the new non-blank range check have already passed
(i.e., reference Validate, validateFilterViewToken, and runtime.Str("range") to
locate where to add the check).
- Around line 112-116: The validation currently only uses
runtime.Cmd.Flags().Changed(...) which allows empty strings; update the checks
to read the actual flag values (e.g., call
runtime.Cmd.Flags().GetString("range"), GetString("filter-view-name") and any
filter-condition flags), trim them and ensure at least one is non-empty before
returning success; if all trimmed values are empty return
common.FlagErrorf("specify at least one of --range or --filter-view-name with a
non-empty value") or a similar message. Apply the same non-empty-value
validation for the other validation block noted (lines 313-321) and add a final
safeguard that the request body being built contains at least one field before
sending the request.

In `@shortcuts/sheets/lark_sheets_float_images.go`:
- Around line 31-79: DryRun currently chooses multipart vs upload_all based on
sheetMediaShouldUseMultipart(runtime.FileIO(), filePath) which treats any Stat
error as false, so an invalid or directory --file will produce an incorrect
single-part plan; update Validate to check runtime.Str("file") exists and is not
a directory (use resolveSheetMediaUploadParent(...) already called plus
runtime.FileIO().Stat(filePath)) and return an error if invalid, and in DryRun
do an explicit Stat on filePath via runtime.FileIO().Stat to detect missing/dir
errors and return common.NewDryRunAPI().Set("error", err.Error()) when Stat
fails before calling sheetMediaShouldUseMultipart, keeping the existing flow and
references to resolveSheetMediaUploadParent, sheetMediaShouldUseMultipart, and
sheetImageParentType.

In `@shortcuts/sheets/lark_sheets_row_column_management.go`:
- Around line 28-80: The token resolution currently overwrites an explicit
--spreadsheet-token whenever --url is present in Validate, DryRun and Execute;
update the logic so you first read token := runtime.Str("spreadsheet-token") and
only call extractSpreadsheetToken(runtime.Str("url")) if token == "" (or
alternatively return an error if both flags are provided), applying this change
in the Validate, DryRun and Execute functions and referencing
extractSpreadsheetToken and the
runtime.Str("spreadsheet-token")/runtime.Str("url") calls so a provided token is
not overridden by a malformed URL.

In `@shortcuts/sheets/lark_sheets_sheet_management.go`:
- Around line 513-521: When runtime.Changed("index") triggers a copy then move,
ensure partial success is returned if the move fails: capture copiedSheetID
(from out["sheet_id"]) and return an error or structured response that includes
that sheet_id so callers can recover; specifically update the move error path
after calling runtime.CallAPI and before returning err to either wrap the error
with the created copiedSheetID or return a partial-result object (e.g., include
sheet_id and a move_error field) instead of discarding out—adjust the move
failure handling around buildMoveCopiedSheetBody/call to include copiedSheetID
and consider using the same mergeSheetOutputs/buildUpdateSheetOutput conventions
for the partial-success payload.

In `@shortcuts/sheets/lark_sheets_spreadsheet_management.go`:
- Around line 283-315: When outputPath == "" the code currently calls
runtime.Out(...) but then continues to attempt downloading and saving; modify
the branch in the function containing outputPath, runtime.Out,
runtime.DoAPIStream and related save logic to return immediately after emitting
the file_token/ticket (i.e., add a return nil right after the runtime.Out call)
so the download and runtime.FileIO().Save path is skipped when --output-path is
omitted.
- Around line 109-116: The DryRun payload for the DryRun function currently only
includes title, causing mismatch when a folder_token is supplied at execute
time; update the DryRun API construction in DryRun (the DryRun: func(...)
*common.DryRunAPI block that creates common.NewDryRunAPI().POST(...).Body(...))
to include folder_token when runtime.Str("folder_token") is non-empty — e.g.,
build the body map to add "folder_token": runtime.Str("folder_token") only if
present so the dry-run preview matches the actual execute payload.
- Around line 33-56: The Validate/DryRun/Execute handlers (Validate, DryRun,
Execute) currently let a URL silently override the --spreadsheet-token by
calling extractSpreadsheetToken(runtime.Str("url")) in each handler; change this
by making the flags mutually exclusive and resolving the token once up‑front: in
Validate, return an error if both runtime.Str("url") and
runtime.Str("spreadsheet-token") are set, compute a single token value (prefer
the explicit token if provided, otherwise extract from URL) and store or pass
that resolved token into DryRun and Execute so they use the same resolved value
instead of re-extracting; update error messages to reference the conflict and
reuse the resolved token in common.NewDryRunAPI().Set("token", token) and
Execute logic.
- Around line 206-249: The Validate/DryRun/Execute flow currently accepts both
--url and --spreadsheet-token and doesn't enforce allowed file extensions or
CSV-specific requirements; update Validate (the Validate function that uses
runtime.Str and extractSpreadsheetToken) to error if both --url and
--spreadsheet-token are provided, require exactly one, validate that
--file-extension is one of "xlsx" or "csv" (use common.FlagErrorf for
user-facing errors), and if file-extension == "csv" require --sheet-id is
provided (error otherwise). Ensure DryRun and Execute use the validated
token/fileExt/sheetID semantics (the token resolution logic around token :=
runtime.Str("spreadsheet-token"); if runtime.Str("url") != "" { token =
extractSpreadsheetToken(...) } should be guarded by the same exclusivity check
done in Validate) so they cannot proceed with invalid combinations.

---

Duplicate comments:
In `@shortcuts/sheets/lark_sheets_sheet_management.go`:
- Around line 422-432: The validator currently returns early and allows
+create-sheet without --title; update Validate (which calls
validateSheetManageToken, validateSheetTitle, validateNonNegativeInt) to (1)
always run the index check (remove the early-return pattern) and (2) enforce
that for the "+create-sheet" command the "title" flag must be present by
returning an error if runtime.CmdName() == "+create-sheet" and
!runtime.Changed("title"); afterwards, if runtime.Changed("title") call
validateSheetTitle("title", runtime.Str("title")), and if
runtime.Changed("index") call validateNonNegativeInt("index",
runtime.Int("index")), so both validations can run and buildCreateSheetBody will
never be allowed to send an empty addSheet.properties.
🪄 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: de1b721f-a47a-4603-84ae-06a9850d5099

📥 Commits

Reviewing files that changed from the base of the PR and between 7365770 and 91036f1.

📒 Files selected for processing (89)
  • shortcuts/sheets/lark_sheets_cell_data.go
  • shortcuts/sheets/lark_sheets_cell_images.go
  • shortcuts/sheets/lark_sheets_cell_style_and_merge.go
  • shortcuts/sheets/lark_sheets_dropdown.go
  • shortcuts/sheets/lark_sheets_filter_views.go
  • shortcuts/sheets/lark_sheets_float_images.go
  • shortcuts/sheets/lark_sheets_row_column_management.go
  • shortcuts/sheets/lark_sheets_sheet_cell_ops_test.go
  • shortcuts/sheets/lark_sheets_sheet_create_test.go
  • shortcuts/sheets/lark_sheets_sheet_dimension_test.go
  • shortcuts/sheets/lark_sheets_sheet_dropdown_test.go
  • shortcuts/sheets/lark_sheets_sheet_filter_view_test.go
  • shortcuts/sheets/lark_sheets_sheet_float_image_test.go
  • shortcuts/sheets/lark_sheets_sheet_manage_test.go
  • shortcuts/sheets/lark_sheets_sheet_management.go
  • shortcuts/sheets/lark_sheets_sheet_media_upload_test.go
  • shortcuts/sheets/lark_sheets_sheet_ranges_test.go
  • shortcuts/sheets/lark_sheets_sheet_write_image_test.go
  • shortcuts/sheets/lark_sheets_spreadsheet_management.go
  • shortcuts/sheets/sheet_add_dimension.go
  • shortcuts/sheets/sheet_append.go
  • shortcuts/sheets/sheet_batch_set_style.go
  • shortcuts/sheets/sheet_create.go
  • shortcuts/sheets/sheet_delete_dimension.go
  • shortcuts/sheets/sheet_export.go
  • shortcuts/sheets/sheet_filter_view.go
  • shortcuts/sheets/sheet_find.go
  • shortcuts/sheets/sheet_info.go
  • shortcuts/sheets/sheet_insert_dimension.go
  • shortcuts/sheets/sheet_media_upload.go
  • shortcuts/sheets/sheet_merge_cells.go
  • shortcuts/sheets/sheet_move_dimension.go
  • shortcuts/sheets/sheet_read.go
  • shortcuts/sheets/sheet_replace.go
  • shortcuts/sheets/sheet_set_style.go
  • shortcuts/sheets/sheet_unmerge_cells.go
  • shortcuts/sheets/sheet_update_dimension.go
  • shortcuts/sheets/sheet_write.go
  • shortcuts/sheets/shortcuts.go
  • skill-template/domains/sheets.md
  • skills/lark-sheets/SKILL.md
  • skills/lark-sheets/references/lark-sheets-add-dimension.md
  • skills/lark-sheets/references/lark-sheets-append.md
  • skills/lark-sheets/references/lark-sheets-batch-set-style.md
  • skills/lark-sheets/references/lark-sheets-cell-data.md
  • skills/lark-sheets/references/lark-sheets-cell-images.md
  • skills/lark-sheets/references/lark-sheets-cell-style-and-merge.md
  • skills/lark-sheets/references/lark-sheets-create-filter-view-condition.md
  • skills/lark-sheets/references/lark-sheets-create-filter-view.md
  • skills/lark-sheets/references/lark-sheets-create-float-image.md
  • skills/lark-sheets/references/lark-sheets-create.md
  • skills/lark-sheets/references/lark-sheets-delete-dimension.md
  • skills/lark-sheets/references/lark-sheets-delete-dropdown.md
  • skills/lark-sheets/references/lark-sheets-delete-filter-view-condition.md
  • skills/lark-sheets/references/lark-sheets-delete-filter-view.md
  • skills/lark-sheets/references/lark-sheets-delete-float-image.md
  • skills/lark-sheets/references/lark-sheets-dropdown.md
  • skills/lark-sheets/references/lark-sheets-export.md
  • skills/lark-sheets/references/lark-sheets-filter-views.md
  • skills/lark-sheets/references/lark-sheets-find.md
  • skills/lark-sheets/references/lark-sheets-float-images.md
  • skills/lark-sheets/references/lark-sheets-formula.md
  • skills/lark-sheets/references/lark-sheets-get-dropdown.md
  • skills/lark-sheets/references/lark-sheets-get-filter-view-condition.md
  • skills/lark-sheets/references/lark-sheets-get-filter-view.md
  • skills/lark-sheets/references/lark-sheets-get-float-image.md
  • skills/lark-sheets/references/lark-sheets-info.md
  • skills/lark-sheets/references/lark-sheets-insert-dimension.md
  • skills/lark-sheets/references/lark-sheets-list-filter-view-conditions.md
  • skills/lark-sheets/references/lark-sheets-list-filter-views.md
  • skills/lark-sheets/references/lark-sheets-list-float-images.md
  • skills/lark-sheets/references/lark-sheets-media-upload.md
  • skills/lark-sheets/references/lark-sheets-merge-cells.md
  • skills/lark-sheets/references/lark-sheets-move-dimension.md
  • skills/lark-sheets/references/lark-sheets-read.md
  • skills/lark-sheets/references/lark-sheets-replace.md
  • skills/lark-sheets/references/lark-sheets-row-column-management.md
  • skills/lark-sheets/references/lark-sheets-set-dropdown.md
  • skills/lark-sheets/references/lark-sheets-set-style.md
  • skills/lark-sheets/references/lark-sheets-sheet-management.md
  • skills/lark-sheets/references/lark-sheets-spreadsheet-management.md
  • skills/lark-sheets/references/lark-sheets-unmerge-cells.md
  • skills/lark-sheets/references/lark-sheets-update-dimension.md
  • skills/lark-sheets/references/lark-sheets-update-dropdown.md
  • skills/lark-sheets/references/lark-sheets-update-filter-view-condition.md
  • skills/lark-sheets/references/lark-sheets-update-filter-view.md
  • skills/lark-sheets/references/lark-sheets-update-float-image.md
  • skills/lark-sheets/references/lark-sheets-write-image.md
  • skills/lark-sheets/references/lark-sheets-write.md
💤 Files with no reviewable changes (57)
  • shortcuts/sheets/sheet_append.go
  • shortcuts/sheets/sheet_find.go
  • shortcuts/sheets/sheet_insert_dimension.go
  • shortcuts/sheets/sheet_media_upload.go
  • shortcuts/sheets/sheet_move_dimension.go
  • shortcuts/sheets/sheet_read.go
  • shortcuts/sheets/sheet_replace.go
  • shortcuts/sheets/sheet_update_dimension.go
  • skills/lark-sheets/references/lark-sheets-add-dimension.md
  • skills/lark-sheets/references/lark-sheets-append.md
  • skills/lark-sheets/references/lark-sheets-batch-set-style.md
  • skills/lark-sheets/references/lark-sheets-delete-filter-view-condition.md
  • skills/lark-sheets/references/lark-sheets-delete-filter-view.md
  • skills/lark-sheets/references/lark-sheets-export.md
  • skills/lark-sheets/references/lark-sheets-create.md
  • skills/lark-sheets/references/lark-sheets-create-filter-view.md
  • skills/lark-sheets/references/lark-sheets-get-dropdown.md
  • skills/lark-sheets/references/lark-sheets-get-filter-view.md
  • skills/lark-sheets/references/lark-sheets-list-filter-view-conditions.md
  • skills/lark-sheets/references/lark-sheets-create-filter-view-condition.md
  • skills/lark-sheets/references/lark-sheets-info.md
  • skills/lark-sheets/references/lark-sheets-merge-cells.md
  • skills/lark-sheets/references/lark-sheets-list-float-images.md
  • skills/lark-sheets/references/lark-sheets-get-filter-view-condition.md
  • skills/lark-sheets/references/lark-sheets-read.md
  • skills/lark-sheets/references/lark-sheets-replace.md
  • skills/lark-sheets/references/lark-sheets-delete-dropdown.md
  • skills/lark-sheets/references/lark-sheets-set-style.md
  • skills/lark-sheets/references/lark-sheets-update-filter-view-condition.md
  • skills/lark-sheets/references/lark-sheets-update-filter-view.md
  • skills/lark-sheets/references/lark-sheets-update-float-image.md
  • skills/lark-sheets/references/lark-sheets-write.md
  • skills/lark-sheets/references/lark-sheets-update-dimension.md
  • shortcuts/sheets/sheet_set_style.go
  • skills/lark-sheets/references/lark-sheets-list-filter-views.md
  • shortcuts/sheets/sheet_add_dimension.go
  • shortcuts/sheets/sheet_delete_dimension.go
  • shortcuts/sheets/sheet_export.go
  • shortcuts/sheets/sheet_filter_view.go
  • shortcuts/sheets/sheet_unmerge_cells.go
  • shortcuts/sheets/sheet_write.go
  • skills/lark-sheets/references/lark-sheets-delete-float-image.md
  • skills/lark-sheets/references/lark-sheets-delete-dimension.md
  • skills/lark-sheets/references/lark-sheets-create-float-image.md
  • skills/lark-sheets/references/lark-sheets-find.md
  • skills/lark-sheets/references/lark-sheets-get-float-image.md
  • skills/lark-sheets/references/lark-sheets-media-upload.md
  • skills/lark-sheets/references/lark-sheets-move-dimension.md
  • skills/lark-sheets/references/lark-sheets-set-dropdown.md
  • shortcuts/sheets/sheet_create.go
  • skills/lark-sheets/references/lark-sheets-update-dropdown.md
  • skills/lark-sheets/references/lark-sheets-write-image.md
  • shortcuts/sheets/sheet_info.go
  • skills/lark-sheets/references/lark-sheets-insert-dimension.md
  • skills/lark-sheets/references/lark-sheets-unmerge-cells.md
  • shortcuts/sheets/sheet_batch_set_style.go
  • shortcuts/sheets/sheet_merge_cells.go
✅ Files skipped from review due to trivial changes (12)
  • shortcuts/sheets/lark_sheets_cell_images.go
  • skills/lark-sheets/references/lark-sheets-cell-images.md
  • skills/lark-sheets/references/lark-sheets-cell-style-and-merge.md
  • skills/lark-sheets/references/lark-sheets-sheet-management.md
  • skills/lark-sheets/references/lark-sheets-cell-data.md
  • skills/lark-sheets/references/lark-sheets-float-images.md
  • skills/lark-sheets/references/lark-sheets-filter-views.md
  • skills/lark-sheets/references/lark-sheets-spreadsheet-management.md
  • skills/lark-sheets/references/lark-sheets-dropdown.md
  • skills/lark-sheets/references/lark-sheets-row-column-management.md
  • skill-template/domains/sheets.md
  • skills/lark-sheets/references/lark-sheets-formula.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • shortcuts/sheets/shortcuts.go
  • skills/lark-sheets/SKILL.md

Comment thread shortcuts/sheets/lark_sheets_cell_data.go
Comment thread shortcuts/sheets/lark_sheets_cell_data.go Outdated
Comment thread shortcuts/sheets/lark_sheets_cell_data.go
Comment thread shortcuts/sheets/lark_sheets_cell_style_and_merge.go
Comment thread shortcuts/sheets/lark_sheets_filter_views.go
Comment thread shortcuts/sheets/lark_sheets_sheet_management.go
Comment thread shortcuts/sheets/lark_sheets_spreadsheet_management.go
Comment thread shortcuts/sheets/lark_sheets_spreadsheet_management.go
Comment thread shortcuts/sheets/lark_sheets_spreadsheet_management.go
Comment thread shortcuts/sheets/lark_sheets_spreadsheet_management.go
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (6)
shortcuts/sheets/lark_sheets_float_images.go (1)

126-136: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reject dual token inputs and malformed sheet URLs in +media-upload.

resolveSheetMediaUploadParent lets --url silently override an explicit --spreadsheet-token, and because it only checks parsed != "", an unparseable URL can be treated as the spreadsheet identifier and sent downstream as parent_node. Please reuse validateSheetManageToken here so this shortcut follows the same mutual-exclusion and URL-shape rules as the other sheet-management commands.

Suggested direction
 func resolveSheetMediaUploadParent(runtime *common.RuntimeContext) (string, error) {
-	token := runtime.Str("spreadsheet-token")
-	if u := runtime.Str("url"); u != "" {
-		if parsed := extractSpreadsheetToken(u); parsed != "" {
-			token = parsed
-		}
-	}
-	if token == "" {
-		return "", common.FlagErrorf("specify --url or --spreadsheet-token")
-	}
-	return token, nil
+	return validateSheetManageToken(runtime)
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/sheets/lark_sheets_float_images.go` around lines 126 - 136,
resolveSheetMediaUploadParent currently lets a provided --url silently override
an explicit --spreadsheet-token and treats unparseable URLs as the token; change
it to reuse validateSheetManageToken to enforce the same mutual-exclusion and
URL-shape rules as other sheet commands: read both
runtime.Str("spreadsheet-token") and runtime.Str("url"), call
validateSheetManageToken with those values (so it errors on both supplied or
malformed URL), and only return the validated token/parent_node from
validateSheetManageToken instead of naively using extractSpreadsheetToken.
shortcuts/sheets/lark_sheets_row_column_management.go (1)

28-35: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use the shared sheet-token validator for these dimension shortcuts.

All five shortcuts still use the unsafe resolution pattern where --url overrides a valid --spreadsheet-token, and malformed non-sheet URLs are never rejected before being encoded into the API path. Please switch these flows to validateSheetManageToken (and reuse the returned token in DryRun/Execute as well) so the new commands match the rest of the Sheets surface.

Suggested direction
 	Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
-		token := runtime.Str("spreadsheet-token")
-		if runtime.Str("url") != "" {
-			token = extractSpreadsheetToken(runtime.Str("url"))
-		}
-		if token == "" {
-			return common.FlagErrorf("specify --url or --spreadsheet-token")
-		}
+		token, err := validateSheetManageToken(runtime)
+		if err != nil {
+			return err
+		}
+		_ = token
 		length := runtime.Int("length")
 		if length < 1 || length > 5000 {
 			return common.FlagErrorf("--length must be between 1 and 5000, got %d", length)
 		}
 		return nil
 	},

Apply the same helper in each shortcut’s DryRun and Execute path instead of re-parsing --url.

Also applies to: 99-106, 183-190, 281-288, 362-369

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/sheets/lark_sheets_row_column_management.go` around lines 28 - 35,
The Validate functions currently resolve the sheet token unsafely by letting
runtime.Str("url") override runtime.Str("spreadsheet-token") and directly
calling extractSpreadsheetToken; replace this with the shared
validateSheetManageToken helper and propagate its returned token into the DryRun
and Execute paths (rather than re-parsing runtime.Str("url") again) so malformed
URLs are rejected and a single validated token is reused; update the code paths
referencing extractSpreadsheetToken, runtime.Str("spreadsheet-token"),
runtime.Str("url") in the Validate, DryRun, and Execute functions (and the other
similar blocks at the mentioned locations) to call validateSheetManageToken and
use its returned token.
shortcuts/sheets/lark_sheets_sheet_management.go (1)

519-524: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve partial success when the follow-up move fails.

After copySheet succeeds, this branch returns the raw move error and drops the created sheet_id. That makes agent-side retries unsafe because callers cannot tell that the copy already happened and may create duplicates. Please include the copied sheet_id in the failure path or emit a structured partial-success result, and cover that branch with a test.

Based on learnings: Design CLI flags, help text, and error messages with AI agent consumption in mind; every error message will be parsed by AI agents.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/sheets/lark_sheets_sheet_management.go` around lines 519 - 524, The
follow-up move error path currently drops the created copiedSheetID, making
retries unsafe; change the error return in the runtime.Changed("index") branch
(where copiedSheetID, moveResp, sheetBatchUpdatePath and
buildMoveCopiedSheetBody are used) to return a partial-success result that
includes the copied sheet_id (e.g. wrap/augment the error with the copiedSheetID
or return a structured error type containing SheetID and the move error), and
add a unit test that simulates a successful copy but failing move to assert the
function returns the error plus the copied sheet_id so callers/agents can detect
the partial success.
shortcuts/sheets/lark_sheets_spreadsheet_management.go (3)

33-56: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make +info reject --url + --spreadsheet-token together.

This still silently prefers --url when both are passed. That ambiguity can fetch the wrong spreadsheet.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/sheets/lark_sheets_spreadsheet_management.go` around lines 33 - 56,
The Validate/DryRun/Execute handlers (Validate, DryRun, Execute) currently
prefer the URL over --spreadsheet-token silently by calling runtime.Str("url")
and extracting a token, which allows both flags to be supplied ambiguously;
update Validate to detect when both runtime.Str("url") and
runtime.Str("spreadsheet-token") are non-empty and return a FlagErrorf
indicating the flags are mutually exclusive (e.g., "specify either --url or
--spreadsheet-token, not both"), and keep DryRun and Execute unchanged or assume
Validate prevents dual usage; reference extractSpreadsheetToken only when url is
present.

287-292: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Return immediately when --output-path is omitted.

After emitting file_token/ticket, execution still falls through to download+save with an empty path, which can fail unexpectedly.

Suggested fix
 		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/lark_sheets_spreadsheet_management.go` around lines 287 -
292, The current branch that handles an empty outputPath emits file_token/ticket
via runtime.Out but then continues execution and attempts to download/save to an
empty path; modify the handler so after calling runtime.Out with file_token and
ticket you immediately return from the surrounding function (e.g., add "return
nil" or the appropriate errorless return) to prevent fall-through into the
download/save logic that uses outputPath; locate the check using variables
outputPath, runtime.Out, fileToken and ticket and add the early return there.

210-219: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate export mode inputs before hitting the API.

+export currently accepts unsupported --file-extension values and allows --file-extension=csv without --sheet-id, which causes avoidable late API failures.

Suggested fix
 Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
-	token := runtime.Str("spreadsheet-token")
-	if runtime.Str("url") != "" {
-		token = extractSpreadsheetToken(runtime.Str("url"))
-	}
+	url := strings.TrimSpace(runtime.Str("url"))
+	token := strings.TrimSpace(runtime.Str("spreadsheet-token"))
+	if url != "" && token != "" {
+		return common.FlagErrorf("specify only one of --url or --spreadsheet-token")
+	}
+	if url != "" {
+		token = extractSpreadsheetToken(url)
+	}
 	if token == "" {
 		return common.FlagErrorf("specify --url or --spreadsheet-token")
 	}
+	ext := runtime.Str("file-extension")
+	if ext != "xlsx" && ext != "csv" {
+		return common.FlagErrorf("--file-extension must be xlsx or csv")
+	}
+	if ext == "csv" && strings.TrimSpace(runtime.Str("sheet-id")) == "" {
+		return common.FlagErrorf("--sheet-id is required when --file-extension=csv")
+	}
 	return nil
 },

Also applies to: 236-253

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/sheets/lark_sheets_spreadsheet_management.go` around lines 210 -
219, Update the Validate function(s) to reject unsupported export file
extensions and require a sheet id for CSV exports: when export mode is requested
(check runtime.Bool("export") or the relevant export flag), read
runtime.Str("file-extension") and ensure it is one of the allowed values (e.g.,
"csv", "xlsx", etc. per product spec); if it is not, return common.FlagErrorf
with a clear message; additionally, if file-extension == "csv" then ensure
runtime.Str("sheet-id") is non-empty and return common.FlagErrorf("specify
--sheet-id when --file-extension=csv") if missing. Apply the same validation
logic in both Validate blocks (the one around the token extraction and the other
block referenced at 236-253) and use the existing
runtime.Str/"sheet-id"/"file-extension"/runtime.Bool("export") symbols and
common.FlagErrorf to surface errors.
🤖 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/sheets/lark_sheets_filter_views.go`:
- Around line 33-42: In validateFilterViewToken, detect when both
runtime.Str("url") and runtime.Str("spreadsheet-token") are non-empty and return
a FlagErrorf indicating the user must not pass both (e.g., "specify only one of
--url or --spreadsheet-token"); otherwise keep the existing behavior of using
extractSpreadsheetToken(runtime.Str("url")) when url is provided and falling
back to runtime.Str("spreadsheet-token") when not.

---

Duplicate comments:
In `@shortcuts/sheets/lark_sheets_float_images.go`:
- Around line 126-136: resolveSheetMediaUploadParent currently lets a provided
--url silently override an explicit --spreadsheet-token and treats unparseable
URLs as the token; change it to reuse validateSheetManageToken to enforce the
same mutual-exclusion and URL-shape rules as other sheet commands: read both
runtime.Str("spreadsheet-token") and runtime.Str("url"), call
validateSheetManageToken with those values (so it errors on both supplied or
malformed URL), and only return the validated token/parent_node from
validateSheetManageToken instead of naively using extractSpreadsheetToken.

In `@shortcuts/sheets/lark_sheets_row_column_management.go`:
- Around line 28-35: The Validate functions currently resolve the sheet token
unsafely by letting runtime.Str("url") override runtime.Str("spreadsheet-token")
and directly calling extractSpreadsheetToken; replace this with the shared
validateSheetManageToken helper and propagate its returned token into the DryRun
and Execute paths (rather than re-parsing runtime.Str("url") again) so malformed
URLs are rejected and a single validated token is reused; update the code paths
referencing extractSpreadsheetToken, runtime.Str("spreadsheet-token"),
runtime.Str("url") in the Validate, DryRun, and Execute functions (and the other
similar blocks at the mentioned locations) to call validateSheetManageToken and
use its returned token.

In `@shortcuts/sheets/lark_sheets_sheet_management.go`:
- Around line 519-524: The follow-up move error path currently drops the created
copiedSheetID, making retries unsafe; change the error return in the
runtime.Changed("index") branch (where copiedSheetID, moveResp,
sheetBatchUpdatePath and buildMoveCopiedSheetBody are used) to return a
partial-success result that includes the copied sheet_id (e.g. wrap/augment the
error with the copiedSheetID or return a structured error type containing
SheetID and the move error), and add a unit test that simulates a successful
copy but failing move to assert the function returns the error plus the copied
sheet_id so callers/agents can detect the partial success.

In `@shortcuts/sheets/lark_sheets_spreadsheet_management.go`:
- Around line 33-56: The Validate/DryRun/Execute handlers (Validate, DryRun,
Execute) currently prefer the URL over --spreadsheet-token silently by calling
runtime.Str("url") and extracting a token, which allows both flags to be
supplied ambiguously; update Validate to detect when both runtime.Str("url") and
runtime.Str("spreadsheet-token") are non-empty and return a FlagErrorf
indicating the flags are mutually exclusive (e.g., "specify either --url or
--spreadsheet-token, not both"), and keep DryRun and Execute unchanged or assume
Validate prevents dual usage; reference extractSpreadsheetToken only when url is
present.
- Around line 287-292: The current branch that handles an empty outputPath emits
file_token/ticket via runtime.Out but then continues execution and attempts to
download/save to an empty path; modify the handler so after calling runtime.Out
with file_token and ticket you immediately return from the surrounding function
(e.g., add "return nil" or the appropriate errorless return) to prevent
fall-through into the download/save logic that uses outputPath; locate the check
using variables outputPath, runtime.Out, fileToken and ticket and add the early
return there.
- Around line 210-219: Update the Validate function(s) to reject unsupported
export file extensions and require a sheet id for CSV exports: when export mode
is requested (check runtime.Bool("export") or the relevant export flag), read
runtime.Str("file-extension") and ensure it is one of the allowed values (e.g.,
"csv", "xlsx", etc. per product spec); if it is not, return common.FlagErrorf
with a clear message; additionally, if file-extension == "csv" then ensure
runtime.Str("sheet-id") is non-empty and return common.FlagErrorf("specify
--sheet-id when --file-extension=csv") if missing. Apply the same validation
logic in both Validate blocks (the one around the token extraction and the other
block referenced at 236-253) and use the existing
runtime.Str/"sheet-id"/"file-extension"/runtime.Bool("export") symbols and
common.FlagErrorf to surface errors.
🪄 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: 1693f147-47e9-41e2-aed6-c69d3a029cf2

📥 Commits

Reviewing files that changed from the base of the PR and between 91036f1 and d5beb0d.

📒 Files selected for processing (90)
  • shortcuts/sheets/lark_sheets_cell_data.go
  • shortcuts/sheets/lark_sheets_cell_images.go
  • shortcuts/sheets/lark_sheets_cell_style_and_merge.go
  • shortcuts/sheets/lark_sheets_dropdown.go
  • shortcuts/sheets/lark_sheets_filter_views.go
  • shortcuts/sheets/lark_sheets_float_images.go
  • shortcuts/sheets/lark_sheets_row_column_management.go
  • shortcuts/sheets/lark_sheets_sheet_cell_ops_test.go
  • shortcuts/sheets/lark_sheets_sheet_create_test.go
  • shortcuts/sheets/lark_sheets_sheet_dimension_test.go
  • shortcuts/sheets/lark_sheets_sheet_dropdown_test.go
  • shortcuts/sheets/lark_sheets_sheet_filter_view_test.go
  • shortcuts/sheets/lark_sheets_sheet_float_image_test.go
  • shortcuts/sheets/lark_sheets_sheet_manage_test.go
  • shortcuts/sheets/lark_sheets_sheet_management.go
  • shortcuts/sheets/lark_sheets_sheet_media_upload_test.go
  • shortcuts/sheets/lark_sheets_sheet_ranges_test.go
  • shortcuts/sheets/lark_sheets_sheet_write_image_test.go
  • shortcuts/sheets/lark_sheets_spreadsheet_management.go
  • shortcuts/sheets/sheet_add_dimension.go
  • shortcuts/sheets/sheet_append.go
  • shortcuts/sheets/sheet_batch_set_style.go
  • shortcuts/sheets/sheet_create.go
  • shortcuts/sheets/sheet_delete_dimension.go
  • shortcuts/sheets/sheet_export.go
  • shortcuts/sheets/sheet_filter_view.go
  • shortcuts/sheets/sheet_find.go
  • shortcuts/sheets/sheet_info.go
  • shortcuts/sheets/sheet_insert_dimension.go
  • shortcuts/sheets/sheet_media_upload.go
  • shortcuts/sheets/sheet_merge_cells.go
  • shortcuts/sheets/sheet_move_dimension.go
  • shortcuts/sheets/sheet_read.go
  • shortcuts/sheets/sheet_replace.go
  • shortcuts/sheets/sheet_set_style.go
  • shortcuts/sheets/sheet_unmerge_cells.go
  • shortcuts/sheets/sheet_update_dimension.go
  • shortcuts/sheets/sheet_write.go
  • shortcuts/sheets/shortcuts.go
  • skill-template/domains/sheets.md
  • skills/lark-sheets/SKILL.md
  • skills/lark-sheets/references/lark-sheets-add-dimension.md
  • skills/lark-sheets/references/lark-sheets-append.md
  • skills/lark-sheets/references/lark-sheets-batch-set-style.md
  • skills/lark-sheets/references/lark-sheets-cell-data.md
  • skills/lark-sheets/references/lark-sheets-cell-images.md
  • skills/lark-sheets/references/lark-sheets-cell-style-and-merge.md
  • skills/lark-sheets/references/lark-sheets-create-filter-view-condition.md
  • skills/lark-sheets/references/lark-sheets-create-filter-view.md
  • skills/lark-sheets/references/lark-sheets-create-float-image.md
  • skills/lark-sheets/references/lark-sheets-create.md
  • skills/lark-sheets/references/lark-sheets-delete-dimension.md
  • skills/lark-sheets/references/lark-sheets-delete-dropdown.md
  • skills/lark-sheets/references/lark-sheets-delete-filter-view-condition.md
  • skills/lark-sheets/references/lark-sheets-delete-filter-view.md
  • skills/lark-sheets/references/lark-sheets-delete-float-image.md
  • skills/lark-sheets/references/lark-sheets-dropdown.md
  • skills/lark-sheets/references/lark-sheets-export.md
  • skills/lark-sheets/references/lark-sheets-filter-views.md
  • skills/lark-sheets/references/lark-sheets-find.md
  • skills/lark-sheets/references/lark-sheets-float-images.md
  • skills/lark-sheets/references/lark-sheets-formula.md
  • skills/lark-sheets/references/lark-sheets-get-dropdown.md
  • skills/lark-sheets/references/lark-sheets-get-filter-view-condition.md
  • skills/lark-sheets/references/lark-sheets-get-filter-view.md
  • skills/lark-sheets/references/lark-sheets-get-float-image.md
  • skills/lark-sheets/references/lark-sheets-info.md
  • skills/lark-sheets/references/lark-sheets-insert-dimension.md
  • skills/lark-sheets/references/lark-sheets-list-filter-view-conditions.md
  • skills/lark-sheets/references/lark-sheets-list-filter-views.md
  • skills/lark-sheets/references/lark-sheets-list-float-images.md
  • skills/lark-sheets/references/lark-sheets-media-upload.md
  • skills/lark-sheets/references/lark-sheets-merge-cells.md
  • skills/lark-sheets/references/lark-sheets-move-dimension.md
  • skills/lark-sheets/references/lark-sheets-read.md
  • skills/lark-sheets/references/lark-sheets-replace.md
  • skills/lark-sheets/references/lark-sheets-row-column-management.md
  • skills/lark-sheets/references/lark-sheets-set-dropdown.md
  • skills/lark-sheets/references/lark-sheets-set-style.md
  • skills/lark-sheets/references/lark-sheets-sheet-management.md
  • skills/lark-sheets/references/lark-sheets-spreadsheet-management.md
  • skills/lark-sheets/references/lark-sheets-unmerge-cells.md
  • skills/lark-sheets/references/lark-sheets-update-dimension.md
  • skills/lark-sheets/references/lark-sheets-update-dropdown.md
  • skills/lark-sheets/references/lark-sheets-update-filter-view-condition.md
  • skills/lark-sheets/references/lark-sheets-update-filter-view.md
  • skills/lark-sheets/references/lark-sheets-update-float-image.md
  • skills/lark-sheets/references/lark-sheets-write-image.md
  • skills/lark-sheets/references/lark-sheets-write.md
  • tests/cli_e2e/sheets/sheets_sheet_shortcuts_dryrun_test.go
💤 Files with no reviewable changes (57)
  • skills/lark-sheets/references/lark-sheets-update-filter-view-condition.md
  • skills/lark-sheets/references/lark-sheets-delete-filter-view-condition.md
  • skills/lark-sheets/references/lark-sheets-get-filter-view-condition.md
  • skills/lark-sheets/references/lark-sheets-delete-filter-view.md
  • skills/lark-sheets/references/lark-sheets-create-filter-view-condition.md
  • skills/lark-sheets/references/lark-sheets-list-filter-views.md
  • skills/lark-sheets/references/lark-sheets-get-dropdown.md
  • skills/lark-sheets/references/lark-sheets-get-filter-view.md
  • skills/lark-sheets/references/lark-sheets-update-dimension.md
  • skills/lark-sheets/references/lark-sheets-list-filter-view-conditions.md
  • skills/lark-sheets/references/lark-sheets-replace.md
  • skills/lark-sheets/references/lark-sheets-export.md
  • skills/lark-sheets/references/lark-sheets-insert-dimension.md
  • skills/lark-sheets/references/lark-sheets-update-dropdown.md
  • skills/lark-sheets/references/lark-sheets-list-float-images.md
  • skills/lark-sheets/references/lark-sheets-merge-cells.md
  • shortcuts/sheets/sheet_update_dimension.go
  • shortcuts/sheets/sheet_set_style.go
  • shortcuts/sheets/sheet_read.go
  • skills/lark-sheets/references/lark-sheets-append.md
  • shortcuts/sheets/sheet_move_dimension.go
  • skills/lark-sheets/references/lark-sheets-update-filter-view.md
  • skills/lark-sheets/references/lark-sheets-set-dropdown.md
  • skills/lark-sheets/references/lark-sheets-batch-set-style.md
  • skills/lark-sheets/references/lark-sheets-get-float-image.md
  • shortcuts/sheets/sheet_write.go
  • shortcuts/sheets/sheet_delete_dimension.go
  • skills/lark-sheets/references/lark-sheets-move-dimension.md
  • skills/lark-sheets/references/lark-sheets-read.md
  • shortcuts/sheets/sheet_find.go
  • skills/lark-sheets/references/lark-sheets-set-style.md
  • skills/lark-sheets/references/lark-sheets-info.md
  • skills/lark-sheets/references/lark-sheets-delete-dropdown.md
  • skills/lark-sheets/references/lark-sheets-find.md
  • shortcuts/sheets/sheet_batch_set_style.go
  • shortcuts/sheets/sheet_append.go
  • shortcuts/sheets/sheet_merge_cells.go
  • shortcuts/sheets/sheet_unmerge_cells.go
  • skills/lark-sheets/references/lark-sheets-create.md
  • skills/lark-sheets/references/lark-sheets-update-float-image.md
  • skills/lark-sheets/references/lark-sheets-delete-float-image.md
  • shortcuts/sheets/sheet_filter_view.go
  • shortcuts/sheets/sheet_replace.go
  • skills/lark-sheets/references/lark-sheets-media-upload.md
  • skills/lark-sheets/references/lark-sheets-write.md
  • shortcuts/sheets/sheet_export.go
  • shortcuts/sheets/sheet_info.go
  • skills/lark-sheets/references/lark-sheets-add-dimension.md
  • shortcuts/sheets/sheet_create.go
  • shortcuts/sheets/sheet_media_upload.go
  • shortcuts/sheets/sheet_insert_dimension.go
  • skills/lark-sheets/references/lark-sheets-delete-dimension.md
  • skills/lark-sheets/references/lark-sheets-create-filter-view.md
  • shortcuts/sheets/sheet_add_dimension.go
  • skills/lark-sheets/references/lark-sheets-unmerge-cells.md
  • skills/lark-sheets/references/lark-sheets-create-float-image.md
  • skills/lark-sheets/references/lark-sheets-write-image.md
✅ Files skipped from review due to trivial changes (9)
  • skill-template/domains/sheets.md
  • shortcuts/sheets/lark_sheets_cell_images.go
  • skills/lark-sheets/references/lark-sheets-dropdown.md
  • skills/lark-sheets/references/lark-sheets-filter-views.md
  • skills/lark-sheets/references/lark-sheets-sheet-management.md
  • skills/lark-sheets/references/lark-sheets-cell-data.md
  • skills/lark-sheets/references/lark-sheets-row-column-management.md
  • skills/lark-sheets/references/lark-sheets-formula.md
  • skills/lark-sheets/references/lark-sheets-cell-style-and-merge.md
🚧 Files skipped from review as they are similar to previous changes (5)
  • skills/lark-sheets/references/lark-sheets-cell-images.md
  • skills/lark-sheets/references/lark-sheets-float-images.md
  • skills/lark-sheets/references/lark-sheets-spreadsheet-management.md
  • skills/lark-sheets/SKILL.md
  • shortcuts/sheets/shortcuts.go

Comment thread shortcuts/sheets/lark_sheets_filter_views.go
@larksuite larksuite deleted a comment from codecov Bot Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/ccm PR touches the ccm domain size/XL Architecture-level or global-impact change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants