docs(im): clarify media key formats for message media flags#991
Merged
Conversation
github-actions
Bot
added
domain/im
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughUpdated media input flag descriptions across CLI help text, validation tests, and user documentation to explicitly document that media flags accept API keys, URLs, or cwd-relative local paths while rejecting absolute paths and ChangesMedia Input Path Restrictions Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
shortcuts/im/validate_media_test.go (1)
68-71: ⚡ Quick winCover key-format contract in this test too.
Please also assert the descriptions mention
img_xxxand/orfile_xxxso the primary doc contract from this PR is regression-tested, not just URL/path constraints.Proposed test extension
- for _, want := range []string{"URL", "cwd-relative local path", "absolute paths", ".. are rejected"} { + for _, want := range []string{"URL", "cwd-relative local path", "absolute paths", ".. are rejected"} { if !strings.Contains(desc, want) { t.Fatalf("%s --%s description = %q, want it to mention %q", sc.name, flagName, desc, want) } } + // Key format contract + if flagName == "image" || flagName == "video-cover" { + if !strings.Contains(desc, "img_xxx") { + t.Fatalf("%s --%s description = %q, want it to mention %q", sc.name, flagName, desc, "img_xxx") + } + } + if flagName == "file" || flagName == "video" || flagName == "audio" { + if !strings.Contains(desc, "file_xxx") { + t.Fatalf("%s --%s description = %q, want it to mention %q", sc.name, flagName, desc, "file_xxx") + } + }🤖 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/validate_media_test.go` around lines 68 - 71, The test currently checks that descriptions mention URL/path constraints but omits the key-format contract; update the loop in validate_media_test.go (the block using sc.name, flagName, desc and the []string{"URL", "cwd-relative local path", "absolute paths", ".. are rejected"}) to also assert the description references the key-name contract by either containing "img_" or "file_": after the existing checks add a condition that verifies strings.Contains(desc, "img_") || strings.Contains(desc, "file_") and call t.Fatalf with a clear message (including sc.name, flagName, desc and the expected key prefix) when neither is present so the test regresses if the doc contract is removed.
🤖 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.
Inline comments:
In `@skills/lark-im/references/lark-im-messages-reply.md`:
- Line 30: The table row for "Reply with media" currently implies keys are
uploaded; update the wording to say it "accepts keys/URLs/cwd-relative local
files; uploads URLs/local files automatically" so it no longer claims keys are
uploaded. Edit the cell containing "Reply with media" / the description for the
`--image` / `--file` / `--video` / `--audio` flags in lark-im-messages-reply.md
to replace the phrase implying upload with the corrected phrasing ("accepts
keys/URLs/cwd-relative local files; uploads URLs/local files automatically") so
the behavior of keys versus URLs/local files is accurately described.
In `@skills/lark-im/references/lark-im-messages-send.md`:
- Line 30: Update the table row that currently claims "Shortcut uploads keys,
URLs, or cwd-relative local files automatically" (the row for "Send image / file
/ video / audio" with options "`--image` / `--file` / `--video` / `--audio`") to
clarify that only URLs and cwd-relative local file paths are automatically
uploaded, while existing keys/identifiers are sent as-is and are not uploaded;
reword the cell to explicitly state this distinction.
---
Nitpick comments:
In `@shortcuts/im/validate_media_test.go`:
- Around line 68-71: The test currently checks that descriptions mention
URL/path constraints but omits the key-format contract; update the loop in
validate_media_test.go (the block using sc.name, flagName, desc and the
[]string{"URL", "cwd-relative local path", "absolute paths", ".. are rejected"})
to also assert the description references the key-name contract by either
containing "img_" or "file_": after the existing checks add a condition that
verifies strings.Contains(desc, "img_") || strings.Contains(desc, "file_") and
call t.Fatalf with a clear message (including sc.name, flagName, desc and the
expected key prefix) when neither is present so the test regresses if the doc
contract is removed.
🪄 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: 3e309d82-45f4-4ca9-8f82-91c1ae592559
📒 Files selected for processing (5)
shortcuts/im/im_messages_reply.goshortcuts/im/im_messages_send.goshortcuts/im/validate_media_test.goskills/lark-im/references/lark-im-messages-reply.mdskills/lark-im/references/lark-im-messages-send.md
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@c79f45a8c5768df31e073a18db505bce634a2b88🧩 Skill updatenpx skills add YangJunzhou-01/cli#fix/pr-956-improved -y -g |
Change-Id: I329ca0db9e7a01b774846d522d1b2a64da74233c
YangJunzhou-01
force-pushed
the
fix/pr-956-improved
branch
from
May 20, 2026 08:00
21771a4 to
c79f45a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #991 +/- ##
=======================================
Coverage 67.58% 67.58%
=======================================
Files 575 575
Lines 54269 54269
=======================================
Hits 36679 36679
Misses 14548 14548
Partials 3042 3042 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
haozhenghua-code
approved these changes
May 20, 2026
liangshuo-1
approved these changes
May 20, 2026
2 tasks
4 tasks
tuxedomm
pushed a commit
to zhumiaoxin/cli
that referenced
this pull request
Jun 6, 2026
…e#991) * docs(im): clarify media path restrictions * docs(im): clarify file key formats for message file flags Change-Id: I329ca0db9e7a01b774846d522d1b2a64da74233c --------- Co-authored-by: mtsui-cmyk <mervyntsui@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Based on #956 by @mtsui-cmyk
im +messages-sendandim +messages-replymedia flag help text by explicitly documentingimg_xxxandfile_xxxkey formats.lark-imskill references so send/reply media parameter docs match the CLI help text.Related Issues
Fixes #872
Summary by CodeRabbit