fix(kiro): explain empty exec output and stop reopening a delivered final answer

devlog/_plan/260828_kiro_turn_termination/000_research.md

@@ -0,0 +1,63 @@
+# Kiro turn termination — residual defect research
+
+Observed 2026-08-28 21:20 KST by the user in a Codex desktop session routed
+`kiro/claude-opus-5` through the local proxy on port 10100.
+
+## Symptom
+
+1. A plain question ("근데 코드 모드가 뭐임") produced the final answer TWICE in
+   one turn, the second a near-duplicate rewrite of the first.
+2. The user reports the "answer finishes, then continues like a goal" loop is
+   still present after `cf1a5720c`.
+
+## Live-state evidence
+
+- Listener PID 3653, started 2026-08-28 21:17:50, running the checkout at
+  `/Users/jun/Developer/new/700_projects/opencodex/src/cli/index.ts start --port 10100`.
+- `cf1a5720c` committed 21:09:50 — the running process DID load that fix.
+  Confirmed independently: a probe that produced no stdout in this session
+  returned the new empty-exec wording added by that commit.
+- HEAD advanced to `60537f067` at 21:26:53 (a different session's commit), so
+  the running proxy is stale relative to HEAD but not relative to `cf1a5720c`.
+
+So the residual behaviour is a real defect, not a stale process.
+
+## Mechanism 1 — duplicate final answer (rendering)
+
+Kiro emits answer-like ordinary text, then calls the private completion tool in
+the SAME inference. The adapter releases the prose as `phase: "commentary"` and
+the completion `answer` as `phase: "final_answer"`. `src/bridge.ts` closes the
+commentary message on the phase change and opens a new assistant message, so the
+client renders two assistant messages whose text is nearly identical.
+
+This is pinned by the existing suite, so it is verified behaviour rather than a
+hypothesis — `tests/kiro-stream.test.ts` asserts exactly:
+
+```
+{ type: "text_delta", text: "Done.", phase: "commentary" },
+{ type: "text_delta", text: "Done.", phase: "final_answer" },
+```
+
+## Mechanism 2 — non-terminating turn (upstream fetch)
+
+When replayed history ends in a delivered final answer, `buildKiroPayload` still
+appends a synthetic trailing user turn carrying `KIRO_ANSWER_DELIVERED_MESSAGE`
+and performs a real upstream inference. Neutral wording removes the instruction
+to resume but does not remove the prompt: the model is asked again and answers
+again. `60537f067` additionally suppresses the completion contract for that
+shape, which narrows the loop, but there is still no terminal boundary that
+avoids the fetch.
+
+## Hypotheses tested
+
+| id | claim | verdict | evidence |
+|----|-------|---------|----------|
+| H1 | the completion TOOL CALL never sets the phase flag | refuted | the proxy consumes the completion tool; a replayed history containing it throws at `src/adapters/kiro-wire.ts:88` (reproduced directly) |
+| H2 | any synthetic trailing user turn re-invokes the model | confirmed | `src/adapters/kiro.ts` trailing-turn append still yields an upstream inference |
+| H3 | another layer replays the answer | partial | the generic Responses guard is not involved; the Kiro-owned bounded completion fallback does perform a second fetch |
+| H4 | commentary is the last recorded component | confirmed | ordinary text is forced to commentary; the completion answer is a separate final message, split at `src/bridge.ts:922` |
+
+## Verification baseline
+
+`bun test tests/kiro-adapter.test.ts tests/kiro-stream.test.ts tests/server-kiro-completion-e2e.test.ts`
+-> 180 pass, 0 fail at `60537f067`.

devlog/_plan/260828_kiro_turn_termination/010_wp1_terminal_boundary.md

@@ -0,0 +1,81 @@
+# wp1 — terminal boundary for a delivered final answer
+
+Consumes: `000_research.md` mechanism 2.
+
+## Problem
+
+`buildKiroPayload` (`src/adapters/kiro.ts`) turns a trailing delivered final
+answer into a synthetic user turn carrying `KIRO_ANSWER_DELIVERED_MESSAGE` and
+then performs a real upstream inference. Neutral wording is still a prompt, so
+the model answers again — the closed task reads as an open goal.
+
+`60537f067` suppresses the completion contract for that shape. Keep that as
+defence in depth; it is not the boundary.
+
+## Change (revised after the A-phase audit — audit verdict was FAIL on the
+## original placement, see 011_audit_round1.md)
+
+Short-circuit BEFORE the provider fetch, but NOT inside `buildRequest` and NOT
+as a bare outputless `done`. Three constraints the audit established, each
+verified against current source:
+
+1. **An outputless `done` is retried, not accepted.** `guardEmptyCompletionEventStream`
+   treats a `done` with no content event as an empty completion, suppresses the
+   terminal, and re-invokes the identical turn; a second empty terminal becomes
+   `empty_completion_retry_failed`
+   (`src/server/responses/empty-completion-guard.ts:246-270`). So the naive
+   terminal turns one loop into either another inference or a stated error.
+   The local terminal must therefore bypass the empty-completion guard as well as
+   the transport.
+2. **`buildRequest` cannot emit events.** The adapter contract returns an
+   `AdapterRequest`; events only exist once a `Response` reaches `parseStream`
+   (`src/adapters/base.ts`). The server then records and sends the attempt
+   unconditionally. Manufacturing a fake `Response` inside `fetchResponse` is
+   also wrong: it records a physical send and still meets the guard.
+3. **A phantom estimate must not be logged.** Kiro attaches an estimated input
+   count during build and the server notes the attempt send before fetching, so
+   short-circuiting after a build would log a request that never happened.
+
+Placement: an explicit adapter-owned local-terminal decision consulted in
+`handleResponsesInner` AFTER adapter resolution and BEFORE the ordinary
+build/send path, short-circuiting to a locally constructed terminal response.
+Reuse `hasTrailingDeliveredFinalAnswer` as the predicate; do not introduce a
+second notion of "delivered". The hook must not intercept the adapter-owned
+bounded retry, which builds with a forced `text_fallback` mode.
+
+Usage accounting for the local terminal: no build-time estimate, `sendCount`
+zero, response usage explicitly zero for input/output/total, and no estimated
+usage in the request log.
+
+## Out of scope
+
+- Any change to how the completion tool is parsed or consumed.
+- Any change to the empty-exec normalisation from `cf1a5720c` / `60537f067`.
+- The duplicate-rendering defect, which is wp2.
+
+## Criteria
+
+1. Replaying a delivered final answer issues ZERO upstream requests and yields a
+   completed turn with `endTurn: true`.
+2. Criterion 1 holds with `emptyCompletionRetry` BOTH enabled and disabled, for
+   streaming and non-streaming Responses. This is the criterion the audit added;
+   without it the fix passes a test and still loops in the user's config.
+3. The short-circuited turn logs `sendCount === 0` and no estimated usage.
+4. A genuine later user message after a delivered final answer still performs a
+   normal inference (control).
+5. An unfinished trailing assistant turn still gets the continuation prompt and
+   still performs an inference (control, already covered — must stay green).
+6. The adapter-owned bounded `text_fallback` retry is NOT intercepted.
+7. `60537f067`'s completion-mode suppression remains asserted.
+
+## Completion language
+
+Closing wp1 fixes the repeated inference ONLY. The user-visible duplicate answer
+remains until wp2 lands, and the wp1 report must say so rather than implying the
+reported symptom is fully resolved.
+
+## Evidence
+
+`bun test tests/kiro-adapter.test.ts tests/kiro-stream.test.ts tests/server-kiro-completion-e2e.test.ts`
+plus new public-server coverage in `tests/server-kiro-completion-e2e.test.ts`,
+each new assertion driven red once by reverting the change.

devlog/_plan/260828_kiro_turn_termination/011_audit_round1.md

@@ -0,0 +1,25 @@
+# A-phase audit round 1 — verdict FAIL
+
+Reviewer: independent subagent (gpt-5.6-sol, medium effort), read-only lane.
+Audited: `000_research.md`, `010_wp1_terminal_boundary.md` as first written.
+Reviewer's own checks: 180 pass / 0 fail on the three Kiro suites,
+`bun x tsc --noEmit` exit 0, live proxy untouched.
+
+The plan was rewritten rather than argued with. Findings and dispositions:
+
+| # | Finding | Disposition |
+|---|---------|-------------|
+| 1 | An outputless `done` is consumed by the empty-completion guard, which suppresses the terminal and re-invokes the identical turn (`src/server/responses/empty-completion-guard.ts:246-270`). The "safe terminal" becomes another inference or `empty_completion_retry_failed`. | ACCEPTED. Verified independently by reading the guard. wp1 now requires bypassing the guard as well as the transport, and adds a criterion covering `emptyCompletionRetry` both ON and OFF. |
+| 2 | `buildRequest` cannot emit events under the adapter contract, and faking a `Response` in `fetchResponse` still records a physical send. | ACCEPTED. Placement moved to an adapter-owned local-terminal decision consulted in `handleResponsesInner` before the build/send path. |
+| 3 | Short-circuiting after a build logs a phantom estimated request. | ACCEPTED. Criteria now demand no build-time estimate, `sendCount === 0`, zero response usage, and no estimated usage in the request log. |
+| 4 | `hasTrailingDeliveredFinalAnswer` is sound, but the hook must not intercept the forced `text_fallback` build. | ACCEPTED as a criterion. The predicate was re-read directly: role-and-phase based, so a user message merely QUOTING the acknowledgement is unaffected. |
+| 5 | wp1/wp2 separation is legitimate, but wp1's completion language must not imply the reported symptom is fully fixed. | ACCEPTED. wp1 now carries an explicit completion-language section. |
+| 6 | The broad "buffer commentary" direction is wrong; required-mode commentary is ALREADY deferred, and the real defect is the unconditional flush before the validated answer. Retaining across the bounded fallback would hide progress during a long second inference. | ACCEPTED, and it improves the design: wp2 is now a change to WHEN the existing deferred run is released, not a new buffer. |
+| 7 | No user-visible Responses-level regression was specified; adapter-event coverage cannot prove the rendered duplicate is gone. | ACCEPTED. Both work-phases now name `tests/server-kiro-completion-e2e.test.ts`. |
+
+Process note the reviewer raised: it expected a staged diff and found none, and
+observed HEAD had moved to `761cb4cfe`. Correct on both counts — the plan unit is
+untracked while in P, and two unrelated commits landed from another session
+during the audit. Neither invalidates the substance, and the untracked worktree
+changes in `src/adapters/cursor/` at dispatch time belonged to that other session
+and were left alone.

devlog/_plan/260828_kiro_turn_termination/012_audit_round2.md

@@ -0,0 +1,55 @@
+# A-phase audit round 2 — verdict FAIL on wp2, wp1 cleared
+
+Same reviewer as round 1 (blocker-closure reuse). Round 1's seven findings were
+all accepted; this round re-read the revised plan.
+
+## wp1 — cleared
+
+The reviewer confirms the revised placement, criteria, and completion language
+are sufficient: bypassing transport, request building, and the empty-completion
+guard, with guard-on/guard-off and streaming/non-streaming coverage, zero sends,
+no estimated usage, and the forced `text_fallback` exclusion.
+
+It also rejected `runTurn` as the seam, with specifics worth keeping: adopting
+`runTurn` routes EVERY Kiro request into the custom transport branch, which waits
+on provider pacing and increments `sendCount` before any local decision, still
+meets the empty-completion guard afterwards, and would force Kiro to re-own
+transport, retries, failover, cancellation, and accounting that its existing
+`buildRequest`/`fetchResponse`/`parseStream` path already provides. The local
+terminal therefore belongs immediately after adapter resolution and before
+`buildRequest`.
+
+## wp2 — two execution-path blockers, both accepted
+
+1. **The outer drain.** Skipping the inner flush at `src/adapters/kiro.ts:1467`
+   is not enough: `parseKiroAttempt` independently drains `deferred` at
+   `src/adapters/kiro.ts:996-999` after the inner generator returns. The final
+   answer would be emitted first and the commentary after it — the duplicate
+   survives, reversed. Found independently while reading the same file, so this
+   is confirmed twice. The deferred collection must be consumed, and the claim
+   that this stays clear of retention machinery is withdrawn.
+2. **`text_fallback` has the same shape through a different collection.** It
+   retains in `fallbackEvents`, and `src/adapters/kiro.ts:1470-1477` emits all of
+   them and then the completion answer. The rule must apply independently inside
+   each inference.
+
+Criterion 3 was also overbroad — "any turn whose completion never arrives enters
+the fallback exactly once" is false for real tools, provider/protocol failures,
+and explicit stops like `MAX_TOKENS`. Narrowed to a clean required-mode
+inference, with added controls for failure, explicit incomplete stop,
+text_fallback duplication, and budget return-to-baseline.
+
+The six release paths the reviewer enumerated from source are now a table in
+`020_wp2_duplicate_answer.md` and are treated as controls.
+
+## HEAD movement
+
+Verified: `60537f067..761cb4cfe` touches only
+`src/adapters/cursor/tool-result-normalize.ts` and
+`tests/cursor-exec-empty-result.test.ts`. No Kiro adapter, adapter contract,
+Responses core, bridge, or Kiro test file changed. The plan is unaffected.
+
+## Disposition
+
+wp1 proceeds to implementation. wp2's plan page is corrected here and will be
+re-audited as part of its own cycle rather than blocking wp1.

devlog/_plan/260828_kiro_turn_termination/020_wp2_duplicate_answer.md

@@ -0,0 +1,124 @@
+# wp2 — one visible answer per turn
+
+Consumes: `000_research.md` mechanism 1.
+
+## Problem
+
+Kiro emits answer-like ordinary text and then calls the private completion tool
+in the SAME inference. The adapter releases the prose as `phase: "commentary"`
+and the completion `answer` as `phase: "final_answer"`;
+`src/bridge.ts` closes the commentary message on the phase change and opens a
+new assistant message. The client renders two assistant messages whose text is
+nearly identical. This is what the user saw.
+
+The existing suite pins this pair, so the fix necessarily REPLACES an asserted
+expectation rather than adding to it:
+
+```
+tests/kiro-stream.test.ts
+{ type: "text_delta", text: "Done.", phase: "commentary" },
+{ type: "text_delta", text: "Done.", phase: "final_answer" },
+```
+
+## Constraint that shapes the design
+
+Progress prose is load-bearing UX: a long tool-using turn streams commentary so
+the user is not left staring at nothing. Withholding ALL commentary until the
+turn resolves would trade a cosmetic duplicate for a silent turn, which the
+repository's own comments call out (`#520` gates exist precisely to avoid
+re-emitting or losing flushed progress).
+
+So the buffering must be NARROW: hold back only the trailing commentary run that
+has not yet been followed by a real tool call, and only while the completion tool
+is still capable of arriving. Release it unchanged the moment a real tool starts,
+the stream ends without a completion answer, or the bounded fallback engages.
+
+## Options
+
+1. **Consume the same inference's retained text on a valid completion.** (CHOSEN —
+   narrowed in audit round 1, corrected in round 2.) No new buffer is needed:
+   required-mode commentary is ALREADY deferred. But skipping the inner flush is
+   NOT sufficient, and this is the correction that matters:
+
+   - The inner flush is at `src/adapters/kiro.ts:1467`.
+   - `parseKiroAttempt` INDEPENDENTLY drains whatever remains in `deferred` at
+     `src/adapters/kiro.ts:996-999`, after the inner generator returns.
+
+   So merely skipping the inner flush emits the final answer first and then the
+   commentary from the outer drain — the duplicate survives, in reversed order.
+   The deferred collection must be CONSUMED on a valid completion: discard and
+   release the redundant `text_delta` events, preserve and release every
+   non-text event, and leave nothing for the outer drain. That necessarily
+   touches retention ownership, so the earlier claim that this change stays
+   clear of the retention machinery is withdrawn.
+
+   `text_fallback` has the SAME shape through a DIFFERENT collection: it retains
+   in `fallbackEvents`, and `src/adapters/kiro.ts:1470-1477` emits all of them
+   and then the completion answer. The rule must therefore apply independently
+   inside EACH inference:
+
+   - required inference + valid completion -> suppress its deferred text, keep
+     non-text events, emit the completion answer;
+   - text_fallback inference + valid completion -> suppress that inference's
+     retained text, keep non-text events, emit the completion answer;
+   - never retain one inference's progress across the next inference.
+2. **Retain commentary ACROSS the bounded fallback.** Rejected by the audit: the
+   second inference can be long, so withholding first-attempt progress across it
+   would make the turn look dead and contradicts the deliberate "first attempt
+   already flushed" gate.
+3. **Suppress only on redundancy.** Emit commentary live, and skip the completion
+   answer if it is substantially the same text. Rejected: "substantially the same"
+   is a similarity heuristic, and a wrong guess either drops the real answer or
+   keeps the duplicate.
+4. **Bridge-side coalesce.** Merge a commentary message and an immediately
+   following final answer into one assistant message. Rejected as the primary
+   seam: the phase distinction is deliberate protocol information, and the same
+   split is correct when the commentary genuinely preceded tool work.
+
+Because the deferral already exists, this is a change to WHEN the deferred run is
+released, not a new retention mechanism — which also keeps it clear of the
+retention/budget machinery where duplication bugs have previously lived.
+
+## Criteria
+
+1. A single inference emitting answer-like prose plus a completion answer yields
+   exactly ONE visible answer to the client.
+2. Commentary followed by a REAL tool call is still emitted live and in order.
+3. A CLEAN required-mode inference — text or reasoning present, no real tool, no
+   completion answer, no explicit non-completion stop reason — still shows its
+   progress prose and enters the bounded fallback exactly once. (Narrowed in
+   audit round 2: the unqualified form was false for real tools, provider and
+   protocol failures, and explicit stops such as `MAX_TOKENS` or
+   `CONTENT_FILTERED`.)
+4. A Responses-protocol-level assertion, not only adapter events: the user-visible
+   duplicate must be proven gone through the bridge. Adapter-event coverage cannot
+   prove this, because the split happens in the bridge on the phase change. The
+   assertion belongs in `tests/server-kiro-completion-e2e.test.ts`: one upstream
+   request, exactly one visible assistant answer, one terminal completion, and the
+   near-duplicate prose absent.
+5. The existing `tests/kiro-stream.test.ts` expectation that asserts the
+   commentary/final pair is UPDATED, not deleted — the replacement states the new
+   contract for the same scenario.
+6. `text_fallback` ordinary text plus a valid completion also yields exactly one
+   visible answer.
+7. The translator budget returns to baseline after suppressed events — suppression
+   must release retention, not leak it.
+
+## Release paths that must remain intact
+
+Enumerated from source in audit round 2; each is a control the implementation may
+not regress:
+
+| trigger | release point |
+|---------|---------------|
+| a real tool starts (`sawRealTool`) | `src/adapters/kiro.ts:1172-1177`, released with the tool event |
+| clean no-completion turn needing fallback | flush before `needsFallback` returns, `src/adapters/kiro.ts:1535-1542` / `1600-1603` |
+| stream / protocol / provider failure | outer failure drain, `src/adapters/kiro.ts:996-999` |
+| explicit non-completion stop | released before the incomplete/error branches, `src/adapters/kiro.ts:1545-1598` |
+| plain-text fallback without completion | retained text promoted to `final_answer`, `src/adapters/kiro.ts:1488-1501` |
+| empty / reasoning-only fallback | diagnostics released before the structured incomplete, `src/adapters/kiro.ts:1503-1517` |
+
+## Out of scope
+
+- The terminal-boundary fix (wp1).
+- Changing what `phase` means in the Responses protocol.