diff --git a/packages/opencode/src/altimate/mcp-server/adapter.ts b/packages/opencode/src/altimate/mcp-server/adapter.ts index dda116d5b9..f43c0a576f 100644 --- a/packages/opencode/src/altimate/mcp-server/adapter.ts +++ b/packages/opencode/src/altimate/mcp-server/adapter.ts @@ -9,7 +9,7 @@ import { Agent } from "@/agent/agent" import { Tool } from "@/tool/tool" import { Truncate } from "@/tool/truncate" import { outputLimits } from "./output-settings" -import { MAX_BASIS_CHARACTERS, renderAnswer } from "../tools/answer-commit" +import { MAX_BASIS_CHARACTERS, normalizeAnswerArgs, renderAnswer } from "../tools/answer-commit" import { AnswerCommitConjunction } from "../tools/answer-commit-conjunction" import { MAX_SCANNED_CHARACTERS } from "../tools/answer-commit-checks" import * as Precedence from "../workspace/precedence" @@ -230,7 +230,10 @@ type TurnMessage = Tool.Context["messages"][number] * explanation never becomes part of the answer text a client copies. The basis is kept, * above that line, as context for the user. */ -export function finalAnswerText(args: Record, committed: string, request?: unknown): string { +export function finalAnswerText(raw: Record, committed: string, request?: unknown): string { + // altimate_change start — #1700: read the same decoded shape the tool validated (JSON-string fields/rows) + const args = normalizeAnswerArgs(raw) as Record + // altimate_change end // Answer-only only for a complete commit: rows, or fields covering every item the request names. // Otherwise keep the committed text (basis on the line) so an item kept only in the basis is not lost. const fields = Array.isArray(args.fields) ? args.fields.length : 0 diff --git a/packages/opencode/src/altimate/mcp-server/server.ts b/packages/opencode/src/altimate/mcp-server/server.ts index 8ed203ac74..fda35a2603 100644 --- a/packages/opencode/src/altimate/mcp-server/server.ts +++ b/packages/opencode/src/altimate/mcp-server/server.ts @@ -9,6 +9,7 @@ import { } from "@modelcontextprotocol/sdk/types.js" import * as Registry from "../native/connections/registry" import { createAdapter, descriptions, resultFailed } from "./adapter" +import { normalizeAnswerArgs } from "../tools/answer-commit" import { catalogSummary } from "../tools/warehouse-discovery" import { readOnlyCaveat } from "../native/connections/read-only" import path from "node:path" @@ -183,7 +184,10 @@ export async function createServer(options: ServeOptions = {}) { result: { status?: string }, signal: AbortSignal, ) => { - const readings = Array.isArray(args?.readings) ? (args.readings as { reading?: unknown }[]) : [] + // altimate_change start — #1700: readings may arrive as a JSON string; read the decoded shape + const decoded = normalizeAnswerArgs(args) as Record | undefined + const readings = Array.isArray(decoded?.readings) ? (decoded.readings as { reading?: unknown }[]) : [] + // altimate_change end const choices = readings.map((item) => item.reading).filter((item): item is string => typeof item === "string" && !!item) if (elicited || result.status === "error" || choices.length < 2 || !server.getClientCapabilities()?.elicitation) return undefined elicited = true diff --git a/packages/opencode/src/altimate/tools/answer-commit.ts b/packages/opencode/src/altimate/tools/answer-commit.ts index 256568df00..982a34ec1c 100644 --- a/packages/opencode/src/altimate/tools/answer-commit.ts +++ b/packages/opencode/src/altimate/tools/answer-commit.ts @@ -183,13 +183,45 @@ export type AnswerCommitInput = z.input // Tolerate boolean aliases at runtime while advertising the canonical boolean schema. // Only answer_only is boolean; answer values and table cells must remain untouched. -const parser = z.preprocess((input) => { - if (!input || typeof input !== "object" || Array.isArray(input) || !("answer_only" in input)) return input - const value = typeof input.answer_only === "string" ? input.answer_only.toLowerCase() : input.answer_only - const answer_only = - value === "true" || value === 1 ? true : value === "false" || value === 0 ? false : input.answer_only - return { ...input, answer_only } -}, parameters) +// altimate_change start — #1700: some models (e.g. Altimate Base) send list/object inputs as +// JSON-encoded strings. A string is never valid for these keys, so decode it when it parses as +// JSON and let the schema validate the result as usual; anything else is left for the normal error. +const JSON_SHAPED_KEYS = ["fields", "asked", "rows", "unresolved", "readings"] as const + +/** Parse a string that looks like a JSON array or object; return any other value, or invalid JSON, unchanged. */ +function decodeJsonShaped(value: unknown): unknown { + if (typeof value !== "string" || !/^\s*[[{]/.test(value)) return value + try { + return JSON.parse(value) + } catch { + return value + } +} +// altimate_change end + +// altimate_change start — #1700: exported so raw-argument readers (MCP adapter/server) see the same decoded shape +/** + * Normalize raw answer_commit arguments before validation: decode JSON-string list/object inputs + * and coerce string or numeric `answer_only` flags. Non-object input is returned unchanged. + */ +export function normalizeAnswerArgs(input: unknown): unknown { + if (!input || typeof input !== "object" || Array.isArray(input)) return input + const next: Record = { ...input } + for (const key of JSON_SHAPED_KEYS) { + if (key in next) next[key] = decodeJsonShaped(next[key]) + } + if (!("answer_only" in next)) return next + const value = typeof next.answer_only === "string" ? next.answer_only.toLowerCase() : next.answer_only + if (value === "true" || value === 1) { + next.answer_only = true + } else if (value === "false" || value === 0) { + next.answer_only = false + } + return next +} + +const parser = z.preprocess(normalizeAnswerArgs, parameters) +// altimate_change end function validationError(error: z.ZodError): string { const describe = (issues: z.core.$ZodIssue[], parent: PropertyKey[] = []): string[] => diff --git a/packages/opencode/test/altimate/mcp-server/final-answer.test.ts b/packages/opencode/test/altimate/mcp-server/final-answer.test.ts index eaaa79f7b5..38a8837534 100644 --- a/packages/opencode/test/altimate/mcp-server/final-answer.test.ts +++ b/packages/opencode/test/altimate/mcp-server/final-answer.test.ts @@ -69,5 +69,23 @@ describe("finalAnswerText", () => { test("falls back to the committed text when the args cannot be re-rendered", () => { expect(finalAnswerText({ basis: "" }, "committed")).toBe("FINAL ANSWER: committed") }) + + // #1700: some models send list inputs as JSON strings; the FINAL ANSWER line must match a real array. + test("fields and asked sent as JSON strings give the same answer-only line", () => { + const fields = [{ name: "orders", value: 42 }] + const committed = `${basis}\n\n**Answer:** 42` + const expected = finalAnswerText({ fields, asked: ["orders"], basis }, committed) + expect( + finalAnswerText({ fields: JSON.stringify(fields), asked: JSON.stringify(["orders"]), basis }, committed), + ).toBe(expected) + expect(expected).toBe(`Basis: ${basis}\n\nFINAL ANSWER: 42`) + }) + + test("rows sent as a JSON string give the same answer-only line", () => { + const rows = { columns: ["city", "n"], values: [["Oslo", 3]] } + expect(finalAnswerText({ rows: JSON.stringify(rows), asked: ["city", "n"], basis }, "unused")).toBe( + finalAnswerText({ rows, asked: ["city", "n"], basis }, "unused"), + ) + }) }) // altimate_change end diff --git a/packages/opencode/test/altimate/mcp-server/server.test.ts b/packages/opencode/test/altimate/mcp-server/server.test.ts index ed4dde9b83..29d7c8722f 100644 --- a/packages/opencode/test/altimate/mcp-server/server.test.ts +++ b/packages/opencode/test/altimate/mcp-server/server.test.ts @@ -855,6 +855,20 @@ mcpTest("answer_commit elicits a reading and asks for a recompute when another i expect(asked).toBe(1) }) +// #1700: readings sent as a JSON string still trigger the reading question. +mcpTest("answer_commit elicits a reading when readings arrive as a JSON string", async () => { + const { client, call } = await fixture(false, { elicitation: {} }) + let asked = 0 + client.setRequestHandler(ElicitRequestSchema, async () => { + asked++ + return { action: "accept", content: { reading: "by id" } } + }) + const result = await call("answer_commit", { ...twoReadings, readings: JSON.stringify(twoReadings.readings) }) + expect(result.isError).toBe(true) + expect(text(result)).toContain("by id") + expect(asked).toBe(1) +}) + mcpTest("answer_commit degrades silently when the client cancels or lacks elicitation", async () => { const cancelled = await fixture(false, { elicitation: {} }) cancelled.client.setRequestHandler(ElicitRequestSchema, async () => ({ action: "cancel" })) diff --git a/packages/opencode/test/altimate/tools/answer-commit.test.ts b/packages/opencode/test/altimate/tools/answer-commit.test.ts index 6bcffbeaa8..dc0a02a675 100644 --- a/packages/opencode/test/altimate/tools/answer-commit.test.ts +++ b/packages/opencode/test/altimate/tools/answer-commit.test.ts @@ -50,6 +50,79 @@ describe("answer_commit boolean tolerance", () => { }) }) +// altimate_change start — #1700: some models (e.g. Altimate Base) send list/object inputs as +// JSON-encoded strings. Every commit was rejected with "fields: Invalid input: expected array, +// received string", the model retried with the same shape, and the structured answer was lost. +describe("answer_commit JSON-string tolerance (#1700)", () => { + const basis = "Counted orders at order grain using stg_orders.order_id." + const fields = [{ name: "Is anything wrong", value: "Yes, a join fan-out inflates number_of_orders" }] + + test("fields sent as a JSON string renders the same answer as a real array", async () => { + const expected = AnswerCommit.renderAnswer({ basis, fields }) + const payload = { basis, fields: JSON.stringify(fields) } + expect(AnswerCommit.renderAnswer(payload)).toBe(expected) + const tool = await initTool(AnswerCommit.AnswerCommitTool) + expect((await tool.execute(payload, context)).output).toBe(expected) + }) + + test("the exact shape from the #1700 trace is accepted", async () => { + // Pretty-printed with a space after the colon, as the model sent it. + const payload = { + basis, + asked: '["is anything wrong"]', + fields: '[{"name": "diagnosis", "value": 1}]', + } + const tool = await initTool(AnswerCommit.AnswerCommitTool) + expect((await tool.execute(payload, context)).output).toBe(`${basis}\n\n**Answer:** diagnosis: 1`) + }) + + test("rows and unresolved sent as JSON strings are accepted", async () => { + const rows = { columns: ["customer_id", "orders"], values: [[51, 3]] } + expect(AnswerCommit.renderAnswer({ basis, rows: JSON.stringify(rows) })).toBe( + AnswerCommit.renderAnswer({ basis, rows }), + ) + const unresolved = { reason: "The warehouse was unreachable." } + expect(AnswerCommit.renderAnswer({ basis, unresolved: JSON.stringify(unresolved) })).toBe( + AnswerCommit.renderAnswer({ basis, unresolved }), + ) + }) + + test("readings and asked sent as JSON strings are decoded into metadata", async () => { + const readings = [{ clause: "order counts", reading: "distinct orders", result_summary: "99", decided_by: "request" }] + const tool = await initTool(AnswerCommit.AnswerCommitTool) + const result = await tool.execute( + { basis, fields, asked: '["Is anything wrong"]', readings: JSON.stringify(readings) }, + context, + ) + expect(result.metadata.readings).toEqual(readings) + expect(result.metadata.asked).toEqual(["Is anything wrong"]) + expect(result.output).toBe(AnswerCommit.renderAnswer({ basis, fields })) + }) + + test("a JSON string that is not valid JSON is still rejected with the normal error", async () => { + const payload = { basis, fields: '[{"name": "diagnosis", "value": 1}' } + expect(() => AnswerCommit.renderAnswer(payload)).toThrow(/fields.*expected array/i) + const tool = await initTool(AnswerCommit.AnswerCommitTool) + await rejects(tool.execute(payload, context), /fields.*expected array/i) + }) + + test("a plain string that does not look like JSON is still rejected", () => { + expect(() => AnswerCommit.renderAnswer({ basis, fields: "Yes, a join fan-out" })).toThrow(/fields.*expected array/i) + }) + + test("decoded values still go through full validation", () => { + expect(() => AnswerCommit.renderAnswer({ basis, fields: '[{"name": "x", "value": ""}]' })).toThrow( + /non-empty/i, + ) + }) + + test("string answer values that look like JSON are left untouched", () => { + const value = '[{"looks": "like json"}]' + expect(AnswerCommit.renderAnswer({ basis, fields: [{ name: "Raw", value }] })).toContain(value) + }) +}) +// altimate_change end + describe("answer_commit required basis", () => { test.each([undefined, false, "false", 0, null, "", [], {}, true, "true", "TRUE", "TrUe", 1].map((value) => [value]))( "answer_only %j requires basis for every answer shape",