diff --git a/docs/chatgpt-coding-workflow.md b/docs/chatgpt-coding-workflow.md index f6826617..c8919ee2 100644 --- a/docs/chatgpt-coding-workflow.md +++ b/docs/chatgpt-coding-workflow.md @@ -193,11 +193,11 @@ the review point automatically. Reusing a workspace does not change this workflow. The model-facing result stays compact: DevSpace returns the workspace ID, a -Git-backed `reviewRef`, and the summary text. MCP Apps hosts receive the full -file list and patch in result metadata for immediate rendering. If a host later -restores only the structured result, the review card can reopen that exact -`reviewRef` from DevSpace's Git review history without advancing the current -review point. +Git-backed `reviewRef`, and the summary text. The widget uses that structured +result to reopen the exact review from DevSpace's Git review history and render +the file list and patch without exposing them to the model. The same path is +used for the initial render and after a host reload, and reopening a review does +not advance the current review point. For local inspection, run `devspace show-changes `. Add `--json` to include the parsed summary, file list, and patch. diff --git a/docs/gotchas.md b/docs/gotchas.md index 5f628867..b2b006d8 100644 --- a/docs/gotchas.md +++ b/docs/gotchas.md @@ -252,7 +252,7 @@ metadata and only show text results; `show_changes` remains available there. If both cards are missing in ChatGPT, confirm that `ui.enabled` is not `false` in `~/.devspace/config.jsonc` and reconnect the MCP server. -Historical `show_changes` cards use the `reviewRef` in their structured result -to recover the exact Git-backed review when a host reloads the app without its -original result metadata. `open_workspace` can rebuild its card directly from +`show_changes` cards use the `reviewRef` in their structured result to recover +the exact Git-backed review. That same recovery path is used for the initial +render and after a host reload. `open_workspace` rebuilds its card directly from its structured result. diff --git a/src/server.test.ts b/src/server.test.ts index 127abba1..fddd309b 100644 --- a/src/server.test.ts +++ b/src/server.test.ts @@ -74,7 +74,7 @@ test("open_workspace reports aggregate review availability", async (t) => { assert.deepEqual(gitReview, { available: true }); }); -test("show_changes keeps model output compact and preserves the rich review card", async (t) => { +test("show_changes keeps model output compact and restores rich review data on demand", async (t) => { const context = await fixture(t, { git: true, uiEnabled: false }); const opened = structuredContent( await callOpen(context.client, context.project, "review"), @@ -88,7 +88,7 @@ test("show_changes keeps model output compact and preserves the rich review card arguments: { workspaceId }, }); const structured = structuredContent(review); - assert.equal((review._meta as Record | undefined)?.tool, undefined); + assert.equal(review._meta, undefined); assert.equal(structured.workspaceId, workspaceId); assert.match(structured.reviewRef as string, /^[0-9a-f]{40,64}$/); @@ -97,13 +97,17 @@ test("show_changes keeps model output compact and preserves the rich review card assert.equal("files" in structured, false); assert.equal("patch" in structured, false); - const card = responseCard(review); - assert.deepEqual(card.summary, { + const restored = structuredContent(await context.client.callTool({ + name: "show_changes", + arguments: { workspaceId }, + _meta: { "devspace/reviewRef": structured.reviewRef }, + } as Parameters[0])); + assert.deepEqual(restored.summary, { files: 1, additions: 1, removals: 1, }); - assert.deepEqual(card.files, [ + assert.deepEqual(restored.files, [ { path: "README.md", type: "change", @@ -111,19 +115,18 @@ test("show_changes keeps model output compact and preserves the rich review card removals: 1, }, ]); - assert.match( - ((card.payload as { patch?: string } | undefined)?.patch) ?? "", - /-hello\n\+goodbye/, - ); + assert.match(restored.patch as string, /-hello\n\+goodbye/); const tools = await context.client.listTools(); - const outputProperties = tools.tools.find((tool) => tool.name === "show_changes") - ?.outputSchema?.properties; + const showChangesSchema = tools.tools.find((tool) => tool.name === "show_changes") + ?.outputSchema; + const outputProperties = showChangesSchema?.properties; assert.ok(outputProperties && "workspaceId" in outputProperties); assert.ok(outputProperties && "reviewRef" in outputProperties); assert.equal(outputProperties && "summary" in outputProperties, false); assert.equal(outputProperties && "files" in outputProperties, false); assert.equal(outputProperties && "patch" in outputProperties, false); + assert.notEqual(showChangesSchema?.additionalProperties, false); const inputProperties = tools.tools.find((tool) => tool.name === "show_changes") ?.inputSchema?.properties; assert.equal(inputProperties && "reviewRef" in inputProperties, false); @@ -151,30 +154,31 @@ test("show_changes can reopen a historical review without advancing the checkpoi _meta: { "devspace/reviewRef": reviewRef }, } as Parameters[0]); assert.equal(structuredContent(reopened).reviewRef, reviewRef); - assert.match( - (((responseCard(reopened).payload as { patch?: string } | undefined)?.patch) ?? ""), - /\+first/, - ); + assert.match(structuredContent(reopened).patch as string, /\+first/); const current = await context.client.callTool({ name: "show_changes", arguments: { workspaceId }, }); - assert.match( - (((responseCard(current).payload as { patch?: string } | undefined)?.patch) ?? ""), - /-first\n\+second/, - ); + const currentStructured = structuredContent(current); + assert.equal("patch" in currentStructured, false); + const currentRestored = structuredContent(await context.client.callTool({ + name: "show_changes", + arguments: { workspaceId }, + _meta: { "devspace/reviewRef": currentStructured.reviewRef }, + } as Parameters[0])); + assert.match(currentRestored.patch as string, /-first\n\+second/); }); -test("open_workspace keeps lifecycle flags out of model output and preserves complete card metadata", async (t) => { +test("open_workspace keeps lifecycle flags and card metadata out of its result", async (t) => { const providerNote = "available"; const context = await fixture(t, { localAgentProviders: [{ name: "codex", available: true, note: providerNote }], }); const first = await callOpen(context.client, context.project, "chat-1"); const repeated = await callOpen(context.client, context.project, "chat-1"); - assert.equal((first._meta as Record | undefined)?.tool, undefined); - assert.equal((repeated._meta as Record | undefined)?.tool, undefined); + assert.equal(first._meta, undefined); + assert.equal(repeated._meta, undefined); const tools = await context.client.listTools(); const openTool = tools.tools.find((tool) => tool.name === "open_workspace"); @@ -214,19 +218,9 @@ test("open_workspace keeps lifecycle flags out of model output and preserves com assert.equal(repeatedStructured.skillDiagnostics, undefined); assert.equal("workspaceReused" in repeatedStructured, false); assert.equal("includeBootstrapContext" in repeatedStructured, false); - - const card = responseCard(repeated); - assert.equal(card.workspaceReused, true); - assert.equal(card.includeBootstrapContext, false); - assert.ok(Array.isArray(card.agentsFiles)); - assert.ok(Array.isArray(card.availableAgentsFiles)); - assert.ok(Array.isArray(card.skills)); - assert.ok(Array.isArray(card.agentProviders)); - assert.equal( - (card.agentProviders as Array>)[0]?.note, - providerNote, - ); - assert.ok(Array.isArray(card.agents)); + assert.equal(repeatedStructured.root, context.project); + assert.equal(repeatedStructured.mode, "checkout"); + assert.match(repeatedStructured.instruction as string, /already open/i); }); test("open_workspace refreshes provider availability for each catalog", async (t) => { @@ -531,11 +525,3 @@ function responseText(result: Awaited>): string { assert.equal(typeof first?.text, "string"); return first?.text as string; } - -function responseCard(result: Awaited>): Record { - const metadata = result._meta; - assert.ok(metadata && typeof metadata === "object"); - const card = (metadata as Record).card; - assert.ok(card && typeof card === "object"); - return card as Record; -} diff --git a/src/server.ts b/src/server.ts index 9e7ded7f..3507675f 100644 --- a/src/server.ts +++ b/src/server.ts @@ -489,33 +489,6 @@ export function createMcpServer( return { content: resultContent, - _meta: { - card: { - workspaceId: workspace.id, - root: workspace.root, - path: workspace.root, - mode: workspace.mode, - workspaceReused, - includeBootstrapContext, - sourceRoot: workspace.sourceRoot, - worktree: workspace.worktree, - agentsFiles: cardAgentsFiles, - availableAgentsFiles: cardAvailableAgentsFiles, - skills: cardSkills, - agentProviders: cardAgentProviders, - agents: cardAgents, - review, - instruction: cardInstruction, - summary: { - mode: workspace.mode, - agentsFiles: cardAgentsFiles.length, - availableAgentsFiles: cardAvailableAgentsFiles.length, - skills: cardSkills.length, - agentProviders: cardAgentProviders.length, - agents: cardAgents.length, - }, - }, - }, structuredContent: { workspaceId: workspace.id, root: workspace.root, @@ -637,10 +610,10 @@ export function createMcpServer( inputSchema: { workspaceId: z.string().describe(workspaceIdDescription), }, - outputSchema: resultOutputSchema({ + outputSchema: z.looseObject(resultOutputSchema({ workspaceId: z.string(), reviewRef: z.string().regex(/^[0-9a-f]{40,64}$/), - }), + })), ...workspaceAppDescriptorMeta(config), annotations: { readOnlyHint: true }, }, @@ -672,20 +645,17 @@ export function createMcpServer( return { content, - _meta: { - card: { - workspaceId, - summary: review.summary, - files: review.files, - payload: { - patch: review.patch, - }, - }, - }, structuredContent: { workspaceId, reviewRef: review.reviewRef, result: contentText(content), + ...(reviewRef + ? { + summary: review.summary, + files: review.files, + patch: review.patch, + } + : {}), }, }; }, diff --git a/src/ui/tool-result.test.ts b/src/ui/tool-result.test.ts index b7c5315d..1aa7e1f2 100644 --- a/src/ui/tool-result.test.ts +++ b/src/ui/tool-result.test.ts @@ -28,21 +28,16 @@ test("workspace cards can be rebuilt from structured content without result meta assert.equal(decoded.card.summary?.agentsFiles, 1); }); -test("review results use rich metadata when the host provides it", () => { +test("review hydration renders from rich structured content", () => { const decoded = decodeToolResult({ content: [], structuredContent: { workspaceId: "ws_1", reviewRef: "a".repeat(40), result: "Changed 1 file (+1 -0).", - }, - _meta: { - card: { - workspaceId: "ws_1", - summary: { files: 1, additions: 1, removals: 0 }, - files: [{ path: "new.txt", type: "new", additions: 1, removals: 0 }], - payload: { patch: "diff --git ..." }, - }, + summary: { files: 1, additions: 1, removals: 0 }, + files: [{ path: "new.txt", type: "new", additions: 1, removals: 0 }], + patch: "diff --git ...", }, }); @@ -53,7 +48,7 @@ test("review results use rich metadata when the host provides it", () => { assert.equal(decoded.card.payload?.patch, "diff --git ..."); }); -test("review structured content becomes a reload reference when metadata is missing", () => { +test("compact review structured content becomes a recovery reference", () => { const decoded = decodeToolResult({ content: [], structuredContent: { @@ -70,7 +65,7 @@ test("review structured content becomes a reload reference when metadata is miss }); }); -test("incomplete review metadata falls back to the durable review reference", () => { +test("result metadata is ignored for review rendering", () => { const decoded = decodeToolResult({ content: [], structuredContent: { @@ -78,7 +73,14 @@ test("incomplete review metadata falls back to the durable review reference", () reviewRef: "e".repeat(40), result: "Changed 1 file (+1 -0).", }, - _meta: { card: {} }, + _meta: { + card: { + workspaceId: "ws_1", + summary: { files: 1, additions: 1, removals: 0 }, + files: [{ path: "new.txt", type: "new", additions: 1, removals: 0 }], + payload: { patch: "should not be used" }, + }, + }, }); assert.deepEqual(decoded, { @@ -106,7 +108,7 @@ test("older review results can reload from their structured patch", () => { assert.equal(decoded.card.payload?.patch, "diff --git a/new.txt b/new.txt"); }); -test("ChatGPT globals restore structured output and hidden MCP result metadata together", () => { +test("ChatGPT globals restore tool output without carrying result metadata", () => { const fullResult: CallToolResult = { content: [{ type: "text", text: "Changed 1 file." }], structuredContent: { stale: true }, @@ -128,28 +130,28 @@ test("ChatGPT globals restore structured output and hidden MCP result metadata t reviewRef: "c".repeat(40), result: "Changed 1 file.", }); - assert.deepEqual(restored?._meta, fullResult._meta); + assert.equal(restored?._meta, undefined); }); -test("ChatGPT globals also accept result metadata exposed directly", () => { +test("ChatGPT globals can recover structured output from the MCP result envelope", () => { const restored = toolResultFromChatGptGlobals({ - toolOutput: { - workspaceId: "ws_1", - reviewRef: "d".repeat(40), - result: "Changed 1 file.", - }, toolResponseMetadata: { - card: { - workspaceId: "ws_1", - summary: { files: 1, additions: 1, removals: 0 }, + mcp_tool_result: { + content: [{ type: "text", text: "Changed 1 file." }], + structuredContent: { + workspaceId: "ws_1", + reviewRef: "d".repeat(40), + result: "Changed 1 file.", + }, + _meta: { card: { ignored: true } }, }, }, }); - assert.deepEqual(restored?._meta, { - card: { - workspaceId: "ws_1", - summary: { files: 1, additions: 1, removals: 0 }, - }, + assert.deepEqual(restored?.structuredContent, { + workspaceId: "ws_1", + reviewRef: "d".repeat(40), + result: "Changed 1 file.", }); + assert.equal(restored?._meta, undefined); }); diff --git a/src/ui/tool-result.ts b/src/ui/tool-result.ts index efd1bdb4..3706b226 100644 --- a/src/ui/tool-result.ts +++ b/src/ui/tool-result.ts @@ -13,33 +13,19 @@ export interface ChatGptToolGlobals { export function decodeToolResult(result: CallToolResult): DecodedToolResult { const structured = asRecord(result.structuredContent); - const metaCard = cardFields(asRecord(asRecord(result._meta)?.card)); if (structured) { const workspaceId = stringField(structured.workspaceId); const reviewRef = stringField(structured.reviewRef); if (workspaceId && reviewRef) { - if (isCompleteReviewCard(metaCard)) { - return { - kind: "card", - card: { - ...metaCard, - tool: "show_changes", - workspaceId, - }, - }; - } + const reviewCard = reviewCardFromStructured(structured, workspaceId); + if (reviewCard) return { kind: "card", card: reviewCard }; return { kind: "review-reference", workspaceId, reviewRef }; } if (typeof structured.patch === "string" && Array.isArray(structured.files)) { - const legacyCard = cardFields({ - ...structured, - payload: { patch: structured.patch }, - }); - if (legacyCard) { - return { kind: "card", card: { ...legacyCard, tool: "show_changes" } }; - } + const legacyCard = reviewCardFromStructured(structured); + if (legacyCard) return { kind: "card", card: legacyCard }; } const root = stringField(structured.root); @@ -50,29 +36,36 @@ export function decodeToolResult(result: CallToolResult): DecodedToolResult { kind: "card", card: { ...structuredCard, - ...metaCard, tool: "open_workspace", workspaceId, root, mode, - summary: metaCard?.summary ?? workspaceSummary(structuredCard), + summary: workspaceSummary(structuredCard), }, }; } } - // Existing conversations created before reviewRef was added can still render - // while the host supplies their live MCP Apps result metadata. - if (metaCard?.workspaceId && (metaCard.files?.length || metaCard.payload?.patch)) { - return { kind: "card", card: { ...metaCard, tool: "show_changes" } }; - } - if (metaCard?.workspaceId && metaCard.root && metaCard.mode) { - return { kind: "card", card: { ...metaCard, tool: "open_workspace" } }; - } - return { kind: "invalid" }; } +function reviewCardFromStructured( + structured: Record, + workspaceId?: string, +): ToolResultCard | undefined { + const card = cardFields({ + ...structured, + payload: { patch: structured.patch }, + }); + if (!isCompleteReviewCard(card)) return undefined; + + return { + ...card, + tool: "show_changes", + ...(workspaceId ? { workspaceId } : {}), + }; +} + function isCompleteReviewCard( card: Partial | undefined, ): card is Partial & { @@ -93,28 +86,20 @@ export function toolResultFromChatGptGlobals( ): CallToolResult | undefined { if (!globals) return undefined; - const responseMetadata = asRecord(globals.toolResponseMetadata); const metadataResult = mcpToolResult(globals.toolResponseMetadata); const structuredContent = asRecord(globals.toolOutput) ?? asRecord(metadataResult?.structuredContent); - const resultMeta = asRecord(metadataResult?._meta) - ?? directResultMeta(responseMetadata); - if (!metadataResult && !structuredContent && !resultMeta) return undefined; + if (!metadataResult && !structuredContent) return undefined; + + const { _meta: _ignoredMeta, structuredContent: _ignoredStructured, ...rest } = metadataResult + ?? { content: [] }; return { - ...(metadataResult ?? { content: [] }), + ...rest, ...(structuredContent ? { structuredContent } : {}), - ...(resultMeta ? { _meta: resultMeta } : {}), } as CallToolResult; } -function directResultMeta( - metadata: Record | undefined, -): Record | undefined { - if (!metadata) return undefined; - return "card" in metadata ? metadata : undefined; -} - function mcpToolResult(value: unknown): CallToolResult | undefined { const metadata = asRecord(value); if (!metadata) return undefined;