Skip to content

fix(frontend): suppress tool-quarantine banner for baseline pending tools (Spec 032, MCP-2081)#642

Closed
Dumbris wants to merge 1 commit into
mainfrom
fix/mcp-2081-tool-quarantine-banner
Closed

fix(frontend): suppress tool-quarantine banner for baseline pending tools (Spec 032, MCP-2081)#642
Dumbris wants to merge 1 commit into
mainfrom
fix/mcp-2081-tool-quarantine-banner

Conversation

@Dumbris

@Dumbris Dumbris commented Jun 12, 2026

Copy link
Copy Markdown
Member

Summary

Frontend half of MCP-2081 (Spec 032 tool quarantine). The tool-quarantine banner + per-tool Approve UI in ServerDetail.vue keyed off any non-approved tool (pending OR changed), so:

  1. freshly-discovered baseline pending tools raised a false "N tool(s) require approval" alarm, and
  2. the tool-level banner competed with the server-level Security Quarantine banner.

Per the confirmed trust model, baseline pending tools are not a rug-pull — the backend promotes them to approved when the operator approves the server. Only changed tools (description / input-schema / output-schema hash drift) legitimately need per-tool attention.

Changes

  • Extract the banner-selection decision into a pure, testable helper selectQuarantinedTools(approvals, serverQuarantined) in frontend/src/utils/toolQuarantine.ts.
  • Surface the banner only when a changed tool exists; include residual pending tools alongside it for batch approval; never surface baseline pending alone.
  • Suppress the tool-quarantine banner entirely while the server-level Security Quarantine banner is showing (server.quarantined).
  • Unit test the three states.

Verification

  • vitest — full suite 152/152 pass, including the new tool-quarantine-banner.spec.ts (5 cases: baseline pending → no banner; changed → banner; changed+pending → batch; server quarantined → suppressed; empty/all-approved → none).
  • npm run build (vue-tsc type-check) — pass.

The rendered banner-state Playwright sweep (server quarantined → one banner; after approve → no tool banner; simulate changed → tool banner) is left to the pre-merge QA gate, since seeding a changed/rug-pull tool needs the rugpull test server.

Related: MCP-2081 · pairs with the backend baseline auto-approve child and the merged diff-UI work (#639).

…ools (Spec 032, MCP-2081)

The tool-quarantine banner + per-tool Approve UI keyed off any non-approved
tool (pending OR changed), so freshly-discovered baseline `pending` tools
raised a false "N tool(s) require approval" alarm, and it competed with the
server-level Security Quarantine banner.

Per the confirmed trust model, baseline `pending` tools are not a rug-pull —
the backend promotes them to `approved` when the operator approves the server.
The only tools that legitimately need per-tool attention are `changed`
(rug-pull) tools.

- Extract the banner-selection decision into a pure, testable helper
  `selectQuarantinedTools(approvals, serverQuarantined)` in utils/toolQuarantine.ts.
- Surface the banner only when a `changed` tool exists; include residual
  `pending` tools alongside it for batch approval; never surface baseline
  pending alone.
- Suppress the tool-quarantine banner entirely while the server-level Security
  Quarantine banner is showing (server.quarantined).
- Unit test the three states (baseline pending → no banner; changed → banner;
  server quarantined → suppressed) — 5 cases.

Verified: vitest (full suite 152/152, incl. new spec), npm run build (vue-tsc).
The rendered banner-state Playwright sweep is left to the pre-merge QA gate
(seeding a `changed`/rug-pull tool needs the rugpull test server).
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 12, 2026

Copy link
Copy Markdown

Deploying mcpproxy-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 61cf3f0
Status: ✅  Deploy successful!
Preview URL: https://cd405404.mcpproxy-docs.pages.dev
Branch Preview URL: https://fix-mcp-2081-tool-quarantine.mcpproxy-docs.pages.dev

View logs

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@github-actions

Copy link
Copy Markdown

📦 Build Artifacts

Workflow Run: View Run
Branch: fix/mcp-2081-tool-quarantine-banner

Available Artifacts

  • archive-darwin-amd64 (28 MB)
  • archive-darwin-arm64 (25 MB)
  • archive-linux-amd64 (16 MB)
  • archive-linux-arm64 (14 MB)
  • archive-windows-amd64 (28 MB)
  • archive-windows-arm64 (24 MB)
  • frontend-dist-pr (0 MB)
  • installer-dmg-darwin-amd64 (21 MB)
  • installer-dmg-darwin-arm64 (19 MB)

How to Download

Option 1: GitHub Web UI (easiest)

  1. Go to the workflow run page linked above
  2. Scroll to the bottom "Artifacts" section
  3. Click on the artifact you want to download

Option 2: GitHub CLI

gh run download 27402323582 --repo smart-mcp-proxy/mcpproxy-go

Note: Artifacts expire in 14 days.

@Dumbris

Dumbris commented Jun 14, 2026

Copy link
Copy Markdown
Member Author

Closing as superseded by merged PR #641 (a98ab6a3f64dc3f14d9b199604629520834ad2a2, merged 2026-06-12).

Both PRs implement the same change — suppress the per-server Tool-Quarantine banner for baseline pending tools (Spec 032). #641 (MCP-2101, child of MCP-2081) already landed that work on main:

  • frontend/src/utils/toolQuarantine.ts:21selectQuarantinedTools() present on main.
  • frontend/tests/unit/tool-quarantine-banner.spec.ts — unit test present on main.
  • frontend/src/views/ServerDetail.vue:1306 — banner-selection wiring present on main.

This branch (fix/mcp-2081-tool-quarantine-banner, head 61cf3f04) is stale against current origin/main: GitHub reports mergeable=CONFLICTING / DIRTY, and its diff vs main is almost entirely deletions of code main already has (including data-test banner attributes and the scanReportPath helper). selectQuarantinedTools is functionally identical to main's version. Rebasing would produce a no-op/empty PR, so closing is the correct resolution.

Resolves the CodexReviewer change request on this PR. Related #641, MCP-2081.

@Dumbris Dumbris closed this Jun 14, 2026
@Dumbris

Dumbris commented Jun 14, 2026

Copy link
Copy Markdown
Member Author

Closing as superseded by #641 (a98ab6a3, already merged to main).

While this PR was open and conflicting, the overlapping PR #641 ("key tool-quarantine banner off changed, suppress for baseline pending", MCP-2101) landed the identical fix. Both PRs:

  • add frontend/src/utils/toolQuarantine.ts with the same selectQuarantinedTools(toolApprovals, serverQuarantined) helper and functionally identical rules — suppress while server.quarantined; surface only when a changed (rug-pull) tool exists; then include residual pending for batch approval;
  • rewire ServerDetail.vue's quarantinedTools computed onto that helper.

main's version (#641) is a strict superset of this PR: it additionally adds the data-test Playwright hooks (security-quarantine-banner, tool-quarantine-banner, tool-quarantine-list), the scanReportPath() report-link encode, docs/features/tool-quarantine.md, and a rendered Playwright verification report. The only thing unique to this PR is unused null-safety gold-plating in the helper signature (the call site never passes null).

A rebase of this branch onto origin/main therefore resolves to an empty/no-op diff. Closing rather than landing a redundant PR. The MCP-2081 intent is satisfied on main.

@Dumbris Dumbris deleted the fix/mcp-2081-tool-quarantine-banner branch June 14, 2026 03:36
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.

2 participants