fix(im): +flag-list --page-all no longer silently truncates at default page-limit - #1875
fix(im): +flag-list --page-all no longer silently truncates at default page-limit#1875jacob-qu wants to merge 1 commit into
Conversation
…t page-limit `im +flag-list --page-all` was still capped by the default `--page-limit` of 20 and stopped there without any signal, even when `has_more` was true. Because the server returns canceled flags before active ones, active flags often live past page 20, so callers saw `flag_items: []` and read it as "no flags" — misleading for humans and AI agents alike. Align flag-list with the existing pagination contract used by `im +chat-members-list` and `im +messages-search`: - Allow `--page-limit 0` to mean unlimited (validation now accepts 0-1000). - When the page limit is reached while more pages remain, emit a stderr warning pointing to `--page-all --page-limit 0`, instead of stopping silently. - Update the flag description to document `0 = unlimited`. Tests pin the reversed validation contract (0 accepted, -1/1001 rejected), assert the truncation warning is emitted, and verify an unlimited scan reaches every page without falsely warning. Closes larksuite#1859
|
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 (2)
📝 WalkthroughWalkthroughThe ChangesFlag pagination
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant executeListAllPages
participant FlagsAPI
CLI->>executeListAllPages: pass page-limit
executeListAllPages->>FlagsAPI: request next flags page
FlagsAPI-->>executeListAllPages: flags page and has_more
executeListAllPages-->>CLI: results or page-limit warning
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@1c8defcb1c1f3d0ade486aed138ef3a2080e8b7f🧩 Skill updatenpx skills add jacob-qu/cli#fix/flag-list-page-all-truncation -y -g |
|
Hi @jacob-qu, Thanks a lot for submitting this PR, and for the thorough investigation into the I wanted to let you know that this issue has already been fixed and merged into
That fix also links and closes #1859. It now emits a clear stderr warning when pagination stops at Since the issue has been resolved via #1906, we'll be closing this PR. If you find any scenario that #1906 doesn't cover, feel free to reopen it or file a new issue for discussion. Thanks again for your contribution! 🙏 |
Summary
im +flag-list --page-allwas still capped by the default--page-limitof 20 and stopped there without any signal, even whenhas_morewas true. Because the server returns canceled flags before active ones, active flags often live past page 20, so callers sawflag_items: []and read it as "no flags" — misleading for both humans and AI agents. This aligns+flag-listwith the pagination contract already used byim +chat-members-listandim +messages-search.Changes
--page-limit 0to mean unlimited (validation now accepts0–1000;0was previously rejected).--page-all --page-limit 0, instead of stopping silently.0 = unlimited.Test Plan
go test -race ./shortcuts/im/...)lark-cli im +flag-listflow works as expected--page-limit -1/1001→ typed validation error--page-limit 0→ passes validation, dry-run shows the requestgo vet,gofmt -l, andgolangci-lint run --new-from-rev=origin/main(0 issues) all passNew/updated tests pin the reversed validation contract (
0accepted,-1/1001rejected), assert the truncation warning is emitted, and verify an unlimited scan reaches every page without falsely warning.Related Issues
Summary by CodeRabbit
New Features
+flag-listshortcut by setting--page-limit=0.Bug Fixes