Skip to content

fix(chat): recover synthetic backend retry turns - #8288

Open
Pearcekieser wants to merge 1 commit into
kirodotdev:mainfrom
Pearcekieser:fix/generic-retry-recovery-20260903
Open

fix(chat): recover synthetic backend retry turns#8288
Pearcekieser wants to merge 1 commit into
kirodotdev:mainfrom
Pearcekieser:fix/generic-retry-recovery-20260903

Conversation

@Pearcekieser

@Pearcekieser Pearcekieser commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Problem / Motivation

Kiro CLI can fail its inner agent loop after several successful tool cycles yet close ACP as a normal Ok / UserTurnEnd result whose only assistant text is:

I hit an issue while processing your request. Please retry.

Kiro Crew treated that provider-generated sentence as a real answer. It persisted the text, recorded success, reset recovery budgets, and required the user to repeat a request whose tools may already have produced side effects.

Why it matters

Long-running work can stop after minutes of successful execution with no actionable diagnosis. Blindly replaying the original request is unsafe because completed tools may run twice. The recovery must continue from the live session context, preserve tool results, and remain cancellable before dispatch.

What changed (motivation → approach → change)

  • Added an exact classifier for Kiro CLI's synthetic turn-failure sentence. Broad please retry matching is deliberately avoided.
  • Require positive is_kiro_backend identity, at least one tool call in the turn, and confirmation that the current user message did not itself contain the sentence before treating it as provider control output. Exact answers without tool activity, quoted/requested literals, and non-Kiro output remain ordinary assistant text.
  • Remove the synthetic sentence's trailing chunks, replace any already-streamed browser row with an empty assistant reconciliation, and then show one transcript-visible recovery notice; the terminal authoritative refresh removes the unpersisted blank row.
  • Scope file-change snapshot attachment to the current turn boundary, so a sentinel/error turn with completed writes creates its own established synthetic assistant anchor instead of mutating the previous turn's assistant row.
  • Queue the existing post-token continuation on the same live session, preserving completed tool results and explicitly forbidding replay.
  • Share the existing post-token one-shot so a repeated sentinel terminates instead of looping.
  • Extend dispatch-time intervention cancellation to post-token continuations. A late Stop, queued user follow-up, or steer removes the continuation and restores its one-shot budget before anything can run.
  • Recheck monotonic Stop generation, queued follow-ups, and pending steers after the post-token retry backoff and before consuming the one-shot, so an intervention that begins and resolves during the await cannot be forgotten.
  • Keep sentinel turns out of success recording, consolidation, landed-turn state, and retry-budget resets.
  • Document the recovery contract in the session specification.

Tests

  • Exact Kiro sentinel after tool activity queues one continuation and records no success; reported output-token counts do not override the behavioral evidence because Kiro's credit-only accounting also reports zero for legitimate output.
  • A repeated sentinel is terminal and cannot loop.
  • Exact Kiro text without tool activity remains a normal assistant response.
  • Non-Kiro exact text remains a normal assistant response.
  • Broader prose and quoted forms do not match.
  • A Stop that begins and resolves between enqueue and dispatch purges the continuation and restores the one-shot.
  • Full TestEmptyResponseRetry: 19 passed.
  • Full TestRunChatTransientRetry: 20 passed.
  • File-change, flush, and post-compaction contract suites: 113 passed.
  • Full test_dashboard_chat.py: 759 passed on the final rebased head.
  • Cross-surface frontend suite: 5,682 passed.
  • Electron suite under CI's Node 24 runtime: 1,587 passed, 1 skipped.
  • Production build, TypeScript, ESLint, i18n check/render, phantom classes, duplication, bundle-size analysis, Black, isort, flake8, mypy, docs, harness, policy, vendor, scrub, and CloudFormation gates passed.

The local full-backend profile run reached 84,065 passes. Its remaining failures are unrelated host-topology failures caused by the dashboard sandbox exposing the live KIROCREW_HOME, /local/home ownership, and overlong AF_UNIX scratch paths. The two failures related to this diff's answer-branch source contract were fixed and rerun green. Hosted CI provides the authoritative isolated full-backend result.

