Skip to content

fix: reject out-of-range base pagination flags#1495

Merged
zgz2048 merged 1 commit into
larksuite:mainfrom
zgz2048:codex/base-pagination-validation
Jun 17, 2026
Merged

fix: reject out-of-range base pagination flags#1495
zgz2048 merged 1 commit into
larksuite:mainfrom
zgz2048:codex/base-pagination-validation

Conversation

@zgz2048

@zgz2048 zgz2048 commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

Reject out-of-range Base pagination flags instead of silently clamping or defaulting them. This makes --limit and --page-size failures explicit and machine-readable for agent callers.

Changes

  • Validate every Base limit and page-size pagination flag through the typed range helper.
  • Remove the silent ParseIntBounded clamp path and reuse the existing min/max validation message shape.
  • Add Base unit coverage and dry-run E2E coverage for out-of-range pagination errors.

Test Plan

  • Unit tests pass: make unit-test
  • Manual local verification confirms the lark-cli base +table-list --limit 101 --dry-run flow returns a typed validation error via go test ./tests/cli_e2e/base -run 'DryRun|RejectsOutOfRangeLimit' -count=1\n\n## Related Issues\n- None

Summary by CodeRabbit

Release Notes

  • New Features

    • Pagination flag descriptions now clearly show allowed ranges (e.g., "1-100" instead of "max 100")
    • Enhanced validation with more informative error messages for out-of-range pagination parameters
  • Bug Fixes

    • Fixed pagination handling consistency across all listing and search commands
    • Improved input validation to ensure proper parameter constraints are enforced

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR introduces --page-size as a hidden alias for --limit across all base shortcut list/search commands, enforces typed integer range validation via a refactored ValidatePageSizeTyped (using cobra flag lookup), removes the ParseIntBounded helper, replaces ad-hoc string-trimming for page_size request params with runtime.Int(), and adds comprehensive unit and e2e tests.

Changes

Pagination Alias and Typed Validation Rollout

Layer / File(s) Summary
Core pagination helpers and ValidatePageSizeTyped refactor
shortcuts/common/validate.go, shortcuts/base/base_shortcut_helpers.go, shortcuts/common/validate_test.go
ValidatePageSizeTyped switches to cobra flag lookup; ParseIntBounded is removed. Three new helpers added: pageSizeLimitAliasFlag (hidden alias flag definition), getPaginationLimit (resolves effective limit preferring page-size when limit is at default), and validateLimitPageSizeAlias (mutual exclusion check). Unit test updated to cover ValidatePageSizeTyped with int flag inputs.
page-size typed validation for form, history, workflow, and dashboard commands
shortcuts/base/base_form_list.go, shortcuts/base/record_history_list.go, shortcuts/base/workflow_list.go, shortcuts/base/dashboard_list.go, shortcuts/base/dashboard_block_list.go, shortcuts/base/dashboard_ops.go
Adds typed page-size flag definitions with explicit range descriptions, Validate hooks using common.ValidatePageSizeTyped, and removes string-trimming conditionals for page_size request params—now always set via runtime.Int("page-size").
limit/page-size alias wiring for table, field, and view list commands
shortcuts/base/table_list.go, shortcuts/base/table_ops.go, shortcuts/base/field_list.go, shortcuts/base/field_ops.go, shortcuts/base/field_search_options.go, shortcuts/base/view_list.go, shortcuts/base/view_ops.go
Adds pageSizeLimitAliasFlag() to each command's flags, extends Validate functions to call validateLimitPageSizeAlias and ValidatePageSizeTyped for both limit and conditional page-size. Ops files replace ParseIntBounded with getPaginationLimit.
limit/page-size alias wiring for record list and search commands
shortcuts/base/record_list.go, shortcuts/base/record_search.go, shortcuts/base/record_query.go, shortcuts/base/record_ops.go
Adds pageSizeLimitAliasFlag() to BaseRecordList and BaseRecordSearch; extends validation with alias checks and typed bounds. record_query also adds --page-size to --json exclusivity checks. Ops switch to getPaginationLimit.
Unit and e2e tests
shortcuts/base/base_shortcuts_test.go, shortcuts/base/base_dryrun_ops_test.go, tests/cli_e2e/base/base_limit_dryrun_test.go
Adds assertBasePaginationValidation helper and four test functions (help defaults, hidden alias, out-of-range rejection, mutual-exclusion conflict). Dry-run op tests updated to valid limit values and integer page-size. Three new e2e tests cover out-of-range rejection, alias acceptance, and conflict detection for base +table-list.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • larksuite/cli#1242: Introduced common.ValidatePageSizeTyped and the typed validation error infrastructure that this PR refactors and builds upon across all base shortcut commands.

Suggested reviewers

  • liangshuo-1

🐇 A hop for --page-size, a bound for --limit too,
The alias is hidden, but the validation rings true!
Out-of-range values? Rejected with style,
Typed errors and e2e tests stretch for a mile.
Two flags at once? That's a mutual sin—
This rabbit keeps pagination neat, tidy, and trim! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.42% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: rejecting out-of-range base pagination flags instead of silently handling them.
Description check ✅ Passed The description follows the template structure with Summary, Changes, and Test Plan sections completed. It clearly explains the motivation, lists concrete changes, and confirms testing was performed.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added domain/base PR touches the base domain size/L Large or sensitive change across domains or core paths labels Jun 17, 2026
@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.96296% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.43%. Comparing base (714da97) to head (004a343).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
shortcuts/common/validate.go 33.33% 2 Missing and 2 partials ⚠️
shortcuts/base/field_list.go 77.77% 1 Missing and 1 partial ⚠️
shortcuts/base/field_search_options.go 77.77% 1 Missing and 1 partial ⚠️
shortcuts/base/record_list.go 71.42% 1 Missing and 1 partial ⚠️
shortcuts/base/view_list.go 77.77% 1 Missing and 1 partial ⚠️
shortcuts/base/base_form_list.go 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1495      +/-   ##
==========================================
+ Coverage   73.42%   73.43%   +0.01%     
==========================================
  Files         753      753              
  Lines       70111    70182      +71     
