fix: Admin UI pagination proxy compatibility and fix for count accuracy by madhav165 · Pull Request #2846 · IBM/mcp-context-forge

madhav165 · 2026-02-11T13:36:49Z

🐛 Bug-fix PR

Fixes #2845

Summary

Fix Alpine.js x-data attribute corruption when a reverse proxy rewrites URLs in the response body by moving base_url from inline JavaScript to a data-* attribute
Use request.scope["root_path"] instead of hardcoded settings.app_root_path for HTMX partial endpoint URLs so proxy path rewriting is respected
Adjust pagination total_items when DB rows fail Pydantic conversion so "Showing X of Y" counts match actual displayed items
Clamp page to total_pages in offset_paginate to avoid empty results when a page number exceeds available pages
Hide page navigation buttons when totalPages is 0 and show "No items found" for empty result sets
Fix 6 timezone-sensitive unit tests that used datetime.now() (local time) instead of UTC-based naive datetimes

Changes

`mcpgateway/templates/pagination_controls.html`

Move {{ base_url }} from inside the x-data JavaScript expression to a data-base-url HTML attribute on the outer <div>. The Alpine init() method reads it via this.baseUrl = $el.dataset.baseUrl. This prevents proxy URL rewriting from corrupting the x-data="..." attribute boundary.
Replace new URL('{{ base_url }}', ...) in loadPage() with new URL(this.baseUrl, ...).
Wrap page navigation buttons in <template x-if="totalPages > 0"> so they are hidden for empty results.
Show "No items found" when totalItems === 0 instead of "Showing 1 - 0 of 0 items".
Minor formatting/whitespace normalization (single-quote to double-quote in <script> block).

`mcpgateway/admin.py`

Add _adjust_pagination_for_conversion_failures() helper that decrements pagination.total_items by the number of rows that fail DB-to-Pydantic conversion.
Apply this helper in all 6 admin partial endpoints: servers, tools, prompts, gateways, resources, a2a agents.
Replace settings.app_root_path with request.scope.get("root_path", "") when constructing base_url in all partial endpoints (servers, tools, prompts, gateways, resources, a2a agents, users).

`mcpgateway/utils/pagination.py`

Clamp page to total_pages when total_pages > 0 to prevent out-of-range offsets returning empty results.

`tests/unit/mcpgateway/utils/test_pagination.py`

Add 3 tests for page clamping: page beyond total, zero items, and exact last page.

`tests/unit/mcpgateway/services/test_token_storage_service.py`

Fix test_is_token_expired_naive_datetime: use datetime.now(timezone.utc).replace(tzinfo=None) instead of datetime.now() to create a naive datetime that represents UTC.

`tests/unit/mcpgateway/routers/test_log_search.py`

Fix test_aggregate_custom_windows_adds_timezone_to_naive_earliest_log: same datetime.now() to UTC-based naive datetime fix.

`tests/unit/mcpgateway/test_db.py`

Fix 4 tests (test_email_team_invitation_is_expired_handles_timezone_mismatch, test_email_team_join_request_is_expired_handles_timezone_mismatch_now_naive, test_pending_user_approval_is_expired_handles_timezone_mismatch_now_naive, test_sso_auth_session_is_expired_handles_timezone_mismatch_now_naive): monkeypatched utc_now lambdas now return datetime.now(timezone.utc).replace(tzinfo=None, microsecond=0) instead of datetime.now().replace(microsecond=0).

`uv.lock`

Add pytest-instafail dev dependency.

🧪 Verification

Check	Command	Status
Lint suite	`make lint`	pass
Unit tests	`make test`	pass

📐 MCP Compliance (if relevant)

Matches current MCP spec
No breaking change to MCP clients

✅ Checklist

Code formatted (make black isort pre-commit)
No secrets/credentials committed

Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>

crivetimihai · 2026-02-11T15:59:33Z

Review

Rebased onto main (clean, no conflicts). All 15 new/modified tests pass.

Changes look good