Two independent local reviews found missing dispatch-time cancellation and missing positive Kiro identity; both were fixed. A GPT round then showed that output-token counts are non-authoritative for Kiro's credit-only accounting, so that discriminator was removed in favor of backend identity, tool activity, exact-text, and current-request guards. Later GPT rounds identified the live-browser reconciliation gap and then exposed a broader cross-turn file-change attachment bug. The former now replaces an already-streamed sentinel row before recovery; the latter was restructured globally so file changes scan only assistant rows created after the turn boundary and synthesize a current-turn anchor when needed. On the final current head, GPT restated the inherent text-signal ambiguity as a security blocker, but Opus adjudication downgraded it as disproportionate: the continuation is visible, same-session, one-shot, admission-restamped, and subject to the same tool gates, while Kiro CLI exposes no out-of-band failure event to consume.

Manual verification

N/A — this is backend recovery control flow with deterministic event-stream and queue-dispatch coverage. The triggering upstream failure cannot be induced on demand; the test event shape comes from three observed raw Kiro CLI records.

Related Issues

Fixes #8285

Related but independent:

Pattern harvest

Rule candidate: review-prompt
Pattern: a provider-owned synthetic failure sentence returned under a success envelope must require positive provider identity plus out-of-band failure evidence before changing control flow.

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

CI retry note

The first two dependency-audit attempts for current-head validation timed out after 120 seconds while waiting for npm audit; neither produced a vulnerability finding. This description update retriggers the repository's pull_request: edited review/audit path without changing the reviewed commit.

@Pearcekieser
Pearcekieser requested a review from a team as a code owner September 3, 2026 22:19
@Pearcekieser
Pearcekieser requested a review from cixuuz September 3, 2026 22:19
@github-actions github-actions Bot added readiness: checking Automated validation is still running fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 3, 2026
@Pearcekieser
Pearcekieser force-pushed the fix/generic-retry-recovery-20260903 branch from 57ddd15 to 8f697aa Compare September 4, 2026 00:14
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 07ff84185956a8af5e85589638d800033287f404 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound downstream mitigation, but it hard-couples control flow to an unversioned English sentence owned by kiro-cli — a silent-decay dependency with no upstream fix tracked.

Watch

  • The classifier keys on the exact prose "I hit an issue while processing your request. Please retry." ("Keep the match exact"). Any kiro-cli wording change silently disables the recovery — degradation is back to today's behavior, but no test or log can detect the decay because the string is copied into the tests. The durable fix is an upstream structured failure signal on ACP (this success-envelope prose is a kiro-cli defect); track that so this classifier can be retired, and note the coupled version range in the spec.
  • The recovery-eligibility predicate (_prompt_depth == 0 … not _pending_steers) is now hand-copied at three sites, and the purgeable-continuation tuple is a fourth hand-maintained enumeration — the exact drift this PR itself fixed twice (dispatch-time purge missed _POSTTOKEN_RECOVER_MSG; post-backoff recheck missed Stop). The next continuation kind will miss one site.

Suggestions

  • Extract one _recovery_eligible(slot, stop_gen) helper and one canonical PURGEABLE_CONTINUATIONS set so intervention precedence is defined once, not four times.

[DESIGN-REVIEWED] 07ff841

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 07ff84185956a8af5e85589638d800033287f404 via the fork AI-review pipeline — 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 verification is done. Here is the review.

First-Principles-Verdict: CONCERNS

The fix is real and reuses the existing continuation machinery, but sentinel detection lands in only one of three documented parity copies of this ladder.

What this change ships

