diff --git a/apps/web/components/task/chat/messages/kandev-tool-message.test.tsx b/apps/web/components/task/chat/messages/kandev-tool-message.test.tsx index eca8f73bed..4e611865e5 100644 --- a/apps/web/components/task/chat/messages/kandev-tool-message.test.tsx +++ b/apps/web/components/task/chat/messages/kandev-tool-message.test.tsx @@ -39,7 +39,11 @@ function kandevToolCall(opts: { input: opts.input, output: opts.resultJson !== undefined - ? [{ type: "text", text: JSON.stringify(opts.resultJson) }] + ? { + _meta: null, + content: [{ type: "text", text: JSON.stringify(opts.resultJson) }], + structuredContent: null, + } : undefined, }, }, diff --git a/apps/web/components/task/chat/messages/kandev/parse.test.ts b/apps/web/components/task/chat/messages/kandev/parse.test.ts index 60eff8cf87..c042c5e06c 100644 --- a/apps/web/components/task/chat/messages/kandev/parse.test.ts +++ b/apps/web/components/task/chat/messages/kandev/parse.test.ts @@ -97,6 +97,25 @@ describe("extractMcpResult", () => { ).toEqual({ version: 1, resolved_charts: [] }); }); + it("falls back to text content when structuredContent is null", () => { + expect( + extractMcpResult({ + _meta: null, + content: [{ type: "text", text: '{"total":2,"workspaces":[{"id":"w1"}]}' }], + structuredContent: null, + }), + ).toEqual({ total: 2, workspaces: [{ id: "w1" }] }); + }); + + it("falls back to text content when structured_content is null", () => { + expect( + extractMcpResult({ + content: [{ type: "text", text: '{"total":1}' }], + structured_content: null, + }), + ).toEqual({ total: 1 }); + }); + it("unwraps the raw result wrapper emitted by ACP clients", () => { expect(extractMcpResult({ result: '{"version":1,"resolved_charts":[]}' })).toEqual({ version: 1, diff --git a/apps/web/components/task/chat/messages/kandev/parse.ts b/apps/web/components/task/chat/messages/kandev/parse.ts index a578ce2497..fba0578fab 100644 --- a/apps/web/components/task/chat/messages/kandev/parse.ts +++ b/apps/web/components/task/chat/messages/kandev/parse.ts @@ -69,6 +69,10 @@ function unwrapContentBlocks(blocks: unknown[]): unknown { return tryParseJson(joined); } +function hasValue(value: unknown): boolean { + return value !== undefined && value !== null; +} + // extractMcpResult walks the various shapes the ACP/MCP transport can emit and // returns the structured payload. Returns null if the value is empty/missing. export function extractMcpResult(value: unknown): unknown { @@ -89,8 +93,12 @@ export function extractMcpResult(value: unknown): unknown { structuredContent?: unknown; structured_content?: unknown; }; - if (obj.structuredContent !== undefined) return obj.structuredContent; - if (obj.structured_content !== undefined) return obj.structured_content; + if (hasValue(obj.structuredContent)) { + return obj.structuredContent; + } + if (hasValue(obj.structured_content)) { + return obj.structured_content; + } if (Array.isArray(obj.content)) return unwrapContentBlocks(obj.content); if (typeof obj.output === "string") return tryParseJson(obj.output); if ( diff --git a/docs/plans/kandev-mcp-tool-result-fallback/plan.md b/docs/plans/kandev-mcp-tool-result-fallback/plan.md new file mode 100644 index 0000000000..3a41ef81e3 --- /dev/null +++ b/docs/plans/kandev-mcp-tool-result-fallback/plan.md @@ -0,0 +1,72 @@ +--- +created: 2026-08-26 +status: complete +requirements: + - REQ-UI-KANDEV-MCP-TOOL-RESULTS-001 +system_design: + - ../../specs/ui/system-design/kandev-mcp-tool-results.md +legacy_specs: [] +--- + +# Implementation Plan: Kandev MCP Tool Result Fallback + +## Overview + +This repair preserves JSON text results when an MCP response contains null +structured content. One work order adds the regression first, corrects the +shared parser, and keeps the existing result precedence. + +## Scope + +### In scope + +- Reproduce the persisted Codex ACP result envelope in focused tests. +- Treat null structured content as absent. +- Preserve non-null structured content and historic wrapper support. +- Keep desktop and mobile result values equal through the shared parser. + +### Out of scope + +- Backend MCP or ACP changes. +- New renderer types or presentation changes. +- New mobile composition or Playwright scenarios. + +## Technical approach + +Add an exact null-envelope case to `extractMcpResult` coverage. Update the +renderer fixture so it uses the persisted `CallToolResult` shape. + +Change `extractMcpResult` to accept only non-null structured values. If a +structured value is null, continue to the existing `content` parser. + +## Tests + +- `AC-UI-KANDEV-MCP-TOOL-RESULTS-001.1`: Keep the existing structured-content + precedence test in `kandev/parse.test.ts`. +- `AC-UI-KANDEV-MCP-TOOL-RESULTS-001.2`: Add a test with + `structuredContent: null` and JSON text content. +- `AC-UI-KANDEV-MCP-TOOL-RESULTS-001.3`: Use the shared renderer fixture to + show the returned count from the exact persisted envelope. + +## E2E tests + +The existing `mcp-status.spec.ts` and `mobile-mcp-status.spec.ts` cover the +shared transcript entry point on desktop and mobile. No new Playwright test is +required because this repair changes only data normalization. + +The mock agent flattens MCP results before ACP delivery. Therefore, a focused +parser fixture is the faithful test for the Codex null envelope. + +## Work orders + +- [x] [Task 01: Preserve text fallback](task-01-preserve-text-fallback.md) (done) + +## Verification results + +Task 01 completed. The focused parser and renderer suite passed 51 tests, and +frontend typecheck passed. + +## Risks + +- A broad falsy-value check can change valid structured-result precedence. +- An incomplete fixture can miss the real Codex `CallToolResult` shape again. diff --git a/docs/plans/kandev-mcp-tool-result-fallback/task-01-preserve-text-fallback.md b/docs/plans/kandev-mcp-tool-result-fallback/task-01-preserve-text-fallback.md new file mode 100644 index 0000000000..465bb8fd1f --- /dev/null +++ b/docs/plans/kandev-mcp-tool-result-fallback/task-01-preserve-text-fallback.md @@ -0,0 +1,86 @@ +--- +id: "01-preserve-text-fallback" +title: "Preserve text fallback" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-UI-KANDEV-MCP-TOOL-RESULTS-001 +acceptance_criteria: + - AC-UI-KANDEV-MCP-TOOL-RESULTS-001.1 + - AC-UI-KANDEV-MCP-TOOL-RESULTS-001.2 + - AC-UI-KANDEV-MCP-TOOL-RESULTS-001.3 +system_design: + - ../../specs/ui/system-design/kandev-mcp-tool-results.md +--- + +# Task 01: Preserve Text Fallback + +## Summary + +Add a regression for the persisted Codex MCP envelope. Then correct the shared +parser so a null structured value does not hide JSON text content. + +## In scope + +- Add the failing null-envelope parser test before the production change. +- Update the renderer fixture to match the persisted `CallToolResult` shape. +- Make the minimum null-handling correction in `extractMcpResult`. +- Preserve existing structured-content and historic-wrapper behavior. + +## Out of scope + +- Backend MCP, ACP adapter, or database changes. +- Renderer layout or copy changes. +- New Playwright tests. + +## Acceptance + +- The regression fails because the parser returns null before the correction. +- The corrected parser returns the JSON text payload when `structuredContent` + is null. +- All focused parser and native Kandev renderer tests pass after the change. + +## Verification + +```bash +(cd apps && pnpm --filter @kandev/web test -- --run components/task/chat/messages/kandev/parse.test.ts components/task/chat/messages/kandev-tool-message.test.tsx) +(cd apps/web && pnpm run typecheck) +``` + +## Files likely touched + +- `apps/web/components/task/chat/messages/kandev/parse.ts` +- `apps/web/components/task/chat/messages/kandev/parse.test.ts` +- `apps/web/components/task/chat/messages/kandev-tool-message.test.tsx` +- `docs/plans/kandev-mcp-tool-result-fallback/plan.md` +- `docs/plans/kandev-mcp-tool-result-fallback/task-01-preserve-text-fallback.md` + +## Dependencies + +None. + +## Risks + +- Preserve empty but valid structured objects. Treat only null and undefined + as absent. + +## Parallelism + +`sequential` + +## Inputs + +- `REQ-UI-KANDEV-MCP-TOOL-RESULTS-001` and its system design. +- The persisted task-session message and ACP envelope from the diagnosis. +- Existing parser and renderer tests. + +## Results + +RED: The real Codex envelope test failed because `extractMcpResult` returned +null when `structuredContent` was null. + +GREEN: The parser now falls through to JSON text content only for null or +undefined structured values. The focused suite passed 51 tests, and frontend +typecheck passed. diff --git a/docs/specs/ui/README.md b/docs/specs/ui/README.md index a4bf7dd5de..1a8a669661 100644 --- a/docs/specs/ui/README.md +++ b/docs/specs/ui/README.md @@ -72,6 +72,7 @@ contract that other capabilities can reuse. - [Reload Kandev when a tab is restored from a frozen browser snapshot](requirements/fix-duplicated-tab-stale-data.md) - [GitHub PR Review Actions](requirements/github-pr-review-actions.md) - [GitHub Saved-Query Default Views](requirements/github-saved-query-defaults.md) +- [Kandev MCP Tool Results](requirements/kandev-mcp-tool-results.md) - [Auto-hide empty workflow steps](requirements/kanban-auto-hide-empty-columns.md) - [Repair last-prompt transcript pinning](requirements/last-prompt-pinning-regressions.md) - [Repair merge commit details](requirements/merge-commit-details.md) @@ -161,6 +162,7 @@ contract that other capabilities can reuse. - [Task PR Automation Controls System Design Part 3](system-design/ci-pr-automation-03.md) - [Merge Queue Recovery Controls](system-design/ci-pr-merge-queue-recovery-controls.md) - [Entity Reference Composer](system-design/entity-reference-composer.md) +- [Kandev MCP Tool Results](system-design/kandev-mcp-tool-results.md) - [Mobile Task Chrome](system-design/mobile-task-chrome.md) - [Persistent status motion](system-design/persistent-status-motion.md) - [Sidebar Task Focus](system-design/sidebar-task-focus.md) diff --git a/docs/specs/ui/requirements/kandev-mcp-tool-results.md b/docs/specs/ui/requirements/kandev-mcp-tool-results.md new file mode 100644 index 0000000000..46f1a6f15c --- /dev/null +++ b/docs/specs/ui/requirements/kandev-mcp-tool-results.md @@ -0,0 +1,45 @@ +--- +status: active +system: ui +created: 2026-08-26 +owners: + - kandev +--- + +# Kandev MCP Tool Results Requirements + +## Overview + +Kandev shows native transcript rows for its built-in MCP tools. A completed +result can contain structured content, text content, or both forms. + +The UI system owns this reusable presentation contract. The agent and platform +systems continue to own tool execution, ACP normalization, and persistence. + +## Terminology + +- **Structured result:** A non-null object in `structuredContent` or + `structured_content`. +- **Text fallback:** JSON text in the standard MCP `content` array. + +## Requirements + +### REQ-UI-KANDEV-MCP-TOOL-RESULTS-001: Reliable result display + +**Intent:** Users need each completed Kandev MCP row to show the result that +the tool returned. + +#### Acceptance criteria + +- **AC-UI-KANDEV-MCP-TOOL-RESULTS-001.1:** When a completed result contains + non-null structured content, the transcript shall use that content. +- **AC-UI-KANDEV-MCP-TOOL-RESULTS-001.2:** When structured content is null or + absent, the transcript shall parse an available JSON text fallback. +- **AC-UI-KANDEV-MCP-TOOL-RESULTS-001.3:** Desktop and mobile transcripts shall + show the same result values from the shared parser. + +## Out of scope + +- Changes to MCP handlers, ACP normalization, or message persistence. +- Portable rendering for non-Kandev MCP content. +- Changes to transcript layout, navigation, scrolling, or touch behavior. diff --git a/docs/specs/ui/system-design/kandev-mcp-tool-results.md b/docs/specs/ui/system-design/kandev-mcp-tool-results.md new file mode 100644 index 0000000000..a37b595fd6 --- /dev/null +++ b/docs/specs/ui/system-design/kandev-mcp-tool-results.md @@ -0,0 +1,82 @@ +--- +status: current +system: ui +requirements: + - REQ-UI-KANDEV-MCP-TOOL-RESULTS-001 +--- + +# Kandev MCP Tool Results System Design + +## Purpose and boundaries + +This design defines how the web transcript selects a Kandev MCP result for +native rendering. It does not change tool execution, transport, or storage. + +The ACP adapter removes provider wrappers and keeps the standard MCP +`CallToolResult`. The UI then selects one usable payload from that result. + +## Requirement mapping + +| Requirement | Design section | +| --- | --- | +| `REQ-UI-KANDEV-MCP-TOOL-RESULTS-001` | [Result selection](#result-selection), [Responsive behavior](#responsive-behavior) | + +## Components and responsibilities + +- `normalizeCodexMCPToolResult` removes the Codex `{error, result}` wrapper. + It keeps the complete standard MCP result. +- Persisted tool-message metadata stores that result at + `metadata.normalized.generic.output`. +- `KandevToolMessage` sends the stored output to `extractMcpResult`. +- `extractMcpResult` selects and parses the payload for all native Kandev tool + renderers. +- Each renderer reads its fields and shows the native transcript row. + +## Result selection + +`extractMcpResult` uses this precedence: + +1. Use `structuredContent` when its value is not null. +2. Use `structured_content` when its value is not null. +3. Parse text blocks from `content`. +4. Parse a string in `output`. +5. Unwrap a recognized `result` wrapper. +6. Keep a plain object unchanged. + +An explicit null structured value means that no structured result exists. It +must not suppress a valid text fallback. + +## Control flow + +1. The agent completes a Kandev MCP tool call. +2. The ACP adapter stores the standard result in normalized message metadata. +3. The transcript passes the result to the shared parser. +4. The parser applies the result-selection precedence. +5. The registered renderer shows the parsed values. + +Live updates and replay use the same stored message shape and parser. + +## Failure and recovery + +Malformed JSON text remains plain text. A result with no usable payload keeps +the existing renderer fallback and does not break adjacent transcript rows. + +The repair must preserve non-null structured results and historic wrapper +support. These paths protect rich-output results and older persisted messages. + +## Persistence + +The repair adds no table or migration. Existing message metadata remains the +only persisted source. + +## Responsive behavior + +Desktop and mobile chat use the same parser and renderer registry. This repair +changes data selection only. It does not change composition or interaction. + +Existing desktop and mobile MCP transcript tests remain the nearest surface +examples. Focused parser and renderer tests reproduce the provider envelope. + +## Related decisions + +- [Keep Agent Rich Output Host Native](../../../decisions/2026-08-14-kandev-native-agent-rich-output.md)