feat(mcp): surface MCP tool behavior hints in the tools table - #14885
feat(mcp): surface MCP tool behavior hints in the tools table#14885thesaadmirza wants to merge 6 commits into
Conversation
Gating a tool for approval is already possible per tool, but nothing told the author which tools were worth gating, so on a server with 40 tools you read descriptions and guessed. MCP servers already answer this: the spec puts readOnlyHint and destructiveHint on every tool and the Python SDK parses them into Tool.annotations, which Langflow discarded while reading outputSchema off the same object. A server that sent no annotations gets no badge rather than a default, because the spec defaults destructiveHint to true and applying that to an un-annotated server would mark every one of its tools destructive. The hint stays display only: the MCP spec is explicit that a client must not make tool-use decisions from annotations it does not trust, so nothing gates, exempts, or runs a tool based on it and approval_actions is never touched. Also fixes a data-loss bug in the same table: the selection effect restored suppressRowClickSelection to false instead of the configured true, leaving row-click selection live for the rest of the modal. A plain click on an already-selected row then took ag-grid's clearSelection path, so gating one tool cleared every other row, wrote status:false, and silently dropped those tools from the toolset. The option is read only by ag-grid's onRowClick and never by the programmatic node.setSelected this effect uses, so both calls are removed rather than corrected.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (15)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. WalkthroughMCP annotations now produce access hints that flow into tool metadata and the tools table. The table displays localized access badges when hints exist. Selection synchronization preserves AG Grid row-click suppression. ChangesMCP access hint display
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The PR adds optional, display-only MCP access hints and fixes tool-table selection behavior so enabling approval does not disable unrelated tools. No actionable merge-blocking risk remains at the current head. Sequence Diagram(s)sequenceDiagram
participant MCPToolServer
participant MCPMetadata
participant Component
participant ToolsTable
participant AccessHintBadge
MCPToolServer->>MCPMetadata: provide annotations
MCPMetadata->>Component: attach access_hint
Component->>ToolsTable: provide tool row
ToolsTable->>AccessHintBadge: render access_hint
AccessHintBadge-->>ToolsTable: display localized badge
Suggested reviewers: 🚥 Pre-merge checks | ✅ 6 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (6 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 8 files. (7 skipped: 7 unsupported.) Full details: Test Coverage For New ImplementationsExplanation The PR includes tests for each new behavior. Full details: Test Quality And CoverageExplanation The added tests cover the pure hint classifier and some table configuration, but they do not comprehensively test the changed behavior. The async Resolution Add a pytest async test that mocks an MCP client, awaits Full details: Test File Naming And StructureExplanation The backend test follows pytest conventions: Resolution Rewrite the three new frontend tests as Playwright tests using Full details: Excessive Mock Usage WarningExplanation PASS. The two ToolsTable tests use nine mocks each, but the mocks replace UI children, formatting helpers, and the AG Grid wrapper. The tests execute the real ToolsTable logic and inspect its column definitions and grid API interactions. The AG Grid wrapper is an appropriate test double for the external grid boundary. This pattern already exists in the pre-existing toolsTable-pagination.test.tsx. AccessHintBadge and the Python annotation tests do not add local mocks. The changed tests therefore do not show excessive mocking that obscures the behavior under test. ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Conflicts were all in generated or auto-formatted files that autofix.ci had rewritten on this branch: the component index, four starter projects, and one Playwright spec. Resolved to main's version in every case, the same way the py_autofix workflow resolves component_index.json, so this branch carries only the MCP access-hint change and the tools-table selection fix.
Langflow can already require approval on a single tool, but nothing tells you which tools are worth gating. On a server with 40 tools you read descriptions and guess. MCP servers already answer this: the spec puts
readOnlyHintanddestructiveHinton every tool and the Python SDK parses them intoTool.annotations. Langflow readsoutputSchemaoff that same object and drops the hints on the floor. This surfaces them in the Actions table. While testing it I hit a separate data-loss bug in the same table and fixed it here too.Example: connect an MCP server and the table now shows
search_repositoriesas Read-only,create_issueas Write, anddelete_repositoryas Destructive, right next to the Requires Approval switch. A tool the server says nothing about gets no badge rather than a misleading default.Changes
lfx/base/mcp/util.py:_tool_access_hintderives one of read_only / write / destructive from the server'sToolAnnotations, andupdate_toolsputs it on the tool metadata besideoutput_schema.lfx/custom/custom_component/component.py:_build_tool_datacarriesaccess_hintinto thetools_metadatarow.AccessHintBadge, an Access column in the tools table, and labels in all seven locales.Fix: turning on Requires Approval silently disabled every other tool
The tools grid is configured with
suppressRowClickSelection={true}, but the effect that applies selection restored the option tofalseinstead oftrueafter running. That effect fires on every modal open, so row-click selection was live from then on. In ag-grid 32, a plain click on an already-selected row then takes thesetSelectedParams({newValue: true, clearSelection: true})path, meaning "select only this row". Every other tool's checkbox cleared, which wrotestatus: falseand dropped those tools from the toolset, so gating a delete tool quietly removed the search tool from the agent.Both
setGridOptioncalls are removed rather than corrected:suppressRowClickSelectionis read in exactly one place in ag-grid (onRowClick), so the programmaticnode.setSelected()this effect uses never consulted it and the guard protected nothing.Same click, before and after:
Note that the
stopPropagationon the approval Switch could never have prevented this. React attaches synthetic listeners at the root container, ag-grid's mouse listeners are native and attached inside it, so ag-grid handles the click before React invokes the Switch'sonClick. ag-grid's own opt-out is the__ag_Grid_Stop_Propagationflag it checks at the top ofRowCtrl.onRowEventandCellMouseListenerFeature.onMouseEvent.Design notes
The hint is display-only. The MCP spec is explicit that a client must not make tool-use decisions from annotations it does not trust, so nothing gates, exempts, or runs a tool based on it, and
approval_actionsis never touched. A server that sent no annotations gets no badge: the spec defaultsdestructiveHintto true, and applying that to an un-annotated server would mark every one of its tools destructive. Once either hint is present the defaults do apply, soreadOnlyHint: falsealone reads as destructive.The Access column is dropped entirely when no tool carries a hint, since only MCP servers declare them and every other component's tool table would otherwise show a permanently blank column.
access_hintstores a derived string rather than the raw annotations. That keeps the spec's default rules in one place, at the cost of not persistingidempotentHintandopenWorldHint. Since the field lands in saved flow JSON, happy to switch to storing the raw hints instead if you would rather keep that door open.Verified end to end against a real stdio MCP server exposing one tool per annotation case, including the persisted
tools_metadatain the saved flow.Summary by CodeRabbit