Intent: stop treating Kiro CLI's "I hit an issue… Please retry." success-wrapped failure as a real answer, and resume the live session safely. This is a FIX (#8285).

  1. Sentinel after tool work becomes one auto-continuation, never a persisted answer — justified
  2. Repeated sentinel ends with a terminal notice instead of looping — justified (reuses one-shot)
  3. Already-streamed sentinel text is blanked in the browser until refresh — justified
  4. Failed-turn file-change chips no longer attach to the previous turn's answer — declared second fix, required by 1
  5. Stop/follow-up/steer now purges a queued post-token continuation at dispatch — justified
  6. Stop during retry backoff now shows "recovery cancelled" instead of a false "recovering…" — justified
  7. Existing hiccup notice now appears after the backoff, seconds later — undeclared timing change
  8. Sentinel turns excluded from success recording and budget resets — justified
  9. Session spec documents the contract — mandated (same-commit rule)
  10. Log relabel "promise-only" → "auto-continuation(s)" — rides along, accurate

Watch

  • Point patch with 2 counted unfixed siblings. Grepped _stream_with_transient_retry / _resume_used: the base names three hand-maintained copies of this post-activity ladder (chat_runner.py:10889 PARITY NOTE, subagent_manager/run.py:939, slack/gateway.py:1214). A subagent or Slack/cron turn ending on the same sentinel still records success and reports the sentence as its result. The shared in-repo choke point is providers/acp.py (where is_kiro_backend already lives); detection there would cover all three consumers. Deferring per-surface recovery is fine, but the description doesn't say what's left.
  • Item 7: the honest cancelled/recovering split forces the notice after the sleep, so users now see nothing during the backoff — fine, but unmentioned.

