feat(task): add task shortcuts with skill docs and tests#377
feat(task): add task shortcuts with skill docs and tests#377ILUO wants to merge 23 commits intolarksuite:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds five new task shortcuts (+get-related-tasks, +search, +set-ancestor, +subscribe-event, +tasklist-search), query/output helper utilities, comprehensive unit/integration tests, registration of the shortcuts in the task registry, and documentation pages for each shortcut. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (CLI)
participant CLI as lark-cli Runtime
participant SearchAPI as Task Search API
participant DetailAPI as Task Detail API
User->>CLI: run `task +search --query ...`
CLI->>SearchAPI: POST /open-apis/task/v2/tasks/search (body, page loop)
SearchAPI-->>CLI: returns hits + page_token/has_more
CLI->>DetailAPI: GET /open-apis/task/v2/tasks/:guid (per hit)
DetailAPI-->>CLI: returns task detail or error
CLI->>User: render aggregated results (pretty/json) and next page token
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryAdds five new task shortcuts ( Confidence Score: 4/5Safe to merge once open P1 issues from prior review rounds are resolved; new findings in this pass are P2 only. Multiple P1 findings flagged in earlier review rounds remain unaddressed (scope/risk mismatch on SubscribeTaskEvent, missing user_id_type=open_id on search POSTs, stale pagination contract in GetRelatedTasks, empty SubscribeTaskEvent body). New findings this pass are both P2. Score reflects convergence on code quality but ongoing correctness concerns on several shortcuts. shortcuts/task/task_subscribe_event.go (scope + empty body), shortcuts/task/task_search.go and task_tasklist_search.go (missing query param), shortcuts/task/task_get_related_tasks.go (pagination metadata), shortcuts/task/task_query_helpers.go (dead function) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User invokes task shortcut] --> B{Which command?}
B --> C["+search\ntask_search.go"]
B --> D["+get-related-tasks\ntask_get_related_tasks.go"]
B --> E["+set-ancestor\ntask_set_ancestor.go"]
B --> F["+subscribe-event\ntask_subscribe_event.go"]
B --> G["+tasklist-search\ntask_tasklist_search.go"]
C --> C1[buildTaskSearchBody]
C1 --> C2[POST /task/v2/tasks/search]
C2 --> C3[GET /task/v2/tasks/:guid per hit]
C3 --> C4[outputTaskSummary]
D --> D1[GET list_related_task paginated]
D1 --> D2[Client-side filter created-by-me/followed-by-me]
D2 --> D3[outputRelatedTask]
E --> E1[POST set_ancestor_task]
E1 --> E2[HandleTaskApiResult]
F --> F1[POST task_subscription empty body]
G --> G1[buildTasklistSearchBody]
G1 --> G2[POST /task/v2/tasklists/search]
G2 --> G3[GET /task/v2/tasklists/:guid per hit]
C1 & G1 & D1 -.-> H[task_query_helpers.go]
Reviews (13): Last reviewed commit: "docs(task): remove auth status --json gu..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
shortcuts/task/task_tasklist_search_test.go (1)
109-113: Consider using an explicit field instead of inferring from test name.The validation skip logic relies on the test name containing "error", which is fragile and could break if test names are refactored.
♻️ Suggested improvement
tests := []struct { name string setup func(*cobra.Command) wantParts []string + wantErr bool }{ { name: "valid dry run", setup: func(cmd *cobra.Command) { _ = cmd.Flags().Set("query", "Q2") _ = cmd.Flags().Set("page-token", "pt_tl") }, wantParts: []string{"POST /open-apis/task/v2/tasklists/search", `"query":"Q2"`}, }, { name: "dry run error on invalid create time", setup: func(cmd *cobra.Command) { _ = cmd.Flags().Set("create-time", "bad-time") }, wantParts: []string{"error:"}, + wantErr: true, }, }Then use
if !tt.wantErr { ... }instead ofstrings.Contains(tt.name, "error").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/task/task_tasklist_search_test.go` around lines 109 - 113, The test currently skips calling SearchTasklist.Validate by checking strings.Contains(tt.name, "error"), which is fragile; change the logic to use the test case's explicit expectation field (tt.wantErr) instead: replace the conditional that uses tt.name with if !tt.wantErr { if err := SearchTasklist.Validate(nil, runtime); err != nil { t.Fatalf("Validate() error = %v", err) } } so validation runs for cases not expecting an error and is robust to renaming.
🤖 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/task/task_subscribe_event_test.go`:
- Around line 50-52: The test is widening SubscribeTaskEvent.AuthTypes to
include "bot" and then exercising "--as bot", which bypasses the intended
user-only shortcut; revert the patching and exercise the happy-path with a user
identity instead (call runMountedTaskShortcut with args using "--as", "user"),
or alternatively keep the current positive case but add a separate negative test
that leaves SubscribeTaskEvent.AuthTypes unchanged and asserts that invoking
runMountedTaskShortcut with "--as", "bot" returns an error (i.e., bot auth is
refused). Ensure references to SubscribeTaskEvent, AuthTypes, and
runMountedTaskShortcut are used to locate and update the test.
In `@shortcuts/task/task_tasklist_search.go`:
- Around line 110-113: When getTasklistDetail(runtime, tasklistID) fails we
currently append only {"guid": tasklistID}, which leaves tasklist["name"] nil
and causes the pretty-printer to render "<nil>"; update both failure branches
(the one at getTasklistDetail and the similar one around lines 135-137) to
include a safe "name" value (e.g., the GUID or a placeholder like "(unknown
tasklist: <guid>)") when appending to tasklists so the formatter will print a
sensible fallback instead of "<nil>" while preserving the guid in the map.
In `@skills/lark-task/references/lark-task-get-related-tasks.md`:
- Around line 7-8: The docs and CLI disagree on the unit for page_token (docs
say microseconds, CLI help in shortcuts/task/task_get_related_tasks.go describes
milliseconds); reconcile them by making the CLI help text match the canonical
unit used by +get-related-tasks (choose microseconds if that is intended) —
update the --page-token flag description in the TaskGetRelatedTasks command
(look for the flag/usage string in task_get_related_tasks.go and any references
to PageToken or page-token) so it explicitly states "microseconds" (or convert
behavior and tests to milliseconds if you decide to keep ms), and ensure any
parsing/serialization (e.g., where PageToken is parsed) uses the same unit.
---
Nitpick comments:
In `@shortcuts/task/task_tasklist_search_test.go`:
- Around line 109-113: The test currently skips calling SearchTasklist.Validate
by checking strings.Contains(tt.name, "error"), which is fragile; change the
logic to use the test case's explicit expectation field (tt.wantErr) instead:
replace the conditional that uses tt.name with if !tt.wantErr { if err :=
SearchTasklist.Validate(nil, runtime); err != nil { t.Fatalf("Validate() error =
%v", err) } } so validation runs for cases not expecting an error and is robust
to renaming.
🪄 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: a8a083f7-b4e6-4b8c-990f-a81ef6fb1ebf
📒 Files selected for processing (19)
shortcuts/task/shortcuts.goshortcuts/task/task_get_related_tasks.goshortcuts/task/task_get_related_tasks_test.goshortcuts/task/task_query_helpers.goshortcuts/task/task_query_helpers_test.goshortcuts/task/task_search.goshortcuts/task/task_search_test.goshortcuts/task/task_set_ancestor.goshortcuts/task/task_set_ancestor_test.goshortcuts/task/task_subscribe_event.goshortcuts/task/task_subscribe_event_test.goshortcuts/task/task_tasklist_search.goshortcuts/task/task_tasklist_search_test.goskills/lark-task/SKILL.mdskills/lark-task/references/lark-task-get-related-tasks.mdskills/lark-task/references/lark-task-search.mdskills/lark-task/references/lark-task-set-ancestor.mdskills/lark-task/references/lark-task-subscribe-event.mdskills/lark-task/references/lark-task-tasklist-search.md
|
Tip: Greploop — Automatically fix all review issues by running Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/task/task_query_helpers.go`:
- Around line 25-53: The parseTimeRangeMillis function currently allows reversed
ranges; after successfully parsing both startMillis and endMillis (use
parseTimeFlagSec outputs turned into milliseconds strings in
parseTimeRangeMillis), add a validation step: if both startMillis and endMillis
are non-empty, convert them to integers (e.g., with strconv.ParseInt) and ensure
start <= end, returning a descriptive error (e.g., "start time must be <= end
time") if the check fails; keep existing error behavior for individual
parseTimeFlagSec failures and only perform this additional comparison when both
endpoints are present.
🪄 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: a787f92c-84b4-470f-997f-19a5629eb991
📒 Files selected for processing (2)
shortcuts/task/task_query_helpers.goshortcuts/task/task_query_helpers_test.go
✅ Files skipped from review due to trivial changes (1)
- shortcuts/task/task_query_helpers_test.go
Summary
Add new task shortcuts for related task listing, task search, ancestor setup,
task event subscription, and tasklist search, together with matching skill
documentation and table-driven tests.
Changes
task +get-related-taskstask +searchtask +set-ancestortask +subscribe_eventtask +tasklist-searchshortcuts/task/shortcuts.go+searchfirst)page_tokenbehavior in+get-related-tasksTest Plan
go test ./shortcuts/task ./shortcuts/eventRelated Issues
N/A
Summary by CodeRabbit
New Features
Documentation
Tests