Skip to content

Fix(mcp): "set all" button now follows active search#2046

Open
w3joe wants to merge 1 commit intoPostHog:mainfrom
w3joe:fix(mcp)-set-all-button-follows-active-search
Open

Fix(mcp): "set all" button now follows active search#2046
w3joe wants to merge 1 commit intoPostHog:mainfrom
w3joe:fix(mcp)-set-all-button-follows-active-search

Conversation

@w3joe
Copy link
Copy Markdown

@w3joe w3joe commented May 5, 2026

Problem

When a user searched for tools by name and clicked "Set all", the bulk approval action applied to all tools in the installation, not just the filtered results. This made it impossible to bulk-approve a subset of tools by name prefix (e.g. all create_* tools from a Linear MCP server).

Closes #2043

Changes

  • useMcpInstallationTools: setBulkApproval now accepts an optional targetTools array. When provided, only those tools are updated; otherwise falls back to all tools (preserving existing behaviour).
  • ServerDetailView: when a search is active, the "Set all" buttons pass filteredTools as the target. Tooltips update to say "Approve filtered" / "Require approval for filtered" / "Block filtered" to make the scope clear to the user.

How did you test this?

Manually tested in the app: Settings → MCP Servers → connected server with >5 tools → typed a prefix in the search box → clicked each "Set all" button and confirmed only the matching tools were updated.

Publish to changelog?

No

Allow bulk approval actions to target the currently filtered tool list when a search is active. UI: update tooltips to reflect "filtered" vs "all", disable buttons based on filteredTools length, and pass filteredTools to the action when toolSearch is set. Hook: change setBulkApproval mutation signature to accept an object { approval_state, targetTools? } and forward targetTools (or fallback to tools) to dispatchBulkApproval; expose setBulkApproval as a function taking (approval_state, targetTools?) for callers.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 5, 2026

Comments Outside Diff (1)

  1. apps/code/src/renderer/features/mcp-servers/components/parts/ServerDetailView.tsx, line 251-307 (link)

    P2 Repeated inline expression violates OnceAndOnlyOnce

    The expression toolSearch ? filteredTools : undefined (and the disabled predicate bulkPending || filteredTools.length === 0) are each written three times. Extracting them to local variables removes the duplication and makes the intent of each onClick immediately clear.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: apps/code/src/renderer/features/mcp-servers/components/parts/ServerDetailView.tsx
    Line: 251-307
    
    Comment:
    **Repeated inline expression violates OnceAndOnlyOnce**
    
    The expression `toolSearch ? filteredTools : undefined` (and the disabled predicate `bulkPending || filteredTools.length === 0`) are each written three times. Extracting them to local variables removes the duplication and makes the intent of each `onClick` immediately clear.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
apps/code/src/renderer/features/mcp-servers/components/parts/ServerDetailView.tsx:251-307
**Repeated inline expression violates OnceAndOnlyOnce**

The expression `toolSearch ? filteredTools : undefined` (and the disabled predicate `bulkPending || filteredTools.length === 0`) are each written three times. Extracting them to local variables removes the duplication and makes the intent of each `onClick` immediately clear.

```suggestion
            <Flex gap="2" align="center">
              <Text color="gray" className="text-[13px]">
                Set all:
              </Text>
              {(() => {
                const bulkTarget = toolSearch ? filteredTools : undefined;
                const isBulkDisabled = bulkPending || filteredTools.length === 0;
                return (
                  <>
                    <Tooltip
                      content={toolSearch ? "Approve filtered" : "Approve all"}
                    >
                      <IconButton
                        variant="soft"
                        color="green"
                        size="1"
                        disabled={isBulkDisabled}
                        onClick={() => setBulkApproval("approved", bulkTarget)}
                      >
                        <Check size={12} weight="bold" />
                      </IconButton>
                    </Tooltip>
```

Reviews (1): Last reviewed commit: "Support bulk actions on filtered tools" | Re-trigger Greptile

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.

In the MCP server editor, the "set all" actions should respect the active search

1 participant