Proxy compatibility: Moving base_url from inline JS in x-data to a data-base-url HTML attribute is the correct approach to prevent reverse proxy URL rewriting from corrupting Alpine.js initialization.
Dynamic root_path: Using request.scope.get("root_path", "") instead of settings.app_root_path is the proper ASGI mechanism — consistently applied across all 7 partial endpoints.
Pagination accuracy: _adjust_pagination_for_conversion_failures helper and page clamping logic are correct. Empty-state UX ("No items found", hidden nav buttons) is a good improvement.
Timezone test fixes: datetime.now(timezone.utc).replace(tzinfo=None) is the correct pattern for UTC-based naive datetimes — fixes flaky tests in non-UTC environments.
Security: No issues. The data-base-url attribute approach is actually more secure than the previous JS-in-attribute context (avoids double-context escaping risks). root_path is ASGI-server controlled, not user-input.
Performance: No impact.

Minor issues (non-blocking)

_adjust_pagination_for_conversion_failures only adjusts total_items, not total_pages/has_next/has_prev. Page navigation could show extra pages if many items fail conversion. Acceptable since conversion failures are rare edge cases.
Test name mismatch: test_offset_paginate_page_stays_1_when_no_items asserts page == 5, not page == 1. The test logic is correct (no clamping when total_pages=0), but the name is misleading.
Remaining settings.app_root_path uses in admin.py (out of scope for this PR, but worth a follow-up):
- Lines 2799, 2920: get_overview_partial() and admin_login_required()
- Line 6838: admin_tool_ops_partial() pagination base_url
- Lines 12011, 12084: export functions
- Line 13909: HTMX hx-post attribute in Python string
admin_tool_ops_partial() (line ~6838) has no try/except in its conversion loop (uses a bare list comprehension), unlike the other 6 endpoints updated in this PR. Could crash on corrupt data.

- Recompute total_pages, has_next, has_prev, and page in _adjust_pagination_for_conversion_failures to avoid stale UI state - Fix test_auth_cache_import_error: patch the already-imported function reference instead of sys.modules (module-level import was already resolved) - Fix test_redis_available_cache_hit: patch get_redis_client in the consuming module, not the source module - Rename misleading test to test_offset_paginate_page_not_clamped_when_no_items - Add tests for derived pagination field recomputation Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

- Remove page clamping from _adjust_pagination_for_conversion_failures since data was already fetched for the original page; clamping would make metadata claim a different page than what is displayed - Use request.scope root_path for tool-ops partial endpoint base_url instead of settings.app_root_path, consistent with all other partials Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

crivetimihai · 2026-02-11T16:29:06Z

Additional fixes pushed

Two follow-up commits added after review:

Commit 4: `fix: complete pagination adjustment and fix broken mock targets`

Pagination adjustment — _adjust_pagination_for_conversion_failures previously only mutated total_items, leaving total_pages, has_next, and has_prev stale. Now recomputes all derived fields so the UI stays internally consistent.

Broken test mocks (pre-existing on main, not from this PR):

test_auth_cache_import_error — patched sys.modules["mcpgateway.cache.auth_cache"] but get_auth_cache was already imported at module level (line 33 of team_management_service.py). sys.modules patching only blocks future imports, not already-resolved references. Fixed to patch mcpgateway.services.team_management_service.get_auth_cache directly.
test_redis_available_cache_hit — patched mcpgateway.utils.redis_client.get_redis_client but the code uses from ... import get_redis_client, so the local reference is unaffected. Fixed to patch mcpgateway.services.team_management_service.get_redis_client.

Test improvements:

Renamed test_offset_paginate_page_stays_1_when_no_items → test_offset_paginate_page_not_clamped_when_no_items (test asserts page == 5, not page == 1)
Added assertions for derived pagination fields (total_pages, has_next, has_prev)
Added 2 boundary tests: test_recomputes_has_next_on_boundary and test_page_not_clamped_when_total_pages_shrinks

Commit 5: `fix: do not clamp page after conversion and use root_path for tool-ops`

Page clamping removed from _adjust_pagination_for_conversion_failures — Data is already fetched for the original page before conversion happens. Clamping page afterward would make metadata claim a different page than what's displayed (e.g., metadata says "page 4" but showing data from offset 80 which is page 5). total_items, total_pages, has_next, has_prev are still recomputed.

tool-ops endpoint — admin_tool_ops_partial() (line 6843) still used settings.app_root_path for its pagination base_url. Changed to request.scope.get('root_path', '') for consistency with all other partial endpoints.

All 867 affected tests pass.

…cy (#2846) * fix baseUrl Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * fix x-data base-url Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * add new tests Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * fix: complete pagination adjustment and fix broken mock targets - Recompute total_pages, has_next, has_prev, and page in _adjust_pagination_for_conversion_failures to avoid stale UI state - Fix test_auth_cache_import_error: patch the already-imported function reference instead of sys.modules (module-level import was already resolved) - Fix test_redis_available_cache_hit: patch get_redis_client in the consuming module, not the source module - Rename misleading test to test_offset_paginate_page_not_clamped_when_no_items - Add tests for derived pagination field recomputation Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: do not clamp page after conversion and use root_path for tool-ops - Remove page clamping from _adjust_pagination_for_conversion_failures since data was already fetched for the original page; clamping would make metadata claim a different page than what is displayed - Use request.scope root_path for tool-ops partial endpoint base_url instead of settings.app_root_path, consistent with all other partials Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>

…cy (#2846) * fix baseUrl Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * fix x-data base-url Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * add new tests Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * fix: complete pagination adjustment and fix broken mock targets - Recompute total_pages, has_next, has_prev, and page in _adjust_pagination_for_conversion_failures to avoid stale UI state - Fix test_auth_cache_import_error: patch the already-imported function reference instead of sys.modules (module-level import was already resolved) - Fix test_redis_available_cache_hit: patch get_redis_client in the consuming module, not the source module - Rename misleading test to test_offset_paginate_page_not_clamped_when_no_items - Add tests for derived pagination field recomputation Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: do not clamp page after conversion and use root_path for tool-ops - Remove page clamping from _adjust_pagination_for_conversion_failures since data was already fetched for the original page; clamping would make metadata claim a different page than what is displayed - Use request.scope root_path for tool-ops partial endpoint base_url instead of settings.app_root_path, consistent with all other partials Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>

…cy (IBM#2846) * fix baseUrl Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * fix x-data base-url Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * add new tests Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * fix: complete pagination adjustment and fix broken mock targets - Recompute total_pages, has_next, has_prev, and page in _adjust_pagination_for_conversion_failures to avoid stale UI state - Fix test_auth_cache_import_error: patch the already-imported function reference instead of sys.modules (module-level import was already resolved) - Fix test_redis_available_cache_hit: patch get_redis_client in the consuming module, not the source module - Rename misleading test to test_offset_paginate_page_not_clamped_when_no_items - Add tests for derived pagination field recomputation Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: do not clamp page after conversion and use root_path for tool-ops - Remove page clamping from _adjust_pagination_for_conversion_failures since data was already fetched for the original page; clamping would make metadata claim a different page than what is displayed - Use request.scope root_path for tool-ops partial endpoint base_url instead of settings.app_root_path, consistent with all other partials Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>

Fixes #2851 When items fail Pydantic validation during admin list views, the pagination 'Showing X - Y of Z' display was incorrect. The server adjusted total_items (Z) but the X-Y range was computed client-side using requested items, not actual rendered count. Changes: - Add optional page_items field to PaginationMeta schema - Update _adjust_pagination_for_conversion_failures() to set page_items - Update 6 admin partial endpoints to pass rendered item count - Update pagination_controls.html to use page_items when available - Update tests to match new function signature Example: 20 items requested, 2 fail conversion - Before: 'Showing 1 - 20 of 98 items' (18 rows displayed) - After: 'Showing 1 - 18 of 98 items' (18 rows displayed) Builds on PR #2846 which introduced _adjust_pagination_for_conversion_failures() Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com>

…3238) Fixes #2851 When items fail Pydantic validation during admin list views, the pagination 'Showing X - Y of Z' display was incorrect. The server adjusted total_items (Z) but the X-Y range was computed client-side using requested items, not actual rendered count. Changes: - Add optional page_items field to PaginationMeta schema - Update _adjust_pagination_for_conversion_failures() to set page_items - Update 6 admin partial endpoints to pass rendered item count - Update pagination_controls.html to use page_items when available - Update tests to match new function signature Example: 20 items requested, 2 fail conversion - Before: 'Showing 1 - 20 of 98 items' (18 rows displayed) - After: 'Showing 1 - 18 of 98 items' (18 rows displayed) Builds on PR #2846 which introduced _adjust_pagination_for_conversion_failures() Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com>

…3238) Fixes #2851 When items fail Pydantic validation during admin list views, the pagination 'Showing X - Y of Z' display was incorrect. The server adjusted total_items (Z) but the X-Y range was computed client-side using requested items, not actual rendered count. Changes: - Add optional page_items field to PaginationMeta schema - Update _adjust_pagination_for_conversion_failures() to set page_items - Update 6 admin partial endpoints to pass rendered item count - Update pagination_controls.html to use page_items when available - Update tests to match new function signature Example: 20 items requested, 2 fail conversion - Before: 'Showing 1 - 20 of 98 items' (18 rows displayed) - After: 'Showing 1 - 18 of 98 items' (18 rows displayed) Builds on PR #2846 which introduced _adjust_pagination_for_conversion_failures() Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com> Signed-off-by: Yosief Eyob <yosiefogbazion@gmail.com>

madhav165 requested review from crivetimihai and kevalmahajan as code owners February 11, 2026 13:36

madhav165 added bug Something isn't working ica ICA related issues labels Feb 11, 2026

madhav165 added 3 commits February 11, 2026 13:59

fix baseUrl

fd38de6

Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>

fix x-data base-url

33d153e

Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>

add new tests

3d96b39

Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>

madhav165 changed the title ~~fix: Admin UI pagination proxy compatibility, count accuracy, and timezone test fixes~~ fix: Admin UI pagination proxy compatibility and fix for count accuracy Feb 11, 2026

madhav165 force-pushed the fix-x-data branch from d16240e to d022ddc Compare February 11, 2026 14:45

crivetimihai force-pushed the fix-x-data branch from d2a04c5 to 3d96b39 Compare February 11, 2026 15:59

crivetimihai added 2 commits February 11, 2026 16:15

crivetimihai mentioned this pull request Feb 11, 2026

[BUG]: Admin UI "Showing X - Y of Z" count incorrect when DB-to-Pydantic conversion fails #2851

Closed

1 task

crivetimihai approved these changes Feb 11, 2026

View reviewed changes

crivetimihai merged commit 054d2b3 into main Feb 11, 2026
52 checks passed

crivetimihai deleted the fix-x-data branch February 11, 2026 16:54

suciu-daniel mentioned this pull request Feb 25, 2026

[FIX][UI]: Correct pagination display when DB-to-Pydantic conversion fails (#2851) #3238

Merged

crivetimihai self-assigned this Feb 26, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Admin UI pagination proxy compatibility and fix for count accuracy#2846

fix: Admin UI pagination proxy compatibility and fix for count accuracy#2846
crivetimihai merged 5 commits intomainfrom
fix-x-data

madhav165 commented Feb 11, 2026 •

edited

Loading

Uh oh!

crivetimihai commented Feb 11, 2026

Uh oh!

crivetimihai commented Feb 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

madhav165 commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🐛 Bug-fix PR

Summary

Changes

mcpgateway/templates/pagination_controls.html

mcpgateway/admin.py

mcpgateway/utils/pagination.py

tests/unit/mcpgateway/utils/test_pagination.py

tests/unit/mcpgateway/services/test_token_storage_service.py

tests/unit/mcpgateway/routers/test_log_search.py

tests/unit/mcpgateway/test_db.py

uv.lock

🧪 Verification

📐 MCP Compliance (if relevant)

✅ Checklist

Uh oh!

crivetimihai commented Feb 11, 2026

Review

Changes look good

Minor issues (non-blocking)

Uh oh!

crivetimihai commented Feb 11, 2026

Additional fixes pushed

Commit 4: fix: complete pagination adjustment and fix broken mock targets

Commit 5: fix: do not clamp page after conversion and use root_path for tool-ops

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

madhav165 commented Feb 11, 2026 •

edited

Loading

`mcpgateway/templates/pagination_controls.html`

`mcpgateway/admin.py`

`mcpgateway/utils/pagination.py`

`tests/unit/mcpgateway/utils/test_pagination.py`

`tests/unit/mcpgateway/services/test_token_storage_service.py`

`tests/unit/mcpgateway/routers/test_log_search.py`

`tests/unit/mcpgateway/test_db.py`

`uv.lock`

Commit 4: `fix: complete pagination adjustment and fix broken mock targets`

Commit 5: `fix: do not clamp page after conversion and use root_path for tool-ops`