From 6db95d24f3226bf92e08bc3b272bb40fb55f9c79 Mon Sep 17 00:00:00 2001 From: sissedsauce Date: Wed, 7 Oct 2026 18:22:40 -0700 Subject: [PATCH 01/12] fix(shared): show html_render pages published through OpenCode Code Mode execute dynamic_tool items named execute now resolve an embedded htmlRender reference, and result envelopes shaped like {content: {text}} parse. --- packages/shared/src/htmlRender.test.ts | 10 ++++++++++ packages/shared/src/toolOutput.test.ts | 25 +++++++++++++++++++++++++ packages/shared/src/toolOutput.ts | 14 +++++++------- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/packages/shared/src/htmlRender.test.ts b/packages/shared/src/htmlRender.test.ts index 5ef4d7f973475..c563e9cf7760e 100644 --- a/packages/shared/src/htmlRender.test.ts +++ b/packages/shared/src/htmlRender.test.ts @@ -177,6 +177,16 @@ describe("htmlRenderFromToolItem", () => { htmlRenderFromToolItem({ toolName: "html_render", output: { ...result, isError: true } }), ).toBeUndefined(); }); + + it("reads htmlRender from an execute tool output", () => { + const executeRender = { + attachmentId: "thread-1-00000000-0000-4000-8000-000000000001-html", + title: "Chart", + height: 420, + }; + const output = [{ type: "text", text: JSON.stringify({ htmlRender: executeRender }) }]; + expect(htmlRenderFromToolItem({ toolName: "execute", output })).toEqual(executeRender); + }); }); describe("htmlRenderFrameHeight", () => { diff --git a/packages/shared/src/toolOutput.test.ts b/packages/shared/src/toolOutput.test.ts index 489fa52471476..20aab7d6a5701 100644 --- a/packages/shared/src/toolOutput.test.ts +++ b/packages/shared/src/toolOutput.test.ts @@ -145,6 +145,31 @@ describe("compactDynamicToolOutput", () => { }); expect(compactDynamicToolOutput(compact)).toEqual(compact); }); + + it("keeps htmlRender through the established text envelopes", () => { + const htmlRender = { + attachmentId: "thread-1-00000000-0000-4000-8000-000000000001-html", + title: "Chart", + height: 420, + }; + const json = JSON.stringify({ htmlRender }); + expect(compactDynamicToolOutput([{ type: "text", text: json }])).toEqual({ htmlRender }); + expect(compactDynamicToolOutput({ content: [{ text: { text: json } }] })).toEqual({ + htmlRender, + }); + }); + + it("extracts htmlRender from the ACP registry content envelope", () => { + const htmlRender = { + attachmentId: "thread-1-00000000-0000-4000-8000-000000000001-html", + title: "Chart", + height: 420, + }; + const json = JSON.stringify({ htmlRender }); + expect( + compactDynamicToolOutput([{ type: "content", content: { type: "text", text: json } }]), + ).toEqual({ htmlRender }); + }); }); describe("toolOutputIndicatesFailure", () => { diff --git a/packages/shared/src/toolOutput.ts b/packages/shared/src/toolOutput.ts index 5e1c3190e86d3..9f007b7edd51a 100644 --- a/packages/shared/src/toolOutput.ts +++ b/packages/shared/src/toolOutput.ts @@ -70,12 +70,9 @@ function readResult(value: unknown, budget: ResultReadBudget, depth = 0): Result let data: ResultEnvelope["data"]; let failed = false; for (const block of value) { - const text = Predicate.isObject(block) ? block.text : undefined; - const result = readResult( - Predicate.isObject(text) ? (text.text ?? text) : text, - budget, - depth + 1, - ); + const content = Predicate.isObject(block) ? (block.text ?? block.content) : block; + const text = Predicate.isObject(content) ? (content.text ?? content) : content; + const result = readResult(text, budget, depth + 1); data ??= result.data; failed ||= result.failed; if (budget.exceeded) break; @@ -169,7 +166,10 @@ export function htmlRenderFromToolItem(item: { readonly toolName: string | null | undefined; readonly output?: unknown; }): HtmlRenderReference | undefined { - if (resolveT3McpToolId(item.toolName) !== HTML_RENDER_TOOL_NAME) return undefined; + // OpenCode 2 Code Mode runs T3 tools inside `execute`, so the item keeps + // toolName "execute" even when the script returns html_render's result. + if (item.toolName !== "execute" && resolveT3McpToolId(item.toolName) !== HTML_RENDER_TOOL_NAME) + return undefined; const output = compactDynamicToolOutput(item.output); return output?.isError ? undefined : output?.htmlRender; } From cb0e5e828a0c4587abb1f3b45d8bab2db115102c Mon Sep 17 00:00:00 2001 From: sissedsauce Date: Sun, 11 Oct 2026 08:04:48 -0700 Subject: [PATCH 02/12] fix(shared,acp): recover html_render pages from wrapped provider envelopes Covers the CodeBuddy bare text block, Grok OkayOutput envelope and annotated titles, and Devin/CodeBuddy namespaced identity, in addition to the OpenCode execute wrapper and Devin content envelope. --- .../Adapters/AcpAdapterV2.test.ts | 31 ++++++++ .../orchestration-v2/Adapters/AcpAdapterV2.ts | 15 +++- .../src/provider/acp/AcpRuntimeModel.test.ts | 71 +++++++++++++++++++ .../src/provider/acp/AcpRuntimeModel.ts | 22 ++++-- .../shared/src/t3McpToolPresentation.test.ts | 8 +++ packages/shared/src/t3McpToolPresentation.ts | 11 ++- packages/shared/src/toolOutput.test.ts | 15 ++++ packages/shared/src/toolOutput.ts | 10 +++ 8 files changed, 174 insertions(+), 9 deletions(-) diff --git a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts index 0724c3c0a57f5..b62f306535c03 100644 --- a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts @@ -83,6 +83,7 @@ import { acpPostSettleWakeEvidence, acpPostSettleWakeShouldBuffer, acpProjectedCommandExitCode, + acpMcpToolCallOutput, acpToolCallDiffPatch, acpTurnStartShouldPreserveContinuation, makeAcpAdapterV2, @@ -126,6 +127,36 @@ describe("acpProjectedCommandExitCode", () => { }); }); +describe("acpMcpToolCallOutput", () => { + it("unwraps codex-acp result/content envelopes", () => { + const content = [{ type: "text", text: '{"ok":true}' }]; + assert.deepStrictEqual(acpMcpToolCallOutput({ result: { content } }), content); + assert.deepStrictEqual(acpMcpToolCallOutput({ result: { structuredContent: { ok: true } } }), { + ok: true, + }); + }); + + it("unwraps Grok's OkayOutput result string", () => { + const inner = JSON.stringify({ + htmlRender: { attachmentId: "t-html", title: "T", height: 100 }, + }); + assert.strictEqual( + acpMcpToolCallOutput({ + type: "MCP", + tool_name: "html_render", + server_name: "t3-code", + output: { OkayOutput: inner }, + }), + inner, + ); + // Non-string or missing OkayOutput passes through untouched. + const other = { type: "MCP", output: { OkayOutput: { nested: true } } }; + assert.strictEqual(acpMcpToolCallOutput(other), other); + const plain = { htmlRender: { attachmentId: "t-html" } }; + assert.strictEqual(acpMcpToolCallOutput(plain), plain); + }); +}); + describe("acpToolCallDiffPatch", () => { it("builds a patch per file from ACP v1 oldText/newText, with /dev/null for a new file", () => { assert.equal( diff --git a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts index 179455cb64a8b..018dcdbff423e 100644 --- a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts +++ b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts @@ -793,11 +793,20 @@ function makeProviderThread(input: { * Unwrap a codex-acp style MCP call result (`{ result, error }` around MCP * `content`/`structuredContent`) the same way the native Codex adapter does, * so recovered MCP items render identical output. Unknown shapes pass through. + * + * Grok wraps results one level deeper (`{ type: "MCP", output: { OkayOutput: + * "" } }`, Rust Result serialization); return that string so the + * shared reader parses it like any other result envelope. */ -function acpMcpToolCallOutput(rawOutput: unknown): unknown { +export function acpMcpToolCallOutput(rawOutput: unknown): unknown { const record = unknownRecord(rawOutput); if (record === undefined) return rawOutput; - if (!("result" in record) && !("error" in record)) return rawOutput; + if (!("result" in record) && !("error" in record)) { + const output = unknownRecord(record.output); + const okay = output === undefined ? undefined : output.OkayOutput; + if (typeof okay === "string") return okay; + return rawOutput; + } const result = unknownRecord(record.result); const resultOutput = result === undefined ? undefined : (result.structuredContent ?? result.content ?? undefined); @@ -3471,7 +3480,7 @@ export function makeAcpAdapterV2( type: "dynamic_tool", toolName: toolCall.title ?? toolCall.kind ?? null, input: rawInput ?? {}, - ...(rawOutput === undefined ? {} : { output: rawOutput }), + ...(rawOutput === undefined ? {} : { output: acpMcpToolCallOutput(rawOutput) }), }; } } diff --git a/apps/server/src/provider/acp/AcpRuntimeModel.test.ts b/apps/server/src/provider/acp/AcpRuntimeModel.test.ts index 27cb3234b3752..2959fd1d70501 100644 --- a/apps/server/src/provider/acp/AcpRuntimeModel.test.ts +++ b/apps/server/src/provider/acp/AcpRuntimeModel.test.ts @@ -1425,6 +1425,77 @@ describe("extractMcpToolCallIdentity", () => { }); }); + it("recovers T3 identity from CodeBuddy rawInput.toolName", () => { + // CodeBuddy wraps injected MCP calls in its own ToolSearch layer, so the + // wire title/kind is generic; the qualified name survives in rawInput. + const toolCall = toolCallFromUpdate({ + sessionUpdate: "tool_call", + toolCallId: "tool-1", + kind: "Tool", + title: "Tool", + status: "in_progress", + rawInput: { toolName: "mcp__t3-code__html_render", arguments: {} }, + }); + + expect(extractMcpToolCallIdentity(toolCall)).toEqual({ + server: "t3-code", + tool: "html_render", + }); + }); + + it("recovers T3 identity from Devin namespaced meta", () => { + const toolCall = toolCallFromUpdate({ + sessionUpdate: "tool_call", + toolCallId: "tool-2", + kind: "DeferExecuteTool", + title: "Deferred tool call", + status: "in_progress", + rawInput: { server_name: "other", tool_name: "html_render" }, + _meta: { "cognition.ai/inferenceToolName": "mcp__t3-code__html_render" }, + }); + + expect(extractMcpToolCallIdentity(toolCall)).toEqual({ + server: "t3-code", + tool: "html_render", + }); + }); + + it("recovers T3 identity from Grok snake_case rawInput and annotated titles", () => { + // Grok's inline use_tool calls carry tool_name/tool_input/variant keys, + // and file-passed calls annotate the title with a source note. + const toolCall = toolCallFromUpdate({ + sessionUpdate: "tool_call", + toolCallId: "tool-3", + kind: "UseTool", + title: "t3-code__html_render — source file: /tmp/mcp-render.json", + status: "in_progress", + rawInput: { tool_name: "html_render", tool_input: { height: 200 }, variant: "UseTool" }, + _meta: { is_mcp_tool_call: true }, + }); + + expect(extractMcpToolCallIdentity(toolCall)).toEqual({ + server: "t3-code", + tool: "html_render", + }); + }); + + it("recovers T3 identity from snake_case rawInput pairs", () => { + const toolCall = toolCallFromUpdate({ + sessionUpdate: "tool_call", + toolCallId: "tool-4", + kind: "UseTool", + title: "UseTool", + status: "in_progress", + rawInput: { server_name: "t3-code", tool_name: "html_preview" }, + _meta: { is_mcp_tool_call: true }, + }); + + expect(extractMcpToolCallIdentity(toolCall)).toEqual({ + server: "t3-code", + tool: "html_preview", + }); + }); + it("recovers T3 identity from acp-mcp-call fallback commands", () => { const toolCall = toolCallFromUpdate({ sessionUpdate: "tool_call", diff --git a/apps/server/src/provider/acp/AcpRuntimeModel.ts b/apps/server/src/provider/acp/AcpRuntimeModel.ts index 694f761ff8f75..f858b7f345c43 100644 --- a/apps/server/src/provider/acp/AcpRuntimeModel.ts +++ b/apps/server/src/provider/acp/AcpRuntimeModel.ts @@ -10,7 +10,7 @@ import { deriveToolActivityPresentation, mergeToolActivityData, } from "@t3tools/shared/toolActivity"; -import { T3_MCP_TOOL_NAMES } from "@t3tools/shared/t3McpToolPresentation"; +import { T3_MCP_TOOL_NAMES, normalizeT3McpToolLabel } from "@t3tools/shared/t3McpToolPresentation"; import type { OrchestrationV2ProviderThreadNativeMetadata, ThreadTokenUsageSnapshot, @@ -1098,8 +1098,18 @@ export function extractMcpToolCallIdentity( ): AcpMcpToolCallIdentity | undefined { const rawInput = isRecord(toolCall.data.rawInput) ? toolCall.data.rawInput : undefined; const meta = isRecord(toolCall.data.meta) ? toolCall.data.meta : undefined; - const server = typeof rawInput?.server === "string" ? rawInput.server.trim() : ""; - const tool = typeof rawInput?.tool === "string" ? rawInput.tool.trim() : ""; + const server = + typeof rawInput?.server === "string" + ? rawInput.server.trim() + : typeof rawInput?.server_name === "string" + ? rawInput.server_name.trim() + : ""; + const tool = + typeof rawInput?.tool === "string" + ? rawInput.tool.trim() + : typeof rawInput?.tool_name === "string" + ? rawInput.tool_name.trim() + : ""; if (meta?.is_mcp_tool_call === true && server.length > 0 && tool.length > 0) { return { server, tool }; } @@ -1152,10 +1162,14 @@ export function extractMcpToolCallIdentity( meta?.toolName, claudeCode?.toolName, gooseToolCall?.toolName, + // Devin names the inference tool in namespaced meta; CodeBuddy keeps the + // qualified `mcp____` name in rawInput. + meta?.["cognition.ai/inferenceToolName"], + typeof rawInput?.toolName === "string" ? rawInput.toolName : undefined, toolCall.data.title, ].filter((value): value is string => typeof value === "string"); for (const candidate of candidates) { - const trimmed = candidate.trim(); + const trimmed = normalizeT3McpToolLabel(candidate.trim()); const qualified = /^mcp__(.+?)__(.+)$/i.exec(trimmed); if (qualified?.[1] && qualified[2] && !/^t3[-_ ]?code$/i.test(qualified[1])) return { server: qualified[1], tool: qualified[2] }; diff --git a/packages/shared/src/t3McpToolPresentation.test.ts b/packages/shared/src/t3McpToolPresentation.test.ts index 9a66406e68db1..02515baa80d36 100644 --- a/packages/shared/src/t3McpToolPresentation.test.ts +++ b/packages/shared/src/t3McpToolPresentation.test.ts @@ -94,6 +94,14 @@ describe("resolveT3McpToolPresentation", () => { expect(resolveT3McpToolPresentation("t3-code-thread_opencode2-adapter_not_a_tool")).toBeNull(); }); + it("ignores Grok's file-passed source annotation", () => { + expect( + resolveT3McpToolPresentation("t3-code__html_render — source file: /tmp/mcp-render.json") + ?.displayName, + ).toBe("Render an HTML page"); + expect(resolveT3McpToolPresentation("t3-code__not_a_tool — source file: /tmp/x")).toBeNull(); + }); + it("keeps unknown MCP tools on the generic renderer path", () => { expect(resolveT3McpToolPresentation("mcp__github__search_issues")).toBeNull(); expect(resolveT3McpToolPresentation("t3-code.not_a_real_tool")).toBeNull(); diff --git a/packages/shared/src/t3McpToolPresentation.ts b/packages/shared/src/t3McpToolPresentation.ts index 11abf77f704be..a8ddb974c6891 100644 --- a/packages/shared/src/t3McpToolPresentation.ts +++ b/packages/shared/src/t3McpToolPresentation.ts @@ -323,8 +323,15 @@ const T3_MCP_TOOLS: Readonly> = { */ export const T3_MCP_TOOL_NAMES: ReadonlySet = new Set(Object.keys(T3_MCP_TOOLS)); -function normalizeT3McpToolLabel(value: string): string { - return value.replace(/\s+(?:complete|completed)\s*$/i, "").trim(); +/** Strips provider annotations (completion state, Grok file-passed notes) so name matching sees the raw tool label. */ +export function normalizeT3McpToolLabel(value: string): string { + return ( + value + // Grok annotates file-passed calls ("t3-code__html_render — source file: /tmp/x"). + .replace(/\s+—\s+source file:.*$/i, "") + .replace(/\s+(?:complete|completed)\s*$/i, "") + .trim() + ); } /** diff --git a/packages/shared/src/toolOutput.test.ts b/packages/shared/src/toolOutput.test.ts index 20aab7d6a5701..41f332c7d0697 100644 --- a/packages/shared/src/toolOutput.test.ts +++ b/packages/shared/src/toolOutput.test.ts @@ -170,6 +170,21 @@ describe("compactDynamicToolOutput", () => { compactDynamicToolOutput([{ type: "content", content: { type: "text", text: json } }]), ).toEqual({ htmlRender }); }); + + it("extracts htmlRender from a bare MCP text block", () => { + const htmlRender = { + attachmentId: "thread-1-00000000-0000-4000-8000-000000000001-html", + title: "Chart", + height: 420, + }; + const json = JSON.stringify({ htmlRender }); + expect(compactDynamicToolOutput({ type: "text", text: json })).toEqual({ htmlRender }); + // Plain text and unrelated shapes keep their previous readings. + expect(compactDynamicToolOutput({ type: "text", text: "Private plain text" })).toBeUndefined(); + expect(compactDynamicToolOutput({ threadId: "thread-1", text: "Private note" })).toEqual({ + threadId: "thread-1", + }); + }); }); describe("toolOutputIndicatesFailure", () => { diff --git a/packages/shared/src/toolOutput.ts b/packages/shared/src/toolOutput.ts index 41560d480cd92..8b8fcdafdced8 100644 --- a/packages/shared/src/toolOutput.ts +++ b/packages/shared/src/toolOutput.ts @@ -95,6 +95,16 @@ function readResult(value: unknown, budget: ResultReadBudget, depth = 0): Result const nested = readResult(content, budget, depth + 1); return { ...nested, failed: failed || nested.failed }; } + // Bare MCP text blocks (`{ type: "text", text: "" }`) carry the + // result as a string with no envelope keys. Some ACP agents (CodeBuddy) + // persist the block itself, so parse its text but keep the object when + // the text holds no result data. + if (typeof value.text === "string") { + const nested = readResult(value.text, budget, depth + 1); + if (nested.data !== undefined || nested.failed) { + return { ...nested, failed: failed || nested.failed }; + } + } return { data: value, failed }; } From 292d34544dc3b71b926cef10d6fb80bdeb6873ac Mon Sep 17 00:00:00 2001 From: sissedsauce Date: Sun, 11 Oct 2026 08:23:19 -0700 Subject: [PATCH 03/12] fix(shared): isolate bare-text parse budget in tool output reader --- packages/shared/src/toolOutput.test.ts | 4 ++++ packages/shared/src/toolOutput.ts | 13 ++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/shared/src/toolOutput.test.ts b/packages/shared/src/toolOutput.test.ts index 41f332c7d0697..db5602d892feb 100644 --- a/packages/shared/src/toolOutput.test.ts +++ b/packages/shared/src/toolOutput.test.ts @@ -184,6 +184,10 @@ describe("compactDynamicToolOutput", () => { expect(compactDynamicToolOutput({ threadId: "thread-1", text: "Private note" })).toEqual({ threadId: "thread-1", }); + // An oversized text field never invalidates outer metadata. + expect(compactDynamicToolOutput({ threadId: "thread-1", text: "x".repeat(20_000) })).toEqual({ + threadId: "thread-1", + }); }); }); diff --git a/packages/shared/src/toolOutput.ts b/packages/shared/src/toolOutput.ts index 8b8fcdafdced8..a4b201d45b458 100644 --- a/packages/shared/src/toolOutput.ts +++ b/packages/shared/src/toolOutput.ts @@ -98,10 +98,17 @@ function readResult(value: unknown, budget: ResultReadBudget, depth = 0): Result // Bare MCP text blocks (`{ type: "text", text: "" }`) carry the // result as a string with no envelope keys. Some ACP agents (CodeBuddy) // persist the block itself, so parse its text but keep the object when - // the text holds no result data. + // the text holds no result data. The text parses on an isolated budget so + // an oversized text field cannot invalidate outer metadata (e.g. a valid + // threadId) through the shared exceeded flag. if (typeof value.text === "string") { - const nested = readResult(value.text, budget, depth + 1); - if (nested.data !== undefined || nested.failed) { + const textBudget: ResultReadBudget = { + remainingBytes: MAX_PARSED_BYTES, + remainingNodes: MAX_ENVELOPE_NODES, + exceeded: false, + }; + const nested = readResult(value.text, textBudget, depth + 1); + if (!textBudget.exceeded && (nested.data !== undefined || nested.failed)) { return { ...nested, failed: failed || nested.failed }; } } From c0222ea7e34370ff3ea7b67d466125a2e55fc6be Mon Sep 17 00:00:00 2001 From: sissedsauce Date: Sun, 11 Oct 2026 08:26:12 -0700 Subject: [PATCH 04/12] fix(shared): restrict bare-text fallback to typed text blocks --- packages/shared/src/toolOutput.test.ts | 4 ++++ packages/shared/src/toolOutput.ts | 10 ++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/shared/src/toolOutput.test.ts b/packages/shared/src/toolOutput.test.ts index db5602d892feb..9b8b86f36b824 100644 --- a/packages/shared/src/toolOutput.test.ts +++ b/packages/shared/src/toolOutput.test.ts @@ -188,6 +188,10 @@ describe("compactDynamicToolOutput", () => { expect(compactDynamicToolOutput({ threadId: "thread-1", text: "x".repeat(20_000) })).toEqual({ threadId: "thread-1", }); + // Ordinary objects with a JSON text note keep outer metadata. + expect(compactDynamicToolOutput({ threadId: "thread-1", text: '{"a":1}' })).toEqual({ + threadId: "thread-1", + }); }); }); diff --git a/packages/shared/src/toolOutput.ts b/packages/shared/src/toolOutput.ts index a4b201d45b458..37e519331e27b 100644 --- a/packages/shared/src/toolOutput.ts +++ b/packages/shared/src/toolOutput.ts @@ -98,10 +98,12 @@ function readResult(value: unknown, budget: ResultReadBudget, depth = 0): Result // Bare MCP text blocks (`{ type: "text", text: "" }`) carry the // result as a string with no envelope keys. Some ACP agents (CodeBuddy) // persist the block itself, so parse its text but keep the object when - // the text holds no result data. The text parses on an isolated budget so - // an oversized text field cannot invalidate outer metadata (e.g. a valid - // threadId) through the shared exceeded flag. - if (typeof value.text === "string") { + // the text holds no result data. The `type` gate keeps ordinary objects + // that happen to have a text note (e.g. `{ threadId, text: "{...}" }`) + // on the existing path so outer metadata is preserved. The text parses + // on an isolated budget so an oversized text field cannot invalidate + // outer metadata through the shared exceeded flag. + if (value.type === "text" && typeof value.text === "string") { const textBudget: ResultReadBudget = { remainingBytes: MAX_PARSED_BYTES, remainingNodes: MAX_ENVELOPE_NODES, From dcebc1c697dde4f9138fd799febb87c9ca834f6f Mon Sep 17 00:00:00 2001 From: sissedsauce Date: Sun, 11 Oct 2026 08:32:31 -0700 Subject: [PATCH 05/12] fix(shared): keep outer metadata when text block parses empty --- packages/shared/src/toolOutput.test.ts | 4 ++++ packages/shared/src/toolOutput.ts | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/shared/src/toolOutput.test.ts b/packages/shared/src/toolOutput.test.ts index 9b8b86f36b824..3adadfee7739e 100644 --- a/packages/shared/src/toolOutput.test.ts +++ b/packages/shared/src/toolOutput.test.ts @@ -192,6 +192,10 @@ describe("compactDynamicToolOutput", () => { expect(compactDynamicToolOutput({ threadId: "thread-1", text: '{"a":1}' })).toEqual({ threadId: "thread-1", }); + // An empty JSON text block carries no result: outer metadata wins. + expect(compactDynamicToolOutput({ type: "text", threadId: "thread-1", text: "{}" })).toEqual({ + threadId: "thread-1", + }); }); }); diff --git a/packages/shared/src/toolOutput.ts b/packages/shared/src/toolOutput.ts index 37e519331e27b..d3c0134459d07 100644 --- a/packages/shared/src/toolOutput.ts +++ b/packages/shared/src/toolOutput.ts @@ -110,7 +110,10 @@ function readResult(value: unknown, budget: ResultReadBudget, depth = 0): Result exceeded: false, }; const nested = readResult(value.text, textBudget, depth + 1); - if (!textBudget.exceeded && (nested.data !== undefined || nested.failed)) { + // An empty parse (e.g. text "{}") carries no result: fall through so + // outer metadata such as threadId is preserved. + const nestedHasData = nested.data !== undefined && Object.keys(nested.data).length > 0; + if (!textBudget.exceeded && (nestedHasData || nested.failed)) { return { ...nested, failed: failed || nested.failed }; } } From bc084fe3a30caef6c08512a6f7242fe1026b3c64 Mon Sep 17 00:00:00 2001 From: sissedsauce Date: Mon, 12 Oct 2026 23:46:30 -0700 Subject: [PATCH 06/12] fix(shared): read a bare html_render reference an execute script returned A Code Mode script can return out.htmlRender instead of the whole html_render result. Accept that bare reference when its id carries the -html suffix html_render mints, so unrelated objects are not taken for pages. Co-Authored-By: Claude Opus 5.5 --- packages/shared/src/htmlRender.test.ts | 16 ++++++++++++++++ packages/shared/src/toolOutput.ts | 14 +++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/packages/shared/src/htmlRender.test.ts b/packages/shared/src/htmlRender.test.ts index c563e9cf7760e..faa52f8475ba2 100644 --- a/packages/shared/src/htmlRender.test.ts +++ b/packages/shared/src/htmlRender.test.ts @@ -187,6 +187,22 @@ describe("htmlRenderFromToolItem", () => { const output = [{ type: "text", text: JSON.stringify({ htmlRender: executeRender }) }]; expect(htmlRenderFromToolItem({ toolName: "execute", output })).toEqual(executeRender); }); + + it("reads a bare reference an execute script returned", () => { + const executeRender = { + attachmentId: "thread-1-00000000-0000-4000-8000-000000000001-html", + title: "Chart", + height: 420, + }; + const output = [{ type: "text", text: JSON.stringify(executeRender) }]; + expect(htmlRenderFromToolItem({ toolName: "execute", output })).toEqual(executeRender); + expect( + htmlRenderFromToolItem({ + toolName: "execute", + output: { attachmentId: "thread-1-upload", title: "Chart", height: 420 }, + }), + ).toBeUndefined(); + }); }); describe("htmlRenderFrameHeight", () => { diff --git a/packages/shared/src/toolOutput.ts b/packages/shared/src/toolOutput.ts index d3c0134459d07..f9a065d3e0c72 100644 --- a/packages/shared/src/toolOutput.ts +++ b/packages/shared/src/toolOutput.ts @@ -127,6 +127,17 @@ function boundedId(value: unknown): string | undefined { return Array.from(value).join(""); } +/** + * A Code Mode script can return `html_render`'s reference on its own + * (`return out.htmlRender`). Only ids `html_render` mints end in `-html`, so + * an unrelated object with the same fields is not taken for a page. + */ +function readBareHtmlRenderReference(data: Record) { + return typeof data.attachmentId === "string" && data.attachmentId.endsWith("-html") + ? readHtmlRenderReference(data) + : undefined; +} + /** Keeps only IDs and failure metadata used by T3's grouped tool summaries. */ export function compactDynamicToolOutput(value: unknown): CompactToolOutput | undefined { const budget: ResultReadBudget = { @@ -143,7 +154,8 @@ export function compactDynamicToolOutput(value: unknown): CompactToolOutput | un if (id !== undefined) output[key] = id; } if (data.status === "rolled_back") output.status = "rolled_back"; - const htmlRender = readHtmlRenderReference(data.htmlRender); + const htmlRender = + readHtmlRenderReference(data.htmlRender) ?? readBareHtmlRenderReference(data); if (htmlRender !== undefined) output.htmlRender = htmlRender; const mcpApp = readMcpAppReference(data[MCP_APP_OUTPUT_KEY]); // Kept under its stored key, so the compact wire output reads back the same way. From c049ef386bf6c3fde2c8247c657accc16b793fa5 Mon Sep 17 00:00:00 2001 From: sissedsauce Date: Tue, 13 Oct 2026 00:08:36 -0700 Subject: [PATCH 07/12] fix(server): OpenCode 2 keeps html_render out of Code Mode so every page shows Code Mode reports a nested call only as its execute script, without the call's result, so a page html_render published there never reached the thread unless the script happened to return it. Each thread now also registers a t3-code--html MCP entry with Code Mode off, and the session rules allow html_render only there. Every call becomes its own tool part carrying the result, which clients already show inline and thread deletion already cleans up. The other T3 tools stay in Code Mode. Co-Authored-By: Claude Opus 5.5 --- .../Adapters/OpenCode2AdapterV2.test.ts | 214 ++++++++++++++---- ...penCode2OrchestratorV2.integration.test.ts | 18 +- .../opencode2_fork/opencode_transcript.ndjson | 2 +- .../opencode_transcript.ndjson | 2 +- docs/internals/providers.md | 7 +- .../src/server/v2/adapter.ts | 118 +++++++--- packages/provider-opencode/src/testing.ts | 6 +- packages/shared/src/htmlRender.test.ts | 5 + 8 files changed, 289 insertions(+), 83 deletions(-) diff --git a/apps/server/src/orchestration-v2/Adapters/OpenCode2AdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/OpenCode2AdapterV2.test.ts index 548ff5cb6643b..7d7ec412632be 100644 --- a/apps/server/src/orchestration-v2/Adapters/OpenCode2AdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/OpenCode2AdapterV2.test.ts @@ -23,6 +23,7 @@ import { type OrchestrationV2ProviderTurn, type ProviderReplayEntry, } from "@t3tools/contracts"; +import { htmlRenderFromToolItem } from "@t3tools/shared/toolOutput"; import * as Cause from "effect/Cause"; import * as DateTime from "effect/DateTime"; import * as Deferred from "effect/Deferred"; @@ -45,6 +46,7 @@ import * as ProviderContinuationRequests from "@t3tools/provider-core/server/Pro import { OPENCODE_2_STILL_STOPPING, OPENCODE_PROVIDER, + t3McpHtmlServerName, t3McpServerName, } from "@t3tools/provider-opencode/testing"; import { openCode2ReplayRuntime } from "./OpenCode2AdapterV2.testkit.ts"; @@ -83,12 +85,40 @@ const event = ( }); const durable = { durable: { aggregateID: SESSION, seq: 1, version: 1 } }; -/** The rules T3 gives every session it runs, with only this thread's own T3 MCP server allowed. */ -const mcpRules = [ +/** + * The rules that allow a thread its own two T3 MCP servers: every tool through + * the Code Mode one except `html_render`, which only the html one serves. + */ +const t3ServerRules = (codeMode: string, html: string) => [ { action: "t3-code-*", resource: "*", effect: "deny" }, - { action: "t3-code-thread_opencode2-adapter_*", resource: "*", effect: "allow" }, + { action: `${codeMode}_*`, resource: "*", effect: "allow" }, + { action: `${codeMode}_html_render`, resource: "*", effect: "deny" }, + { action: `${html}_html_render`, resource: "*", effect: "allow" }, ]; +/** The rules T3 gives every session it runs, with only this thread's own T3 MCP servers allowed. */ +const mcpRules = t3ServerRules("t3-code-thread_opencode2-adapter", "t3-code-c26f2f1fb6fbbcc2-html"); const t3Rules = [{ action: "*", resource: "*", effect: "allow" }, ...mcpRules]; +/** Adds a thread's T3 MCP servers: the Code Mode one, then the html one with Code Mode off. */ +const registerT3Servers = (server: string, htmlServer: string) => + [server, htmlServer].flatMap((name) => [ + out("mcp.add", { + server: name, + "location[directory]": WORK, + config: { + type: "remote", + url: "http://127.0.0.1:3773/mcp", + headers: { Authorization: "Bearer thread-credential" }, + oauth: false, + ...(name === htmlServer ? { codemode: false } : {}), + }, + }), + reply("mcp.add", null), + ]); +const removeT3Servers = (server: string, htmlServer: string) => + [server, htmlServer].flatMap((name) => [ + out("mcp.remove", { server: name, "location[directory]": WORK }), + reply("mcp.remove", null), + ]); /** The thread's title, which the recorded session already carries. */ const TITLE = "Prime check"; const sessionInfo = (overrides: Record = {}) => ({ @@ -167,15 +197,15 @@ const withInstructions = ( "skill.list", ].includes(String(entry.frame.type)), ); - // T3's MCP server is added before the entry that describes it. - const after = entries.findIndex( + // T3's MCP servers are added (or, when that fails, removed) before the entry that describes them. + const after = entries.findLastIndex( (entry, index) => index < first && entry.type === "emit_inbound" && typeof entry.frame === "object" && entry.frame !== null && "operation" in entry.frame && - entry.frame.operation === "mcp.add", + (entry.frame.operation === "mcp.add" || entry.frame.operation === "mcp.remove"), ); const at = after < 0 ? first : after + 1; return first < 0 @@ -3415,25 +3445,15 @@ it.layer(McpProviderSessions.layer)("OpenCode2 adapter", (it) => { }); yield* Effect.addFinalizer(() => mcpSessions.clear(threadId)); const server = "t3-code-thread_opencode2-adapter"; + const htmlServer = "t3-code-c26f2f1fb6fbbcc2-html"; const { runtime, thread } = yield* resumed([ - // Registered for the session's directory under the thread's own name; - // the session's rules allow only this name's tools (see `t3Rules`). - out("mcp.add", { - server, - "location[directory]": WORK, - config: { - type: "remote", - url: "http://127.0.0.1:3773/mcp", - headers: { Authorization: "Bearer thread-credential" }, - oauth: false, - }, - }), - reply("mcp.add", null), + // Registered for the session's directory under the thread's own names; + // the session's rules allow only these names' tools (see `t3Rules`). + ...registerT3Servers(server, htmlServer), out("session.prompt", { sessionID: SESSION, text: "" }), promptAccepted, event("session.execution.succeeded", { sessionID: SESSION }), - out("mcp.remove", { server, "location[directory]": WORK }), - reply("mcp.remove", null), + ...removeT3Servers(server, htmlServer), ]); const terminal = yield* terminalOf(runtime).pipe(Effect.forkScoped); yield* runtime.startTurn(turnInput(thread)); @@ -3442,6 +3462,126 @@ it.layer(McpProviderSessions.layer)("OpenCode2 adapter", (it) => { }).pipe(Effect.scoped), ); + it.effect("shows a page html_render published through the thread's html server", () => + Effect.gen(function* () { + const mcpSessions = yield* McpProviderSessions.McpProviderSessions; + yield* mcpSessions.set({ + environmentId: EnvironmentId.make("environment:opencode2-adapter"), + threadId, + providerSessionId: "mcp:opencode2-adapter", + providerInstanceId: instanceId, + endpoint: "http://127.0.0.1:3773/mcp", + authorizationHeader: "Bearer thread-credential", + browserToolsAvailable: false, + }); + yield* Effect.addFinalizer(() => mcpSessions.clear(threadId)); + const server = "t3-code-thread_opencode2-adapter"; + const htmlServer = "t3-code-c26f2f1fb6fbbcc2-html"; + const page = { + attachmentId: "thread_opencode2-adapter-00000000-0000-4000-8000-000000000001-html", + title: "Chart", + height: 420, + }; + // With Code Mode off for the html server, the call is its own tool part + // carrying html_render's result, not a line inside an `execute` script. + const tool = { sessionID: SESSION, assistantMessageID: "msg_assistant", id: "call-render" }; + const name = `${htmlServer}_html_render`; + const { runtime, thread } = yield* resumed([ + ...registerT3Servers(server, htmlServer), + out("session.prompt", { sessionID: SESSION, text: "" }), + promptAccepted, + event("session.tool.input.started", { ...tool, name }), + event("session.tool.called", { + ...tool, + name, + input: { html: "

Chart

", title: "Chart", height: 420 }, + executed: false, + }), + // `session.tool.success` is at version 2. + { + type: "emit_inbound", + frame: { + type: "sdk.event", + event: { + id: "evt_toolsuccess_render", + created: 1, + type: "session.tool.success", + data: { + ...tool, + content: [ + { type: "text", text: JSON.stringify({ htmlRender: page, message: "Shown" }) }, + ], + metadata: { truncated: false }, + executed: false, + }, + durable: { aggregateID: SESSION, seq: 1, version: 2 }, + }, + }, + }, + event("session.execution.succeeded", { sessionID: SESSION }), + ...removeT3Servers(server, htmlServer), + ]); + const events = yield* runtime.events.pipe( + Stream.takeUntil((event) => event.type === "turn.terminal"), + Stream.runCollect, + Effect.forkScoped, + ); + yield* runtime.startTurn(turnInput(thread)); + const collected = yield* Fiber.join(events); + assert.deepInclude(collected.at(-1), { type: "turn.terminal", status: "completed" }); + const rendered = collected.findLast( + (event) => + event.type === "turn_item.updated" && + event.turnItem.type === "dynamic_tool" && + event.turnItem.status === "completed", + ); + assert.deepEqual( + rendered?.type === "turn_item.updated" && rendered.turnItem.type === "dynamic_tool" + ? htmlRenderFromToolItem(rendered.turnItem) + : undefined, + page, + ); + yield* runtime.unloadThread!({ providerThread: thread }); + }).pipe(Effect.scoped), + ); + + it.effect("removes the Code Mode server again when the html server cannot be added", () => + Effect.gen(function* () { + const mcpSessions = yield* McpProviderSessions.McpProviderSessions; + yield* mcpSessions.set({ + environmentId: EnvironmentId.make("environment:opencode2-adapter"), + threadId, + providerSessionId: "mcp:opencode2-adapter", + providerInstanceId: instanceId, + endpoint: "http://127.0.0.1:3773/mcp", + authorizationHeader: "Bearer thread-credential", + browserToolsAvailable: false, + }); + yield* Effect.addFinalizer(() => mcpSessions.clear(threadId)); + const server = "t3-code-thread_opencode2-adapter"; + const htmlServer = "t3-code-c26f2f1fb6fbbcc2-html"; + const [codeModeAdd, codeModeAdded, htmlAdd] = registerT3Servers(server, htmlServer); + const { runtime, thread } = yield* resumed([ + codeModeAdd!, + codeModeAdded!, + htmlAdd!, + reply("mcp.add", { status: 500, body: { message: "boom" } }), + // The rules deny html_render through Code Mode, so the Code Mode server + // alone would leave no way to call it: neither stays, and the turn runs + // without T3's tools. + ...removeT3Servers(server, htmlServer), + out("session.prompt", { sessionID: SESSION, text: "" }), + promptAccepted, + event("session.execution.succeeded", { sessionID: SESSION }), + ]); + const terminal = yield* terminalOf(runtime).pipe(Effect.forkScoped); + yield* runtime.startTurn(turnInput(thread)); + assert.equal((yield* Fiber.join(terminal))?.status, "completed"); + // Nothing is registered, so unloading removes nothing. + yield* runtime.unloadThread!({ providerThread: thread }); + }).pipe(Effect.scoped), + ); + it.effect("registers a long thread id's MCP server under a name OpenCode accepts", () => Effect.gen(function* () { // Spelled out, this delegated thread's server name would be 120 characters. @@ -3469,28 +3609,16 @@ it.layer(McpProviderSessions.layer)("OpenCode2 adapter", (it) => { sessionInfo({ permissions: [ { action: "*", resource: "*", effect: "allow" }, - { action: "t3-code-*", resource: "*", effect: "deny" }, - { action: `${server}_*`, resource: "*", effect: "allow" }, + ...t3ServerRules(server, `${server}-html`), ], }), ), ...noOpenRequests, - out("mcp.add", { - server, - "location[directory]": WORK, - config: { - type: "remote", - url: "http://127.0.0.1:3773/mcp", - headers: { Authorization: "Bearer thread-credential" }, - oauth: false, - }, - }), - reply("mcp.add", null), + ...registerT3Servers(server, `${server}-html`), out("session.prompt", { sessionID: SESSION, text: "" }), promptAccepted, event("session.execution.succeeded", { sessionID: SESSION }), - out("mcp.remove", { server, "location[directory]": WORK }), - reply("mcp.remove", null), + ...removeT3Servers(server, `${server}-html`), ]); const thread = yield* runtime.resumeThread({ providerThread: { ...providerThread(yield* DateTime.now), appThreadId: child }, @@ -3522,6 +3650,14 @@ it.layer(McpProviderSessions.layer)("OpenCode2 adapter", (it) => { assert.equal(names[0], "t3-code-63abb5df2b188bdd"); // A name that already fits stays readable. assert.equal(yield* t3McpServerName(threadId), "t3-code-thread_opencode2-adapter"); + // The html server's tool keeps within model APIs' 64-character tool names. + const htmlNames = yield* Effect.forEach([threadId, ...ids], t3McpHtmlServerName); + for (const name of htmlNames) { + assert.match(name, /^t3-code-[0-9a-f]{16}-html$/); + assert.isAtMost(`${name}_html_render`.length, 64); + } + assert.equal(new Set(htmlNames).size, htmlNames.length); + assert.equal(htmlNames[0], "t3-code-c26f2f1fb6fbbcc2-html"); }).pipe(Effect.provide(NodeCrypto.layer)), ); @@ -4418,8 +4554,10 @@ it.layer(McpProviderSessions.layer)("OpenCode2 adapter", (it) => { sessionID: FORK, permissions: [ { action: "*", resource: "*", effect: "allow" }, - { action: "t3-code-*", resource: "*", effect: "deny" }, - { action: "t3-code-thread_opencode2-adapter_fork_*", resource: "*", effect: "allow" }, + ...t3ServerRules( + "t3-code-thread_opencode2-adapter_fork", + "t3-code-7c81f4ba96860743-html", + ), ], }), reply("session.update", null), diff --git a/apps/server/src/orchestration-v2/OpenCode2OrchestratorV2.integration.test.ts b/apps/server/src/orchestration-v2/OpenCode2OrchestratorV2.integration.test.ts index 3f873bfb6ea36..88d08062b88e0 100644 --- a/apps/server/src/orchestration-v2/OpenCode2OrchestratorV2.integration.test.ts +++ b/apps/server/src/orchestration-v2/OpenCode2OrchestratorV2.integration.test.ts @@ -1,8 +1,10 @@ +// @effect-diagnostics nodeBuiltinImport:off - expected rules name each thread's html server by a digest of its id. /** * OpenCode 2 through the whole orchestrator, against a replayed HTTP server: * the transcript fixes the order of every request the adapter sends, so a * request the orchestrator never lets it make fails the run. */ +import * as NodeCrypto from "node:crypto"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, describe, it } from "@effect/vitest"; import { @@ -73,12 +75,18 @@ const labelled = (entry: ProviderReplayEntry, label: string): ProviderReplayEntr entry.type === "runtime_exit" ? entry : { ...entry, label }; /** * T3's own rules after a mode's: every thread's T3 MCP server is denied, and - * then this thread's own is allowed again (last match wins). + * then this thread's own are allowed again (last match wins), with + * `html_render` served only by the html one. */ -const mcpRules = (name: string) => [ - { action: "t3-code-*", resource: "*", effect: "deny" }, - { action: `t3-code-thread_${name}_*`, resource: "*", effect: "allow" }, -]; +const mcpRules = (name: string) => { + const html = `t3-code-${NodeCrypto.createHash("sha256").update(`thread:${name}`).digest("hex").slice(0, 16)}-html`; + return [ + { action: "t3-code-*", resource: "*", effect: "deny" }, + { action: `t3-code-thread_${name}_*`, resource: "*", effect: "allow" }, + { action: `t3-code-thread_${name}_html_render`, resource: "*", effect: "deny" }, + { action: `${html}_html_render`, resource: "*", effect: "allow" }, + ]; +}; const FULL_ACCESS = [{ action: "*", resource: "*", effect: "allow" }]; /** Full access for the thread named `name`. */ const t3Rules = (name: string) => [...FULL_ACCESS, ...mcpRules(name)]; diff --git a/apps/server/src/orchestration-v2/testkit/fixtures/opencode2_fork/opencode_transcript.ndjson b/apps/server/src/orchestration-v2/testkit/fixtures/opencode2_fork/opencode_transcript.ndjson index 57cbd14853460..f81188db07236 100644 --- a/apps/server/src/orchestration-v2/testkit/fixtures/opencode2_fork/opencode_transcript.ndjson +++ b/apps/server/src/orchestration-v2/testkit/fixtures/opencode2_fork/opencode_transcript.ndjson @@ -47,7 +47,7 @@ {"type": "emit_inbound", "label": "message.list.response.2", "frame": {"type": "sdk.response", "operation": "message.list", "data": {"data": [], "cursor": {}}}} {"type": "expect_outbound", "label": "session.fork", "frame": {"type": "session.fork", "input": {"sessionID": "ses_f1484ea8affe5aTo8CcgbGIszj", "before": "msg_0eb7b1f9f001g95Bduh3bi8syS"}}} {"type": "emit_inbound", "label": "session.fork.response", "frame": {"type": "sdk.response", "operation": "session.fork", "data": {"data": {"id": "ses_f1484db83ffeLGtrRCFimo1H0e", "fork": {"sessionID": "ses_f1484ea8affe5aTo8CcgbGIszj", "boundary": {"type": "before", "messageID": "msg_0eb7b1f9f001g95Bduh3bi8syS"}}, "projectID": "global", "model": {"id": "big-pickle", "providerID": "opencode", "variant": "default"}, "cost": 0, "tokens": {"input": 0, "output": 0, "reasoning": 0, "cache": {"read": 0, "write": 0}}, "time": {"created": 1790657111165, "updated": 1790657111165}, "title": "spike fork (fork #1)", "location": {"directory": ""}, "permissions": [{"action": "*", "resource": "*", "effect": "allow"}, {"action": "subagent", "resource": "*", "effect": "deny"}]}}}} -{"type": "expect_outbound", "label": "session.update", "frame": {"type": "session.update", "input": {"sessionID": "ses_f1484db83ffeLGtrRCFimo1H0e", "permissions": [{"action": "*", "resource": "*", "effect": "allow"}, {"action": "t3-code-*", "resource": "*", "effect": "deny"}, {"action": "t3-code-thread_opencode2-fork_target_*", "resource": "*", "effect": "allow"}]}}} +{"type": "expect_outbound", "label": "session.update", "frame": {"type": "session.update", "input": {"sessionID": "ses_f1484db83ffeLGtrRCFimo1H0e", "permissions": [{"action": "*", "resource": "*", "effect": "allow"}, {"action": "t3-code-*", "resource": "*", "effect": "deny"}, {"action": "t3-code-thread_opencode2-fork_target_*", "resource": "*", "effect": "allow"}, {"action": "t3-code-thread_opencode2-fork_target_html_render", "resource": "*", "effect": "deny"}, {"action": "t3-code-f74b9d2803b51629-html_html_render", "resource": "*", "effect": "allow"}]}}} {"type": "emit_inbound", "label": "session.update.response", "frame": {"type": "sdk.response", "operation": "session.update", "data": null}} {"type": "emit_inbound", "label": "session.forked", "frame": {"type": "sdk.event", "event": {"id": "evt_0eb7b247d001fXzFroPg5P0bps", "created": 1790657111165, "type": "session.forked", "location": {"directory": ""}, "data": {"sessionID": "ses_f1484db83ffeLGtrRCFimo1H0e", "parentID": "ses_f1484ea8affe5aTo8CcgbGIszj", "boundary": {"type": "before", "messageID": "msg_0eb7b1f9f001g95Bduh3bi8syS"}, "instructions": {"core/environment": "db4a793a05fec32fdfa985a3432125a86d6cb42debc8b9ee8f6a99fbfcf2f93b", "core/date": "bf08b8486572ab3eef677a3c9ee12468e411ae41a8f4d2fb6cf02a6ca73f1986", "core/codemode": "616a7ad8b8e40e8a71eab238dfbbe9eaf0f4a9e6fdad853226d075bfe602e1e6", "core/skill-guidance": "0d001919496102654717308c72635523bf7c3cd99c81e058cc79f2947104f9a5"}, "instructionEntries": []}, "durable": {"aggregateID": "ses_f1484db83ffeLGtrRCFimo1H0e", "seq": 0, "version": 2}}}} {"type": "expect_outbound", "label": "session.update.title", "frame": {"type": "session.update", "input": {"sessionID": "ses_f1484db83ffeLGtrRCFimo1H0e", "title": ""}}} diff --git a/apps/server/src/orchestration-v2/testkit/fixtures/opencode2_switch/opencode_transcript.ndjson b/apps/server/src/orchestration-v2/testkit/fixtures/opencode2_switch/opencode_transcript.ndjson index ee2519419915a..7099c2afa7c99 100644 --- a/apps/server/src/orchestration-v2/testkit/fixtures/opencode2_switch/opencode_transcript.ndjson +++ b/apps/server/src/orchestration-v2/testkit/fixtures/opencode2_switch/opencode_transcript.ndjson @@ -53,7 +53,7 @@ {"type": "emit_inbound", "label": "session.execution.succeeded", "frame": {"type": "sdk.event", "event": {"id": "evt_0eb7eef75001z9Sy13uGDVqV3F", "created": 1790657359733, "type": "session.execution.succeeded", "data": {"sessionID": "ses_f14812197ffeCbw641wM2r3ZmP"}, "durable": {"aggregateID": "ses_f14812197ffeCbw641wM2r3ZmP", "seq": 15, "version": 1}}}} {"type": "expect_outbound", "label": "session.get", "frame": {"type": "session.get", "input": {"sessionID": "ses_f14812197ffeCbw641wM2r3ZmP"}}} {"type": "emit_inbound", "label": "session.get.response", "frame": {"type": "sdk.response", "operation": "session.get", "data": {"data": {"id": "ses_f14812197ffeCbw641wM2r3ZmP", "projectID": "global", "agent": "plan", "model": {"id": "big-pickle", "providerID": "opencode", "variant": "default"}, "cost": 0, "tokens": {"input": 8910, "output": 271, "reasoning": 0, "cache": {"read": 384, "write": 0}}, "outcome": "succeeded", "time": {"created": 1790657355410, "updated": 1790657355463}, "title": "spike switch", "location": {"directory": ""}, "permissions": [{"action": "*", "resource": "*", "effect": "allow"}, {"action": "subagent", "resource": "*", "effect": "deny"}]}}}} -{"type": "expect_outbound", "label": "session.update", "frame": {"type": "session.update", "input": {"sessionID": "ses_f14812197ffeCbw641wM2r3ZmP", "permissions": [{"action": "*", "resource": "*", "effect": "allow"}, {"action": "t3-code-*", "resource": "*", "effect": "deny"}, {"action": "t3-code-thread_opencode2_switch_*", "resource": "*", "effect": "allow"}]}}} +{"type": "expect_outbound", "label": "session.update", "frame": {"type": "session.update", "input": {"sessionID": "ses_f14812197ffeCbw641wM2r3ZmP", "permissions": [{"action": "*", "resource": "*", "effect": "allow"}, {"action": "t3-code-*", "resource": "*", "effect": "deny"}, {"action": "t3-code-thread_opencode2_switch_*", "resource": "*", "effect": "allow"}, {"action": "t3-code-thread_opencode2_switch_html_render", "resource": "*", "effect": "deny"}, {"action": "t3-code-421cb593d6fcf559-html_html_render", "resource": "*", "effect": "allow"}]}}} {"type": "emit_inbound", "label": "session.update.response", "frame": {"type": "sdk.response", "operation": "session.update", "data": null}} {"type": "expect_outbound", "label": "session.switchAgent.2", "frame": {"type": "session.switchAgent", "input": {"sessionID": "ses_f14812197ffeCbw641wM2r3ZmP", "agent": "build"}}} {"type": "emit_inbound", "label": "session.switchAgent.response.2", "frame": {"type": "sdk.response", "operation": "session.switchAgent", "data": null}} diff --git a/docs/internals/providers.md b/docs/internals/providers.md index 5f9642c2db27d..1a1156b9ac863 100644 --- a/docs/internals/providers.md +++ b/docs/internals/providers.md @@ -22,7 +22,12 @@ directory must not share one T3 MCP entry. - **2.x** serves every directory from one [server per instance](../../packages/provider-opencode/src/server/v2/OpenCode2Server.ts). Each thread registers its own `t3-code-` MCP entry, and session permission rules deny every other - thread's entry. See the [2.x adapter](../../packages/provider-opencode/src/server/v2/adapter.ts). + thread's entry. OpenCode's Code Mode runs MCP tools inside one `execute` call and reports nested + calls without their results, so a page `html_render` published from there would never reach the + thread. Each thread therefore also registers a `t3-code--html` entry with Code Mode off, + and its rules allow `html_render` only there. Every other T3 tool stays in Code Mode, because + exposing all of them directly would add every tool schema to every request. + See the [2.x adapter](../../packages/provider-opencode/src/server/v2/adapter.ts). External OpenCode servers remain externally owned and can require an external restart to pick up configuration changes. OpenCode stores "always" approval grants for the whole project. Automatic diff --git a/packages/provider-opencode/src/server/v2/adapter.ts b/packages/provider-opencode/src/server/v2/adapter.ts index 785bf1a06e70c..8b0f40afcd30a 100644 --- a/packages/provider-opencode/src/server/v2/adapter.ts +++ b/packages/provider-opencode/src/server/v2/adapter.ts @@ -79,6 +79,7 @@ import * as McpProviderSessions from "@t3tools/provider-core/server/McpProviderS import { buildRuntimeInstructions } from "@t3tools/provider-core/server/runtimeInstructions"; import { t3OrchestrationSystemPrompt } from "@t3tools/provider-core/server/orchestrationInstructions"; import { SKILL_MENTION_PATTERN } from "@t3tools/shared/composerInlineTokens"; +import { HTML_RENDER_TOOL_NAME } from "@t3tools/shared/htmlRender"; import * as KeyedLock from "@t3tools/shared/KeyedLock"; import { getModelSelectionStringOptionValue, modelSelectionsEqual } from "@t3tools/shared/model"; import { causeErrorTag } from "@t3tools/shared/observability"; @@ -414,9 +415,13 @@ interface ThreadState { * inbox, where the next prompt would deliver them first, so it cancels them. */ readonly strandedSteers: Set; - /** T3's MCP server as registered for this thread, and the instructions entry sent with it. */ + /** T3's MCP servers as registered for this thread, and the instructions entry sent with it. */ mcp: - | { readonly name: string; readonly directory: string; readonly credential: string } + | { + readonly servers: T3McpServers; + readonly directory: string; + readonly credential: string; + } | undefined; instructions: string | undefined; } @@ -475,32 +480,59 @@ const rule = (action: string, effect: Rule["effect"]): Rule => ({ action, resour export const t3McpServerName = Effect.fn("t3McpServerName")(function* (threadId: string) { const name = `t3-code-${threadId.replaceAll(/[^a-zA-Z0-9_-]/g, "_")}`; if (name.length <= 64) return name; + return `t3-code-${yield* threadDigest(threadId)}`; +}); + +/** + * The thread's second T3 server, registered with Code Mode off so that + * `html_render` stays a direct tool (see `mcpRules`). The model calls it as + * `_html_render`, and model APIs cap tool names at 64 characters, so + * it always takes the digest form. + */ +export const t3McpHtmlServerName = Effect.fn("t3McpHtmlServerName")(function* (threadId: string) { + return `t3-code-${yield* threadDigest(threadId)}-html`; +}); + +const threadDigest = Effect.fnUntraced(function* (threadId: string) { const crypto = yield* Crypto.Crypto; const digest = yield* crypto .digest("SHA-256", new TextEncoder().encode(threadId)) .pipe(Effect.orDie); - return `t3-code-${Hex.encode(digest).slice(0, 16)}`; + return Hex.encode(digest).slice(0, 16); }); +/** A thread's T3 servers: every tool through Code Mode, and `html_render` alone directly. */ +interface T3McpServers { + readonly codeMode: string; + readonly html: string; +} + /** * The rules that keep T3's MCP servers to their own thread, after the mode's: * the last matching rule wins, so every thread's T3 server is denied and then * this thread's own is allowed again, in every mode. A subagent's session * inherits the thread's. + * + * Code Mode reports a nested call only as its `execute` script, without the + * call's result, so a page `html_render` published from there never reached + * the thread. `html_render` is therefore taken out of the Code Mode server and + * allowed only on the html server, where each call is its own tool part. */ -const mcpRules = (mcpServerName: string | null): ReadonlyArray => - mcpServerName === null +const mcpRules = (servers: T3McpServers | null): ReadonlyArray => + servers === null ? [] : [ { action: "t3-code-*", resource: "*", effect: "deny" }, - { action: `${mcpServerName}_*`, resource: "*", effect: "allow" }, + { action: `${servers.codeMode}_*`, resource: "*", effect: "allow" }, + { action: `${servers.codeMode}_${HTML_RENDER_TOOL_NAME}`, resource: "*", effect: "deny" }, + { action: `${servers.html}_${HTML_RENDER_TOOL_NAME}`, resource: "*", effect: "allow" }, ]; const sessionRules = ( policy: RulesPolicy, paths: ReadonlyArray, grants: ReadonlyArray, - mcpServerName: string | null, + mcpServers: T3McpServers | null, ): ReadonlyArray => [ ...(policy.runtimeMode === "full-access" ? [rule("*", "allow")] @@ -514,7 +546,7 @@ const sessionRules = ( // are never denied: the free tier refuses sessions whose rules deny them. ...(policy.interactionMode === "plan" ? [rule("edit", "deny")] : []), ...paths, - ...mcpRules(mcpServerName), + ...mcpRules(mcpServers), ]; const sameRules = (left: ReadonlyArray | undefined, right: ReadonlyArray) => @@ -853,8 +885,11 @@ export const make = Effect.fn("OpenCode2Adapter.make")(function* (instanceId: Pr const continuationRequests = yield* ProviderContinuationRequests.ProviderContinuationRequests; const crypto = yield* Crypto.Crypto; const driver = OPENCODE_PROVIDER; - const mcpServerNameFor = (threadId: string) => - t3McpServerName(threadId).pipe(Effect.provideService(Crypto.Crypto, crypto)); + const mcpServersFor = (threadId: string) => + Effect.all({ + codeMode: t3McpServerName(threadId), + html: t3McpHtmlServerName(threadId), + }).pipe(Effect.provideService(Crypto.Crypto, crypto)); /** * Lends the instance's server to a session until its scope closes. A spawned @@ -3101,7 +3136,7 @@ export const make = Effect.fn("OpenCode2Adapter.make")(function* (instanceId: Pr policy, paths, policy.runtimeMode === "full-access" ? [] : thread.grants, - appThreadId === null ? null : yield* mcpServerNameFor(appThreadId), + appThreadId === null ? null : yield* mcpServersFor(appThreadId), ); }); @@ -3343,10 +3378,15 @@ export const make = Effect.fn("OpenCode2Adapter.make")(function* (instanceId: Pr resolveAttachmentPath: host.resolveAttachmentPath, }).trim(); - const removeMcp = (mcp: { readonly name: string; readonly directory: string }) => - Effect.suspend(() => - client.mcp.remove({ server: mcp.name, location: { directory: mcp.directory } }), - ).pipe(Effect.timeout("5 seconds"), Effect.ignore({ log: true })); + const removeMcp = (mcp: { readonly servers: T3McpServers; readonly directory: string }) => + Effect.forEach( + [mcp.servers.codeMode, mcp.servers.html], + (server) => + Effect.suspend(() => + client.mcp.remove({ server, location: { directory: mcp.directory } }), + ).pipe(Effect.timeout("5 seconds"), Effect.ignore({ log: true })), + { discard: true }, + ); // T3's MCP registrations outlive a session only on an external server; a // spawned one forgets them when it stops. @@ -3373,12 +3413,12 @@ export const make = Effect.fn("OpenCode2Adapter.make")(function* (instanceId: Pr ) { const mcpSession = yield* mcpSessions.read(turnInput.threadId); const directory = turnInput.runtimePolicy.cwd ?? host.paths.cwd; - const name = yield* mcpServerNameFor(turnInput.threadId); + const servers = yield* mcpServersFor(turnInput.threadId); // An external server may not reach T3's MCP endpoint, as with 1.x. const wanted = mcpSession === undefined || connection.external ? undefined - : { name, directory, credential: mcpSession.authorizationHeader }; + : { servers, directory, credential: mcpSession.authorizationHeader }; if ( state.mcp !== undefined && (wanted === undefined || @@ -3389,27 +3429,33 @@ export const make = Effect.fn("OpenCode2Adapter.make")(function* (instanceId: Pr state.mcp = undefined; } // T3's tools are an addition: a server that cannot add them still runs the turn. + // The two servers are added as one: the rules deny `html_render` through + // Code Mode, so the Code Mode server alone would leave no way to call it. if (wanted !== undefined && state.mcp === undefined) { - const added = yield* client.mcp - .add({ - server: name, - location: { directory }, - config: new Mcp.RemoteConfig({ - type: "remote", - url: mcpSession!.endpoint, - headers: { Authorization: wanted.credential }, - oauth: false, - }), - }) - .pipe( - Effect.timeout(INVENTORY_TIMEOUT), - Effect.as(true), - Effect.catchCause((cause) => - Effect.logWarning("Could not add T3 Code's MCP server to OpenCode.", cause).pipe( - Effect.as(false), - ), + const add = (server: string, codemode: boolean) => + client.mcp + .add({ + server, + location: { directory }, + config: new Mcp.RemoteConfig({ + type: "remote", + url: mcpSession!.endpoint, + headers: { Authorization: wanted.credential }, + oauth: false, + ...(codemode ? {} : { codemode: false }), + }), + }) + .pipe(Effect.timeout(INVENTORY_TIMEOUT)); + const added = yield* add(servers.codeMode, true).pipe( + Effect.andThen(add(servers.html, false)), + Effect.as(true), + Effect.catchCause((cause) => + Effect.logWarning("Could not add T3 Code's MCP server to OpenCode.", cause).pipe( + Effect.andThen(removeMcp(wanted)), + Effect.as(false), ), - ); + ), + ); if (added) state.mcp = wanted; } const instructions = [ diff --git a/packages/provider-opencode/src/testing.ts b/packages/provider-opencode/src/testing.ts index 5fa87cde4e508..98275612309eb 100644 --- a/packages/provider-opencode/src/testing.ts +++ b/packages/provider-opencode/src/testing.ts @@ -10,4 +10,8 @@ export { OPENCODE_PROVIDER, OPENCODE_SDK_PROTOCOL, } from "./server/adapter.ts"; -export { OPENCODE_2_STILL_STOPPING, t3McpServerName } from "./server/v2/adapter.ts"; +export { + OPENCODE_2_STILL_STOPPING, + t3McpHtmlServerName, + t3McpServerName, +} from "./server/v2/adapter.ts"; diff --git a/packages/shared/src/htmlRender.test.ts b/packages/shared/src/htmlRender.test.ts index faa52f8475ba2..56d90d62bcc64 100644 --- a/packages/shared/src/htmlRender.test.ts +++ b/packages/shared/src/htmlRender.test.ts @@ -160,6 +160,11 @@ describe("htmlRenderFromToolItem", () => { ["mcp__t3-code__html_render", [{ type: "text", text: JSON.stringify(result) }]], ["t3-code.html_render", { structuredContent: result, content: [] }], ["t3-code-thread_1_html_render", JSON.stringify(result)], + // OpenCode 2's html server, which keeps html_render out of Code Mode. + [ + "t3-code-c26f2f1fb6fbbcc2-html_html_render", + [{ type: "text", text: JSON.stringify(result) }], + ], ["html_render", result], ] as const) { expect(htmlRenderFromToolItem({ toolName, output })).toEqual(reference); From ae99a510d27b89350f165316d2001f9ab2ca9125 Mon Sep 17 00:00:00 2001 From: sissedsauce Date: Wed, 14 Oct 2026 12:33:35 -0700 Subject: [PATCH 08/12] Revert "fix(server): OpenCode 2 keeps html_render out of Code Mode so every page shows" This reverts commit c049ef386bf6c3fde2c8247c657accc16b793fa5. --- .../Adapters/OpenCode2AdapterV2.test.ts | 214 ++++-------------- ...penCode2OrchestratorV2.integration.test.ts | 18 +- .../opencode2_fork/opencode_transcript.ndjson | 2 +- .../opencode_transcript.ndjson | 2 +- docs/internals/providers.md | 7 +- .../src/server/v2/adapter.ts | 118 +++------- packages/provider-opencode/src/testing.ts | 6 +- packages/shared/src/htmlRender.test.ts | 5 - 8 files changed, 83 insertions(+), 289 deletions(-) diff --git a/apps/server/src/orchestration-v2/Adapters/OpenCode2AdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/OpenCode2AdapterV2.test.ts index 7d7ec412632be..548ff5cb6643b 100644 --- a/apps/server/src/orchestration-v2/Adapters/OpenCode2AdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/OpenCode2AdapterV2.test.ts @@ -23,7 +23,6 @@ import { type OrchestrationV2ProviderTurn, type ProviderReplayEntry, } from "@t3tools/contracts"; -import { htmlRenderFromToolItem } from "@t3tools/shared/toolOutput"; import * as Cause from "effect/Cause"; import * as DateTime from "effect/DateTime"; import * as Deferred from "effect/Deferred"; @@ -46,7 +45,6 @@ import * as ProviderContinuationRequests from "@t3tools/provider-core/server/Pro import { OPENCODE_2_STILL_STOPPING, OPENCODE_PROVIDER, - t3McpHtmlServerName, t3McpServerName, } from "@t3tools/provider-opencode/testing"; import { openCode2ReplayRuntime } from "./OpenCode2AdapterV2.testkit.ts"; @@ -85,40 +83,12 @@ const event = ( }); const durable = { durable: { aggregateID: SESSION, seq: 1, version: 1 } }; -/** - * The rules that allow a thread its own two T3 MCP servers: every tool through - * the Code Mode one except `html_render`, which only the html one serves. - */ -const t3ServerRules = (codeMode: string, html: string) => [ +/** The rules T3 gives every session it runs, with only this thread's own T3 MCP server allowed. */ +const mcpRules = [ { action: "t3-code-*", resource: "*", effect: "deny" }, - { action: `${codeMode}_*`, resource: "*", effect: "allow" }, - { action: `${codeMode}_html_render`, resource: "*", effect: "deny" }, - { action: `${html}_html_render`, resource: "*", effect: "allow" }, + { action: "t3-code-thread_opencode2-adapter_*", resource: "*", effect: "allow" }, ]; -/** The rules T3 gives every session it runs, with only this thread's own T3 MCP servers allowed. */ -const mcpRules = t3ServerRules("t3-code-thread_opencode2-adapter", "t3-code-c26f2f1fb6fbbcc2-html"); const t3Rules = [{ action: "*", resource: "*", effect: "allow" }, ...mcpRules]; -/** Adds a thread's T3 MCP servers: the Code Mode one, then the html one with Code Mode off. */ -const registerT3Servers = (server: string, htmlServer: string) => - [server, htmlServer].flatMap((name) => [ - out("mcp.add", { - server: name, - "location[directory]": WORK, - config: { - type: "remote", - url: "http://127.0.0.1:3773/mcp", - headers: { Authorization: "Bearer thread-credential" }, - oauth: false, - ...(name === htmlServer ? { codemode: false } : {}), - }, - }), - reply("mcp.add", null), - ]); -const removeT3Servers = (server: string, htmlServer: string) => - [server, htmlServer].flatMap((name) => [ - out("mcp.remove", { server: name, "location[directory]": WORK }), - reply("mcp.remove", null), - ]); /** The thread's title, which the recorded session already carries. */ const TITLE = "Prime check"; const sessionInfo = (overrides: Record = {}) => ({ @@ -197,15 +167,15 @@ const withInstructions = ( "skill.list", ].includes(String(entry.frame.type)), ); - // T3's MCP servers are added (or, when that fails, removed) before the entry that describes them. - const after = entries.findLastIndex( + // T3's MCP server is added before the entry that describes it. + const after = entries.findIndex( (entry, index) => index < first && entry.type === "emit_inbound" && typeof entry.frame === "object" && entry.frame !== null && "operation" in entry.frame && - (entry.frame.operation === "mcp.add" || entry.frame.operation === "mcp.remove"), + entry.frame.operation === "mcp.add", ); const at = after < 0 ? first : after + 1; return first < 0 @@ -3445,15 +3415,25 @@ it.layer(McpProviderSessions.layer)("OpenCode2 adapter", (it) => { }); yield* Effect.addFinalizer(() => mcpSessions.clear(threadId)); const server = "t3-code-thread_opencode2-adapter"; - const htmlServer = "t3-code-c26f2f1fb6fbbcc2-html"; const { runtime, thread } = yield* resumed([ - // Registered for the session's directory under the thread's own names; - // the session's rules allow only these names' tools (see `t3Rules`). - ...registerT3Servers(server, htmlServer), + // Registered for the session's directory under the thread's own name; + // the session's rules allow only this name's tools (see `t3Rules`). + out("mcp.add", { + server, + "location[directory]": WORK, + config: { + type: "remote", + url: "http://127.0.0.1:3773/mcp", + headers: { Authorization: "Bearer thread-credential" }, + oauth: false, + }, + }), + reply("mcp.add", null), out("session.prompt", { sessionID: SESSION, text: "" }), promptAccepted, event("session.execution.succeeded", { sessionID: SESSION }), - ...removeT3Servers(server, htmlServer), + out("mcp.remove", { server, "location[directory]": WORK }), + reply("mcp.remove", null), ]); const terminal = yield* terminalOf(runtime).pipe(Effect.forkScoped); yield* runtime.startTurn(turnInput(thread)); @@ -3462,126 +3442,6 @@ it.layer(McpProviderSessions.layer)("OpenCode2 adapter", (it) => { }).pipe(Effect.scoped), ); - it.effect("shows a page html_render published through the thread's html server", () => - Effect.gen(function* () { - const mcpSessions = yield* McpProviderSessions.McpProviderSessions; - yield* mcpSessions.set({ - environmentId: EnvironmentId.make("environment:opencode2-adapter"), - threadId, - providerSessionId: "mcp:opencode2-adapter", - providerInstanceId: instanceId, - endpoint: "http://127.0.0.1:3773/mcp", - authorizationHeader: "Bearer thread-credential", - browserToolsAvailable: false, - }); - yield* Effect.addFinalizer(() => mcpSessions.clear(threadId)); - const server = "t3-code-thread_opencode2-adapter"; - const htmlServer = "t3-code-c26f2f1fb6fbbcc2-html"; - const page = { - attachmentId: "thread_opencode2-adapter-00000000-0000-4000-8000-000000000001-html", - title: "Chart", - height: 420, - }; - // With Code Mode off for the html server, the call is its own tool part - // carrying html_render's result, not a line inside an `execute` script. - const tool = { sessionID: SESSION, assistantMessageID: "msg_assistant", id: "call-render" }; - const name = `${htmlServer}_html_render`; - const { runtime, thread } = yield* resumed([ - ...registerT3Servers(server, htmlServer), - out("session.prompt", { sessionID: SESSION, text: "" }), - promptAccepted, - event("session.tool.input.started", { ...tool, name }), - event("session.tool.called", { - ...tool, - name, - input: { html: "

Chart

", title: "Chart", height: 420 }, - executed: false, - }), - // `session.tool.success` is at version 2. - { - type: "emit_inbound", - frame: { - type: "sdk.event", - event: { - id: "evt_toolsuccess_render", - created: 1, - type: "session.tool.success", - data: { - ...tool, - content: [ - { type: "text", text: JSON.stringify({ htmlRender: page, message: "Shown" }) }, - ], - metadata: { truncated: false }, - executed: false, - }, - durable: { aggregateID: SESSION, seq: 1, version: 2 }, - }, - }, - }, - event("session.execution.succeeded", { sessionID: SESSION }), - ...removeT3Servers(server, htmlServer), - ]); - const events = yield* runtime.events.pipe( - Stream.takeUntil((event) => event.type === "turn.terminal"), - Stream.runCollect, - Effect.forkScoped, - ); - yield* runtime.startTurn(turnInput(thread)); - const collected = yield* Fiber.join(events); - assert.deepInclude(collected.at(-1), { type: "turn.terminal", status: "completed" }); - const rendered = collected.findLast( - (event) => - event.type === "turn_item.updated" && - event.turnItem.type === "dynamic_tool" && - event.turnItem.status === "completed", - ); - assert.deepEqual( - rendered?.type === "turn_item.updated" && rendered.turnItem.type === "dynamic_tool" - ? htmlRenderFromToolItem(rendered.turnItem) - : undefined, - page, - ); - yield* runtime.unloadThread!({ providerThread: thread }); - }).pipe(Effect.scoped), - ); - - it.effect("removes the Code Mode server again when the html server cannot be added", () => - Effect.gen(function* () { - const mcpSessions = yield* McpProviderSessions.McpProviderSessions; - yield* mcpSessions.set({ - environmentId: EnvironmentId.make("environment:opencode2-adapter"), - threadId, - providerSessionId: "mcp:opencode2-adapter", - providerInstanceId: instanceId, - endpoint: "http://127.0.0.1:3773/mcp", - authorizationHeader: "Bearer thread-credential", - browserToolsAvailable: false, - }); - yield* Effect.addFinalizer(() => mcpSessions.clear(threadId)); - const server = "t3-code-thread_opencode2-adapter"; - const htmlServer = "t3-code-c26f2f1fb6fbbcc2-html"; - const [codeModeAdd, codeModeAdded, htmlAdd] = registerT3Servers(server, htmlServer); - const { runtime, thread } = yield* resumed([ - codeModeAdd!, - codeModeAdded!, - htmlAdd!, - reply("mcp.add", { status: 500, body: { message: "boom" } }), - // The rules deny html_render through Code Mode, so the Code Mode server - // alone would leave no way to call it: neither stays, and the turn runs - // without T3's tools. - ...removeT3Servers(server, htmlServer), - out("session.prompt", { sessionID: SESSION, text: "" }), - promptAccepted, - event("session.execution.succeeded", { sessionID: SESSION }), - ]); - const terminal = yield* terminalOf(runtime).pipe(Effect.forkScoped); - yield* runtime.startTurn(turnInput(thread)); - assert.equal((yield* Fiber.join(terminal))?.status, "completed"); - // Nothing is registered, so unloading removes nothing. - yield* runtime.unloadThread!({ providerThread: thread }); - }).pipe(Effect.scoped), - ); - it.effect("registers a long thread id's MCP server under a name OpenCode accepts", () => Effect.gen(function* () { // Spelled out, this delegated thread's server name would be 120 characters. @@ -3609,16 +3469,28 @@ it.layer(McpProviderSessions.layer)("OpenCode2 adapter", (it) => { sessionInfo({ permissions: [ { action: "*", resource: "*", effect: "allow" }, - ...t3ServerRules(server, `${server}-html`), + { action: "t3-code-*", resource: "*", effect: "deny" }, + { action: `${server}_*`, resource: "*", effect: "allow" }, ], }), ), ...noOpenRequests, - ...registerT3Servers(server, `${server}-html`), + out("mcp.add", { + server, + "location[directory]": WORK, + config: { + type: "remote", + url: "http://127.0.0.1:3773/mcp", + headers: { Authorization: "Bearer thread-credential" }, + oauth: false, + }, + }), + reply("mcp.add", null), out("session.prompt", { sessionID: SESSION, text: "" }), promptAccepted, event("session.execution.succeeded", { sessionID: SESSION }), - ...removeT3Servers(server, `${server}-html`), + out("mcp.remove", { server, "location[directory]": WORK }), + reply("mcp.remove", null), ]); const thread = yield* runtime.resumeThread({ providerThread: { ...providerThread(yield* DateTime.now), appThreadId: child }, @@ -3650,14 +3522,6 @@ it.layer(McpProviderSessions.layer)("OpenCode2 adapter", (it) => { assert.equal(names[0], "t3-code-63abb5df2b188bdd"); // A name that already fits stays readable. assert.equal(yield* t3McpServerName(threadId), "t3-code-thread_opencode2-adapter"); - // The html server's tool keeps within model APIs' 64-character tool names. - const htmlNames = yield* Effect.forEach([threadId, ...ids], t3McpHtmlServerName); - for (const name of htmlNames) { - assert.match(name, /^t3-code-[0-9a-f]{16}-html$/); - assert.isAtMost(`${name}_html_render`.length, 64); - } - assert.equal(new Set(htmlNames).size, htmlNames.length); - assert.equal(htmlNames[0], "t3-code-c26f2f1fb6fbbcc2-html"); }).pipe(Effect.provide(NodeCrypto.layer)), ); @@ -4554,10 +4418,8 @@ it.layer(McpProviderSessions.layer)("OpenCode2 adapter", (it) => { sessionID: FORK, permissions: [ { action: "*", resource: "*", effect: "allow" }, - ...t3ServerRules( - "t3-code-thread_opencode2-adapter_fork", - "t3-code-7c81f4ba96860743-html", - ), + { action: "t3-code-*", resource: "*", effect: "deny" }, + { action: "t3-code-thread_opencode2-adapter_fork_*", resource: "*", effect: "allow" }, ], }), reply("session.update", null), diff --git a/apps/server/src/orchestration-v2/OpenCode2OrchestratorV2.integration.test.ts b/apps/server/src/orchestration-v2/OpenCode2OrchestratorV2.integration.test.ts index 88d08062b88e0..3f873bfb6ea36 100644 --- a/apps/server/src/orchestration-v2/OpenCode2OrchestratorV2.integration.test.ts +++ b/apps/server/src/orchestration-v2/OpenCode2OrchestratorV2.integration.test.ts @@ -1,10 +1,8 @@ -// @effect-diagnostics nodeBuiltinImport:off - expected rules name each thread's html server by a digest of its id. /** * OpenCode 2 through the whole orchestrator, against a replayed HTTP server: * the transcript fixes the order of every request the adapter sends, so a * request the orchestrator never lets it make fails the run. */ -import * as NodeCrypto from "node:crypto"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, describe, it } from "@effect/vitest"; import { @@ -75,18 +73,12 @@ const labelled = (entry: ProviderReplayEntry, label: string): ProviderReplayEntr entry.type === "runtime_exit" ? entry : { ...entry, label }; /** * T3's own rules after a mode's: every thread's T3 MCP server is denied, and - * then this thread's own are allowed again (last match wins), with - * `html_render` served only by the html one. + * then this thread's own is allowed again (last match wins). */ -const mcpRules = (name: string) => { - const html = `t3-code-${NodeCrypto.createHash("sha256").update(`thread:${name}`).digest("hex").slice(0, 16)}-html`; - return [ - { action: "t3-code-*", resource: "*", effect: "deny" }, - { action: `t3-code-thread_${name}_*`, resource: "*", effect: "allow" }, - { action: `t3-code-thread_${name}_html_render`, resource: "*", effect: "deny" }, - { action: `${html}_html_render`, resource: "*", effect: "allow" }, - ]; -}; +const mcpRules = (name: string) => [ + { action: "t3-code-*", resource: "*", effect: "deny" }, + { action: `t3-code-thread_${name}_*`, resource: "*", effect: "allow" }, +]; const FULL_ACCESS = [{ action: "*", resource: "*", effect: "allow" }]; /** Full access for the thread named `name`. */ const t3Rules = (name: string) => [...FULL_ACCESS, ...mcpRules(name)]; diff --git a/apps/server/src/orchestration-v2/testkit/fixtures/opencode2_fork/opencode_transcript.ndjson b/apps/server/src/orchestration-v2/testkit/fixtures/opencode2_fork/opencode_transcript.ndjson index f81188db07236..57cbd14853460 100644 --- a/apps/server/src/orchestration-v2/testkit/fixtures/opencode2_fork/opencode_transcript.ndjson +++ b/apps/server/src/orchestration-v2/testkit/fixtures/opencode2_fork/opencode_transcript.ndjson @@ -47,7 +47,7 @@ {"type": "emit_inbound", "label": "message.list.response.2", "frame": {"type": "sdk.response", "operation": "message.list", "data": {"data": [], "cursor": {}}}} {"type": "expect_outbound", "label": "session.fork", "frame": {"type": "session.fork", "input": {"sessionID": "ses_f1484ea8affe5aTo8CcgbGIszj", "before": "msg_0eb7b1f9f001g95Bduh3bi8syS"}}} {"type": "emit_inbound", "label": "session.fork.response", "frame": {"type": "sdk.response", "operation": "session.fork", "data": {"data": {"id": "ses_f1484db83ffeLGtrRCFimo1H0e", "fork": {"sessionID": "ses_f1484ea8affe5aTo8CcgbGIszj", "boundary": {"type": "before", "messageID": "msg_0eb7b1f9f001g95Bduh3bi8syS"}}, "projectID": "global", "model": {"id": "big-pickle", "providerID": "opencode", "variant": "default"}, "cost": 0, "tokens": {"input": 0, "output": 0, "reasoning": 0, "cache": {"read": 0, "write": 0}}, "time": {"created": 1790657111165, "updated": 1790657111165}, "title": "spike fork (fork #1)", "location": {"directory": ""}, "permissions": [{"action": "*", "resource": "*", "effect": "allow"}, {"action": "subagent", "resource": "*", "effect": "deny"}]}}}} -{"type": "expect_outbound", "label": "session.update", "frame": {"type": "session.update", "input": {"sessionID": "ses_f1484db83ffeLGtrRCFimo1H0e", "permissions": [{"action": "*", "resource": "*", "effect": "allow"}, {"action": "t3-code-*", "resource": "*", "effect": "deny"}, {"action": "t3-code-thread_opencode2-fork_target_*", "resource": "*", "effect": "allow"}, {"action": "t3-code-thread_opencode2-fork_target_html_render", "resource": "*", "effect": "deny"}, {"action": "t3-code-f74b9d2803b51629-html_html_render", "resource": "*", "effect": "allow"}]}}} +{"type": "expect_outbound", "label": "session.update", "frame": {"type": "session.update", "input": {"sessionID": "ses_f1484db83ffeLGtrRCFimo1H0e", "permissions": [{"action": "*", "resource": "*", "effect": "allow"}, {"action": "t3-code-*", "resource": "*", "effect": "deny"}, {"action": "t3-code-thread_opencode2-fork_target_*", "resource": "*", "effect": "allow"}]}}} {"type": "emit_inbound", "label": "session.update.response", "frame": {"type": "sdk.response", "operation": "session.update", "data": null}} {"type": "emit_inbound", "label": "session.forked", "frame": {"type": "sdk.event", "event": {"id": "evt_0eb7b247d001fXzFroPg5P0bps", "created": 1790657111165, "type": "session.forked", "location": {"directory": ""}, "data": {"sessionID": "ses_f1484db83ffeLGtrRCFimo1H0e", "parentID": "ses_f1484ea8affe5aTo8CcgbGIszj", "boundary": {"type": "before", "messageID": "msg_0eb7b1f9f001g95Bduh3bi8syS"}, "instructions": {"core/environment": "db4a793a05fec32fdfa985a3432125a86d6cb42debc8b9ee8f6a99fbfcf2f93b", "core/date": "bf08b8486572ab3eef677a3c9ee12468e411ae41a8f4d2fb6cf02a6ca73f1986", "core/codemode": "616a7ad8b8e40e8a71eab238dfbbe9eaf0f4a9e6fdad853226d075bfe602e1e6", "core/skill-guidance": "0d001919496102654717308c72635523bf7c3cd99c81e058cc79f2947104f9a5"}, "instructionEntries": []}, "durable": {"aggregateID": "ses_f1484db83ffeLGtrRCFimo1H0e", "seq": 0, "version": 2}}}} {"type": "expect_outbound", "label": "session.update.title", "frame": {"type": "session.update", "input": {"sessionID": "ses_f1484db83ffeLGtrRCFimo1H0e", "title": ""}}} diff --git a/apps/server/src/orchestration-v2/testkit/fixtures/opencode2_switch/opencode_transcript.ndjson b/apps/server/src/orchestration-v2/testkit/fixtures/opencode2_switch/opencode_transcript.ndjson index 7099c2afa7c99..ee2519419915a 100644 --- a/apps/server/src/orchestration-v2/testkit/fixtures/opencode2_switch/opencode_transcript.ndjson +++ b/apps/server/src/orchestration-v2/testkit/fixtures/opencode2_switch/opencode_transcript.ndjson @@ -53,7 +53,7 @@ {"type": "emit_inbound", "label": "session.execution.succeeded", "frame": {"type": "sdk.event", "event": {"id": "evt_0eb7eef75001z9Sy13uGDVqV3F", "created": 1790657359733, "type": "session.execution.succeeded", "data": {"sessionID": "ses_f14812197ffeCbw641wM2r3ZmP"}, "durable": {"aggregateID": "ses_f14812197ffeCbw641wM2r3ZmP", "seq": 15, "version": 1}}}} {"type": "expect_outbound", "label": "session.get", "frame": {"type": "session.get", "input": {"sessionID": "ses_f14812197ffeCbw641wM2r3ZmP"}}} {"type": "emit_inbound", "label": "session.get.response", "frame": {"type": "sdk.response", "operation": "session.get", "data": {"data": {"id": "ses_f14812197ffeCbw641wM2r3ZmP", "projectID": "global", "agent": "plan", "model": {"id": "big-pickle", "providerID": "opencode", "variant": "default"}, "cost": 0, "tokens": {"input": 8910, "output": 271, "reasoning": 0, "cache": {"read": 384, "write": 0}}, "outcome": "succeeded", "time": {"created": 1790657355410, "updated": 1790657355463}, "title": "spike switch", "location": {"directory": ""}, "permissions": [{"action": "*", "resource": "*", "effect": "allow"}, {"action": "subagent", "resource": "*", "effect": "deny"}]}}}} -{"type": "expect_outbound", "label": "session.update", "frame": {"type": "session.update", "input": {"sessionID": "ses_f14812197ffeCbw641wM2r3ZmP", "permissions": [{"action": "*", "resource": "*", "effect": "allow"}, {"action": "t3-code-*", "resource": "*", "effect": "deny"}, {"action": "t3-code-thread_opencode2_switch_*", "resource": "*", "effect": "allow"}, {"action": "t3-code-thread_opencode2_switch_html_render", "resource": "*", "effect": "deny"}, {"action": "t3-code-421cb593d6fcf559-html_html_render", "resource": "*", "effect": "allow"}]}}} +{"type": "expect_outbound", "label": "session.update", "frame": {"type": "session.update", "input": {"sessionID": "ses_f14812197ffeCbw641wM2r3ZmP", "permissions": [{"action": "*", "resource": "*", "effect": "allow"}, {"action": "t3-code-*", "resource": "*", "effect": "deny"}, {"action": "t3-code-thread_opencode2_switch_*", "resource": "*", "effect": "allow"}]}}} {"type": "emit_inbound", "label": "session.update.response", "frame": {"type": "sdk.response", "operation": "session.update", "data": null}} {"type": "expect_outbound", "label": "session.switchAgent.2", "frame": {"type": "session.switchAgent", "input": {"sessionID": "ses_f14812197ffeCbw641wM2r3ZmP", "agent": "build"}}} {"type": "emit_inbound", "label": "session.switchAgent.response.2", "frame": {"type": "sdk.response", "operation": "session.switchAgent", "data": null}} diff --git a/docs/internals/providers.md b/docs/internals/providers.md index 1a1156b9ac863..5f9642c2db27d 100644 --- a/docs/internals/providers.md +++ b/docs/internals/providers.md @@ -22,12 +22,7 @@ directory must not share one T3 MCP entry. - **2.x** serves every directory from one [server per instance](../../packages/provider-opencode/src/server/v2/OpenCode2Server.ts). Each thread registers its own `t3-code-` MCP entry, and session permission rules deny every other - thread's entry. OpenCode's Code Mode runs MCP tools inside one `execute` call and reports nested - calls without their results, so a page `html_render` published from there would never reach the - thread. Each thread therefore also registers a `t3-code--html` entry with Code Mode off, - and its rules allow `html_render` only there. Every other T3 tool stays in Code Mode, because - exposing all of them directly would add every tool schema to every request. - See the [2.x adapter](../../packages/provider-opencode/src/server/v2/adapter.ts). + thread's entry. See the [2.x adapter](../../packages/provider-opencode/src/server/v2/adapter.ts). External OpenCode servers remain externally owned and can require an external restart to pick up configuration changes. OpenCode stores "always" approval grants for the whole project. Automatic diff --git a/packages/provider-opencode/src/server/v2/adapter.ts b/packages/provider-opencode/src/server/v2/adapter.ts index 8b0f40afcd30a..785bf1a06e70c 100644 --- a/packages/provider-opencode/src/server/v2/adapter.ts +++ b/packages/provider-opencode/src/server/v2/adapter.ts @@ -79,7 +79,6 @@ import * as McpProviderSessions from "@t3tools/provider-core/server/McpProviderS import { buildRuntimeInstructions } from "@t3tools/provider-core/server/runtimeInstructions"; import { t3OrchestrationSystemPrompt } from "@t3tools/provider-core/server/orchestrationInstructions"; import { SKILL_MENTION_PATTERN } from "@t3tools/shared/composerInlineTokens"; -import { HTML_RENDER_TOOL_NAME } from "@t3tools/shared/htmlRender"; import * as KeyedLock from "@t3tools/shared/KeyedLock"; import { getModelSelectionStringOptionValue, modelSelectionsEqual } from "@t3tools/shared/model"; import { causeErrorTag } from "@t3tools/shared/observability"; @@ -415,13 +414,9 @@ interface ThreadState { * inbox, where the next prompt would deliver them first, so it cancels them. */ readonly strandedSteers: Set; - /** T3's MCP servers as registered for this thread, and the instructions entry sent with it. */ + /** T3's MCP server as registered for this thread, and the instructions entry sent with it. */ mcp: - | { - readonly servers: T3McpServers; - readonly directory: string; - readonly credential: string; - } + | { readonly name: string; readonly directory: string; readonly credential: string } | undefined; instructions: string | undefined; } @@ -480,59 +475,32 @@ const rule = (action: string, effect: Rule["effect"]): Rule => ({ action, resour export const t3McpServerName = Effect.fn("t3McpServerName")(function* (threadId: string) { const name = `t3-code-${threadId.replaceAll(/[^a-zA-Z0-9_-]/g, "_")}`; if (name.length <= 64) return name; - return `t3-code-${yield* threadDigest(threadId)}`; -}); - -/** - * The thread's second T3 server, registered with Code Mode off so that - * `html_render` stays a direct tool (see `mcpRules`). The model calls it as - * `_html_render`, and model APIs cap tool names at 64 characters, so - * it always takes the digest form. - */ -export const t3McpHtmlServerName = Effect.fn("t3McpHtmlServerName")(function* (threadId: string) { - return `t3-code-${yield* threadDigest(threadId)}-html`; -}); - -const threadDigest = Effect.fnUntraced(function* (threadId: string) { const crypto = yield* Crypto.Crypto; const digest = yield* crypto .digest("SHA-256", new TextEncoder().encode(threadId)) .pipe(Effect.orDie); - return Hex.encode(digest).slice(0, 16); + return `t3-code-${Hex.encode(digest).slice(0, 16)}`; }); -/** A thread's T3 servers: every tool through Code Mode, and `html_render` alone directly. */ -interface T3McpServers { - readonly codeMode: string; - readonly html: string; -} - /** * The rules that keep T3's MCP servers to their own thread, after the mode's: * the last matching rule wins, so every thread's T3 server is denied and then * this thread's own is allowed again, in every mode. A subagent's session * inherits the thread's. - * - * Code Mode reports a nested call only as its `execute` script, without the - * call's result, so a page `html_render` published from there never reached - * the thread. `html_render` is therefore taken out of the Code Mode server and - * allowed only on the html server, where each call is its own tool part. */ -const mcpRules = (servers: T3McpServers | null): ReadonlyArray => - servers === null +const mcpRules = (mcpServerName: string | null): ReadonlyArray => + mcpServerName === null ? [] : [ { action: "t3-code-*", resource: "*", effect: "deny" }, - { action: `${servers.codeMode}_*`, resource: "*", effect: "allow" }, - { action: `${servers.codeMode}_${HTML_RENDER_TOOL_NAME}`, resource: "*", effect: "deny" }, - { action: `${servers.html}_${HTML_RENDER_TOOL_NAME}`, resource: "*", effect: "allow" }, + { action: `${mcpServerName}_*`, resource: "*", effect: "allow" }, ]; const sessionRules = ( policy: RulesPolicy, paths: ReadonlyArray, grants: ReadonlyArray, - mcpServers: T3McpServers | null, + mcpServerName: string | null, ): ReadonlyArray => [ ...(policy.runtimeMode === "full-access" ? [rule("*", "allow")] @@ -546,7 +514,7 @@ const sessionRules = ( // are never denied: the free tier refuses sessions whose rules deny them. ...(policy.interactionMode === "plan" ? [rule("edit", "deny")] : []), ...paths, - ...mcpRules(mcpServers), + ...mcpRules(mcpServerName), ]; const sameRules = (left: ReadonlyArray | undefined, right: ReadonlyArray) => @@ -885,11 +853,8 @@ export const make = Effect.fn("OpenCode2Adapter.make")(function* (instanceId: Pr const continuationRequests = yield* ProviderContinuationRequests.ProviderContinuationRequests; const crypto = yield* Crypto.Crypto; const driver = OPENCODE_PROVIDER; - const mcpServersFor = (threadId: string) => - Effect.all({ - codeMode: t3McpServerName(threadId), - html: t3McpHtmlServerName(threadId), - }).pipe(Effect.provideService(Crypto.Crypto, crypto)); + const mcpServerNameFor = (threadId: string) => + t3McpServerName(threadId).pipe(Effect.provideService(Crypto.Crypto, crypto)); /** * Lends the instance's server to a session until its scope closes. A spawned @@ -3136,7 +3101,7 @@ export const make = Effect.fn("OpenCode2Adapter.make")(function* (instanceId: Pr policy, paths, policy.runtimeMode === "full-access" ? [] : thread.grants, - appThreadId === null ? null : yield* mcpServersFor(appThreadId), + appThreadId === null ? null : yield* mcpServerNameFor(appThreadId), ); }); @@ -3378,15 +3343,10 @@ export const make = Effect.fn("OpenCode2Adapter.make")(function* (instanceId: Pr resolveAttachmentPath: host.resolveAttachmentPath, }).trim(); - const removeMcp = (mcp: { readonly servers: T3McpServers; readonly directory: string }) => - Effect.forEach( - [mcp.servers.codeMode, mcp.servers.html], - (server) => - Effect.suspend(() => - client.mcp.remove({ server, location: { directory: mcp.directory } }), - ).pipe(Effect.timeout("5 seconds"), Effect.ignore({ log: true })), - { discard: true }, - ); + const removeMcp = (mcp: { readonly name: string; readonly directory: string }) => + Effect.suspend(() => + client.mcp.remove({ server: mcp.name, location: { directory: mcp.directory } }), + ).pipe(Effect.timeout("5 seconds"), Effect.ignore({ log: true })); // T3's MCP registrations outlive a session only on an external server; a // spawned one forgets them when it stops. @@ -3413,12 +3373,12 @@ export const make = Effect.fn("OpenCode2Adapter.make")(function* (instanceId: Pr ) { const mcpSession = yield* mcpSessions.read(turnInput.threadId); const directory = turnInput.runtimePolicy.cwd ?? host.paths.cwd; - const servers = yield* mcpServersFor(turnInput.threadId); + const name = yield* mcpServerNameFor(turnInput.threadId); // An external server may not reach T3's MCP endpoint, as with 1.x. const wanted = mcpSession === undefined || connection.external ? undefined - : { servers, directory, credential: mcpSession.authorizationHeader }; + : { name, directory, credential: mcpSession.authorizationHeader }; if ( state.mcp !== undefined && (wanted === undefined || @@ -3429,33 +3389,27 @@ export const make = Effect.fn("OpenCode2Adapter.make")(function* (instanceId: Pr state.mcp = undefined; } // T3's tools are an addition: a server that cannot add them still runs the turn. - // The two servers are added as one: the rules deny `html_render` through - // Code Mode, so the Code Mode server alone would leave no way to call it. if (wanted !== undefined && state.mcp === undefined) { - const add = (server: string, codemode: boolean) => - client.mcp - .add({ - server, - location: { directory }, - config: new Mcp.RemoteConfig({ - type: "remote", - url: mcpSession!.endpoint, - headers: { Authorization: wanted.credential }, - oauth: false, - ...(codemode ? {} : { codemode: false }), - }), - }) - .pipe(Effect.timeout(INVENTORY_TIMEOUT)); - const added = yield* add(servers.codeMode, true).pipe( - Effect.andThen(add(servers.html, false)), - Effect.as(true), - Effect.catchCause((cause) => - Effect.logWarning("Could not add T3 Code's MCP server to OpenCode.", cause).pipe( - Effect.andThen(removeMcp(wanted)), - Effect.as(false), + const added = yield* client.mcp + .add({ + server: name, + location: { directory }, + config: new Mcp.RemoteConfig({ + type: "remote", + url: mcpSession!.endpoint, + headers: { Authorization: wanted.credential }, + oauth: false, + }), + }) + .pipe( + Effect.timeout(INVENTORY_TIMEOUT), + Effect.as(true), + Effect.catchCause((cause) => + Effect.logWarning("Could not add T3 Code's MCP server to OpenCode.", cause).pipe( + Effect.as(false), + ), ), - ), - ); + ); if (added) state.mcp = wanted; } const instructions = [ diff --git a/packages/provider-opencode/src/testing.ts b/packages/provider-opencode/src/testing.ts index 98275612309eb..5fa87cde4e508 100644 --- a/packages/provider-opencode/src/testing.ts +++ b/packages/provider-opencode/src/testing.ts @@ -10,8 +10,4 @@ export { OPENCODE_PROVIDER, OPENCODE_SDK_PROTOCOL, } from "./server/adapter.ts"; -export { - OPENCODE_2_STILL_STOPPING, - t3McpHtmlServerName, - t3McpServerName, -} from "./server/v2/adapter.ts"; +export { OPENCODE_2_STILL_STOPPING, t3McpServerName } from "./server/v2/adapter.ts"; diff --git a/packages/shared/src/htmlRender.test.ts b/packages/shared/src/htmlRender.test.ts index 56d90d62bcc64..faa52f8475ba2 100644 --- a/packages/shared/src/htmlRender.test.ts +++ b/packages/shared/src/htmlRender.test.ts @@ -160,11 +160,6 @@ describe("htmlRenderFromToolItem", () => { ["mcp__t3-code__html_render", [{ type: "text", text: JSON.stringify(result) }]], ["t3-code.html_render", { structuredContent: result, content: [] }], ["t3-code-thread_1_html_render", JSON.stringify(result)], - // OpenCode 2's html server, which keeps html_render out of Code Mode. - [ - "t3-code-c26f2f1fb6fbbcc2-html_html_render", - [{ type: "text", text: JSON.stringify(result) }], - ], ["html_render", result], ] as const) { expect(htmlRenderFromToolItem({ toolName, output })).toEqual(reference); From 79d5986aac23b516533afa8329b3341225265043 Mon Sep 17 00:00:00 2001 From: sissedsauce Date: Fri, 23 Oct 2026 05:16:59 -0700 Subject: [PATCH 09/12] fix(shared): only show an execute script's page when it belongs to the row's thread An execute item's output is whatever the agent's script returned, so a reference read from it could name a page published for another thread. htmlRenderFromToolItem now takes the item's thread and accepts an execute reference only when its attachment id was minted for that thread. Web and mobile pass each row's source thread, so delegated and inherited rows are checked against the thread they came from, and thread-deletion cleanup passes the thread being deleted. The thread-segment helper moves to @t3tools/shared/attachmentIds so the clients can use it; the server re-exports it unchanged. Co-Authored-By: Claude Opus 5.5 --- apps/mobile/src/lib/threadActivity.test.ts | 27 ++++++++++++++++ apps/mobile/src/lib/threadActivity.ts | 2 +- apps/server/src/attachmentStore.ts | 26 +++++---------- apps/web/src/session-logic.test.ts | 37 ++++++++++++++++++++++ apps/web/src/session-logic.ts | 2 +- packages/shared/package.json | 4 +++ packages/shared/src/attachmentIds.ts | 35 ++++++++++++++++++++ packages/shared/src/htmlRender.test.ts | 24 ++++++++++---- packages/shared/src/toolOutput.ts | 30 ++++++++++++------ 9 files changed, 152 insertions(+), 35 deletions(-) create mode 100644 packages/shared/src/attachmentIds.ts diff --git a/apps/mobile/src/lib/threadActivity.test.ts b/apps/mobile/src/lib/threadActivity.test.ts index 1163df73f4a0e..1a2cf32a5908c 100644 --- a/apps/mobile/src/lib/threadActivity.test.ts +++ b/apps/mobile/src/lib/threadActivity.test.ts @@ -75,6 +75,33 @@ it("keeps historical plan detail accessible from its paged turn item", () => { expect(presented[2]).toMatchObject({ activities: [{ projectedItem: { item } }] }); }); +it("shows a page an execute script returned only when it was published for the row's thread", () => { + const executeCall = (id: string, thread: string) => + ({ + ...base(id, "2026-06-20T00:00:02.000Z", 1), + type: "dynamic_tool", + toolName: "execute", + input: { code: "return await tools.render()" }, + output: JSON.stringify({ + htmlRender: { + attachmentId: `${thread}-00000000-0000-4000-8000-000000000001-html`, + title: "Chart", + height: 420, + }, + }), + }) as OrchestrationV2TurnItem; + const pages = (rows: OrchestrationV2ProjectedTurnItem[]) => + buildThreadFeed(rows).flatMap((entry) => (entry.type === "html-render" ? [entry.id] : [])); + + expect(pages([projected(executeCall("own", threadId), 0)])).toHaveLength(1); + expect(pages([projected(executeCall("foreign", "thread-2"), 0)])).toEqual([]); + // An inherited row is checked against the thread it came from, not the one showing it. + expect(pages([projected(executeCall("inherited", sourceThreadId), 0, "inherited")])).toHaveLength( + 1, + ); + expect(pages([projected(executeCall("inherited-local", threadId), 0, "inherited")])).toEqual([]); +}); + function base(id: string, updatedAt: string, ordinal: number) { const timestamp = DateTime.makeUnsafe(updatedAt); return { diff --git a/apps/mobile/src/lib/threadActivity.ts b/apps/mobile/src/lib/threadActivity.ts index 1e942fc8ca762..6c2eec2bd04f7 100644 --- a/apps/mobile/src/lib/threadActivity.ts +++ b/apps/mobile/src/lib/threadActivity.ts @@ -2099,7 +2099,7 @@ export function buildThreadFeed( const createdAt = DateTime.formatIso(item.startedAt ?? item.updatedAt); const render = item.type === "dynamic_tool" && item.status === "completed" - ? htmlRenderFromToolItem(item) + ? htmlRenderFromToolItem(item, row.sourceThreadId) : undefined; if (render) { const entry: RawThreadFeedEntry = { diff --git a/apps/server/src/attachmentStore.ts b/apps/server/src/attachmentStore.ts index ab4948b2f1517..40803afb16f64 100644 --- a/apps/server/src/attachmentStore.ts +++ b/apps/server/src/attachmentStore.ts @@ -4,6 +4,10 @@ import * as NodeFS from "node:fs"; import * as NodePath from "node:path"; import type { ChatAttachment } from "@t3tools/contracts"; +import { + PENDING_ATTACHMENT_THREAD_SEGMENT, + toSafeThreadAttachmentSegment, +} from "@t3tools/shared/attachmentIds"; import { htmlRenderFromToolItem, mcpAppFromToolItem } from "@t3tools/shared/toolOutput"; import { @@ -13,7 +17,6 @@ import { import { inferImageExtension, SAFE_IMAGE_FILE_EXTENSIONS } from "./imageMime.ts"; const ATTACHMENT_FILENAME_EXTENSIONS = [...SAFE_IMAGE_FILE_EXTENSIONS, ".bin"]; -const ATTACHMENT_ID_THREAD_SEGMENT_MAX_CHARS = 80; const ATTACHMENT_ID_THREAD_SEGMENT_PATTERN = "[a-z0-9_]+(?:-[a-z0-9_]+)*"; const ATTACHMENT_ID_UUID_PATTERN = "[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}"; const ATTACHMENT_ID_FILE_EXTENSION_PATTERN = "[a-z0-9]{1,10}"; @@ -22,24 +25,10 @@ const ATTACHMENT_ID_PATTERN = new RegExp( "i", ); -export const PENDING_ATTACHMENT_THREAD_SEGMENT = "pending"; const PENDING_ATTACHMENT_MAX_AGE_MS = 24 * 60 * 60 * 1000; -const PARTIAL_UPLOAD_MAX_AGE_MS = 60 * 60 * 1000; +export { PENDING_ATTACHMENT_THREAD_SEGMENT, toSafeThreadAttachmentSegment }; -export function toSafeThreadAttachmentSegment(threadId: string): string | null { - const segment = threadId - .trim() - .toLowerCase() - .replace(/[^a-z0-9_-]+/gi, "-") - .replace(/-+/g, "-") - .replace(/^[-_]+|[-_]+$/g, "") - .slice(0, ATTACHMENT_ID_THREAD_SEGMENT_MAX_CHARS) - .replace(/[-_]+$/g, ""); - if (segment.length === 0) { - return null; - } - return segment === PENDING_ATTACHMENT_THREAD_SEGMENT ? "_pending" : segment; -} +const PARTIAL_UPLOAD_MAX_AGE_MS = 60 * 60 * 1000; export function attachmentFileExtension(fileName: string): string { const extension = NodePath.extname(fileName).toLowerCase(); @@ -142,7 +131,8 @@ export function threadHtmlRenderAttachmentIds( if (segment === null) return []; return Array.from(items).flatMap((item) => { const attachmentId = - htmlRenderFromToolItem(item)?.attachmentId ?? mcpAppFromToolItem(item)?.attachmentId; + htmlRenderFromToolItem(item, threadId)?.attachmentId ?? + mcpAppFromToolItem(item)?.attachmentId; return attachmentId !== undefined && parseThreadSegmentFromAttachmentId(attachmentId) === segment ? [attachmentId] diff --git a/apps/web/src/session-logic.test.ts b/apps/web/src/session-logic.test.ts index de8b3373e44b2..5aa46c0ab7fa9 100644 --- a/apps/web/src/session-logic.test.ts +++ b/apps/web/src/session-logic.test.ts @@ -1509,6 +1509,43 @@ describe("HTML renders in the timeline", () => { expect(rowsFor(entries, true)).toEqual(["message", "turn-fold", "attempt-fold", "html-render"]); }); + it("shows a page an execute script returned only when it was published for the row's thread", () => { + const page = (thread: string) => ({ + attachmentId: `${thread}-00000000-0000-4000-8000-000000000001-html`, + title: "Chart", + height: 420, + }); + const executeCall = (render: ReturnType): OrchestrationV2TurnItem => ({ + ...base("render", 2), + type: "dynamic_tool", + toolName: "execute", + input: { code: "return await tools.render()" }, + output: JSON.stringify({ htmlRender: render }), + }); + const own = turn(executeCall(page("render-thread"))); + expect(own.find((entry) => entry.kind === "html-render")).toMatchObject({ + htmlRender: page("render-thread"), + }); + const foreign = turn(executeCall(page("other-thread"))); + expect(foreign.some((entry) => entry.kind === "html-render")).toBe(false); + expect(foreign.find((entry) => entry.id === "render")?.kind).toBe("work"); + // A delegated child's row is checked against the child, not the thread showing it. + const child = ThreadId.make("child-thread"); + const merged = deriveTimelineEntriesFromVisibleTurnItems({ + visibleTurnItems: [ + { ...visible(executeCall(page("child-thread"))), sourceThreadId: child }, + { + ...visible({ ...executeCall(page("render-thread")), id: TurnItemId.make("render-2") }), + sourceThreadId: child, + }, + ], + optimisticMessages: [], + }); + expect(merged.filter((entry) => entry.kind === "html-render").map((entry) => entry.id)).toEqual( + ["render"], + ); + }); + it.each([ ["running", "running", undefined], ["failed", "failed", { content: [{ type: "text", text: "Invalid HTML" }], isError: true }], diff --git a/apps/web/src/session-logic.ts b/apps/web/src/session-logic.ts index b504606b4d5e1..2d4847de59417 100644 --- a/apps/web/src/session-logic.ts +++ b/apps/web/src/session-logic.ts @@ -757,7 +757,7 @@ export function deriveTimelineEntriesFromVisibleTurnItems( const htmlRender = item.type === "dynamic_tool" && item.status === "completed" - ? htmlRenderFromToolItem(item) + ? htmlRenderFromToolItem(item, row.sourceThreadId) : undefined; if (htmlRender !== undefined) { entries.push({ diff --git a/packages/shared/package.json b/packages/shared/package.json index 50e15ca422c54..102f9c0fc2314 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -127,6 +127,10 @@ "types": "./src/toolOutput.ts", "import": "./src/toolOutput.ts" }, + "./attachmentIds": { + "types": "./src/attachmentIds.ts", + "import": "./src/attachmentIds.ts" + }, "./htmlRender": { "types": "./src/htmlRender.ts", "import": "./src/htmlRender.ts" diff --git a/packages/shared/src/attachmentIds.ts b/packages/shared/src/attachmentIds.ts new file mode 100644 index 0000000000000..db7332c07cd5d --- /dev/null +++ b/packages/shared/src/attachmentIds.ts @@ -0,0 +1,35 @@ +const THREAD_SEGMENT_MAX_CHARS = 80; +const ATTACHMENT_ID_SUFFIX = + /^-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}(?:-[a-z0-9]{1,10})?$/i; + +export const PENDING_ATTACHMENT_THREAD_SEGMENT = "pending"; + +/** The thread part of every attachment id minted for `threadId`. */ +export function toSafeThreadAttachmentSegment(threadId: string): string | null { + const segment = threadId + .trim() + .toLowerCase() + .replace(/[^a-z0-9_-]+/gi, "-") + .replace(/-+/g, "-") + .replace(/^[-_]+|[-_]+$/g, "") + .slice(0, THREAD_SEGMENT_MAX_CHARS) + .replace(/[-_]+$/g, ""); + if (segment.length === 0) { + return null; + } + return segment === PENDING_ATTACHMENT_THREAD_SEGMENT ? "_pending" : segment; +} + +/** + * Whether `attachmentId` was minted for `threadId`. A thread segment can + * itself hold dashes, so the rest of the id must be exactly the uuid and + * optional extension that follow it. + */ +export function attachmentIdBelongsToThread(attachmentId: string, threadId: string): boolean { + const segment = toSafeThreadAttachmentSegment(threadId); + return ( + segment !== null && + attachmentId.toLowerCase().startsWith(segment) && + ATTACHMENT_ID_SUFFIX.test(attachmentId.slice(segment.length)) + ); +} diff --git a/packages/shared/src/htmlRender.test.ts b/packages/shared/src/htmlRender.test.ts index 0d1082234734b..b46f1cc3ec32c 100644 --- a/packages/shared/src/htmlRender.test.ts +++ b/packages/shared/src/htmlRender.test.ts @@ -139,7 +139,14 @@ describe("htmlRenderFromToolItem", () => { height: 420, }; const output = [{ type: "text", text: JSON.stringify({ htmlRender: executeRender }) }]; - expect(htmlRenderFromToolItem({ toolName: "execute", output })).toEqual(executeRender); + expect(htmlRenderFromToolItem({ toolName: "execute", output }, "thread-1")).toEqual( + executeRender, + ); + // The script wrote this output, so a page published for another thread is not shown, + // including one whose thread segment merely starts with this thread's. + expect(htmlRenderFromToolItem({ toolName: "execute", output }, "thread-2")).toBeUndefined(); + expect(htmlRenderFromToolItem({ toolName: "execute", output }, "thread")).toBeUndefined(); + expect(htmlRenderFromToolItem({ toolName: "execute", output })).toBeUndefined(); }); it("reads a bare reference an execute script returned", () => { @@ -149,12 +156,17 @@ describe("htmlRenderFromToolItem", () => { height: 420, }; const output = [{ type: "text", text: JSON.stringify(executeRender) }]; - expect(htmlRenderFromToolItem({ toolName: "execute", output })).toEqual(executeRender); + expect(htmlRenderFromToolItem({ toolName: "execute", output }, "thread-1")).toEqual( + executeRender, + ); expect( - htmlRenderFromToolItem({ - toolName: "execute", - output: { attachmentId: "thread-1-upload", title: "Chart", height: 420 }, - }), + htmlRenderFromToolItem( + { + toolName: "execute", + output: { attachmentId: "thread-1-upload", title: "Chart", height: 420 }, + }, + "thread-1", + ), ).toBeUndefined(); }); }); diff --git a/packages/shared/src/toolOutput.ts b/packages/shared/src/toolOutput.ts index 92f8868917f28..d05d58e01ced8 100644 --- a/packages/shared/src/toolOutput.ts +++ b/packages/shared/src/toolOutput.ts @@ -10,6 +10,7 @@ import { type HtmlRenderReference, } from "./htmlRender.ts"; import { MCP_APP_OUTPUT_KEY, readMcpAppReference, type McpAppReference } from "./mcpApp.ts"; +import { attachmentIdBelongsToThread } from "./attachmentIds.ts"; import { resolveT3McpToolId } from "./t3McpToolPresentation.ts"; const MAX_PARSED_BYTES = 16_384; @@ -208,16 +209,27 @@ export function compactDynamicToolOutput(value: unknown): CompactToolOutput | un return Object.keys(output).length === 0 ? undefined : output; } -export function htmlRenderFromToolItem(item: { - readonly toolName: string | null | undefined; - readonly output?: unknown; -}): HtmlRenderReference | undefined { - // OpenCode 2 Code Mode runs T3 tools inside `execute`, so the item keeps - // toolName "execute" even when the script returns html_render's result. - if (item.toolName !== "execute" && resolveT3McpToolId(item.toolName) !== HTML_RENDER_TOOL_NAME) - return undefined; +/** + * `threadId` is the thread the item belongs to. OpenCode 2 Code Mode runs T3 + * tools inside `execute`, whose output is whatever the agent's script + * returned, so a reference read from it only counts when its page was + * published for that thread. + */ +export function htmlRenderFromToolItem( + item: { + readonly toolName: string | null | undefined; + readonly output?: unknown; + }, + threadId?: string, +): HtmlRenderReference | undefined { + const fromScript = item.toolName === "execute"; + if (!fromScript && resolveT3McpToolId(item.toolName) !== HTML_RENDER_TOOL_NAME) return undefined; const output = compactDynamicToolOutput(item.output); - return output?.isError ? undefined : output?.htmlRender; + const render = output?.isError ? undefined : output?.htmlRender; + if (render === undefined || !fromScript) return render; + return threadId !== undefined && attachmentIdBelongsToThread(render.attachmentId, threadId) + ? render + : undefined; } /** From f4c02b8097b9b8dd6e085e7c33993ffcbd6ba313 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 26 Oct 2026 18:54:27 +0000 Subject: [PATCH 10/12] fix: add takeover regression tests for wrapped inline pages Co-Authored-By: Claude Opus 5.5 --- apps/mobile/src/lib/threadActivity.test.ts | 27 ++++++++ .../Adapters/AcpAdapterV2.test.ts | 17 ++++++ apps/web/src/session-logic.test.ts | 35 +++++++++++ .../src/server/runtimeModel.test.ts | 61 +++++++++++++++++++ packages/shared/src/htmlRender.test.ts | 33 ++++++++++ packages/shared/src/toolOutput.test.ts | 12 ++++ 6 files changed, 185 insertions(+) diff --git a/apps/mobile/src/lib/threadActivity.test.ts b/apps/mobile/src/lib/threadActivity.test.ts index d64763aa2788e..d13ca1da1148a 100644 --- a/apps/mobile/src/lib/threadActivity.test.ts +++ b/apps/mobile/src/lib/threadActivity.test.ts @@ -77,6 +77,33 @@ it("keeps historical plan detail accessible from its paged turn item", () => { expect(presented[2]).toMatchObject({ activities: [{ projectedItem: { item } }] }); }); +it("shows a page an execute script returned only when it was published for the row's thread", () => { + const executeCall = (id: string, thread: string) => + ({ + ...base(id, "2026-06-20T00:00:02.000Z", 1), + type: "dynamic_tool", + toolName: "execute", + input: { code: "return await tools.render()" }, + output: JSON.stringify({ + htmlRender: { + attachmentId: `${thread}-00000000-0000-4000-8000-000000000001-html`, + title: "Chart", + height: 420, + }, + }), + }) as OrchestrationV2TurnItem; + const pages = (row: OrchestrationV2ProjectedTurnItem) => + buildThreadFeed([row]).filter((entry) => entry.type === "html-render"); + + expect(pages(projected(executeCall("own", threadId), 0))).toHaveLength(1); + expect(pages(projected(executeCall("foreign", "thread-2"), 0))).toEqual([]); + // An inherited row is checked against the thread it came from, not the one showing it. + expect(pages(projected(executeCall("inherited", sourceThreadId), 0, "inherited"))).toHaveLength( + 1, + ); + expect(pages(projected(executeCall("inherited-local", threadId), 0, "inherited"))).toEqual([]); +}); + it("shows a compact command as one mobile divider before, during, and after compaction", () => { const startedAt = "2026-06-20T00:00:01.000Z"; const prompt = projected({ ...userMessage(), text: "/compact", idleCompaction: "onReturn" }, 0); diff --git a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts index 1cb40e1fab2ed..962c4eed8189d 100644 --- a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts @@ -79,6 +79,7 @@ import { acpScopedNativeId, acpPostSettleContinuationOfferEvidence, acpPostSettleWakeEvidence, + acpMcpToolCallOutput, acpToolCallDiffPatch, makeAcpAdapterV2, type AcpAdapterV2ExtensionContext, @@ -110,6 +111,22 @@ const layerTest = Layer.mergeAll( const ACP_TEST_DRIVER = ProviderDriverKind.make("acp-test"); const decodeUnknownJson = Schema.decodeUnknownOption(Schema.fromJsonString(Schema.Unknown)); +describe("acpMcpToolCallOutput", () => { + it("unwraps Grok's OkayOutput result string", () => { + const inner = JSON.stringify({ htmlRender: { attachmentId: "t-html", title: "T", height: 1 } }); + assert.strictEqual( + acpMcpToolCallOutput({ + type: "MCP", + tool_name: "html_render", + output: { OkayOutput: inner }, + }), + inner, + ); + const nested = { type: "MCP", output: { OkayOutput: { nested: true } } }; + assert.strictEqual(acpMcpToolCallOutput(nested), nested); + }); +}); + describe("acpToolCallDiffPatch", () => { it("builds a patch per file from ACP v1 oldText/newText, with /dev/null for a new file", () => { assert.equal( diff --git a/apps/web/src/session-logic.test.ts b/apps/web/src/session-logic.test.ts index de8b3373e44b2..19d11075d2230 100644 --- a/apps/web/src/session-logic.test.ts +++ b/apps/web/src/session-logic.test.ts @@ -1509,6 +1509,41 @@ describe("HTML renders in the timeline", () => { expect(rowsFor(entries, true)).toEqual(["message", "turn-fold", "attempt-fold", "html-render"]); }); + it("shows a page an execute script returned only when it was published for the row's thread", () => { + const page = (thread: string) => ({ + attachmentId: `${thread}-00000000-0000-4000-8000-000000000001-html`, + title: "Chart", + height: 420, + }); + const executeCall = (render: ReturnType): OrchestrationV2TurnItem => ({ + ...base("render", 2), + type: "dynamic_tool", + toolName: "execute", + input: { code: "return await tools.render()" }, + output: JSON.stringify({ htmlRender: render }), + }); + expect( + turn(executeCall(page("render-thread"))).find((e) => e.kind === "html-render"), + ).toMatchObject({ htmlRender: page("render-thread") }); + const foreign = turn(executeCall(page("other-thread"))); + expect(foreign.some((entry) => entry.kind === "html-render")).toBe(false); + // A delegated child's row is checked against the child, not the thread showing it. + const child = ThreadId.make("child-thread"); + const merged = deriveTimelineEntriesFromVisibleTurnItems({ + visibleTurnItems: [ + { ...visible(executeCall(page("child-thread"))), sourceThreadId: child }, + { + ...visible({ ...executeCall(page("render-thread")), id: TurnItemId.make("render-2") }), + sourceThreadId: child, + }, + ], + optimisticMessages: [], + }); + expect(merged.filter((entry) => entry.kind === "html-render").map((entry) => entry.id)).toEqual( + ["render"], + ); + }); + it.each([ ["running", "running", undefined], ["failed", "failed", { content: [{ type: "text", text: "Invalid HTML" }], isError: true }], diff --git a/packages/provider-acp/src/server/runtimeModel.test.ts b/packages/provider-acp/src/server/runtimeModel.test.ts index 70682ec4824a5..b21981a3e1b4a 100644 --- a/packages/provider-acp/src/server/runtimeModel.test.ts +++ b/packages/provider-acp/src/server/runtimeModel.test.ts @@ -1168,6 +1168,67 @@ describe("extractMcpToolCallIdentity", () => { }); }); + it("recovers T3 identity from CodeBuddy's qualified rawInput.toolName", () => { + const toolCall = toolCallFromUpdate({ + sessionUpdate: "tool_call", + toolCallId: "tool-1", + kind: "Tool", + title: "Tool", + status: "in_progress", + rawInput: { toolName: "mcp__t3-code__html_render", arguments: {} }, + }); + + expect(extractMcpToolCallIdentity(toolCall)).toEqual({ + server: "t3-code", + tool: "html_render", + }); + }); + + it("recovers T3 identity from Devin's namespaced inference tool name", () => { + const toolCall = toolCallFromUpdate({ + sessionUpdate: "tool_call", + toolCallId: "tool-2", + kind: "DeferExecuteTool", + title: "Deferred tool call", + status: "in_progress", + rawInput: { server_name: "other", tool_name: "html_render" }, + _meta: { "cognition.ai/inferenceToolName": "mcp__t3-code__html_render" }, + }); + + expect(extractMcpToolCallIdentity(toolCall)).toEqual({ + server: "t3-code", + tool: "html_render", + }); + }); + + it("recovers T3 identity from Grok's snake_case rawInput and annotated titles", () => { + const snakeCase = toolCallFromUpdate({ + sessionUpdate: "tool_call", + toolCallId: "tool-3", + kind: "UseTool", + title: "UseTool", + status: "in_progress", + rawInput: { server_name: "t3-code", tool_name: "html_render" }, + _meta: { is_mcp_tool_call: true }, + }); + const filePassed = toolCallFromUpdate({ + sessionUpdate: "tool_call", + toolCallId: "tool-4", + kind: "UseTool", + title: "t3-code__html_render — source file: /tmp/mcp-render.json", + status: "in_progress", + rawInput: { tool_name: "html_render", tool_input: { height: 200 }, variant: "UseTool" }, + _meta: { is_mcp_tool_call: true }, + }); + + for (const toolCall of [snakeCase, filePassed]) { + expect(extractMcpToolCallIdentity(toolCall)).toEqual({ + server: "t3-code", + tool: "html_render", + }); + } + }); + it("recovers T3 identity from acp-mcp-call fallback commands", () => { const toolCall = toolCallFromUpdate({ sessionUpdate: "tool_call", diff --git a/packages/shared/src/htmlRender.test.ts b/packages/shared/src/htmlRender.test.ts index 793fde8b8246c..99ea1c95dde69 100644 --- a/packages/shared/src/htmlRender.test.ts +++ b/packages/shared/src/htmlRender.test.ts @@ -131,4 +131,37 @@ describe("htmlRenderFromToolItem", () => { expect(htmlRenderFromToolItem({ toolName, output })).toEqual(reference); } }); + + it("reads the reference from ACP content and bare text block wrappers", () => { + const text = JSON.stringify(result); + for (const output of [ + [{ type: "content", content: { type: "text", text } }], + { type: "text", text }, + ]) { + expect(htmlRenderFromToolItem({ toolName: "t3-code.html_render", output })).toEqual( + reference, + ); + } + }); + + it("shows a page an execute script returned only for the thread it was published for", () => { + const page = { + attachmentId: "thread-1-00000000-0000-4000-8000-000000000001-html", + title: "Chart", + height: 420, + }; + const execute = (value: unknown) => ({ + toolName: "execute", + output: [{ type: "text", text: JSON.stringify(value) }], + }); + expect(htmlRenderFromToolItem(execute({ htmlRender: page }), "thread-1")).toEqual(page); + expect(htmlRenderFromToolItem(execute(page), "thread-1")).toEqual(page); + expect(htmlRenderFromToolItem(execute({ htmlRender: page }), "thread-2")).toBeUndefined(); + expect(htmlRenderFromToolItem(execute(page), "thread")).toBeUndefined(); + expect(htmlRenderFromToolItem(execute(page))).toBeUndefined(); + // Only ids html_render mints are pages; another upload with the same fields is not. + expect( + htmlRenderFromToolItem(execute({ ...page, attachmentId: "thread-1-upload" }), "thread-1"), + ).toBeUndefined(); + }); }); diff --git a/packages/shared/src/toolOutput.test.ts b/packages/shared/src/toolOutput.test.ts index 405905bf2fad9..8544935cbe2ca 100644 --- a/packages/shared/src/toolOutput.test.ts +++ b/packages/shared/src/toolOutput.test.ts @@ -132,6 +132,18 @@ describe("compactDynamicToolOutput", () => { threadId: "known", }); }); + + it("keeps outer metadata when a text field holds no result", () => { + expect(compactDynamicToolOutput({ threadId: "thread-1", text: '{"a":1}' })).toEqual({ + threadId: "thread-1", + }); + expect(compactDynamicToolOutput({ type: "text", threadId: "thread-1", text: "{}" })).toEqual({ + threadId: "thread-1", + }); + expect(compactDynamicToolOutput({ threadId: "thread-1", text: "x".repeat(20_000) })).toEqual({ + threadId: "thread-1", + }); + }); }); describe("toolOutputIndicatesFailure", () => { From 6374f21afcf7c47d6cbc23f398399fc53fa41149 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 26 Oct 2026 19:03:41 +0000 Subject: [PATCH 11/12] fix: show pages that OpenCode execute scripts and ACP agents return inline OpenCode Code Mode runs html_render inside `execute`, so the page reference came back as the script's result (wrapped or bare) and never rendered. `execute` rows now yield a page only when its attachment id was minted for the row's own thread (sourceThreadId, so inherited and delegated rows check their origin), and thread deletion cleans those pages up too. ACP envelopes now normalize: Devin content arrays, CodeBuddy bare text blocks and qualified rawInput.toolName, Devin's namespaced inference tool name, and Grok's OkayOutput result plus snake_case and annotated names. Co-Authored-By: Claude Opus 5.5 --- apps/mobile/src/lib/threadActivity.ts | 2 +- apps/server/src/attachmentStore.ts | 26 ++++------ apps/server/src/guest/GuestShareService.ts | 2 +- .../src/orchestration-v2/ProjectionStore.ts | 7 ++- apps/web/src/session-logic.ts | 2 +- packages/provider-acp/src/server/adapter.ts | 8 +++- .../provider-acp/src/server/runtimeModel.ts | 15 ++++-- packages/shared/package.json | 4 ++ packages/shared/src/attachmentIds.ts | 30 ++++++++++++ packages/shared/src/toolOutput.ts | 48 +++++++++++++++---- 10 files changed, 109 insertions(+), 35 deletions(-) create mode 100644 packages/shared/src/attachmentIds.ts diff --git a/apps/mobile/src/lib/threadActivity.ts b/apps/mobile/src/lib/threadActivity.ts index b9fe76a8b07b2..c60ef51aa090b 100644 --- a/apps/mobile/src/lib/threadActivity.ts +++ b/apps/mobile/src/lib/threadActivity.ts @@ -2189,7 +2189,7 @@ export function buildThreadFeed( const createdAt = DateTime.formatIso(item.startedAt ?? item.updatedAt); const render = item.type === "dynamic_tool" && item.status === "completed" - ? htmlRenderFromToolItem(item) + ? htmlRenderFromToolItem(item, row.sourceThreadId) : undefined; if (render) { const entry: RawThreadFeedEntry = { diff --git a/apps/server/src/attachmentStore.ts b/apps/server/src/attachmentStore.ts index 5d8ab2e90181b..adde9f91f5324 100644 --- a/apps/server/src/attachmentStore.ts +++ b/apps/server/src/attachmentStore.ts @@ -4,6 +4,10 @@ import * as NodeFS from "node:fs"; import * as NodePath from "node:path"; import type { ChatAttachment } from "@t3tools/contracts"; +import { + PENDING_ATTACHMENT_THREAD_SEGMENT, + toSafeThreadAttachmentSegment, +} from "@t3tools/shared/attachmentIds"; import { htmlRenderFromToolItem, mcpAppFromToolItem } from "@t3tools/shared/toolOutput"; import { @@ -13,7 +17,6 @@ import { import { inferImageExtension, SAFE_IMAGE_FILE_EXTENSIONS } from "./imageMime.ts"; const ATTACHMENT_FILENAME_EXTENSIONS = [...SAFE_IMAGE_FILE_EXTENSIONS, ".bin"]; -const ATTACHMENT_ID_THREAD_SEGMENT_MAX_CHARS = 80; const ATTACHMENT_ID_THREAD_SEGMENT_PATTERN = "[a-z0-9_]+(?:-[a-z0-9_]+)*"; const ATTACHMENT_ID_UUID_PATTERN = "[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}"; const ATTACHMENT_ID_FILE_EXTENSION_PATTERN = "[a-z0-9]{1,10}"; @@ -22,25 +25,11 @@ const ATTACHMENT_ID_PATTERN = new RegExp( "i", ); -export const PENDING_ATTACHMENT_THREAD_SEGMENT = "pending"; +export { PENDING_ATTACHMENT_THREAD_SEGMENT, toSafeThreadAttachmentSegment }; + const PENDING_ATTACHMENT_MAX_AGE_MS = 24 * 60 * 60 * 1000; const PARTIAL_UPLOAD_MAX_AGE_MS = 60 * 60 * 1000; -export function toSafeThreadAttachmentSegment(threadId: string): string | null { - const segment = threadId - .trim() - .toLowerCase() - .replace(/[^a-z0-9_-]+/gi, "-") - .replace(/-+/g, "-") - .replace(/^[-_]+|[-_]+$/g, "") - .slice(0, ATTACHMENT_ID_THREAD_SEGMENT_MAX_CHARS) - .replace(/[-_]+$/g, ""); - if (segment.length === 0) { - return null; - } - return segment === PENDING_ATTACHMENT_THREAD_SEGMENT ? "_pending" : segment; -} - export function attachmentFileExtension(fileName: string): string { const extension = NodePath.extname(fileName).toLowerCase(); // .part is reserved for pending uploads; the stale-upload sweep would delete stored files. @@ -133,7 +122,8 @@ export function threadHtmlRenderAttachmentIds( if (segment === null) return []; return Array.from(items).flatMap((item) => { const attachmentId = - htmlRenderFromToolItem(item)?.attachmentId ?? mcpAppFromToolItem(item)?.attachmentId; + htmlRenderFromToolItem(item, threadId)?.attachmentId ?? + mcpAppFromToolItem(item)?.attachmentId; return attachmentId !== undefined && parseThreadSegmentFromAttachmentId(attachmentId) === segment ? [attachmentId] diff --git a/apps/server/src/guest/GuestShareService.ts b/apps/server/src/guest/GuestShareService.ts index 1f65217e45212..35e16d36e04d4 100644 --- a/apps/server/src/guest/GuestShareService.ts +++ b/apps/server/src/guest/GuestShareService.ts @@ -576,7 +576,7 @@ const make = Effect.gen(function* () { const render = Effect.fn("GuestShareService.render")(function* (session: string, itemId: string) { const { thread, item } = yield* threadItem(session, itemId); if (item.type !== "dynamic_tool") return yield* unavailable; - const reference = htmlRenderFromToolItem(item); + const reference = htmlRenderFromToolItem(item, thread.id); const [attachmentId] = threadHtmlRenderAttachmentIds(thread.id, [item]); const path = attachmentId === undefined diff --git a/apps/server/src/orchestration-v2/ProjectionStore.ts b/apps/server/src/orchestration-v2/ProjectionStore.ts index ed064325bfe16..250e3ecbc0290 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.ts @@ -5323,7 +5323,12 @@ export const layer: Layer.Layer = FROM orchestration_v2_projection_turn_items WHERE thread_id = ${threadId} AND type = 'dynamic_tool' - AND (payload_json LIKE '%htmlRender%' OR payload_json LIKE ${`%${MCP_APP_OUTPUT_KEY}%`}) + AND ( + payload_json LIKE '%htmlRender%' + -- An execute script can return a page's reference without the htmlRender key. + OR payload_json LIKE '%-html%' + OR payload_json LIKE ${`%${MCP_APP_OUTPUT_KEY}%`} + ) `, // A guest's waiting files are pending uploads that no message row holds yet. sql<{ id: string }>` diff --git a/apps/web/src/session-logic.ts b/apps/web/src/session-logic.ts index f386a858818b8..b07e6d98be525 100644 --- a/apps/web/src/session-logic.ts +++ b/apps/web/src/session-logic.ts @@ -755,7 +755,7 @@ export function deriveTimelineEntriesFromVisibleTurnItems( const htmlRender = item.type === "dynamic_tool" && item.status === "completed" - ? htmlRenderFromToolItem(item) + ? htmlRenderFromToolItem(item, row.sourceThreadId) : undefined; if (htmlRender !== undefined) { entries.push({ diff --git a/packages/provider-acp/src/server/adapter.ts b/packages/provider-acp/src/server/adapter.ts index b3ce13af0a705..b0adb4fa0188c 100644 --- a/packages/provider-acp/src/server/adapter.ts +++ b/packages/provider-acp/src/server/adapter.ts @@ -700,10 +700,14 @@ function makeProviderThread(input: { }; } -function acpMcpToolCallOutput(rawOutput: unknown): unknown { +export function acpMcpToolCallOutput(rawOutput: unknown): unknown { const record = unknownRecord(rawOutput); if (record === undefined) return rawOutput; - if (!("result" in record) && !("error" in record)) return rawOutput; + if (!("result" in record) && !("error" in record)) { + // Grok serializes Rust's Result: `{ type: "MCP", output: { OkayOutput: "" } }`. + const okay = unknownRecord(record.output)?.OkayOutput; + return typeof okay === "string" ? okay : rawOutput; + } const result = unknownRecord(record.result); const resultOutput = result === undefined ? undefined : (result.structuredContent ?? result.content ?? undefined); diff --git a/packages/provider-acp/src/server/runtimeModel.ts b/packages/provider-acp/src/server/runtimeModel.ts index 15c583f67de42..a059edc7faaeb 100644 --- a/packages/provider-acp/src/server/runtimeModel.ts +++ b/packages/provider-acp/src/server/runtimeModel.ts @@ -1082,8 +1082,11 @@ export function extractMcpToolCallIdentity( ): AcpMcpToolCallIdentity | undefined { const rawInput = isRecord(toolCall.data.rawInput) ? toolCall.data.rawInput : undefined; const meta = isRecord(toolCall.data.meta) ? toolCall.data.meta : undefined; - const server = typeof rawInput?.server === "string" ? rawInput.server.trim() : ""; - const tool = typeof rawInput?.tool === "string" ? rawInput.tool.trim() : ""; + // Grok spells these snake_case. + const rawServer = rawInput?.server ?? rawInput?.server_name; + const rawTool = rawInput?.tool ?? rawInput?.tool_name; + const server = typeof rawServer === "string" ? rawServer.trim() : ""; + const tool = typeof rawTool === "string" ? rawTool.trim() : ""; if (meta?.is_mcp_tool_call === true && server.length > 0 && tool.length > 0) { return { server, tool }; } @@ -1126,10 +1129,16 @@ export function extractMcpToolCallIdentity( meta?.toolName, claudeCode?.toolName, gooseToolCall?.toolName, + meta?.["cognition.ai/inferenceToolName"], + // CodeBuddy keeps the qualified `mcp____` name here. + rawInput?.toolName, toolCall.data.title, ].filter((value): value is string => typeof value === "string"); for (const candidate of candidates) { - const trimmed = candidate.trim(); + // Grok annotates file-passed calls: "t3-code__html_render — source file: /tmp/x.json". + // oxlint-disable-next-line t3code/no-em-dash-copy -- Matches Grok's title, not product copy. + const annotation = candidate.indexOf(" — source file:"); + const trimmed = (annotation === -1 ? candidate : candidate.slice(0, annotation)).trim(); const qualified = /^mcp__(.+?)__(.+)$/i.exec(trimmed); if (qualified?.[1] && qualified[2] && !/^t3[-_ ]?code$/i.test(qualified[1])) return { server: qualified[1], tool: qualified[2] }; diff --git a/packages/shared/package.json b/packages/shared/package.json index f968d276883e1..1a0f4a7e44912 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -123,6 +123,10 @@ "types": "./src/toolOutput.ts", "import": "./src/toolOutput.ts" }, + "./attachmentIds": { + "types": "./src/attachmentIds.ts", + "import": "./src/attachmentIds.ts" + }, "./htmlRender": { "types": "./src/htmlRender.ts", "import": "./src/htmlRender.ts" diff --git a/packages/shared/src/attachmentIds.ts b/packages/shared/src/attachmentIds.ts new file mode 100644 index 0000000000000..6d66a718f0e31 --- /dev/null +++ b/packages/shared/src/attachmentIds.ts @@ -0,0 +1,30 @@ +const THREAD_SEGMENT_MAX_CHARS = 80; +const ATTACHMENT_ID_SUFFIX = + /^-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}(?:-[a-z0-9]{1,10})?$/i; + +export const PENDING_ATTACHMENT_THREAD_SEGMENT = "pending"; + +export function toSafeThreadAttachmentSegment(threadId: string): string | null { + const segment = threadId + .trim() + .toLowerCase() + .replace(/[^a-z0-9_-]+/gi, "-") + .replace(/-+/g, "-") + .replace(/^[-_]+|[-_]+$/g, "") + .slice(0, THREAD_SEGMENT_MAX_CHARS) + .replace(/[-_]+$/g, ""); + if (segment.length === 0) { + return null; + } + return segment === PENDING_ATTACHMENT_THREAD_SEGMENT ? "_pending" : segment; +} + +/** Thread segments can hold dashes, so everything after the segment must be exactly the uuid and extension. */ +export function attachmentIdBelongsToThread(attachmentId: string, threadId: string): boolean { + const segment = toSafeThreadAttachmentSegment(threadId); + return ( + segment !== null && + attachmentId.toLowerCase().startsWith(segment) && + ATTACHMENT_ID_SUFFIX.test(attachmentId.slice(segment.length)) + ); +} diff --git a/packages/shared/src/toolOutput.ts b/packages/shared/src/toolOutput.ts index 2a80641d13b5e..3ea869c976296 100644 --- a/packages/shared/src/toolOutput.ts +++ b/packages/shared/src/toolOutput.ts @@ -9,6 +9,7 @@ import { readHtmlRenderReference, type HtmlRenderReference, } from "./htmlRender.ts"; +import { attachmentIdBelongsToThread } from "./attachmentIds.ts"; import { MCP_APP_OUTPUT_KEY, readMcpAppReference, type McpAppReference } from "./mcpApp.ts"; import { resolveT3McpToolId } from "./t3McpToolPresentation.ts"; @@ -75,7 +76,7 @@ function readResult(value: unknown, budget: ResultReadBudget, depth = 0): Result let data: ResultEnvelope["data"]; let failed = false; for (const block of value) { - const text = Predicate.isObject(block) ? block.text : undefined; + const text = Predicate.isObject(block) ? (block.text ?? block.content) : undefined; const result = readResult( Predicate.isObject(text) ? (text.text ?? text) : text, budget, @@ -88,6 +89,7 @@ function readResult(value: unknown, budget: ResultReadBudget, depth = 0): Result return { ...(data === undefined ? {} : { data }), failed }; } if (!Predicate.isObject(value)) return { failed: false }; + if (isTextContentBlock(value)) return readResult(value.text, budget, depth + 1); const failed = value.isError === true || value.is_error === true || @@ -101,6 +103,19 @@ function readResult(value: unknown, budget: ResultReadBudget, depth = 0): Result return { data: value, failed }; } +const TEXT_CONTENT_BLOCK_KEYS = new Set(["type", "text", "annotations", "_meta"]); + +/** Some ACP agents persist a bare MCP text block; anything with extra keys keeps its own metadata. */ +function isTextContentBlock( + value: Record, +): value is Record & { text: string } { + return ( + value.type === "text" && + typeof value.text === "string" && + Object.keys(value).every((key) => TEXT_CONTENT_BLOCK_KEYS.has(key)) + ); +} + function boundedId(value: unknown): string | undefined { if (typeof value !== "string" || value.length > MAX_ID_LENGTH || value.trim() === "") { return undefined; @@ -123,7 +138,12 @@ export function compactDynamicToolOutput(value: unknown): CompactToolOutput | un if (id !== undefined) output[key] = id; } if (data.status === "rolled_back") output.status = "rolled_back"; - const htmlRender = readHtmlRenderReference(data.htmlRender); + // A Code Mode script may return html_render's reference unwrapped; only html_render mints `-html` ids. + const htmlRender = + readHtmlRenderReference(data.htmlRender) ?? + (typeof data.attachmentId === "string" && data.attachmentId.endsWith("-html") + ? readHtmlRenderReference(data) + : undefined); if (htmlRender !== undefined) output.htmlRender = htmlRender; const mcpApp = readMcpAppReference(data[MCP_APP_OUTPUT_KEY]); if (mcpApp !== undefined) output[MCP_APP_OUTPUT_KEY] = mcpApp; @@ -176,13 +196,25 @@ export function compactDynamicToolOutput(value: unknown): CompactToolOutput | un return Object.keys(output).length === 0 ? undefined : output; } -export function htmlRenderFromToolItem(item: { - readonly toolName: string | null | undefined; - readonly output?: unknown; -}): HtmlRenderReference | undefined { - if (resolveT3McpToolId(item.toolName) !== HTML_RENDER_TOOL_NAME) return undefined; +/** + * OpenCode Code Mode runs T3 tools inside `execute`, whose output is whatever the script returned, + * so its page only counts when it was published for `threadId`, the thread that owns the item. + */ +export function htmlRenderFromToolItem( + item: { + readonly toolName: string | null | undefined; + readonly output?: unknown; + }, + threadId?: string, +): HtmlRenderReference | undefined { + const fromScript = item.toolName === "execute"; + if (!fromScript && resolveT3McpToolId(item.toolName) !== HTML_RENDER_TOOL_NAME) return undefined; const output = compactDynamicToolOutput(item.output); - return output?.isError ? undefined : output?.htmlRender; + const render = output?.isError ? undefined : output?.htmlRender; + if (render === undefined || !fromScript) return render; + return threadId !== undefined && attachmentIdBelongsToThread(render.attachmentId, threadId) + ? render + : undefined; } /** Accept app references only when server and tool match the item, so tools cannot impersonate another app source. */ From 34fcb08ce3146c0b6424d676b1d05ec8b49014c4 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 27 Oct 2026 01:00:47 +0000 Subject: [PATCH 12/12] fix(acp): preserve tool identity and unwrap child MCP output --- .../Adapters/AcpAdapterV2.test.ts | 27 +++++++++++++++++++ packages/provider-acp/src/server/adapter.ts | 7 ++++- .../src/server/runtimeModel.test.ts | 13 +++++++++ .../provider-acp/src/server/runtimeModel.ts | 6 +++-- 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts index 962c4eed8189d..aee3255e3644c 100644 --- a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts @@ -32,6 +32,7 @@ import { GrokSettings } from "@t3tools/provider-grok/settings"; import * as HostProcess from "@t3tools/shared/HostProcess"; import { deriveBlockedOnThreadWait } from "@t3tools/shared/orchestrationV2PendingBackgroundWork"; import { resolveSelfInvocation } from "@t3tools/shared/nodeRuntime"; +import { htmlRenderFromToolItem } from "@t3tools/shared/toolOutput"; import * as DateTime from "effect/DateTime"; import * as Deferred from "effect/Deferred"; import type * as Duration from "effect/Duration"; @@ -1555,6 +1556,7 @@ describe("AcpAdapterV2", () => { it.effect("keeps Devin parent paragraphs intact while projecting native child work", () => Effect.gen(function* () { + const childPage = { attachmentId: "child-html", title: "Child page", height: 120 }; const childProcessSpawner = yield* ChildProcessSpawner.ChildProcessSpawner; const path = yield* Path.Path; const mockAgentPath = yield* path.fromFileUrl( @@ -1666,6 +1668,21 @@ describe("AcpAdapterV2", () => { "cognition.ai/subagent_context": { parentAgentId: "child-a" }, }, }, + { + sessionUpdate: "tool_call", + toolCallId: "child-html", + title: "Render page", + status: "completed", + rawInput: { server: "t3-code", tool: "html_render", arguments: {} }, + rawOutput: { + type: "MCP", + output: { OkayOutput: JSON.stringify({ htmlRender: childPage }) }, + }, + _meta: { + is_mcp_tool_call: true, + "cognition.ai/subagent_context": { parentAgentId: "child-a" }, + }, + }, { sessionUpdate: "tool_call", toolCallId: "child-mcp-fallback", @@ -1838,6 +1855,16 @@ describe("AcpAdapterV2", () => { item.type === "dynamic_tool" && item.toolName === "weather.get_weather", ); + const childHtml = items.findLast( + (item) => + item.threadId === task?.childThreadId && + item.type === "dynamic_tool" && + item.toolName === "t3-code.html_render", + ); + assert.deepEqual( + childHtml?.type === "dynamic_tool" ? htmlRenderFromToolItem(childHtml) : undefined, + childPage, + ); for (const item of [parentMcp, childMcp]) { assert.equal(item?.title, "get weather"); assert.deepEqual(item?.toolSource, { diff --git a/packages/provider-acp/src/server/adapter.ts b/packages/provider-acp/src/server/adapter.ts index b0adb4fa0188c..4971bc84b7522 100644 --- a/packages/provider-acp/src/server/adapter.ts +++ b/packages/provider-acp/src/server/adapter.ts @@ -2672,7 +2672,12 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* ( ? (merged.title ?? merged.kind ?? "Tool") : `${mcpIdentity.server}.${mcpIdentity.tool}`, input: merged.data.rawInput ?? null, - output: merged.data.rawOutput ?? merged.data.content ?? null, + output: + (mcpIdentity === undefined + ? merged.data.rawOutput + : acpMcpToolCallOutput(merged.data.rawOutput)) ?? + merged.data.content ?? + null, }, }); }); diff --git a/packages/provider-acp/src/server/runtimeModel.test.ts b/packages/provider-acp/src/server/runtimeModel.test.ts index b21981a3e1b4a..1fa36c6d39adf 100644 --- a/packages/provider-acp/src/server/runtimeModel.test.ts +++ b/packages/provider-acp/src/server/runtimeModel.test.ts @@ -1184,6 +1184,19 @@ describe("extractMcpToolCallIdentity", () => { }); }); + it("ignores an unqualified rawInput.toolName such as a ToolSearch query", () => { + const toolCall = toolCallFromUpdate({ + sessionUpdate: "tool_call", + toolCallId: "tool-1", + kind: "search", + title: "ToolSearch", + status: "in_progress", + rawInput: { toolName: "html_render", query: "render html" }, + }); + + expect(extractMcpToolCallIdentity(toolCall)).toBeUndefined(); + }); + it("recovers T3 identity from Devin's namespaced inference tool name", () => { const toolCall = toolCallFromUpdate({ sessionUpdate: "tool_call", diff --git a/packages/provider-acp/src/server/runtimeModel.ts b/packages/provider-acp/src/server/runtimeModel.ts index a059edc7faaeb..d8d3adcd00b12 100644 --- a/packages/provider-acp/src/server/runtimeModel.ts +++ b/packages/provider-acp/src/server/runtimeModel.ts @@ -1130,8 +1130,10 @@ export function extractMcpToolCallIdentity( claudeCode?.toolName, gooseToolCall?.toolName, meta?.["cognition.ai/inferenceToolName"], - // CodeBuddy keeps the qualified `mcp____` name here. - rawInput?.toolName, + // CodeBuddy keeps the qualified `mcp____` name here; built-ins such as ToolSearch put bare names here. + typeof rawInput?.toolName === "string" && /^mcp__.+?__.+$/i.test(rawInput.toolName.trim()) + ? rawInput.toolName + : undefined, toolCall.data.title, ].filter((value): value is string => typeof value === "string"); for (const candidate of candidates) {