Skip to content

fix: validate pagination params in memory tools - #2517

Open
ZayanKhan-12 wants to merge 1 commit into
ChromeDevTools:mainfrom
ZayanKhan-12:fix/memory-pagination-schema
Open

fix: validate pagination params in memory tools#2517
ZayanKhan-12 wants to merge 1 commit into
ChromeDevTools:mainfrom
ZayanKhan-12:fix/memory-pagination-schema

Conversation

@ZayanKhan-12

Copy link
Copy Markdown
Contributor

Summary

The pagination params in src/tools/memory.ts were declared as plain zod.number().optional(), while the equivalent params in console.ts and network.ts use stricter schemas (.int().positive() for pageSize, .int().min(0) for pageIdx).

Because of the laxer schemas, invalid values reached paginate() in src/utils/pagination.ts:

  • pageSize: 0 produces totalPages: Infinity (Math.ceil(total / 0)) with an empty items array
  • negative or fractional pageSize slices nonsensical ranges

This PR aligns all five paginated memory tools (get_heapsnapshot_details, get_heapsnapshot_class_nodes, get_heapsnapshot_retainers, get_heapsnapshot_edges, get_heapsnapshot_duplicate_strings) with the console/network schema pattern, including the describe() wording, so invalid values are rejected at the schema layer with a clear validation error instead of producing broken pagination output.

docs/tool-reference.md and src/bin/chrome-devtools-cli-options.ts are regenerated via npm run gen.

This inconsistency was noticed while writing pagination tests in #2510.

Changes

  • src/tools/memory.ts: pageSizezod.number().int().positive().optional(), pageIdxzod.number().int().min(0).optional() for all paginated memory tools
  • tests/tools/memory.test.ts: new pagination schemas tests asserting that 0, -1, and non-integer values are rejected and valid values still parse
  • docs/tool-reference.md, src/bin/chrome-devtools-cli-options.ts: regenerated

Testing

  • npm run build — passes
  • npm run gen — regenerated docs/CLI options, no unexpected diffs
  • node scripts/test.js tests/tools/memory.test.ts — passes, including the new schema-rejection tests
  • npm run test:no-build — full suite passes (exit 0)
  • npm run check-format — clean

🤖 Generated with Claude Code

@google-cla

google-cla Bot commented Aug 8, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@ZayanKhan-12
ZayanKhan-12 force-pushed the fix/memory-pagination-schema branch from 6a8bc93 to be935fc Compare August 8, 2026 21:46
@OrKoN
OrKoN requested a review from Lightning00Blade August 10, 2026 09:06

@Lightning00Blade Lightning00Blade left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, let's remove the test as we are testing as it test a library not our code.

});
});

describe('pagination schemas', () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test technically tests the zod, so we can remove it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants