Skip to content

fix: sheets scope typo, IM flag docs, dry-run membership warning, proxy warning env var#976

Open
mengqiuzhen wants to merge 3 commits into
larksuite:mainfrom
mengqiuzhen:main
Open

fix: sheets scope typo, IM flag docs, dry-run membership warning, proxy warning env var#976
mengqiuzhen wants to merge 3 commits into
larksuite:mainfrom
mengqiuzhen:main

Conversation

@mengqiuzhen
Copy link
Copy Markdown

@mengqiuzhen mengqiuzhen commented May 19, 2026

Summary

Four small fixes across 3 areas:

#838 — sheets scope name typo

Fixed "sheets:spreadsheet:read""sheets:spreadsheet:readonly" across all sheets shortcuts, scope_priorities.json, and tests. The wrong scope name is not recognized by Feishu Open Platform. Confirmed via lark-cli schema sheets.spreadsheets. (40 replacements, 11 files, string-only.)

#872 — media flag --help now documents relative-path restriction

--image/--file/--video/--audio flag descriptions changed from "local file path" to "relative path to file" in im +messages-send and im +messages-reply, so users learn the restriction from --help instead of hitting a validation error at runtime.

#915 — dry-run warns it does not verify chat membership

im +messages-send --dry-run now appends a caveat when --chat-id is used. Prevents agents from misinterpreting dry-run success as proof the bot/user can deliver to the chat.

#811 — new LARK_CLI_NO_PROXY_WARNING env var

Scripted/agent callers can now silence the per-invocation proxy warning with LARK_CLI_NO_PROXY_WARNING=1 while still using the proxy. (LARK_CLI_NO_PROXY=1 already disables the proxy entirely; this only suppresses the stderr warning.)

Verification

  • go build ./... — clean
  • go test ./internal/registry/... — all pass
  • go test ./internal/util/... — all pass
  • go test ./shortcuts/im/... — all pass (including DryRun shape tests)
  • go test ./shortcuts/sheets/... — pass (2 pre-existing image test failures unrelated)

Closes #838
Closes #872
Closes #915

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added new environment variable option to suppress proxy warning
  • Documentation & Improvements

    • Refined OAuth permissions for spreadsheet operations to use granular read-only scope
    • Improved help text for media input flags in messaging commands
    • Enhanced dry-run messaging to clarify that validation applies to request format only

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cd18e02c-b4c9-4ae7-9a39-9dd7e45c7a94

📥 Commits

Reviewing files that changed from the base of the PR and between 1f259bd and 90a2a7a.

📒 Files selected for processing (14)
  • internal/registry/registry_test.go
  • internal/registry/scope_priorities.json
  • internal/util/proxy.go
  • shortcuts/im/im_messages_reply.go
  • shortcuts/im/im_messages_send.go
  • 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_management.go
  • shortcuts/sheets/lark_sheets_spreadsheet_management.go

📝 Walkthrough

Walkthrough

Replaces all occurrences of sheets:spreadsheet:read with sheets:spreadsheet:readonly across registry, tests, and Sheets shortcuts; adds a proxy-warning env var and refines IM media flag help and DryRun messaging.

Changes

Sheets OAuth Scope Correction

Layer / File(s) Summary
Registry configuration and test updates
internal/registry/scope_priorities.json, internal/registry/registry_test.go
The registry entry and tests are updated to use sheets:spreadsheet:readonly instead of sheets:spreadsheet:read.
Cell data operation shortcuts
shortcuts/sheets/lark_sheets_cell_data.go
SheetRead, SheetWrite, SheetAppend, SheetFind, SheetReplace now request sheets:spreadsheet:readonly (write shortcuts keep sheets:spreadsheet:write_only).
Image operation shortcuts
shortcuts/sheets/lark_sheets_cell_images.go, shortcuts/sheets/lark_sheets_float_images.go
SheetWriteImage, SheetGetFloatImage, SheetListFloatImages now use sheets:spreadsheet:readonly.
Cell styling and merge shortcuts
shortcuts/sheets/lark_sheets_cell_style_and_merge.go
SheetSetStyle, SheetBatchSetStyle, SheetMergeCells, SheetUnmergeCells now use sheets:spreadsheet:readonly.
Dropdown operation shortcuts
shortcuts/sheets/lark_sheets_dropdown.go
SheetSetDropdown, SheetUpdateDropdown, SheetGetDropdown, SheetDeleteDropdown now use sheets:spreadsheet:readonly.
Filter view operation shortcuts
shortcuts/sheets/lark_sheets_filter_views.go
All filter view and filter view condition shortcuts updated to use sheets:spreadsheet:readonly.
Row and column dimension management shortcuts
shortcuts/sheets/lark_sheets_row_column_management.go
SheetAddDimension, SheetInsertDimension, SheetUpdateDimension, SheetMoveDimension, SheetDeleteDimension now use sheets:spreadsheet:readonly.
Sheet management shortcuts
shortcuts/sheets/lark_sheets_sheet_management.go
SheetCreateSheet, SheetCopySheet, SheetDeleteSheet, SheetUpdateSheet now use sheets:spreadsheet:readonly.
Spreadsheet information shortcut
shortcuts/sheets/lark_sheets_spreadsheet_management.go
SheetInfo now requests sheets:spreadsheet:readonly.