Subtractions

  • Drop the turn_boundary: int = 0 default on _flush_file_changes — both call sites (2/2, grepped _flush_file_changes\() pass it explicitly, so the default has zero consumers and its value silently restores the exact cross-turn attach bug item 4 fixes.

[FIRST-PRINCIPLES-REVIEWED] 07ff841

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review (fork) — 🔴 changes requested (blocking)

Reviewed 07ff84185956a8af5e85589638d800033287f404 via the fork AI-review pipeline; updated in place on each push.

Adjudication emitted 0 well-formed verdict line(s) for total=1, so GPT's verdict stands.

BLOCKING -- src/kiro_crew/dashboard/chat_runner.py:9644 -- Model text can forge the backend-failure control signal

and is_upstream_turn_failure_text(_answer_text)
Untrusted tool content induces the exact sentence after a tool call -> _upstream_turn_failed -> _queue_recovery dispatches another tool-capable turn under existing approvals.
Anchor: backend-security-controls
Fix: Require an out-of-band Kiro failure event; never infer provider control state from assistant text.
[BLOCK-MERGE] 07ff841
[GPT-REVIEWED] 07ff841

Adjudication (Opus 4.8) — is blocking on each finding proportionate?

I've verified the mechanism against the base code and the diff.

F1 — The forged sentinel (exact-match is_upstream_turn_failure_text(_answer_text), patch hunk at chat_runner.py:9644) at worst triggers _queue_recovery (chat_runner.py:5519), which re-stamps fresh admission context via containment_meta and enqueues a CONTINUATION on the same live session under existing approvals — no new capability, no governance bypass, subject to the same PreToolUse gate. It is one-shot: _will_recover requires not slot._posttoken_retry_used and sets it True (diff), the allowance only refreshed on a genuine user turn (chat_runner.py:5569), and it is visible to the user (⟳ Backend turn failed — recovering…). The finding's "under existing approvals" concedes there is no escalation; this is not the security-class rung.

Harm rung: LOW/MEDIUM — a bounded, one-shot, visible extra continuation an attacker controlling tool output could already influence within the turn; no privilege escalation, no ceiling bypass, no credential exposure, self-correcting. Remedy proposed ("require an out-of-band Kiro failure event") is infeasible without changing the external kiro-cli/ACP protocol — the very reason the text-inference exists is that kiro-cli closes a failed inner loop as an ordinary Ok/UserTurnEnd with no out-of-band event — so the real fix is a cross-boundary protocol change or abandoning the feature. That cost clearly exceeds the bounded harm.

[ADJUDICATION] 07ff84185956a8af5e85589638d800033287f404 total=1 uphold=0 downgrade=1
DOWNGRADE F1 src/kiro_crew/dashboard/chat_runner.py:9644 reason=disproportionate-remedy
[GPT-ADJUDICATED] 07ff84185956a8af5e85589638d800033287f404

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 07ff84185956a8af5e85589638d800033287f404 via the fork AI-review pipeline; updated in place on each push.

Review details

The candidate's core harm — that a nested (depth>0) turn discards the sentinel and broadcasts a blank assistant row that is never reconciled ("a nested turn may not emit that terminal refresh, leaving a stray empty bubble") — is falsified. The depth-1 _run_chat re-entry (/prompts get) runs the full turn and reaches the unconditional outer finally, which calls _finish_queue_cycle at chat_runner.py:11453 → broadcast_ws("chat_done", ...) at 5139. That chat_done drives the same authoritative refreshSlot re-fetch used at depth 0, and since the blank frame is deliberately never persisted, it is removed identically at any depth. The recovery/continuation is depth-gated (_prompt_depth == 0 in _will_recover); at depth>0 the turn merely shows an honest terminal "Backend turn failed — please retry." notice instead of persisting the failure sentence as an answer and recording a false success — that is not a worse outcome than the pre-PR behavior, so (c) an observable wrong outcome does not hold.

No new grounded defect surfaced under Step 2.

No findings.

[OPUS-REVIEWED] 07ff841

@Pearcekieser
Pearcekieser force-pushed the fix/generic-retry-recovery-20260903 branch from 8f697aa to a4d87da Compare September 4, 2026 02:40
@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • span=db37e66c9e96 — fixed

Kiro output-token check deletes legitimate answers because Kiro's credit-only usage leaves output_tokens at zero.

Fixed in a4d87da1002a81201ffd54fd45657fed345c60b5. The sentinel path no longer reads token counts. It now requires positive Kiro identity, at least one executed tool, the exact final sentence, and that the current user message did not itself contain that literal. New controls preserve no-tool exact output, non-Kiro output, and an explicit exact-response request after tool activity; the observed unrequested tool-active failure still recovers once.

@Pearcekieser
Pearcekieser force-pushed the fix/generic-retry-recovery-20260903 branch from a4d87da to a2affcc Compare September 4, 2026 03:33
@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • span=db37e66c9e96 — fixed: Stop during retry backoff is forgotten

A Stop can begin and resolve during post-token retry backoff; snapshotting afterward makes dispatch believe no intervention occurred.

Fixed in a2affcc940efa6f44fa020449b295510bff6d808. Recovery now requires the turn-start Stop generation before backoff and rechecks it—plus active Stop, queued user follow-ups, pending steers, and stage execution—after the await and before consuming the one-shot or enqueueing. A cancelled recovery produces an untagged terminal receipt. The new regression drives Stop from pending back to idle inside the mocked sleep and proves no queue entry, no consumed allowance, preserved partial output, and no pending-retry tag.

@Pearcekieser
Pearcekieser force-pushed the fix/generic-retry-recovery-20260903 branch from a2affcc to 9299643 Compare September 4, 2026 04:36
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 4, 2026
@Pearcekieser
Pearcekieser force-pushed the fix/generic-retry-recovery-20260903 branch from 9299643 to b5162c5 Compare September 4, 2026 05:31
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 4, 2026
@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • span=db37e66c9e96 — rebutted: Generated sentinel text is trusted as provider control output

Require completion usage to report zero output tokens before classifying the sentinel.

That discriminator is not valid for Kiro CLI. The captured real failure session records credit-only accounting and zero token fields, but legitimate Kiro model text has the same zero-token shape; requiring zero therefore does not distinguish provider control output from model output and would only appear to close the ambiguity. The implemented behavioral gate instead requires positive Kiro-backend identity, UserTurnEnd, tool activity, exact whole-response equality, no compaction notice, and absence of the literal from the current user request. Recovery is a one-shot continuation on the same session that explicitly forbids replaying completed tools, not a replay or a privilege-bearing action. test_reported_output_tokens_do_not_override_behavioral_evidence pins the accounting boundary. No token-count condition was added.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • span=db37e66c9e96 — fixed: Server-only purge leaves a phantom assistant message

Sentinel streams to the client, then chat_done finalizes it as an assistant row.

Fixed in b5162c50b792a7e52fcc819e9366227a34a9bbce. After purging the server-only chunk rows, the runner now broadcasts an empty assistant reconciliation before resetting the stream redactor and before appending the recovery notice. That replaces any live browser streaming row; the normal terminal refreshSlot then removes the unpersisted blank row from the authoritative transcript. The regression proves a sentinel chat_chunk reached the wire before the empty assistant reconciliation, while the server transcript contains no sentinel answer. Full dashboard validation: 758 passed; adjacent continuation suite: 55 passed; static/type/docs gates passed.

@Pearcekieser
Pearcekieser force-pushed the fix/generic-retry-recovery-20260903 branch from b5162c5 to 3db8995 Compare September 4, 2026 06:29
@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • span=db37e66c9e96 — fixed via restructure: Clearing the sentinel misattributes file changes

A file-writing tool followed by the sentinel can attach changes to the previous turn.

Fixed in 3db89952532e70fda1a926185de2691b015536f0 as a general attachment-boundary correction rather than another sentinel special case. _flush_file_changes now accepts the same len(slot.messages) turn boundary already used by _attach_turn_stats; both its landed and unconditional-finalizer calls pass that boundary. It scans only assistant rows created by this turn and, when none exists, creates the established synthetic “files were modified” assistant anchor. The regression starts with a prior assistant row, executes a file-writing sentinel turn, and proves the prior row remains untouched while the new current-turn anchor owns the snapshot. Validation: 759 dashboard tests, 113 file-change/flush/post-compaction contracts, and full static/type/docs gates passed; the final rebase retained identical patch-id and 77 focused tests re-passed.

@bolichen97

Copy link
Copy Markdown
Collaborator

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 #7626 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 #7626: REBASE. Two independent fixes for two different provider artifacts converging on one shared recovery mechanism and duplicating its purge/guard edits. They should agree on one owner for the _POSTTOKEN_RECOVER_MSG purge set and one policy for sharing (or splitting) the one-shot before either lands. Files: src/kiro_crew/dashboard/chat_runner.py.
  • PR #7709 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 #7709: KEEP. Same function, disjoint hunks and unrelated user goals. Files: src/kiro_crew/dashboard/chat_runner.py. The two independent directions used different labels; the matrix conservatively retains OVERLAPPING for coordination.
  • PR #8215 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 #8215: KEEP. Two different backend faults on the same recovery ladder; neither implements the other's behaviour and each is useless for the other's trigger. 8215's timeline carries a cross-reference to 8288, so the authors are aware. Worth one review note on landing order: 8288's arm gates on Stop, stop-generation and queued user input and registers its continuation as purgeable, and 8215's arm deliberately does not (Opus rebutted that finding by pointing at the pipe-death sibling, GPT still holds it blocking). Files: src/kiro_crew/dashboard/chat_runner.py.
  • This PR is OVERLAPPING with PR #6850. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8288: KEEP. Complementary halves of the same lifecycle-honesty problem: 6850 diagnoses false model progress prose without replaying, 8288 recovers a provider control sentence by continuing. Neither subsumes the other, and 8288's own Description already names 6850 as related-but-independent. Merge order only decides which one resolves a trivial import-block and chat_utils placement conflict. Files: src/kiro_crew/dashboard/chat_utils.py, src/kiro_crew/dashboard/chat_runner.py.
  • This PR is OVERLAPPING with PR #8275. 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.

Recognize Kiro CLI's successful-looking turn-failure sentinel, preserve completed tool work, and continue once without recording a false success.
@Pearcekieser
Pearcekieser force-pushed the fix/generic-retry-recovery-20260903 branch from 3db8995 to 07ff841 Compare September 4, 2026 07:10
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed merge conflict Branch has merge conflicts with its base — author must resolve before merge readiness: action required A blocking check or review needs attention labels Sep 4, 2026
@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • span=db37e66c9e96 — rebutted: Model text can forge the backend-failure control signal

Require an out-of-band Kiro failure event; never infer provider control state from assistant text.

No such event exists in the observed Kiro CLI/ACP failure shape: the real incidents persist Ok + UserTurnEnd + the exact fixed sentence after tool cycles, with no out-of-band failure frame. Abandoning text recognition would abandon the requested recovery behavior. The current-head Opus adjudication independently downgraded this finding as disproportionate: a forged match causes at most one visible same-session continuation, re-stamped with current containment, still subject to the same PreToolUse/approval gates, with no capability or governance escalation; user follow-up, steer, or Stop cancels it. This is the protocol-bound residual ambiguity, not a blocking security bypass. No code change is warranted.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 4, 2026
@github-actions github-actions Bot added the merge conflict Branch has merge conflicts with its base — author must resolve before merge label Sep 4, 2026
@NicholasRBowers NicholasRBowers added needs-pr-triage PR scanner: awaiting automated triage needs-author-decision PR blocked on author input and removed needs-pr-triage PR scanner: awaiting automated triage labels Sep 7, 2026
@NicholasRBowers

Copy link
Copy Markdown
Contributor

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]: This PR has been inactive for 7+ days. I reviewed the blockers but they require input beyond what automation can decide:

  • GPT security-lane block on span db37e66c9e96 (chat_runner.py:9644 — "Model text can forge the backend-failure control signal") is on its 6th consecutive round on the same span. You have rebutted it with structural evidence (no out-of-band failure frame exists in the observed kiro-cli/ACP shape), and the Opus adjudication on head 07ff841 independently rated the harm LOW/MEDIUM and the demanded remedy infeasible without a cross-boundary protocol change — but the adjudication emitted no well-formed verdict line, so GPT's block stands. Fork review lanes re-derive verdicts and honor no /ai-review override, so the remaining gate is a maintainer ruling on the rebuttal (accept it, or decide the feature needs the protocol change / descope). Automation pushing a 7th same-span patch would violate the stall protocol.
  • Once that ruling lands, the merge conflict with main is mechanical and the pipeline can pick this up again on its next cycle.

