chore(im): clarify messages-send dry-run chat membership

[YangJunzhou-01]

base on @xu91102

Summary

Clarify that im +messages-send --dry-run validates request shape only and does not prove the selected bot/user can send to the target chat.

Changes

• Add a dry-run warning on chat-id sends explaining that chat membership is not verified.
• Preserve existing dry-run descriptions such as media placeholder notes.
• Add a dry-run shape regression test for the membership warning.

Test Plan

• Targeted regression: go test ./shortcuts/im -run 'TestShortcutDryRunShapes/ImMessagesSend_dry_run_warns_chat_membership_is_not_verified' -count=1
• Related dry-run shape tests: go test ./shortcuts/im -run 'TestShortcutDryRunShapes' -count=1
• Related IM send tests: go test ./shortcuts/im -run 'TestImMessagesSend|TestShortcutDryRunShapes' -count=1
• Format check: gofmt -l shortcuts/im/im_messages_send.go shortcuts/im/builders_test.go
• Diff check: git diff --check -- shortcuts/im/im_messages_send.go shortcuts/im/builders_test.go

Related Issues

• Closes Clarify that im messages-send --dry-run does not verify chat membership #915

Summary by CodeRabbit

Release Notes

• New Features
  • Added a membership-warning message in dry-run output when using --chat-id to clarify that dry-run validates request shape only and does not verify bot/user membership in the target chat.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR adds a membership verification warning to the im messages-send --dry-run command. When dry-run is invoked with --chat-id, a note is appended to the output clarifying that dry-run validates request shape only and does not verify the bot/user is a member of the target chat, where a real send may fail with an out-of-chat error. A test case validates this warning appears.

Changes

Chat membership warning for dry-run

Layer / File(s)	Summary
Membership verification warning in dry-run handler and test validation shortcuts/im/im_messages_send.go, shortcuts/im/builders_test.go	The DryRun method conditionally appends a warning note when --chat-id is provided, stating that dry-run validates request shape only and does not verify bot/user membership. A new subtest in TestShortcutDryRunShapes verifies the warning strings appear in the dry-run output.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

domain/im, size/M

Suggested reviewers

• haozhenghua-code
• liangshuo-1

Poem

🐰 A warning whispers clear,
"Dry-run won't check if you're near,"
No chat membership tested here,
So the real send could fail, my dear!
Now users see the truth so bright. 🌟

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding a membership warning to the messages-send dry-run output.
Description check	✅ Passed	The description is complete with all required sections: Summary, Changes, Test Plan, and Related Issues, matching the repository template.
Linked Issues check	✅ Passed	The PR fully implements the requirements from #915: adds a membership-warning message to dry-run output and includes a regression test verifying the warning appears.
Out of Scope Changes check	✅ Passed	All changes are scoped to the objective: the test addition verifies the warning, and the code modification adds the warning message. No unrelated changes detected.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

shortcuts/im/builders_test.go (1)

732-742: ⚡ Quick win

Consider adding a negative test case for --user-id sends.

The current test validates that the membership warning appears when --chat-id is provided. Consider adding a complementary test case that verifies the warning does NOT appear when --user-id is used instead, to ensure the conditional behavior is correct in both branches.

Example:

t.Run("ImMessagesSend dry run does not warn for user-id sends", func(t *testing.T) {
	runtime := newTestRuntimeContext(t, map[string]string{
		"user-id": "ou_123",
		"text":    "hello",
	}, nil)
	got := mustMarshalDryRun(t, ImMessagesSend.DryRun(context.Background(), runtime))
	if strings.Contains(got, "membership in the target chat is not verified") {
		t.Fatalf("ImMessagesSend.DryRun() unexpected membership warning for --user-id: %s", got)
	}
})

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@shortcuts/im/builders_test.go` around lines 732 - 742, Add a complementary
negative test that ensures ImMessagesSend.DryRun does NOT emit the
chat-membership warning when invoked with --user-id: create a new t.Run similar
to the existing one but pass runtime := newTestRuntimeContext(t,
map[string]string{"user-id":"ou_123","text":"hello"}, nil), call got :=
mustMarshalDryRun(t, ImMessagesSend.DryRun(context.Background(), runtime)), and
assert that the returned string does NOT contain the membership warning text
(fail the test if it does); place this alongside the existing test to cover the
alternate branch.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@shortcuts/im/builders_test.go`:
- Around line 732-742: Add a complementary negative test that ensures
ImMessagesSend.DryRun does NOT emit the chat-membership warning when invoked
with --user-id: create a new t.Run similar to the existing one but pass runtime
:= newTestRuntimeContext(t,
map[string]string{"user-id":"ou_123","text":"hello"}, nil), call got :=
mustMarshalDryRun(t, ImMessagesSend.DryRun(context.Background(), runtime)), and
assert that the returned string does NOT contain the membership warning text
(fail the test if it does); place this alongside the existing test to cover the
alternate branch.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8c1798c0-fa45-46fa-9a99-0a57ad11c7bf

📥 Commits

Reviewing files that changed from the base of the PR and between b91f6a2 and c4b5ea1.

📒 Files selected for processing (2)

• shortcuts/im/builders_test.go
• shortcuts/im/im_messages_send.go

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.63%. Comparing base (b91f6a2) to head (0359c3f).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1150   +/-   ##
=======================================
  Coverage   68.63%   68.63%           
=======================================
  Files         625      625           
  Lines       58386    58389    +3     
=======================================
+ Hits        40071    40076    +5     
+ Misses      15027    15026    -1     
+ Partials     3288     3287    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@0359c3f395eb0d1a46c6a16624b4f846d05cf67a

🧩 Skill update

npx skills add YangJunzhou-01/cli#fix-im-dry-run-chat-membership -y -g