==========================================
+ Hits        51476    51540      +64     
- Misses      14826    14829       +3     
- Partials     3809     3813       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zgz2048
zgz2048 force-pushed the codex/base-pagination-validation branch 4 times, most recently from 4c36a08 to 9174e1b Compare June 17, 2026 07:27
@zgz2048
zgz2048 force-pushed the codex/base-pagination-validation branch from 9174e1b to 004a343 Compare June 17, 2026 07:29
@zgz2048
zgz2048 marked this pull request as ready for review June 17, 2026 07:30
@github-actions

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@004a343c6e293786de0a1c73e2b6de38427c2b95

🧩 Skill update

npx skills add zgz2048/cli#codex/base-pagination-validation -y -g

@zgz2048

zgz2048 commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

Test Report: Base Pagination Validation and Hidden Alias

Scope

Validated the current PR build for Base pagination behavior, including:

  • Range validation for --limit
  • Hidden alias --page-size mapping to limit
  • Mutual exclusion between --limit and --page-size
  • Help/default rendering
  • Real API smoke tests against a newly created empty Base

Build Verification

Commands run:

make build
./lark-cli --version
go version -m ./lark-cli

Result:

  • Built binary corresponds to commit 004a343c
  • ./lark-cli --version reported v1.0.50-63-g004a343c
  • go version -m ./lark-cli confirmed the same VCS revision and vcs.modified=false

Live Test Resource

Created a temporary empty Base with +base-create and used it for live read smoke tests.

Redacted identifiers:

  • Base token: JWvX...dnSb
  • Default table: tbleTw6NxkdFq9RZ
  • Text field: fldGvXYA6Q
  • Select field: fldx3uO5rt
  • Default view: vewJU3CR55

Live API Smoke Tests

Commands verified successfully with real API responses (ok: true):

./lark-cli base +table-list --base-token JWvX...dnSb --page-size 40 --as user --format json
./lark-cli base +field-list --base-token JWvX...dnSb --table-id tbleTw6NxkdFq9RZ --page-size 40 --as user --format json
./lark-cli base +field-search-options --base-token JWvX...dnSb --table-id tbleTw6NxkdFq9RZ --field-id fldx3uO5rt --page-size 40 --as user --format json
./lark-cli base +view-list --base-token JWvX...dnSb --table-id tbleTw6NxkdFq9RZ --page-size 40 --as user --format json
./lark-cli base +record-list --base-token JWvX...dnSb --table-id tbleTw6NxkdFq9RZ --page-size 40 --as user --format json
./lark-cli base +record-search --base-token JWvX...dnSb --table-id tbleTw6NxkdFq9RZ --keyword test --search-field fldGvXYA6Q --page-size 40 --as user --format json

Observed behavior:

  • --page-size is accepted by all changed Base shortcut commands.
  • Requests execute successfully against a real empty Base.
  • Returned data matches an empty/default Base: one default table, default fields/views, empty record results.

Help and Default Verification

Subagent verification covered:

./lark-cli base +table-list --help
./lark-cli base +field-list --help
./lark-cli base +field-search-options --help
./lark-cli base +record-list --help
./lark-cli base +record-search --help
./lark-cli base +view-list --help

Result:

Command Visible flag Default shown once Hidden alias shown
+table-list --limit 50 no
+field-list --limit 100 no
+field-search-options --limit 30 no
+record-list --limit 100 no
+record-search --limit 10 no
+view-list --limit 100 no

Dry-Run Alias Verification

Subagent dry-run verification covered all changed commands with --page-size 7.

Result:

  • +table-list: params.limit = 7
  • +field-list: params.limit = 7
  • +field-search-options: params.limit = 7
  • +record-list: URL query contains limit=7
  • +record-search: body contains "limit": 7
  • +view-list: params.limit = 7

No request used page_size for these --limit based commands.

Validation Behavior

Verified error cases:

./lark-cli base +table-list --base-token JWvX...dnSb --limit 101 --as user --format json
./lark-cli base +table-list --base-token JWvX...dnSb --page-size 101 --as user --format json
./lark-cli base +table-list --base-token JWvX...dnSb --limit 20 --page-size 40 --as user --format json

Results:

  • Out-of-range --limit returns typed validation error with param=--limit.
  • Out-of-range --page-size returns typed validation error with param=--page-size.
  • Passing both --limit and --page-size returns typed validation error with param=--page-size and a mutually-exclusive message.

Subagent also verified +record-search equivalents, including:

  • --limit 201 -> param=--limit
  • --page-size 201 -> param=--page-size
  • --limit plus --page-size -> param=--page-size
  • --json plus --page-size -> existing --json mutual-exclusion validation

Automated Tests

Commands run:

go test ./shortcuts/base ./shortcuts/common ./tests/cli_e2e/base -run 'TestBase(Pagination|Limit|ListDryRun)|TestDryRun(Table|Field|Record|View)Ops' -count=1

Result: passed.

Cleanup Note

Attempted to delete the temporary Base via Drive delete with both user and bot identities, but the current auth/app lacks delete scopes:

  • user missing space:document:delete
  • bot missing drive:drive and space:document:delete

The test Base token is intentionally redacted in this report.

@zgz2048
zgz2048 merged commit 7eeb111 into larksuite:main Jun 17, 2026
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/base PR touches the base domain size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants