Repository navigation
MCP: full pages on Connected apps and a progress state on consent - #3570
Conversation
The connected apps page read 50 grant records per page and dropped the revoked and expired ones afterwards, so an account with a long history of reconnects saw one or two apps per page. Active grants are now collected until the page is full. The consent form gave no sign that Allow selected access had been clicked. It now relabels the button, shows a status line and ignores a second submission. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
|
/review |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex:gpt-6-astraOverall EvaluationThis PR is ready to merge within scope. It addresses both reported problems with localized changes and no unnecessary abstractions. Review was based on the supplied diff and context; no commands or tests were run. ✅ Correct pagination continuation — Active grants fill the page, the 51st active grant establishes continuation without being skipped, and the scan-limit cursor advances past processed records. ✅ Preserved consent submission — Using ✅ Focused regression coverage — Tests cover sparse active grants, pagination without gaps or repeats, ownership isolation, and consent submission feedback. The supplied current-head checks report 19 passed and no failures. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Exercise scan-limit boundariesAdd optional coverage in S2: 🟢 Cover consent recovery statesExtend ScoreThe implementation satisfies the stated objective with focused changes, useful tests, and passing current-head checks. Static review found no merge-blocking regressions. Score: 9/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
Problems
/mcp/apps) showed one or two apps per page. The page read 50 grant records, then dropped the revoked and expired ones. Each reconnect of an app leaves its earlier grant stored, so on an account with a long history most of each 50 were hidden and "Next page" led to another near-empty page.Changes
loadConnectedAppsPageinpackages/api/mcp/browser.tscollects active grants until the page holds 50, skipping revoked and expired records while reading (their state is sealed, so it cannot be filtered in SQL). "Next page" appears only when a 51st active app exists. One request reads at most 5,000 grant records; past that the page is returned with a "Next page" link that continues the scan.aria-disabled, a status line with a spinner says "Connecting… You will be returned to the app in a moment.", and a second submit is ignored. After 8 seconds the message changes to say the tab can be closed if the app already shows the result (for apps opened through a custom URL scheme, where the page stays in place). The buttons are notdisabled, because a disabled submitter would drop itsdecisionvalue from the form data. A page restored from the back/forward cache is reset.Tests
packages/api/test/mcpConnectedAppsPage.test.ts(new): six active apps among 180 records are one page; 134 active among 400 list as 50, 50, 34 with no gaps or repeats; exactly 50 active offers no next page; another user's grant is never listed.packages/api/test/mcpBrowser.test.ts: the real consent flow in Chromium asserts the submitted state (label,aria-disabled,aria-busy, status text, second submit prevented).🤖 Generated with Claude Code