Fix/v4 reviews#6
Conversation
- Item 2: Add explicit `filter` param to list_mcp_servers()
- Item 3: deploy_studio() no longer sends empty {} body
- Item 4: MCP list_repos passes through requested page/size metadata
- Item 5: stop_studio() explicitly sends no body
- Item 6: get_studio() now requires auth (removed require_token=False)
- Item 8: Default page_size changed from 20 to 10 (matches spec)
- Item 10: Idempotent POST paths (deploy/stop/undeploy) now retried
- Section III: Client-side pagination limit validation (<=3000, <=100 for MCP)
- Add 26 unit tests covering all changes
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add run_all.py for unified integration test execution (--quick, --dry-run) - Add list_repos facade tests: all repo types, legacy/openapi paths, pagination - Add MCP server facade tests: list, search, get - Add OpenAPI client tests: MCP filter, skills search, pagination defaults - Add pagination edge cases: boundary, empty pages, consistency - Total: 73 integration tests (up from 45) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- _legacy_api.py: catch requests.exceptions.RetryError → ServerError (prevents raw urllib3 exception from leaking to callers) - test_list_datasets: add owner param to avoid unstable unfiltered query - test_large_page_number: reduce to 200*10=2000 (within 3000 limit) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…catalog Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several updates to the ModelScope Hub API client, including retry logic for semantically idempotent POST endpoints, updated pagination limits and defaults (reducing default page size from 20 to 10), token requirements for fetching studio metadata, and support for nested filters in MCP server discovery. It also adds comprehensive unit and integration tests. The review feedback highlights a potential bypass in the pagination limit checks where negative or zero values for page_number or page_size could circumvent the validation, and suggests explicitly ensuring both parameters are positive integers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if page_number * page_size > 3000: | ||
| raise InvalidParameter( | ||
| f"page_number * page_size must be <= 3000 (got {page_number * page_size})." | ||
| ) |
There was a problem hiding this comment.
The pagination parameters page_number and page_size are multiplied to check if they exceed the limit of 3000. However, if either parameter is negative or zero, the check page_number * page_size > 3000 can be bypassed (e.g., -1 * 10 = -10 <= 3000). To prevent sending invalid pagination values to the backend, we should explicitly validate that both page_number and page_size are positive integers (greater than or equal to 1).
if page_number < 1 or page_size < 1:
raise InvalidParameter("page_number and page_size must be positive integers.")
if page_number * page_size > 3000:
raise InvalidParameter(
f"page_number * page_size must be <= 3000 (got {page_number * page_size})."
)| if page_number * page_size > 3000: | ||
| raise InvalidParameter( | ||
| f"page_number * page_size must be <= 3000 (got {page_number * page_size})." | ||
| ) |
There was a problem hiding this comment.
The pagination parameters page_number and page_size are multiplied to check if they exceed the limit of 3000. However, if either parameter is negative or zero, the check page_number * page_size > 3000 can be bypassed (e.g., -1 * 10 = -10 <= 3000). To prevent sending invalid pagination values to the backend, we should explicitly validate that both page_number and page_size are positive integers (greater than or equal to 1).
if page_number < 1 or page_size < 1:
raise InvalidParameter("page_number and page_size must be positive integers.")
if page_number * page_size > 3000:
raise InvalidParameter(
f"page_number * page_size must be <= 3000 (got {page_number * page_size})."
)| if page_number * page_size > 3000: | ||
| raise InvalidParameter( | ||
| f"page_number * page_size must be <= 3000 (got {page_number * page_size})." | ||
| ) |
There was a problem hiding this comment.
The pagination parameters page_number and page_size are multiplied to check if they exceed the limit of 3000. However, if either parameter is negative or zero, the check page_number * page_size > 3000 can be bypassed (e.g., -1 * 10 = -10 <= 3000). To prevent sending invalid pagination values to the backend, we should explicitly validate that both page_number and page_size are positive integers (greater than or equal to 1).
if page_number < 1 or page_size < 1:
raise InvalidParameter("page_number and page_size must be positive integers.")
if page_number * page_size > 3000:
raise InvalidParameter(
f"page_number * page_size must be <= 3000 (got {page_number * page_size})."
)| if page_number * page_size > 100: | ||
| raise InvalidParameter( | ||
| f"page_number * page_size must be <= 100 for MCP servers (got {page_number * page_size})." | ||
| ) |
There was a problem hiding this comment.
The pagination parameters page_number and page_size are multiplied to check if they exceed the limit of 100. However, if either parameter is negative or zero, the check page_number * page_size > 100 can be bypassed (e.g., -1 * 10 = -10 <= 100). To prevent sending invalid pagination values to the backend, we should explicitly validate that both page_number and page_size are positive integers (greater than or equal to 1).
if page_number < 1 or page_size < 1:
raise InvalidParameter("page_number and page_size must be positive integers.")
if page_number * page_size > 100:
raise InvalidParameter(
f"page_number * page_size must be <= 100 for MCP servers (got {page_number * page_size})."
)1 participant
No description provided.