arch(ws-2): strategy traits beta - batch, gate, recovery

[henrypark133]

Context

Second strategy-trait slice. It covers decisions around capability batch execution, gate handling, and error recovery while still leaving concrete execution to the canonical executor and host ports.

Master spec: docs/reborn/agent-loop-skeleton.md
Workstream brief: docs/reborn/agent-loop-briefs/strategy-traits-beta.md
Stack base: arch/ws-0

Latest stack maintenance on 2026-05-14:

• Rebased this branch onto its current stack base after the WS0 prompt-authority fix and the follow-up WS8/WS14-parent/WS16/WS17 conflict resolutions.
• Pushed the updated branch with force-with-lease where the remote already existed, or published it as a new branch where it did not.
• Verified the final ancestry chain from origin/reborn-integration through WS17 before publishing the PR descriptions.

What landed

• BatchPolicyStrategy plus sequential/parallel batch-policy values derived from the capability view.
• GateHandlingStrategy and gate outcomes that let the executor block, skip, or abort through the normal LoopExit path.
• RecoveryStrategy and recovery outcomes for transient model/capability failures.
• Default beta strategy implementations with bounded retry/block behavior and no direct mutation of runner state.
• Integration with WS0 state slots so gate/recovery updates remain value-immutable and strategy-owned.

Reviewer focus

• Gate and recovery outcomes must return state transitions to the executor; they should not apply durable run-state changes directly.
• Batch-policy decisions should use ConcurrencyHint and visible descriptor metadata, not raw lower-layer capability internals.
• Policy-denied and no-progress failure kinds must flow through sanitized failure surfaces.

Non-goals / deferred work

• Actual capability invocation is WS6a plus WS9.
• Checkpoint-on-block semantics are enforced in the executor, not here.
• Product runtime cutover is WS16/WS17.

Validation

• Branch rebased onto the current WS0 after conflict resolution in the strategy module.
• Included in arch/level1-merged validation: cargo check -p ironclaw_agent_loop -p ironclaw_reborn.
• Included in the final stack ancestry verification.

Stack position

