feat(feed): add +sensitive shortcut for time-sensitive status#717
feat(feed): add +sensitive shortcut for time-sensitive status#717jinzemo wants to merge 10 commits intolarksuite:mainfrom
Conversation
|
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 Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as CLI Handler
participant Validator
participant APIClient
participant APIServer as Lark API
participant Formatter
User->>CLI: feed +sensitive --enable/disable --feed-card-id <id> --user-ids <ids>
CLI->>Validator: Validate parameters
Validator->>Validator: Check --enable/--disable mutual exclusivity
Validator->>Validator: Validate chat ID format (oc_ prefix)
Validator->>Validator: Check non-empty user IDs
alt Validation fails
Validator-->>CLI: Return validation error
CLI-->>User: Exit with error message
else Validation passes
Validator-->>CLI: OK
alt Dry-run mode
CLI->>APIClient: Build PATCH request (no execute)
APIClient-->>CLI: Request preview
else Normal execution
CLI->>APIClient: Build PATCH request
APIClient->>APIServer: PATCH /open-apis/im/v2/feed_cards/{feedCardId}
APIServer-->>APIClient: Response with failed_user_reasons
end
APIClient-->>Formatter: Parsed response
Formatter->>Formatter: Extract failed_user_reasons by user
alt All users succeeded
Formatter-->>CLI: Success summary
CLI-->>User: Exit code 0, output success
else Partial failures
Formatter-->>CLI: Success summary + warnings
CLI-->>User: Exit API error, stderr warnings
else All users failed
Formatter-->>CLI: All failed error
CLI-->>User: Exit API error, stderr warnings
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
tests_e2e/feed/2026_04_29_feed_sensitive_test.go (1)
221-262: Drop the unusedparentTparameter.
createChatForFeedonly keepsparentTto silence the unused variable, which makes the helper noisier than necessary. Removing it would simplify both the helper and its callers.🤖 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 221 - 262, The helper createChatForFeed currently accepts an unused parentT parameter which is only referenced as `_ = parentT`; remove the unused parameter by changing the signature from func createChatForFeed(t *testing.T, parentT *testing.T, ctx context.Context, name string) string to func createChatForFeed(t *testing.T, ctx context.Context, name string) string, delete the `_ = parentT` line, and update all call sites to pass only (t, ctx, name) so callers compile cleanly and the helper is simplified.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/lark-feed/references/lark-feed-sensitive.md`:
- Around line 77-84: Update the "退出码" table to distinguish Validate failures
during dry-run flows from ordinary parameter validation errors: keep the
existing row for "参数校验错误" with exit code 1 for normal runs, and add a new row
(or annotate) stating that validation failures that occur inside the Validate
step during dry-run invocations exit with code 2 and emit the structured JSON
error envelope; reference the Validate step and the dry-run handler (e.g.,
"Validate" and "dry-run") so readers can locate where this behavior originates.
- Around line 67-75: Add the Markdown language tag "text" to both fenced code
blocks in the examples so they become ```text ... ```, specifically update the
block containing "Time-sensitive updated for 2 user(s) (feed_card_id: oc_xxx)"
and the warning block that starts with "warning: 1 user(s) failed:" so they pass
markdownlint MD040; keep the content unchanged, only add the language specifier
to the opening fence of each block.
In `@tests_e2e/feed/2026_04_29_feed_sensitive_test.go`:
- Around line 148-219: Update TestFeedSensitive_ValidationErrors to assert the
specific validation failure semantics instead of a broad non-zero check: replace
the assert.NotEqual(t, 0, result.ExitCode) checks with assert.Equal(t, 2,
result.ExitCode) for each subtest (use the same result.ExitCode from
clie2e.RunCmd), and add assertions on result.Stdout to confirm the structured
JSON error envelope (unmarshal or use JSON assertions to verify an error object
with a validation/validate stage indicator and a message referencing the
offending flag or "oc_" prefix). Ensure you reference the existing symbols
TestFeedSensitive_ValidationErrors, clie2e.RunCmd, result.ExitCode, and
result.Stdout when making these changes.
- Around line 118-145: TestFeedSensitive_DryRunFlagBehavior runs clie2e.RunCmd
without setting test-local env vars so it may pick up ambient credentials;
before calling clie2e.RunCmd in that subtest, set the four env vars using
t.Setenv: LARKSUITE_CLI_CONFIG_DIR (use t.TempDir()), LARKSUITE_CLI_APP_ID
("app"), LARKSUITE_CLI_APP_SECRET ("secret"), and LARKSUITE_CLI_BRAND ("feishu")
so the dry-run is isolated from real credentials and mirrors other dry-run tests
(refer to TestFeedSensitive_DryRunFlagBehavior and the clie2e.RunCmd invocation
to locate where to add the t.Setenv calls).
---
Nitpick comments:
In `@tests_e2e/feed/2026_04_29_feed_sensitive_test.go`:
- Around line 221-262: The helper createChatForFeed currently accepts an unused
parentT parameter which is only referenced as `_ = parentT`; remove the unused
parameter by changing the signature from func createChatForFeed(t *testing.T,
parentT *testing.T, ctx context.Context, name string) string to func
createChatForFeed(t *testing.T, ctx context.Context, name string) string, delete
the `_ = parentT` line, and update all call sites to pass only (t, ctx, name) so
callers compile cleanly and the helper is simplified.
🪄 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: 09fd7ed4-3a4f-484b-8360-d0982ca7c8ab
📒 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
| ``` | ||
| Time-sensitive updated for 2 user(s) (feed_card_id: oc_xxx) | ||
| ``` | ||
|
|
||
| 部分失败时,stdout 显示成功数,stderr 显示警告: | ||
| ``` | ||
| warning: 1 user(s) failed: | ||
| ou_yyy: The user is not in the chat | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the output examples.
The plain fences trip markdownlint MD040.
Suggested fix
-```
+```text
Time-sensitive updated for 2 user(s) (feed_card_id: oc_xxx)
-```
+```textApply the same text tag to the warning block below.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 67-67: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 72-72: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/lark-feed/references/lark-feed-sensitive.md` around lines 67 - 75, Add
the Markdown language tag "text" to both fenced code blocks in the examples so
they become ```text ... ```, specifically update the block containing
"Time-sensitive updated for 2 user(s) (feed_card_id: oc_xxx)" and the warning
block that starts with "warning: 1 user(s) failed:" so they pass markdownlint
MD040; keep the content unchanged, only add the language specifier to the
opening fence of each block.
| ## 退出码 | ||
|
|
||
| | 条件 | 退出码 | | ||
| |------|--------| | ||
| | 全部成功 | 0 | | ||
| | 部分或全部用户失败 | 1 | | ||
| | 参数校验错误 | 1 | | ||
| | API 错误 | 1 | |
There was a problem hiding this comment.
Clarify the validation exit code for dry-run flows.
The table currently says parameter validation errors exit 1, but Validate failures in dry-run invocations happen before the dry-run handler and exit 2. Please split that row or annotate the dry-run exception.
Suggested fix
-| 参数校验错误 | 1 |
+| 参数校验错误(非 dry-run) | 1 |
+| 参数校验错误(dry-run) | 2 |Based on learnings, validation failures inside Validate for dry-run flows exit with code 2 and emit the structured JSON error envelope.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/lark-feed/references/lark-feed-sensitive.md` around lines 77 - 84,
Update the "退出码" table to distinguish Validate failures during dry-run flows
from ordinary parameter validation errors: keep the existing row for "参数校验错误"
with exit code 1 for normal runs, and add a new row (or annotate) stating that
validation failures that occur inside the Validate step during dry-run
invocations exit with code 2 and emit the structured JSON error envelope;
reference the Validate step and the dry-run handler (e.g., "Validate" and
"dry-run") so readers can locate where this behavior originates.
| // TestFeedSensitive_DryRunFlagBehavior proves that --dry-run produces a PATCH | ||
| // request preview on stdout and exits 0 without making a real API call. | ||
| 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) | ||
|
|
||
| combined := result.Stdout + result.Stderr | ||
| assert.Contains(t, combined, "PATCH", | ||
| "dry-run output must contain HTTP method PATCH, combined:\n%s", combined) | ||
| assert.Contains(t, combined, "/im/v2/feed_cards/", | ||
| "dry-run output must contain the feed_cards API path, combined:\n%s", combined) | ||
| assert.Contains(t, combined, "oc_dryruntestid", | ||
| "dry-run output must contain the feed_card_id path segment, combined:\n%s", combined) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "LARKSUITE_CLI_(APP_ID|APP_SECRET|CONFIG_DIR)|BRAND" tests/cli_e2e shortcutsRepository: larksuite/cli
Length of output: 4053
🏁 Script executed:
#!/bin/bash
# Find the feed_sensitive test file
find . -name "*feed*sensitive*test.go" -type fRepository: larksuite/cli
Length of output: 148
🏁 Script executed:
#!/bin/bash
# Also search in tests directory structure
find tests* -name "*feed*" -type d 2>/dev/null | head -20Repository: larksuite/cli
Length of output: 72
🏁 Script executed:
#!/bin/bash
# Broader search for feed tests
find . -path "*/feed/*test.go" -type fRepository: larksuite/cli
Length of output: 183
🏁 Script executed:
#!/bin/bash
# Read the feed test file to check for env vars
cat -n tests_e2e/feed/2026_04_29_feed_sensitive_test.goRepository: larksuite/cli
Length of output: 11120
Add env vars to isolate the dry-run test from ambient credentials.
The test at lines 124-145 must set LARKSUITE_CLI_APP_ID, APP_SECRET, BRAND, and LARKSUITE_CLI_CONFIG_DIR before calling clie2e.RunCmd(). Other dry-run tests in the suite (drive, calendar, mail, okr) already do this; this test should follow the same pattern:
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir())
t.Setenv("LARKSUITE_CLI_APP_ID", "app")
t.Setenv("LARKSUITE_CLI_APP_SECRET", "secret")
t.Setenv("LARKSUITE_CLI_BRAND", "feishu")
Without these, the test will inherit the developer's real credentials or fail only in CI.
🤖 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 118 - 145,
TestFeedSensitive_DryRunFlagBehavior runs clie2e.RunCmd without setting
test-local env vars so it may pick up ambient credentials; before calling
clie2e.RunCmd in that subtest, set the four env vars using t.Setenv:
LARKSUITE_CLI_CONFIG_DIR (use t.TempDir()), LARKSUITE_CLI_APP_ID ("app"),
LARKSUITE_CLI_APP_SECRET ("secret"), and LARKSUITE_CLI_BRAND ("feishu") so the
dry-run is isolated from real credentials and mirrors other dry-run tests (refer
to TestFeedSensitive_DryRunFlagBehavior and the clie2e.RunCmd invocation to
locate where to add the t.Setenv calls).
| // TestFeedSensitive_ValidationErrors proves the CLI validation layer: | ||
| // - missing --enable/--disable → exit non-zero with a descriptive error | ||
| // - both --enable and --disable together → exit non-zero (mutual exclusion) | ||
| // - invalid feed-card-id (no oc_ prefix) → exit non-zero with validation error | ||
| // | ||
| // The spec says exit 1 for validation errors; the CLI framework may return exit 2 | ||
| // for flag-parse failures. Both are accepted here as "not 0". | ||
| func TestFeedSensitive_ValidationErrors(t *testing.T) { | ||
| ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) | ||
| t.Cleanup(cancel) | ||
|
|
||
| t.Run("missing enable or disable flag", func(t *testing.T) { | ||
| result, err := clie2e.RunCmd(ctx, clie2e.Request{ | ||
| Args: []string{ | ||
| "feed", "+sensitive", | ||
| "--feed-card-id", "oc_testid", | ||
| "--user-ids", sandboxUserOpenID, | ||
| // intentionally omitting --enable and --disable | ||
| }, | ||
| DefaultAs: "bot", | ||
| }) | ||
| require.NoError(t, err) | ||
| assert.NotEqual(t, 0, result.ExitCode, | ||
| "must fail when neither --enable nor --disable is provided, stdout:\n%s\nstderr:\n%s", | ||
| result.Stdout, result.Stderr) | ||
|
|
||
| combined := result.Stdout + result.Stderr | ||
| assert.Contains(t, combined, "--enable", | ||
| "error message must mention --enable flag, combined:\n%s", combined) | ||
| }) | ||
|
|
||
| t.Run("enable and disable are mutually exclusive", func(t *testing.T) { | ||
| result, err := clie2e.RunCmd(ctx, clie2e.Request{ | ||
| Args: []string{ | ||
| "feed", "+sensitive", | ||
| "--feed-card-id", "oc_testid", | ||
| "--enable", | ||
| "--disable", | ||
| "--user-ids", sandboxUserOpenID, | ||
| }, | ||
| DefaultAs: "bot", | ||
| }) | ||
| require.NoError(t, err) | ||
| assert.NotEqual(t, 0, result.ExitCode, | ||
| "must fail when both --enable and --disable are provided, stdout:\n%s\nstderr:\n%s", | ||
| result.Stdout, result.Stderr) | ||
|
|
||
| combined := result.Stdout + result.Stderr | ||
| assert.Contains(t, combined, "--disable", | ||
| "error message must mention --disable flag, combined:\n%s", combined) | ||
| }) | ||
|
|
||
| t.Run("invalid feed-card-id without oc_ prefix", func(t *testing.T) { | ||
| result, err := clie2e.RunCmd(ctx, clie2e.Request{ | ||
| Args: []string{ | ||
| "feed", "+sensitive", | ||
| "--feed-card-id", "invalid_id_no_oc_prefix", | ||
| "--enable", | ||
| "--user-ids", sandboxUserOpenID, | ||
| }, | ||
| DefaultAs: "bot", | ||
| }) | ||
| require.NoError(t, err) | ||
| assert.NotEqual(t, 0, result.ExitCode, | ||
| "must fail for feed-card-id that does not start with oc_, stdout:\n%s\nstderr:\n%s", | ||
| result.Stdout, result.Stderr) | ||
|
|
||
| combined := result.Stdout + result.Stderr | ||
| assert.Contains(t, combined, "oc_", | ||
| "error message must mention the oc_ prefix requirement, combined:\n%s", combined) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Tighten the negative-path assertions.
assert.NotEqual(..., 0) is broad enough to pass on unrelated startup failures. These validation cases should assert the actual validation exit (2) and structured JSON error envelope so regressions in +sensitive validation don't slip through.
Based on learnings: Validate-stage rejects exit code 2 and a structured JSON error envelope to stdout.
🤖 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 148 - 219,
Update TestFeedSensitive_ValidationErrors to assert the specific validation
failure semantics instead of a broad non-zero check: replace the
assert.NotEqual(t, 0, result.ExitCode) checks with assert.Equal(t, 2,
result.ExitCode) for each subtest (use the same result.ExitCode from
clie2e.RunCmd), and add assertions on result.Stdout to confirm the structured
JSON error envelope (unmarshal or use JSON assertions to verify an error object
with a validation/validate stage indicator and a message referencing the
offending flag or "oc_" prefix). Ensure you reference the existing symbols
TestFeedSensitive_ValidationErrors, clie2e.RunCmd, result.ExitCode, and
result.Stdout when making these changes.
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
New Features
feed +sensitivecommand to manage Feed Card time-sensitive state for group chats. Enable or disable per-user time-sensitive behavior using--enable/--disableflags. Supports multiple user IDs and--dry-runmode for API request preview.Documentation
lark-feedskill documentation andfeed +sensitivecommand reference guide, including usage examples, prerequisites, and exit code information.