Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,6 @@ fn generic_failure_recovery(error_kind: &CapabilityFailureKind) -> ToolRecoveryO
| CapabilityFailureKind::Resource
| CapabilityFailureKind::Internal
| CapabilityFailureKind::Unknown(_) => SameCallRetryConstraint::Allowed,
_ => SameCallRetryConstraint::Allowed,
};
ToolRecoveryObservation {
same_call_retry,
Expand Down
106 changes: 78 additions & 28 deletions crates/ironclaw_agent_loop/src/executor/mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,6 @@ pub(super) fn capability_error_class(kind: &CapabilityFailureKind) -> Capability
CapabilityFailureKind::Cancelled | CapabilityFailureKind::Permanent => {
CapabilityErrorClass::Permanent
}
// CapabilityFailureKind is #[non_exhaustive]. Treat unrecognised future
// variants as recoverable (OperationFailed) to match the host_runtime
// disposition layer's recoverable default: by design no capability
// failure should abort the run, so an unknown kind becomes a
// model-visible tool error rather than killing the run.
&_ => CapabilityErrorClass::OperationFailed,
}
}

Expand All @@ -183,7 +177,25 @@ pub(super) fn capability_failure_kind(kind: &CapabilityFailureKind) -> LoopFailu
CapabilityFailureKind::Authorization
| CapabilityFailureKind::GateDeclined
| CapabilityFailureKind::PolicyDenied => LoopFailureKind::PolicyDenied,
_ => LoopFailureKind::CapabilityProtocolError,
// Every remaining kind maps to the protocol-error failure. Enumerated
// explicitly (no wildcard) so a new `CapabilityFailureKind` variant must
// be classified here deliberately rather than silently inheriting this
// terminal fate — `CapabilityFailureKind` is no longer `#[non_exhaustive]`.
CapabilityFailureKind::Backend
| CapabilityFailureKind::Cancelled
| CapabilityFailureKind::Dispatcher
| CapabilityFailureKind::InvalidOutput
| CapabilityFailureKind::MissingRuntime
| CapabilityFailureKind::Network
| CapabilityFailureKind::OperationFailed
| CapabilityFailureKind::OutputTooLarge
| CapabilityFailureKind::Process
| CapabilityFailureKind::Resource
| CapabilityFailureKind::Transient
| CapabilityFailureKind::Unavailable
| CapabilityFailureKind::Internal
| CapabilityFailureKind::Permanent
| CapabilityFailureKind::Unknown(_) => LoopFailureKind::CapabilityProtocolError,
}
}

Expand Down Expand Up @@ -269,28 +281,66 @@ mod tests {
);
}

