Repository navigation
fix(kiro): correct OAuth CodeWhisperer wire protocol (#6164) - #6165
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Draft until a live Kiro OAuth (Power) request confirms it. Item 1 is backed by the reporter's curl A/B; items 2–3 and the — |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0d9fe08c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const toolUseId = ev.toolUseId ?? ""; | ||
| const name = ev.name ?? ""; | ||
| const input = ev.input ?? ""; | ||
|
|
||
| // Accumulate input fragments per toolUseId | ||
| let accumulated = accumulator.get(toolUseId); | ||
| if (!accumulated) { | ||
| accumulated = { name, input: "" }; | ||
| accumulator.set(toolUseId, accumulated); |
There was a problem hiding this comment.
When CodeWhisperer sends continuation payloads containing only {input} or the terminal {stop: true}, ev.toolUseId ?? "" creates a separate accumulator under the empty key, so the terminal event emits a tool call with an empty ID/name while the real call remains buffered. The sibling Kiro parser explicitly handles these standalone input and stop fragments in kiro-api-key.ts:769-772; this transport likewise needs to associate ID-less fragments with the currently active call rather than indexing them by "".
Useful? React with 👍 / 👎.
| const requestHeaders: Record<string, string> = { | ||
| "content-type": "application/json", | ||
| accept: "application/vnd.amazon.eventstream", | ||
| authorization: `Bearer ${bearerToken}`, | ||
| "amzn-X-amz-target": "AmazonCodeWhispererService.GenerateAssistantResponse", | ||
| "Content-Type": "application/x-amz-json-1.0", | ||
| Accept: "application/vnd.amazon.eventstream", | ||
| Authorization: `Bearer ${bearerToken}`, | ||
| "X-Amz-Target": "AmazonCodeWhispererStreamingService.GenerateAssistantResponse", |
There was a problem hiding this comment.
When options.headers supplies a conventional lowercase override such as authorization, content-type, or x-amz-target, the later Object.assign no longer replaces these newly title-cased defaults. Fetch normalizes header names case-insensitively and combines the duplicate values, producing values such as Bearer original, Bearer override or two media types, which AWS can reject; merge through Headers.set() or otherwise replace keys without regard to casing.
Useful? React with 👍 / 👎.
| // Mock fetch to capture request and serve responses | ||
| let capturedRequest: { headers: Record<string, string>; body: string } | null = null; | ||
|
|
||
| global.fetch = async (url: string, init?: RequestInit) => { |
There was a problem hiding this comment.
Loading this test file permanently replaces global.fetch, and the final two tests replace it again without cleanup. Other test files executed in the same Bun process can therefore inherit this synthetic Kiro response, making the suite order-dependent; install the mock with vi.spyOn and restore it in cleanup, as required by the repository's prohibition on long-lived global mutations.
AGENTS.md reference: AGENTS.md:L169-L173
Useful? React with 👍 / 👎.
f75fa3b to
2c573f8
Compare
2c573f8 to
10256d9
Compare
PR #6165 Fix-Forward ReportNew Head SHA: Findings FixedP1: Associate ID-less fragments with the active tool callIssue: When CodeWhisperer sends continuation payloads containing only Fix: Modified Files Changed:
P2: Merge custom headers case-insensitivelyIssue: When Fix: Replaced Files Changed:
P1: Restore global fetch mock after test suiteIssue: The test file permanently replaced Fix: Refactored test mock setup to use Files Changed:
Test ResultsType ChecksUnit Tests - Kiro CodeWhisperer Wire ProtocolAll Kiro TestsRebase Status✅ Rebased onto SummaryAll three PR review findings have been fixed at the source:
The changes maintain backward compatibility and all existing Kiro tests pass. New test case validates the ID-less fragment fix. — |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10256d9b45
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (blocks.length === 0) { | ||
| stream.push({ type: "start", partial: output }); | ||
| } |
There was a problem hiding this comment.
For a tool-only response whose arguments arrive in multiple frames, blocks remains empty until the terminal stop, so this branch emits a start event for every fragment. The agent loop treats every start as a new assistant message (packages/agent/src/agent-loop.ts:4909-4914), leaving duplicate assistant entries in the conversation after the final message replaces only the last one; the next tool-result turn can consequently send invalid history. Track whether start has already been emitted independently of blocks.length.
Useful? React with 👍 / 👎.
| afterEach(() => { | ||
| mockFetch.mockClear(); | ||
| }); |
There was a problem hiding this comment.
The follow-up changed the direct assignment to a spy, but the fresh code only calls mockClear(), which clears invocation history without restoring globalThis.fetch or resetting the final custom implementation. Tests subsequently loaded in the same Bun process can therefore inherit the last synthetic Kiro response; restore the spy in afterAll (or install and restore it per test).
AGENTS.md reference: AGENTS.md:L173-L173
Useful? React with 👍 / 👎.
probepark
left a comment
probepark
left a comment
There was a problem hiding this comment.
Review (head 10256d9, gajae-reviewer on behalf of probepark)
CI: green so far — Affected path validation (check:@gajae-code/ai, 3 kiro/aws test shards, ts-build) passed; native-build / gjc-state-gates / native addon still pending. No failures.
Scope: +486 / -59, 5 files — packages/ai/src/providers/kiro-codewhisperer.ts, 3 tests under packages/ai/test/, packages/ai/changelog.d/ fragment
Conventions: changelog fragment present, no generated files, no labels
Notable:
packages/ai/src/providers/kiro-codewhisperer.ts:312-314—startis still gated onblocks.length === 0, buthandleToolUseEventnow only pushes a block onstop. For a tool-only response whose input arrives in N fragments,startis emitted N times.packages/agent/src/agent-loop.ts:4909-4914pushes a new assistant message intocontext.messageson everystart, so the history gets duplicate partial assistant entries and the following tool-result turn can send invalid history. Before this PR everytoolUseEventpushed a block immediately, so this is a regression introduced by the accumulator. Track astartedflag independent ofblocks.length(and cover a tool-only multi-fragment stream that asserts exactly onestart). The new tests don't catch it because they only look uptoolcall_end.packages/ai/test/kiro-codewhisperer-wire-protocol.test.ts:360-364—vi.spyOn(globalThis, "fetch")is installed at module load and onlymockClear()ed inafterEach. Nothing callsmockRestore(), and the last two tests replace the implementation withmockImplementation. The synthetic Kiro response leaks to later files in the same Bun process (AGENTS.md: spies must be cleaned up / no long-lived global mutation). Restore it inafterAll, or install and restore it per test.
Non-blocking:
kiro-codewhisperer.ts:325-328: the outertoolInputAccumulator.delete(ev.toolUseId ?? "")is redundant (handleToolUseEventalready deletes the resolved id). For an ID-lessstopit deletes""instead of the resolved id, so it does nothing.- The changelog fragment says profileArn was "moved from header to request body". The body field already existed on base (
profileArn: options.profileArninbuildConversationState); this PR only drops the header.
Blocking: 1 and 2 above.
Body verdict line count=0 — PR body not updated. Suggested verdict line: gajae.pr-review-verdict.v1 needs-human sha256:8ddaf1ae10e52249df852b3837e690d78a530d6c0fc83f79bdf7a2346e79da0c reviewer:critic reviewer-id:gajae-reviewer evidence:duplicate-start-on-fragmented-toolcall;fetch-spy-not-restored
Verdict: gajae.pr-review-verdict.v1 needs-human sha256:8ddaf1ae10e52249df852b3837e690d78a530d6c0fc83f79bdf7a2346e79da0c reviewer:critic reviewer-id:gajae-reviewer evidence:duplicate-start-on-fragmented-toolcall;fetch-spy-not-restored
…oad structure
Fix critical issues in the Kiro OAuth CodeWhisperer streaming transport:
1. Request headers: correct typo 'amzn-X-amz-target' to 'x-amz-target'
with correct streaming service target value
2. Content-Type: change from 'application/json' to 'application/x-amz-json-1.0'
3. profileArn: move from header 'x-amzn-codewhisperer-proflearn' to request
body conversationState (already done in buildConversationState)
4. userInputMessageContext.tools: flatten from {tools:[...]} to direct array
5. Event payloads: handle flat structure ({content}) not nested ({assistantResponseEvent:{content}})
6. Tool input fragments: accumulate streaming chunks per toolUseId until stop signal
Cross-verified against kiro-api-key.ts which already uses correct headers.
Live Kiro verification not possible from this host.
Reported by: nomo
Fixes: Kiro OAuth wire protocol validation failures
Tests: failing-first unit tests for all six issues
Tested: aws-region-credential-destination.test.ts, kiro-codewhisperer-endpoint.test.ts
Lore-id: kiro-oauth-wire-protocol-fix
Confidence: high
Scope-risk: contained to kiro-codewhisperer.ts streaming only
…, ID-less fragments, test cleanup
- Track 'started' flag separate from blocks.length to prevent duplicate start events on fragmented tool-only responses - Add mockRestore() in afterAll() to cleanup fetch spy and prevent leaks to other test files - Add test verifying exactly one start event for multi-fragment tool stream
10256d9 to
f4790d6
Compare
Fix Lane Review — PR #6165New head: Blocking Findings (Fixed)1. Duplicate start events on fragmented tool-only responses
2. Fetch spy not restored (test leak)
3. Added test coverage for fragmented tool-only streams
Test ResultsKiro-specific tests (wire protocol + endpoint)bun test packages/ai/test/kiro-codewhisperer-wire-protocol.test.ts \
packages/ai/test/kiro-codewhisperer-endpoint.test.ts✅ 21 pass, 0 fail (all passing, including new test) Type checkingcd packages/ai && bunx tsc --noEmit✅ No errors (types are clean) Agent test suite (affected by message handling)bun test packages/agent/test/✅ 948 pass, 0 fail (verify no regression in message history handling) Rebase Status✅ Rebased onto current origin/dev — no conflicts. New head after rebase: PR Body VerdictUpdated verdict line (exact head matching SHA): Known Pre-existing Issues (Not Fixed Here)
[repo owner's gaebal-gajae (clawdbot) 🦞] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4790d65e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| apiKey: "test-token", | ||
| }); | ||
|
|
||
| const events: any[] = []; |
There was a problem hiding this comment.
Replace this any[] collector, and the identical collectors below, with AssistantMessageEvent[] and narrow on event.type. The current declarations bypass type-checking for every assertion on delta and toolCall, so these regression tests can silently drift from the provider event contract; the repository explicitly prohibits any, and the exported event union already provides the required type.
AGENTS.md reference: AGENTS.md:L126-L126
Useful? React with 👍 / 👎.
probepark
left a comment
probepark
left a comment
There was a problem hiding this comment.
Review (head f4790d6, gajae-reviewer on behalf of probepark)
CI: green. All planned checks pass at this head: Affected path validation with check:@gajae-code/ai, 3 kiro/aws test shards, ts-build, native-build, plus gjc-state-gates and Virtual integration. The approve gate returned ALLOW (covered: check:@gajae-code/ai).
Scope: +584 / -61, 5 files. Reviewable source: packages/ai/src/providers/kiro-codewhisperer.ts (+71/-59). Also in the diff: 3 tests under packages/ai/test/ and the packages/ai/changelog.d/ fragment.
Conventions: changelog fragment present, no generated files, no labels.
Both prior blockers (review on 10256d9) are resolved:
kiro-codewhisperer.ts:189,304-306,314-316:startis now gated by a dedicatedstartedflag, notblocks.length === 0. A tool-only stream whose input arrives in fragments now emitsstartonce. This is covered by the new testkiro-codewhisperer-wire-protocol.test.ts:410("emits start exactly once for a tool-only multi-fragment stream", asserting onstartevents at :488).kiro-codewhisperer-wire-protocol.test.ts:102-104:afterAll(() => mockFetch.mockRestore())restores the module-levelfetchspy.
Checked and clean: the header merge (new Headers + case-insensitive set, :229-235) lets user headers override the defaults without leaving lower-case/Pascal-case duplicates. The flat tools array (:462) matches the WireUserMessage type change. messageMetadataEvent and assistantResponseEvent now read the flat payloads. The ID-less fragment fallback attaches to the most recently inserted accumulator key.
Notes (non-blocking):
kiro-codewhisperer.ts:319-322: the outertoolInputAccumulator.delete(ev.toolUseId ?? "")is still redundant, becausehandleToolUseEventalready deletes the resolved id. Separately, a tool call whose fragments never get astopbefore the stream ends is silently dropped at finalize: nothing is emitted, andstopReasonbecomesstop. That may be acceptable for the service contract, but it is not covered.- The changelog fragment still says profileArn was "moved from header to request body". The body field already existed on base; this PR only drops the
x-amzn-codewhisperer-proflearnheader.
Blocking: none.
PR body verdict line count=0, not updated. Suggested verdict line: gajae.pr-review-verdict.v1 merge-approved sha256:dab02bb02bc2880d17f08b89b8d0d7dcd72c28ff416caf8fe4d001dbdfcea48c reviewer:human reviewer-id:probepark evidence:ci-green;approve-gate-allow;prior-blockers-fixed;started-flag-and-spy-restore-checked
Verdict: gajae.pr-review-verdict.v1 merge-approved sha256:dab02bb02bc2880d17f08b89b8d0d7dcd72c28ff416caf8fe4d001dbdfcea48c reviewer:human reviewer-id:probepark evidence:ci-green;approve-gate-allow;prior-blockers-fixed;started-flag-and-spy-restore-checked
snowykr
left a comment
snowykr
left a comment
There was a problem hiding this comment.
Verdict
CHANGES_REQUESTED
Summary
The PR corrects the OAuth CodeWhisperer request headers and tool-array shape, reads flat response events, and accumulates tool arguments until stop. The normal completed-stream path is consistent with the intended correction. One terminal-state defect remains: a clean transport EOF can silently discard unfinished tool calls while reporting a successful assistant turn.
Reviewed head: f4790d65e448bc9dc7cef487801ddb25d65097a4. Base and merge-base: 7acdc8e501a9d13ce551eb8e12addac5cb35efe4.
Findings / Required Changes
- [P2] Reject EOF while tool input is still being accumulated —
packages/ai/src/providers/kiro-codewhisperer.ts:637-638- This PR introduces deferred per-ID tool state: non-stop fragments remain in
toolInputAccumulatorrather thanoutput.content. Successful finalization at lines 352–367 never checks that new state. The base handler materialized recognized nested events immediately; it also mishandled the real flat protocol, so this finding concerns the newly introduced accumulator's missing completion invariant, not a claim that flat streams previously worked. - A reachable failure is a valid eventstream that completes tool A, starts tool B with partial input, and then closes cleanly after a complete AWS frame without B's
stop. Text followed by an unfinished tool has the same problem. The shared decoder accepts frame-aligned EOF (aws-eventstream.ts:166-182), and the provider emitsdonewith only A, or with the preceding text. CRC, trailing-byte, explicit exception, and abort guards do not detect this semantic truncation. - The terminal stream resolves that incomplete message (
utils/event-stream.ts:255-270); the agent records it (packages/agent/src/agent-loop.ts:4980-5003). Its error/aborted guard is bypassed, and surviving valid calls can execute (agent-loop.ts:4270-4340). Argument validation cannot detect B because B is absent from the message. This silently loses an operation and can execute a completed subset of an incomplete turn, so correction is required before merge. - Before successful finalization, reject a nonempty accumulator through the existing error path. Do not flush unfinished arguments as completed calls. Add complete-frame EOF fixtures for completed A plus unfinished B and for text plus unfinished B, asserting an error rather than
done.
- This PR introduces deferred per-ID tool state: non-stop fragments remain in
CI / Verification
Dev CI run 36818348772 completed successfully on the exact reviewed head. The wire-protocol, endpoint, and AWS region/credential-destination test tasks all executed successfully. The affected-evidence producer and protected aggregate validation succeeded; the AI package check and dependent TypeScript builds were also successful.
The new wire-protocol suite contains nine tests and exercises production request construction and AWS frame decoding with synthetic responses, including explicit-ID and ID-less fragment completion and single-start behavior. Its tool-fragment fixtures finish with stop; they do not cover the semantic EOF failure above. No PR code or tests were executed locally. Materialized-merge canary steps were skipped; the green virtual-integration job is not evidence that those canaries ran. Manual/host-policy checks were excluded from the assessment.
Axis Coverage
| Axis | Verdict | Coverage |
|---|---|---|
| A1 — Intent / Policy / Contract | APPROVED | Request and completed-stream behavior match the stated wire correction; policies and terminal consumers inspected. Shared completion defect is counted once under A2. No intent_projection was available. |
| A2 — Architecture / Correctness / Failure | CHANGES_REQUESTED | Finding 1: new deferred tool state is not checked at successful EOF; decoder, terminal result, execution guards, and history consumers traced. |
| A3 — Security / Privacy / Trust | APPROVED | Validated credential destinations, redirect rejection, caller-header authority, Unicode evidence, and tool-validation boundaries; no introduced trust-boundary defect established. |
| A4 — Verification / Tests / CI | APPROVED | Exact-head selected tests and aggregate CI validated; no independent blocking test/CI defect. The regression fixture requested in Finding 1 supports that correctness fix. |
| A5 — Context / Compatibility / Platform | APPROVED | OAuth/API-key dispatch, real stream consumers, header overrides, region routing, diagnostics, and shared abstractions inspected; no independent compatibility or materially costly duplication defect. |
Limitations
All substantive code analysis was performed by complementary review subagents against the pinned commits, including successful retries after output-schema failures. Conclusions are based on static source/consumer tracing and exact-head CI metadata, not live Kiro service verification. Job-log and finalized-artifact downloads returned authorization errors in the verification lane, so test counts and artifact contents were not independently verified. Service guarantees about interleaved ID-less tool events were unavailable and were not promoted to findings.
Add validation to reject clean transport EOF when tool input is being accumulated but not yet finished. The toolInputAccumulator now checks for incomplete tool calls at successful finalization before emitting the done message. Add two test fixtures for incomplete tool states: 1. Completed tool A plus unfinished tool B 2. Text content plus unfinished tool B Both scenarios now properly error instead of silently discarding the incomplete tool call. Fixes review finding P2 on PR #6165.
Fix SummaryNew head: Finding Fixed: [P2] Reject EOF while tool input is still being accumulatedLocation: Issue: When a clean transport EOF occurred while Fix: Added a validation check before successful finalization that rejects any EOF with non-empty // Reject EOF while tool input is still being accumulated
if (toolInputAccumulator.size > 0) {
const unfinishedIds = Array.from(toolInputAccumulator.keys()).join(", ");
throw new Error(`Kiro CodeWhisperer stream ended with incomplete tool calls: ${unfinishedIds}`);
}Test Coverage AddedTwo comprehensive regression fixtures were added to
Both fixtures use the frame encoding utilities from VerificationTest Results: Behavioral Validation:
Files Changed
[repo owner's gaebal-gajae (clawdbot) 🦞] |
|
@snowykr the current head — |
probepark
left a comment
probepark
left a comment
There was a problem hiding this comment.
Review (head 153235e, gajae-reviewer on behalf of probepark)
CI: green. All planned checks pass at this head (run 36824647340): Affected path validation with check:@gajae-code/ai, the 3 kiro/aws test shards, both ts-builds, native-build, cli-smoke, plus gjc-state-gates and Virtual integration. Approve gate: ALLOW (covered: check:@gajae-code/ai).
Scope: +758 / -61, 5 files. Delta since my f4790d6 review is one commit: packages/ai/src/providers/kiro-codewhisperer.ts +6 and packages/ai/test/kiro-codewhisperer-endpoint.test.ts +168.
Conventions: changelog fragment present, no generated files, no labels.
snowykr's P2 on f4790d6 (EOF with unfinished tool input) is resolved:
kiro-codewhisperer.ts:355-359: before successful finalization, a non-emptytoolInputAccumulatornow throws. The throw lands in the existing catch at :375, which setsstopReason: "error"and pusheserrorinstead ofdone. Unfinished arguments are not flushed as completed calls. The plainErrorhas no status/code, sotransportFailureFactsyields no retryable transport facts. The check sits after the abort check (:353), so an aborted stream still reportsaborted.kiro-codewhisperer-endpoint.test.ts:367(completed A + unfinished B) and:433(text + unfinished B) build CRC-valid, frame-aligned EOF streams and assertincomplete tool calls/tool-bon the error message. This matches the fixtures snowykr asked for.streamErrorrestoresglobalThis.fetchinfinally(:242-244).
Re-checked and clean at this head: the started flag (:189, :304-307, :314-317) and the afterAll fetch-spy restore from the previous round are unchanged.
Notes (non-blocking):
kiro-codewhisperer-endpoint.test.ts:438: the "text" frame in the second test uses the nested{ assistantResponseEvent: { content } }payload. Since this PR, the provider only reads the flatev.content(:299-300), so the frame emits no text, and the test really covers "unfinished B alone". Switch it to{ content: "Reading file" }so the text+unfinished shape is actually exercised. The first test's text frame (:372) has the same issue.kiro-codewhisperer.ts:319-322: the outertoolInputAccumulator.delete(ev.toolUseId ?? "")is still redundant. Because of the new EOF check, it can also mask one case: an ID-lessstopthat resolves to the last key deletes that key insidehandleToolUseEvent, and the outer call then deletes a leftover""placeholder, which makes it bypass the EOF rejection. This is contrived; dropping the outer delete removes it. (The codex note onany[]collectors in the wire-protocol test is style, not blocking.)
Blocking: none. snowykr's CHANGES_REQUESTED is on f4790d6. Its single finding is fixed here, but reviewDecision will keep aggregating it until snowykr re-reviews.
PR body verdict line count=0, not updated. Suggested verdict line: gajae.pr-review-verdict.v1 merge-approved sha256:230cd30419c72888d7d8826c6dc77958220562105a726d06c58f7eac184dba0e reviewer:human reviewer-id:probepark evidence:ci-green;approve-gate-allow;eof-unfinished-tool-rejected;peer-p2-fixed;fixtures-checked
Verdict: gajae.pr-review-verdict.v1 merge-approved sha256:230cd30419c72888d7d8826c6dc77958220562105a726d06c58f7eac184dba0e reviewer:human reviewer-id:probepark evidence:ci-green;approve-gate-allow;eof-unfinished-tool-rejected;peer-p2-fixed;fixtures-checked
|
@snowykr your CHANGES_REQUESTED is on old head |
snowykr
left a comment
snowykr
left a comment
There was a problem hiding this comment.
Verdict
APPROVED
Summary
This PR corrects the Kiro OAuth CodeWhisperer streaming request headers and payload, parses flat stream events, accumulates fragmented tool input, and rejects incomplete tool calls at EOF. No merge-blocking defects were found at the reviewed head.
Findings / Required Changes
- [P3] Correct the
profileArnchangelog wording —packages/ai/changelog.d/kiro-oauth-wire-protocol.md:7- The note says
profileArnmoved intoconversationState, but the base already included it there. This change removes the extraprofileArnheader while retaining its existing payload placement. There is no runtime impact; this is non-blocking. Describe the change as removing the erroneous header while retaining the existing body field.
- The note says
Non-blocking Observations
- The EOF fixtures at
packages/ai/test/kiro-codewhisperer-endpoint.test.ts:372and:438encode the former nested assistant-text payload. The head parser reads the flat event shape, so those text frames do not exercise the claimed text-plus-unfinished-tool scenario, though the incomplete-tool EOF assertions remain valid. Use flat payloads and assert the emitted text, or remove the unrelated frames. - The changed tests could optionally assert successful terminal
done/absence oferrorevents and that incomplete-EOF cases do not emitdone.
CI / Verification
GitHub reported 27 completed check runs for reviewed head 153235e330439ec103a719d88d2bab415649ffc7: 20 successful, 7 skipped, 0 failed, and 0 pending. The changed Kiro wire-protocol and endpoint tests, AWS-region credential test, and affected-path validation passed. Skips were platform/opt-in checks. No tests or gates were run during this review.
Axis Coverage
| Axis | Verdict | Coverage |
|---|---|---|
| A1 — Intent / Policy / Contract | APPROVED |
The principal protocol changes align with the implementation; only the non-blocking changelog wording above is inaccurate. |
| A2 — Architecture / Correctness / Failure | APPROVED |
Fragment accumulation, EOF handling, framing failures, and event lifecycle were reviewed; no demonstrated merge-blocking regression. |
| A3 — Security / Privacy / Trust | APPROVED |
Credential handling, fixed endpoint/region checks, remote tool data, and downstream tool dispatch were reviewed; no new boundary bypass found. |
| A4 — Verification / Tests / CI | APPROVED |
Relevant exact-head tests and CI outcomes were reviewed; optional terminal-sequence assertions are noted above. |
| A5 — Context / Compatibility / Platform | APPROVED |
Provider registration, consumers, shared eventstream decoding and existing utilities were traced; no material integration or reuse defect found. |
Limitations
No live Kiro request or authoritative upstream wire schema was available. In particular, repository evidence does not establish whether ID-less tool fragments can overlap multiple active tool IDs; that conditional case is not reported as a defect. Seven host/platform or opt-in checks were skipped, while all relevant changed-test checks passed.
|
Merged into dev.
— |
3 participants
Closes #6164.
X-Amz-Target: AmazonCodeWhispererStreamingService.GenerateAssistantResponse,Content-Type: application/x-amz-json-1.0; dropx-amzn-codewhisperer-proflearn.userInputMessageContext.toolssent as a flat array.assistantResponseEvent/toolUseEvent/messageMetadataEventpayloads; accumulatetoolUseEvent.inputfragments pertoolUseIdand emittoolcall_endonstop.Tests: new
test/kiro-codewhisperer-wire-protocol.test.ts(7 cases) — 6 fail on origin/main, all pass here; kiro + aws-region suites 110/110 pass.Credit: nomo (community report with curl verification). Live Kiro verification was not possible from this host.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]