Repository navigation
fix(kiro): reapply #6151 content-filter fixes with all code review corrections #6451
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| ### Fixed | ||
|
|
||
| - Kiro streams now surface explicit content-filter refusal messages with category and explanation instead of the generic "no tokens" error when a refusal is detected. Both bearer token (via `messageMetadataEvent`) and API-key (`ksk_`, via `metadataEvent`) paths are supported, with proper provider safety-stop minting. Text is streamed incrementally unless a thinking block precedes it, in which case the entire answer (after thinking) is buffered until the stream end; tool calls are emitted only at stream end (#6150). | ||
| - Detail-free `stopReason: "CONTENT_FILTERED"` (without `stopDetails.refusal` object) is now treated as a refusal and suppresses unconfirmed content. | ||
| - Text events are now emitted immediately when a completion event is received, instead of being deferred to EOF. This prevents text events from being lost if subsequent reads fail. | ||
| - Parser no longer resyncs into nested objects when an incomplete outer JSON frame is encountered. Incomplete frames are retained as remainder; EOF with incomplete frame throws an error. | ||
| - Server error events now emit `text_end` for confirmed text before throwing the error, ensuring complete text lifecycle. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -444,6 +444,7 @@ type KiroStreamEvent = | |
| | { type: "toolUseInput"; data: { input: string } } | ||
| | { type: "toolUseStop"; data: { stop: boolean } } | ||
| | { type: "usage"; data: { inputTokens?: number; outputTokens?: number } } | ||
| | { type: "completion"; data: { stopReason?: string } } | ||
| | { | ||
| type: "refusal"; | ||
| data: { stopReason?: string; stopDetails?: { refusal?: { category?: string; explanation?: string } } }; | ||
|
|
@@ -497,38 +498,10 @@ export function parseKiroApiEvents(buffer: string): { events: KiroStreamEvent[]; | |
| const end = findJsonEnd(buffer, start); | ||
| if (end < 0) { | ||
| // Unclosed brace at 'start'. Could be incomplete JSON waiting for more data, or junk/corruption. | ||
| // Finding #3: Do NOT resync into nested objects that might be part of the outer incomplete frame. | ||
| // Keep the incomplete frame as remainder for the next parse call or EOF error handling. | ||
| const strayBraceEnd = Math.min(start + MAX_RESCAN_DISTANCE, buffer.length); | ||
| return { events, remaining: buffer.slice(start, strayBraceEnd) }; | ||
| } | ||
|
|
||
| let parsed: Record<string, unknown> | undefined; | ||
|
|
@@ -567,20 +540,21 @@ export function parseKiroApiEvents(buffer: string): { events: KiroStreamEvent[]; | |
|
|
||
| // Check for refusal after usage so that usage is processed first and recorded before refusal terminates | ||
| // This check is now order-independent: stopDetails can appear at any position in the JSON object | ||
| // Also handle detail-free CONTENT_FILTERED as a refusal (no stopDetails.refusal needed) | ||
| if ( | ||
| (typeof parsed.stopDetails === "object" && | ||
| parsed.stopDetails !== null && | ||
| (parsed.stopDetails as Record<string, unknown>).refusal) || | ||
| parsed.stopReason === "CONTENT_FILTERED" | ||
| ) { | ||
| // Emit refusal event if stopDetails contains actual refusal data or stopReason is CONTENT_FILTERED | ||
| events.push({ | ||
| type: "refusal", | ||
| data: { | ||
| stopReason: parsed.stopReason as string | undefined, | ||
| stopDetails: parsed.stopDetails as { refusal?: { category?: string; explanation?: string } }, | ||
| }, | ||
| }); | ||
| } else if (parsed.error || parsed.Error) { | ||
| events.push({ | ||
| type: "error", | ||
|
|
@@ -589,6 +563,14 @@ export function parseKiroApiEvents(buffer: string): { events: KiroStreamEvent[]; | |
| message: (parsed.message || parsed.Message) as string | undefined, | ||
| }, | ||
| }); | ||
| } else if (parsed.stopReason && typeof parsed.stopReason === "string") { | ||
| // Normal terminal metadata without refusal or error: emit completion event | ||
| events.push({ | ||
| type: "completion", | ||
| data: { | ||
| stopReason: parsed.stopReason, | ||
| }, | ||
| }); | ||
| } | ||
| } catch { | ||
| // JSON parsing failed for a balanced frame: could be a junk wrapper like '{junk{...}}' | ||
|
|
@@ -807,6 +789,14 @@ export const streamKiroApiKey: StreamFunction<"kiro-codewhisperer-stream"> = ( | |
| let toolComplete = false; // Track whether the current tool has been completed (has stop flag) | ||
| let toolcallIndex: number | undefined; | ||
| const pendingToolCalls: Array<{ input: string; toolCall: ToolCall; index: number }> = []; | ||
| // Track terminal metadata so we can emit confirmed text even if later reads fail | ||
| let hasReceivedTerminalMetadata = false; // Set to true when usage event is received | ||
| let completionSeen = false; // Set to true when completion event is received | ||
| let textIndex: number | undefined; // Track text block index (accessible in catch) | ||
| let textContentConfirmed = false; // Track whether text has been committed to blocks (accessible in catch) | ||
| let textStartDeferred = false; // Track if text_start is deferred (accessible in catch) | ||
| let pendingTextContent = ""; // Text buffer (accessible in catch) | ||
| const pendingTextEvents: Array<{ type: "start" | "delta" | "end"; delta?: string; content?: string }> = []; // Deferred text events (accessible in catch) | ||
|
|
||
| try { | ||
| // Use validated identity snapshot from trust check if available (no second read of getters), | ||
|
|
@@ -879,7 +869,8 @@ export const streamKiroApiKey: StreamFunction<"kiro-codewhisperer-stream"> = ( | |
| let buffer = ""; | ||
| let lastContent = ""; | ||
| let thinkingIndex: number | undefined; | ||
| // Note: textIndex, textContentConfirmed, textStartDeferred, pendingTextContent, pendingTextEvents | ||
| // are declared before the try block so they're accessible in the catch block | ||
|
|
||
| // Add tool to blocks without emitting events (deferred until stream end) | ||
| const addToolToBlocks = () => { | ||
|
|
@@ -938,8 +929,6 @@ export const streamKiroApiKey: StreamFunction<"kiro-codewhisperer-stream"> = ( | |
| toolComplete = false; // Reset tool completion flag | ||
| }; | ||
|
|
||
| const appendText = (delta: string) => { | ||
| // Set firstTokenTime on first real (non-thinking) text delta | ||
| if (!firstTokenEmitted) { | ||
|
|
@@ -951,48 +940,56 @@ export const streamKiroApiKey: StreamFunction<"kiro-codewhisperer-stream"> = ( | |
| } | ||
| if (textIndex === undefined) { | ||
| textIndex = blocks.length; | ||
| // Create block with empty text (content will be added only after confirmation) | ||
| blocks.push({ type: "text", text: "", index: textIndex }); | ||
| // ALWAYS defer text_start until we have terminal metadata or reach EOF without error. | ||
| // This ensures unconfirmed text is not exposed if an error occurs. | ||
| textStartDeferred = true; | ||
| } | ||
| // Buffer text content separately (don't add to blocks yet) | ||
| pendingTextContent += delta; | ||
| // Don't emit text_delta yet; defer until stream end or terminal metadata is received. | ||
| // Queue delta for later emission when confirmed. | ||
| pendingTextEvents.push({ type: "delta", delta }); | ||
| }; | ||
|
|
||
| // Commit pending text to blocks (only safe at stream end when refusal is ruled out) | ||
| const commitPendingText = () => { | ||
| if (textContentConfirmed || pendingTextContent.length === 0) return; // Already committed or no text | ||
| textContentConfirmed = true; | ||
| if (textIndex !== undefined && textIndex < blocks.length) { | ||
| // Populate the text block that was created with empty text | ||
| const block = blocks[textIndex] as TextContent; | ||
| block.text = pendingTextContent; | ||
| } | ||
| }; | ||
|
|
||
| // Emit deferred text events if text_start was not yet sent | ||
| const emitDeferredTextEvents = () => { | ||
| if (textStartDeferred && textIndex !== undefined && textIndex < blocks.length) { | ||
| const block = blocks[textIndex]; | ||
| if (block && block.type === "text" && block.text.length > 0) { | ||
| // Emit deferred text_start | ||
| stream.push({ type: "text_start", contentIndex: textIndex, partial: output }); | ||
| // Emit all pending text_delta events | ||
| for (const evt of pendingTextEvents) { | ||
| if (evt.type === "delta" && evt.delta) { | ||
| stream.push({ | ||
| type: "text_delta", | ||
| contentIndex: textIndex, | ||
| delta: evt.delta, | ||
| partial: output, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| textStartDeferred = false; | ||
| } | ||
| }; | ||
|
|
||
| // Emit text_end if text block was opened | ||
| const closeTextBlock = () => { | ||
| if (textIndex !== undefined && textContentConfirmed) { | ||
| const block = blocks[textIndex] as TextContent; | ||
| stream.push({ type: "text_end", contentIndex: textIndex, content: block.text, partial: output }); | ||
| textIndex = undefined; | ||
|
|
@@ -1110,7 +1107,12 @@ export const streamKiroApiKey: StreamFunction<"kiro-codewhisperer-stream"> = ( | |
| firstTokenEmitted = true; | ||
| firstTokenTime = Date.now(); | ||
| } | ||
| if (currentTool && currentTool.id !== event.data.toolUseId) { | ||
| // Only emit previous tool if it was completed | ||
| if (toolComplete) addToolToBlocks(); | ||
| currentTool = undefined; // Clear the incomplete tool | ||
| toolComplete = false; // Reset for new tool | ||
| } | ||
| if (!currentTool) { | ||
| currentTool = { id: event.data.toolUseId, name: event.data.name, input: event.data.input }; | ||
| toolComplete = false; // New tool starts as incomplete | ||
|
|
@@ -1135,15 +1137,30 @@ export const streamKiroApiKey: StreamFunction<"kiro-codewhisperer-stream"> = ( | |
| if (event.data.inputTokens !== undefined) output.usage.input = event.data.inputTokens; | ||
| if (event.data.outputTokens !== undefined) output.usage.output = event.data.outputTokens; | ||
| output.usage.totalTokens = output.usage.input + output.usage.output; | ||
| // Mark that terminal metadata has been received - text is now confirmed safe | ||
| hasReceivedTerminalMetadata = true; | ||
| } else if (event.type === "completion") { | ||
| // Normal terminal metadata (stopReason: "COMPLETED", etc.) without refusal | ||
| // Mark completion as seen - text is now confirmed safe even without usage event | ||
| completionSeen = true; | ||
| hasReceivedTerminalMetadata = true; | ||
| // Finding #2: Immediately commit and emit all pending text events on completion | ||
| commitPendingText(); | ||
| emitThinking(); | ||
| emitDeferredTextEvents(); | ||
| closeTextBlock(); | ||
|
Comment on lines
+1184
to
+1188
There was a problem hiding this comment.
When another content frame arrives after AGENTS.md reference: AGENTS.md:L171-L173 Useful? React with 👍 / 👎.
Comment on lines
+1184
to
+1188
There was a problem hiding this comment.
When content is pending from an earlier read and a later AGENTS.md reference: AGENTS.md:L169-L173 Useful? React with 👍 / 👎. |
||
| } else if (event.type === "refusal") { | ||
| const refusalData = event.data as { | ||
| stopReason?: string; | ||
| stopDetails?: { refusal?: { category?: string; explanation?: string } }; | ||
| }; | ||
| if (refusalData.stopDetails?.refusal) { | ||
| // Handle refusal: clear pending content and emit error (don't emit any text/tool that came before) | ||
| consumeContent(""); // Flush any pending thinking | ||
| clearPendingToolCalls(); // DROP any pending tool call without emitting events | ||
| // Clear pending text (it was never committed to blocks, so it's safe to discard) | ||
| pendingTextContent = ""; | ||
| pendingTextEvents.length = 0; | ||
|
|
||
| const refusal = refusalData.stopDetails.refusal; | ||
| const category = refusal.category; | ||
|
|
@@ -1182,26 +1199,74 @@ export const streamKiroApiKey: StreamFunction<"kiro-codewhisperer-stream"> = ( | |
| return; | ||
| } | ||
| } else if (event.type === "error") { | ||
| // On ordinary server errors (error event in stream), emit deferred text and tool events before error | ||
| // (refusals drop them, incomplete tools must not be emitted) | ||
|
|
||
| // For server-sent errors, emit the deferred text first (it was received before the error) | ||
| // Only emit confirmed text (text is confirmed safe only after completion or usage event) | ||
| if (textContentConfirmed === false && (completionSeen || hasReceivedTerminalMetadata)) { | ||
| // Commit pending text to blocks | ||
| if (textIndex !== undefined && textIndex < blocks.length && pendingTextContent.length > 0) { | ||
| const block = blocks[textIndex] as TextContent; | ||
| block.text = pendingTextContent; | ||
| textContentConfirmed = true; | ||
| } | ||
| // Emit deferred text start if not yet emitted | ||
| if (textStartDeferred && textIndex !== undefined && textIndex < blocks.length) { | ||
| const block = blocks[textIndex]; | ||
| if (block && block.type === "text" && block.text.length > 0) { | ||
| stream.push({ type: "text_start", contentIndex: textIndex, partial: output }); | ||
| // Emit all pending text_delta events | ||
| for (const evt of pendingTextEvents) { | ||
| if (evt.type === "delta" && evt.delta) { | ||
| stream.push({ | ||
| type: "text_delta", | ||
| contentIndex: textIndex, | ||
| delta: evt.delta, | ||
| partial: output, | ||
| }); | ||
| } | ||
| } | ||
| // Finding #4: Emit text_end for confirmed text before error | ||
| stream.push({ type: "text_end", contentIndex: textIndex, content: block.text, partial: output }); | ||
| } | ||
| textStartDeferred = false; | ||
|
There was a problem hiding this comment.
When one batch contains content, successful terminal metadata, and then an ordinary error, this branch emits AGENTS.md reference: AGENTS.md:L171-L173 Useful? React with 👍 / 👎. |
||
| } | ||
| } | ||
|
|
||
| // Only emit the tool if it was explicitly completed (has stop flag) | ||
| if (toolComplete) addToolToBlocks(); | ||
| emitPendingToolCalls(); | ||
|
|
||
| // Preserve text in the error context only if confirmed (text is confirmed safe only after completion or usage event) | ||
| const accumulatedText = textContentConfirmed | ||
| ? blocks | ||
| .filter((b): b is TextContent => b.type === "text") | ||
| .map(b => b.text) | ||
| .join("") | ||
| : ""; // Discard unconfirmed pending text before error | ||
| const errorMsg = sanitizeKiroError(`${event.data.error}: ${event.data.message ?? ""}`, apiKey); | ||
| const fullErrorMsg = accumulatedText ? `${errorMsg}\n\nPartial output: ${accumulatedText}` : errorMsg; | ||
| throw new Error(fullErrorMsg); | ||
|
Comment on lines
1304
to
1306
There was a problem hiding this comment.
When one response read contains AGENTS.md reference: AGENTS.md:L171-L171 Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Addressed on exact head Verification at this head: Disposition: finding fixed; both human reviewers were requested on this exact head. No merge before exact-head CI and review disposition are clear. — |
||
| } | ||
| } | ||
| } | ||
|
|
||
| // Check for incomplete JSON frame in leftover buffer at EOF | ||
| if (buffer.trim().length > 0) { | ||
| // Non-whitespace content remains in buffer | ||
| const openBraceIdx = buffer.indexOf("{"); | ||
| if (openBraceIdx >= 0 && findJsonEnd(buffer, openBraceIdx) < 0) { | ||
| // Incomplete JSON frame at EOF (opening brace with no matching close) | ||
| throw new Error(`Kiro API key stream truncated: incomplete JSON frame at EOF`); | ||
| } | ||
| } | ||
|
|
||
| // Stream is done - commit pending text (safe since no refusal occurred), add pending tool, emit accumulated thinking, etc. | ||
| // First commit pending text to blocks since stream ended without refusal | ||
| commitPendingText(); | ||
| // Emit tool only if explicitly completed (has stop flag); incomplete tools are discarded at EOF | ||
| if (currentTool && toolComplete) addToolToBlocks(); | ||
| emitThinking(); | ||
| emitDeferredTextEvents(); | ||
| // Now emit all tool call events in order (safe since no refusal occurred) | ||
|
|
@@ -1237,10 +1302,41 @@ export const streamKiroApiKey: StreamFunction<"kiro-codewhisperer-stream"> = ( | |
| stream.push({ type: "done", reason: output.stopReason as "stop" | "toolUse", message: output }); | ||
| stream.end(); | ||
| } catch (error) { | ||
| // On ordinary errors (including reader.read() throws): | ||
| // - Emit COMPLETED tool events (same semantics as in the event processing loop) | ||
| // - Emit confirmed text only if terminal metadata (usage) was received before the error | ||
| // - Incomplete currentTool is NOT finalized/emitted (only completed tools in pendingToolCalls flush). | ||
| // - Unconfirmed text is suppressed (no deferred text emission without terminal metadata) | ||
|
|
||
| // Emit confirmed text if we received completion event or terminal metadata (usage) | ||
| if ((hasReceivedTerminalMetadata || completionSeen) && textContentConfirmed === false) { | ||
| // Commit pending text to blocks | ||
| if (textIndex !== undefined && textIndex < blocks.length && pendingTextContent.length > 0) { | ||
| const block = blocks[textIndex] as TextContent; | ||
| block.text = pendingTextContent; | ||
| textContentConfirmed = true; | ||
| } | ||
| // Emit deferred text start if not yet emitted | ||
| if (textStartDeferred && textIndex !== undefined && textIndex < blocks.length) { | ||
| const block = blocks[textIndex]; | ||
| if (block && block.type === "text" && block.text.length > 0) { | ||
| stream.push({ type: "text_start", contentIndex: textIndex, partial: output }); | ||
| // Emit all pending text_delta events | ||
| for (const evt of pendingTextEvents) { | ||
| if (evt.type === "delta" && evt.delta) { | ||
| stream.push({ | ||
| type: "text_delta", | ||
| contentIndex: textIndex, | ||
| delta: evt.delta, | ||
| partial: output, | ||
| }); | ||
| } | ||
| } | ||
| stream.push({ type: "text_end", contentIndex: textIndex, content: block.text, partial: output }); | ||
| } | ||
| textStartDeferred = false; | ||
| } | ||
| } | ||
|
|
||
| // Emit all pending tool call events (inline of emitPendingToolCalls logic) | ||
| for (const { input, toolCall, index } of pendingToolCalls) { | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When Kiro returns
stopReason: "CONTENT_FILTERED"without a populatedstopDetails.refusalobject, this branch classifies the frame as a successful completion. If the connection subsequently rejects or is aborted, the catch path treats the quarantined text as confirmed and emits it; on clean EOF it also returns the text as a successful response. Branch onCONTENT_FILTEREDbefore the generic completion case and produce a refusal even when category or explanation details are absent.Useful? React with 👍 / 👎.