diff --git a/.changeset/completed-mcp-tool-name.md b/.changeset/completed-mcp-tool-name.md new file mode 100644 index 0000000000..1f97e79c55 --- /dev/null +++ b/.changeset/completed-mcp-tool-name.md @@ -0,0 +1,5 @@ +--- +"@executor-js/execution": patch +--- + +Completed MCP execute results now include `toolName` when a script successfully uses exactly one connected tool. Executions that use distinct tools remain unlabeled, and internal call provenance is not exposed in the MCP response. diff --git a/apps/cloud/src/mcp/session-build-semaphore.test.ts b/apps/cloud/src/mcp/session-build-semaphore.test.ts index 3d4ad76343..584b65ee0e 100644 --- a/apps/cloud/src/mcp/session-build-semaphore.test.ts +++ b/apps/cloud/src/mcp/session-build-semaphore.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, beforeEach } from "@effect/vitest"; +import { describe, expect, it, beforeEach, afterEach, vi } from "@effect/vitest"; import { acquireBuildSlot, @@ -13,6 +13,10 @@ describe("session-build-semaphore", () => { resetBuildSlotsForTest(); }); + afterEach(() => { + vi.useRealTimers(); + }); + it("grants up to the cap immediately, with no wait", async () => { const results = await Promise.all([ acquireBuildSlot().promise, @@ -214,6 +218,7 @@ describe("session-build-semaphore", () => { }); it("proceeds without a slot when the queue wait exceeds the timeout, and does not count it as active", async () => { + vi.useFakeTimers(); await Promise.all([ acquireBuildSlot().promise, acquireBuildSlot().promise, @@ -223,6 +228,10 @@ describe("session-build-semaphore", () => { expect(currentActiveBuildsForTest()).toBe(4); const timedOutHandle = acquireBuildSlot(10); + await vi.advanceTimersByTimeAsync(9); + expect(currentQueueLengthForTest()).toBe(1); + expect(currentActiveBuildsForTest()).toBe(4); + await vi.advanceTimersByTimeAsync(1); const result = await timedOutHandle.promise; expect(result).toEqual({ acquired: false, waitMs: expect.any(Number), timedOut: true }); diff --git a/e2e/scenarios/tool-call-contract.test.ts b/e2e/scenarios/tool-call-contract.test.ts index 7cb5394e3b..b693e50602 100644 --- a/e2e/scenarios/tool-call-contract.test.ts +++ b/e2e/scenarios/tool-call-contract.test.ts @@ -21,7 +21,7 @@ import { randomBytes } from "node:crypto"; import { createServer } from "node:http"; import { expect } from "@effect/vitest"; -import { Effect } from "effect"; +import { Effect, Schema } from "effect"; import { composePluginApi } from "@executor-js/api/server"; import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; import { @@ -38,6 +38,13 @@ import type { McpSession } from "../src/surfaces/mcp"; const api = composePluginApi([openApiHttpPlugin()] as const); +const completion = Schema.Struct({ + structuredContent: Schema.Struct({ + status: Schema.String, + toolName: Schema.optional(Schema.String), + }), +}); + const unique = (prefix: string) => `${prefix}_${randomBytes(4).toString("hex")}`; type UpstreamHandle = { @@ -114,14 +121,14 @@ const executeApproved = (session: McpSession, code: string) => guard += 1; } expect(result.ok, `execute completed (got: ${result.text.slice(0, 400)})`).toBe(true); - return result.text; + return result; }); /** Invoke a dynamic tool by full address and parse the envelope it returns. */ const invokeEnvelope = (session: McpSession, address: string, args: unknown = {}) => Effect.map( executeApproved(session, invokeByAddressCode(address, args)), - (text) => JSON.parse(text) as ToolEnvelope, + (result) => JSON.parse(result.text) as ToolEnvelope, ); // --------------------------------------------------------------------------- @@ -136,6 +143,20 @@ const widgetsSpec = (baseUrl: string): string => info: { title: "Widgets API", version: "1.0.0" }, servers: [{ url: baseUrl }], paths: { + "/unavailable": { + get: { + operationId: "unavailableWidgets", + summary: "Unavailable widgets", + responses: { "200": { description: "widgets" } }, + }, + }, + "/widgets/count": { + get: { + operationId: "countWidgets", + summary: "Count widgets", + responses: { "200": { description: "widget count" } }, + }, + }, "/widgets": { get: { operationId: "listWidgets", @@ -184,7 +205,7 @@ scenario( }, }); const created = JSON.parse( - yield* executeApproved(session, createConnectionCode(slug)), + (yield* executeApproved(session, createConnectionCode(slug))).text, ) as ToolEnvelope; expect(created.ok, `the no-auth connection was created: ${JSON.stringify(created)}`).toBe( true, @@ -199,7 +220,8 @@ scenario( const path = address!.replace(/^tools\./, ""); // 1. A well-addressed call executes and carries the upstream's payload. - const success = yield* invokeEnvelope(session, address!); + const successfulCall = yield* executeApproved(session, invokeByAddressCode(address!, {})); + const success = JSON.parse(successfulCall.text) as ToolEnvelope; expect( success.ok, `the call succeeded (got: ${JSON.stringify(success.error ?? {}).slice(0, 400)})`, @@ -207,7 +229,73 @@ scenario( expect(JSON.stringify(success.data), "the upstream's payload comes back").toContain( "anvil", ); + const structured = (successfulCall.raw as { readonly structuredContent?: unknown }) + .structuredContent; + expect(structured, "the completed MCP result includes structured content").toMatchObject({ + status: "completed", + toolName: path, + }); + expect(structured, "the internal tool-call trace is not exposed").not.toHaveProperty( + "toolPaths", + ); expect(upstream.requests(), "the upstream served exactly one call").toBe(1); + const discovery = yield* executeApproved( + session, + 'return await tools.search({ query: "widgets" });', + ); + const discoveryResult = yield* Schema.decodeUnknownEffect(completion)(discovery.raw); + expect(discoveryResult.structuredContent.toolName).toBeUndefined(); + expect(discovery.raw).not.toHaveProperty("structuredContent.toolPaths"); + + const failed = yield* executeApproved( + session, + invokeByAddressCode(address!.replace(/listWidgets$/, "missingWidget"), {}), + ); + const failedResult = yield* Schema.decodeUnknownEffect(completion)(failed.raw); + expect(failedResult.structuredContent.toolName).toBeUndefined(); + expect(failed.raw).not.toHaveProperty("structuredContent.toolPaths"); + const unavailableAddress = yield* Schema.decodeUnknownEffect(Schema.String)( + tools + .filter((tool) => String(tool.integration) === slug) + .map((tool) => String(tool.address)) + .find((candidate) => candidate.endsWith("unavailableWidgets")), + ); + const upstreamFailure = yield* executeApproved( + session, + invokeByAddressCode(unavailableAddress, {}), + ); + const upstreamFailureResult = yield* Schema.decodeUnknownEffect(completion)( + upstreamFailure.raw, + ); + expect(upstreamFailure.text).toContain('"ok":false'); + expect(upstreamFailureResult.structuredContent.toolName).toBeUndefined(); + expect(upstreamFailure.raw).not.toHaveProperty("structuredContent.toolPaths"); + + const repeated = yield* executeApproved( + session, + ` + await tools[${JSON.stringify(path)}]({}); + return await tools[${JSON.stringify(path)}]({}); + `, + ); + const repeatedResult = yield* Schema.decodeUnknownEffect(completion)(repeated.raw); + expect(repeatedResult.structuredContent.toolName).toBe(path); + expect(repeated.raw).not.toHaveProperty("structuredContent.toolPaths"); + const anotherAddress = tools + .filter((tool) => String(tool.integration) === slug) + .map((tool) => String(tool.address)) + .find((candidate) => candidate.endsWith("countWidgets")); + const anotherPath = yield* Schema.decodeUnknownEffect(Schema.String)(anotherAddress); + const multiple = yield* executeApproved( + session, + ` + await tools[${JSON.stringify(path)}]({}); + return await tools[${JSON.stringify(anotherPath.replace(/^tools\./, ""))}]({}); + `, + ); + const multipleResult = yield* Schema.decodeUnknownEffect(completion)(multiple.raw); + expect(multipleResult.structuredContent.toolName).toBeUndefined(); + expect(multiple.raw).not.toHaveProperty("structuredContent.toolPaths"); // 2a. A wrong TOOL name on a live connection: tool_not_found, and the // suggestions name the connection's real tools so the agent can @@ -257,7 +345,9 @@ scenario( "the defect mask never surfaces for a missing connection", ).not.toContain("Internal tool error"); - expect(upstream.requests(), "no misaddressed call ever reached the upstream").toBe(1); + expect(upstream.requests(), "only the five successful calls reached the upstream").toBe( + 5, + ); }), // Selfhost shares one workspace identity — leaked resources fail other // scenarios' zero-state assertions. diff --git a/packages/core/execution/src/engine.test.ts b/packages/core/execution/src/engine.test.ts index 06b9e20888..e1ca25eef6 100644 --- a/packages/core/execution/src/engine.test.ts +++ b/packages/core/execution/src/engine.test.ts @@ -309,6 +309,35 @@ describe("formatExecuteResult output identity", () => { expect(formatted.isError).toBe(false); }); + it("returns the sole distinct connected tool name without exposing the call trace", () => { + const result = { + result: { issues: [] }, + logs: [], + toolPaths: ["linear.org.work.issues.list", "linear.org.work.issues.list"], + } as ExecuteResult & { readonly toolPaths: readonly string[] }; + + const formatted = formatExecuteResult(result); + + expect(formatted.structured).toEqual({ + status: "completed", + result: { issues: [] }, + toolName: "linear.org.work.issues.list", + logs: [], + }); + }); + + it("omits a tool name when distinct connected tools were used", () => { + const result = { + result: { issues: [], projects: [] }, + logs: [], + toolPaths: ["linear.org.work.issues.list", "linear.org.work.projects.list"], + } as ExecuteResult & { readonly toolPaths: readonly string[] }; + + const formatted = formatExecuteResult(result); + + expect(formatted.structured).not.toHaveProperty("toolName"); + }); + it("truncates a long preview with the exact suffix and untouched structured value", () => { const value = { data: "é🎉".repeat(12_000) }; const pretty = JSON.stringify(value, null, 2); diff --git a/packages/core/execution/src/engine.ts b/packages/core/execution/src/engine.ts index 8bb9bda071..7e4e06340b 100644 --- a/packages/core/execution/src/engine.ts +++ b/packages/core/execution/src/engine.ts @@ -138,6 +138,11 @@ const truncate = (value: string, max: number): string => ? `${value.slice(0, max)}\n... [truncated ${value.length - max} chars]` : value; +const soleConnectedToolName = (toolPaths: readonly string[] | undefined): string | undefined => { + const names = [...new Set(toolPaths ?? [])]; + return names.length === 1 ? names[0] : undefined; +}; + export const formatExecuteResult = ( result: ExecuteResult, ): { @@ -183,11 +188,13 @@ export const formatExecuteResult = ( ? `(no return value; ${emittedNote})` : "(no result)"; const parts = [resultPart, ...(logText ? [`\nLogs:\n${logText}`] : [])]; + const toolName = soleConnectedToolName(result.toolPaths); return { text: parts.join("\n"), structured: { status: "completed", result: result.result ?? null, + ...(toolName ? { toolName } : {}), ...emittedField, logs: result.logs ?? [], }, @@ -318,8 +325,9 @@ const makeFullInvoker = ( executor: Executor, invokeOptions: InvokeOptions, toolDiscoveryProvider: ToolDiscoveryProvider, + onConnectedToolCall?: (path: string) => void, ): SandboxToolInvoker => { - const base = makeExecutorToolInvoker(executor, { invokeOptions }); + const base = makeExecutorToolInvoker(executor, { invokeOptions, onConnectedToolCall }); return { invoke: ({ path, args }) => { if (path === "search") { @@ -694,13 +702,18 @@ export const createExecutionEngine = toolPaths.push(path), ); fiber = yield* Effect.forkDetach( - codeExecutor.execute(code, invoker).pipe(Effect.withSpan("executor.code.exec")), + codeExecutor.execute(code, invoker).pipe( + Effect.map((result) => (toolPaths.length === 0 ? result : { ...result, toolPaths })), + Effect.withSpan("executor.code.exec"), + ), ); liveSandboxFibers.add(fiber); @@ -825,16 +838,19 @@ export const createExecutionEngine = toolPaths.push(path), + ); + const result = yield* codeExecutor.execute(code, invoker).pipe( + Effect.map((result) => (toolPaths.length === 0 ? result : { ...result, toolPaths })), + Effect.withSpan("executor.code.exec"), ); - const result = yield* codeExecutor - .execute(code, invoker) - .pipe(Effect.withSpan("executor.code.exec")); yield* annotateExecuteOutcome(result); return result; }); diff --git a/packages/core/execution/src/tool-invoker.test.ts b/packages/core/execution/src/tool-invoker.test.ts index 747bd23105..97beb69ed4 100644 --- a/packages/core/execution/src/tool-invoker.test.ts +++ b/packages/core/execution/src/tool-invoker.test.ts @@ -674,6 +674,25 @@ describe("tool discovery", () => { }), ); + it.effect("records only the connected tool resolved after discovery", () => + Effect.gen(function* () { + const executor = yield* makeSearchExecutor(); + const engine = createExecutionEngine({ executor, codeExecutor }); + + const execution = yield* engine.execute( + [ + 'const search = await tools.search({ query: "repository details", namespace: "github", limit: 1 });', + 'const result = await tools[search.items[0].path]({ owner: "executor", repo: "executor" });', + "return result;", + ].join("\n"), + { onElicitation: acceptAll }, + ); + + expect(execution.error).toBeUndefined(); + expect(execution.toolPaths).toEqual(["github.org.main.getRepositoryDetails"]); + }), + ); + it.effect("lets execution hosts provide custom tool discovery", () => Effect.gen(function* () { const executor = yield* makeSearchExecutor(); diff --git a/packages/core/execution/src/tool-invoker.ts b/packages/core/execution/src/tool-invoker.ts index 2df47644ed..6a09de027b 100644 --- a/packages/core/execution/src/tool-invoker.ts +++ b/packages/core/execution/src/tool-invoker.ts @@ -306,7 +306,10 @@ const extractNamespace = (path: string): string => { */ export const makeExecutorToolInvoker = ( executor: Executor, - options: { readonly invokeOptions: InvokeOptions }, + options: { + readonly invokeOptions: InvokeOptions; + readonly onConnectedToolCall?: (path: string) => void; + }, ): SandboxToolInvoker => ({ invoke: Effect.fn("mcp.tool.dispatch")(function* ({ path, args }) { yield* Effect.annotateCurrentSpan({ @@ -372,6 +375,12 @@ export const makeExecutorToolInvoker = ( // outcome annotation the dispatch span reads as healthy even when the // caller hit an upstream error or auth wall. yield* annotateToolResultOutcome(result); + const connectedToolPath = parseToolAddress(String(address)) + ? addressToPath(String(address)) + : undefined; + if (connectedToolPath && (!isToolResult(result) || result.ok)) { + options.onConnectedToolCall?.(connectedToolPath); + } if (isToolResult(result)) { return result; } diff --git a/packages/hosts/mcp/src/tool-server.test.ts b/packages/hosts/mcp/src/tool-server.test.ts index 1b7bb8aa45..c83fdabb0f 100644 --- a/packages/hosts/mcp/src/tool-server.test.ts +++ b/packages/hosts/mcp/src/tool-server.test.ts @@ -1039,12 +1039,15 @@ describe("MCP host server — native form-only elicitation", () => { // --------------------------------------------------------------------------- describe("MCP host server — client without elicitation (pause/resume)", () => { - it("completed execution returns result directly", async () => { + it("completed execution returns result and connected-tool metadata directly", async () => { const engine = makeStubEngine({ executeWithPause: () => Effect.succeed({ status: "completed", - result: { result: "done" }, + result: { + result: "done", + toolPaths: ["linear.org.work.issues.list"], + }, }), }); @@ -1054,6 +1057,10 @@ describe("MCP host server — client without elicitation (pause/resume)", () => arguments: { code: "ok" }, }); expect(result.content).toEqual([{ type: "text", text: "done" }]); + expect(result.structuredContent).toMatchObject({ + status: "completed", + toolName: "linear.org.work.issues.list", + }); expect(result.isError).toBeFalsy(); }); }); diff --git a/packages/kernel/core/src/types.ts b/packages/kernel/core/src/types.ts index 1480b27b9a..d6dee10ad4 100644 --- a/packages/kernel/core/src/types.ts +++ b/packages/kernel/core/src/types.ts @@ -46,6 +46,8 @@ export type ExecuteResult = { /** Enumerable failure class for telemetry; never carries message content. */ errorKind?: ExecuteErrorKind; logs?: string[]; + /** Successful connected-tool paths observed during this execution. */ + toolPaths?: readonly string[]; }; /**