When these are addressed, the pipeline will re-assess on its next cycle.

@bolichen97

Copy link
Copy Markdown
Collaborator

Hi @Pearcekieser — from a repo-wide audit of open PRs, this one overlaps #9156 (@buluoray), and the two need one decision before either lands.

The shared ground is src/kiro_crew/dashboard/chat_runner.py, src/kiro_crew/dashboard/chat_utils.py and test/test_dashboard_chat.py, specifically the _flush_file_changes attach block. This PR adds turn_boundary: int = 0 and slices the reverse scan (for m in reversed(slot.messages[boundary:])). #9156 deletes that reverse scan and attaches to an identity-bound target_row captured in a new _dedupe_file_changes.

They are not redundant. #9156's target_row is the last assistant message at flush time, which for an error-only write turn is the previous turn's row, the exact case this PR fixes. Your boundary slice, in turn, does not address the detached-flush successor-turn race #9156 targets. So whichever lands second must re-express its fix in the other's shape: your turn_boundary would have to apply when the row is captured, not when it is attached.

Concrete suggestion: split the _flush_file_changes row-attachment fix out of both PRs into one small PR that lands first, and keep this PR to the sentinel-recovery scope (is_upstream_turn_failure_text plus the _POSTTOKEN_RECOVER_MSG continuation). @buluoray, does that split work on your side?

Separately, this PR is 497 commits behind and merges dirty. #8318 landed _append_redaction_notice(slot, _safe) inside the same post-token block you rewrite, so a rebase is needed before review.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor) merge conflict Branch has merge conflicts with its base — author must resolve before merge needs-author-decision PR blocked on author input readiness: action required A blocking check or review needs attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Kiro CLI synthetic retry response is recorded as a successful turn

3 participants