diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index d9c536333c8..c49a246480c 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1097,6 +1097,7 @@ "kiro-review-regressions.test.ts": "providers/kiro", "kiro-metering-events.test.ts": "providers/kiro", "kiro-metering-usage.test.ts": "providers/kiro", + "kiro-completion-with-real-tools.test.ts": "providers/kiro", "kiro-single-final.test.ts": "providers/kiro", "kiro-stream.test.ts": "providers/kiro", "kiro-transport-parity.test.ts": "providers/kiro", diff --git a/src/adapters/kiro/stream.ts b/src/adapters/kiro/stream.ts index dbd39e83764..731e77a9f32 100644 --- a/src/adapters/kiro/stream.ts +++ b/src/adapters/kiro/stream.ts @@ -454,25 +454,28 @@ async function* parseKiroAttemptEvents( }; }; + // A real tool call and the private final-answer call can share one turn, in either order. The + // turn then continues through the real tools, so the final answer is dropped: it is held until + // the stream ends and is never relayed, which makes the drop invisible to the client. A second + // completion call, a completion call in `disabled` mode and a malformed completion stay errors. + const noteDroppedCompletion = (): void => { + console.warn("[kiro] dropped the private final-answer call; the turn also carried a real tool call"); + }; + const classifyTool = ( tool: { id: string; name: string; chunks: string[]; completion: boolean }, ): AdapterEvent | undefined => { if (tool.name !== KIRO_COMPLETION_TOOL_NAME) { tool.completion = false; - return completionAnswer !== undefined || completionCalls > 0 - ? protocolTerminal("Kiro returned a real tool call alongside a private final answer") - : undefined; + return undefined; } if (mode === "disabled") { return protocolTerminal("Kiro returned the reserved private final-answer tool while explicit completion was disabled"); } tool.completion = true; - if (completionAnswer !== undefined || completionCalls > 0) { + if (completionCalls > 0) { return protocolTerminal("Kiro returned more than one private final-answer tool call", true); } - if (sawRealTool) { - return protocolTerminal("Kiro returned a private final answer alongside a real tool call"); - } return undefined; }; @@ -575,18 +578,20 @@ async function* parseKiroAttemptEvents( if (completionCalls > 1) { return { events: [], terminal: protocolTerminal("Kiro returned more than one private final-answer tool call", true) }; } - if (sawRealTool) { - return { events: [], terminal: protocolTerminal("Kiro returned a private final answer alongside a real tool call") }; - } const answer = parseCompletion(tool.chunks); if (answer instanceof Error) return { events: [], terminal: protocolTerminal(answer.message, true) }; + if (sawRealTool) { + noteDroppedCompletion(); + return { events: [] }; + } completionAnswer = answer; return { events: [] }; } - if (completionAnswer !== undefined || completionCalls > 0) { - return { events: [], terminal: protocolTerminal("Kiro returned a real tool call alongside a private final answer") }; - } sawRealTool = true; + if (completionAnswer !== undefined) { + completionAnswer = undefined; + noteDroppedCompletion(); + } const restored = nameMap?.get(tool.name) ?? tool.name; return { events: [ diff --git a/structure/providers/kiro.md b/structure/providers/kiro.md index e0e26d4ff12..38288c52dd6 100644 --- a/structure/providers/kiro.md +++ b/structure/providers/kiro.md @@ -140,7 +140,16 @@ budget until emitted, discarded, or cancelled; replay collectors are released af retry construction. Native `END_TURN` and `STOP_SEQUENCE` alone do not distinguish progress from an answer and therefore still require validation. Normal private completion and real tool calls need no completion retry. -Coverage: `tests/providers/kiro/kiro-single-final.test.ts` and + +A turn can carry a real tool call and the private completion call together, in either order. +The turn then continues through the real tools: the parser drops the completion call, ends the +turn with `endTurn: false`, and logs one `[kiro] dropped the private final-answer call` line. The +answer is held until the stream ends and is never relayed, so the drop is invisible to the client; +only the model's ordinary prose and reasoning, released as commentary with the tool call, precede +the tool. A second completion call, a completion call while `disabled`, and a malformed completion +stay protocol errors. +Coverage: `tests/providers/kiro/kiro-single-final.test.ts`, +`tests/providers/kiro/kiro-completion-with-real-tools.test.ts` and `tests/server/server-kiro-completion-e2e.test.ts`. `src/adapters/kiro-retry.ts` uses the configured executor for every generation send and may try the existing `q.{region}.amazonaws.com` host once after a canonical-host HTTP 502/503/504 before output, subject to the same send budget. Reset, 429, alternate, and completion-fallback sends wait for a pacing slot; only the first send is pre-paid. Kiro web-search turns are paced as well. A Kiro-local wrapper maps its header deadline to HTTP 504 without changing shared or Google fetch behavior; caller cancellation remains an abort. Final HTTP 5xx text is fixed for clients, and opt-in provider diagnostics carry only closed-set status and classification codes. diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 28f9d9dd49f..05ca616c4ee 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -795,7 +795,8 @@ "kiro-refusal-failover.test.ts": "providers/kiro", "kiro-refusal.test.ts": "providers/kiro", "kiro-remote-image.test.ts": "providers/kiro", "kiro-retry.test.ts": "providers/kiro", "kiro-review-regressions.test.ts": "providers/kiro", "kiro-metering-events.test.ts": "providers/kiro", - "kiro-metering-usage.test.ts": "providers/kiro", "kiro-single-final.test.ts": "providers/kiro", + "kiro-metering-usage.test.ts": "providers/kiro", "kiro-completion-with-real-tools.test.ts": "providers/kiro", + "kiro-single-final.test.ts": "providers/kiro", "kiro-stream.test.ts": "providers/kiro", "kiro-transport-parity.test.ts": "providers/kiro", "kiro-usage-quota.test.ts": "providers/kiro", "kiro-usage-restart.test.ts": "providers/kiro", "kiro-windows-cli-db-path.test.ts": "providers/kiro", "kiro-windows-cli-executable-path.test.ts": "providers/kiro", diff --git a/tests/providers/kiro/kiro-completion-with-real-tools.test.ts b/tests/providers/kiro/kiro-completion-with-real-tools.test.ts new file mode 100644 index 00000000000..7459cc9464f --- /dev/null +++ b/tests/providers/kiro/kiro-completion-with-real-tools.test.ts @@ -0,0 +1,205 @@ +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import { createKiroAdapter } from "../../../src/adapters/kiro"; +import { KIRO_COMPLETION_TOOL_NAME } from "../../../src/adapters/kiro-constants"; +import { resetKiroThrottleStateForTests } from "../../../src/adapters/kiro-retry"; +import { buildResponseJSON } from "../../../src/bridge"; +import { encodeMessage } from "../../../src/lib/eventstream-decoder"; +import { createTranslatorBudget, releaseTranslatedEvent } from "../../../src/lib/translator-budget"; +import type { AdapterEvent, OcxParsedRequest, OcxProviderConfig } from "../../../src/types"; + +/** + * The model can send a real tool call and the private final-answer call in one turn, because the + * prompt tells it to put independent calls in one block. The turn then continues through the real + * tools and the final answer is dropped. It was a non-retryable 502 once the tool call had been + * relayed, which no combo can recover from. + */ +const provider: OcxProviderConfig = { + adapter: "kiro", baseUrl: "https://runtime.us-east-1.kiro.dev", apiKey: "ksk_test", +}; +const parsed: OcxParsedRequest = { + modelId: "claude-sonnet-4.5", stream: true, options: {}, + context: { + messages: [{ role: "user", content: "Inspect the workspace." }], + tools: [{ name: "bash", description: "Run a command", parameters: { type: "object" } }], + }, +}; +const ANSWER = "The workspace is ready and nothing needs changing."; +const frame = (type: string, payload: object) => encodeMessage( + { ":message-type": "event", ":event-type": type }, + new TextEncoder().encode(JSON.stringify(payload)), +); +const text = (content: string) => frame("assistantResponseEvent", { content }); +function tool(name: string, input: object, id: string): Uint8Array[] { + return [ + frame("toolUseEvent", { name, toolUseId: id }), + frame("toolUseEvent", { name, toolUseId: id, input: JSON.stringify(input) }), + frame("toolUseEvent", { name, toolUseId: id, stop: true }), + ]; +} +const bash = (id = "call-1") => tool("bash", { command: "pwd" }, id); +const completion = (answer = ANSWER, id = "complete-1") => tool(KIRO_COMPLETION_TOOL_NAME, { answer }, id); +function response(frames: Uint8Array[]): Response { + return new Response(new ReadableStream({ + start(controller) { + for (const value of frames) controller.enqueue(value); + controller.close(); + }, + })); +} + +let warnings: string[] = []; +let warn: ReturnType; +beforeEach(() => { + warnings = []; + warn = spyOn(console, "warn").mockImplementation((...args: unknown[]) => { warnings.push(args.join(" ")); }); +}); +afterEach(() => { + warn.mockRestore(); + resetKiroThrottleStateForTests(); +}); + +/** One physical send per entry of `legs`; a second entry answers the bounded completion retry. */ +async function run(legs: Uint8Array[][], buffered = false) { + const adapter = createKiroAdapter(provider); + const budget = createTranslatorBudget(); + const events: AdapterEvent[] = []; + let physicalRequests = 0; + try { + const request = await adapter.buildRequest(structuredClone(parsed)); + const upstream = await adapter.fetchResponse!(request, { + executor: (async () => response(legs[Math.min(physicalRequests++, legs.length - 1)]!)) as typeof fetch, + }); + if (buffered) events.push(...await adapter.parseResponse!(upstream, budget)); + else for await (const event of adapter.parseStream(upstream, budget)) events.push(event); + if (buffered) for (const event of events) releaseTranslatedEvent(event, budget); + expect(budget.snapshot().currentBytes).toBe(0); + return { events, physicalRequests }; + } finally { + budget.dispose(); + } +} + +const droppedLines = () => warnings.filter(line => line.startsWith("[kiro] dropped the private final-answer call")); + +/** What the client sees: the real tool call, the tool_use end of turn, and no trace of the answer. */ +function expectToolTurn(events: AdapterEvent[]): void { + expect(events.some(event => event.type === "error")).toBe(false); + expect(events.filter(event => event.type === "tool_call_start")).toEqual([ + { type: "tool_call_start", id: "call-1", name: "bash" }, + ]); + expect(events.at(-1)).toMatchObject({ type: "done", endTurn: false }); + expect(events.filter(event => event.type === "text_delta" && event.phase === "final_answer")).toEqual([]); + const wire = JSON.stringify(events); + expect(wire).not.toContain(ANSWER); + expect(wire).not.toContain(KIRO_COMPLETION_TOOL_NAME); + const json = buildResponseJSON(events, "claude-sonnet-4.5") as { output: Array<{ type: string }> }; + expect(json.output.filter(item => item.type === "function_call")).toHaveLength(1); +} + +describe("a real tool call and the private final answer in one turn", () => { + for (const buffered of [false, true]) { + describe(buffered ? "buffered parse" : "streamed parse", () => { + test("real tool first, then the completion: the tool call goes out, the answer is dropped", async () => { + const { events, physicalRequests } = await run([[ + text("Checking the workspace."), ...bash(), ...completion(), + frame("metadataEvent", { stopReason: "END_TURN" }), + ]], buffered); + expectToolTurn(events); + // The prose before the tool is ordinary commentary, released with the tool call. + expect(events.filter(event => event.type === "text_delta")).toEqual([ + { type: "text_delta", text: "Checking the workspace.", phase: "commentary" }, + ]); + expect(physicalRequests).toBe(1); + expect(droppedLines()).toHaveLength(1); + }); + + test("completion first, then the real tool: nothing of the answer was relayed before the tool", async () => { + const { events, physicalRequests } = await run([[ + text("Checking the workspace."), ...completion(), ...bash(), + frame("metadataEvent", { stopReason: "END_TURN" }), + ]], buffered); + expectToolTurn(events); + expect(events.filter(event => event.type === "text_delta")).toEqual([ + { type: "text_delta", text: "Checking the workspace.", phase: "commentary" }, + ]); + // No event of the completion call, and no answer text, comes before the tool call. + const toolAt = events.findIndex(event => event.type === "tool_call_start"); + expect(JSON.stringify(events.slice(0, toolAt))).not.toContain(ANSWER); + expect(physicalRequests).toBe(1); + expect(droppedLines()).toHaveLength(1); + }); + + test("several real tools around one completion all go out; one line is logged", async () => { + const { events } = await run([[ + ...bash("call-1"), ...completion(), ...bash("call-2"), + ]], buffered); + expect(events.some(event => event.type === "error")).toBe(false); + expect(events.filter(event => event.type === "tool_call_start").map(event => event.type === "tool_call_start" && event.id)) + .toEqual(["call-1", "call-2"]); + expect(events.at(-1)).toMatchObject({ type: "done", endTurn: false }); + expect(JSON.stringify(events)).not.toContain(ANSWER); + expect(droppedLines()).toHaveLength(1); + }); + + test("the mix on the bounded completion retry drops the answer the same way", async () => { + const { events, physicalRequests } = await run([ + [text("Checking the workspace.")], + [...completion(), ...bash()], + ], buffered); + expect(physicalRequests).toBe(2); + expectToolTurn(events); + expect(droppedLines()).toHaveLength(1); + }); + + test("a plain completion with no real tool is still the final answer and logs nothing", async () => { + const { events } = await run([[...completion()]], buffered); + expect(events.filter(event => event.type === "text_delta")).toEqual([ + { type: "text_delta", text: ANSWER, phase: "final_answer" }, + ]); + expect(events.at(-1)).toMatchObject({ type: "done", endTurn: true }); + expect(droppedLines()).toEqual([]); + }); + + test("two completion calls are still an error", async () => { + const { events } = await run([[...completion("FIRST-ANSWER", "complete-1"), ...completion("SECOND-ANSWER", "complete-2")]], buffered); + expect(events.at(-1)).toMatchObject({ type: "error", code: "invalid_kiro_completion", retryable: false }); + expect(events.some(event => event.type === "done")).toBe(false); + expect(JSON.stringify(events)).not.toContain("-ANSWER"); + }); + + test("a second completion call is still an error when a real tool sits between or before them", async () => { + for (const frames of [ + [...bash(), ...completion("one", "complete-1"), ...completion("two", "complete-2")], + [...completion("one", "complete-1"), ...bash(), ...completion("two", "complete-2")], + ]) { + const { events } = await run([frames], buffered); + expect(events.at(-1)).toMatchObject({ type: "error", code: "invalid_kiro_completion" }); + expect(events.some(event => event.type === "done")).toBe(false); + } + }); + + test("a malformed completion is still an error next to a real tool", async () => { + for (const frames of [ + [...bash(), ...tool(KIRO_COMPLETION_TOOL_NAME, { answer: "" }, "complete-1")], + [...tool(KIRO_COMPLETION_TOOL_NAME, { answer: "" }, "complete-1"), ...bash()], + [...bash(), frame("toolUseEvent", { name: KIRO_COMPLETION_TOOL_NAME, toolUseId: "complete-1", input: "{not json" }), + frame("toolUseEvent", { name: KIRO_COMPLETION_TOOL_NAME, toolUseId: "complete-1", stop: true })], + ]) { + const { events } = await run([frames], buffered); + expect(events.at(-1)).toMatchObject({ type: "error" }); + expect(events.some(event => event.type === "done")).toBe(false); + } + }); + }); + } + + test("the completion call stays reserved when explicit completion is disabled", async () => { + const adapter = createKiroAdapter(provider); + const events: AdapterEvent[] = []; + for await (const event of adapter.parseStream(response([...bash(), ...completion()]), createTranslatorBudget())) { + events.push(event); + } + expect(events.at(-1)).toMatchObject({ type: "error", code: "kiro_stream_protocol_error", retryable: false }); + expect(droppedLines()).toEqual([]); + }); +}); diff --git a/tests/providers/kiro/kiro-stream.test.ts b/tests/providers/kiro/kiro-stream.test.ts index c09fac50430..cf776fd17b7 100644 --- a/tests/providers/kiro/kiro-stream.test.ts +++ b/tests/providers/kiro/kiro-stream.test.ts @@ -1003,21 +1003,10 @@ describe("kiro adapter — parseStream", () => { expect(JSON.stringify(events)).not.toContain(KIRO_COMPLETION_TOOL_NAME); }); - test("duplicate completion and completion mixed with real tools fail closed", async () => { + // A completion mixed with real tools is not an error; see kiro-completion-with-real-tools.test.ts. + test("duplicate completion fails closed", async () => { const cases: Uint8Array[][] = [ [...completionFrames("one", "complete-1"), ...completionFrames("two", "complete-2")], - [ - eventFrame({ name: "bash", toolUseId: "call-1" }), - eventFrame({ input: "{}", name: "bash", toolUseId: "call-1" }), - eventFrame({ name: "bash", stop: true, toolUseId: "call-1" }), - ...completionFrames("done", "complete-3"), - ], - [ - ...completionFrames("done", "complete-4"), - eventFrame({ name: "bash", toolUseId: "call-2" }), - eventFrame({ input: "{}", name: "bash", toolUseId: "call-2" }), - eventFrame({ name: "bash", stop: true, toolUseId: "call-2" }), - ], ]; for (const frames of cases) { const adapter = createKiroAdapter(provider);