feat(reborn): host-private ReplayPayloadStore for gate/auth resume-read (§5.3) - #6256
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. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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
scope: docs
![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 a host-private replay-payload persistence mechanism (ReplayPayloadStore and FilesystemReplayPayloadStore) in the ironclaw_capabilities crate. This allows the host to persist raw capability inputs and estimates for gate/auth resumes, avoiding round-tripping them through the untrusted loop checkpoint. The feedback suggests replacing pretty-printed JSON serialization with compact serialization to improve performance and reduce storage overhead.
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.
| } | ||
|
|
||
| fn record_entry(record: &StoredReplayPayload) -> Result<Entry, ReplayPayloadStoreError> { | ||
| let body = serialize_pretty(record)?; |
There was a problem hiding this comment.
Using pretty-printed JSON for internal host-private persistence is less efficient in terms of both CPU and storage space. Since this payload is only consumed programmatically by the host on resume and is never user-facing, we should use compact serialization instead.
| let body = serialize_pretty(record)?; | |
| let body = serialize(record)?; |
| fn serialize_pretty<T>(value: &T) -> Result<Vec<u8>, ReplayPayloadStoreError> | ||
| where | ||
| T: Serialize, | ||
| { | ||
| serde_json::to_vec_pretty(value) | ||
| .map_err(|error| ReplayPayloadStoreError::Serialization(error.to_string())) | ||
| } |
There was a problem hiding this comment.
Replace serialize_pretty with a compact serialize helper using serde_json::to_vec to improve performance and reduce storage overhead for persisted replay payloads.
| fn serialize_pretty<T>(value: &T) -> Result<Vec<u8>, ReplayPayloadStoreError> | |
| where | |
| T: Serialize, | |
| { | |
| serde_json::to_vec_pretty(value) | |
| .map_err(|error| ReplayPayloadStoreError::Serialization(error.to_string())) | |
| } | |
| fn serialize<T>(value: &T) -> Result<Vec<u8>, ReplayPayloadStoreError> | |
| where | |
| T: Serialize, | |
| { | |
| serde_json::to_vec(value) | |
| .map_err(|error| ReplayPayloadStoreError::Serialization(error.to_string())) | |
| } |
5edfc56 to
7b4de44
Compare
![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 | ffed267998af |
Head: ffed267998af150a0846335d0ddb2267f1be62e9
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 replay-payload store is focused, but this PR also regresses durable dependent-run result recovery introduced by the base branch.
Findings
Blocking: 1 / Notes: 0
Blocking findings
1. ❌ [HIGH] Preserve the staged dependent-run result origin
Location: crates/ironclaw_turns/src/run_profile/resolution_mapping.rs:305-308
Removing result_origin drops the only durable association between the freshly minted ResultRef and the loop-side result_ref under which the child output was staged. RefBindings is transient, so after the gate record is persisted a resumed dependent run cannot recover that output. Restore the optional origin field on GateRecord::DependentRun and populate it here (with the prior serde default retained for existing records).
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.
| GateRecord::DependentRun { | ||
| summary: safe_summary_or_placeholder(safe_summary), | ||
| result: minted_result, | ||
| byte_len, |
There was a problem hiding this comment.
Dropping result_origin makes the staged child output unreachable after this gate record is persisted: the minted ResultRef has no durable mapping back to the loop's result_ref, and RefBindings is transient. Please restore and populate the optional origin field.
…ityOutcome collapse (§5.3 Stage 1)
Additive vocabulary work so a later stage can delete `CapabilityOutcome`.
`host_api::Resolution` can now losslessly represent every recoverable field the
loop-facing `CapabilityOutcome` carried, and `capability_outcome_to_resolution`
populates them.
New host_api vocabulary (`result_meta.rs`, plain redacted vocabulary only —
bounded enums, a hash value, bounded validated identifiers; no secrets, raw
paths, backend error strings, or runtime handles):
- `FailureKind` (+ `FailureKindValue`) — recovery classification, on
`ToolVerdict::RecoverableFailure { error_kind }` (was G1-dropped).
- `ResultProgress`, `TerminateHint`, `OutputDigest` — loop-derived completion
signals, on `Outcome`/`OutcomeRefs` (were G4-dropped).
- `ResumeToken` — opaque gate-resume identity, on the gate `GateWaypoint`.
- `LoopRef` — preserved originating loop ref, on `GateWaypoint`/`ProcessWaypoint`
origin and `OutcomeRefs.origin`.
Channel reshapes (constructed only by host_api + the mapping — no production
consumers yet): `Blocked`/`Suspension` variants now carry `GateWaypoint`/
`ProcessWaypoint` (kernel handle + preserved origin + optional resume token);
`OutcomeRefs` gains `origin`/`output_digest`; `Outcome` gains `progress`/
`terminate_hint`; `ToolVerdict::RecoverableFailure` gains `error_kind`.
The mapping enrichment deletes the G1/G4 "dropped" comments and populates the
new fields; a round-trip test (written test-first, watched fail with the
enrichment neutralized) pins each. The kernel uuid handle stays freshly minted
(its host-owned semantics) — the loop ref is preserved additively in `origin`
rather than smuggled into the uuid, keeping the "kernel refs are opaque uuids,
never caller-composed" invariant intact.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…origin on the durable record; review perf nits - IronLoop: AwaitDependentRun's staged result_ref was preserved nowhere durable — the minted GateRecord::DependentRun.result is a fresh uuid and the RefBindings side-table is transient, so the child output the loop staged under its own ref would be unreachable from the record a later resume turn renders from. GateRecord::DependentRun gains result_origin: Option<LoopRef> (serde default — pre-existing rows rehydrate as None), the mapping populates it, and the mapping + wire tests pin it. - Gemini: FailureKind deserializes via Cow<str> (no allocation for the 18 named variants); validate_safe_tag checks bytes, not chars. Reported-by: ironloopai, gemini-code-assist (PR #6254 review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…st to the Blocked waypoint reshape Semantic rebase fallout: this PR reshaped Blocked to carry GateWaypoint, and the since-merged W1b/W1c authorize folds on main construct Blocked::Approval at three kernel sites (plus the authorized_seal test). Bare waypoints there are correct — the kernel witness's approval resume rides the lease machinery; origin/resume waypoint fields are loop-mapping concerns. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ad (§5.3) Adds the persistence slice that unblocks the capability-result collapse (docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md §3/§5.3). Mirrors the sibling GateRecordStore (#6243) but is unwired: it provides the port + a Filesystem implementation + contract tests only. A later slice (Stage 2a) wires write-at-gate-raise + read-on-resume and flips CapabilityOutcome. Today the raw replay payload rides in-band through the UNTRUSTED loop on CapabilityApprovalResume/CapabilityAuthResume and is stashed in the loop's own serialized checkpoint (raw input+estimate). The collapse makes the loop-facing Resolution carry only an opaque resume token (== InvocationId), so the host must persist the replay payload itself and reconstitute it on resume. Moving it host-side ALSO retires a real exposure — raw tool input no longer round-trips through the loop's checkpoint. Host-privacy is a security requirement here. Record schema (host-private; the opposite of a model-visible GateRecord — no SafeSummary): ReplayPayload { input: serde_json::Value, estimate: ResourceEstimate, prior_approval: Option<AuthResumeApprovalIdentity>, input_ref: CapabilityInputRef, correlation_id: CorrelationId }. The field types are imported from their owners (ironclaw_turns for CapabilityInputRef/AuthResumeApprovalIdentity, host_api for the rest), not re-typed — no lossy re-typing per type-placement.md. Placement: ironclaw_capabilities, NOT ironclaw_run_state. The run_state charter (CLAUDE.md line 7) forbids persisting raw replay input in run-state records, and the ironclaw_turns charter forbids persisting raw tool input in turn state/events — both candidate "clean" homes are charter-hostile to raw replay input. capabilities owns the caller-facing invoke/resume/spawn workflow this payload exists to serve and has no such prohibition (type-placement.md §2). The two new dependency edges (capabilities -> turns, capabilities -> filesystem) are both permitted by the layer matrix (kernel -> kernel, kernel -> substrates) and match existing kernel topology (host_runtime -> turns; run_state -> filesystem); ironclaw_architecture is green. CAS/mount lane reused from the sibling: ScopedFilesystem<F: RootFilesystem> + shared cas_update (fail-closed CasUnsupported on non-CAS backends), a replay_payload_record RecordKind gate rejecting byte-only backends, a private StoredReplayPayload wrapper carrying the scope for a same_scope_owner defense-in-depth check, and a new /replay-payloads mount alias (tenant/user in the MountView, within-tenant axes in the alias-relative path). save() is write-once (dup InvocationId -> ReplayPayloadAlreadyExists); load() returns Option. No removal method — the payload is consumed once on resume; a later retention contract can add deletion (database.md "Data safety"). Test-first: 6 contract tests mirror gate_record_store_contract.rs and were watched red against a stub before implementing — all-fields round-trip (raw input+estimate survive), the auth-without-prior-approval shape, missing -> None, write-once dup rejection (original intact), and cross-tenant + within-tenant scope isolation (tenant1 vs tenant2; project A vs B look unknown). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ffed267 to
d9f6bed
Compare
5e3b561 to
9ef2c6d
Compare
1 participant
What
Adds the persistence slice that unblocks the capability-result collapse (
docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md§3/§5.3). Mirrors the siblingGateRecordStore(#6243) but is unwired, exactly like #6243: it provides theReplayPayloadStoreport + aFilesystemReplayPayloadStoreimplementation + contract tests only. It does not touchCapabilityOutcomeor the loop — a later slice (Stage 2a) wires write-at-gate-raise + read-on-resume + the type flip.Why host-private (security)
Today the raw replay payload rides in-band through the UNTRUSTED loop on
CapabilityApprovalResume/CapabilityAuthResume(crates/ironclaw_turns/src/run_profile/host.rs) and is stashed in the loop's own serialized checkpoint (LoopExecutionState.pending_approval_resume/pending_auth_resume, rawinput+estimate). The collapse makes the loop-facingResolutioncarry only an opaque resume token (equal to theInvocationId), so the host must persist the replay payload itself and reconstitute it on resume. Moving it host-side retires a real exposure: raw tool input no longer round-trips through the loop's checkpoint.ReplayPayloadis the exact opposite of a model-visibleGateRecord— it carries noSafeSummary, holds the raw toolinput, and must never reach the model, an event, an error, a snapshot, or a log.Record schema
The field types are imported from their owners, not re-typed — no lossy re-typing per
type-placement.md. This is the host-private superset of theCapabilityApprovalResume/CapabilityAuthResume(replay)fields.Placement decision (the one real judgment call)
Owning crate:
ironclaw_capabilities— notironclaw_run_state, notironclaw_turns.ironclaw_run_state's charter (CLAUDE.mdline 7) explicitly forbids persisting raw replay input in run-state records.ironclaw_turns's charter forbids persisting raw tool input in turn state or events.ironclaw_capabilitiesowns the caller-facing invoke/resume/spawn workflow this payload exists to serve (type-placement.md§2) and has no such prohibition — it is the concept owner of resume.Cost: two new normal dependency edges,
capabilities -> ironclaw_turns(for the resume-payload field types) andcapabilities -> ironclaw_filesystem(for the CAS lane). Both are permitted by the layer matrix (kernel -> kernel, kernel -> substrates) and match existing kernel topology (host_runtime -> turns;run_state -> filesystem).cargo test -p ironclaw_architectureis green with the new edges. Neither is a "runtime crate" in the sense the capabilities charter forbids (turns is neutral coordination contracts; filesystem is the storage substraterun_statealready sits on).CAS / mount lane reused (from #6243)
ScopedFilesystem<F: RootFilesystem>+ sharedcas_update(fail-closedCasUnsupportedon non-CAS backends — no blind overwrite).replay_payload_recordRecordKindtag so byte-only backends (DiskFilesystem) are rejected on first put.StoredReplayPayload { scope, payload }wrapper soloadcan apply the samesame_scope_ownerdefense-in-depth check the sibling gate-record store does — a wrong-scope read looks unknown./replay-payloadsmount alias: tenant/user identity lives in the caller'sMountView; within-tenant axes (agent/project/mission/thread) stay in the alias-relative path. No catalog/composition wiring in this unwired slice (mirrors feat(run_state): persistent GateRecordStore for the capability-result collapse (§5.2.9) #6243; the alias target sits under the existing/enginecanonical root).saveis write-once (dupInvocationId->ReplayPayloadAlreadyExists);load -> Option. No removal method — the payload is consumed once on resume, and (like the sibling) there is no scope-safe soft-delete to mirror; hard deletion of a retained record needs an explicit retention contract (database.md"Data safety"). Documented inline.Tests (test-first)
6 contract tests in
crates/ironclaw_capabilities/tests/replay_payload_store_contract.rs, mirroringgate_record_store_contract.rs, written and watched red against a stub before implementing:inputJSON +estimatesurviveprior_approval: None) round-tripsNoneReplayPayloadAlreadyExists, original intact)Noneunder tenant2/user1Verification
cargo test -p ironclaw_capabilities --all-features— pass (incl. the 6 new contract tests)cargo clippy -p ironclaw_capabilities --all-targets --all-features -- -D warnings— clean (also default lane; crate has no feature gates)cargo test -p ironclaw_architecture— pass (new dependency edges permitted)scripts/pre-commit-safety.sh— pass🤖 Generated with Claude Code