[#3550 ws-0] state/checkpoint foundation -> reborn-integration
   |-- #3551 ws-1 strategy alpha -> ws-0
   |-- #3552 ws-2 strategy beta -> ws-0
   |-- #3553 ws-3 strategy gamma -> ws-0
   |-- #3643 ws-3.5 loop family registry -> ws-0
   '-- #3554 level1-merged -> ws-0
         |-- #3555 ws-4 planner facade -> level1
         |-- #3556 ws-5 default strategies -> level1
         '-- #3557 level2-merged -> level1
               '-- #3596 ws-6a canonical executor -> level2
                     '-- #3597 ws-7 PlannedDriver adapter -> ws-6a
                           '-- #3598 ws-8 integration/test support -> ws-7
                                 |-- #3644 ws-9 capability host wiring -> ws-8
                                 |-- #3645 ws-10 checkpoint load/resume -> ws-8
                                 |-- #3646 ws-11 input port -> ws-8
                                 |-- #3647 ws-12 progress port -> ws-8
                                 |-- #3648 ws-13 cancellation accessor -> ws-8
                                 |-- #3649 ws-15 prompt/identity context -> ws-8
                                 '-- #3650 ws-14-parent integrated host ports -> ws-8
                                       '-- #3651 ws-14 planned default registration -> ws-14-parent
                                             '-- #3652 ws-16 live runtime wiring -> ws-14
                                                   '-- #3653 ws-17 product live cutover -> ws-16

[gemini-code-assist]

Code Review

This pull request introduces the strategy trait contracts for the agent loop, defining the core interfaces and data structures for batch policy, gate handling, and error recovery. The implementation includes comprehensive serialization tests for all new enums and structs. Review feedback suggests consolidating 5xx-like error variants in the CapabilityErrorClass and ModelErrorClass enums to simplify retry logic and ensure consistent recovery semantics. Additionally, it is recommended to replace std::time::Duration with a primitive u64 for backoff delays to improve the stability and compatibility of the serialized wire format for non-Rust consumers.

[gemini-code-assist]

The CapabilityErrorClass enum splits 5xx-like errors into Transient, Unavailable, and Internal. According to the general rules, all HTTP 5xx errors should be grouped into a single variant (e.g., Unavailable) because they share the same recovery semantics (retry with backoff). This simplifies the retry logic and ensures consistency across the system. Specific error details can still be preserved in the safe_summary or via the diagnostic_ref for debugging.

Suggested change

	pub enum CapabilityErrorClass {
	Transient,
	Permanent,
	InputInvalid,
	PolicyDenied,
	Unavailable,
	Internal,
	}
	pub enum CapabilityErrorClass {
	/// Transient or 5xx-class error (BadGateway, ServiceUnavailable, etc).
	/// Shared recovery semantics: retry with backoff.
	Unavailable,
	Permanent,
	InputInvalid,
	PolicyDenied,
	}

References

1. Group all HTTP 5xx errors into a single 'BadGateway' or 'temporarily unavailable' error variant to maintain consistent recovery semantics.

[gemini-code-assist]

Similar to CapabilityErrorClass, the ModelErrorClass enum should group 5xx-like errors (Transient, Unavailable, Internal) into a single variant to align with the general rule regarding HTTP 5xx error handling and recovery semantics.

pub enum ModelErrorClass {
    /// Transient or 5xx-class error. Shared recovery semantics: retry with backoff.
    Unavailable,
    ContextOverflow,
    ContentFiltered,
}

References

1. Group all HTTP 5xx errors into a single 'BadGateway' or 'temporarily unavailable' error variant to maintain consistent recovery semantics.

[serrrfirat]

Summary

Reviewed WS2 PR #3552 only.
Base 3db2d97bf1718554c8936e2ad4346794f02b7ec3 → head 9df56cdec4237b37a670acfd6cff14e05d3ffe68.

PR adds crate-internal strategy traits beta: batch, gate, recovery. Merge stance for reviewed scope: no blocking security/correctness findings.

Tests run:

• cargo test -p ironclaw_agent_loop --lib ✅
• cargo check -p ironclaw_agent_loop -p ironclaw_reborn ✅

Findings

#	Sev	Category	File:Line	Issue	Fix suggestion
—	—	—	—	No actionable security/correctness findings in reviewed scope.	—

Security/data-flow notes

• Gate/recovery strategies return outcome values carrying strategy-owned slot state only; no durable run-state writes added.
• Batch policy consumes CapabilityCallSummary with capability id + ConcurrencyHint; no raw tool inputs or lower-layer capability internals exposed.
• Recovery error surfaces carry sanitized class, safe_summary, and optional opaque diagnostic refs only.
• PolicyDenied / NoProgressDetected remain typed failure-kind values; no raw reason strings added to strategy state.
• New strategy modules remain pub(crate).

Correctness/invariant notes

• GateOutcome::{Block, SkipAndContinue, Abort} carry GateStrategyState as intended.
• RecoveryOutcome::{Retry, SkipResult, Abort} carry RecoveryStrategyState as intended.
• RetryAlteration::{ShrinkContext, Backoff, AdvanceFallback} round-trip; docs require executor reject AdvanceFallback until route-chain support lands.
• Batch policy/object-safety/serde tests present.

Missing tests

• None required for changed behavior beyond existing compile/default/serde/slot-carrying tests.

Suggested fixes

1. None.

[serrrfirat]

Security/correctness review: PR #3552

Reviewed current PR stack state for #3552 (arch(ws-2): strategy traits beta - batch, gate, recovery).

v2 impact verdict: Clean in reviewed scope. Internal trait-contract landing only. No direct behavior change or durable side-effect path added in changed files.

Findings

No actionable security/correctness findings in requested scope.

Checked categories

• Gate/recovery outcome state transitions
• No durable side effects in strategy contracts
• Batch policy uses ConcurrencyHint, not raw internals
• Sanitized error surfaces only (safe_summary, opaque refs)
• Policy-denied / no-progress contract shape
• pub(crate) visibility boundaries
• Wire-stable serde on outcome/error enums used in checkpoints/events
• Tests for round-trip/object safety/slot-carrying shape

Validation run

• cargo test -p ironclaw_agent_loop --lib ✅

[zmanian]

Review notes — WS2 strategy traits β

Two items worth resolving.

Tests reference LoopFailureKind::{NoProgressDetected, PolicyDenied}

Both variants are referenced in recovery.rs / gate.rs tests. On staging's loop_exit.rs neither variant exists — they're added in WS0 (#3550). When this PR sits on the stacked base everything should compile, but it's worth a quick sanity check that the dev branch you build against actually carries WS0; otherwise tests fail in isolation.

Sync vs async trait-style inconsistency with WS1

WS1 made all three α traits async-trait even when pure policy (CapabilityStrategy::filter does no I/O). WS2 went the other way: BatchPolicyStrategy is sync, GateHandlingStrategy and RecoveryStrategy are async. Defensible per trait (gate/recovery may consult host), but inconsistent with WS1's blanket-async choice. Worth picking one rule and documenting it — otherwise future trait additions will diverge again.

RecoveryOutcome::Retry { alter: Option<RetryAlteration> }

Doc says "the executor decides" retry scope (call-level vs iteration-level). Punts a real coordination question to WS-6 with no encoded contract. A brief invariant in the outcome (or a RetryScope enum) would lock it down before two concrete impls disagree.

Minor

• Backoff { delay: Duration } has no upper bound or monotonic check; consider validated newtype per .claude/rules/types.md.
• CapabilityErrorClass has both Permanent and Internal — semantic overlap with PolicyDenied/InputInvalid. A 1-line doc per variant would lock the taxonomy.
• Sanitization invariant (safe_summary: String) is asserted in many doc comments but enforced nowhere — a SanitizedSummary newtype would mirror the discipline.

Wire-stable serde discipline + object-safety _check tests + pub(crate) sealing are all good.

[henrypark133]

Addressed the WS2 review notes across 9ebcd03, 62c7038, and a57ae7e. Highlights: retry scope is now encoded with RetryScope, backoff uses bounded numeric milliseconds, strategy safe summaries validate construction/deserialization, object-safety checks are module-scoped, and the sync-vs-async strategy rule is documented. Verified with cargo test -p ironclaw_agent_loop --lib, cargo check -p ironclaw_agent_loop -p ironclaw_reborn, cargo fmt --check, and clippy over the touched crates.

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!