fix(dashboard): stop refusal recovery re-answering an answered turn

[iamwhatever]

Problem / Motivation

When a tool call is blocked by a Kiro Crew safety policy, the chat runner queues a
[Tool refusal — automatic recovery] continuation so the model learns the real
reason — the backend's own tool result for a rejected permission is the fixed
string User denied tool execution, and on a backend without mid-turn steer
(ACP_BACKENDS_STEER omits the claude backend, so supports_steer is False and
notices_sent is always 0) that continuation is the only channel for it.

The body of that continuation asserts the block "ended the turn early" and closes
with "continue the task where you left off". Both are false for a turn that went on
to answer anyway — the model routed around the block, or already had enough
evidence without the blocked call. Acting on the instruction makes it re-answer a
question the user has already read.

Observed in a live session: three separate blocked calls in one investigation
produced three recovery turns, and the user saw the same verdict written out three
times.

Why it matters

Every duplicate is a full billed turn, and the count scales with the number of
blocked calls in the turn rather than with the number of open questions. The
user-visible damage is worse than the cost: an answer restated three times reads as
the agent losing track of the conversation, and it buries the one thing the recovery
turn exists to deliver — the block reason.

What changed (motivation → approach → change)

Symptom: a turn that answered despite a policy block answers again, once per
blocked call.