Miscellaneous

Layer / File(s) Summary
Proxy warning suppression
internal/util/proxy.go
Adds EnvNoProxyWarning constant (LARK_CLI_NO_PROXY_WARNING) and makes WarnIfProxied skip the one-time warning when set.
IM help text and DryRun note
shortcuts/im/im_messages_reply.go, shortcuts/im/im_messages_send.go
Reword media flag help to accept *_key or relative path; DryRun now appends an explanatory note when --chat-id is provided.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • larksuite/cli#935: Overlapping changes to lark_sheets_cell_style_and_merge.go (scopes / DryRun behavior) that touch the same shortcuts.
  • larksuite/cli#494: Introduced float-image shortcuts; related to float-image scope adjustments here.

Suggested reviewers

  • fangshuyu-768
  • YangJunzhou-01
  • liuxinyanglxy

🐰 I hopped through code with tiny feet,
found "read" where "readonly" should meet,
I nudged the scopes and fixed the test,
now Sheets accept tokens — peace and rest. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% 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 accurately summarizes the main changes: sheets scope name correction, IM flag documentation improvements, dry-run membership warning, and proxy warning env var.
Description check ✅ Passed The PR description follows the template structure with Summary, Changes (organized by issue number), Test Plan showing verification steps, and Related Issues section that closes #838, #872, and #915.
Linked Issues check ✅ Passed All four coding requirements from linked issues are met: #838 sheets scope name corrections (40 replacements, 11 files), #872 media flag help text updated to document relative-path restriction, #915 dry-run membership warning added for chat-id, and #811 LARK_CLI_NO_PROXY_WARNING env var implemented.
Out of Scope Changes check ✅ Passed All changes are directly aligned with the four linked issues: scope name updates, flag documentation, dry-run warning, and proxy env var. No unrelated modifications detected.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions github-actions Bot added domain/ccm PR touches the ccm domain size/M Single-domain feat or fix with limited business impact labels May 19, 2026
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 19, 2026

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions Bot added domain/im PR touches the im domain size/L Large or sensitive change across domains or core paths and removed size/M Single-domain feat or fix with limited business impact labels May 19, 2026
@mengqiuzhen mengqiuzhen changed the title fix(sheets): correct scope name from "read" to "readonly" matching Feishu Open Platform fix: sheets scope typo, IM flag docs, dry-run membership warning, proxy warning env var May 19, 2026
mengqiuzhen and others added 2 commits May 20, 2026 09:51
…ishu Open Platform

The scope "sheets:spreadsheet:read" is not recognized by the Feishu Open
Platform. The correct read scope for sheets is "sheets:spreadsheet:readonly",
as defined by the Feishu OpenAPI schema and already referenced in
scope_overrides.json. Using the wrong scope name causes token authorization
gaps — the granted token lacks the expected permission for sheets shortcuts.

Closes larksuite#838

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…y-run membership warning

larksuite#872: --image/--file/--video/--audio flag descriptions now say "relative
path to file" instead of "local file path", so users and AI agents know
absolute paths are rejected before hitting the validation error.

larksuite#915: im +messages-send --dry-run now appends a membership caveat when
--chat-id is used, clarifying that dry-run only validates request shape,
not chat membership.

larksuite#811: add LARK_CLI_NO_PROXY_WARNING env var to suppress the per-invocation
proxy warning while still using the proxy, for scripted/agent callers.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@fangshuyu-768
Copy link
Copy Markdown
Collaborator

The sheets scope change in this PR (sheets:spreadsheet:readsheets:spreadsheet:readonly) follows the same direction as #909, which was already closed. Per the discussion in #909 (specifically @caojie0621's comment):

  1. sheets:spreadsheet:readonly is deprecated. The current recommended scope name on the Feishu Open Platform is sheets:spreadsheet:read, not readonly.
  2. The CLI scope pre-check uses exact string matching, so the declared scope must match the current platform scope name. Changing read back to readonly would cause tokens with the correct scope to be rejected.
  3. For users whose tokens only have the legacy sheets:spreadsheet:readonly scope, the right approach is to guide them to re-authorize with sheets:spreadsheet:read, not to downgrade the CLI scope declaration to a deprecated alias.

I'd suggest reverting the sheets scope changes (the readreadonly replacements across 11 files) and keeping sheets:spreadsheet:read. The other three fixes (IM flag docs, dry-run membership warning, proxy warning env var) look good and can stay.

Ref: #909

…ching Feishu Open Platform"

This reverts commit 1122127.
@github-actions github-actions Bot added size/M Single-domain feat or fix with limited business impact and removed domain/ccm PR touches the ccm domain size/L Large or sensitive change across domains or core paths labels May 20, 2026
@YangJunzhou-01
Copy link
Copy Markdown
Collaborator

Thanks for the PR!
A couple of notes:
One issue per PR — We recommend keeping each PR focused on a single issue to make reviews easier and keep the change history clean.
#872 is already closed — The issue referenced here has been addressed in #956, which is already merged.
Thanks for your attention to the project — feel free to reach out if you have any questions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/im PR touches the im domain size/M Single-domain feat or fix with limited business impact

Projects

None yet

4 participants