fix(webui): surface tool-level quarantine on the Servers list#5
Merged
electrolobzik merged 1 commit intoMay 23, 2026
Merged
Conversation
The Servers grid only renders the big orange "Server is quarantined" banner + Approve button when `server.quarantined === true`. Servers that are trusted at the server level but ship tools blocked by Spec 032 tool-level quarantine (pending / changed) had only a tiny `stat-desc` under the tool count — easy to miss, and shadowed entirely by the `X disabled` line when any tool was also disabled, because both lines shared a `v-if / v-else-if` chain. The asymmetry is confusing: the detail page DOES escalate tool-level quarantine to a yellow alert panel with an Approve-All button, so users who hit the "tool requires approval" error from the daemon find clear remediation on Details but nothing on the list view. They report the list as broken or inconsistent. This commit: * Adds a second `alert-warning` banner (`v-else-if="quarantineToolCount > 0"`) with a Review link that routes to `/servers/<name>?tab=tools`. Server and tool quarantine remain mutually exclusive on the card — the server-level banner already implies the tools won't run, so we don't stack two warnings. * Splits the stat-desc `v-if / v-else-if` chain so `X disabled` and `N pending approval` can render side-by-side. Token-size hint stays the "nothing wrong" fallback. * Computes a context-aware summary string: - "All N tools pending security approval" when every tool is pending - "N of M tools pending security approval" when partial - "N tools changed since approval — re-review needed" for rug-pull - mixed pending+changed when both apply No backend change: the `quarantine.pending_count` / `changed_count` / `blocked_count` block is already populated by `enrichServersWithQuarantineStats` (REST + SSE). Extends the existing ServerCard test with five new cases covering the partial / full / changed / mixed-with-server-level / mixed-with-disabled scenarios.
📝 WalkthroughWalkthroughServerCard component updates its quarantine UI from a single server-level alert to a two-tier system: server quarantine takes precedence with its own alert, otherwise tools show a quarantine banner with computed summary text and a Review link to the Tools tab. ChangesTwo-Tier Quarantine UI System
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/ServerCard.vue`:
- Around line 125-136: The quarantine banner and its Review link are missing
Playwright data-test attributes; add them to the root banner div (the v-else-if
block rendering quarantineToolCount > 0), the summary span that displays
toolQuarantineSummary, and the router-link that navigates to
`/servers/${server.name}?tab=tools` (use clear identifiers like
data-test="tool-quarantine-banner", data-test="tool-quarantine-summary" and
data-test="tool-quarantine-review-link") so E2E tests can reliably target these
elements in ServerCard.vue.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5d355b96-cf2d-4cc9-97c0-d7ab3b965f06
📒 Files selected for processing (2)
frontend/src/components/ServerCard.vuefrontend/src/components/__tests__/ServerCard.test.ts
Comment on lines
+125
to
+136
| <div v-else-if="quarantineToolCount > 0" class="alert alert-warning alert-sm mb-4"> | ||
| <svg class="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | ||
| <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 9v2m0 4h.01m-6.938 4h13.856c1.54 0 2.502-1.667 1.732-2.5L13.732 4c-.77-.833-1.732-.833-2.5 0L3.732 16.5c-.77.833.192 2.5 1.732 2.5z" /> | ||
| </svg> | ||
| <span class="text-xs flex-1">{{ toolQuarantineSummary }}</span> | ||
| <router-link | ||
| :to="`/servers/${server.name}?tab=tools`" | ||
| class="btn btn-xs btn-warning" | ||
| > | ||
| Review | ||
| </router-link> | ||
| </div> |
There was a problem hiding this comment.
Add data-test attributes for Playwright E2E testing.
The new tool-level quarantine banner and Review link are missing data-test attributes. As per coding guidelines, Vue components in frontend/src/**/*.vue should use data-test attributes for E2E testing with Playwright.
Suggested additions
- <div v-else-if="quarantineToolCount > 0" class="alert alert-warning alert-sm mb-4">
+ <div v-else-if="quarantineToolCount > 0" class="alert alert-warning alert-sm mb-4" data-test="tool-quarantine-banner">
<svg class="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24">
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 9v2m0 4h.01m-6.938 4h13.856c1.54 0 2.502-1.667 1.732-2.5L13.732 4c-.77-.833-1.732-.833-2.5 0L3.732 16.5c-.77.833.192 2.5 1.732 2.5z" />
</svg>
<span class="text-xs flex-1">{{ toolQuarantineSummary }}</span>
<router-link
:to="`/servers/${server.name}?tab=tools`"
- class="btn btn-xs btn-warning"
+ class="btn btn-xs btn-warning"
+ data-test="tool-quarantine-review-link"
>
Review
</router-link>
</div>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/ServerCard.vue` around lines 125 - 136, The
quarantine banner and its Review link are missing Playwright data-test
attributes; add them to the root banner div (the v-else-if block rendering
quarantineToolCount > 0), the summary span that displays toolQuarantineSummary,
and the router-link that navigates to `/servers/${server.name}?tab=tools` (use
clear identifiers like data-test="tool-quarantine-banner",
data-test="tool-quarantine-summary" and data-test="tool-quarantine-review-link")
so E2E tests can reliably target these elements in ServerCard.vue.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-pick of the upstream PR onto
halo-mainso the Halo build picks up the fix without waiting on upstream review.Upstream PR: smart-mcp-proxy#519
Problem
Servers added by
setup.share registered with--no-quarantineand then have their tools auto-approved in a post-add poll. For OAuth-gated servers (atlassian), lazy-spawn servers, or anything that exceeds the ~90s approval-poll timeout, the server lands withquarantined=falsebutquarantine.pending_count > 0. The Servers list view treats that as a minor footnote: a tinystat-descunder the tool count, hidden entirely behindX disabledwhenever any tool is also disabled.Meanwhile the detail page does show a yellow alert + Approve-All for the same state, so users hitting the daemon's
quarantine pending approvalerror find clear remediation on Details but nothing on the list. They report the list as broken / inconsistent.Change
alert-warningbanner onServerCard.vuefor tool-level quarantine, with aReviewrouter-link to/servers/<name>?tab=tools. Mutually exclusive with the existing server-level banner.v-if / v-else-ifchain soX disabledandN pending approvalcan render side-by-side.ServerCard.test.tswith five new cases covering each path.No backend change;
quarantine.{pending_count,changed_count,blocked_count}is already on the REST + SSE response (seeenrichServersWithQuarantineStats).Test plan
@guolao/vue-monaco-editorpreventednpm install).--no-quarantinewhose tools weren't all auto-approved.Summary by CodeRabbit
Bug Fixes
Tests