fix(task): default +get-my-tasks to incomplete results#309
Conversation
|
|
github-actions
Bot
added
domain/task
📝 WalkthroughWalkthroughThe PR fixes a bug where omitting the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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)
Comment |
Greptile SummaryThis PR fixes a behavioral bug where omitting Confidence Score: 5/5This PR is safe to merge — it correctly fixes a filter-defaulting bug, aligns behavior with documentation, and adds focused regression tests. The change is minimal and targeted. runtime.Bool("complete") returns false by default when the flag is not passed, so strconv.FormatBool(false) always produces the correct default. All three filter scenarios are covered by new tests, and reg.Verify(t) via t.Cleanup confirms stubs were hit. No P0 or P1 findings. No files require special attention.
|
| Filename | Overview |
|---|---|
| shortcuts/task/task_get_my_tasks.go | Removes the Flags().Changed("complete") guard in both DryRun and Execute, so completed is always sent — defaulting to false — regardless of whether the flag was explicitly passed. |
| shortcuts/task/task_get_my_tasks_test.go | Adds three regression tests covering omitted, explicit-false, and explicit-true completion filters using the httpmock substring URL matcher. |
| skills/lark-task/references/lark-task-get-my-tasks.md | Updates the --complete parameter description to document that omitting the flag now defaults to incomplete tasks. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["+get-my-tasks invoked"] --> B{--complete flag\nexplicitly set?}
B -- "Yes (--complete=true)" --> C["completed=true\nsent to API"]
B -- "Yes (--complete=false)" --> D["completed=false\nsent to API"]
B -- "No (omitted)" --> D
C --> E["API returns completed tasks"]
D --> F["API returns incomplete tasks (default)"]
Reviews (1): Last reviewed commit: "fix(task): default +get-my-tasks to inco..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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_get_my_tasks_test.go`:
- Around line 118-120: The test helper doesn't isolate CLI config state causing
machine-local cache interference; modify taskTestConfig (used by
taskShortcutTestFactory) to set a dedicated temp config dir for each test by
calling t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) so config state is
isolated, ensuring taskShortcutTestFactory and any callers (e.g.,
task_get_my_tasks_test.go helpers) use cmdutil.TestFactory(t, config) with the
per-test env override.
🪄 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: f6e7ecbf-850c-491e-b310-933d18a8e05b
📒 Files selected for processing (3)
shortcuts/task/task_get_my_tasks.goshortcuts/task/task_get_my_tasks_test.goskills/lark-task/references/lark-task-get-my-tasks.md
| f, stdout, _, reg := taskShortcutTestFactory(t) | ||
| warmTenantToken(t, f, reg) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether task test factory helper follows required test setup conventions.
# 1) Locate helper definition and inspect nearby lines.
rg -n -C4 'func\s+taskShortcutTestFactory\s*\('
# 2) Check usage of required factory constructor.
rg -n -C3 'cmdutil\.TestFactory\s*\('
# 3) Check test config isolation via temp config dir env var.
rg -n -C3 'LARKSUITE_CLI_CONFIG_DIR'Repository: larksuite/cli
Length of output: 50370
🏁 Script executed:
#!/bin/bash
# Search for taskTestConfig definition and nearby code
rg -n -B2 -A10 'func\s+taskTestConfig\s*\('Repository: larksuite/cli
Length of output: 926
🏁 Script executed:
#!/bin/bash
# Check other test config functions
rg -n -B1 -A8 'func\s+(driveTestConfig|mailTestConfig|defaultConfig)\s*\(' shortcuts/
# Also check the internal registry test pattern mentioned in learnings
rg -n -B2 -A8 'func\s+ensureFreshRegistry\s*\('Repository: larksuite/cli
Length of output: 3307
🏁 Script executed:
#!/bin/bash
# Search for ensureFreshRegistry in internal/registry
rg -n -B2 -A10 'ensureFreshRegistry' internal/registry/Repository: larksuite/cli
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Search for the pattern in internal/registry/registry_test.go
rg -n -A10 'LARKSUITE_CLI_CONFIG_DIR' internal/registry/Repository: larksuite/cli
Length of output: 5165
🏁 Script executed:
#!/bin/bash
# Check the actual test function at lines 118-120 in task_get_my_tasks_test.go
rg -n -B10 -A5 'taskShortcutTestFactory' shortcuts/task/task_get_my_tasks_test.go | head -40Repository: larksuite/cli
Length of output: 1005
Add config state isolation to test setup helper.
The taskShortcutTestFactory helper correctly uses cmdutil.TestFactory(t, config), satisfying the first guideline. However, taskTestConfig(t) does not isolate config state per coding guidelines. Add t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) to the helper to prevent machine-local cache interference, following the pattern established in internal/registry/remote_test.go.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@shortcuts/task/task_get_my_tasks_test.go` around lines 118 - 120, The test
helper doesn't isolate CLI config state causing machine-local cache
interference; modify taskTestConfig (used by taskShortcutTestFactory) to set a
dedicated temp config dir for each test by calling
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) so config state is isolated,
ensuring taskShortcutTestFactory and any callers (e.g.,
task_get_my_tasks_test.go helpers) use cmdutil.TestFactory(t, config) with the
per-test env override.
|
Hi @OwenYWT, Thank you so much for your PR and for looking into this issue! We really appreciate your effort in adding the tests and trying to fix this behavior. After discussing this internally (similar to what was brought up in issue #172), we realized that the core issue was actually a mistake in our documentation and help text, rather than a bug in the code logic itself. If we were to change the default behavior to explicitly send completed=false when the flag is omitted, it would remove the ability for users to fetch both completed and incomplete tasks simultaneously. To preserve the ability to query all tasks, we have decided to keep the original logic (omitting the parameter when the flag is not passed) but update the documentation and the command's help text to accurately reflect this three-state behavior (omitted = both, true = completed, false = incomplete). The documentation and help text fix has been merged in PR #310. Again, thank you for your keen eye and for taking the time to contribute to the project! We look forward to seeing more of your contributions in the future. |
3 participants
Summary
Make
task +get-my-tasksdefault to incomplete tasks when--completeis omitted, matching the command help and issue expectation.Changes
completedfilter, defaulting tofalsewhen--completeis omitted--complete=trueand--complete=falseas explicit overridestrue, andfalsecompletion filtersRelated Issues
Test Plan
GOCACHE=/tmp/go-build-cache GOMODCACHE=/tmp/go-mod-cache /opt/homebrew/opt/go/bin/go test ./shortcuts/task -run 'TestGetMyTasks_CompletionFilterDefaultsToIncomplete|TestGetMyTasks_LocalTimeFormatting'GOCACHE=/tmp/go-build-cache GOMODCACHE=/tmp/go-mod-cache /opt/homebrew/opt/go/bin/go test ./shortcuts/taskSummary by CodeRabbit
Release Notes
Changes
--completeflag behavior: the API request now always includes the completion filter instead of omitting it when the flag is not provided.--completenow queries incomplete tasks only (previously returned all tasks).Documentation
--completeflag documentation to reflect the new default filtering behavior.