Root cause: the recovery body has exactly one wording, and its premise ("the
turn ended early") is true in only one of the two cases that reach it. The decision
to queue was right; the text was wrong about what had happened.

Change: build_refusal_recovery_prompt() in
src/kiro_crew/dashboard/state.py gains an answered flag. When it is set, the
blocked-call list and the whole per-class remediation_for guidance block still go
out unchanged, but the framing becomes awareness rather than resumption, and the
closing instruction becomes an explicit prohibition on restating what was already
sent. The unanswered wording is reproduced byte-for-byte, so no existing recovery is
altered.

The flag means text was sent, never the task is done, and the body is worded to
match: continuing from there is the default, and the one-line-and-stop case is
conditioned explicitly on the task being finished. That distinction is load-bearing
because there is no way to tell a delivered answer from a one-line preamble ("Let me
check the logs.") before the blocked call — both are plain prose flushed at the same
point in the stream — so a body that asserted a finished answer would tell a turn
that had only narrated its intent to stop with the work undone.

The single call site in src/kiro_crew/dashboard/chat_runner.py passes
answered=bool(_answer_text.strip()) or _produced_visible_output or _turn_flushed_visible_text. _answer_text is the turn's own answer with backend
control notices removed; _produced_visible_output covers the paths that reset
assistant_text after emitting it without a tool boundary (steer cut, compaction,
clear, agent switch); _turn_flushed_visible_text is new and covers the three
tool-boundary flushes (post-tool-group text, EVENT_TOOL_CALL, the permission flow),
which persist the segment and reset the buffer — so answer → blocked call → turn
end would otherwise reach the predicate with an empty assistant_text and be scored
unanswered. It is a separate flag rather than a widening of _produced_visible_output
because that flag's narrow meaning is what the promise-only guard below it depends on.

Two things deliberately NOT done, because the existing design rules them out:

• No turn cap. There has never been one on this path (git log -S for
  refusal_recovery_depth, max_refusal, recovery_depth finds nothing), and the
  absence is a choice rather than an omission: should_queue_refusal_recovery's
  docstring records the asymmetry it was written for — skipping wrongly leaves the
  model with User denied tool execution and no correction, while queueing wrongly
  costs one turn the model would otherwise have been told twice. A cap is the
  skipping direction, and it bites hardest exactly when the model is most confused
  (after several blocks in a row). The sibling Stop-hook path does have
  agent.max_stop_hook_nudges, for a reason that does not transfer: a hook is an
  external program that can return block unconditionally forever with zero model
  agency, whereas a refusal chain requires the model to freely choose a fresh
  blocked call each turn.
• No suppression of the recovery turn. Dropping it would take the block reason
  with it, leaving the model's last word on the subject as User denied tool execution — which is how it starts attributing policy blocks to the user in later
  turns. Awareness-without-redo delivers the reason at the same one-turn cost the
  design already accepted.

No frontend change is needed: RecoveryCard.tsx dispatches on the prefix and counts
- bullets, both unchanged, and the default body is byte-identical, so the
capture-recovery-card.mjs and RecoveryCard.test.tsx fixtures still match.

Tests

test/test_dashboard_approval.py:

• test_answered_turn_gets_awareness_body_not_a_continuation — the reason is still
  present, "ended the turn early" and "continue the task where you left off" are
  gone, the awareness framing and "Do NOT repeat" are present.
• test_unanswered_turn_keeps_the_continuation_body — pins the default wording so a
  later edit cannot collapse the two modes into one.
• test_answered_keeps_per_class_remediation — equal "How to do this properly:"
  counts in both modes, so the awareness path cannot quietly drop the remediation
  guidance that is the reason the turn is dispatched at all.
• test_deny_after_an_answer_injects_awareness_not_a_redo — drives the real
  _run_chat with a ToolHookResult.deny(...) and a first stream that emits text,
  then asserts the injected recovery row carries the reason and the prohibition but
  not the redo instruction. This is the one that proves answered=True is reached on
  the production path, not only in the builder.
• test_answer_before_the_block_also_gets_awareness_not_a_redo — the ordering the
  predicate previously missed: text streamed before the blocked call, so the
  tool-boundary flush has emptied assistant_text by end of turn. Reverting
  or _turn_flushed_visible_text fails it with
  assert 'continue th...you left off' not in ....

test/test_deny_guidance.py:

• test_answered_body_never_tells_an_unfinished_turn_to_stop — locks the
  sent-vs-finished distinction into the body itself: the awareness branch keeps
  "Do NOT repeat", still avoids the resume instruction, and must make
  continue-from-there the default with the stop case explicitly conditioned on the
  task being finished. Without it, a turn that only narrated its intent before the
  block could be told to stop with the work undone.

Manual verification

N/A — unit coverage sufficient: the last test drives the real _run_chat deny path
end to end, which is the integration this change lives on.

Local pre-push review, stated so the server lanes are read with it in mind: the
opus lane ran on its pinned claude-opus-4.8 against the extracted
claude-review.yml contract and returned no findings. The gpt lane did not
run. This gateway rejects gpt-5.6-sol, gpt-5.6-terra and gpt-5.6-luna with
Invalid value for config option model, and grok-4.6 too, so no model in or near
the gpt-5.x tier was reachable; the retry on claude-opus-4.8 was stopped before
it finished. The server-side codex-review lane is therefore the first GPT pass on
this diff — treat its verdict as unpreviewed rather than as a re-run of a local
green.

Related Issues

no linked issue: found while triaging a live session, not tracked.

Pattern harvest

Rule candidate: review-prompt
Pattern: an automatic continuation whose text asserts a precondition ("the turn
ended early", "you were interrupted") that the code queuing it never checked — the
decision to queue and the claim about why it was queued need the same evidence.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 59631661fd6328ff12883b5a29a6bf5eebc40b04 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 5963166

Verdict parsed from the review's SHA-scoped output markers for commit 59631661fd6328ff12883b5a29a6bf5eebc40b04.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 59631661fd6328ff12883b5a29a6bf5eebc40b04: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 59631661fd6328ff12883b5a29a6bf5eebc40b04 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks done. The final verification confirmed: _answer_text is derived from the end-of-turn buffer only (chat_runner.py:9530), so the new flag isn't a second spelling; the other consumers of the visible-output pair are gated on not _refusal_reasons / zero tool calls, so no unfixed siblings; and the default wording concatenates byte-identically to the removed lines.

First-Principles-Verdict: PASS

A reported live defect — the agent restating its answer once per blocked call — fixed at the wrong-premise text that caused it, nothing riding along.

What this change ships

Intent: stop the automatic post-block recovery turn from making the agent re-answer a question it already answered. This is a FIX.

1. A turn that answered despite a policy block now gets "carry on, don't restate" instead of "continue where you left off" — justified (reported defect, harm is duplicate billed turns).
2. A turn that never answered keeps the old recovery wording byte-for-byte — justified (verified identical).
3. The answer-then-blocked-call ordering is now detected too, via a new internal flushed-text flag — justified; not a second spelling (_answer_text derives only from the end-of-turn buffer, chat_runner.py:9530; _produced_visible_output's narrow meaning is load-bearing at chat_runner.py:9869–9878).
4. New answered= boolean on build_refusal_recovery_prompt — one non-test consumer (chat_runner.py:10314), already the singular form, so not premature generalization.
5. Six tests pinning both wordings, both orderings, and remediation parity — justified.

Counts run: build_refusal_recovery_prompt has 1 production caller; of the 9 assistant_text = "" sites, the three tool-boundary flushes are flagged and the rest already set _produced_visible_output; the pair's other consumers (lines 9696, 9878, 9986) are gated on not _refusal_reasons or zero tool calls, so the flush blind spot had exactly one victim — this one. Suppressing the turn instead is ruled out by a named constraint (no-steer backends have no other channel for the block reason), so the fix sits at cause level.

[FIRST-PRINCIPLES-REVIEWED] 5963166

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 59631661fd6328ff12883b5a29a6bf5eebc40b04 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The design is coherent: the steer-delivered case already skips recovery entirely (should_queue_refusal_recovery returns False when all refusals got confirmed notices), so the awareness body only runs where the recovery turn is the sole delivery channel for the block reason. The predicate's known ambiguity (preamble vs. answer) is handled in the body wording rather than papered over, suppression and caps are argued against with reasons grounded in the existing design, the new flag is deliberately kept separate from _produced_visible_output to protect the promise-only guard, and the change is backward-compatible (keyword arg, default body byte-identical, frontend contract untouched). Description and diff match bidirectionally; tests pin both wordings and the production path.

Design-Verdict: PASS

Targets a real observed harm at the actual defect — the recovery text's false premise — while preserving the reason-delivery guarantee the turn exists for.

[DESIGN-REVIEWED] 5963166

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 59631661fd6328ff12883b5a29a6bf5eebc40b04 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 5963166

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 59631661fd6328ff12883b5a29a6bf5eebc40b04: <one-sentence reason>

[iamwhatever]

Disposition: fixed — 59631661f

• The answered predicate misses answers flushed at tool/permission boundaries — span=8023cfe9506b (src/kiro_crew/dashboard/chat_runner.py:10296)

"bool(_answer_text.strip()) or _produced_visible_output" misses answers flushed at tool/permission boundaries, causing duplicate-answer recovery
Fix: include current-turn persisted assistant segments in the predicate.

The finding holds, and it is this PR's own bug on the mirror-image ordering. Three
sites flush the pre-tool text and reset the buffer — post-tool-group text,
EVENT_TOOL_CALL, and the permission flow — each calling _flush_segment(...) and
then assistant_text = "". So answer → blocked tool call → turn end reaches the
predicate with an empty assistant_text and _produced_visible_output still
False, and was scored unanswered: precisely the duplicate-answer recovery this PR
exists to stop, arriving through the ordering the first cut did not cover.

What changed. A third turn-level flag, _turn_flushed_visible_text, set at those
three flush sites and OR'd into the predicate. It is a new flag rather than a
widening of _produced_visible_output deliberately: that flag's narrow meaning —
only the paths that reset the buffer without a tool boundary (steer cut,
compaction, clear, agent switch) — is load-bearing for the promise-only guard
directly below it, which an earlier round pinned with its own comment. Widening it
would have silently changed that guard too.

Pinned by test_answer_before_the_block_also_gets_awareness_not_a_redo, which drives
the real _run_chat with text streamed before the denied call. Red-proofed:
reverting just or _turn_flushed_visible_text fails exactly that test with
assert 'continue th...you left off' not in ..., so the test holds the predicate and
not merely the wording.

One consequence the fix surfaced, closed in the same commit: a flushed segment proves
text was sent, never that the task is done — narration ("Let me check the
logs.") and a delivered answer are indistinguishable prose flushed at the same point
in the stream, so no predicate can separate them. The awareness body is therefore
worded so that continuing from there is the default and the one-line-and-stop case is
conditioned explicitly on the task being finished, instead of asserting a finished
answer. test_answered_body_never_tells_an_unfinished_turn_to_stop locks that in.

[chenmingwei23]

Approved: PR Readiness green on this head, all check-runs pass on best-per-name.

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #8288 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8288: KEEP. Adjacent, non-conflicting edits to the same recovery chain with different triggers and different fixes. Worth reviewing together only because both reason about whether a turn produced a real answer, which is the shared invariant a reviewer should check once. Files: src/kiro_crew/dashboard/chat_runner.py.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.