fix(workflows): MCP tool node display + preflight (TKAI-172) - #100
fix(workflows): MCP tool node display + preflight (TKAI-172)#100yourbuddyconner wants to merge 1 commit into
Conversation
Tool-node summaries and error messages built `${node.service}.${node.action}`, but action ids already carry the service prefix by convention (MCP: minted as `${serviceName}.${tool.name}`; native plugin actions like `slack.dm_owner` are declared the same way). Result on Linear: `linear.linear.list_issues`.
Add a `formatToolCall(service, action)` helper that guards against double-prefixing, and apply it to the four client display sites and the worker-side disabled-action error string.
The workflow preflight also called `source.listActions()` with no credential context. For MCP sources this returns `[]` when no token is threaded through, so the executor threw `action "linear.list_issues" not found in linear package` and every MCP tool node failed in every workflow. Fall back to the `mcp_tool_cache` (populated by interactive tool listing and the catalog endpoint) to recover the risk level without touching the remote server. Mirrors how the catalog route mixes live + cache.
Added two targeted tests: one asserting the cache fallback resolves and executes; one asserting the "not found" error still fires when both live and cache miss.
|
Preview deployment: https://pr-100.dev-valet-turnkey-client.pages.dev |
xBalbinus
left a comment
xBalbinus
left a comment
There was a problem hiding this comment.
formatToolCall is the right fix, and reusing it across the trace card, tool body, and node summarizer kills the linear.linear.* labels everywhere they showed up. Approving — two things worth a look as follow-ups, neither blocking.
The preflight fallback depends on a warm cache: listActions() runs without credentials (tool.ts:74), so authenticated MCP sources return [] and the check falls through to mcp_tool_cache (tool.ts:86-95). On a cold cache a perfectly valid action fails with "not found in package" — a misleading hard error for something that would have executed fine. Either thread the credential context into the preflight listing or make the cold-cache miss a soft warning. Same failure class as the save-time validator's cold-cache gap flagged on #117, so one fix could cover both.
formatToolCallLabel (tool.ts:401) is a byte-identical twin of the client's formatToolCall (workflow-editor-model.ts:1361); both encode the action-id minting rule from the MCP action source. If that rule ever changes, the two copies drift silently — @valet/shared is the natural home. Related nit: the comment at tool.ts:78 cites action-source.ts by line numbers, which rot; referencing the symbol survives edits.
xBalbinus
left a comment
xBalbinus
left a comment
There was a problem hiding this comment.
Preflight resolves an MCP node's risk level from mcp_tool_cache and throws "action not found" on a miss (tool.ts:91), so a valid, executable MCP node is rejected purely on a cold cache. Fall back to the source's defaultRiskLevel (or a conservative default) and proceed — execution still resolves credentials and re-gates risk on invocation.
formatToolCallLabel (tool.ts:401) is a byte-for-byte duplicate of the client's exported formatToolCall; the prefix-dedup rule now lives in two independently named copies that have to be edited together. Export it once from @valet/shared and import it in both places.
|
Closing as part of a backlog sweep, not on the merits of the change. Every file in this PR is in the frozen legacy stack ( Reopen if this needs to land against the pinned deploy, or reimplement on the v2 stack ( |
2 participants
Fixes TKAI-172.
Two related bugs in workflow tool nodes.
Display: doubled service prefix. Tool-node summaries and error strings built
${node.service}.${node.action}, but action ids already carry the service prefix — MCP action ids are minted as${service}.${tool.name}inpackages/sdk/src/mcp/action-source.ts, and native plugin actions likeslack.dm_ownerdeclare it the same way. Result on Linear:linear.linear.list_issues; on Slack:slack.slack.dm_owner. Added aformatToolCall(service, action)helper that only joins when the action id isn't already prefixed, applied at the four client display sites and the worker-side "is disabled" error string.Preflight: MCP tool nodes always threw "not found".
packages/worker/src/workflows/nodes/tool.tscalledsource.listActions()with noActionListContext. For MCP sources this returns[]when no token is threaded through, sodefs.find(a.id === node.action)failed and the executor threwaction "linear.list_issues" not found in linear packagefor every MCP tool node in every workflow. Fall back to themcp_tool_cache(populated by interactive session-tool discovery and read by the catalog endpoint) to recover the risk level without touching the remote MCP server. If both live and cache miss, the same "not found" error still surfaces.Before / after
linear.linear.list_issues→linear.list_issuesaction "…" not found→ resolves via cache fallbackTesting
pnpm -F @valet/worker testandpnpm -F @valet/client test— all 1425 worker + 269 client tests pass. Two targeted tests added inpackages/worker/src/workflows/nodes/tool.test.ts: cache-fallback happy path, and guard that "not found" still fires when both live and cache miss.Known limitations
Cache-warm dependency: MCP integrations that are never used interactively (no session ever ran
list_toolsagainst them) have no cached entries, so workflow tool nodes for those services still throw "not found" until credential threading is added to the preflight. Follow-up worth filing if we ship an MCP integration that's workflow-only from day one.