feat(host_api): Slice C.3 — Blocked/Suspension gate & suspension channels (#6168) - #6226
Conversation
🔎 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. |
|
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 (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds UUID-backed reference types and introduces publicly exported ChangesResolution contracts
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 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
risk: low
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the 'resolution' module in 'ironclaw_host_api', defining 'Blocked' and 'Suspension' enums to represent distinct result channels for re-entrant gates and parked work, respectively. It also adds new string ID types ('GateRef', 'ProcessRef', 'DenyRef'). The feedback suggests adding ergonomic accessor methods ('gate_ref' and 'process_ref') to the 'Suspension' enum to match the pattern of 'Blocked::gate_ref' and avoid repetitive pattern matching boilerplate.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| impl Suspension { | ||
| /// Stable discriminant (matches the serde tag) for logs/routing. | ||
| pub fn kind(&self) -> &'static str { | ||
| match self { | ||
| Suspension::Process(_) => "process", | ||
| Suspension::DependentRun(_) => "dependent_run", | ||
| Suspension::ExternalTool(_) => "external_tool", | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
To match the convenience of Blocked::gate_ref, Suspension should provide safe, ergonomic accessor methods to retrieve the underlying GateRef or ProcessRef when applicable. This prevents downstream consumers from having to write repetitive pattern matching boilerplate just to extract these references.
impl Suspension {
/// Stable discriminant (matches the serde tag) for logs/routing.
pub fn kind(&self) -> &'static str {
match self {
Suspension::Process(_) => "process",
Suspension::DependentRun(_) => "dependent_run",
Suspension::ExternalTool(_) => "external_tool",
}
}
/// Returns the underlying gate reference if this suspension is gate-backed.
pub fn gate_ref(&self) -> Option<&GateRef> {
match self {
Suspension::Process(_) => None,
Suspension::DependentRun(g) | Suspension::ExternalTool(g) => Some(g),
}
}
/// Returns the underlying process reference if this suspension is process-backed.
pub fn process_ref(&self) -> Option<&ProcessRef> {
match self {
Suspension::Process(p) => Some(p),
Suspension::DependentRun(_) | Suspension::ExternalTool(_) => None,
}
}
}There was a problem hiding this comment.
Added in eb5e580: Suspension::gate_ref() -> Option<&GateRef> (answers for the gate-shaped DependentRun/ExternalTool) and Suspension::process_ref() -> Option<&ProcessRef> (answers for Process), mirroring Blocked::gate_ref — with the shape-exclusivity of the two accessors pinned in the serde-tag test.
![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 | 1 | 0 | 1 | 1e45f5dd7fe0 |
Head: 1e45f5dd7fe045e894341cc870dfa8e05f81f948
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 head reverses the supplied base’s ErrRef leak-prevention fix, reopening a raw-error/secret exposure through serialized and displayed HostFailure values. No other actionable issues found in the small stack-layer diff.
Findings
Blocking: 1 / Notes: 0
Blocking findings
1. ❌ [HIGH] Restore ErrRef as a UUID-backed opaque handle
Location: crates/ironclaw_host_api/src/ids.rs:238
Changing ErrRef back to string_id! reverts the security fix present in the supplied base. HostFailure serializes and displays this value, while ErrRef::new accepts arbitrary non-path text and inherited from_trusted bypasses validation entirely; a raw backend error or secret can therefore cross the sanitized failure boundary. Restore the UUID-backed ErrRef and its regression test rather than carrying free-form correlation text.
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.
| @@ -236,6 +236,15 @@ string_id!(RoutineId, "routine", validate_name_segment); | |||
| // scope-id validation (bounded, no path separators / control chars) so it can | |||
| // carry an existing id (invocation/correlation) verbatim. | |||
| string_id!(ErrRef, "err_ref", validate_scope_id); | |||
There was a problem hiding this comment.
This reverts the base branch’s ErrRef hardening. Since HostFailure serializes and displays this value, a string-backed ref (especially with public from_trusted) can carry raw backend text or a secret across the sanitized boundary. Please retain the UUID-backed ErrRef and its regression coverage.
There was a problem hiding this comment.
Fixed in eb5e580: rebased onto main keeping #6225's uuid_id!(ErrRef) hardening (the branch had been cut from the pre-fix head), and — same reasoning applied consistently — the three refs this slice introduces (GateRef/ProcessRef/DenyRef) are now uuid_id!s too: they ride serialized Blocked/Suspension/verdict values across sanitized boundaries, so free text is structurally unrepresentable rather than merely scope-validated (from_trusted doesn't exist for uuid ids). Fixtures use fixed UUIDs for deterministic wire assertions.
|
🚅 Deployed to the ironclaw-pr-6226 environment in ironclaw-ci-preview
|
…nels (#6168) Third sub-slice of Slice C (arch-simplification §3/§5.3). Adds two of the five result channels that replace the overloaded ten-variant `CapabilityOutcome` (§1.2): the re-entrant gate channel and the parked-work channel. - ids.rs: `GateRef` / `ProcessRef` / `DenyRef` (via `string_id!`) — opaque handles into durably-stored control-plane records. Model-visible content (what the approver sees, the deny reason, the process summary) is rendered from the referenced record (§5.2.9), never carried inline — same shape as `ErrRef`. - resolution.rs (new): the result-channel family, built up across slices. - `Blocked { Approval | Auth | Resource }(GateRef)` — re-entrant gates. `is_dispatch_time_permitted()` pins §5.3.1: dispatch() may surface only `Auth`; an Approval/Resource gate from a lane is a contract violation. - `Suspension { Process(ProcessRef) | DependentRun(GateRef) | ExternalTool(GateRef) }` — parked work. `SpawnedChildRun` is deliberately excluded (non-suspending, §5.3 table). Keeping Blocked (waiting on a decision) distinct from Suspension (waiting on a result) makes the #6137 mis-route class a compile error. Additive only — nothing produces these yet; they fold into `Resolution` once `Outcome` lands (next slices). Stacked on C.2 (#6225). No dup-scan collision. Crate-tier tests (4): snake_case serde round-trip, kind()↔tag agreement, gate_ref() reachability, and the dispatch-time-only-Auth contract (§5.3.1). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses both review findings on #6226 and the rebase across the merged ErrRef hardening: - IronLoop: the branch reverted #6225's ErrRef fix (it was cut from the pre-fix head) — rebased onto main keeping uuid_id!(ErrRef), and the three NEW refs this slice adds (GateRef/ProcessRef/DenyRef) get the same structural guarantee for the same reason: they ride serialized Blocked/Suspension/verdict values across sanitized boundaries, so free text must be unrepresentable, not merely validated. Test fixtures use fixed UUIDs for deterministic wire assertions. - Gemini: Suspension gains gate_ref()/process_ref() accessors mirroring Blocked::gate_ref, with the shape-exclusivity pinned in the serde test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1e45f5d to
eb5e580
Compare
github-actions
Bot
added
size: L
✅ Ready for mergeReviewed, both findings fixed with replies, all CI checks green (55 pass / 0 fail; the red row is a superseded Railway preview deploy cancelled by the fix push).
🤖 Generated with Claude Code |
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.59% — 306742 / 358375 lines Per-crate breakdown (65 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)
|
This branch was successfully deployed
1 participant
What
Third sub-slice of Slice C (capability-path result collapse, §3/§5.3). Stacked on C.2 (#6225). Adds two of the five result channels that replace the overloaded ten-variant
CapabilityOutcome(§1.2).ids.rs:GateRef/ProcessRef/DenyRef— opaque handles into durably-stored control-plane records. Model-visible content is rendered from the referenced record (§5.2.9 gate-rendering contract), never carried inline — same shape as C.2'sErrRef.resolution.rs(new, grows across slices):Blocked { Approval | Auth | Resource }(GateRef)— re-entrant gates.is_dispatch_time_permitted()pins §5.3.1:dispatch()may surface onlyAuth(a lane discovers a credential demand by calling the thing — MCP 401, WASM fault); an Approval/Resource gate from a lane is aHostFailure::Permanent, never a gate.Suspension { Process(ProcessRef) | DependentRun(GateRef) | ExternalTool(GateRef) }— parked work.SpawnedChildRunis deliberately excluded (non-suspending, §5.3 table).Why two separate types
Blockedwaits on a decision (resolving it re-entersauthorize());Suspensionwaits on a result (the effect is already in flight or handed off). Confusing them is the #6137 bug class — separate types make it a compile error.Additive (§9)
Nothing produces these yet; they fold into
ResolutiononceOutcomelands (next slices).check-type-duplicates.py: no collision.Testing
4 crate-tier tests: snake_case serde round-trip,
kind()↔tag agreement,gate_ref()reachability, and the dispatch-time-only-Authcontract (§5.3.1). Crate-tier because unwired; integration coverage owed at wiring (testing.md).Checks
cargo test -p ironclaw_host_apigreen ·clippy -D warningsclean ·ironclaw_architectureratchets green · pre-commit clean.Next
validate_loop_safe_summary→host_apiasSafeSummary, thenToolVerdict+ResultRef/OutcomeRefs+Outcome.Resolutionumbrella (Done/Denied/Blocked/Suspended) with an acceptance-table test mapping all 10CapabilityOutcomerows (§5.3).Authorized+authorize()(security milestone), then wire the mediators (§9 steps 3–4).🤖 Generated with Claude Code