Skip to content

MCP: full pages on Connected apps and a progress state on consent - #3570

Merged
integry merged 1 commit into
mainfrom
fix/mcp-apps-pagination-consent-progress
Nov 14, 2026
Merged

integry merged 1 commit into
mainfrom
fix/mcp-apps-pagination-consent-progress

Conversation

@integry

@integry integry commented Nov 14, 2026

Copy link
Copy Markdown
Owner

Problems

  1. Connected apps (/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.
  2. Consent gave no feedback after "Allow selected access". The form posts and then redirects to the app; until the app took over, the page looked exactly as before the click.

Changes

  • loadConnectedAppsPage in packages/api/mcp/browser.ts collects 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.
  • The consent form marks itself busy on submit: the clicked button reads "Allowing…" or "Denying…", both decision buttons are 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 not disabled, because a disabled submitter would drop its decision value from the form data. A page restored from the back/forward cache is reset.
  • Without JavaScript the form behaves as before.

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).
  • API typecheck and lint pass with zero warnings.

🤖 Generated with Claude Code

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>
@integry integry added the AI label Nov 14, 2026
@integry

integry commented Nov 14, 2026

Copy link
Copy Markdown
Owner Author

/review

@propr-dev

propr-dev Bot commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

✅ AI Code Review Complete requested by @integry

Posted 1 review:

View Task Details

@propr-dev

propr-dev Bot commented Nov 14, 2026

Copy link
Copy Markdown
Contributor

🔍 AI Code Review — codex:gpt-6-astra

Overall Evaluation

This 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 aria-disabled preserves the submitter’s decision value, while the submit handler prevents duplicate submissions and resets state on pageshow.

✅ 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 blockers

No merge blockers.

Suggestions

These are optional follow-ups and are not sent to /fix.

S1: 🟢 Exercise scan-limit boundaries

Add optional coverage in packages/api/test/mcpConnectedAppsPage.test.ts for 5,000 inactive records followed by an active grant, and for exactly 5,000 total records. These cases exercise the bounded-scan continuation branch that the current fixtures never reach. Static tracing shows the cursor advances correctly; this would protect that behavior against future changes.

S2: 🟢 Cover consent recovery states

Extend packages/api/test/mcpBrowser.test.ts to assert the delayed status message and pageshow reset, including restored labels and successful resubmission. The current added assertions cover initial approval feedback and duplicate prevention, but not these lifecycle branches. This is optional regression coverage, not a demonstrated defect.

Score

The 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

  • Model: GPT-6 Astra
  • Time: 31s
  • Tokens: 45,600 (45,145 in / 455 out)
  • Cost: $0.37

View Task


💡 Next step: Comment /fix to address every F# merge blocker, /fix all for every pending blocker and suggestion, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them or request /fix all, and they never relax a merge blocker.

Copy and edit to choose records from this review:

/fix S1 S2

🤖 Review by ProPR

@integry
integry merged commit 08f1eff into main Nov 14, 2026
47 checks passed
@integry
integry deleted the fix/mcp-apps-pagination-consent-progress branch November 14, 2026 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants