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

CLAassistant commented Aug 14, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@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