feat(host_api): result-record vocabulary (GateRecord/DenyRecord + OutcomeRefs preview/child_run) (#6168) - #6237
Conversation
…tcomeRefs preview/child_run (#6168) The §5.2.9 "render from record" contract for the capability-result collapse: the model-visible content that `Resolution`'s opaque refs (GateRef/DenyRef/ResultRef) point at, so the thin result channels don't lose data the loop consumes. - gate_record.rs (new): - `GateRecord { Approval | Auth{credential_requirements} | Resource | DependentRun{result,byte_len} | ExternalTool }` keyed by `GateRef` — carries the resume/credential payloads (G3) + dependent-run staged result (G2) that ride inline on today's `CapabilityOutcome`. `summary()`/`kind()` accessors. - `DenyRecord { reason: DenyReason, summary }` keyed by `DenyRef` — the model-visible denial content behind `Resolution::Denied`. - resolution.rs: `OutcomeRefs` gains `preview: Option<SafeSummary>` (was model_observation) and `child_run: Option<RunId>` (was SpawnedChildRun.child_run_id, set only for the ChildSpawned verdict). Additive + unused (like C.1–C.7) pending the result-channel migration slice. The records are model-visible → they carry redacted SafeSummary/DenyReason; the loop renders FROM them and never reconstructs credential requirements from model-visible data (tool-evidence.md / safety-and-sandbox.md). Off main. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🔎 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. |
|
Warning Review limit reached
Next review available in: 4 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds public gate and denial records with redacted summaries, stable serde discriminants, and round-trip tests. Changes ChangesHost API outcome contracts
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 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 |
There was a problem hiding this comment.
⏭️ IronLoop Review Declined: reviewer
Review at a glance
| Disposition | Head |
|---|---|
| ⏭️ Review declined | 555da533504c |
Head: 555da533504ca71b29c3a32330c23dea6c3a2e4e
Reason: The required base (0ae4197) and head (555da53) are neither ancestor of the other (merge base ba31ee1). Their comparison spans 54 files and 11,635 changed lines, including removal of unrelated CLI onboarding, WebUI, composition, LLM, secrets, and Docker behavior. A reliable complete review is not possible within the configured focused-review scope.
Next: Rebase or merge the PR head onto base 0ae4197 (preserving the intended host_api change), then request review of the resulting focused comparison.
Run details
Status: Current
Trustworthy review produced: no
Summary
Skipped: the supplied base/head comparison is an oversized, mixed divergence rather than the focused host_api change described by the PR.
There was a problem hiding this comment.
Code Review
This pull request introduces the gate_record module containing DenyRecord and GateRecord to support the "render from record" result contract. Additionally, it updates OutcomeRefs in resolution.rs to include optional preview and child_run fields. The review feedback suggests adding validation to enforce that child_run is only populated when the verdict is ToolVerdict::ChildSpawned to prevent inconsistent states.
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.
| /// The child run this outcome spawned (was `SpawnedChildRun.child_run_id`). | ||
| /// Set only for the `ToolVerdict::ChildSpawned` outcome; `None` otherwise. | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub child_run: Option<RunId>, |
There was a problem hiding this comment.
The documentation states that child_run is set only for the ToolVerdict::ChildSpawned outcome and should be None otherwise. However, there is currently no validation enforcing this invariant at construction or deserialization boundaries. An inconsistent state (such as a ToolVerdict::ChildSpawned with child_run: None, or a ToolVerdict::Success with child_run: Some(...)) could be deserialized or constructed, which could lead to unexpected behavior or logic errors in the execution loop.
Consider implementing a validation step (e.g., via a custom Deserialize implementation or a validation helper) on Outcome to strictly enforce this invariant, similar to how Obligations validates its invariants.
There was a problem hiding this comment.
Right that the invariant was unenforced — fixed one step stronger than a validation hook: the RunId now lives ON the variant (ToolVerdict::ChildSpawned { child_run: RunId }) and OutcomeRefs.child_run is gone. Both illegal states are now unrepresentable rather than validated: a Success/RecoverableFailure has no field to carry a child ref, and a child_spawned wire tag without the run fails deserialization structurally (pinned by child_spawned_verdict_carries_the_run_on_the_variant, including the {"child_spawned": {}} rejection case). A ToolVerdict::child_run() accessor gives uniform Option<RunId> access where the old field shape was wanted. Nothing outside host_api consumed the field yet (additive vocabulary), so the wire-shape change is free.
|
🚅 Deployed to the ironclaw-pr-6237 environment in ironclaw-ci-preview
|
…ned — invariant unrepresentable, not validated OutcomeRefs.child_run was an Option whose doc promised 'set only for the ChildSpawned verdict' with nothing enforcing it: a Success outcome carrying a child ref (or a ChildSpawned without one) could be built or deserialized. Moving the RunId onto the variant makes both illegal states impossible to express — a child_spawned wire tag without the run fails deserialization structurally (pinned by test), and no validation hook is needed. Reported-by: gemini-code-assist (PR #6237 review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.49% — 309494 / 362020 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)
|
✅ Ready for mergeReviewed, review comment addressed, CI fully green — 59 pass / 0 fail.
🤖 Generated with Claude Code |
What
The §5.2.9 "render from record" contract for the capability-result collapse — the model-visible content that
Resolution's opaque refs point at, so the thin new channels don't drop data the loop consumes (the G1–G5 gaps the analysis surfaced).gate_record.rs(new):GateRecord { Approval | Auth{credential_requirements} | Resource | DependentRun{result,byte_len} | ExternalTool }keyed byGateRef(carries resume/credential payloads + dependent-run staged result);DenyRecord { reason: DenyReason, summary }keyed byDenyRef.resolution.rs:OutcomeRefsgainspreview: Option<SafeSummary>+child_run: Option<RunId>(set only for theChildSpawnedverdict).Additive + unused (like the C.1–C.7 vocabulary), pending the result-channel migration. Off
main— lands independently of the#6229review gate.Security
Records are model-visible → they carry redacted
SafeSummary/DenyReason; the loop renders FROM them and never reconstructs credential requirements from model-visible data (tool-evidence.md/safety-and-sandbox.md).Checks
cargo test -p ironclaw_host_api(64+57) · clippy-D warningsclean ·ironclaw_capabilities/ironclaw_turnsbuild clean (theOutcomeRefsfield-add) ·ironclaw_architecturegreen.🤖 Generated with Claude Code