Skip to content

fix(im): make reaction scope conditional - #2353

Open
logarithmone1128 wants to merge 1 commit into
larksuite:mainfrom
logarithmone1128:fix/im-no-reactions-scope
Open

fix(im): make reaction scope conditional#2353
logarithmone1128 wants to merge 1 commit into
larksuite:mainfrom
logarithmone1128:fix/im-no-reactions-scope

Conversation

@logarithmone1128

@logarithmone1128 logarithmone1128 commented Aug 14, 2026

Copy link
Copy Markdown

Summary

Make the reaction scope conditional for message-pulling shortcuts so --no-reactions can fetch messages without requiring im:message.reactions:read. The default enrichment path still preflights the reaction scope before sending a request.

Changes

  • Move im:message.reactions:read from unconditional scopes to ConditionalScopes for the four message-pulling shortcuts.
  • Enforce the conditional scope from validation only when reaction enrichment is enabled.
  • Add command-level regression tests for both the opt-out and default paths, plus scope declaration coverage and updated documentation.

Test Plan

  • Unit tests pass (make unit-test)
  • Local command-level dry-run regression covers the lark-cli im +chat-messages-list flow
  • make build
  • make vet
  • make fmt-check
  • node scripts/skill-format-check/index.js

Related Issues

Summary by CodeRabbit

  • Bug Fixes

    • Reaction-related permissions are now requested only when reaction enrichment is enabled.
    • Commands using --no-reactions skip both reaction loading and permission checks.
    • Improved validation for message lists, searches, threads, pagination, and message retrieval.
  • Documentation

    • Clarified when reaction permissions are required and when they can be omitted.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions github-actions Bot added domain/im PR touches the im domain size/M Single-domain feat or fix with limited business impact labels Aug 14, 2026
@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3171d3d9-8f92-4c42-a73b-e0f0b5b617e4

📥 Commits

Reviewing files that changed from the base of the PR and between 0c5530d and 4854166.

📒 Files selected for processing (7)
  • shortcuts/im/im_chat_messages_list.go
  • shortcuts/im/im_message_reaction_scope.go
  • shortcuts/im/im_message_reaction_scope_test.go
  • shortcuts/im/im_messages_mget.go
  • shortcuts/im/im_messages_search.go
  • shortcuts/im/im_threads_messages_list.go
  • skills/lark-im/references/lark-im-message-enrichment.md

📝 Walkthrough

Walkthrough

Message-pull shortcuts now declare im:message.reactions:read conditionally and preflight it only when reaction enrichment is enabled. The --no-reactions option bypasses the scope check. Tests cover metadata, disabled reactions, and missing-scope errors.

Changes

Message reaction scope handling

Layer / File(s) Summary
Conditional scope contract
shortcuts/im/im_message_reaction_scope.go, shortcuts/im/im_*messages*.go, shortcuts/im/im_threads_messages_list.go
The shortcuts move reaction access to ConditionalScopes. The shared helper skips preflight when reactions are disabled or required runtime configuration is unavailable.
Shortcut validation and coverage
shortcuts/im/im_chat_messages_list.go, shortcuts/im/im_messages_mget.go, shortcuts/im/im_messages_search.go, shortcuts/im/im_threads_messages_list.go, shortcuts/im/im_message_reaction_scope_test.go, skills/lark-im/references/lark-im-message-enrichment.md
Validation calls the conditional preflight after existing argument checks. Tests cover conditional declarations, --no-reactions, and missing-scope errors. Documentation reflects the updated behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 48541

This change makes reaction permissions conditional for message-pulling shortcuts while preserving the default enrichment behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: liangshuo-1

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address issue #2352 by skipping reaction scope preflight and enrichment when --no-reactions is enabled.
Out of Scope Changes check ✅ Passed The code, tests, and documentation changes directly support the conditional reaction-scope behavior described in issue #2352.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The description includes the required Summary, Changes, Test Plan, and Related Issues sections with clear implementation details and verification steps.
Title check ✅ Passed The title clearly and concisely describes the main change: making the IM reaction scope conditional.
✨ 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.

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

Development

Successfully merging this pull request may close these issues.

[Bug] --no-reactions still requires im:message.reactions:read in +chat-messages-list

2 participants