test(reborn): compile-forced recoverability conformance matrix (§11.7 / #6284 item 7) - #6677
serrrfirat wants to merge 1 commit into
Conversation
Item 7 of the error-recoverability endgame (#6284), done first so every later item is provably complete rather than assertedly complete. No runtime behavior changes. This records what the code does today, including where that is wrong, so later PRs flip rows and the table diff is the review evidence. Vocabulary follows the governing design (`docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md`) rather than competing with it. §5.3.4 folds the epic's contract onto the five-channel resolution model; §11.7 specifies this matrix and names the enums it must cover. So the type is `RecoverabilityClass { Retry, ModelVisible, Park, Terminal }` in `ironclaw_host_api` (leaf crate, no new dependency edges), with each variant documented against the resolution channel it corresponds to — `ModelVisible` -> `Done(Outcome)` with a failure verdict or `Denied`, `Park` -> `Blocked`/`Suspended`, `Terminal` -> `HostFailure`, legitimate only for cancellation, budget exhaustion, and `DriverBug`. Seven exhaustive classifiers, each a `const fn` with no `_` arm, derived by tracing the live path rather than the doc comments. Each lives beside the enum it classifies so a new variant is a compile error there: RuntimeDispatchErrorKind 0/22 Terminal (host_api) RuntimeFailureKind 1/17 Terminal (host_runtime) CapabilityFailureKind 2/19 Terminal (turns) CapabilityErrorClass 1/7 Terminal (agent_loop) ModelErrorClass 3/10 Terminal (agent_loop) AgentLoopHostErrorKind 10/17 Terminal on the model stage 17/17 Terminal on the capability stage LoopFailureKind 13/13 Terminal, 11 of them unsanctioned `RuntimeDispatchErrorKind` is the enum §11.7 names; `RuntimeFailureKind` is the sanitized kind one fold downstream that the recovery strategy actually consumes. Both are real and both are classified, and `runtime_dispatch_error_kind_class_survives_the_runtime_fold` pins that `From<DispatchFailureKind>` preserves the class — the upstream table is derived, not asserted. `LoopFailureKind` is classified in `ironclaw_turns` because it is `#[non_exhaustive]`: cross-crate exhaustiveness is impossible, so the compile-forcing has to happen in the defining crate. It is also where the doc and the code disagree — §11.7 asks every variant to map to retry, a model-visible observation, or a park, which this enum cannot do: it names the reason a `LoopExit::Failed` already happened, so every row is terminal by construction. What it can carry is the second half of the sentence, "never *unclassified*", so each row states whether it is one of the three sanctioned invariants; the ratchet counts the eleven that aren't. `AgentLoopHostErrorKind` gets two stage-scoped classifiers because a single one would be a lie: `Unavailable` is retried twelve times on the model stage and kills the run outright on the capability stage (`capability_host_error` maps every non-`Cancelled` kind to `HostUnavailable{Capability}`). That 17/17 row is the largest remaining bork surface on the epic's list; it is recorded, not fixed, here. Second axis: `RemediationHint { Substantive, Absent, NotApplicable }`. §11.7 requires model-visible failures to carry a *non-empty* remediation hint, and today most do not — `generic_failure_recovery` hardcodes `RespectFailureConstraint` with an always-empty `repairs` vec for every capability kind but one, and denials reach the loop with `model_observation: None`. That is epic item 4 and a separate PR; this records it as a ratchet instead, so the item's progress is measurable: CapabilityFailureKind 11 of 12 model-visible kinds hint-less RuntimeFailureKind 10 of 11 RuntimeDispatchErrorKind 14 of 15 CapabilityErrorClass 2 of 3 ModelErrorClass 0 of 1 (ContentFiltered already complies) AgentLoopHostErrorKind 0 of 1 model stage; capability stage has no model-visible row to owe one Every terminal and hint-less count is pinned in a test whose comment says the number may only go DOWN. Arms whose fate is ambiguous or budget-dependent carry `// AUDIT:` notes: `BudgetExceeded` overloading spend-budget with context overflow, `BudgetApprovalRequired` parking only when a `gate_ref` is present, availability exhaustion aborting with no observation, `GateDeclined` being a declined gate rather than a pending one, `NetworkDenied` retrying a policy refusal, and the substantive-hint path being detail-driven rather than kind-driven. Three drift fixes the classification exposed: - `strategies/recovery.rs` — delete `_ => Abort { DriverBug }`. It was reachable only because a guard arm above it defeated exhaustiveness, so any newly added `CapabilityErrorClass` silently became run-terminal — the inverse of the invariant, in the file that implements it. The guard's single-use predicate is inlined as explicit variant arms; behavior for every existing class is unchanged and pinned by a new test over all seven. - `runner/text_loop_driver.rs` — delete the private `loop_failure_kind_name` copy with its `_ => "driver_bug"` catch-all (already swallowing `CompactionUnavailable`) and call `LoopFailureKind::as_str()`, which is string-identical for every arm the local fn listed. All call sites pass literals, so no behavior changes; a new test pins that only `DriverBug` may render as `driver_bug`. - `host_runtime/production.rs` — `capability_failure_disposition_maps_ failure_kinds_once` was a hand-written array of 16 rows for 17 variants, missing `GateDeclined`. Restructured to an exhaustive match over a shared variant list so it cannot drift again. §11.7 places this matrix in the authorize/dispatch conformance suite. That suite does not exist yet — the workspace has `ProductSurface` and channel conformance harnesses but nothing for authorize/dispatch — so the rows stay beside the enums they classify, which is also where the compile-forcing has to live. Consolidating them under that suite is whoever stands it up. Conformance tests are the classifiers' only consumer today, by design — `LoopProgressEvent::FailureRecovered` (item 7, later PR) becomes the production one. Each is annotated so it is not deleted as dead code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
github-actions
Bot
added
size: XL
📝 WalkthroughWalkthroughThis PR adds shared recoverability and remediation classifications, exposes exhaustive mappings across host, turn, and agent-loop error types, removes a recovery wildcard fallback, strengthens ratchet-based conformance tests, and standardizes runner error reason-kind serialization. ChangesRecoverability classification contract
Estimated code review effort: 4 (Complex) | ~60 minutes 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 |
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ❌ Changes requested | 2 | 0 | 2 | ae585179bd1b |
Head: ae585179bd1b9db202e87423fb289616e927ea04
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
The new conformance matrix cannot accurately classify two payload-dependent outcomes from enum kinds alone, so its ratchet counts can conceal real terminal and hint-less paths.
Findings
Blocking: 2 / Notes: 0
Blocking findings
1. ❌ [MEDIUM] Budget approval without gate evidence is misclassified as parked
Location: crates/ironclaw_turns/src/run_profile/host/error.rs:107
BudgetApprovalRequired is only parked when the error carries gate_ref. Without it, the model stage falls through to HostUnavailableWithDiagnostics and is terminal (already covered by model_budget_approval_required_without_gate_ref_fails_diagnostics_not_recovery). Classifying the kind alone as Park makes the conformance test and the 10-terminal ratchet miss this live terminal path. Make the classification depend on the error instance/gate state, or represent and test the conditional outcome.
2. ❌ [MEDIUM] Remediation-hint ratchets treat optional repairs as guaranteed
Location: crates/ironclaw_turns/src/run_profile/host/capability.rs:661
InvalidInput does not guarantee a substantive repair: existing capability-port paths create this kind with detail: None, and the observation renderer then takes the generic branch with empty repairs. Marking every InvalidInput as Substantive therefore undercounts hint-less observations; the same optimistic result propagates to the runtime, dispatch, and capability-class ratchets. Base this axis on the actual diagnostic/issues (or model the conditional state) and cover both structured and detail-less failures.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas.
| // AUDIT: only when the error actually carries a `gate_ref`. Without | ||
| // one it falls through to the unclassified path and becomes | ||
| // terminal — the parked outcome is not structurally guaranteed. | ||
| Self::BudgetApprovalRequired => RecoverabilityClass::Park, |
There was a problem hiding this comment.
BudgetApprovalRequired cannot be classified from the kind alone. The model stage parks only with gate_ref; the existing no-gate test drives the same kind into HostUnavailableWithDiagnostics (terminal). This matrix will therefore miss a live terminal path. Classify the error instance/gate state, or explicitly model and test the conditional outcome.
| /// §5.3.4's "`Denied` carries what would unlock the call". | ||
| pub const fn remediation_hint(&self) -> RemediationHint { | ||
| match self { | ||
| Self::InvalidInput => RemediationHint::Substantive, |
There was a problem hiding this comment.
InvalidInput is not always substantive: capability-port paths create it with detail: None, and the renderer then emits generic recovery with empty repairs. This makes the hint-less ratchet optimistic and propagates through the derived classifiers. Classify from the actual diagnostic/issues, or represent the conditional state and test both paths.
|
🚅 Deployed to the ironclaw-pr-6677 environment in ironclaw-ci-preview
|
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.59% — 307515 / 359273 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
left a comment
serrrfirat
left a comment
There was a problem hiding this comment.
Code Review (multi-agent)
Intent: Add compile-forced recoverability conformance classifiers and ratchet tests that document current behavior across seven Reborn error enums.
Mode: normal — preflight found an XL but non-stacked, hand-written Rust diff across 11 files, with no generated/vendor or mechanical-move component.
Coverage: GitHub unified diff; 10 production files and 1 test file. diff_truncated=true in reviewer packets, but all reviewers had the complete isolated PR worktree and repository rules available.
Stats: 11 findings from 13 raw findings after confidence filtering and overlap deduplication, across 7 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 0.
Review event: COMMENT — self-review by @serrrfirat; findings are advisory and cannot block the PR.
Bugs
- Medium Budget approval without a gate is incorrectly classified as parked (
crates/ironclaw_turns/src/run_profile/host/error.rs:107, confidence 99) — anchor:crates/ironclaw_turns/src/run_profile/host/error.rs:107
BudgetApprovalRequiredonly parks whengate_refexists; without one the live path terminates, so the matrix and terminal ratchet disagree with current behavior.
Tests
-
Medium Remediation test does not pin which capability class has the hint (
crates/ironclaw_agent_loop/src/strategies/recovery.rs:173-180, confidence 100) — anchor:crates/ironclaw_agent_loop/src/strategies/recovery.rs:173
Aggregate applicability/count assertions allow per-variant hint assignments to swap while the suite stays green. -
Medium Dispatch hint ratchet does not pin InputEncode membership (
crates/ironclaw_host_api/src/dispatch.rs:406-432, confidence 100) — anchor:crates/ironclaw_host_api/src/dispatch.rs:406
Aggregate assertions allow the per-kind hint matrix to drift. Also flagged by Bugs and Maintainability because hint quality depends on payload detail, so bareInputEncodefailures are not always substantive. -
Medium Capability hint test pins only the aggregate count (
crates/ironclaw_turns/src/run_profile/host/capability.rs:659-682, confidence 100) — anchor:crates/ironclaw_turns/src/run_profile/host/capability.rs:659
The suite does not prove thatInvalidInput, rather than another model-visible variant, owns the one substantive assignment. -
Medium Sanctioned terminal identities are not behavior-locked (
crates/ironclaw_turns/src/loop_exit.rs:586-600, confidence 100) — anchor:crates/ironclaw_turns/src/loop_exit.rs:586
Counting two sanctioned variants does not pin which two variants are sanctioned.
Conventions
- Medium Provider categories are omitted from the promised conformance matrix (
crates/ironclaw_host_api/src/recoverability.rs:12-18, confidence 95) — anchor:AGENTS.md:171
The module promises exhaustive provider-category coverage, but provider-facingLlmErrorhas no corresponding compile-forced classifier. Enforce that row or soften the cross-layer guarantee.
Local patterns
- Low Comment claims a progress event that does not exist yet (
crates/ironclaw_turns/src/run_profile/host/error.rs:60-62, confidence 100) — anchor:crates/ironclaw_turns/src/run_profile/host/error.rs:60
LoopProgressEvent::FailureRecoveredis described as existing here but correctly described later as a future consumer.
Maintainability
-
Medium Retry policy now has two independent classifiers (
crates/ironclaw_host_runtime/src/lib.rs:670-705, confidence 94) — anchor:crates/ironclaw_host_runtime/src/lib.rs:670
The new conformance classifier duplicates the production retry/disposition partition, allowing the two policy tables to drift independently. -
Low The loop-exit classifier is a vacuous second match (
crates/ironclaw_turns/src/loop_exit.rs:545-577, confidence 91) — anchor:crates/ironclaw_turns/src/loop_exit.rs:545
EveryLoopFailureKindnecessarily maps toTerminal; the adjacent sanctioned-terminal classifier already carries the meaningful exhaustive distinction. -
Low Inline conformance tests push the capability contract over 1,000 lines (
crates/ironclaw_turns/src/run_profile/host/capability.rs:816-824, confidence 87) — anchor:crates/ironclaw_turns/src/run_profile/host/capability.rs:816
Move the large inline test module to a sibling file while leaving the exhaustive production classifier beside its enum.
Approach
- Medium Best-case classification hides terminal retry outcomes (
crates/ironclaw_host_api/src/recoverability.rs:73-81, confidence 90) — anchor:crates/ironclaw_host_api/src/recoverability.rs:77
A retryable error can still terminate after budget exhaustion, and a model-visible error can terminate after its observation is consumed. A state-aware disposition or caller-level ratchets would measure those real terminal outcomes.
Clean lenses
- Security: no findings.
- Performance/concurrency: no findings.
| /// | ||
| /// Derived from the live path: | ||
| /// `ironclaw_agent_loop::executor::model` handles `Cancelled` and | ||
| /// gate-shaped `BudgetApprovalRequired` structurally, asks |
There was a problem hiding this comment.
Medium — Budget approval without a gate is incorrectly classified as parked.
BudgetApprovalRequired only parks when AgentLoopHostError.gate_ref is present. Production can emit this kind without a gate, after which the executor returns terminal HostUnavailableWithDiagnostics. The unconditional Park classification makes the matrix and terminal ratchet disagree with current behavior.
Fix: Classify the full error or gate-presence state, returning Park only with a gate ref and Terminal otherwise; cover both cases.
| // arch-exempt: large_file, keep recovery policy beside its exhaustive mapping tests until the item-7 conformance-matrix extraction, plan #6284 | ||
|
|
||
| use async_trait::async_trait; | ||
| use ironclaw_host_api::{RecoverabilityClass, RemediationHint}; |
There was a problem hiding this comment.
Medium — Remediation test does not pin which capability class has the hint.
The tests assert only applicability and an aggregate hintless count. Swapping InputInvalid to Absent and OperationFailed to Substantive preserves both assertions, so the claimed per-variant matrix can drift without failing.
Fix: Add every_capability_error_class_has_a_recorded_remediation_hint with an exhaustive expected hint for every CapabilityErrorClass variant.
| /// Exists so the §11.7 recoverability matrix can be asserted over the whole | ||
| /// enum from crates that cannot write an exhaustive `match` arm list of | ||
| /// their own without duplicating it (`ironclaw_host_runtime` pins that the | ||
| /// dispatch→runtime fold preserves each kind's class). Kept beside the |
There was a problem hiding this comment.
Medium — Dispatch hint ratchet does not pin InputEncode membership.
The conformance test checks hint applicability and only one aggregate substantive count. Swapping InputEncode with another model-visible kind preserves those assertions, so the per-kind matrix can drift while tests remain green.
Fix: Add an exhaustive exact remediation-hint expectation for every RuntimeDispatchErrorKind; because hint quality depends on failure detail, represent and test that conditional state rather than treating every InputEncode as substantive.
Also flagged by: bugs/Medium, maintainability/Medium
| CapabilityFailureKindValue::new(value).map(Self::Unknown) | ||
| } | ||
|
|
||
| /// Which §5.3.4 cell this capability failure lands in today. See |
There was a problem hiding this comment.
Medium — Capability hint test pins only the aggregate count.
The tests prove that model-visible variants have applicable hints and exactly one is substantive, but not that InvalidInput is that variant. Exchanging assignments preserves both assertions.
Fix: Add every_capability_failure_kind_has_a_recorded_remediation_hint with an exhaustive expected hint for every variant, including Unknown.
| /// sentence ("maps to retry / a model-visible observation / a park — never | ||
| /// an unclassified terminal bork") cannot hold here as written; what it can | ||
| /// hold, and what this classifier records, is *which* terminal exits are | ||
| /// one of the three sanctioned invariants and which are the defect surface |
There was a problem hiding this comment.
Medium — Sanctioned terminal identities are not behavior-locked.
The test only counts two sanctioned variants. Swapping one sanctioned identity with an unsanctioned variant leaves the count unchanged, so the contract can drift undetected.
Fix: Add every_loop_failure_kind_has_the_expected_terminal_sanction with an exhaustive expected boolean for every LoopFailureKind.
| /// model stage `Unavailable` is retried twelve times, while on the | ||
| /// capability stage it kills the run outright. So there are two | ||
| /// classifiers, and | ||
| /// [`Self::capability_stage_recoverability_class`] is the other one. (The epic's |
There was a problem hiding this comment.
Low — Comment claims a progress event that does not exist yet.
The comment says LoopProgressEvent::FailureRecovered already carries a stage field, but that variant does not exist yet. The same file later correctly calls it a future consumer.
Fix: Describe the event as planned by the design, or remove the parenthetical until FailureRecovered exists.
| /// names — they are different enums on different layers, and both are real. | ||
| /// | ||
| /// Derived from the live path, not from intent: | ||
| /// [`capability_failure_disposition`] picks `RetrySameCall` or |
There was a problem hiding this comment.
Medium — Retry policy now has two independent classifiers.
RuntimeFailureKind::recoverability_class() repeats the retry-versus-model-visible partition already owned by capability_failure_disposition() and runtime_failure_is_retryable(). Independent handwritten tests do not prevent the production policy and conformance matrix from diverging.
Fix: Derive capability_failure_disposition() from recoverability_class() and remove the duplicate retryability classifier so one exhaustive table owns the policy.
| } | ||
|
|
||
| impl LoopFailureKind { | ||
| /// Every variant, in declaration order. Kept beside the exhaustive matches |
There was a problem hiding this comment.
Low — The loop-exit classifier is a vacuous second match.
Every LoopFailureKind necessarily returns Terminal, while the adjacent sanctioned-terminal classifier already supplies the compile-forced distinction. The additional public method and 13-arm match encode no information.
Fix: Delete LoopFailureKind::recoverability_class() and its all-terminal test; retain the exhaustive sanctioned-terminal classifier and ratchet.
| use super::*; | ||
|
|
||
| /// Every `CapabilityFailureKind` variant, including one `Unknown` value for | ||
| /// the open-set escape hatch. The exhaustive `match` in the conformance |
There was a problem hiding this comment.
Low — Inline conformance tests push the capability contract over 1,000 lines.
The file grows from roughly 770 to 1,002 lines, with the new inline test module accounting for most of the increase and mixing the contract, serialization, port trait, helpers, and matrix harness.
Fix: Move the #[cfg(test)] module into a focused sibling test file while keeping the exhaustive classifier itself beside the enum.
| /// Which §5.3.4 cell a failure kind lands in — the four outcomes §11.7's | ||
| /// matrix admits. | ||
| /// | ||
| /// Deliberately **not** `#[non_exhaustive]`: this enum is the classification |
There was a problem hiding this comment.
Medium — Best-case classification hides terminal retry outcomes.
The matrix records only the first/best outcome. Retryable errors can still terminate after retry-budget exhaustion, and model-visible errors can terminate after their observation is consumed. Terminal ratchets therefore omit real run-ending paths despite the stated goal of recording current behavior.
Fix: Represent exhausted/repeated outcomes in the disposition, or ratchet the real recovery caller at fresh, exhausted, and post-observation states separately.
|
Closing in favor of a structural collapse rather than per-crate classifiers. Investigation after this PR went up (six read-only audits, summarized on #6284) found the five failure-kind enums are ~84 slots describing ~33 real situations — two are byte-identical 19/19, one is a pure identity subset, and the drift between them is the root cause of both the wrongful-terminal sites (item 1) and the hint-less model errors (item 4). The replacement: one closed enum in ironclaw_host_api with projection functions (fate, retry bucket, wire tag, HTTP status), so every classification is compile-forced beside a single definition instead of ratcheted across seven. The baseline counts this PR pinned informed that design; the per-crate matrices it added would be deleted by it. Superseding PR to follow on this branch's successor. |
This branch was successfully deployed
1 participant
Summary
RecoverabilityClass { Retry, ModelVisible, Park, Terminal }(crates/ironclaw_host_api/src/recoverability.rs) and an exhaustive, no-wildcard classifier for seven error enums, implementing the §11.7 recoverability conformance matrix fromdocs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md(§5.3.4, [EPIC] error-recoverability endgame — the model recovers from 100% of the errors it sees #6284 item 7). A new variant is now a compile error until someone classifies it.RemediationHint { Substantive, Absent, NotApplicable }axis, so "the model saw the error" and "the model was told how to fix it" stop being the same measurement ([EPIC] error-recoverability endgame — the model recovers from 100% of the errors it sees #6284 item 4 becomes measurable)._ => Abort { DriverBug }catch-all inon_capability_error(a newCapabilityErrorClasssilently became run-terminal — the exact inverse of the invariant, per §11.9's no-wildcard rule), and fixes two drifted case lists that were already swallowing variants.What the numbers say
The same failure question is asked in five different vocabularies. Classifying every enum exhaustively surfaced that the layered "failure kind" enums are not refinements of each other — three of them are the same list:
RuntimeDispatchErrorKindironclaw_host_apiFailureKindironclaw_host_apiCapabilityFailureKindironclaw_turnsFailureKind, 19/19RuntimeFailureKindironclaw_host_runtimeCapabilityErrorClassironclaw_agent_loopFailureKindandRuntimeDispatchErrorKindlive in the same crate.CapabilityFailureKinddiffers fromFailureKindin zero names.CapabilityFailureKind=RuntimeFailureKind+{Permanent, Unknown}.The first fold is where remediation dies. 17 of the 22 mechanism-precise names have no counterpart downstream:
Guest,Manifest,Memory,MethodMissing,UndeclaredCapability,InputEncode,OutputDecode,SecretDenied,FilesystemDenied,NetworkDenied,UnsupportedRunner,ExtensionRuntimeMismatch,ExitFailure,InvalidResult,Client,Executor,Unknown. Those are precisely the names that imply a fix. They collapse into generic buckets before anything can render advice from them — so the hint-less column below is a consequence of the duplication, not an independent gap. Relevant to #6284 item 4.Earlier reading retracted. An earlier version of this section read the 0/22 → 17/17 spread as recoverability being discarded on the way up. That was wrong: those enums answer different questions (what went wrong with this call vs. why did the run stop), and a stopped run is terminal by construction. See the retraction under Review status in #6284. The counts below are unaffected — this PR pins counts, not interpretations.
RuntimeDispatchErrorKindRuntimeFailureKindCapabilityFailureKindCapabilityErrorClassModelErrorClassAgentLoopHostErrorKind(model stage)AgentLoopHostErrorKind(capability stage)LoopFailureKindAgentLoopHostErrorKindis classified per stage deliberately: the same kind has opposite fates depending on where it is raised (Unavailable= 12 retries on the model stage, immediate run death on the capability stage). A stage-agnostic class would be a lie, and §5.3.4'sFailureRecovered { stage, .. }needs the stage anyway.LoopFailureKindis classified in-crate inironclaw_turns— it is#[non_exhaustive], so cross-crate exhaustiveness is impossible, but an in-crate match still compile-forces.Change Type
Refactor + test-infrastructure; maintainer-requested as PR 1 of the #6284 sequence.
Linked Issue
Related #6284 (item 7 — enforcement, deliberately sequenced first so items 1–6 land measurable rather than asserted). Implements §11.7 / §5.3.4 of the architecture-simplification design note (#6291).
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warnings— zero warningscargo build— covered; clippy compiled the full workspace target graphironclaw_host_api,ironclaw_turns,ironclaw_agent_loop,ironclaw_host_runtime,ironclaw_runner,ironclaw_architecturecargo test --features integration— Not applicable: no database or integration behavior changed; this PR adds no production call path.bash scripts/pre-commit-safety.sh— exit 0Stated plainly:
ironclaw_host_runtime::first_party_tools::trace_commons::tests::dispatch_profile_{set,token}_without_enrollment_returns_onboard_guidancefail locally withDispatch { kind: NetworkDenied }. Confirmed pre-existing by re-running on a cleanorigin/mainworktree — same two tests, same lines (trace_commons.rs:1841/:1858), same panic. That file is untouched here.Test Strategy
User behavior:
No user-visible behavior changes. This PR makes an existing invariant machine-checkable: a contributor adding an error variant cannot leave it unclassified, and a contributor who re-buckets an error away from recoverable must edit a ratchet count to do it.
Risk areas:
host_api,turns,host_runtime,agent_loopTests added or updated:
match(not hand-maintained arrays, which is how the two fixed case lists drifted); terminal-count and hint-less-count ratchets with "may only go DOWN" comments; a behavior lock over everyCapabilityErrorClass→RecoveryOutcomepinning the catch-all deletion as byte-identical;runtime_dispatch_error_kind_class_survives_the_runtime_foldprovingFrom<DispatchFailureKind> for RuntimeFailureKindpreserves the class.FailureRecoveredevent (a later PR) is the production consumer. Comments in the tests say so, so they are not deleted as dead code.What the tests prove: every variant of all seven enums has a deliberate recoverability class and hint status; a new variant fails the build; the
on_capability_errorrestructure changed no behavior; and the class survives the dispatch→runtime fold.Commands run:
cargo fmt;cargo clippy --all --benches --tests --examples --all-features;cargo test -p ironclaw_host_api -p ironclaw_turns -p ironclaw_agent_loop -p ironclaw_host_runtime -p ironclaw_runner -p ironclaw_architecture --no-fail-fast;bash scripts/pre-commit-safety.sh.Security Impact
None directly. Indirectly positive: the deleted
_ => Abort { DriverBug }was a fail-closed-by-accident wildcard that made any newly addedCapabilityErrorClassrun-terminal without review — §11.9 names exactly this pattern as a mechanical review reject. No permission, network, secret, file-access, tool-execution, or sandbox-policy behavior changes.Reborn Trust-Boundary Checklist
RecoverabilityClassandRemediationHintare inert plain enums with no authority and no constructor privilege; they classify, they do not permit.cargo clippy --all --benches --tests --examples --all-featurescompiles every match site in the workspace, and the new classifiers are exhaustive by construction.serde(default)fields: N/A — no serialized state added.Database Impact
None. No migrations, no schema, no backend-specific behavior.
Blast Radius
Touches
ironclaw_host_api,ironclaw_turns,ironclaw_host_runtime,ironclaw_agent_loop,ironclaw_runner. Additive except three deletions, each behavior-preserving and test-locked:on_capability_error's_ =>wildcard → explicit variant arms (single-use predicate inlined; all 7 existing classes verified byte-identical).text_loop_driver.rs's localloop_failure_kind_name→LoopFailureKind::as_str(), verified string-identical for every arm it listed, and additionally coveringCompactionUnavailable, which the local fn was silently rendering asdriver_bug. All 11 call sites pass literals, so the wildcard was unreachable in practice.production.rs's hand-written case array (16 rows for 17 variants —GateDeclinedwas missing) → an exhaustive match that cannot drift again.What could break: nothing at runtime. The realistic failure mode is a future PR being blocked by a ratchet — which is the intent.
Rollback Plan
git revertthe single commit. No migrations, no persisted state, no production call path; reverting restores the prior wildcards and case arrays exactly.Review Follow-Through
Known follow-ups, all deliberately not in this PR:
ProductSurface, channel, and auth harnesses but no authorize/dispatch one, so matrix rows live beside the enums they classify — which is also the only place compile-forcing can live, since each exhaustive match must sit in the crate that can add a variant. If reviewers want the suite stood up,crates/ironclaw_capabilities/tests/authorize_dispatch_conformance.rsmatches the existing pattern; this PR's classifiers become its recoverability section.MethodMissingandUndeclaredCapabilityfold ontoRuntimeFailureKind::InvalidInputalongsideInputEncode, but onlyInputEncodecarries structured issues — so "invalid input" arriving at the loop may or may not be repairable and the loop cannot tell. The consistency test asserts class equality but hint inequality (upstream may be more pessimistic, never more optimistic). Surfaced as a genuine test failure during the work. Relevant to [EPIC] error-recoverability endgame — the model recovers from 100% of the errors it sees #6284 item 4.LoopFailureKind— it names why aLoopExit::Failedalready happened, so every row is terminal by construction. It satisfies "never unclassified" instead, via a sanctioned/unsanctioned axis; 11 of 13 are unsanctioned. Reviewer judgment welcome on whether that reading is right.// AUDIT:comments mark four ambiguities found while tracing:BudgetExceededoverloads spend-budget with context overflow;BudgetApprovalRequiredparks only when agate_refis present; availability-budget exhaustion aborts with no observation;RuntimeDispatchErrorKind::NetworkDeniedcarries both a real fault and a policy refusal, and the policy half is retried, which retrying cannot fix.crates/ironclaw_runner/src/failure_lane.rshas a vacuousfailure_lane()(it ignores itscategoryargument). Deliberately out of scope — that module has zero production callers and belongs with the wire-or-delete decision in [EPIC] error-recoverability endgame — the model recovers from 100% of the errors it sees #6284 item 7.Review track: B (maintainer-requested refactor; PR 1 of the #6284 sequence)
🤖 Generated with Claude Code