feat(feed): add +sensitive shortcut for time-sensitive status#718
Closed
jinzemo wants to merge 10 commits intolarksuite:mainfrom
Closed
feat(feed): add +sensitive shortcut for time-sensitive status#718jinzemo wants to merge 10 commits intolarksuite:mainfrom
jinzemo wants to merge 10 commits intolarksuite:mainfrom
Conversation
added 10 commits
April 29, 2026 16:52
|
maojinze.7 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
📝 WalkthroughWalkthroughThis PR introduces a new CLI shortcut Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as CLI Command
participant Validator as Parameter Validator
participant RequestBuilder as Request Builder
participant API as Lark API
participant Parser as Response Parser
User->>CLI: feed +sensitive --feed-card-id oc_xxx --enable --user-ids id1,id2
CLI->>Validator: Validate parameters
Validator->>Validator: Check --enable/--disable mutuality
Validator->>Validator: Verify oc_ prefix in feed-card-id
Validator->>Validator: Ensure non-empty user-ids
alt Validation Fails
Validator-->>CLI: Error
CLI-->>User: Exit with validation error
else Validation Passes
Validator-->>CLI: Valid params
CLI->>RequestBuilder: Build PATCH request
RequestBuilder-->>CLI: PATCH /im/v2/feed_cards/oc_xxx
CLI->>API: Send PATCH with time_sensitive=true, user_ids
API-->>CLI: JSON response {data, failed_user_reasons}
CLI->>Parser: Parse response
Parser->>Parser: Extract failed_user_reasons
Parser->>Parser: Compute success count
alt All Users Failed
Parser-->>CLI: All failed error
CLI-->>User: stderr: warnings, stdout: error message
else Some Users Failed
Parser-->>CLI: Partial failure error
CLI-->>User: stderr: warnings, stdout: success message
else All Users Succeeded
Parser-->>CLI: Success
CLI-->>User: stdout: success message
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
github-actions
Bot
added
the
size/L
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/feed/feed_sensitive.go`:
- Around line 59-62: The current check only ensures userIDs has non-zero length
but allows blank or whitespace-only elements; after retrieving userIDs :=
runtime.StrSlice("user-ids"), iterate the slice, trim each entry using
strings.TrimSpace and reject if any trimmed value == "" (return
output.ErrValidation with a clear message like "--user-ids must not contain
empty values"); optionally replace the slice with the cleaned/trimmed values
(e.g., build a new slice of trimmed IDs) before further use so downstream code
receives normalized IDs.
In `@tests_e2e/feed/2026_04_29_feed_sensitive_test.go`:
- Around line 237-260: createChatForFeed currently leaves created chats behind;
add a best-effort cleanup by registering t.Cleanup in createChatForFeed to
remove the chat_id when the test finishes. After extracting chatID (in
createChatForFeed), call t.Cleanup(func(){ ... }) that attempts to delete the
chat using the available removal path (e.g. use clie2e.RunCmd with the
appropriate "im +chat-delete" args if supported, or call the API client used
elsewhere in tests), ensure deletion failures are non-fatal but logged (use
t.Log or t.Errorf), and remove the unused parentT handling; keep the cleanup
best-effort and tolerant of missing delete commands/errors.
- Around line 120-136: The dry-run test TestFeedSensitive_DryRunFlagBehavior
must not read real developer credentials; before calling clie2e.RunCmd, set
LARKSUITE_CLI_CONFIG_DIR, LARKSUITE_CLI_APP_ID, LARKSUITE_CLI_APP_SECRET, and
LARKSUITE_CLI_BRAND to fake values (either by adding them to the clie2e.Request
env/Env field if supported or by calling os.Setenv with defer restore) so the
subprocess is isolated; implement this inline in
TestFeedSensitive_DryRunFlagBehavior (or add a small helper used by this test
and other dry-run tests) and then call clie2e.RunCmd as before.
🪄 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: f9d8de14-589e-47c9-a5f5-ee09561ab4f9
📒 Files selected for processing (9)
shortcuts/common/types.goshortcuts/feed/feed_sensitive.goshortcuts/feed/feed_sensitive_test.goshortcuts/feed/shortcuts.goshortcuts/feed/shortcuts_test.goshortcuts/register.goskills/lark-feed/SKILL.mdskills/lark-feed/references/lark-feed-sensitive.mdtests_e2e/feed/2026_04_29_feed_sensitive_test.go
Comment on lines
+59
to
+62
| userIDs := runtime.StrSlice("user-ids") | ||
| if len(userIDs) == 0 { | ||
| return output.ErrValidation("--user-ids is required and must not be empty") | ||
| } |
There was a problem hiding this comment.
Strengthen --user-ids validation to reject blank elements.
Line 60 only checks list length; values like --user-ids "," or whitespace-only entries can still pass.
🛠️ Suggested patch
import (
"context"
"fmt"
"io"
+ "strings"
"github.com/larksuite/cli/internal/output"
"github.com/larksuite/cli/internal/validate"
"github.com/larksuite/cli/shortcuts/common"
)
@@
userIDs := runtime.StrSlice("user-ids")
if len(userIDs) == 0 {
return output.ErrValidation("--user-ids is required and must not be empty")
}
+ for _, id := range userIDs {
+ if strings.TrimSpace(id) == "" {
+ return output.ErrValidation("--user-ids must not contain empty values")
+ }
+ }
return nil
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| userIDs := runtime.StrSlice("user-ids") | |
| if len(userIDs) == 0 { | |
| return output.ErrValidation("--user-ids is required and must not be empty") | |
| } | |
| import ( | |
| "context" | |
| "fmt" | |
| "io" | |
| "strings" | |
| "github.com/larksuite/cli/internal/output" | |
| "github.com/larksuite/cli/internal/validate" | |
| "github.com/larksuite/cli/shortcuts/common" | |
| ) | |
| // ... (other code) | |
| Validate: func(ctx context.Context, _ *flags.FlagSet) *output.Result { | |
| userIDs := runtime.StrSlice("user-ids") | |
| if len(userIDs) == 0 { | |
| return output.ErrValidation("--user-ids is required and must not be empty") | |
| } | |
| for _, id := range userIDs { | |
| if strings.TrimSpace(id) == "" { | |
| return output.ErrValidation("--user-ids must not contain empty values") | |
| } | |
| } | |
| return nil | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@shortcuts/feed/feed_sensitive.go` around lines 59 - 62, The current check
only ensures userIDs has non-zero length but allows blank or whitespace-only
elements; after retrieving userIDs := runtime.StrSlice("user-ids"), iterate the
slice, trim each entry using strings.TrimSpace and reject if any trimmed value
== "" (return output.ErrValidation with a clear message like "--user-ids must
not contain empty values"); optionally replace the slice with the
cleaned/trimmed values (e.g., build a new slice of trimmed IDs) before further
use so downstream code receives normalized IDs.
Comment on lines
+120
to
+136
| func TestFeedSensitive_DryRunFlagBehavior(t *testing.T) { | ||
| ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) | ||
| t.Cleanup(cancel) | ||
|
|
||
| t.Run("dry-run prints PATCH preview", func(t *testing.T) { | ||
| result, err := clie2e.RunCmd(ctx, clie2e.Request{ | ||
| Args: []string{ | ||
| "feed", "+sensitive", | ||
| "--feed-card-id", "oc_dryruntestid", | ||
| "--enable", | ||
| "--user-ids", sandboxUserOpenID, | ||
| "--dry-run", | ||
| }, | ||
| DefaultAs: "bot", | ||
| }) | ||
| require.NoError(t, err) | ||
| result.AssertExitCode(t, 0) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify where RunCmd is implemented and whether required env vars are injected there.
fd -i '.*\.go' tests | xargs rg -n -C3 'func RunCmd|LARKSUITE_CLI_CONFIG_DIR|LARKSUITE_CLI_APP_ID|LARKSUITE_CLI_APP_SECRET|LARKSUITE_CLI_BRAND'Repository: larksuite/cli
Length of output: 9319
🏁 Script executed:
fd -i '*feed*sensitive*test.go' tests/Repository: larksuite/cli
Length of output: 360
🏁 Script executed:
fd -g '*feed*sensitive*test.go' tests/Repository: larksuite/cli
Length of output: 39
🏁 Script executed:
find . -name '*feed*sensitive*test.go' -type fRepository: larksuite/cli
Length of output: 148
🏁 Script executed:
cat -n ./tests_e2e/feed/2026_04_29_feed_sensitive_test.goRepository: larksuite/cli
Length of output: 11120
🏁 Script executed:
find ./tests_e2e -name '*_test.go' -type f | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && head -40 {}'Repository: larksuite/cli
Length of output: 1491
Add environment isolation to the dry-run test to prevent reading real credentials.
The TestFeedSensitive_DryRunFlagBehavior test (lines 120–146) is a dry-run E2E test but does not set LARKSUITE_CLI_CONFIG_DIR, LARKSUITE_CLI_APP_ID, LARKSUITE_CLI_APP_SECRET, or LARKSUITE_CLI_BRAND. Without this isolation, the subprocess can read a developer's real credentials from the local machine instead of the fake ones needed for --dry-run.
All other dry-run E2E tests in the repository follow this pattern:
drive_apply_permission_dryrun_test.go(L32–35)drive_search_dryrun_test.go(L334–337)mail_share_to_chat_dryrun_test.go(L23–26)calendar_update_dryrun_test.go(L17–20)
Add a helper function to set these vars before calling RunCmd, or inline them in the test like the existing examples.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests_e2e/feed/2026_04_29_feed_sensitive_test.go` around lines 120 - 136, The
dry-run test TestFeedSensitive_DryRunFlagBehavior must not read real developer
credentials; before calling clie2e.RunCmd, set LARKSUITE_CLI_CONFIG_DIR,
LARKSUITE_CLI_APP_ID, LARKSUITE_CLI_APP_SECRET, and LARKSUITE_CLI_BRAND to fake
values (either by adding them to the clie2e.Request env/Env field if supported
or by calling os.Setenv with defer restore) so the subprocess is isolated;
implement this inline in TestFeedSensitive_DryRunFlagBehavior (or add a small
helper used by this test and other dry-run tests) and then call clie2e.RunCmd as
before.
Comment on lines
+237
to
+260
| // createChatForFeed creates a private group chat via im +chat-create and | ||
| // returns the chat_id (which doubles as the feed_card_id for feed tests). | ||
| // No cleanup is registered because lark-cli im has no chat-delete command. | ||
| func createChatForFeed(t *testing.T, parentT *testing.T, ctx context.Context, name string) string { | ||
| t.Helper() | ||
|
|
||
| result, err := clie2e.RunCmd(ctx, clie2e.Request{ | ||
| Args: []string{ | ||
| "im", "+chat-create", | ||
| "--name", name, | ||
| "--type", "private", | ||
| }, | ||
| DefaultAs: "bot", | ||
| }) | ||
| require.NoError(t, err) | ||
| result.AssertExitCode(t, 0) | ||
| result.AssertStdoutStatus(t, true) | ||
|
|
||
| chatID := gjson.Get(result.Stdout, "data.chat_id").String() | ||
| require.NotEmpty(t, chatID, "chat_id must not be empty, stdout:\n%s", result.Stdout) | ||
|
|
||
| // No chat-delete command exists in lark-cli im; chats are left in the account. | ||
| _ = parentT | ||
|
|
There was a problem hiding this comment.
Live E2E chat creation should include cleanup flow.
Leaving chats behind introduces long-term state pollution and potential quota/noise issues in shared environments. Please add best-effort cleanup in t.Cleanup (via available CLI/API path), even if failures are non-fatal.
Based on learnings: "Live E2E tests must be self-contained with create/use/cleanup flows and placed in tests/cli_e2e/<domain>/."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests_e2e/feed/2026_04_29_feed_sensitive_test.go` around lines 237 - 260,
createChatForFeed currently leaves created chats behind; add a best-effort
cleanup by registering t.Cleanup in createChatForFeed to remove the chat_id when
the test finishes. After extracting chatID (in createChatForFeed), call
t.Cleanup(func(){ ... }) that attempts to delete the chat using the available
removal path (e.g. use clie2e.RunCmd with the appropriate "im +chat-delete" args
if supported, or call the API client used elsewhere in tests), ensure deletion
failures are non-fatal but logged (use t.Log or t.Errorf), and remove the unused
parentT handling; keep the cleanup best-effort and tolerant of missing delete
commands/errors.
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
2 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
Add a new
feeddomain with a single shortcut+sensitivethat calls the Lark PATCH/im/v2/feed_cards/:feed_card_idAPI to set or unset the time-sensitive (即时提醒/置顶) status of a feed card for specified users in a group chat.Changes
shortcuts/feed/package with+sensitivecommand--enable/--disablemutual exclusion,oc_prefix check for feed-card-idskills/lark-feed/SKILL.mdandreferences/lark-feed-sensitive.mdTest Plan
Summary by CodeRabbit
Release Notes
New Features
feed +sensitiveshortcut to manage time-sensitive status for feed cards in group chats. Supports per-user enable/disable operations, dry-run mode for previewing changes, and detailed error reporting.Documentation
Tests