fix(reborn): complete model error recovery contract - #6845
Merged
Merged
Conversation
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6845
July 29, 2026 09:54
Destroyed
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR separates spend-budget, context-overflow, and output-truncation errors; adds typed observation-based recovery and provider-usage reconciliation; validates fallback-route evidence; and introduces non-retryable typed checkpoint rejection with bounded host-authored failure details. ChangesTyped model error recovery
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Provider
participant ModelStage
participant RecoveryStrategy
Provider->>ModelStage: FinishReason::Length
ModelStage->>RecoveryStrategy: OutputTruncated
RecoveryStrategy->>ModelStage: typed continue-or-condense observation
ModelStage->>Provider: recovery request
Provider->>ModelStage: finalized reply
sequenceDiagram
participant Executor
participant CheckpointHost
participant PlannedDriver
participant ProductProjection
Executor->>CheckpointHost: write checkpoint
CheckpointHost-->>Executor: CheckpointRejected
Executor->>PlannedDriver: typed rejection
PlannedDriver->>ProductProjection: host-authored failure detail
ProductProjection->>ProductProjection: validate detail without explainer
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size: XL
Contributor
🔎 Review · PR #6845
Submitted review →Reviewed the complete trusted comparison a0e91d1..2cc0ae7 across all 31 changed files. The typed error projections, bounded recovery state, truncation handling, budget-accounting path, runner categories, checkpoint serialization, and integration support are internally consistent. No concrete actionable findings were identified. Automatic · PR opened · attempt 1 of 3 · completed in 2m 17s Run details
|
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🔍 Review complete · PR #6845
✅ No actionable findings
Reviewed the complete trusted comparison a0e91d1..2cc0ae7 across all 31 changed files. The typed error projections, bounded recovery state, truncation handling, budget-accounting path, runner categories, checkpoint serialization, and integration support are internally consistent. No concrete actionable findings were identified.
Validation and technical details
- Inspected every changed production and test area plus surrounding model gateway, executor, checkpoint, prompt, budget-accountant, failure-mapping, and runner call paths.
- Verified the trusted base and head refs and used their complete comparison rather than the checkout HEAD.
git diff --check refs/ironloop/base..refs/ironloop/headpassed.- Reviewed exhaustive error-kind/category matrices and tests covering output truncation, stale/availability recovery, spend-budget exhaustion, accounting warnings, checkpoint round trips, and suppression of truncated textual tool calls.
- Could not execute Cargo tests because the review environment does not provide the
cargoexecutable; this is an environment limitation, not a human-decision blocker. - Base:
main - Head:
codex/error-recoverability-ws2at2cc0ae7 - Run:
7d2f8c37-4be1-4626-a8f5-86ba97b155ca
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_runner/src/turn_runner.rs (1)
51-58: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve terminal context/truncation categories.
model_context_overflowandmodel_output_truncatedare registered infailure_lane.rsand have dedicated summaries/retry dispositions, but this allowlist omits both. When recovery finally returns either category, this branch rewrites it todriver_failed, losing the typed durable/public failure contract. Preserve both categories here and add regression cases alongside the spend-budget test.As per coding guidelines, preserve observable failure kinds and durable state exactly.
Also applies to: 175-188
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_runner/src/turn_runner.rs` around lines 51 - 58, Extend the allowlist in the base-category selection logic of turn_runner to include model_context_overflow and model_output_truncated. Preserve these categories unchanged when recovery returns them, and add regression coverage alongside the existing spend-budget test verifying their durable/public failure kinds remain intact.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_loop_host/src/budget_accountant.rs`:
- Around line 1040-1046: Update the approval-threshold test setup to set
max_output_tokens to 27, producing a $9.10 total below the $10 hard cap, and
change the assertion around err.kind to require
AgentLoopHostErrorKind::BudgetApprovalRequired exclusively. Add or update a
caller-level test at the meaningful contract seam to verify this approval-path
behavior.
---
Outside diff comments:
In `@crates/ironclaw_runner/src/turn_runner.rs`:
- Around line 51-58: Extend the allowlist in the base-category selection logic
of turn_runner to include model_context_overflow and model_output_truncated.
Preserve these categories unchanged when recovery returns them, and add
regression coverage alongside the existing spend-budget test verifying their
durable/public failure kinds remain intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bce3aabe-d03d-45e8-9cf7-57b29679fd60
📒 Files selected for processing (31)
crates/ironclaw_agent_loop/src/executor/mapping.rscrates/ironclaw_agent_loop/src/executor/model.rscrates/ironclaw_agent_loop/src/executor/tests.rscrates/ironclaw_agent_loop/src/executor/tests/failure_matrix.rscrates/ironclaw_agent_loop/src/families/mod.rscrates/ironclaw_agent_loop/src/families/subagent.rscrates/ironclaw_agent_loop/src/state/model_recovery.rscrates/ironclaw_agent_loop/src/state/slots.rscrates/ironclaw_agent_loop/src/state/terminal_warning.rscrates/ironclaw_agent_loop/src/strategies/recovery.rscrates/ironclaw_loop_host/src/budget_accountant.rscrates/ironclaw_loop_host/src/identity_context.rscrates/ironclaw_loop_host/src/lib.rscrates/ironclaw_loop_host/src/skill_context.rscrates/ironclaw_loop_host/src/system_inference.rscrates/ironclaw_runner/src/failure_categories.rscrates/ironclaw_runner/src/failure_lane.rscrates/ironclaw_runner/src/failure_summary.rscrates/ironclaw_runner/src/model_failure_mapping.rscrates/ironclaw_runner/src/model_gateway.rscrates/ironclaw_runner/src/retry_disposition.rscrates/ironclaw_runner/src/text_loop_driver.rscrates/ironclaw_runner/src/turn_runner.rscrates/ironclaw_runner/tests/llm_gateway.rscrates/ironclaw_turns/src/run_profile/host/error.rscrates/ironclaw_turns/src/run_profile/model.rscrates/ironclaw_turns/tests/agent_loop_host_contract.rstests/integration/model_recovery.rstests/integration/support/assertions.rstests/integration/support/builder.rstests/integration/support/scripted_provider.rs
|
🚅 Deployed to the ironclaw-pr-6845 environment in ironclaw-ci-preview
|
This was referenced Jul 29, 2026
This was referenced Jul 29, 2026
Closed
…ility-ws2 # Conflicts: # crates/ironclaw_agent_loop/src/executor/tests.rs
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6845
July 29, 2026 16:51
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6845
July 29, 2026 16:55
Destroyed
Collaborator
Author
|
Addressed the remaining CodeRabbit findings in
Focused validation: runner category regression passed; formatting and diff checks passed. The accountant test compilation was attempted locally but the shared macOS build volume exhausted disk; the pushed CI run is the clean-runner verification. |
Contributor
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.55% — 315441 / 368740 lines Per-crate breakdown (60 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
serrrfirat
added a commit
that referenced
this pull request
Jul 29, 2026
* fix(reborn): explain rejected checkpoints durably * Format reconciled checkpoint projection imports
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6845
July 29, 2026 19:16
Destroyed
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_product/src/projection/tests/failure_explanation.rs`:
- Around line 218-221: Remove the duplicate
MODEL_SPEND_BUDGET_EXHAUSTED_CATEGORY entry from the expected
failure-explanation table, retaining the earlier row and its summary so the
table contains each category exactly once.
In `@crates/ironclaw_runner/src/failure_summary.rs`:
- Line 44: Update the LoopSafeSummary::new call in the failure-summary
construction to avoid .ok()?; explicitly handle its validation error while
preserving the existing None fallback and retaining diagnostic context for the
rejected persisted detail. Ensure production Rust does not silently discard this
error.
In `@crates/ironclaw_turns/src/run_profile/host/refs.rs`:
- Around line 199-203: Update CheckpointRejected::checkpoint_rejected to use
cause-neutral fallback wording rather than implying validation failure, while
preserving its role for host-rejected checkpoints without a valid bounded cause.
Add a caller-level regression test through checkpoint_host_error covering an
invalid producer summary and asserting the neutral fallback message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: abf3619b-81d1-4c58-ae4a-f8bb4a8b55de
📒 Files selected for processing (17)
crates/ironclaw_agent_loop/src/executor.rscrates/ironclaw_agent_loop/src/executor/checkpoint.rscrates/ironclaw_agent_loop/src/executor/tests.rscrates/ironclaw_agent_loop/src/executor/tests/cancellation.rscrates/ironclaw_agent_loop/src/executor/tests/failure_matrix.rscrates/ironclaw_product/src/projection/tests/failure_explanation.rscrates/ironclaw_product/src/projection/turn_events.rscrates/ironclaw_runner/src/failure_categories.rscrates/ironclaw_runner/src/failure_summary.rscrates/ironclaw_runner/src/planned_driver.rscrates/ironclaw_runner/src/turn_runner.rscrates/ironclaw_runner/tests/loop_driver_host.rscrates/ironclaw_turns/src/run_profile/host/refs.rscrates/ironclaw_turns/src/turn_state_row_store/turn_state_engine/transitions.rsdocs/reborn/contracts/loop-exit.mddocs/reborn/contracts/turn-runner.mdscripts/reborn-e2e-rust.sh
…ility-ws2 # Conflicts: # crates/ironclaw_agent_loop/src/strategies/recovery.rs
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6845
July 29, 2026 20:44
Destroyed
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/ironclaw_runner/src/model_gateway.rs (1)
2143-2146: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAccount for truncated completion usage before recovery.
FinishReason::Lengthdropsusagewhen it becomesOutputTruncated. The post-call accountant then sees a failure and releases the reservation, so tokens consumed by the truncated attempt are not charged before the continuation call. Preserve/reconcile usage on this failure path in both text and tool response conversion, with a caller-path regression test.As per coding guidelines, “Test through the caller” applies when a transform gates model dispatch and accounting side effects.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_runner/src/model_gateway.rs` around lines 2143 - 2146, Update both text and tool response conversion paths handling FinishReason::Length so they preserve and reconcile the attempt’s usage before returning OutputTruncated, allowing post-call accounting to charge consumed tokens before recovery. Add a regression test through the caller path that exercises the truncation, continuation, and accounting side effects rather than testing the conversion helper alone.Source: Coding guidelines
crates/ironclaw_loop_host/src/lib.rs (1)
1820-1822: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRequire and validate fallback-route evidence.
effective_fallback_indexis declared authoritative, but a missing serialized field becomes0; this passes the existing default-route equality check. System inference also consumes gateway responses without checking the reported route at all. Fail closed instead of accepting absent or mismatched runtime-selection evidence.
crates/ironclaw_loop_host/src/lib.rs#L1820-L1822: remove the zero default for authoritative response evidence, or represent absence explicitly and reject it before accepting a response.crates/ironclaw_loop_host/src/system_inference.rs#L160-L166: retain the requested index and reject a response whose effective index differs before reading its output.As per coding guidelines, “Fail closed for ... runtime selection” is required.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_loop_host/src/lib.rs` around lines 1820 - 1822, Require explicit fallback-route evidence: in crates/ironclaw_loop_host/src/lib.rs lines 1820-1822, remove the default zero from effective_fallback_index or make absence explicit and reject it before accepting the response; in crates/ironclaw_loop_host/src/system_inference.rs lines 160-166, retain the requested index and reject any response whose effective index differs before reading its output.Sources: Coding guidelines, Path instructions
crates/ironclaw_runner/src/planned_driver.rs (1)
350-358: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftDo not forward raw prompt-stage detail to the driver boundary.
Line 351 can expose
AgentLoopHostError.detail; the current scrubber preserves path-like values, while this new prompt-stage path returns that detail inAgentLoopDriverError::Failed. Keep the actionableLoopSafeSummary, but drop raw detail or map it to a stable redacted explanation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_runner/src/planned_driver.rs` around lines 350 - 358, Update the prompt-stage failure handling in permanent_prompt_stage_failure_category so AgentLoopDriverError::Failed never receives raw AgentLoopHostError.detail. Preserve the actionable LoopSafeSummary via the existing safe_summary path, and either omit detail or replace it with a stable redacted explanation before constructing the Failed result.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_agent_loop/src/strategies/recovery.rs`:
- Around line 388-405: Preserve the host-selected fallback index through
recovery: in crates/ironclaw_agent_loop/src/strategies/recovery.rs lines
388-405, pass next_fallback_index as payload in RetryAlteration instead of using
payload-free AdvanceFallback; in
crates/ironclaw_agent_loop/src/executor/model.rs lines 462-473, validate the
requested index advances monotonically and assign that exact index to
state.model_state.fallback_index rather than incrementing the local index.
In `@tests/integration/model_recovery.rs`:
- Around line 187-228: Extend
output_truncation_recovers_without_shrinking_input_context, or add a
capability-backed regression alongside it, using recognizable partial textual
tool-call content instead of only the default echo response and empty
tool_calls. Configure a capability that would visibly record invocation or
egress, then assert it is never invoked before the recovery turn while
preserving the existing successful recovery assertions.
---
Outside diff comments:
In `@crates/ironclaw_loop_host/src/lib.rs`:
- Around line 1820-1822: Require explicit fallback-route evidence: in
crates/ironclaw_loop_host/src/lib.rs lines 1820-1822, remove the default zero
from effective_fallback_index or make absence explicit and reject it before
accepting the response; in crates/ironclaw_loop_host/src/system_inference.rs
lines 160-166, retain the requested index and reject any response whose
effective index differs before reading its output.
In `@crates/ironclaw_runner/src/model_gateway.rs`:
- Around line 2143-2146: Update both text and tool response conversion paths
handling FinishReason::Length so they preserve and reconcile the attempt’s usage
before returning OutputTruncated, allowing post-call accounting to charge
consumed tokens before recovery. Add a regression test through the caller path
that exercises the truncation, continuation, and accounting side effects rather
than testing the conversion helper alone.
In `@crates/ironclaw_runner/src/planned_driver.rs`:
- Around line 350-358: Update the prompt-stage failure handling in
permanent_prompt_stage_failure_category so AgentLoopDriverError::Failed never
receives raw AgentLoopHostError.detail. Preserve the actionable LoopSafeSummary
via the existing safe_summary path, and either omit detail or replace it with a
stable redacted explanation before constructing the Failed result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2db1b57d-d88d-4e41-83a1-fecf3b45cddb
📒 Files selected for processing (25)
crates/ironclaw_agent_loop/src/executor.rscrates/ironclaw_agent_loop/src/executor/mapping.rscrates/ironclaw_agent_loop/src/executor/model.rscrates/ironclaw_agent_loop/src/executor/tests.rscrates/ironclaw_agent_loop/src/executor/tests/cancellation.rscrates/ironclaw_agent_loop/src/families/mod.rscrates/ironclaw_agent_loop/src/strategies/recovery.rscrates/ironclaw_loop_host/src/budget_accountant.rscrates/ironclaw_loop_host/src/lib.rscrates/ironclaw_loop_host/src/system_inference.rscrates/ironclaw_runner/src/model_failure_mapping.rscrates/ironclaw_runner/src/model_gateway.rscrates/ironclaw_runner/src/planned_driver.rscrates/ironclaw_runner/src/text_loop_driver.rscrates/ironclaw_runner/tests/llm_gateway.rscrates/ironclaw_runner/tests/loop_driver_host.rscrates/ironclaw_turns/src/run_profile/host/error.rscrates/ironclaw_turns/src/run_profile/model.rscrates/ironclaw_turns/src/turn_state_row_store/turn_state_engine/transitions.rscrates/ironclaw_turns/tests/agent_loop_host_contract.rsdocs/reborn/contracts/turn-runner.mdtests/integration/model_recovery.rstests/integration/support/assertions.rstests/integration/support/builder.rstests/integration/support/scripted_provider.rs
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6845
July 29, 2026 21:02
Destroyed
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/ironclaw_runner/src/failure_summary.rs (2)
167-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unreachable duplicate checkpoint branch.
reborn_failure_summary_for_categoryreturns at Lines 68–70 whenpinned_failure_summary_for_categoryrecognizesCHECKPOINT_REJECTED_CATEGORYat Lines 303–309. Therefore the match arm at Line 167 cannot execute; keeping both definitions creates two sources of truth that can drift.Also applies to: 303-309
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_runner/src/failure_summary.rs` at line 167, Remove the unreachable CHECKPOINT_REJECTED_CATEGORY match arm from reborn_failure_summary_for_category and remove the corresponding pinned fallback definition in pinned_failure_summary_for_category, leaving a single authoritative checkpoint rejection mapping.
329-346: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest the newly added invalid-cause rejection path.
The production change at Lines 44–50 validates the checkpoint cause, but this test covers only a valid round trip and an unknown stage. Add a valid-stage envelope with a validator-rejected cause and assert
None.As per path instructions, every changed failure-handling path needs regression coverage at the nearest meaningful seam.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_runner/src/failure_summary.rs` around lines 329 - 346, Add regression coverage in checkpoint_rejection_explanation_is_bounded_and_provenance_validated for a valid CheckpointKind envelope containing a cause rejected by the validator, and assert checkpoint_rejection_host_explanation_from_detail returns None. Keep the existing valid round-trip and unknown-stage assertions unchanged.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/ironclaw_runner/src/failure_summary.rs`:
- Line 167: Remove the unreachable CHECKPOINT_REJECTED_CATEGORY match arm from
reborn_failure_summary_for_category and remove the corresponding pinned fallback
definition in pinned_failure_summary_for_category, leaving a single
authoritative checkpoint rejection mapping.
- Around line 329-346: Add regression coverage in
checkpoint_rejection_explanation_is_bounded_and_provenance_validated for a valid
CheckpointKind envelope containing a cause rejected by the validator, and assert
checkpoint_rejection_host_explanation_from_detail returns None. Keep the
existing valid round-trip and unknown-stage assertions unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 14b48dd5-2b4d-4812-93dc-e2bea5c89c94
📒 Files selected for processing (4)
crates/ironclaw_agent_loop/src/executor/checkpoint.rscrates/ironclaw_product/src/projection/tests/failure_explanation.rscrates/ironclaw_runner/src/failure_summary.rscrates/ironclaw_turns/src/run_profile/host/refs.rs
💤 Files with no reviewable changes (1)
- crates/ironclaw_product/src/projection/tests/failure_explanation.rs
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6845
July 29, 2026 21:11
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6845
July 29, 2026 21:30
Destroyed
Collaborator
Author
|
Addressed the three outside-diff findings from CodeRabbit review 4812851195 in commit 7c4bfe4:
Verification: full ironclaw_turns, ironclaw_loop_host, and ironclaw_runner test suites; all 18 reborn_integration_model_recovery tests; warning-free clippy for affected crates and the integration target; dependent product/parity targets compiled. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_loop_host/src/lib.rs (1)
1275-1283: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve provider usage on every rejected model call.
The accountant treats failures without
usageas releasable reservations. These paths can have provider-reported tokens, so dropping usage undercharges spend and can bypass durable budget limits.
crates/ironclaw_loop_host/src/lib.rs#L1275-L1283: attach the destructuredusagewhen rejecting missing or mismatched fallback evidence.crates/ironclaw_runner/src/model_gateway.rs#L1815-L1824: attach usage to every tool-response failure arm, not onlyFinishReason::Length.crates/ironclaw_runner/src/model_gateway.rs#L2149-L2153: apply the same handling to text-only failures, including empty stop responses, content filters, unsupported tool use, and unknown finishes.As per path instructions, fail-loud handling must preserve the real accounting outcome; the PR objective also requires provider usage to survive rejected calls.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_loop_host/src/lib.rs` around lines 1275 - 1283, Preserve provider-reported usage on every rejected model call. In crates/ironclaw_loop_host/src/lib.rs lines 1275-1283, update the fallback-evidence rejection around the response destructuring to attach usage to the error. In crates/ironclaw_runner/src/model_gateway.rs lines 1815-1824 and 2149-2153, attach usage in every tool-response and text-only failure arm, including empty stop responses, content filters, unsupported tool use, unknown finishes, and non-length failures; retain fail-loud accounting behavior.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_loop_host/src/budget_accountant.rs`:
- Around line 612-624: The ModelCallOutcome::Failure reconciliation path
currently derives pricing from the request preference or run default instead of
the model that actually handled the call. Persist the gateway-selected effective
model alongside failure usage, then have the Some(usage) branch use that
persisted model when calling usage_for_reported_usage; add a regression covering
a fallback model with different costs.
---
Outside diff comments:
In `@crates/ironclaw_loop_host/src/lib.rs`:
- Around line 1275-1283: Preserve provider-reported usage on every rejected
model call. In crates/ironclaw_loop_host/src/lib.rs lines 1275-1283, update the
fallback-evidence rejection around the response destructuring to attach usage to
the error. In crates/ironclaw_runner/src/model_gateway.rs lines 1815-1824 and
2149-2153, attach usage in every tool-response and text-only failure arm,
including empty stop responses, content filters, unsupported tool use, unknown
finishes, and non-length failures; retain fail-loud accounting behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4b8f5aa3-2685-4df9-83ba-2f0fc4aaf86c
📒 Files selected for processing (16)
crates/ironclaw_loop_host/src/budget_accountant.rscrates/ironclaw_loop_host/src/lib.rscrates/ironclaw_loop_host/src/system_inference.rscrates/ironclaw_loop_host/tests/thread_loop_host_contract.rscrates/ironclaw_product/tests/support/planned_agent_loop.rscrates/ironclaw_runner/src/model_gateway.rscrates/ironclaw_runner/src/model_gateway_error_mapping.rscrates/ironclaw_runner/src/planned_driver.rscrates/ironclaw_runner/tests/llm_gateway.rscrates/ironclaw_runner/tests/loop_driver_host.rscrates/ironclaw_turns/src/run_profile/host/error.rscrates/ironclaw_turns/src/run_profile/model.rstests/integration/model_recovery.rstests/integration/support/assertions.rstests/integration/support/scripted_provider.rstests/support/reborn_parity_qa/binary_e2e.rs
Comment on lines
+612
to
+624
| ModelCallOutcome::Failure(error) => match error.usage { | ||
| Some(usage) => { | ||
| let effective_model = request | ||
| .model_preference | ||
| .as_ref() | ||
| .unwrap_or(&context.resolved_run_profile.model_profile_id); | ||
| PendingAccounting::Reconcile(usage_for_reported_usage( | ||
| usage, | ||
| 0, | ||
| self.cost_table.as_ref(), | ||
| effective_model, | ||
| &self.default_cost, | ||
| )) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Preserve the failed call’s effective model for reconciliation.
Line 614 prices failure usage from request.model_preference or the run default. That is not necessarily the route that ran: a fallback-indexed call may consume tokens on a differently priced model. Persist the gateway-selected effective model with failure usage and use it here; add a differing-cost fallback regression.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_loop_host/src/budget_accountant.rs` around lines 612 - 624,
The ModelCallOutcome::Failure reconciliation path currently derives pricing from
the request preference or run default instead of the model that actually handled
the call. Persist the gateway-selected effective model alongside failure usage,
then have the Some(usage) branch use that persisted model when calling
usage_for_reported_usage; add a regression covering a fallback model with
different costs.
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6845
July 30, 2026 08:40
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6845
July 30, 2026 08:53
Destroyed
serrrfirat
added a commit
that referenced
this pull request
Jul 30, 2026
* fix(reborn): complete model error recovery contract * fix(reborn): preserve terminal model error explanations * fix(reborn): address terminal error review findings * fix(reborn): address recovery review findings (#6845) * fix(reborn): close rejected checkpoints durably (#6861) * fix(reborn): explain rejected checkpoints durably * Format reconciled checkpoint projection imports * fix(reborn): retry idempotent transcript writes * fix(reborn): address checkpoint recovery review comments * fix(reborn): preserve selected model fallback * fix(reborn): harden model failure accounting * fix(reborn): address terminal error review * fix(agent-loop): use honest transcript fallback * test(loop-host): exercise transcript error classification * test(threads): cover backend error classification * fix(ci): exclude Rust unit tests from coverage gate
serrrfirat
added a commit
that referenced
this pull request
Aug 7, 2026
… SSE transport event-source-plus (fetch/ReadableStream). The legacy suites still faked window.EventSource, so the app never opened a stream and every duration-1/duration-4 legacy test that emitted frames failed with "no EventSource stream is open". - Extract the smoke suite's proven fetch fake into install_fake_v2_event_stream() in reborn_webui_harness, extended to record request URLs and headers for reconnect assertions. - Port all seven legacy scenario files onto it, updating cursor/token assertions to the header contract (Authorization bearer, Last-Event-ID) instead of the retired token/after_cursor query params. - legacy_skills delete: use the shared in-app confirmation dialog instead of a native browser dialog. - legacy_dom_resource_limits reconnect-timer: assert the pending reconnect is cancelled when the tab hides (the fetch transport schedules retries internally). - legacy_rendering: assert no live onerror/iframe/img nodes instead of substring-scanning escaped text. - extensions_api: restore the #6520 wire contract (retired authenticated/active/needs_setup/has_auth/onboarding_state booleans must be absent). - tool_execution truncated-tool test: expect model_output_truncated failure per #6845's no-recovery contract instead of an assistant recovery message. - streaming_run_control_api: drop the stream=true assertion for the OpenAI-compatible mock, which rides the buffered fallback since #7120 (rig-core cannot distinguish a complete stream from a truncated one).
serrrfirat
added a commit
that referenced
this pull request
Aug 8, 2026
… SSE transport event-source-plus (fetch/ReadableStream). The legacy suites still faked window.EventSource, so the app never opened a stream and every duration-1/duration-4 legacy test that emitted frames failed with "no EventSource stream is open". - Extract the smoke suite's proven fetch fake into install_fake_v2_event_stream() in reborn_webui_harness, extended to record request URLs and headers for reconnect assertions. - Port all seven legacy scenario files onto it, updating cursor/token assertions to the header contract (Authorization bearer, Last-Event-ID) instead of the retired token/after_cursor query params. - legacy_skills delete: use the shared in-app confirmation dialog instead of a native browser dialog. - legacy_dom_resource_limits reconnect-timer: assert the pending reconnect is cancelled when the tab hides (the fetch transport schedules retries internally). - legacy_rendering: assert no live onerror/iframe/img nodes instead of substring-scanning escaped text. - extensions_api: restore the #6520 wire contract (retired authenticated/active/needs_setup/has_auth/onboarding_state booleans must be absent). - tool_execution truncated-tool test: expect model_output_truncated failure per #6845's no-recovery contract instead of an assistant recovery message. - streaming_run_control_api: drop the stream=true assertion for the OpenAI-compatible mock, which rides the buffered fallback since #7120 (rig-core cannot distinguish a complete stream from a truncated one).
serrrfirat
added a commit
that referenced
this pull request
Aug 8, 2026
… SSE transport event-source-plus (fetch/ReadableStream). The legacy suites still faked window.EventSource, so the app never opened a stream and every duration-1/duration-4 legacy test that emitted frames failed with "no EventSource stream is open". - Extract the smoke suite's proven fetch fake into install_fake_v2_event_stream() in reborn_webui_harness, extended to record request URLs and headers for reconnect assertions. - Port all seven legacy scenario files onto it, updating cursor/token assertions to the header contract (Authorization bearer, Last-Event-ID) instead of the retired token/after_cursor query params. - legacy_skills delete: use the shared in-app confirmation dialog instead of a native browser dialog. - legacy_dom_resource_limits reconnect-timer: assert the pending reconnect is cancelled when the tab hides (the fetch transport schedules retries internally). - legacy_rendering: assert no live onerror/iframe/img nodes instead of substring-scanning escaped text. - extensions_api: restore the #6520 wire contract (retired authenticated/active/needs_setup/has_auth/onboarding_state booleans must be absent). - tool_execution truncated-tool test: expect model_output_truncated failure per #6845's no-recovery contract instead of an assistant recovery message. - streaming_run_control_api: drop the stream=true assertion for the OpenAI-compatible mock, which rides the buffered fallback since #7120 (rig-core cannot distinguish a complete stream from a truncated one).
pull Bot
pushed a commit
to Stars1233/ironclaw
that referenced
this pull request
Aug 10, 2026
* fix(composition): read landed attachments through the per-caller workspace mount
/projects/workspace/tenants/{tenant}/users/{user}, but the loop-host
attachment_read_port still read through the shared read-only fixed view
(services.workspace_filesystem), which resolves the workspace root. A
landed image therefore came back NotFound at model-gateway time and was
silently dropped, so vision-capable model payloads lost every inline
image (the duration-4 Playwright attachment failure).
Wire the read port over the same per-caller scoped handle the WebUI
lander uses (runtime_mounts::read_write_workspace_filesystem), mirroring
what nearai#7062 already did for the channel-host assembly. Under the Shared
policy the handle is byte-identical to the old fixed view; under
PerCaller it now resolves the caller's subtree.
* test(playwright): reconcile legacy WebUI v2 suites to the fetch-based SSE transport
event-source-plus (fetch/ReadableStream). The legacy suites still faked
window.EventSource, so the app never opened a stream and every
duration-1/duration-4 legacy test that emitted frames failed with "no
EventSource stream is open".
- Extract the smoke suite's proven fetch fake into
install_fake_v2_event_stream() in reborn_webui_harness, extended to
record request URLs and headers for reconnect assertions.
- Port all seven legacy scenario files onto it, updating cursor/token
assertions to the header contract (Authorization bearer,
Last-Event-ID) instead of the retired token/after_cursor query
params.
- legacy_skills delete: use the shared in-app confirmation dialog
instead of a native browser dialog.
- legacy_dom_resource_limits reconnect-timer: assert the pending
reconnect is cancelled when the tab hides (the fetch transport
schedules retries internally).
- legacy_rendering: assert no live onerror/iframe/img nodes instead of
substring-scanning escaped text.
- extensions_api: restore the nearai#6520 wire contract (retired
authenticated/active/needs_setup/has_auth/onboarding_state booleans
must be absent).
- tool_execution truncated-tool test: expect model_output_truncated
failure per nearai#6845's no-recovery contract instead of an assistant
recovery message.
- streaming_run_control_api: drop the stream=true assertion for the
OpenAI-compatible mock, which rides the buffered fallback since
nearai#7120 (rig-core cannot distinguish a complete stream from a truncated
one).
Kampouse
pushed a commit
to Kampouse/ironclaw
that referenced
this pull request
Aug 13, 2026
* fix(composition): read landed attachments through the per-caller workspace mount
/projects/workspace/tenants/{tenant}/users/{user}, but the loop-host
attachment_read_port still read through the shared read-only fixed view
(services.workspace_filesystem), which resolves the workspace root. A
landed image therefore came back NotFound at model-gateway time and was
silently dropped, so vision-capable model payloads lost every inline
image (the duration-4 Playwright attachment failure).
Wire the read port over the same per-caller scoped handle the WebUI
lander uses (runtime_mounts::read_write_workspace_filesystem), mirroring
what nearai#7062 already did for the channel-host assembly. Under the Shared
policy the handle is byte-identical to the old fixed view; under
PerCaller it now resolves the caller's subtree.
* test(playwright): reconcile legacy WebUI v2 suites to the fetch-based SSE transport
event-source-plus (fetch/ReadableStream). The legacy suites still faked
window.EventSource, so the app never opened a stream and every
duration-1/duration-4 legacy test that emitted frames failed with "no
EventSource stream is open".
- Extract the smoke suite's proven fetch fake into
install_fake_v2_event_stream() in reborn_webui_harness, extended to
record request URLs and headers for reconnect assertions.
- Port all seven legacy scenario files onto it, updating cursor/token
assertions to the header contract (Authorization bearer,
Last-Event-ID) instead of the retired token/after_cursor query
params.
- legacy_skills delete: use the shared in-app confirmation dialog
instead of a native browser dialog.
- legacy_dom_resource_limits reconnect-timer: assert the pending
reconnect is cancelled when the tab hides (the fetch transport
schedules retries internally).
- legacy_rendering: assert no live onerror/iframe/img nodes instead of
substring-scanning escaped text.
- extensions_api: restore the nearai#6520 wire contract (retired
authenticated/active/needs_setup/has_auth/onboarding_state booleans
must be absent).
- tool_execution truncated-tool test: expect model_output_truncated
failure per nearai#6845's no-recovery contract instead of an assistant
recovery message.
- streaming_run_control_api: drop the stream=true assertion for the
OpenAI-compatible mock, which rides the buffered fallback since
nearai#7120 (rig-core cannot distinguish a complete stream from a truncated
one).
Kampouse
pushed a commit
to Kampouse/ironclaw
that referenced
this pull request
Aug 13, 2026
* fix(composition): read landed attachments through the per-caller workspace mount
/projects/workspace/tenants/{tenant}/users/{user}, but the loop-host
attachment_read_port still read through the shared read-only fixed view
(services.workspace_filesystem), which resolves the workspace root. A
landed image therefore came back NotFound at model-gateway time and was
silently dropped, so vision-capable model payloads lost every inline
image (the duration-4 Playwright attachment failure).
Wire the read port over the same per-caller scoped handle the WebUI
lander uses (runtime_mounts::read_write_workspace_filesystem), mirroring
what nearai#7062 already did for the channel-host assembly. Under the Shared
policy the handle is byte-identical to the old fixed view; under
PerCaller it now resolves the caller's subtree.
* test(playwright): reconcile legacy WebUI v2 suites to the fetch-based SSE transport
event-source-plus (fetch/ReadableStream). The legacy suites still faked
window.EventSource, so the app never opened a stream and every
duration-1/duration-4 legacy test that emitted frames failed with "no
EventSource stream is open".
- Extract the smoke suite's proven fetch fake into
install_fake_v2_event_stream() in reborn_webui_harness, extended to
record request URLs and headers for reconnect assertions.
- Port all seven legacy scenario files onto it, updating cursor/token
assertions to the header contract (Authorization bearer,
Last-Event-ID) instead of the retired token/after_cursor query
params.
- legacy_skills delete: use the shared in-app confirmation dialog
instead of a native browser dialog.
- legacy_dom_resource_limits reconnect-timer: assert the pending
reconnect is cancelled when the tab hides (the fetch transport
schedules retries internally).
- legacy_rendering: assert no live onerror/iframe/img nodes instead of
substring-scanning escaped text.
- extensions_api: restore the nearai#6520 wire contract (retired
authenticated/active/needs_setup/has_auth/onboarding_state booleans
must be absent).
- tool_execution truncated-tool test: expect model_output_truncated
failure per nearai#6845's no-recovery contract instead of an assistant
recovery message.
- streaming_run_control_api: drop the stream=true assertion for the
OpenAI-compatible mock, which rides the buffered fallback since
nearai#7120 (rig-core cannot distinguish a complete stream from a truncated
one).
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
* fix(reborn): complete model error recovery contract * fix(reborn): address recovery review findings (nearai#6845) * fix(reborn): close rejected checkpoints durably (nearai#6861) * fix(reborn): explain rejected checkpoints durably * Format reconciled checkpoint projection imports * fix(reborn): address checkpoint recovery review comments * fix(reborn): preserve selected model fallback * fix(reborn): harden model failure accounting * Fix Reborn E2E checkpoint retry gate
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
* fix(reborn): complete model error recovery contract * fix(reborn): preserve terminal model error explanations * fix(reborn): address terminal error review findings * fix(reborn): address recovery review findings (nearai#6845) * fix(reborn): close rejected checkpoints durably (nearai#6861) * fix(reborn): explain rejected checkpoints durably * Format reconciled checkpoint projection imports * fix(reborn): retry idempotent transcript writes * fix(reborn): address checkpoint recovery review comments * fix(reborn): preserve selected model fallback * fix(reborn): harden model failure accounting * fix(reborn): address terminal error review * fix(agent-loop): use honest transcript fallback * test(loop-host): exercise transcript error classification * test(threads): cover backend error classification * fix(ci): exclude Rust unit tests from coverage gate
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
* fix(composition): read landed attachments through the per-caller workspace mount
/projects/workspace/tenants/{tenant}/users/{user}, but the loop-host
attachment_read_port still read through the shared read-only fixed view
(services.workspace_filesystem), which resolves the workspace root. A
landed image therefore came back NotFound at model-gateway time and was
silently dropped, so vision-capable model payloads lost every inline
image (the duration-4 Playwright attachment failure).
Wire the read port over the same per-caller scoped handle the WebUI
lander uses (runtime_mounts::read_write_workspace_filesystem), mirroring
what nearai#7062 already did for the channel-host assembly. Under the Shared
policy the handle is byte-identical to the old fixed view; under
PerCaller it now resolves the caller's subtree.
* test(playwright): reconcile legacy WebUI v2 suites to the fetch-based SSE transport
event-source-plus (fetch/ReadableStream). The legacy suites still faked
window.EventSource, so the app never opened a stream and every
duration-1/duration-4 legacy test that emitted frames failed with "no
EventSource stream is open".
- Extract the smoke suite's proven fetch fake into
install_fake_v2_event_stream() in reborn_webui_harness, extended to
record request URLs and headers for reconnect assertions.
- Port all seven legacy scenario files onto it, updating cursor/token
assertions to the header contract (Authorization bearer,
Last-Event-ID) instead of the retired token/after_cursor query
params.
- legacy_skills delete: use the shared in-app confirmation dialog
instead of a native browser dialog.
- legacy_dom_resource_limits reconnect-timer: assert the pending
reconnect is cancelled when the tab hides (the fetch transport
schedules retries internally).
- legacy_rendering: assert no live onerror/iframe/img nodes instead of
substring-scanning escaped text.
- extensions_api: restore the nearai#6520 wire contract (retired
authenticated/active/needs_setup/has_auth/onboarding_state booleans
must be absent).
- tool_execution truncated-tool test: expect model_output_truncated
failure per nearai#6845's no-recovery contract instead of an assistant
recovery message.
- streaming_run_control_api: drop the stream=true assertion for the
OpenAI-compatible mock, which rides the buffered fallback since
nearai#7120 (rig-core cannot distinguish a complete stream from a truncated
one).
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FinishReason::Lengththrough a model-visible continue-or-condense turn without consuming context-shrink attempts, and prevent truncated textual tool calls from being dispatched.Change Type
Linked Issue
Fixes #6700
Related #6284
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warningscargo clippy -p ironclaw_turns -p ironclaw_agent_loop -p ironclaw_loop_host -p ironclaw_runner --tests -- -D warnings.cargo buildcargo test --features integrationif database-backed or integration behavior changedcargo test -p ironclaw_reborn_integration_tests --test reborn_integration_model_recovery.review-prorpr-shepherd --fixwas run before requesting reviewTest Strategy
User behavior: A provider-truncated response now gets one actionable request to continue or condense, without losing context-shrink budget or dispatching an incomplete textual tool call. Recoverable provider availability/stale errors and a budget-accounting failure receive the allowed model-visible observation/final turn.
Risk areas:
Tests added or updated:
FinishReason::Lengththrough the real gateway caller and the run survives to a durable final response without shrinking input context.What the tests prove: Typed failure identity survives every producer/projection seam; output truncation cannot consume
ShrinkContext; partial tool-like output cannot escape as a side effect; observations and the budget-accounting warning are bounded and survive checkpoint serialization; and the provider caller can recover to a durable completed run.Commands run:
Security Impact
None. No permissions, network boundaries, secrets, file access, runtime policy, or sandbox behavior changed. Recovery instructions are host-authored typed observations; provider partial text is discarded on truncation and is not persisted or dispatched.
Reborn Trust-Boundary Checklist
rg -n "AgentLoopHostErrorKind::|HostManagedModelErrorKind::" crates tests; exhaustive matrices, touched-crate clippy, and caller tests passed.serde(default)fields fail closed or have migration tests. No permissive default was added; new serialized enum variants and checkpoint-carried warning/observation state have round-trip tests.Transient,Permanent,Misconfigured,PolicyDeniedor equivalent). Spend, context, truncation, and accounting categories remain distinct through summaries and retry disposition.Database Impact
None. No migration or database schema changed.
Blast Radius
The change touches Reborn agent-loop recovery, loop-host error producers, turns host/model contracts, runner failure projection, and the scripted integration provider. Regressions could affect model retry/finalization behavior, failure telemetry categories, or deserialization of an in-flight checkpoint containing one of the newly serialized variants.
Rollback Plan
Revert this commit. Existing pre-change checkpoints remain readable, but do not roll an affected deployment back to an older binary while runs with the new serialized warning/observation variants are in flight; drain or restart those runs first.
Review Follow-Through
Reviewer judgment is requested on the retracted
CheckpointRejectedwarning box in #6284: the live rejection occurs while writing the pre-model checkpoint, so requesting another model turn would cross the durable-event boundary that failed. This PR therefore preserves checkpoint rejection as terminal and adds the bounded final warning only forBudgetAccountingFailed.UnauthorizedandTranscriptWriteFailedalso remain sanctioned terminal/unreachable model-observation classes. Additionally, rig-core does not expose streaming finish reasons for every provider; this fixes every caller path whereFinishReason::Lengthis currently reachable.Review track: C (runtime and durable checkpoint contract)