fix(threads): word-boundary sensitive-marker match so tool results containing "Secretary" aren't scrubbed as "secret" on replay — undoes #5902 tool-result eviction / re-fetch loop (+ 24KB/48KB caps) - #6129
Merged
Conversation
Scopes allow_driver_specific_nudges to interactive_default and scheduled_trigger via a builder method, avoiding a shared-base flip that would leak into planned_default/subagent. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…anned_default Real production interactive/chat/CLI turns request no explicit run profile (submit_user_turn passes requested_run_profile: None) and the production resolver defaults that to planned_default, not the literal interactive_profile() construct. Retargets the design accordingly and simplifies the implementation (no shared-base change needed at all). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…trigger nudges Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lear review Tasks 6/7 previously copy-pasted the same scripted scenario and completion assertion; extract no_progress_script()/ assert_completed_via_nudge() once, following the file's existing run_request/run_context_for_driver helper-extraction pattern. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…pecific_nudges builder" This reverts commit 326a16c.
…mark regression PR #5902 (fixing #5838's context-compaction crash) cut the model-visible tool-result preview from 100,000 to 2,048 bytes and capped result_read pagination at the same 2,048-byte chunk with no bulk-fetch option -- recovering a 100KB tool result now takes ~49 manual result_read calls, which most agent policies won't do reliably. This is a likely cause of the reported benchmark score regression. Raises TOOL_RESULT_RECORD_READ_MAX_BYTES to 40KB (still per-call bounded, which is the property #5838 actually needed -- unbounded accumulation in the compacted transcript caused the crash, not single-call size). Derives MAX_MODEL_OBSERVATION_BYTES from that constant (was an independent 4096 literal) so the whole-envelope validation cap can't silently fall behind the preview cap and start dropping large observations to bare summaries. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6129
July 15, 2026 19:36
Destroyed
github-actions
Bot
added
scope: docs
henrypark133
commented
Jul 16, 2026
henrypark133
left a comment
henrypark133
left a comment
Collaborator
Author
There was a problem hiding this comment.
Code Review (multi-agent)
Intent: Fix thread replay scrubbing that mistakes "Secretary" for "secret," retain paged tool results, and increase preview/envelope caps to prevent re-fetch loops.
Stats: 3 findings (from 15 raw candidate findings, 3 after live-thread dedup) across 2 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 0.
Tests
-
Medium Add a safe-summary regression for Secretary text (
crates/ironclaw_threads/src/tool_result_reference.rs:467-481, confidence 90) — anchor:crates/ironclaw_threads/src/tool_result_reference.rs:480The new boundary matcher is also used by
validate_tool_result_safe_summary, but the added regression only exercises model-observation validation and the helper directly. A future change could leave safe summaries incorrectly rejecting ordinary text such as "Secretary" without failing the tests. -
Medium Cover Secretary content through the replay path (
crates/ironclaw_threads/src/tool_result_reference.rs:1059-1078, confidence 85) — anchor:crates/ironclaw_threads/src/tool_result_reference.rs:1060The regression currently validates the matcher and constructs an envelope in a crate test, but no caller-level integration case proves that a same-thread replay containing "Secretary of the Treasury" retains the document preview instead of scrubbing it. That is the production failure mode described by the PR.
Conventions
-
Medium Correct the public cap documentation (
crates/ironclaw_threads/src/contract.rs:468-475, confidence 100) — anchor:crates/ironclaw_threads/src/contract.rs:468-475The exported constant is 24 KiB, but its public doc comment says it was raised to 40KB and claims recovery in 1-2 calls based on that different value. The comment therefore misstates the contract and can mislead callers and maintainers about the actual retention and paging behavior.
Existing unresolved current-head threads already cover the stale 4096-byte runtime assertion, stale 2 KiB pagination expectations, and the matcher’s UTF-8 slicing/iteration hazard; those concerns were not duplicated here.
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6129
July 16, 2026 04:42
Destroyed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/ironclaw_threads/src/tool_result_reference.rs (1)
598-603: 📐 Maintainability & Code Quality | 🟡 Minor | 💤 Low valueEnforce
is_char_boundaryrepo invariant for string slicing.Although
match_indicesguarantees valid character boundaries, the use ofhaystack[..start]andhaystack[end..]still formally violates the repository invariant. As per path instructions, "Never byte-slice user or external strings with&value[..n]; use character-aware APIs such aschar_indices(),chars(), or anis_char_boundary()-checked boundary."Please add the explicit
is_char_boundary()checks to satisfy the repo rule.♻️ Proposed fix
- let before_ok = !starts_alnum - || start == 0 - || !haystack[..start].ends_with(|c: char| c.is_ascii_alphanumeric()); - let after_ok = !ends_alnum - || end >= haystack.len() - || !haystack[end..].starts_with(|c: char| c.is_ascii_alphanumeric()); + let before_ok = !starts_alnum + || start == 0 + || (haystack.is_char_boundary(start) && !haystack[..start].ends_with(|c: char| c.is_ascii_alphanumeric())); + let after_ok = !ends_alnum + || end >= haystack.len() + || (haystack.is_char_boundary(end) && !haystack[end..].starts_with(|c: char| c.is_ascii_alphanumeric()));🤖 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_threads/src/tool_result_reference.rs` around lines 598 - 603, Update the boundary checks in the match-validation logic containing before_ok and after_ok to explicitly require haystack.is_char_boundary(start) before haystack[..start] and haystack.is_char_boundary(end) before haystack[end..]. Preserve the existing alphanumeric and start/end boundary behavior while ensuring both byte slices are guarded by explicit character-boundary validation.Source: Path instructions
🤖 Prompt for all review comments with 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.
Duplicate comments:
In `@crates/ironclaw_threads/src/tool_result_reference.rs`:
- Around line 598-603: Update the boundary checks in the match-validation logic
containing before_ok and after_ok to explicitly require
haystack.is_char_boundary(start) before haystack[..start] and
haystack.is_char_boundary(end) before haystack[end..]. Preserve the existing
alphanumeric and start/end boundary behavior while ensuring both byte slices are
guarded by explicit character-boundary validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bfa2b8f3-99d6-4074-a2f4-48648e87235c
📒 Files selected for processing (4)
crates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_threads/src/contract.rscrates/ironclaw_threads/src/tool_result_reference.rscrates/ironclaw_threads/tests/session_thread_contract.rs
Contributor
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.67% — 304804 / 355796 lines Per-crate breakdown (63 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)
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6129
July 16, 2026 05:11
Destroyed
ilblackdragon
added a commit
that referenced
this pull request
Jul 19, 2026
….3 Stage 2 — the flip)
Collapses the loop-facing capability result — the overloaded ten-variant
`CapabilityOutcome`/`CapabilityBatchOutcome` — into the five-channel
`host_api::Resolution`/`ResolutionBatch` at the `LoopCapabilityPort` trait
boundary. `CapabilityOutcome` is retained (Stage 2b deletes it); producers
convert at their boundary via `capability_outcome_to_resolution(..).resolution`.
The trait flip: `invoke_capability -> Result<Resolution, _>`,
`invoke_capability_batch -> Result<ResolutionBatch, _>`. Batch loops gate on
`Resolution::parks()` (a re-entrant `Blocked` gate stops the batch too, H1),
replacing `CapabilityOutcome::is_suspension()`. All ~49 impls + decorators/
factories migrated in dep order (turns -> loop_host -> hooks -> composition ->
runner -> agent_loop), plus every test double and the `tests/integration`
doubles.
Executor consumer rewrite (ironclaw_agent_loop): `handle_capability_outcome`,
the batch fast-path drain, `handle_capability_error` retry,
`shared_await_dependent_gate`, and `capability_batch_counts` match exhaustively
over `Resolution` (no wildcard, §11.9). host_api->loop inverse reconstruction:
loop refs from the channel's preserved `origin`; `Done` re-split on
`ToolVerdict`; model-visible failure/denial content from PR-B
(`RecoverableFailure.diagnostic`, `Denial`); the DependentRun staged result from
`Suspension::dependent_result()`.
Resume identity stays byte-stable: `approval_request_id` reconstructed
deterministically from the `gate:approval-{id}` routing ref (fingerprinted lease
byte-identical); `prior_approval` kept on the wire (host-side move deferred to
§5.3 Stage 2a-ii); `input_ref` advisory on resume (host reconstitutes from
`ReplayPayloadStore`, Stage 0); `correlation_id` observability-only, regenerated
at the loop boundary. Stage-0 resume + missing-payload fail-closed + cross-tenant
tests stay green.
Auth `credential_requirements` render-from-record: moved off the loop-facing
`Blocked::Auth` channel onto `GateRecord::Auth` (§5.2.9), keyed deterministically
by the new name-based `GateRef::for_auth_gate` (the auth gate id is
`auth-{sha256hex}`, not a uuid, so it derives a stable v5 uuid); the runner
re-reads them from the durable record at the blocked-exit application into
`TurnRunRecord.credential_requirements` — the auth analogue of approval's Stage-0
render-from-record.
Model-visible first-look preview (#5838) fixed correctly, not dropped: the
collapse mis-routed tool-result CONTENT through `SafeSummary`, a caption type
that rejects `{ } [ ] /` delimiters (all structured output) and — via a
substring `"secret"` match — scrubbed the ordinary word "Secretary" (the #6129
bug: an amnesia re-read loop that tanked benchmark scores). Ports the canonical
word-boundary credential matcher into host_api (`credential_redaction`) and
applies it to `SafeSummary` so captions like "Secretary" survive; adds
`ModelResultPreview` — the correct 24 KiB content vehicle that tolerates
delimiters/newlines and redacts only genuine credentials (word-boundary markers
+ secret-like tokens) — and re-points `Outcome.refs.preview` at it, plus
`ResultPreviewMeta` carrying the truncated-preview continuation metadata
(referenced ref, total bytes, next offset, item count) so `result_read`
pagination survives. The executor reconstructs the `ResultReference` observation
from real content, preserving the inline first-look optimization (no extra
`result_read` round-trip).
Resume-core completions surfaced driving the production integration tiers green
(the flip drops the loop-facing `correlation_id`, so every resume consumer must
reconstitute it host-side, not read it off the wire):
- Loop-host runtime resume reconstitutes `correlation_id` from the persisted
`ReplayPayload` (keyed by invocation id) and applies it to the invocation
context before the fingerprinted approval/auth lease is matched — the loop's
post-flip value is advisory. Without this the lease's correlation match fails
("approval request does not match invocation: correlation_id") and the resume
terminalizes (caught by `reborn_group_approvals`, `reborn_group_journeys`).
- The synthetic outbound-delivery handler (raises its OWN approval gate outside
the loop-host persist seam) reconstitutes `correlation_id` from its own replay
payload for the approval cross-check instead of trusting the executor's now-
fresh `resume.correlation_id`, completing the same §5.3 Stage 2a-i render-from-
record pattern it already applied to `{input, estimate}` (caught by
`reborn_integration_outbound_target`).
- Deterministic gate-record keys (`for_auth_gate`, and `for_approval_request` on
the authorize path) make a re-raised gate derive the SAME content-addressed
key; the write-once `GateRecordStore` reports `GateRecordAlreadyExists`, which
the host now treats as benign (first-write-wins, record byte-identical),
mirroring the existing `ReplayPayloadAlreadyExists` tolerance — it strengthens,
not weakens, the write-once contract (caught by the auth-convergence journey).
- The completed-result `ResultReference` observation's own producer-authored
`summary` (the truncation/continuation hint, distinct from the generic result-
message caption) is carried through the collapse on `ResultPreviewMeta.summary`
so the executor rebuilds the observation with the producer's exact text rather
than the caption (caught by `reborn_integration_tool_call`'s truncated-preview
and array-item-count transcript assertions).
Full gate green: workspace `cargo clippy --all-targets --all-features -D
warnings`; `ironclaw_architecture`; the changed-crate unit suites (the only red
is the 3 pre-existing `llm_admin::nearai_mcp` env-var tests, unrelated); and all
51 Reborn integration bins (706 scenarios) including every §11.9 per-channel /
gate / resume / auth tier.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ilblackdragon
added a commit
that referenced
this pull request
Jul 20, 2026
….3 Stage 2 — the flip) (#6287) * refactor(reborn): loop-facing result becomes host_api::Resolution (§5.3 Stage 2 — the flip) Collapses the loop-facing capability result — the overloaded ten-variant `CapabilityOutcome`/`CapabilityBatchOutcome` — into the five-channel `host_api::Resolution`/`ResolutionBatch` at the `LoopCapabilityPort` trait boundary. `CapabilityOutcome` is retained (Stage 2b deletes it); producers convert at their boundary via `capability_outcome_to_resolution(..).resolution`. The trait flip: `invoke_capability -> Result<Resolution, _>`, `invoke_capability_batch -> Result<ResolutionBatch, _>`. Batch loops gate on `Resolution::parks()` (a re-entrant `Blocked` gate stops the batch too, H1), replacing `CapabilityOutcome::is_suspension()`. All ~49 impls + decorators/ factories migrated in dep order (turns -> loop_host -> hooks -> composition -> runner -> agent_loop), plus every test double and the `tests/integration` doubles. Executor consumer rewrite (ironclaw_agent_loop): `handle_capability_outcome`, the batch fast-path drain, `handle_capability_error` retry, `shared_await_dependent_gate`, and `capability_batch_counts` match exhaustively over `Resolution` (no wildcard, §11.9). host_api->loop inverse reconstruction: loop refs from the channel's preserved `origin`; `Done` re-split on `ToolVerdict`; model-visible failure/denial content from PR-B (`RecoverableFailure.diagnostic`, `Denial`); the DependentRun staged result from `Suspension::dependent_result()`. Resume identity stays byte-stable: `approval_request_id` reconstructed deterministically from the `gate:approval-{id}` routing ref (fingerprinted lease byte-identical); `prior_approval` kept on the wire (host-side move deferred to §5.3 Stage 2a-ii); `input_ref` advisory on resume (host reconstitutes from `ReplayPayloadStore`, Stage 0); `correlation_id` observability-only, regenerated at the loop boundary. Stage-0 resume + missing-payload fail-closed + cross-tenant tests stay green. Auth `credential_requirements` render-from-record: moved off the loop-facing `Blocked::Auth` channel onto `GateRecord::Auth` (§5.2.9), keyed deterministically by the new name-based `GateRef::for_auth_gate` (the auth gate id is `auth-{sha256hex}`, not a uuid, so it derives a stable v5 uuid); the runner re-reads them from the durable record at the blocked-exit application into `TurnRunRecord.credential_requirements` — the auth analogue of approval's Stage-0 render-from-record. Model-visible first-look preview (#5838) fixed correctly, not dropped: the collapse mis-routed tool-result CONTENT through `SafeSummary`, a caption type that rejects `{ } [ ] /` delimiters (all structured output) and — via a substring `"secret"` match — scrubbed the ordinary word "Secretary" (the #6129 bug: an amnesia re-read loop that tanked benchmark scores). Ports the canonical word-boundary credential matcher into host_api (`credential_redaction`) and applies it to `SafeSummary` so captions like "Secretary" survive; adds `ModelResultPreview` — the correct 24 KiB content vehicle that tolerates delimiters/newlines and redacts only genuine credentials (word-boundary markers + secret-like tokens) — and re-points `Outcome.refs.preview` at it, plus `ResultPreviewMeta` carrying the truncated-preview continuation metadata (referenced ref, total bytes, next offset, item count) so `result_read` pagination survives. The executor reconstructs the `ResultReference` observation from real content, preserving the inline first-look optimization (no extra `result_read` round-trip). Resume-core completions surfaced driving the production integration tiers green (the flip drops the loop-facing `correlation_id`, so every resume consumer must reconstitute it host-side, not read it off the wire): - Loop-host runtime resume reconstitutes `correlation_id` from the persisted `ReplayPayload` (keyed by invocation id) and applies it to the invocation context before the fingerprinted approval/auth lease is matched — the loop's post-flip value is advisory. Without this the lease's correlation match fails ("approval request does not match invocation: correlation_id") and the resume terminalizes (caught by `reborn_group_approvals`, `reborn_group_journeys`). - The synthetic outbound-delivery handler (raises its OWN approval gate outside the loop-host persist seam) reconstitutes `correlation_id` from its own replay payload for the approval cross-check instead of trusting the executor's now- fresh `resume.correlation_id`, completing the same §5.3 Stage 2a-i render-from- record pattern it already applied to `{input, estimate}` (caught by `reborn_integration_outbound_target`). - Deterministic gate-record keys (`for_auth_gate`, and `for_approval_request` on the authorize path) make a re-raised gate derive the SAME content-addressed key; the write-once `GateRecordStore` reports `GateRecordAlreadyExists`, which the host now treats as benign (first-write-wins, record byte-identical), mirroring the existing `ReplayPayloadAlreadyExists` tolerance — it strengthens, not weakens, the write-once contract (caught by the auth-convergence journey). - The completed-result `ResultReference` observation's own producer-authored `summary` (the truncation/continuation hint, distinct from the generic result- message caption) is carried through the collapse on `ResultPreviewMeta.summary` so the executor rebuilds the observation with the producer's exact text rather than the caption (caught by `reborn_integration_tool_call`'s truncated-preview and array-item-count transcript assertions). Full gate green: workspace `cargo clippy --all-targets --all-features -D warnings`; `ironclaw_architecture`; the changed-crate unit suites (the only red is the 3 pre-existing `llm_admin::nearai_mcp` env-var tests, unrelated); and all 51 Reborn integration bins (706 scenarios) including every §11.9 per-channel / gate / resume / auth tier. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reborn): gate record keyed to returned Resolution + fail unsubmittable auth block (#6287 IronLoop) Two blocking gate-record correctness fixes on the §5.3 Stage 2 flip: 1. Map once (capability_port.rs). `invoke_capability` mapped the outcome twice — once inside `persist_gate_record_for_outcome` to derive the key it saved under, and once via `capability_outcome_to_resolution` to build the return value. The approval/resource/dependent/external channels mint a fresh random `GateRef` per mapping (only auth is deterministic), so the persisted record and the returned `Resolution` pointed at different refs and a resume could never load the record. Now maps once and persists/returns the same `MappedResolution` (`persist_gate_record_for_outcome` -> `persist_gate_record_for_mapped`, no internal re-map). 2. Fail the exit on unsourceable auth requirements (turn_run_executor.rs). The flip made the durable `GateRecord::Auth` the ONLY source of `credential_requirements`, so a lookup that does not yield the record is a real regression: applying the block leaves an unsubmittable, provider-null auth gate. `enrich_auth_block_credential_requirements` now returns `Result`; the three lookup-result miss arms (store fault, `Ok(None)`, wrong kind) return `Err`, and `apply_exit` records a terminal failure (shared `record_exit_failure` helper) instead of applying the incomplete block. The two tolerant pre-conditions (no store, non-auth ref) stay warn+empty. Regression tests: extend `approval_gate_outcome_persists_gate_record_at_the_seam` to assert the returned Resolution's gate ref equals the persisted key; new `auth_block_with_unsourceable_requirements_fails_the_exit` drives `apply_exit` with a missing-record store and asserts a terminal failure is recorded (this test also caught a `&'static str` lifetime bug in the helper). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reborn): forward dependent-run observation caption to the resumed parent (#6287 IronLoop) `dependent_run_result_message` hardcoded `model_observation: None`, so `append_capability_result_ref`'s synthesize-fallback re-dropped the child's staged observation and the resumed parent saw only a bare success sentinel — not the caption/result reference the `AwaitDependentRun` channel forwarded before the flip. The mapping already preserves the caption on `DependentRunResult.observation` ("model_observation now rides the inline observation preview (was dropped entirely)"); the consumer just dropped it. Forward it as a Success `ResultReference` observation carrying the caption as its summary and pointing at the staged child result (`result_ref` + `byte_len`), so the parent can `result_read` the child output. The inline first-look preview content stays host-owned (the completed-`Outcome` path), per the mapping's "bounded SafeSummary caption" design — so `preview` stays `None` here. Test: `await_dependent_run_preserves_model_observation_for_replay` now asserts the forwarded `ResultReference` observation instead of the previously-pinned synthesized `GenericFailure` sentinel. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reborn): replay returns the record-backed gate ref, not a fresh mint (#6287 IronLoop) The map-once fix made the first invocation's persisted key and returned gate ref agree, but the replay path still diverged: on an idempotent replay the dispatch cache returns the same CapabilityOutcome, invoke_capability re-maps it (a fresh random GateRef for the approval/resource/dependent/external channels), and the set-based guard then skipped the persist — so the replayed Resolution carried a gate ref no record was ever saved under. Turn the replay guard into a resolution cache: `persisted_gate_resolutions: HashMap<IdempotencyKey, Resolution>`. The first invocation reserves the key with its resolution (atomic check-and-insert under one lock) and persists exactly one record under that resolution's gate ref; a repeat (or concurrent duplicate) finds the reservation and returns the SAME resolution — the one whose gate ref the record is under — instead of re-minting. Rollback-on-failed-save is preserved so the next replay retries. Regression: extend `replayed_gate_invocation_does_not_persist_a_duplicate_record` to assert the replayed Resolution's gate ref equals the first invocation's and the single persisted record's key, and that the record loads by the replayed ref. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reborn): gate resolution reservation waits for durable persist (#6287 IronLoop) The reserve-then-save replay cache had a concurrency window: a concurrent duplicate could find the reservation and return the resolution before the owner's save durably completed, so a transient store fault (owner rolls back and errors) left the waiter holding a blocked resolution whose gate record was never persisted — an unresumable gate. Make the reservation a proper in-flight/persisted state machine mirroring the crate's own DispatchRecordStore + wait_for_dispatch_completion: - persisted_gate_resolutions: HashMap<IdempotencyKey, GateResolutionState>, where GateResolutionState = InFlight(Arc<Notify>) | Persisted(Box<Resolution>). - The first caller reserves InFlight and is the sole persister. A concurrent duplicate / later replay finds InFlight and WAITS on the notify, using the lost-wakeup-safe pattern (create notified(), re-check the same reservation under the lock via gate_resolution_in_flight_matches, await only if still in-flight). - The owner publishes only AFTER the durable save resolves: Persisted on Ok/benign GateRecordAlreadyExists, cleared on a transient fault; it wakes the waiters either way, so a woken waiter receives the now-durable resolution or re-owns and retries. No waiter receives a resolution before durable persist. Covered by the existing replay/rollback regression tests (replayed_gate_invocation_does_not_persist_a_duplicate_record, failed_gate_record_persist_is_retried_on_replay), which still hold. [skip-regression-check] deterministic single-threaded coverage is unchanged; the concurrency window is a narrow race the existing tests already pin the sequential behavior of, and a hermetic multi-waiter race harness is infeasible to make reliably deterministic here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reborn): wire durable gate/replay stores into the ProductLive capability port (#6287 IronLoop) The ProductLive planned-runtime path built its capability port with only runtime/input/result/milestone/mounts — never `.with_gate_record_store(..)` / `.with_replay_payload_store(..)` — so on that path the gate record was never persisted and the replay payload never reconstituted, breaking approval/auth resumes (the local-dev path already wires both). - `ProductLivePlannedRuntimeAdapterConfig` + `ProductLiveLoopCapabilityPortFactory` gain `gate_record_store` + `replay_payload_store`; `create_capability_port` now calls `.with_gate_record_store(..).with_replay_payload_store(..)` on the host-runtime factory, exactly as the local-dev path does. - The product-workflow planned-loop harness builds both stores over ONE in-memory filesystem (production mount view via `wrap_scoped`) so a raise and its resume round-trip through the same records; the composition adapter tests supply in-memory-backed stores through the config. - Adds `ironclaw_capabilities` to product-workflow dev-deps for the harness's `FilesystemReplayPayloadStore`. Verified: `cargo test -p ironclaw_reborn_composition --test product_live_adapters` passes; product-workflow planned-loop harness tests build; clippy --all-features clean on both crates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reborn): RAII guard clears orphaned gate-resolution reservation on cancel/fault (#6287 IronLoop) The in-flight gate-resolution reservation cleaned up only on the paths that ran to completion. If the owning persist future was cancelled (dropped mid-`save`), or returned early before the publish block, the `InFlight` entry was orphaned and same-key replays waited on it forever; the transient-failure cleanup relied on code after the `await` running. Protect the reservation with an RAII drop guard mirroring the crate's own `DispatchReservationGuard`: - On reserving `InFlight`, the owner holds `GateResolutionReservationGuard`. Any drop without `commit()` — cancellation, transient store fault, or an early error — calls `clear_gate_resolution_reservation`, which removes the entry (only if still `InFlight`, never a published resolution) and wakes its waiters so one re-owns and retries. - The success / benign `GateRecordAlreadyExists` path publishes the durable `Persisted` resolution (`publish_gate_resolution`), wakes waiters, then `commit()`s the guard so its drop is a no-op. No waiter can be left blocked on an orphaned reservation regardless of how the owner exits. Regression: `cancelled_gate_persist_clears_reservation_so_replay_can_re_own` blocks the first `save` on a barrier, cancels the invocation while parked in `save`, and asserts a replay re-owns, completes within a timeout (no hang), and persists exactly one record. `failed_gate_record_persist_is_retried_on_replay` covers the transient-fault cleanup path through the guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reborn): share the ProductLive gate store with the turn executor too (#6287 IronLoop) The prior ProductLive wiring fix only wired the gate-record store into the capability port (the persist side). The turn executor (`RebornTurnRunExecutor`, via `DefaultPlannedRuntimeParts.gate_record_store`) still got `None`, so its render-from-record path (`enrich_auth_block_credential_requirements`, §5.2.9) could not read the record — an auth block was applied with empty requirements / failed the exit on the ProductLive path. The harness now shares ONE gate-record store between both sides: the store built for the capability port is captured into `turn_executor_gate_store` and passed to `DefaultPlannedRuntimeParts.gate_record_store`, so a persisted auth gate record round-trips to the executor read. Stays `None` for the non-ProductLive capability fakes (which do not persist gate records), preserving the executor's tolerant "no store wired" path; `inbound_turn_contract.rs` uses `EmptyCapabilityFactory`, so its `None` is correct and unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reborn): commit the dispatch reservation only after the replay-payload write (#6287 IronLoop) `guard.commit()` ran before the fallible `persist_replay_payload_for_fresh_gate` store write. On a transient store error the `?` returned with the dispatch reservation still `InFlight`, and the already-committed guard skipped its cleanup — stranding retries/duplicates of the same idempotency key waiting on the reservation forever. Move `guard.commit()` to AFTER the replay-payload write succeeds, so a store error leaves the guard uncommitted; its drop then clears the reservation and wakes waiters so one re-dispatches. The finish-runtime-outcome ordering is unchanged. Covered by `failed_gate_record_persist_is_retried_on_replay` and the `replayed_gate` seam tests (the reservation-clears-then-retry path). [skip-regression-check] the store-write-failure reservation-clear path is exercised by the existing retry/replay seam tests; a dedicated dispatch-cancel harness for this specific reordering is redundant with them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ilblackdragon
added a commit
that referenced
this pull request
Jul 20, 2026
… delete CapabilityOutcome/mirror DTOs (§5.3 Slice C) (#6299) * refactor(reborn): resume replay payload moves host-side via ReplayPayloadStore; wire real stores (§5.3 Stage 2a-i) Moves the raw gate/auth resume replay payload out of the untrusted loop and into the host-private `ReplayPayloadStore`, and wires the real durable stores in production composition. The loop-facing result stays `CapabilityOutcome` (no `Resolution` flip — that is Stage 2a-ii). Security fix: the loop checkpoint no longer stores raw tool args, retiring the charter violation in `crates/ironclaw_agent_loop/CLAUDE.md` ("State stores refs... Do not store raw prompts, raw model output, tool args ... in state"). Write (two gate-raise paths): at a FRESH runtime gate raise (`HostRuntimeLoopCapabilityPort::invoke_capability_dispatch`, gated on `is_fresh_dispatch` before `finish_runtime_outcome`) and at the synthetic outbound-delivery approval-gate raise (`outbound_delivery.rs::request_approval`), the host `save`s a `ReplayPayload{input, estimate, prior_approval:None, input_ref, correlation_id}` keyed by `InvocationId` (write-once; a benign same-invocation duplicate is tolerated). Read on resume: `runtime_outcome_to_loop` no longer embeds input/estimate; `invocation_replay_input` is replaced by `ReplayPayloadStore::load` keyed by the `InvocationId` recovered from the resume token, in both the runtime seam (`replay_payload_for_resume`) and the synthetic decorator/handler. Fail closed on a miss: an absent payload (including a wrong-scope read) is a sanitized terminal `Unavailable`, never a silent empty-input dispatch. The fingerprinted approval lease claim ordering is preserved. Dropped loop fields (security): `input`/`estimate` from `CapabilityApprovalResume` and `PendingApprovalResume`; the whole `CapabilityAuthResumeReplay` type and the `replay` field on `CapabilityAuthResume`/`PendingAuthResume`. `approval_request_id`/ `correlation_id`/`input_ref`/`prior_approval` stay on the wire (they move in 2a-ii). Composition (real stores): `capability_wiring` builds `FilesystemGateRecordStore` (closes the #6245 production gap where it defaulted to `NoopGateRecordStore`) and `FilesystemReplayPayloadStore` over the shared scoped filesystem and threads both through the refreshing capability port. Adds the `/replay-payloads` per-user mount alias. `loop_host` gains a dependency-inversion dep on `ironclaw_capabilities` for the port trait. Tests (all failed first): fail-closed on missing payload (loop_host seam); extended the #6245 approval-resume seam test to assert the store persisted the payload and the resume redispatches the correct original input; the `ReplayPayloadStore` contract (round-trip / write-once / cross-tenant / within-tenant) rides the base commit. Full-infra resume edges pass through the production-shaped harness: group_approvals (runtime approval resume), group_journeys (approval→auth convergence), outbound_target (synthetic approval resume). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(turns): move the removed-replay design note out of prior_approval's rustdoc Reported-by: gemini-code-assist (PR #6271 review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(reborn): Resolution carries model-visible failure diagnostic + denial content (§5.3 flip prep / PR-B) Additive vocabulary slice (§3/§5.2.9/§5.3): make `host_api::Resolution` carry the model-visible result content the executor's `handle_capability_outcome` reads off `CapabilityOutcome` today, so a later slice (PR-C) can flip `invoke_capability` to `Resolution` without the loop reading host storage (its charter forbids that). host_api additions (redacted vocabulary only — charter fit): - `ModelInputIssue`: redacted mirror of the loop's `CapabilityInputIssue` — `DispatchInputIssueCode` (existing host_api enum) + bounded redacted `SafeSummary` path/expected/received/schema_path. - `ModelFailureDiagnostic { InvalidInput { issues }, Diagnostic { text } }`: redacted mirror of `CapabilityFailureDetail`; rides `ToolVerdict::RecoverableFailure.diagnostic` (additive Option). - `Denial { deny, reason_kind, summary }`: `Resolution::Denied(DenyRef)` → `Resolution::Denied(Denial)`, carrying the model-visible `DenyReason` + redacted `SafeSummary` on the channel (a projection of the sibling DenyRecord). Mapping (`resolution_mapping.rs`): `failed_outcome` now redacts the loop `detail` into the diagnostic (per-field SafeSummary; path-shaped free text degrades to the placeholder); the Denied arm populates the Denial channel from the same reason/summary the DenyRecord holds. Tests (failed first, then green): structured `InvalidInput` issues and free-text `Diagnostic` round-trip through the mapping; Denied carries reason_kind + redacted summary; redaction proof — path- and secret-shaped content is redacted in the `Resolution` output, never carried raw. GateRecord/ DenyRecord persistence round-trips (ironclaw_run_state) stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(host_api): enforce the 16-issue diagnostic cap in the type, not one producer ModelFailureDiagnostic::InvalidInput now carries a bounded ModelInputIssues newtype: validated at construction, revalidated on the wire (try_from), with a truncating() producer constructor for the mapping. A 17-item payload is rejected at every entry point — pinned by model_input_issues_cap_is_enforced_at_construction_and_on_the_wire. Reported-by: ironloopai (PR #6273 review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(reborn): ResolutionBatch + parks() loop-suspension predicate + scripted-outcome fixture (§5.3 flip prep / Stage 1) Additive vocabulary + test-fixture slice ahead of the capability-result flip. Nothing consumes the new items yet, so the tree stays green. Closes a verified correctness hazard: `Resolution::is_suspension()` is `Suspended(_)` ONLY, but the batch loops the flip rewrites must also stop on the re-entrant gate variants (`Resolution::Blocked` — Approval/Auth/ Resource), which map to `Blocked`, not `Suspended`. A naive `is_suspension()` batch guard would silently let a gate fall through as if the call had completed (the #6137 bug class). This provides the correct predicate now so the flip has one canonical, tested site to call. - `ironclaw_host_api::Resolution::parks()`: the loop-semantics suspension predicate — true for every `Blocked` variant AND every `Suspended` variant; false for `Done` (incl. non-suspending `ChildSpawned`) and terminal `Denied`. `parks()` ⊋ `is_suspension()`: a `Blocked::Approval` parks but is not a suspension. `is_suspension()` keeps its narrow meaning unchanged. - `ironclaw_host_api::ResolutionBatch { resolutions: Vec<Resolution>, stopped_on_suspension: bool }`: the loop-facing batch result the flip adopts, mirroring `CapabilityBatchOutcome`'s shape/semantics over `Resolution`. Wire-stable serde. - `ironclaw_agent_loop::test_support::{resolution_from_scripted_outcome, resolution_batch_from_scripted}` (behind the existing `test-support` feature): convert existing `CapabilityOutcome` fixtures to `Resolution`/`ResolutionBatch` via the production mapping, for the flip's ~150 test sites. Tests (written first, watched fail): - host_api: acceptance table extended with a `parks` column across all 10 CapabilityOutcome→Resolution rows; a dedicated exhaustive `match` (compile-fails if a new `Resolution` variant is added) proving `parks()` ⊋ `is_suspension()`; a `ResolutionBatch` round-trip. Verified red: a naive `is_suspension()`-based `parks()` fails ("parks() disagrees for blocked"). - agent_loop test-support: round-trips scripted Completed/Failed/ ApprovalRequired → the right Resolution channel, and a batch preserving order + stop flag. No trait flip, no consumer change, `CapabilityOutcome` untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(host_api): Resolution::parks uses an exhaustive match, not matches! A new Resolution variant must be a compile error in parks() (§11.9 no-wildcard) so its parking behavior is decided deliberately rather than silently defaulting to false — the exact class of silent fall-through parks() exists to prevent (#6137). Reported-by: gemini-code-assist (PR #6275 review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(runner): drop removed Pending{Approval,Auth}Resume replay fields (inherited base debt) The replay-field removal lands in a lower stack slice; until this branch's base picks it up, planned_driver.rs's cfg(test) fixtures still set the removed fields and fail the workspace clippy lane. Harmless once the base merges. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(reborn): complete host-side gate reconstitution — resume input_ref, local-dev gate persistence, GateRef reconcile (§5.3 flip prep / Stage 0) Stage 0 no-flip host-reconstitution slice: closes two verified correctness hazards ahead of the atomic capability-result flip. The loop-facing trait and `CapabilityOutcome` are UNCHANGED; only host-side plumbing moves. Fix 1 — resume-time input_ref reconstitution (hazard 3). On a gate/auth resume the effective input_ref used for the idempotency key + validation is now derived from the host-private `ReplayPayload` persisted at the fresh gate raise (loaded by the resume's invocation id), not the advisory loop-supplied `approval_resume.input_ref`. A new `resume_replay_payload` helper centralizes the derivation so `invoke_capability` (seam) and `invoke_capability_dispatch` compute the SAME byte-stable key. The key is now derived lazily inside `persist_gate_record_for_outcome` (after dispatch, only for a gate outcome) so a missing/stale resume payload never pre-empts dispatch's own resume identity/activity validation — a malformed resume still surfaces `InvalidInvocation`, and a genuinely-missing payload still fails closed (2a-i missing-payload test stays green). Test proves a resume derives the same input_ref/key from the store even when the loop-supplied ref differs (a second resume differing only in that advisory ref replays the cached outcome instead of re-dispatching); failed first (re-dispatched, count 2) before the fix. Fix 2 — gate-record persistence for the local-dev synthetic approval producer (hazard 2). `OutboundDeliveryTargetSetHandler::request_approval` raises its gate OUTSIDE the loop-host persist seam, so it now persists a `GateRecord::Approval` itself at the raise (via the wired `GateRecordStore`), keyed by the canonical `GateRef::for_approval_request`, so host-side gate rendering (§5.2.9) has a record to read. Test proves the record persists and round-trips; failed first (load returned None) before the fix. Fix 3 — reconcile the two approval GateRef encodings. Canonicalized on the typed `GateRef::for_approval_request(id) = GateRef::from_uuid(id.as_uuid())` — a new `ironclaw_host_api` helper — as the GateRecordStore key. This is the host_api uuid GateRef (record key), a DIFFERENT type from the loop-facing `gate:approval-{id}` routing ref (`ironclaw_turns::GateRef`), so reconciling here touches NO persisted wire format and does not break the `is_approval_gate_ref` prefix-routing predicate. `ironclaw_capabilities::host` now uses the helper at both authorize sites. Test proves a host-persisted approval gate ref resolves through the read model: the routing ref recovers the approval id (`approval_request_id_from_gate_ref`), the canonical key re-derives, and the persisted GateRecord is found. Note: the task suggested canonicalizing on `from_uuid`; that is exactly what this does for the record-key GateRef. The RISKY direction (rewriting the `gate:approval-{id}` routing string to a bare uuid) was NOT taken — it would break the `is_approval_gate_ref` prefix predicate used in production routing, projection, and channel delivery, plus persisted adapter command strings and delivered-gate route records. Tests: loop_host 378 lib (incl. new + updated resume/seam/2a-i); host_api helper round-trip; composition 1629 (env-clean); capabilities/product_workflow/run_state green; architecture green; integration outbound_target/auth_gate/ reopen_resume_through_gate/group_approvals green; clippy -D warnings clean on all touched crates; pre-commit-safety clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(reborn): Suspension::DependentRun carries the staged child result (§5.3 flip prep / Stage 1b) Closes a verified hazard blocking the atomic capability-result flip (§5.3): `host_api::Resolution`'s `Suspension::DependentRun` channel could not carry the dependent child's staged result, which the loop needs on resume. After the flip the executor gets only `Resolution::Suspended(Suspension::DependentRun(..))`; the mapping folded `result_ref`/`byte_len`/`safe_summary` into the host-persisted `GateRecord::DependentRun` (the loop cannot read host storage) and dropped `model_observation` entirely — so byte caps, model-visible child text, and the model observation would silently regress ("never drop model output"). This ADDITIVE slice gives `Suspension::DependentRun` an inline staged-result payload alongside its `GateWaypoint`, mirroring how `Done(Outcome)` carries a spawned child run's content: - New `DependentRunResult { byte_len, summary: SafeSummary, observation: Option<SafeSummary>, origin: Option<LoopRef> }` — plain redacted host_api vocabulary; the full child bytes stay host-owned behind the record's `ResultRef`. `Suspension::DependentRun` becomes a struct variant `{ waypoint, result }`; accessors split from `ExternalTool`, plus a `dependent_result()` accessor. - `capability_outcome_to_resolution`'s `AwaitDependentRun` arm populates it from the loop's `result_ref`/`byte_len`/`safe_summary`/`model_observation` (redaction applied), and the "model_observation has no home … dropped" behavior is deleted — it now rides the inline observation preview (the same `observation_preview` reduction Completed/SpawnedChildRun use). The durable `GateRecord::DependentRun` sidecar is still persisted (host durability); the inline payload is the loop-visible copy — the same dual pattern as PR-B. Nothing consumes the new payload yet: `LoopCapabilityPort` is not flipped, `CapabilityOutcome` is not deleted, executor consumers are untouched — the tree stays green. Test-first: extended the `resolution` + `resolution_mapping` acceptance tables and added dedicated round-trip + redaction tests — an `AwaitDependentRun` outcome round-trips its `result_ref` origin, `byte_len`, `safe_summary`, AND `model_observation` onto `Suspension::DependentRun`, and a secret/path-shaped summary/observation degrades (dropped / placeholder). These fail against the pre-change tuple variant (fields absent / model_observation dropped). The `GateRecord::DependentRun` persistence round-trip stays green. Verified: `cargo test -p ironclaw_host_api -p ironclaw_turns -p ironclaw_run_state --all-features`; `cargo clippy -p ironclaw_host_api -p ironclaw_turns --all-targets --all-features -- -D warnings`; `cargo test -p ironclaw_architecture`; `scripts/pre-commit-safety.sh`; downstream build of loop_host/agent_loop/runner/reborn_composition. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(reborn): loop-facing result becomes host_api::Resolution (§5.3 Stage 2 — the flip) (#6287) * refactor(reborn): loop-facing result becomes host_api::Resolution (§5.3 Stage 2 — the flip) Collapses the loop-facing capability result — the overloaded ten-variant `CapabilityOutcome`/`CapabilityBatchOutcome` — into the five-channel `host_api::Resolution`/`ResolutionBatch` at the `LoopCapabilityPort` trait boundary. `CapabilityOutcome` is retained (Stage 2b deletes it); producers convert at their boundary via `capability_outcome_to_resolution(..).resolution`. The trait flip: `invoke_capability -> Result<Resolution, _>`, `invoke_capability_batch -> Result<ResolutionBatch, _>`. Batch loops gate on `Resolution::parks()` (a re-entrant `Blocked` gate stops the batch too, H1), replacing `CapabilityOutcome::is_suspension()`. All ~49 impls + decorators/ factories migrated in dep order (turns -> loop_host -> hooks -> composition -> runner -> agent_loop), plus every test double and the `tests/integration` doubles. Executor consumer rewrite (ironclaw_agent_loop): `handle_capability_outcome`, the batch fast-path drain, `handle_capability_error` retry, `shared_await_dependent_gate`, and `capability_batch_counts` match exhaustively over `Resolution` (no wildcard, §11.9). host_api->loop inverse reconstruction: loop refs from the channel's preserved `origin`; `Done` re-split on `ToolVerdict`; model-visible failure/denial content from PR-B (`RecoverableFailure.diagnostic`, `Denial`); the DependentRun staged result from `Suspension::dependent_result()`. Resume identity stays byte-stable: `approval_request_id` reconstructed deterministically from the `gate:approval-{id}` routing ref (fingerprinted lease byte-identical); `prior_approval` kept on the wire (host-side move deferred to §5.3 Stage 2a-ii); `input_ref` advisory on resume (host reconstitutes from `ReplayPayloadStore`, Stage 0); `correlation_id` observability-only, regenerated at the loop boundary. Stage-0 resume + missing-payload fail-closed + cross-tenant tests stay green. Auth `credential_requirements` render-from-record: moved off the loop-facing `Blocked::Auth` channel onto `GateRecord::Auth` (§5.2.9), keyed deterministically by the new name-based `GateRef::for_auth_gate` (the auth gate id is `auth-{sha256hex}`, not a uuid, so it derives a stable v5 uuid); the runner re-reads them from the durable record at the blocked-exit application into `TurnRunRecord.credential_requirements` — the auth analogue of approval's Stage-0 render-from-record. Model-visible first-look preview (#5838) fixed correctly, not dropped: the collapse mis-routed tool-result CONTENT through `SafeSummary`, a caption type that rejects `{ } [ ] /` delimiters (all structured output) and — via a substring `"secret"` match — scrubbed the ordinary word "Secretary" (the #6129 bug: an amnesia re-read loop that tanked benchmark scores). Ports the canonical word-boundary credential matcher into host_api (`credential_redaction`) and applies it to `SafeSummary` so captions like "Secretary" survive; adds `ModelResultPreview` — the correct 24 KiB content vehicle that tolerates delimiters/newlines and redacts only genuine credentials (word-boundary markers + secret-like tokens) — and re-points `Outcome.refs.preview` at it, plus `ResultPreviewMeta` carrying the truncated-preview continuation metadata (referenced ref, total bytes, next offset, item count) so `result_read` pagination survives. The executor reconstructs the `ResultReference` observation from real content, preserving the inline first-look optimization (no extra `result_read` round-trip). Resume-core completions surfaced driving the production integration tiers green (the flip drops the loop-facing `correlation_id`, so every resume consumer must reconstitute it host-side, not read it off the wire): - Loop-host runtime resume reconstitutes `correlation_id` from the persisted `ReplayPayload` (keyed by invocation id) and applies it to the invocation context before the fingerprinted approval/auth lease is matched — the loop's post-flip value is advisory. Without this the lease's correlation match fails ("approval request does not match invocation: correlation_id") and the resume terminalizes (caught by `reborn_group_approvals`, `reborn_group_journeys`). - The synthetic outbound-delivery handler (raises its OWN approval gate outside the loop-host persist seam) reconstitutes `correlation_id` from its own replay payload for the approval cross-check instead of trusting the executor's now- fresh `resume.correlation_id`, completing the same §5.3 Stage 2a-i render-from- record pattern it already applied to `{input, estimate}` (caught by `reborn_integration_outbound_target`). - Deterministic gate-record keys (`for_auth_gate`, and `for_approval_request` on the authorize path) make a re-raised gate derive the SAME content-addressed key; the write-once `GateRecordStore` reports `GateRecordAlreadyExists`, which the host now treats as benign (first-write-wins, record byte-identical), mirroring the existing `ReplayPayloadAlreadyExists` tolerance — it strengthens, not weakens, the write-once contract (caught by the auth-convergence journey). - The completed-result `ResultReference` observation's own producer-authored `summary` (the truncation/continuation hint, distinct from the generic result- message caption) is carried through the collapse on `ResultPreviewMeta.summary` so the executor rebuilds the observation with the producer's exact text rather than the caption (caught by `reborn_integration_tool_call`'s truncated-preview and array-item-count transcript assertions). Full gate green: workspace `cargo clippy --all-targets --all-features -D warnings`; `ironclaw_architecture`; the changed-crate unit suites (the only red is the 3 pre-existing `llm_admin::nearai_mcp` env-var tests, unrelated); and all 51 Reborn integration bins (706 scenarios) including every §11.9 per-channel / gate / resume / auth tier. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reborn): gate record keyed to returned Resolution + fail unsubmittable auth block (#6287 IronLoop) Two blocking gate-record correctness fixes on the §5.3 Stage 2 flip: 1. Map once (capability_port.rs). `invoke_capability` mapped the outcome twice — once inside `persist_gate_record_for_outcome` to derive the key it saved under, and once via `capability_outcome_to_resolution` to build the return value. The approval/resource/dependent/external channels mint a fresh random `GateRef` per mapping (only auth is deterministic), so the persisted record and the returned `Resolution` pointed at different refs and a resume could never load the record. Now maps once and persists/returns the same `MappedResolution` (`persist_gate_record_for_outcome` -> `persist_gate_record_for_mapped`, no internal re-map). 2. Fail the exit on unsourceable auth requirements (turn_run_executor.rs). The flip made the durable `GateRecord::Auth` the ONLY source of `credential_requirements`, so a lookup that does not yield the record is a real regression: applying the block leaves an unsubmittable, provider-null auth gate. `enrich_auth_block_credential_requirements` now returns `Result`; the three lookup-result miss arms (store fault, `Ok(None)`, wrong kind) return `Err`, and `apply_exit` records a terminal failure (shared `record_exit_failure` helper) instead of applying the incomplete block. The two tolerant pre-conditions (no store, non-auth ref) stay warn+empty. Regression tests: extend `approval_gate_outcome_persists_gate_record_at_the_seam` to assert the returned Resolution's gate ref equals the persisted key; new `auth_block_with_unsourceable_requirements_fails_the_exit` drives `apply_exit` with a missing-record store and asserts a terminal failure is recorded (this test also caught a `&'static str` lifetime bug in the helper). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reborn): forward dependent-run observation caption to the resumed parent (#6287 IronLoop) `dependent_run_result_message` hardcoded `model_observation: None`, so `append_capability_result_ref`'s synthesize-fallback re-dropped the child's staged observation and the resumed parent saw only a bare success sentinel — not the caption/result reference the `AwaitDependentRun` channel forwarded before the flip. The mapping already preserves the caption on `DependentRunResult.observation` ("model_observation now rides the inline observation preview (was dropped entirely)"); the consumer just dropped it. Forward it as a Success `ResultReference` observation carrying the caption as its summary and pointing at the staged child result (`result_ref` + `byte_len`), so the parent can `result_read` the child output. The inline first-look preview content stays host-owned (the completed-`Outcome` path), per the mapping's "bounded SafeSummary caption" design — so `preview` stays `None` here. Test: `await_dependent_run_preserves_model_observation_for_replay` now asserts the forwarded `ResultReference` observation instead of the previously-pinned synthesized `GenericFailure` sentinel. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reborn): replay returns the record-backed gate ref, not a fresh mint (#6287 IronLoop) The map-once fix made the first invocation's persisted key and returned gate ref agree, but the replay path still diverged: on an idempotent replay the dispatch cache returns the same CapabilityOutcome, invoke_capability re-maps it (a fresh random GateRef for the approval/resource/dependent/external channels), and the set-based guard then skipped the persist — so the replayed Resolution carried a gate ref no record was ever saved under. Turn the replay guard into a resolution cache: `persisted_gate_resolutions: HashMap<IdempotencyKey, Resolution>`. The first invocation reserves the key with its resolution (atomic check-and-insert under one lock) and persists exactly one record under that resolution's gate ref; a repeat (or concurrent duplicate) finds the reservation and returns the SAME resolution — the one whose gate ref the record is under — instead of re-minting. Rollback-on-failed-save is preserved so the next replay retries. Regression: extend `replayed_gate_invocation_does_not_persist_a_duplicate_record` to assert the replayed Resolution's gate ref equals the first invocation's and the single persisted record's key, and that the record loads by the replayed ref. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reborn): gate resolution reservation waits for durable persist (#6287 IronLoop) The reserve-then-save replay cache had a concurrency window: a concurrent duplicate could find the reservation and return the resolution before the owner's save durably completed, so a transient store fault (owner rolls back and errors) left the waiter holding a blocked resolution whose gate record was never persisted — an unresumable gate. Make the reservation a proper in-flight/persisted state machine mirroring the crate's own DispatchRecordStore + wait_for_dispatch_completion: - persisted_gate_resolutions: HashMap<IdempotencyKey, GateResolutionState>, where GateResolutionState = InFlight(Arc<Notify>) | Persisted(Box<Resolution>). - The first caller reserves InFlight and is the sole persister. A concurrent duplicate / later replay finds InFlight and WAITS on the notify, using the lost-wakeup-safe pattern (create notified(), re-check the same reservation under the lock via gate_resolution_in_flight_matches, await only if still in-flight). - The owner publishes only AFTER the durable save resolves: Persisted on Ok/benign GateRecordAlreadyExists, cleared on a transient fault; it wakes the waiters either way, so a woken waiter receives the now-durable resolution or re-owns and retries. No waiter receives a resolution before durable persist. Covered by the existing replay/rollback regression tests (replayed_gate_invocation_does_not_persist_a_duplicate_record, failed_gate_record_persist_is_retried_on_replay), which still hold. [skip-regression-check] deterministic single-threaded coverage is unchanged; the concurrency window is a narrow race the existing tests already pin the sequential behavior of, and a hermetic multi-waiter race harness is infeasible to make reliably deterministic here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reborn): wire durable gate/replay stores into the ProductLive capability port (#6287 IronLoop) The ProductLive planned-runtime path built its capability port with only runtime/input/result/milestone/mounts — never `.with_gate_record_store(..)` / `.with_replay_payload_store(..)` — so on that path the gate record was never persisted and the replay payload never reconstituted, breaking approval/auth resumes (the local-dev path already wires both). - `ProductLivePlannedRuntimeAdapterConfig` + `ProductLiveLoopCapabilityPortFactory` gain `gate_record_store` + `replay_payload_store`; `create_capability_port` now calls `.with_gate_record_store(..).with_replay_payload_store(..)` on the host-runtime factory, exactly as the local-dev path does. - The product-workflow planned-loop harness builds both stores over ONE in-memory filesystem (production mount view via `wrap_scoped`) so a raise and its resume round-trip through the same records; the composition adapter tests supply in-memory-backed stores through the config. - Adds `ironclaw_capabilities` to product-workflow dev-deps for the harness's `FilesystemReplayPayloadStore`. Verified: `cargo test -p ironclaw_reborn_composition --test product_live_adapters` passes; product-workflow planned-loop harness tests build; clippy --all-features clean on both crates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reborn): RAII guard clears orphaned gate-resolution reservation on cancel/fault (#6287 IronLoop) The in-flight gate-resolution reservation cleaned up only on the paths that ran to completion. If the owning persist future was cancelled (dropped mid-`save`), or returned early before the publish block, the `InFlight` entry was orphaned and same-key replays waited on it forever; the transient-failure cleanup relied on code after the `await` running. Protect the reservation with an RAII drop guard mirroring the crate's own `DispatchReservationGuard`: - On reserving `InFlight`, the owner holds `GateResolutionReservationGuard`. Any drop without `commit()` — cancellation, transient store fault, or an early error — calls `clear_gate_resolution_reservation`, which removes the entry (only if still `InFlight`, never a published resolution) and wakes its waiters so one re-owns and retries. - The success / benign `GateRecordAlreadyExists` path publishes the durable `Persisted` resolution (`publish_gate_resolution`), wakes waiters, then `commit()`s the guard so its drop is a no-op. No waiter can be left blocked on an orphaned reservation regardless of how the owner exits. Regression: `cancelled_gate_persist_clears_reservation_so_replay_can_re_own` blocks the first `save` on a barrier, cancels the invocation while parked in `save`, and asserts a replay re-owns, completes within a timeout (no hang), and persists exactly one record. `failed_gate_record_persist_is_retried_on_replay` covers the transient-fault cleanup path through the guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reborn): share the ProductLive gate store with the turn executor too (#6287 IronLoop) The prior ProductLive wiring fix only wired the gate-record store into the capability port (the persist side). The turn executor (`RebornTurnRunExecutor`, via `DefaultPlannedRuntimeParts.gate_record_store`) still got `None`, so its render-from-record path (`enrich_auth_block_credential_requirements`, §5.2.9) could not read the record — an auth block was applied with empty requirements / failed the exit on the ProductLive path. The harness now shares ONE gate-record store between both sides: the store built for the capability port is captured into `turn_executor_gate_store` and passed to `DefaultPlannedRuntimeParts.gate_record_store`, so a persisted auth gate record round-trips to the executor read. Stays `None` for the non-ProductLive capability fakes (which do not persist gate records), preserving the executor's tolerant "no store wired" path; `inbound_turn_contract.rs` uses `EmptyCapabilityFactory`, so its `None` is correct and unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reborn): commit the dispatch reservation only after the replay-payload write (#6287 IronLoop) `guard.commit()` ran before the fallible `persist_replay_payload_for_fresh_gate` store write. On a transient store error the `?` returned with the dispatch reservation still `InFlight`, and the already-committed guard skipped its cleanup — stranding retries/duplicates of the same idempotency key waiting on the reservation forever. Move `guard.commit()` to AFTER the replay-payload write succeeds, so a store error leaves the guard uncommitted; its drop then clears the reservation and wakes waiters so one re-dispatches. The finish-runtime-outcome ordering is unchanged. Covered by `failed_gate_record_persist_is_retried_on_replay` and the `replayed_gate` seam tests (the reservation-clears-then-retry path). [skip-regression-check] the store-write-failure reservation-clear path is exercised by the existing retry/replay seam tests; a dedicated dispatch-cancel harness for this specific reordering is redundant with them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(reborn): producers emit Resolution directly; delete CapabilityOutcome (§5.3 Stage 2b — collapse complete) (#6293) * chore(reborn): anchor s2b worktree at flip base Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(reborn): add producer-facing Resolution constructors in ironclaw_turns (§5.3 Stage 2b) New run_profile::resolution module with completed/failed/spawned_process/ spawned_child_run (return Resolution), approval_required/auth_required/ resource_blocked/await_dependent_run/external_tool_pending (return GatedResolution), and denied (returns DeniedResolution). Moves the non-lossy redaction verbatim from resolution_mapping (ModelResultPreview 24 KiB word-boundary redaction, ModelFailureDiagnostic, deterministic GateRef::for_auth_gate auth key, resume-token carry, DependentRunResult inline staged result, DenyReason mapping). resolution_mapping:: capability_outcome_to_resolution is retained transitionally as a thin delegator so unmigrated producers keep compiling; RefBindings is now empty (loop refs ride the channel origin). 17 retargeted constructor tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(reborn): loop_host producers emit Resolution directly (§5.3 Stage 2b) capability_port.rs: invoke_capability_dispatch now returns GatedResolution; the seam persists the durable GateRecord from .gate_record (GateRef::for_auth_gate key + replay-payload persistence intact) and returns .resolution — deleting the capability_outcome_to_resolution re-map. runtime_outcome_to_loop and the synthetic/inline dispatch arms emit Resolution/GatedResolution via the new constructors; the runtime failure classifier returns a private LoopFailureClass (Failed | Denied) so the per-tool display preview still stages from the raw fields. lib.rs empty-surface denial, subagent_spawn_port (spawn gates + batch coalescing keyed on the DependentRun channel origin), and capability_surface_filter denials all emit Resolution directly. 378 loop_host tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(reborn): hooks middleware emits Resolution directly (§5.3 Stage 2b) HookedLoopCapabilityPort::decision_to_outcome and fail_closed_gate_ref_unavailable now return host_api Resolution (deny/approval/auth) via the producer constructors instead of building CapabilityOutcome and re-mapping; single and batch invoke paths consume the Resolution directly. Test double emits resolution::completed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(reborn): local_dev synthetic capabilities emit Resolution directly (§5.3 Stage 2b) SyntheticCapabilityHandler::invoke now returns host_api Resolution; the synthetic port and external-tool port delegate it straight through (no capability_outcome_to_resolution re-map). outbound_delivery (approval gate + denial/failure paths, internal ApprovedResumeDecision + approval_lease_outcome now carry Resolution), skill_activation, project_create, result_read (parse error boxed for result_large_err), and external_tool_capability all build Resolution via the producer constructors. Tests assert Resolution::Done recoverable-failure verdicts and Resolution::Denied. 230 local_dev tests green; crate clippy -D warnings clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(reborn): runner tool-disclosure + subagent flavors emit Resolution directly (§5.3 Stage 2b) tool_disclosure_port bridge helpers (invoke_bridge/tool_search/describe/ describe_first/completed_bridge_result, failed_invalid_input) and the test double now build host_api Resolution via the producer constructors; subagent flavors test double likewise. No capability_outcome_to_resolution re-map remains in runner producers. Runner tests + clippy -D warnings green (all-features). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(reborn): agent_loop executor test fixtures build Resolution via constructors (§5.3 Stage 2b) shared_await_dependent_gate fixtures (await_dependent/completed/approval) now use the producer constructors directly instead of mapping a CapabilityOutcome. 401 agent_loop tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn): migrate agent_loop capability fixtures off CapabilityOutcome (§5.3 Stage 2b) MockHost stores Resolution/ResolutionBatch directly; scripted_capability_outcome maps ScriptedCapabilityOutcome -> Resolution via the producer constructors; ~90 executor test fixtures rebuilt as resolution::* + ironclaw_host_api::ResolutionBatch (transformed by a comment/brace-aware script). resolution_from_scripted_outcome deleted; resolution_batch_from_scripted takes Resolutions. 401 agent_loop tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(reborn): delete CapabilityOutcome and the transitional mapping (§5.3 Stage 2b — collapse complete) Deletes CapabilityOutcome, CapabilityBatchOutcome, CapabilityResultMessage, CapabilityFailure, CapabilityDenied, ProcessHandleSummary from run_profile::host, the resolution_mapping delegator (capability_outcome_to_resolution / MappedResolution / RefBindings), and their re-exports. Retargets the last serde-fixture tests (turn_coordinator, agent_loop_host_contract, content_digest) onto the surviving vocabulary (CapabilityDeniedReasonKind / CapabilityProgress / Resolution). CapabilityApprovalResume/AuthResume/ResumeToken kept (resume requests). Architecture ratchet trims CapabilityOutcome from FROZEN_COLLAPSE_DTOS per its own shrink-only instructions. No production code references CapabilityOutcome; turns + architecture ratchet green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(reborn): retain CapabilityResultMessage/CapabilityFailure as executor-internal vocabulary Scope correction: these two are NOT dead payloads — the agent_loop executor reconstructs them from host_api::Outcome (capability_result_from_outcome / capability_failure_from_recoverable, added by the flip) and consumes them across gates/capability_helpers/strategies. They are loop-internal working types now (no producer emits them; documented as such). The genuinely-dead CapabilityOutcome/CapabilityBatchOutcome/CapabilityDenied/ProcessHandleSummary stay deleted. Workspace builds clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn): migrate integration harness capability double off CapabilityOutcome (§5.3 Stage 2b) RecordingTestCapabilityPort (the shared tests/integration double) builds host_api::Resolution via the producer constructors; completed_result returns Resolution, approval/echo paths emit resolution::approval_required/completed directly. Unblocks every reborn_integration_* binary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(reborn): refresh loop_host seam comments post-collapse (§5.3 Stage 2b) The seam TODO to delete CapabilityOutcome is done; dispatch emits GatedResolution directly. Comment-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(reborn): cargo fmt the collapse tree (import ordering + line wrapping) Formatting-only. The capability-result collapse and the -s ours main merge left import ordering and long-line wrapping unformatted across the loop-host, agent-loop executor tests, composition local_dev, and turns run_profile files; `cargo fmt --all` resolves them. No logic change. Fixes the red Formatting lane on PR #6299. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reborn): retain gate-record + replay-payload stores on ProductLivePlannedRuntimeAdapters (#6299 IronLoop) `ProductLivePlannedRuntimeAdapters::from_services` moved the host-private `gate_record_store` / `replay_payload_store` into the capability factory but did not retain them on the bundle. The bundle is the production API for composing a product-live planned runtime, so a consumer building the runner (`DefaultPlannedRuntimeParts`) from it had no way to wire the SAME store the capability port persists `GateRecord::Auth` into — it would pass `gate_record_store: None`, and on an `AuthRequired` resume the turn executor reloads an empty requirement set and applies an unactionable auth block (the §5.2.9 render-from-record seam, same class as #6287 IronLoop f6/f8). Expose both stores as `pub` fields, populated from cheap `Arc` clones taken before the originals move into the factory, so both sides share one durable store. Additive, no behavior change to existing consumers. Note: the only current bundle consumer (the product-workflow planned-loop test harness) extracts just `.capability_factory`; its runner already wires the same store via `turn_executor_gate_store`, so the end-to-end product-live auth-gate tests through that path pass unchanged. This closes the bundle's API gap so a future/production runner consumer wires the store, not None. cargo check + clippy (test-support,libsql) clean on ironclaw_reborn_composition. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
teknium1
added a commit
to NousResearch/hermes-agent
that referenced
this pull request
Jul 27, 2026
…cretary, tokenizer, author=) Port from nearai/ironclaw#6129: their sensitive-marker scrubber matched markers as bare substrings, so tool results containing 'Secretary of the Treasury' were scrubbed as 'secret' on replay, evicting legitimate content and forcing the model into a re-fetch loop. Hermes' lowercase/dotted/YAML config-key redaction patterns (_CFG_DOTTED_RE, _CFG_ANCHORED_RE, _YAML_ASSIGN_RE) had the same false-positive class: their key classes allow arbitrary alphanumeric affixes around the keyword, so ordinary document text like 'Secretary: J.Smith', 'tokenizer: cl100k_base' (HF model cards), and BibTeX 'author=Smith' got value-masked on the surfaces that run these passes (browser snapshots, log lines, kanban summaries, CLI-echoed output). Fix: post-match word-boundary validation of the keyword occurrence inside the matched key. Boundaries: key edges, non-letters (_ - . digits), camelCase transitions (clientSecret, secretKey, APIToken), plural 's' (secrets:, tokens:). Concatenated real-world compounds keep matching via explicit alternatives (authtoken, authkey, secretkey, accesstoken). ALL-CAPS keys keep legacy embedded matching (MYTOKEN=...) — all-caps is almost never prose, same rationale as _ENV_ASSIGN_RE. Same discipline the file already applies to exact-match body/query keys (ported from ironclaw#2529) and the deliberate 'auth' exclusion that keeps 'author:' from matching.
29 tasks
randlee
pushed a commit
to randlee/hermes-agent
that referenced
this pull request
Aug 11, 2026
…cretary, tokenizer, author=) Port from nearai/ironclaw#6129: their sensitive-marker scrubber matched markers as bare substrings, so tool results containing 'Secretary of the Treasury' were scrubbed as 'secret' on replay, evicting legitimate content and forcing the model into a re-fetch loop. Hermes' lowercase/dotted/YAML config-key redaction patterns (_CFG_DOTTED_RE, _CFG_ANCHORED_RE, _YAML_ASSIGN_RE) had the same false-positive class: their key classes allow arbitrary alphanumeric affixes around the keyword, so ordinary document text like 'Secretary: J.Smith', 'tokenizer: cl100k_base' (HF model cards), and BibTeX 'author=Smith' got value-masked on the surfaces that run these passes (browser snapshots, log lines, kanban summaries, CLI-echoed output). Fix: post-match word-boundary validation of the keyword occurrence inside the matched key. Boundaries: key edges, non-letters (_ - . digits), camelCase transitions (clientSecret, secretKey, APIToken), plural 's' (secrets:, tokens:). Concatenated real-world compounds keep matching via explicit alternatives (authtoken, authkey, secretkey, accesstoken). ALL-CAPS keys keep legacy embedded matching (MYTOKEN=...) — all-caps is almost never prose, same rationale as _ENV_ASSIGN_RE. Same discipline the file already applies to exact-match body/query keys (ported from ironclaw#2529) and the deliberate 'auth' exclusion that keeps 'author:' from matching.
sanshi2018
pushed a commit
to sanshi2018/hermes-agent
that referenced
this pull request
Aug 18, 2026
…cretary, tokenizer, author=) Port from nearai/ironclaw#6129: their sensitive-marker scrubber matched markers as bare substrings, so tool results containing 'Secretary of the Treasury' were scrubbed as 'secret' on replay, evicting legitimate content and forcing the model into a re-fetch loop. Hermes' lowercase/dotted/YAML config-key redaction patterns (_CFG_DOTTED_RE, _CFG_ANCHORED_RE, _YAML_ASSIGN_RE) had the same false-positive class: their key classes allow arbitrary alphanumeric affixes around the keyword, so ordinary document text like 'Secretary: J.Smith', 'tokenizer: cl100k_base' (HF model cards), and BibTeX 'author=Smith' got value-masked on the surfaces that run these passes (browser snapshots, log lines, kanban summaries, CLI-echoed output). Fix: post-match word-boundary validation of the keyword occurrence inside the matched key. Boundaries: key edges, non-letters (_ - . digits), camelCase transitions (clientSecret, secretKey, APIToken), plural 's' (secrets:, tokens:). Concatenated real-world compounds keep matching via explicit alternatives (authtoken, authkey, secretkey, accesstoken). ALL-CAPS keys keep legacy embedded matching (MYTOKEN=...) — all-caps is almost never prose, same rationale as _ENV_ASSIGN_RE. Same discipline the file already applies to exact-match body/query keys (ported from ironclaw#2529) and the deliberate 'auth' exclusion that keeps 'author:' from matching. (cherry picked from commit b41eee4)
sanshi2018
pushed a commit
to sanshi2018/hermes-agent
that referenced
this pull request
Aug 18, 2026
…cretary, tokenizer, author=) Port from nearai/ironclaw#6129: their sensitive-marker scrubber matched markers as bare substrings, so tool results containing 'Secretary of the Treasury' were scrubbed as 'secret' on replay, evicting legitimate content and forcing the model into a re-fetch loop. Hermes' lowercase/dotted/YAML config-key redaction patterns (_CFG_DOTTED_RE, _CFG_ANCHORED_RE, _YAML_ASSIGN_RE) had the same false-positive class: their key classes allow arbitrary alphanumeric affixes around the keyword, so ordinary document text like 'Secretary: J.Smith', 'tokenizer: cl100k_base' (HF model cards), and BibTeX 'author=Smith' got value-masked on the surfaces that run these passes (browser snapshots, log lines, kanban summaries, CLI-echoed output). Fix: post-match word-boundary validation of the keyword occurrence inside the matched key. Boundaries: key edges, non-letters (_ - . digits), camelCase transitions (clientSecret, secretKey, APIToken), plural 's' (secrets:, tokens:). Concatenated real-world compounds keep matching via explicit alternatives (authtoken, authkey, secretkey, accesstoken). ALL-CAPS keys keep legacy embedded matching (MYTOKEN=...) — all-caps is almost never prose, same rationale as _ENV_ASSIGN_RE. Same discipline the file already applies to exact-match body/query keys (ported from ironclaw#2529) and the deliberate 'auth' exclusion that keeps 'author:' from matching. (cherry picked from commit b41eee4)
sanshi2018
pushed a commit
to sanshi2018/hermes-agent
that referenced
this pull request
Aug 18, 2026
…cretary, tokenizer, author=) Port from nearai/ironclaw#6129: their sensitive-marker scrubber matched markers as bare substrings, so tool results containing 'Secretary of the Treasury' were scrubbed as 'secret' on replay, evicting legitimate content and forcing the model into a re-fetch loop. Hermes' lowercase/dotted/YAML config-key redaction patterns (_CFG_DOTTED_RE, _CFG_ANCHORED_RE, _YAML_ASSIGN_RE) had the same false-positive class: their key classes allow arbitrary alphanumeric affixes around the keyword, so ordinary document text like 'Secretary: J.Smith', 'tokenizer: cl100k_base' (HF model cards), and BibTeX 'author=Smith' got value-masked on the surfaces that run these passes (browser snapshots, log lines, kanban summaries, CLI-echoed output). Fix: post-match word-boundary validation of the keyword occurrence inside the matched key. Boundaries: key edges, non-letters (_ - . digits), camelCase transitions (clientSecret, secretKey, APIToken), plural 's' (secrets:, tokens:). Concatenated real-world compounds keep matching via explicit alternatives (authtoken, authkey, secretkey, accesstoken). ALL-CAPS keys keep legacy embedded matching (MYTOKEN=...) — all-caps is almost never prose, same rationale as _ENV_ASSIGN_RE. Same discipline the file already applies to exact-match body/query keys (ported from ironclaw#2529) and the deliberate 'auth' exclusion that keeps 'author:' from matching. (cherry picked from commit b41eee4)
jh1nresh
pushed a commit
to jh1nresh/hermes-agent
that referenced
this pull request
Aug 26, 2026
…cretary, tokenizer, author=) Port from nearai/ironclaw#6129: their sensitive-marker scrubber matched markers as bare substrings, so tool results containing 'Secretary of the Treasury' were scrubbed as 'secret' on replay, evicting legitimate content and forcing the model into a re-fetch loop. Hermes' lowercase/dotted/YAML config-key redaction patterns (_CFG_DOTTED_RE, _CFG_ANCHORED_RE, _YAML_ASSIGN_RE) had the same false-positive class: their key classes allow arbitrary alphanumeric affixes around the keyword, so ordinary document text like 'Secretary: J.Smith', 'tokenizer: cl100k_base' (HF model cards), and BibTeX 'author=Smith' got value-masked on the surfaces that run these passes (browser snapshots, log lines, kanban summaries, CLI-echoed output). Fix: post-match word-boundary validation of the keyword occurrence inside the matched key. Boundaries: key edges, non-letters (_ - . digits), camelCase transitions (clientSecret, secretKey, APIToken), plural 's' (secrets:, tokens:). Concatenated real-world compounds keep matching via explicit alternatives (authtoken, authkey, secretkey, accesstoken). ALL-CAPS keys keep legacy embedded matching (MYTOKEN=...) — all-caps is almost never prose, same rationale as _ENV_ASSIGN_RE. Same discipline the file already applies to exact-match body/query keys (ported from ironclaw#2529) and the deliberate 'auth' exclusion that keeps 'author:' from matching.
prmartinow
pushed a commit
to prmartinow/hermes-agent
that referenced
this pull request
Aug 26, 2026
…cretary, tokenizer, author=) Port from nearai/ironclaw#6129: their sensitive-marker scrubber matched markers as bare substrings, so tool results containing 'Secretary of the Treasury' were scrubbed as 'secret' on replay, evicting legitimate content and forcing the model into a re-fetch loop. Hermes' lowercase/dotted/YAML config-key redaction patterns (_CFG_DOTTED_RE, _CFG_ANCHORED_RE, _YAML_ASSIGN_RE) had the same false-positive class: their key classes allow arbitrary alphanumeric affixes around the keyword, so ordinary document text like 'Secretary: J.Smith', 'tokenizer: cl100k_base' (HF model cards), and BibTeX 'author=Smith' got value-masked on the surfaces that run these passes (browser snapshots, log lines, kanban summaries, CLI-echoed output). Fix: post-match word-boundary validation of the keyword occurrence inside the matched key. Boundaries: key edges, non-letters (_ - . digits), camelCase transitions (clientSecret, secretKey, APIToken), plural 's' (secrets:, tokens:). Concatenated real-world compounds keep matching via explicit alternatives (authtoken, authkey, secretkey, accesstoken). ALL-CAPS keys keep legacy embedded matching (MYTOKEN=...) — all-caps is almost never prose, same rationale as _ENV_ASSIGN_RE. Same discipline the file already applies to exact-match body/query keys (ported from ironclaw#2529) and the deliberate 'auth' exclusion that keeps 'author:' from matching.
melon-xf
added a commit
to melon-xf/hermes-agent
that referenced
this pull request
Sep 3, 2026
…cretary, tokenizer, author=) Port from nearai/ironclaw#6129: their sensitive-marker scrubber matched markers as bare substrings, so tool results containing 'Secretary of the Treasury' were scrubbed as 'secret' on replay, evicting legitimate content and forcing the model into a re-fetch loop. Hermes' lowercase/dotted/YAML config-key redaction patterns (_CFG_DOTTED_RE, _CFG_ANCHORED_RE, _YAML_ASSIGN_RE) had the same false-positive class: their key classes allow arbitrary alphanumeric affixes around the keyword, so ordinary document text like 'Secretary: J.Smith', 'tokenizer: cl100k_base' (HF model cards), and BibTeX 'author=Smith' got value-masked on the surfaces that run these passes (browser snapshots, log lines, kanban summaries, CLI-echoed output). Fix: post-match word-boundary validation of the keyword occurrence inside the matched key. Boundaries: key edges, non-letters (_ - . digits), camelCase transitions (clientSecret, secretKey, APIToken), plural 's' (secrets:, tokens:). Concatenated real-world compounds keep matching via explicit alternatives (authtoken, authkey, secretkey, accesstoken). ALL-CAPS keys keep legacy embedded matching (MYTOKEN=...) — all-caps is almost never prose, same rationale as _ENV_ASSIGN_RE. Same discipline the file already applies to exact-match body/query keys (ported from ironclaw#2529) and the deliberate 'auth' exclusion that keeps 'author:' from matching.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
#5902 (fixing #5838's compaction crash) moved tool results behind a bounded preview +
result_readreference. Two regressions surfaced on read-heavy suites (OfficeQA, PinchBench doc tasks); the second is the dominant one and is why the cap bump alone didn't recover scores.Bug 1 — preview/chunk cap too small
Preview dropped 100,000 → 2,048 bytes;
result_readpaged at the same 2KB. Recovering a large result took dozens of manualresult_readcalls most policies won't do.Bug 2 — sensitive-marker substring scrub (dominant) 🔴
The replay validator matched
SENSITIVE_OBSERVATION_MARKERSas raw substrings. The list contains"secret", and"secretary".contains("secret")istrue. Every OfficeQA tool result contains "Secretary of the Treasury", so on every replay the model-observation was flagged and scrubbed to a bare stub — first-look previews andresult_readchunk observations alike (both flow through the samenormalized_model_observation→validate_model_observationscrub). The model lost what it just read, re-issued the identical command to get it back, lost it again: an amnesia loop — identical commands re-run 16–26×,result_readoffset=0 storms, 3–10× calls/cost, timeouts that starved the run. Happens regardless of cap size — which is why the cap bump alone left tasks failing. Diagnostic tell:RUST_LOG=ironclaw_threads=debug→scrubbed unsafe model-observation fields ... sensitive marker 'secret'.The fix
contains_marker_at_word_boundary) — a marker only trips as a standalone token, never inside "Secretary". Delimiter-bounded markers (bearer,authorization:) and real standalone credentials (client secret) still match. Applied to all three marker sites (summary + observation).2 × previewso it can never fall behind the preview and start dropping large observations to stubs (the fix: keep LocalDev tool results out of model context #5902 drift)."Issues #1/#2" (paged content lost / can't reconstruct) fold into the marker fix
A
result_readchunk observation flows through the same normalize/scrub path as the first-look preview — the scrub was what evicted paged chunks, notInlineOnly(which only skips re-storing a slice of the already-durable original). Fixing the matcher keeps paged chunks intact too; no separate change. Pinned by tests below.Deterministic regression tests — run natively in CI, no bespoke wiring
These are plain
#[test]s inironclaw_threads+ironclaw_reborn_composition, both in theironclaw_reborn_clidependency closure that seeds thereborn-tests.ymlpackage matrix (andreborn_compositionalso runs intest.yml).regression-test-check.ymladditionally enforces bug-fix PRs carry added tests.sensitive_markers_match_on_word_boundary_not_substringdocument_content_preview_is_retained_on_replay_not_scrubbedresult_readchunk retainedresult_read_chunk_observation_with_document_content_is_retainedlocal_dev_result_read_continues_exactly_where_first_look_preview_truncatedobservation_envelope_cap_covers_the_preview_capfull_cap_preview_survives_replayProof (pinned-commit A/B, deepseek-v4-flash / OpenRouter, OfficeQA)
Root-caused by building the bench against pre-5902
808b9a71vs HEAD (same task/model/env, only ironclaw differs), then confirming with the fix:secretscrub eventsresult_readcallsNote: exact per-task scores fluctuate with deepseek run-to-run variance (n=1 is noisy); the deterministic facts — scrub gone, calls crushed, retention invariants — are what the tests above pin.
Test plan
cargo test -p ironclaw_threads— 82/82 (incl. all retention/marker tests)cargo test -p ironclaw_reborn_composition --lib runtime::local_dev— pagination/retention greenresult_readpages cleanly🤖 Generated with Claude Code