diff --git a/changelog.d/fixes/13652-kiro-tooldocs-repeat-every-turn.md b/changelog.d/fixes/13652-kiro-tooldocs-repeat-every-turn.md new file mode 100644 index 00000000000..9e2ad49c793 --- /dev/null +++ b/changelog.d/fixes/13652-kiro-tooldocs-repeat-every-turn.md @@ -0,0 +1 @@ +- **fix(sse):** Kiro translator no longer re-prepends the full relocated tool-documentation block onto every subsequent turn of a multi-turn conversation; it now stays anchored to the turn that originally carried it. (#13652) — thanks @KelvinKSPS diff --git a/open-sse/translator/request/openai-to-kiro.ts b/open-sse/translator/request/openai-to-kiro.ts index 03bae61bb13..1c6238d5d2e 100644 --- a/open-sse/translator/request/openai-to-kiro.ts +++ b/open-sse/translator/request/openai-to-kiro.ts @@ -183,6 +183,13 @@ function convertMessages(messages, tools, model) { let currentRole = null; let toolsAttached = false; let toolDocs = ""; + // The actual turn object that ends up carrying `toolDocs` (issue #13652). + // `buildKiroPayload()` only prepends the doc block onto `currentMessage`, so + // once this turn is demoted into `history` (any turn after the first, on a + // resent multi-turn request) we need to know it was NOT promoted, and embed + // the doc text directly onto it instead of letting it get re-glued onto + // whatever the newest turn happens to be. + let toolDocsCarrier = null; // Only Claude models support images in Kiro. Kiro also routes non-Claude // models (deepseek, minimax, glm, qwen3-coder-next) that do not accept image @@ -242,7 +249,10 @@ function convertMessages(messages, tools, model) { } const built = buildKiroToolSpecs(tools); userMsg.userInputMessage.userInputMessageContext.tools = built.specs; - if (built.docs) toolDocs = built.docs; + if (built.docs) { + toolDocs = built.docs; + toolDocsCarrier = userMsg; + } toolsAttached = true; } @@ -530,10 +540,30 @@ function convertMessages(messages, tools, model) { } const built = buildKiroToolSpecs(tools); currentMessage.userInputMessage.userInputMessageContext.tools = built.specs; - if (built.docs) toolDocs = built.docs; + if (built.docs) { + toolDocs = built.docs; + toolDocsCarrier = currentMessage; + } toolsAttached = true; } + // The relocated doc text is only safe to leave in `toolDocs` (which + // `buildKiroPayload()` unconditionally prepends onto `currentMessage`) when + // the turn that originally carried it IS `currentMessage` — true for a + // single-turn conversation and the "no user turn" fallback above. On any + // later turn of a resent multi-turn request, the tool-bearing turn has been + // demoted into `history` instead, so re-prepending `toolDocs` here would + // glue the *already delivered* doc block onto the newest turn every time + // (issue #13652). Embed it directly onto the carrier turn's own content — + // still in `history` at this point — and clear `toolDocs` so + // `buildKiroPayload()` does not also inject it. + if (toolDocs && toolDocsCarrier && toolDocsCarrier !== currentMessage) { + const carrierMessage = toolDocsCarrier.userInputMessage; + const existingContent = carrierMessage.content || ""; + carrierMessage.content = `# Tool Documentation\n\n${toolDocs}\n\n---\n\n${existingContent}`; + toolDocs = ""; + } + // Clean up history for Kiro API compatibility history.forEach((item) => { if (item.userInputMessage?.userInputMessageContext?.tools) { diff --git a/tests/unit/issue-13652-kiro-tooldocs-repeat.test.ts b/tests/unit/issue-13652-kiro-tooldocs-repeat.test.ts new file mode 100644 index 00000000000..f4d165cc8f6 --- /dev/null +++ b/tests/unit/issue-13652-kiro-tooldocs-repeat.test.ts @@ -0,0 +1,83 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { buildKiroPayload } from "../../open-sse/translator/request/openai-to-kiro.ts"; + +// Issue #13652: `convertMessages()` in `openai-to-kiro.ts` is stateless per HTTP +// request. OpenAI-compatible clients resend the full growing message array on +// every turn, so the same tool-bearing first user message is re-scanned on +// every subsequent request and its relocated documentation (`toolDocs`) +// rebuilt from scratch. `buildKiroPayload()` then unconditionally prepends it +// onto whatever the *current* turn is, so the ~10KB+ doc block keeps landing +// on the newest user message instead of staying where it was first delivered. + +const DOCS_HEADING = "# Tool Documentation"; + +function bigTool(length: number) { + return [ + { + type: "function", + function: { + name: "big_tool", + description: "D".repeat(length), + parameters: { type: "object", properties: {} }, + }, + }, + ]; +} + +test("issue #13652: relocated tool doc is not re-prepended to a later turn once already delivered", () => { + const tools = bigTool(12000); + + // Turn 1: client sends only the first user message. + const turn1Messages = [{ role: "user", content: "hello, please help" }]; + const payload1 = buildKiroPayload( + "claude-sonnet-4.5", + { messages: turn1Messages, tools }, + true, + {} + ); + const turn1Content = payload1.conversationState.currentMessage.userInputMessage.content; + + assert.ok( + turn1Content.includes(DOCS_HEADING), + "turn 1: the relocated tool documentation must reach the model at least once" + ); + + // Turn 2: the OpenAI-compatible client resends the FULL prior history plus + // the assistant reply and a new user message. The client's own copy of turn + // 1's user message does NOT contain the "# Tool Documentation" block, since + // that was only ever prepended server-side to the outgoing Kiro payload. + const turn2Messages = [ + { role: "user", content: "hello, please help" }, + { role: "assistant", content: "Sure, how can I help?" }, + { role: "user", content: "what is 2+2?" }, + ]; + const payload2 = buildKiroPayload( + "claude-sonnet-4.5", + { messages: turn2Messages, tools }, + true, + {} + ); + const turn2CurrentContent = payload2.conversationState.currentMessage.userInputMessage.content; + + assert.ok( + !turn2CurrentContent.includes(DOCS_HEADING), + "turn 2: the tool documentation must NOT be re-prepended to the newest turn's " + + "content once it has already been delivered earlier in the conversation" + ); + + // The docs must not simply vanish — they must still reach the model exactly + // once, anchored on the turn that originally carried the tools (now in history). + const historyContents = payload2.conversationState.history + .map((h: { userInputMessage?: { content?: string } }) => h.userInputMessage?.content || "") + .join("\n"); + const occurrences = (historyContents.match(new RegExp(DOCS_HEADING, "g")) || []).length; + + assert.equal( + occurrences, + 1, + "turn 2: the tool documentation must still reach the model exactly once, anchored to " + + "the turn that originally carried the tools" + ); +}); diff --git a/tests/unit/kiro-long-tool-description-docs.test.ts b/tests/unit/kiro-long-tool-description-docs.test.ts index c7f5dcd28a1..330a2be465d 100644 --- a/tests/unit/kiro-long-tool-description-docs.test.ts +++ b/tests/unit/kiro-long-tool-description-docs.test.ts @@ -53,7 +53,15 @@ const TURN_SHAPES = { "no user messages": [{ role: "assistant", content: "only" }], }; -test("relocated tool documentation reaches the current turn for every turn shape", () => { +// Issue #13652: the doc block is anchored to the turn that originally carried +// the tools, not unconditionally glued onto `currentMessage`. For a shape with +// more than one user turn, the tool-bearing turn ends up demoted into +// `history` (currentMessage becomes the newest turn instead), so the doc now +// lives on `history[0]`'s content. Only when the tool-bearing turn IS +// `currentMessage` (a single user turn, or the "no user turn" fallback) does +// it stay there. Whichever turn carries it, it must reach the model exactly +// once — resending it on both would reintroduce #13652's duplication bug. +test("relocated tool documentation reaches exactly one turn for every turn shape", () => { for (const [label, messages] of Object.entries(TURN_SHAPES)) { const payload = buildKiroPayload( "claude-sonnet-4.5", @@ -62,13 +70,21 @@ test("relocated tool documentation reaches the current turn for every turn shape {} ); const current = payload.conversationState.currentMessage.userInputMessage; + const history = payload.conversationState.history as Array<{ + userInputMessage?: { content?: string }; + }>; + const allContents = [...history.map((h) => h.userInputMessage?.content || ""), current.content]; + const combined = allContents.join("\n"); + const occurrences = (combined.match(new RegExp(DOCS_HEADING, "g")) || []).length; - assert.ok( - current.content.includes(DOCS_HEADING), - `${label}: full tool documentation must be prepended to the current turn` + assert.equal( + occurrences, + 1, + `${label}: the tool documentation must reach the model exactly once, not zero ` + + `(silently dropped) and not more than once (re-injected, issue #13652)` ); assert.ok( - current.content.includes("D".repeat(12000)), + combined.includes("D".repeat(12000)), `${label}: the relocated description text itself must survive` ); assert.equal( @@ -187,12 +203,20 @@ test("only oversized descriptions are relocated in a mixed tool inventory", () = ); const current = payload.conversationState.currentMessage.userInputMessage; const specs = current.userInputMessageContext?.tools; + const history = payload.conversationState.history as Array<{ + userInputMessage?: { content?: string }; + }>; + // "multi-turn conversation" demotes the tool-bearing turn into history[0] + // (issue #13652) — the doc block lives there, not on currentMessage. + const combined = [...history.map((h) => h.userInputMessage?.content || ""), current.content].join( + "\n" + ); assert.equal(specs[0].toolSpecification.description, "compact"); assert.equal(specs[1].toolSpecification.description, POINTER); - assert.ok(current.content.includes("## Tool: big_tool")); + assert.ok(combined.includes("## Tool: big_tool")); assert.ok( - !current.content.includes("## Tool: small_tool"), + !combined.includes("## Tool: small_tool"), "a tool that was never relocated must not get a documentation section" ); });