/// Classification lock for `capability_error_class`: every
/// `CapabilityFailureKind` variant maps to a deliberate recovery class.
///
/// This complements the compile-time guarantee (the match is exhaustive with
/// no `_ =>` wildcard, since `CapabilityFailureKind` is no longer
/// `#[non_exhaustive]`, so a *new* variant fails to compile until classified)
/// by also catching a silent *re-bucketing* of an *existing* variant — e.g.
/// moving a recoverable kind into the run-aborting `Permanent` class, or vice
/// versa. Only the genuinely-terminal kinds (`Cancelled` and `Permanent`)
/// may map to `Permanent`; runtime-dispositioned tool failures such as
/// `Dispatcher`, `InvalidOutput`, and the open-set `Unknown` stay
/// model-visible. See
/// `docs/plans/2026-06-28-reborn-error-recoverability-audit.md` §6.1.
#[test]
fn model_recoverable_capability_failures_are_operation_failed_not_permanent() {
// The host_runtime disposition layer treats Dispatcher, InvalidOutput,
// and Unknown as model-visible (run-continuing) errors. The recovery
// class mapping must agree: these become OperationFailed (a
// model-visible tool error) rather than Permanent (which aborts the
// run). E.g. the model calling a nonexistent tool becomes a recoverable
// tool error, not a run-ending protocol failure.
assert_eq!(
capability_error_class(&CapabilityFailureKind::Dispatcher),
CapabilityErrorClass::OperationFailed
);
assert_eq!(
capability_error_class(&CapabilityFailureKind::InvalidOutput),
CapabilityErrorClass::OperationFailed
);
let unknown = CapabilityFailureKind::unknown("some_future_kind".to_string())
.expect("valid unknown kind");
assert_eq!(
capability_error_class(&unknown),
CapabilityErrorClass::OperationFailed
);
fn every_capability_failure_kind_has_a_deliberate_recovery_class() {
use CapabilityErrorClass as C;
use CapabilityFailureKind as K;

let unknown = K::unknown("some_future_kind").expect("valid unknown kind");
let cases: &[(K, C)] = &[
(K::Network, C::Transient),
(K::Transient, C::Transient),
(K::Backend, C::Unavailable),
(K::Unavailable, C::Unavailable),
(K::InvalidInput, C::InputInvalid),
(K::MissingRuntime, C::OperationFailed),
(K::OperationFailed, C::OperationFailed),
(K::OutputTooLarge, C::OperationFailed),
(K::Process, C::OperationFailed),
(K::Resource, C::OperationFailed),
(K::Authorization, C::PolicyDenied),
(K::GateDeclined, C::PolicyDenied),
(K::PolicyDenied, C::PolicyDenied),
(K::Internal, C::Internal),
(K::Dispatcher, C::OperationFailed),
(K::Cancelled, C::Permanent),
(K::InvalidOutput, C::OperationFailed),
(K::Permanent, C::Permanent),
(unknown.clone(), C::OperationFailed),
];

for (kind, expected) in cases {
assert_eq!(
capability_error_class(kind),
*expected,
"recovery class for {kind:?} changed — re-confirm it is deliberate \
and does not silently abort a recoverable failure"
);
}

// Only these kinds may abort the run.
for (kind, class) in cases {
if *class == C::Permanent {
assert!(
matches!(kind, K::Cancelled | K::Permanent),
"{kind:?} maps to the run-aborting Permanent class but is not a \
recognized terminal kind — a recoverable failure must not abort"
);
}
}
}

#[test]
Expand Down
10 changes: 7 additions & 3 deletions crates/ironclaw_host_runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -695,8 +695,14 @@ mod raw_http_diagnostic_policy_tests {
}

/// Stable, sanitized failure categories.
///
// Deliberately NOT `#[non_exhaustive]`: the `Unknown` variant is the open-set
// escape hatch for unrecognized runtime failures, so the attribute would only
// force classifiers to keep a wildcard arm that silently buckets a new named
// variant. Without it, disposition/classification matches are exhaustive and a
// new named variant fails to compile until classified. See
// `docs/plans/2026-06-28-reborn-error-recoverability-audit.md` §6.1.
Comment on lines 697 to +704
Comment on lines +698 to +704

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Comment contradicts the enum it documents. RuntimeFailureKind no longer has an Unknown variant (this PR removed it), yet the block asserts "the Unknown variant is the open-set escape hatch for unrecognized runtime failures." That rationale belongs to CapabilityFailureKind, which keeps Unknown(_); here it's just wrong and will mislead the next reader. The valid reason for dropping #[non_exhaustive] is only the "matches stay exhaustive, new variants fail to compile until classified" clause.

📝 Suggested rewrite
-///
-// Deliberately NOT `#[non_exhaustive]`: the `Unknown` variant is the open-set
-// escape hatch for unrecognized runtime failures, so the attribute would only
-// force classifiers to keep a wildcard arm that silently buckets a new named
-// variant. Without it, disposition/classification matches are exhaustive and a
-// new named variant fails to compile until classified. See
-// `docs/plans/2026-06-28-reborn-error-recoverability-audit.md` §6.1.
+///
+// Deliberately NOT `#[non_exhaustive]` and intentionally has no open-set
+// `Unknown` variant: uncategorized dispatch errors collapse to `Internal`.
+// Without the attribute, disposition/classification matches stay exhaustive and
+// a new named variant fails to compile until it is explicitly classified. See
+// `docs/plans/2026-06-28-reborn-error-recoverability-audit.md` §6.1.

As per coding guidelines: "Add comments only for non-obvious logic in Rust code."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
///
// Deliberately NOT `#[non_exhaustive]`: the `Unknown` variant is the open-set
// escape hatch for unrecognized runtime failures, so the attribute would only
// force classifiers to keep a wildcard arm that silently buckets a new named
// variant. Without it, disposition/classification matches are exhaustive and a
// new named variant fails to compile until classified. See
// `docs/plans/2026-06-28-reborn-error-recoverability-audit.md` §6.1.
///
// Deliberately NOT `#[non_exhaustive]` and intentionally has no open-set
// `Unknown` variant: uncategorized dispatch errors collapse to `Internal`.
// Without the attribute, disposition/classification matches stay exhaustive and
// a new named variant fails to compile until it is explicitly classified. See
// `docs/plans/2026-06-28-reborn-error-recoverability-audit.md` §6.1.
🤖 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_host_runtime/src/lib.rs` around lines 697 - 703, Update the
comment attached to RuntimeFailureKind so it no longer references an Unknown
variant that was removed; the current rationale is incorrect and belongs to
CapabilityFailureKind. Keep only the valid explanation for omitting
#[non_exhaustive] in this enum, using the RuntimeFailureKind symbol to locate
the block, and ensure the comment reflects that matches remain exhaustive and
new variants fail to compile until classified. Also consider removing the
comment entirely if the remaining behavior is obvious enough to satisfy the Rust
commenting guideline.

Source: Coding guidelines

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
#[non_exhaustive]
pub enum RuntimeFailureKind {
Authorization,
Backend,
Expand All @@ -714,7 +720,6 @@ pub enum RuntimeFailureKind {
Resource,
Transient,
Unavailable,
Unknown,
}

impl RuntimeFailureKind {
Expand All @@ -737,7 +742,6 @@ impl RuntimeFailureKind {
Self::Resource => "resource",
Self::Transient => "transient",
Self::Unavailable => "unavailable",
Self::Unknown => "unknown",
}
}
}
Expand Down
15 changes: 11 additions & 4 deletions crates/ironclaw_host_runtime/src/production.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2365,7 +2365,13 @@ impl From<DispatchFailureKind> for RuntimeFailureKind {
| DispatchFailureKind::Runtime(RuntimeDispatchErrorKind::UnsupportedRunner) => {
RuntimeFailureKind::Backend
}
DispatchFailureKind::Runtime(RuntimeDispatchErrorKind::Unknown) => Self::Unknown,
// The fail-safe "uncategorized" redaction bucket collapses to a
// concrete internal failure rather than propagating a dedicated
// `Unknown` category downstream. `Internal` is retryable and
// surfaces to the model/user, so an unclassified dispatch error is
// no longer an opaque run-ending dead-end. See
// `docs/plans/2026-06-28-reborn-error-recoverability-audit.md`.
DispatchFailureKind::Runtime(RuntimeDispatchErrorKind::Unknown) => Self::Internal,
}
}
}
Expand Down Expand Up @@ -2591,9 +2597,12 @@ output_schema_ref = "schemas/test.output.json"
RuntimeDispatchErrorKind::UnsupportedRunner,
RuntimeFailureKind::Backend,
),
// The fail-safe "uncategorized" redaction bucket collapses to a
// concrete, surfacing `Internal` rather than a dedicated `Unknown`
// category (which no longer exists on `RuntimeFailureKind`).
(
RuntimeDispatchErrorKind::Unknown,
RuntimeFailureKind::Unknown,
RuntimeFailureKind::Internal,
),
];
for (variant, expected) in cases {
Expand Down Expand Up @@ -2845,7 +2854,6 @@ output_schema_ref = "schemas/test.output.json"
assert_eq!(RuntimeFailureKind::Resource.as_str(), "resource");
assert_eq!(RuntimeFailureKind::Transient.as_str(), "transient");
assert_eq!(RuntimeFailureKind::Unavailable.as_str(), "unavailable");
assert_eq!(RuntimeFailureKind::Unknown.as_str(), "unknown");
}

#[test]
Expand All @@ -2869,7 +2877,6 @@ output_schema_ref = "schemas/test.output.json"
(RuntimeFailureKind::Resource, ModelVisibleToolError),
(RuntimeFailureKind::Transient, RetrySameCall),
(RuntimeFailureKind::Unavailable, RetrySameCall),
(RuntimeFailureKind::Unknown, ModelVisibleToolError),
];

for (kind, expected) in cases {
Expand Down
9 changes: 0 additions & 9 deletions crates/ironclaw_loop_support/src/capability_port.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2852,8 +2852,6 @@ fn runtime_failure_kind_to_loop(
RuntimeFailureKind::Resource => CapabilityFailureKind::Resource,
RuntimeFailureKind::Transient => CapabilityFailureKind::Transient,
RuntimeFailureKind::Unavailable => CapabilityFailureKind::Unavailable,
RuntimeFailureKind::Unknown => capability_failure_kind("unknown")?,
_ => capability_failure_kind(kind.as_str())?,
})
}

Expand Down Expand Up @@ -3307,13 +3305,6 @@ mod tests {
"{runtime:?}"
);
}

assert_eq!(
runtime_failure_kind_to_loop(RuntimeFailureKind::Unknown)
.expect("unknown failure kind")
.as_str(),
"unknown"
);
}

#[test]
Expand Down
12 changes: 11 additions & 1 deletion crates/ironclaw_turns/src/run_profile/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1965,7 +1965,17 @@ pub struct CapabilityFailure {
pub detail: Option<CapabilityFailureDetail>,
}

#[non_exhaustive]
// Deliberately NOT `#[non_exhaustive]`: the `Unknown(CapabilityFailureKindValue)`
// variant is the forward-compat / open-set escape hatch (a newer producer's
// unrecognized wire string deserializes into `Unknown`), and the manual
// `Serialize`/`Deserialize` impls below route every value through `as_str()` /
// that variant. Leaving the attribute on would force callers — notably the
// recovery classifier `capability_error_class` — to keep a wildcard `_ =>` arm,
// which silently buckets any newly-added *named* variant (e.g. a future
// `QuotaExceeded`) into a run-aborting class. Without the attribute, those
// classifiers match exhaustively, so a new named variant fails to compile until
// it is deliberately classified. See
// `docs/plans/2026-06-28-reborn-error-recoverability-audit.md` §6.1.
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum CapabilityFailureKind {
Authorization,
Expand Down
Loading