feat(reborn): Context management — progressive tool disclosure (flag-gated, default off)

[serrrfirat]

Summary

Cuts the per-call prompt that drives NEAR AI latency/timeouts. From production logs, every model call shipped all ~91 tool schemas + system prompt + history ≈ 25.8k tokens, re-sent ~4× per turn — pushing NEAR AI past its 120s request timeout (retries exhausted → no answer). This PR adds progressive tool disclosure (Hermes-style) behind a flag, plus shadow measurement, and documents that compression/caching already exist in the codebase.

Everything new is behind REBORN_TOOL_DISCLOSURE (off | bridged), default off ⇒ byte-identical request path when off.

What's here (by commit)

• Design doc (docs/plans/2026-06-23-hermes-style-context-management.md) — fusion-council design (Opus 4.8 + GPT-5.5), signed off.
• Phase 0 — shadow measurement (zero behavior change): parse NEAR AI prompt_tokens_details.cached_tokens (+ top-level fallback) into cache_read_input_tokens; per-call debug! on target: ironclaw::reborn::context_shadow of prompt/cached tokens + tool-schema token estimate.
• Phase 1.1 — disclosure catalog + selector + token benchmark: CapabilityCatalog, select_active_set (bridges-always + append-only earned promotion, canonical never-resorted order), 3 bridge tool defs, deterministic tool_search_rank. Benchmark: 21,240 → 1,427 tool-schema tokens = 93.3% reduction on a representative 91-tool fixture.
• Phase 1.2 — live wiring + bridge execution: a LoopCapabilityPort decorator (ToolDisclosureCapabilityDecorator). visible_capabilities() filters to the active set; bridge tool_call routes its synthetic target through inner.validate/register/invoke — same audit/safety/approval/validation as a direct call. Guards reject recursion / unknown / not-disclosed-this-turn as model-recoverable InvalidInput failures (never run death). Append-only per-thread promotion.
• Safety tests: recursion + unknown-target tool_call → recoverable failure with the inner port never dispatched; flag-off defaults verified.

Behavior of disclosure (flag = bridged)

off       →  [ 91 full tool schemas ]                              ~21k tokens
bridged   →  [ ~10 core tools ] + [ tool_search·describe·call ]     ~1.4k tokens
                                    └─ discover/promote the rest on demand

Core set (always advertised): tool_search, tool_describe, tool_call, result_read, memory_search/read/write, skill_search, file_read, list_dir, plus profile pins. Deliberately read-leaning; side-effecting tools (shell/write_file/patch/http) are discoverable + promoted on use. Membership is telemetry-tunable.

Plan premises corrected during implementation

Grounding in the real code surfaced that two design phases already exist and were not reimplemented:

• Phase 3 (context compression) is already shipped: ActiveTaskPreservingCompactionStrategy + BudgetStrategy + summary-only LoopContextMessage refs. A from-scratch compressor was prototyped, found to be the wrong layer (LoopModelMessage is {role, content_ref} — content already externalized), and discarded.
• Phase 2 (stable prefix) is largely pre-existing: deterministic InstructionBundleFingerprint assembly, now reinforced by the deterministic memoized tool list; cached_tokens measurement landed in Phase 0.

Validation

• cargo fmt; cargo clippy -p ironclaw_reborn --features root-llm-provider --tests (zero warnings); default + root-llm-provider builds.
• ironclaw_reborn lib suite: 270 passed, incl. the disclosure selector tests, the through-the-caller bridge dispatch test, and the negative-path/flag-off tests.
• Token benchmark via cargo test -p ironclaw_reborn --lib tool_disclosure -- --nocapture.

Remaining (operational, not in this PR)

Enabling disclosure in production is a flagged canary: set REBORN_TOOL_DISCLOSURE=bridged on one deployment, validate against the Phase-0 shadow logs (real token delta + whether NEAR populates cached_tokens), gated on PinchBench ≥ 0.768. Core-tool membership to be tuned from the resulting telemetry.

Live results & hardening (update — 2026-06-25)

Since the original write-up the flag has been exercised live (profile local-dev, against NEAR AI) on real tasks, and a chain of run-borks found only under a live model has been fixed at source. The mechanism now works end-to-end on a general model.

Tool-schema token reduction — measured

The advertised floor (core + 3 bridges ≈ 3.1k tokens) is roughly constant, so the % reduction grows with catalog size:

Setting	Catalog	Advertised	Schema tokens	Reduction
Synthetic benchmark (full core)	91	core + bridges	21,240 → 1,427	93.3%
Synthetic benchmark (20-tool core)	91	20 + bridges	21,131 → 3,155	85.1%
Live, deepseek-V4-Flash	45	19	7,567 → 3,287	56.6%
Live, gpt-5.5 (marker index-names-only-v16)	81	24	12,352 → 4,690	62.0%

Live numbers come straight from the Phase-0 shadow log (target: ironclaw::reborn::context_shadow, reduction_pct=… per turn).

NEAR AI prefix caching — confirmed

The original open empirical-gate question — does NEAR actually cache the stable prefix? — is answered yes. Within a single turn, live cached_tokens climbed 0 → 256 → 6,912 ≈ 97% cache hit on the stable prefix. Stable-prefix discipline (deferred tools never re-sorted, bridges-always, append-only promotion) pays off on NEAR's vLLM/SGLang backend.

End-to-end behavior

Two failures that only appeared under a live model, both now fixed:

1. Discovery gap — a general model on a non-coding task ("what's on my calendar") burned ~26 turns of glob/read_file/grep and never called tool_search: it can't reach for tools it can't see. Fixed by moving discovery into the harness — the tool_search description now indexes every deferred tool by exact name (names-only, byte-budgeted under the 4096-byte capability-description cap), and the callable set unions the synthesized bridges back in so the visible-surface filter can't strip them. gpt-5.5 now discovers, describes, and promotes the Google tools (calendar.list_events, docs.get_document, drive.list_files).

2. Blind-call run-bork → describe-first — on "prepare a meeting brief", gpt-5.5 called docs.get_document / drive.get_file without the required id (deferred schemas meant it called by name alone), looped ~30 identical invalid_input failures, and the run died Failed at the iteration cap. This failure mode did not exist pre-disclosure, when the full schema was always in the tool list. Fix (tool_disclosure_port.rs): when the model uses a deferred tool whose schema it hasn't loaded this turn, probe-validate its arguments — valid → dispatch directly (zero round-trip), invalid → return the parameter schema instead of dispatching blind, one-shot per undisclosed tool so a still-invalid retry dispatches and fails normally (the no-progress detector still sees repeats). After this, the meeting-brief task completes successfully.

Still pending before this is merge-ready

• PinchBench ≥ 0.768 quality re-check — the name-index and activation enumeration touch the signed-off cache-stability path.
• Pre-GA cleanup currently on the branch for benchmarking: the temporary default-on, the disclosure_build startup markers, and the temporary direct-deferred diagnostics.
• Current state: ironclaw_reborn lib suite 285 passed, clippy clean.

🤖 Generated with Claude Code

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds cached-token parsing, disclosure-mode wiring, progressive tool surfacing, gateway resolution changes, repeated-error compaction, and a design plan.

Changes

Hermes-Style Context Management

Layer / File(s)	Summary
Token and usage capture crates/ironclaw_reborn/src/context_shadow.rs, crates/ironclaw_reborn/src/lib.rs, crates/ironclaw_llm/src/nearai_chat.rs, docs/plans/2026-06-23-hermes-style-context-management.md	Adds token estimation, cached-token extraction and tracing, and the Hermes-style context-management design document.
Disclosure catalog and runtime mode crates/ironclaw_reborn/src/tool_disclosure.rs, crates/ironclaw_reborn/src/runtime.rs, crates/ironclaw_reborn_composition/src/runtime_input.rs, crates/ironclaw_reborn_composition/src/runtime.rs, crates/ironclaw_reborn_composition/src/default_system_prompt.rs, crates/ironclaw_reborn_composition/assets/prompts/tool-disclosure-protocol.md, crates/ironclaw_reborn_composition/src/runtime/tests/default_system_prompt.rs, tests/support/reborn/qa_trace.rs	Adds the disclosure catalog, bridge tools, runtime disclosure mode, prompt append behavior, and runtime input wiring.
Callable surface propagation and gateway resolution crates/ironclaw_reborn/src/loop_driver_host.rs, crates/ironclaw_reborn/src/model_gateway.rs, crates/ironclaw_reborn/tests/llm_gateway.rs, crates/ironclaw_loop_support/src/capability_surface_filter.rs, crates/ironclaw_loop_support/src/lib.rs, crates/ironclaw_loop_support/src/capability_port.rs, crates/ironclaw_loop_support/src/subagent_spawn_port/tests.rs, crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs, crates/ironclaw_product_workflow/tests/support/planned_agent_loop.rs, crates/ironclaw_reborn/src/subagent/flavors.rs, crates/ironclaw_reborn/tests/hooks_integration.rs, crates/ironclaw_agent_loop/src/executor/prompt.rs, crates/ironclaw_agent_loop/src/executor/tests/support.rs, crates/ironclaw_agent_loop/src/test_support/mod.rs, crates/ironclaw_hooks/src/middleware/capability_port.rs, crates/ironclaw_hooks/src/middleware/tests/capability_port.rs, crates/ironclaw_turns/src/run_profile/host.rs, crates/ironclaw_turns/tests/agent_loop_host_contract.rs	Adds callable capability IDs to visible surfaces, delegates provider-call resolution to inner ports, and updates hidden-tool gateway handling and related tests.
Repeated error replay compaction crates/ironclaw_threads/src/tool_result_reference.rs, crates/ironclaw_reborn/src/model_gateway.rs	Adds repeated-error fingerprinting, marker replacement, replay collapse logic, and tests for the threshold behavior.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~90 minutes

Possibly related issues

• Reborn: regression test — every visible capability must be callable (visible_capabilities ⇔ tool_definitions parity) #4431: The changes touch the visible-versus-callable capability boundary and the new callable_capability_ids plumbing.

Possibly related PRs

• nearai/ironclaw#4871: Both PRs modify crates/ironclaw_reborn/src/model_gateway.rs’s message conversion path.
• nearai/ironclaw#3036: Same cached-token and tool-disclosure runtime area.
• nearai/ironclaw#3607: Related to the NEAR usage capture and disclosure catalog work.

Suggested reviewers

• henrypark133

Poem

Cached tokens whisper, tools step into view,
Bridges and prompts now travel as one through.
A shadow count, a stable path,
Then hidden tools answer the call at last. ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description has a solid summary and validation notes, but it omits most required template sections like change type, linked issue, security, rollback, and review track.	Reformat the PR description to match the template and add the missing sections: Change Type, Linked Issue, full Validation, Security/Trust-Boundary, Database Impact, Blast Radius, Rollback Plan, Review Follow-Through, and Review track.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The Conventional Commits-style title clearly matches the progressive tool disclosure change.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands.}

[gemini-code-assist]

Code Review

This pull request implements Hermes-Style Context Management for Reborn, introducing progressive tool disclosure, NEAR AI prompt caching, and context shadow usage measurements. Feedback on the changes focuses on several performance, correctness, and security improvements: pre-computing search terms in CatalogEntry to avoid redundant allocations during search scoring; canonicalizing JSON arguments before digesting to ensure deterministic identifiers; optimizing a nested loop in visible_capabilities using a BTreeSet; caching token estimates for static bridge tools; and using structured keys instead of raw string concatenation with a null separator to prevent potential key collisions.

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.

[gemini-code-assist]

Performance Optimization: Avoid Redundant HashSet Allocation and Parsing

In score_tool_entry, the blob_terms HashSet is parsed, allocated, and collected from entry.search_blob on every single call (which occurs for every tool in the catalog on every search query). With 91 tools, this results in significant garbage collection and CPU overhead during search operations.

Since CapabilityCatalog is built once and frozen, we can pre-compute and store the search_terms HashSet<String> directly inside CatalogEntry during catalog construction. This makes score_tool_entry completely allocation-free.

Suggested Refactoring:

1. Update CatalogEntry to store search_terms:

   pub(crate) struct CatalogEntry {
       definition: ProviderToolDefinition,
       est_schema_tokens: u32,
       search_blob: String,
       search_terms: HashSet<String>,
       tier: ToolTier,
   }

2. Pre-compute search_terms in CapabilityCatalog::new:

   let search_terms: HashSet<String> = search_blob
       .split_whitespace()
       .map(|term| term.trim_matches(|c: char| !c.is_alphanumeric() && c != '_').to_string())
       .filter(|term| !term.is_empty())
       .collect();

3. Simplify score_tool_entry to:

   fn score_tool_entry(entry: &CatalogEntry, query_terms: &[&str]) -> u32 {
       let mut keyword_score = 0_u32;
       for term in query_terms {
           if entry.definition.name.eq_ignore_ascii_case(term) || entry.search_terms.contains(*term) {
               keyword_score = keyword_score.saturating_add(10);
           } else if entry.search_blob.contains(term) {
               keyword_score = keyword_score.saturating_add(5);
           }
       }
       keyword_score.min(MAX_KEYWORD_SCORE)
   }

[gemini-code-assist]

Correctness & Determinism: Canonicalize JSON Arguments Before Digesting

The digest_input is derived directly from tool_call.arguments.to_string(). However, serde_json::Value::to_string() does not guarantee deterministic key ordering for JSON objects. If the keys are serialized in a different order, logically identical tool calls will produce different digests, leading to unstable input_ref identifiers and breaking the "stable-prefix discipline" required for prompt caching.

To ensure determinism, we should canonicalize the JSON arguments using the existing canonicalize_json helper before stringifying them.

        let canonical_args = crate::tool_disclosure::canonicalize_json(&tool_call.arguments);
        let digest_input = canonical_args.to_string();
        let digest = ironclaw_host_api::sha256_digest_token(digest_input.as_bytes());

[gemini-code-assist]

Performance Optimization: Avoid $O(N \times M)$ Nested Loop in visible_capabilities

In visible_capabilities, descriptors.retain performs a linear scan over surface.descriptors for every single descriptor. This results in an $O(N \times M)$ nested loop.

Since surface.descriptors can be collected into a BTreeSet (which is already imported) of capability IDs, we can optimize this lookup to $O(N \log M)$.

Suggested change

	descriptors.retain(|descriptor| {
	surface
	.descriptors
	.iter()
	.any(|inner| inner.capability_id == descriptor.capability_id)
	|| is_bridge_capability_id(&descriptor.capability_id)
	});
	let inner_ids: BTreeSet<&CapabilityId> = surface
	.descriptors
	.iter()
	.map(|d| &d.capability_id)
	.collect();
	descriptors.retain(|descriptor| {
	inner_ids.contains(&descriptor.capability_id)
	|| is_bridge_capability_id(&descriptor.capability_id)
	});

[gemini-code-assist]

Performance Optimization: Avoid Redundant Token Estimation for Static Bridge Tools

In select_active_set, estimate_definition_tokens(&definition) is called for each bridge tool definition on every single invocation. Since bridge_tool_definitions() returns static, pre-defined tools that never change, canonicalizing their schemas and estimating their tokens on every turn is redundant.

We could pre-compute these token estimates once (e.g., as a lazy static or constants) or cache them to avoid unnecessary JSON serialization and string processing. However, please note that performance optimizations, such as adding a caching layer, should be gated on profiling to ensure they provide a meaningful benefit.

References

1. Performance optimizations, such as adding a caching layer, should be gated on profiling, especially when existing mechanisms (e.g., ETags, short-circuiting for specific paths) already reduce overhead for common cases.

[gemini-code-assist]

Security & Robustness: Avoid Raw String Concatenation for Deterministic Identifiers

The digest_input is derived using raw string concatenation with a null separator:

let digest_input = format!("{}\0{}", tool_call.id, target.name);

To prevent key collisions, we should use structured keys instead of string concatenation with a separator.

Suggested Change:
Consider using a structured key format or tuple representation to avoid potential collisions.

References

1. To prevent key collisions, use structured keys instead of string concatenation with a separator.

[serrrfirat]

⚠️ Temporary change (commit 7aff964): tool disclosure now defaults ON when REBORN_TOOL_DISCLOSURE is unset — the original PR summary's "default off" no longer holds on this branch.

Reason: the PinchBench harness runs remotely and can't set env vars, so disclosure must be on without configuration to be benchmarked. Explicit REBORN_TOOL_DISCLOSURE=off is still the escape hatch (and the benchmark control arm).

This must be reverted before merge/GA — change the catch-all arm in ToolDisclosureMode::from_raw back to Self::Off. Tracked with a ⚠️ note in docs/plans/2026-06-23-hermes-style-context-management.md §13.

Do not merge to a shared environment while this is in place unless default-on is intended there.

[coderabbitai]

Actionable comments posted: 10

🤖 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.

Inline comments:
In `@crates/ironclaw_llm/src/nearai_chat.rs`:
- Around line 587-594: The cache_read_input_tokens field in the
CompletionResponse struct is being set to an unclamped cached_tokens value,
which can exceed the input_tokens count and cause incorrect billing/analytics
attribution. Clamp the cache_read_input_tokens to not exceed input_tokens by
using a minimum operation between the cached_tokens value and input_tokens.
Apply this same clamping fix in both locations mentioned: the CompletionResponse
initialization around line 593 and the second location around line 706.

In `@crates/ironclaw_reborn/src/runtime.rs`:
- Around line 106-112: The from_env method in the runtime module silently
defaults to Self::Off for any unrecognized environment variable value, which
masks typos and invalid configurations. Change the from_env method to return a
Result type instead of Self, and return an error (using a thiserror type with
context) when an invalid value is encountered for REBORN_TOOL_DISCLOSURE_ENV
instead of defaulting to Off. This way, invalid configuration becomes observable
during rollout rather than silently disabling the feature.

In `@crates/ironclaw_reborn/src/tool_disclosure_port.rs`:
- Around line 280-312: The promoted_by_thread HashMap is currently keyed only by
thread_id, which can cause scope collisions if thread IDs are reused across
tenants, projects, or agents. Create a scoped newtype key that includes the full
tenant/agent/project/thread identity instead of just thread_id. In both the
promoted_for_thread() and promote_target() methods, replace the simple
self.run_context.thread_id.to_string() key with a composite key that captures
the complete scope identity. This ensures that promotions in one scope do not
inadvertently affect the tool surface of another scope.
- Around line 171-172: All `.map_err(|_| invalid_invocation(...))` patterns
throughout the file discard the source error instead of preserving it as the
cause. Replace each instance by capturing the error variable with `|e|` instead
of `|_|` and include the original error in the context when calling
invalid_invocation or route through a helper function that preserves the cause.
This applies to all occurrences: the one at lines 171-172 in the target store
lock operation, the ones at lines 213-214, 254-258, 282-286, 306-310, 328-332,
and the one at 364-365. Ensure each `.map_err` call binds the actual error and
passes it along so the original cause is not lost.
- Around line 118-125: The code is using `let Ok(Some(...))` guard patterns
around calls to `allowed_tool_call_target()` which silently suppresses errors
instead of propagating them. Replace the combined error and option handling with
the `?` operator to propagate errors loudly, then branch only on the `Option`
type. In the block starting with `if tool_call.name == TOOL_CALL_NAME`, change
the guard condition to call `self.allowed_tool_call_target(tool_call)?` and use
`if let Some(target)` to handle the Option separately. Apply this same fix
pattern to the other occurrences at lines 150-154 and 165-177 where similar `let
Ok(Some(...))` patterns are used with `synthetic_target_call()` and other
fallible methods.
- Around line 192-202: Instead of replacing surface.descriptors with newly
constructed descriptors from the catalog, keep the existing inner
surface.descriptors and filter them to retain only those whose capability_id
matches an active/disclosed capability ID or is identified as a bridge
capability by is_bridge_capability_id. The active and disclosed capability IDs
can be determined from the descriptors returned by
state.catalog.active_or_disclosed_descriptors, and only append bridge
descriptors that are not already present in the filtered inner descriptors list.
- Around line 384-395: The query parameter extraction in this code block uses
unwrap_or_default() which silently converts a missing or non-string query into
an empty string. Instead, replace this logic with error handling that validates
the query is present and is a string, returning a recoverable InvalidInput error
if the validation fails, similar to how tool_describe handles malformed inputs.
Focus on the chain of get("query"), and_then(Value::as_str), and
unwrap_or_default() calls to implement proper validation instead of defaulting
to an empty string.
- Around line 326-339: The digest_input is currently computed from only
tool_call.arguments, which causes collisions when different bridge calls have
identical JSON arguments, leading to entries being overwritten in the
bridge_inputs map. Modify the digest_input variable to include the bridge name
(tool_call.name) and a unique call identifier along with the arguments before
passing it to ironclaw_host_api::sha256_digest_token. This will ensure each
BridgeInvocation has a unique input_ref based on both its content and identity,
preventing collisions in the bridge_inputs store.

In `@crates/ironclaw_reborn/src/tool_disclosure.rs`:
- Around line 277-346: In the `select_active_set` function, the deferred mode
path (after the initial check fails) appends Core tools, bridge tools, and
promoted tools without enforcing the `max_tools` or `max_tokens` caps from
`DisclosureCaps`. Add validation checks before appending each definition in the
loops that iterate over catalog entries, bridge_tool_definitions(), and promoted
items to ensure the advertised_tokens does not exceed
caps.defer_threshold_tokens() and the total definitions count does not exceed
caps.max_tools. If caps would be exceeded, either stop appending additional
definitions or implement an eviction policy as noted in the TODO comment.
- Around line 83-84: The logic that marks definitions as Core by checking
CORE_TOOL_NAMES needs to be adjusted to prevent reserved bridge names
(tool_search, tool_describe, tool_call) from colliding with real inner
capabilities. Either exclude these reserved bridge names from the catalog
entries before they are processed, or ensure bridges are appended before catalog
entries so their names take precedence in the deduplication logic of
append_definition(), or add explicit collision detection to reject definitions
using reserved names. This issue appears in the tier assignment logic around the
CORE_TOOL_NAMES check and also in the code region at lines 319-328.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0e3b723f-00ab-4c4b-ad4c-6b9fa642303d

📥 Commits

Reviewing files that changed from the base of the PR and between a0bd021 and 336aa16.

📒 Files selected for processing (8)

• crates/ironclaw_llm/src/nearai_chat.rs
• crates/ironclaw_reborn/src/context_shadow.rs
• crates/ironclaw_reborn/src/lib.rs
• crates/ironclaw_reborn/src/model_gateway.rs
• crates/ironclaw_reborn/src/runtime.rs
• crates/ironclaw_reborn/src/tool_disclosure.rs
• crates/ironclaw_reborn/src/tool_disclosure_port.rs
• docs/plans/2026-06-23-hermes-style-context-management.md

[serrrfirat]

/benchmark pinchbench --framework ironclaw-reborn

[github-actions]

🧪 Started pinchbench on ironclaw-reborn against ironclaw 7aff964cce — watch run.

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

docs/plans/2026-06-23-hermes-style-context-management.md (1)

125-125: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Rollout section says default-off while status section says current branch is default-on.

Please add an explicit exception note in §8 (or link to §13) so operators don’t apply the wrong default during canary setup.

Also applies to: 169-171

🤖 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 `@docs/plans/2026-06-23-hermes-style-context-management.md` at line 125, The
Rollout section at line 125 states that all phases are default-off, but this
contradicts the status section which indicates the current branch is default-on.
Add an explicit exception note in section 8 that clarifies this discrepancy and
specifies which default applies during canary setup, either by providing inline
clarification or by linking to section 13 for additional context. Apply the same
clarification to the related statements at lines 169-171 to ensure consistency
across all mentions of default behavior.

🤖 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.

Inline comments:
In `@crates/ironclaw_reborn/src/runtime.rs`:
- Around line 116-121: The from_raw method currently defaults
unset/empty/unrecognized values to Self::Bridged (ON state), but this was
explicitly marked as temporary for benchmarking and must be reverted before
merge. Change the catch-all pattern in the from_raw method to return Self::Off
instead of Self::Bridged to restore the correct default-off behavior.
Additionally, update the related unit tests (referenced in the also applies to
section around lines 775-796) that currently lock in the temporary ON behavior
to instead verify the correct default-off behavior for unset and unrecognized
values.
- Line 107: The line using
`std::env::var(REBORN_TOOL_DISCLOSURE_ENV).ok().as_deref()` silently discards
error information from VarError, violating the fail-loud invariant for settings
reads. Replace the `.ok().as_deref()` chain with proper error handling that
preserves error context, either by using `.as_deref()` directly on the Result
without the intermediate `.ok()` call, or by using the `?` operator to propagate
the error with appropriate context. Ensure that NotUnicode and other VarError
variants are not silently ignored during the environment variable read
operation.

---

Outside diff comments:
In `@docs/plans/2026-06-23-hermes-style-context-management.md`:
- Line 125: The Rollout section at line 125 states that all phases are
default-off, but this contradicts the status section which indicates the current
branch is default-on. Add an explicit exception note in section 8 that clarifies
this discrepancy and specifies which default applies during canary setup, either
by providing inline clarification or by linking to section 13 for additional
context. Apply the same clarification to the related statements at lines 169-171
to ensure consistency across all mentions of default behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 56e12771-c11d-4c3e-af76-5d5ffb4f2421

📥 Commits

Reviewing files that changed from the base of the PR and between 336aa16 and 7aff964.

📒 Files selected for processing (2)

• crates/ironclaw_reborn/src/runtime.rs
• docs/plans/2026-06-23-hermes-style-context-management.md

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Revert temporary default-on before merge; test currently cements non-GA behavior.

Catch-all from_raw → Bridged makes disclosure ON for unset/garbage values, and the unit test locks that behavior in. Your own plan/PR objective marks this as temporary and requires default-off for merge safety.

Suggested patch

 fn from_raw(raw: Option<&str>) -> Self {
     match raw {
         Some(value) if value.eq_ignore_ascii_case("off") => Self::Off,
         Some(value) if value.eq_ignore_ascii_case("bridged") => Self::Bridged,
-        // unset / empty / unrecognized -> temporarily ON for benchmarking.
-        _ => Self::Bridged,
+        _ => Self::Off,
     }
 }

-assert!(ToolDisclosureMode::from_raw(None).is_bridged());
-assert!(ToolDisclosureMode::from_raw(Some("")).is_bridged());
-assert!(ToolDisclosureMode::from_raw(Some("garbage")).is_bridged());
+assert!(!ToolDisclosureMode::from_raw(None).is_bridged());
+assert!(!ToolDisclosureMode::from_raw(Some("")).is_bridged());
+assert!(!ToolDisclosureMode::from_raw(Some("garbage")).is_bridged());

Also applies to: 775-796

🤖 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_reborn/src/runtime.rs` around lines 116 - 121, The from_raw
method currently defaults unset/empty/unrecognized values to Self::Bridged (ON
state), but this was explicitly marked as temporary for benchmarking and must be
reverted before merge. Change the catch-all pattern in the from_raw method to
return Self::Off instead of Self::Bridged to restore the correct default-off
behavior. Additionally, update the related unit tests (referenced in the also
applies to section around lines 775-796) that currently lock in the temporary ON
behavior to instead verify the correct default-off behavior for unset and
unrecognized values.

[railway-app]

🚅 Deployed to the ironclaw-pr-5149 environment in ironclaw-ci-preview

Service	Status	Web	Updated (UTC)
ironclaw	✅ Success (View Logs)	Web	Jun 25, 2026 at 7:32 pm

[serrrfirat]

Addressed all Gemini + CodeRabbit review comments in 5d8f068c1:

Correctness / fail-loud

• Propagate target-resolution errors with ? instead of swallowing them as "unknown target".
• Bind mapped errors to preserve the source cause (PoisonError / constructor errors).
• Reject missing/blank tool_search.query with a recoverable InvalidInput.
• Reserve bridge names — exclude inner tools named tool_search/tool_describe/tool_call from the catalog so a real tool can't shadow a bridge.
• Clamp cache_read_input_tokens to input_tokens (no over-attribution).
• Log unrecognized REBORN_TOOL_DISCLOSURE values via debug!.

Determinism / collision-safety

• Canonical-JSON + provider-call-id + tool-name in both the bridge-input and tool_call-target digests (order-stable, no overwrite of distinct calls with identical args).

Multi-tenant safety

• Scope promotions by tenant+agent+project+thread (was thread-id only).

Bounds / perf

• Enforce max_tools/token caps on promoted tools in deferred mode (core+bridges always kept; deterministic truncation).
• Filter the inner visible surface instead of reconstructing it — preserves provider/runtime/concurrency metadata and is O(N log M).
• Precompute per-entry search terms (alloc-free scoring) and cache bridge defs + token estimates via LazyLock.

Regression tests added for bridge-name reservation, scoped promotion, and caps. Verified: clippy clean (ironclaw_reborn + ironclaw_llm); ironclaw_reborn lib 274 passed, ironclaw_llm lib 884 passed.

(One item — the eviction/epoch-reset policy beyond the cap truncation — is left as a documented follow-up in design doc §13.)

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/ironclaw_reborn/src/tool_disclosure_port.rs (1)

182-183: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not let inner descriptors shadow bridge capability IDs.

Line 225 retains any inner descriptor whose capability_id is a bridge ID, and lines 234-237 then skip appending the canonical bridge descriptor. A non-bridge inner capability with a reserved bridge ID will surface wrong metadata and later be routed to invoke_bridge() by line 246 instead of the inner port. Drop inner bridge IDs before appending canonical bridge descriptors, and reject non-bridge inner candidates that return reserved bridge IDs.

Suggested shape

-        surface.descriptors.retain(|descriptor| {
-            active_or_disclosed_ids.contains(&descriptor.capability_id)
-                || is_bridge_capability_id(&descriptor.capability_id)
-        });
+        surface.descriptors.retain(|descriptor| {
+            active_or_disclosed_ids.contains(&descriptor.capability_id)
+                && !is_bridge_capability_id(&descriptor.capability_id)
+        });

Also applies to: 225-238, 246-263

🤖 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_reborn/src/tool_disclosure_port.rs` around lines 182 - 183,
Inner descriptors with reserved bridge capability IDs are not being filtered
out, causing wrong metadata to surface and incorrect routing to invoke_bridge.
When processing non-bridge inner tool calls in register_provider_tool_call,
validate that the inner descriptor returned does not have a capability_id
matching a reserved bridge ID and reject such candidates. Additionally, filter
out any inner descriptors with bridge capability IDs before appending canonical
bridge descriptors. This ensures bridge IDs are never retained from inner
descriptors and prevents shadowing of the correct bridge metadata and routing
behavior.

🤖 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.

Inline comments:
In `@crates/ironclaw_reborn/src/tool_disclosure_port.rs`:
- Around line 94-110: The PromotionScopeKey struct is storing all identifiers as
raw String types instead of using the existing typed ID newtypes. Replace the
String field types in the PromotionScopeKey struct definition with the
appropriate ID types (TenantId, Option<AgentId>, Option<ProjectId>, and
ThreadId). Then update the from_run_context method implementation to directly
use the typed IDs from the run_context scope instead of converting them to
strings via as_str().to_string() calls.

In `@crates/ironclaw_reborn/src/tool_disclosure.rs`:
- Line 83: The filter at line 83 in tool_disclosure.rs currently only excludes
tools by bridge name using is_bridge_name, but tools can also have bridge
capability IDs as a reserved routing namespace (as established at line 300).
Extend the filter condition to also exclude definitions with bridge capability
IDs by adding an additional check with
is_bridge_capability_id(&definition.capability_id) alongside the existing
is_bridge_name check, ensuring tools cannot bypass the normal dispatch path by
using a non-bridge name with a bridge CapabilityId.

---

Outside diff comments:
In `@crates/ironclaw_reborn/src/tool_disclosure_port.rs`:
- Around line 182-183: Inner descriptors with reserved bridge capability IDs are
not being filtered out, causing wrong metadata to surface and incorrect routing
to invoke_bridge. When processing non-bridge inner tool calls in
register_provider_tool_call, validate that the inner descriptor returned does
not have a capability_id matching a reserved bridge ID and reject such
candidates. Additionally, filter out any inner descriptors with bridge
capability IDs before appending canonical bridge descriptors. This ensures
bridge IDs are never retained from inner descriptors and prevents shadowing of
the correct bridge metadata and routing behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 21487f16-624e-4958-9885-3c2eefa08f74

📥 Commits

Reviewing files that changed from the base of the PR and between 7aff964 and 5d8f068.

📒 Files selected for processing (4)

• crates/ironclaw_llm/src/nearai_chat.rs
• crates/ironclaw_reborn/src/runtime.rs
• crates/ironclaw_reborn/src/tool_disclosure.rs
• crates/ironclaw_reborn/src/tool_disclosure_port.rs

[coderabbitai]

♻️ Duplicate comments (2)

crates/ironclaw_reborn/src/runtime.rs (1)

131-144: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Restore default-off before merge.

Unset/empty/garbage REBORN_TOOL_DISCLOSURE still enables Bridged, and the test locks in the temporary benchmark behavior.

Suggested patch

-            Some(value) if !value.is_empty() => {
+            Some(value) if !value.trim().is_empty() => {
                 tracing::debug!(
                     target: "ironclaw::reborn::runtime",
                     value,
-                    "unrecognized REBORN_TOOL_DISCLOSURE value; falling back to temporary default Bridged"
+                    "unrecognized REBORN_TOOL_DISCLOSURE value; falling back to Off"
                 );
-                Self::Bridged
+                Self::Off
             }
-            // unset / empty -> temporarily ON for benchmarking.
-            _ => Self::Bridged,
+            _ => Self::Off,

As per PR objectives, default-on is temporary and must be reverted before merge/GA.

Also applies to: 817-841

🤖 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_reborn/src/runtime.rs` around lines 131 - 144, Restore the
intended default-off behavior in RuntimeDisclosure::from_raw and related tests:
unset, empty, or unrecognized REBORN_TOOL_DISCLOSURE values should no longer
fall back to Bridged, and the temporary benchmarking default must be removed.
Update the match arms in from_raw to return Off for missing/empty/invalid input,
keep the explicit off/bridged mappings, and adjust the tests around
RuntimeDisclosure so they assert the production default rather than the
temporary benchmark setting.

crates/ironclaw_reborn/src/tool_disclosure_port.rs (1)

165-183: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Preserve inner capability expansion in the pre-check.

target.target_call is already canonicalized and is the same resolved call used by validate/register, so resolving IDs directly here can drop inner effective_capability_ids / approval expansion.

Suggested patch

-                return Ok(ProviderToolCallCapabilityIds::single(
-                    target.definition.capability_id,
-                ));
+                return self
+                    .inner
+                    .provider_tool_call_capability_ids(&target.target_call);
@@
-            return Ok(ProviderToolCallCapabilityIds {
-                provider_capability_id: target.definition.capability_id.clone(),
-                effective_capability_ids: vec![target.definition.capability_id],
-            });
+            return self
+                .inner
+                .provider_tool_call_capability_ids(&target.target_call);

As per coding guidelines, fail closed for auth/approvals.

🤖 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_reborn/src/tool_disclosure_port.rs` around lines 165 - 183,
The pre-check in tool_disclosure_port should not bypass the inner capability
expansion for canonicalized target.target_call, because returning only the
target.definition.capability_id drops the inner effective_capability_ids and
approval expansion used by validate/register. Update the TOOL_CALL_NAME path in
the provider_tool_call_capability_ids logic to preserve the inner
provider_tool_call_capability_ids result for the resolved target call while
still failing closed for auth/approvals, and use the existing
allowed_tool_call_target/inner capability lookup flow to keep the same canonical
call behavior.

Source: Coding guidelines

🤖 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_reborn/src/runtime.rs`:
- Around line 131-144: Restore the intended default-off behavior in
RuntimeDisclosure::from_raw and related tests: unset, empty, or unrecognized
REBORN_TOOL_DISCLOSURE values should no longer fall back to Bridged, and the
temporary benchmarking default must be removed. Update the match arms in
from_raw to return Off for missing/empty/invalid input, keep the explicit
off/bridged mappings, and adjust the tests around RuntimeDisclosure so they
assert the production default rather than the temporary benchmark setting.

In `@crates/ironclaw_reborn/src/tool_disclosure_port.rs`:
- Around line 165-183: The pre-check in tool_disclosure_port should not bypass
the inner capability expansion for canonicalized target.target_call, because
returning only the target.definition.capability_id drops the inner
effective_capability_ids and approval expansion used by validate/register.
Update the TOOL_CALL_NAME path in the provider_tool_call_capability_ids logic to
preserve the inner provider_tool_call_capability_ids result for the resolved
target call while still failing closed for auth/approvals, and use the existing
allowed_tool_call_target/inner capability lookup flow to keep the same canonical
call behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 749b0ec5-259e-4d77-afd5-e282231b8b97

📥 Commits

Reviewing files that changed from the base of the PR and between f9a436a and 0c34188.

📒 Files selected for processing (2)

• crates/ironclaw_reborn/src/runtime.rs
• crates/ironclaw_reborn/src/tool_disclosure_port.rs

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/ironclaw_reborn/src/runtime.rs (1)

131-144: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Trim before matching the off escape hatch.

REBORN_TOOL_DISCLOSURE="off " currently falls through to the unrecognized branch and enables Bridged. Normalize with trim() before matching so the documented escape hatch works reliably. The PR objective says REBORN_TOOL_DISCLOSURE=off remains the escape hatch.

Suggested patch

     fn from_raw(raw: Option<&str>) -> Self {
-        match raw {
+        match raw.map(str::trim) {
             Some(value) if value.eq_ignore_ascii_case("off") => Self::Off,
             Some(value) if value.eq_ignore_ascii_case("bridged") => Self::Bridged,

🤖 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_reborn/src/runtime.rs` around lines 131 - 144, The `from_raw`
parser in `runtime.rs` should trim the incoming `REBORN_TOOL_DISCLOSURE` value
before matching, because values like `off ` currently miss the `off` escape
hatch and fall into the fallback branch. Update the matching logic in `from_raw`
to normalize `raw` with `trim()` and then compare against `off` and `bridged`,
keeping the existing debug logging and `Self::Bridged` fallback for unrecognized
non-empty values.

🤖 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.

Outside diff comments:
In `@crates/ironclaw_reborn/src/runtime.rs`:
- Around line 131-144: The `from_raw` parser in `runtime.rs` should trim the
incoming `REBORN_TOOL_DISCLOSURE` value before matching, because values like
`off ` currently miss the `off` escape hatch and fall into the fallback branch.
Update the matching logic in `from_raw` to normalize `raw` with `trim()` and
then compare against `off` and `bridged`, keeping the existing debug logging and
`Self::Bridged` fallback for unrecognized non-empty values.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6a712098-3e9d-4104-ae66-2842d43c2db1

📥 Commits

Reviewing files that changed from the base of the PR and between 0c34188 and d167e71.

📒 Files selected for processing (2)

• crates/ironclaw_reborn/src/runtime.rs
• crates/ironclaw_reborn/src/tool_disclosure_port.rs

[coderabbitai]

Actionable comments posted: 2

🤖 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.

Inline comments:
In `@crates/ironclaw_reborn/src/model_gateway.rs`:
- Around line 1834-1884: There is only helper-level coverage for
repeated-failure compaction, so add a regression that exercises the real caller
path through convert_messages rather than collapse_repeated_failure_observations
directly. Use the existing test fixtures and a scenario with repeated error tool
results plus a success, then assert the resulting ChatMessage sequence contains
the repeated_error_elided marker in the interior duplicate after conversion.
Keep the helper tests if useful, but make the new test validate the behavior at
the convert_messages call site so the guard cannot be removed without failing.

In `@crates/ironclaw_threads/src/tool_result_reference.rs`:
- Around line 252-257: The error observation fingerprinting in
error_observation_fingerprint currently accepts any JSON with status: "error"
and fingerprints it directly, which can let malformed observations bypass the
safe-summary fallback. Add validation before returning a fingerprint by reusing
the same shape checks used by model_visible_content_or_safe_summary or a shared
helper, so only genuine error observations are accepted. Update
error_observation_fingerprint to return None for malformed error payloads, and
add a regression test covering malformed status: "error" observations producing
None.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 25728921-8a5c-40fc-893b-88b1a2b928a4

📥 Commits

Reviewing files that changed from the base of the PR and between d167e71 and c701c33.

📒 Files selected for processing (3)

• crates/ironclaw_reborn/src/model_gateway.rs
• crates/ironclaw_reborn/src/runtime.rs
• crates/ironclaw_threads/src/tool_result_reference.rs

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Test the compaction through convert_messages, not only the helper.

These assertions would still pass if Line 1385 were removed. Add a regression that calls convert_messages(...) and verifies the replayed ChatMessage contains the repeated-error marker for the interior duplicate. Repo invariant violated: test through the caller. As per path instructions, “Test through the caller: when a helper gates a side effect, require a test driving the real call site.”

🤖 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_reborn/src/model_gateway.rs` around lines 1834 - 1884, There
is only helper-level coverage for repeated-failure compaction, so add a
regression that exercises the real caller path through convert_messages rather
than collapse_repeated_failure_observations directly. Use the existing test
fixtures and a scenario with repeated error tool results plus a success, then
assert the resulting ChatMessage sequence contains the repeated_error_elided
marker in the interior duplicate after conversion. Keep the helper tests if
useful, but make the new test validate the behavior at the convert_messages call
site so the guard cannot be removed without failing.

Source: Path instructions

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Validate before fingerprinting error observations.

Line 254 accepts any JSON with status: "error", so three malformed observations get replaced by a valid marker before model_visible_content_or_safe_summary() can reject them and fall back to the safe summary. That masks corrupt model-visible data despite the “genuine errors” contract.

Suggested fix

 pub fn error_observation_fingerprint(&self) -> Option<String> {
     let observation = self.model_observation.as_ref()?;
+    if validate_model_observation(observation).is_err() {
+        return None;
+    }
     if observation.get("status").and_then(serde_json::Value::as_str) != Some("error") {
         return None;
     }
     Some(observation.to_string())
 }

Add a regression for malformed status: "error" observations returning None. Repo invariant violated: guarantees in comments must be enforced. As per coding guidelines, “Comments that promise guarantees across layers must either be enforced by code/tests or softened to describe intent.”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	pub fn error_observation_fingerprint(&self) -> Option<String> {
	let observation = self.model_observation.as_ref()?;
	if observation.get("status").and_then(serde_json::Value::as_str) != Some("error") {
	return None;
	}
	Some(observation.to_string())
	pub fn error_observation_fingerprint(&self) -> Option<String> {
	let observation = self.model_observation.as_ref()?;
	if validate_model_observation(observation).is_err() {
	return None;
	}
	if observation.get("status").and_then(serde_json::Value::as_str) != Some("error") {
	return None;
	}
	Some(observation.to_string())
	}

🤖 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 252 - 257,
The error observation fingerprinting in error_observation_fingerprint currently
accepts any JSON with status: "error" and fingerprints it directly, which can
let malformed observations bypass the safe-summary fallback. Add validation
before returning a fingerprint by reusing the same shape checks used by
model_visible_content_or_safe_summary or a shared helper, so only genuine error
observations are accepted. Update error_observation_fingerprint to return None
for malformed error payloads, and add a regression test covering malformed
status: "error" observations producing None.

Source: Coding guidelines

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/ironclaw_reborn/src/runtime.rs (1)

131-135: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Trim mode values before matching.

REBORN_TOOL_DISCLOSURE="off " is treated as unrecognized and falls through to Bridged, so the documented off escape hatch can fail open because of whitespace.

As per coding guidelines, “Fail closed for ... runtime selection.”

Suggested patch

     fn from_raw(raw: Option<&str>) -> Self {
+        let raw = raw.map(str::trim);
         match raw {
             Some(value) if value.eq_ignore_ascii_case("off") => Self::Off,
             Some(value) if value.eq_ignore_ascii_case("bridged") => Self::Bridged,

         assert!(ToolDisclosureMode::from_raw(Some("bridged")).is_bridged());
+        assert!(ToolDisclosureMode::from_raw(Some(" bridged ")).is_bridged());
         assert!(
             !ToolDisclosureMode::from_raw(Some("off")).is_bridged(),
             "explicit REBORN_TOOL_DISCLOSURE=off must still disable disclosure"
         );
+        assert!(!ToolDisclosureMode::from_raw(Some(" off ")).is_bridged());

Also applies to: 827-837

🤖 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_reborn/src/runtime.rs` around lines 131 - 135, Trim the
incoming value before matching in the `TrimMode::from_raw` logic so
whitespace-padded settings like `off ` are recognized correctly instead of
falling through to the default branch. Update the `from_raw` match in
`runtime.rs` to normalize `raw` before checking `off`, `bridged`, and the
non-empty fallback, and ensure any other runtime selection paths in the
referenced `827-837` section use the same trimmed handling so the selection
fails closed.

Source: Coding guidelines

🤖 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.

Outside diff comments:
In `@crates/ironclaw_reborn/src/runtime.rs`:
- Around line 131-135: Trim the incoming value before matching in the
`TrimMode::from_raw` logic so whitespace-padded settings like `off ` are
recognized correctly instead of falling through to the default branch. Update
the `from_raw` match in `runtime.rs` to normalize `raw` before checking `off`,
`bridged`, and the non-empty fallback, and ensure any other runtime selection
paths in the referenced `827-837` section use the same trimmed handling so the
selection fails closed.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 790ec2d1-da2c-4dd7-a523-d4d740974503

📥 Commits

Reviewing files that changed from the base of the PR and between c701c33 and 83ae258.

📒 Files selected for processing (2)

• crates/ironclaw_reborn/src/model_gateway.rs
• crates/ironclaw_reborn/src/runtime.rs

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (2)

crates/ironclaw_reborn/src/tool_disclosure_port.rs (1)

165-183: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Delegate resolved target IDs to the inner port.

Both forgiving paths collapse resolved targets to ProviderToolCallCapabilityIds::single(...). If the inner port expands effective_capability_ids for approval/policy, the gateway pre-check sees a narrower authority surface than registration/invocation.

Suggested patch

-                return Ok(ProviderToolCallCapabilityIds::single(
-                    target.definition.capability_id,
-                ));
+                return self
+                    .inner
+                    .provider_tool_call_capability_ids(&target.target_call);
@@
-            return Ok(ProviderToolCallCapabilityIds {
-                provider_capability_id: target.definition.capability_id.clone(),
-                effective_capability_ids: vec![target.definition.capability_id],
-            });
+            return self
+                .inner
+                .provider_tool_call_capability_ids(&target.target_call);

As per coding guidelines, “Fail closed for auth, approvals.”

🤖 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_reborn/src/tool_disclosure_port.rs` around lines 165 - 183,
The `provider_tool_call_capability_ids` fallback for resolved targets is
collapsing to `ProviderToolCallCapabilityIds::single(...)`, which bypasses any
broader `effective_capability_ids` the inner port may produce. Update the
`tool_disclosure_port` logic so the `TOOL_CALL_NAME`/`allowed_tool_call_target`
path delegates resolved target capability resolution to
`self.inner.provider_tool_call_capability_ids` (or otherwise preserves the inner
port’s expanded IDs) instead of forcing a single capability id, while keeping
the existing fail-closed gate for unresolved names.

Source: Coding guidelines

crates/ironclaw_reborn/src/runtime.rs (1)

131-144: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Revert the temporary default-on mode before merge.

from_raw(None), empty, and garbage values all return Bridged, and the test locks that in. Because Line 633 wires the decorator when bridged, this ships the new disclosure path enabled by default; the PR objective marks this as temporary benchmark behavior.

Suggested patch

 fn from_raw(raw: Option<&str>) -> Self {
-    match raw {
+    match raw.map(str::trim) {
         Some(value) if value.eq_ignore_ascii_case("off") => Self::Off,
         Some(value) if value.eq_ignore_ascii_case("bridged") => Self::Bridged,
         Some(value) if !value.is_empty() => {
             tracing::debug!(
                 target: "ironclaw::reborn::runtime",
                 value,
-                "unrecognized REBORN_TOOL_DISCLOSURE value; falling back to temporary default Bridged"
+                "unrecognized REBORN_TOOL_DISCLOSURE value; falling back to Off"
             );
-            Self::Bridged
+            Self::Off
         }
-        // unset / empty -> temporarily ON for benchmarking.
-        _ => Self::Bridged,
+        _ => Self::Off,
     }
 }

Also applies to: 817-841

🤖 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_reborn/src/runtime.rs` around lines 131 - 144, The temporary
default-on behavior in `from_raw` should be reverted before merge: `None`,
empty, and unrecognized `REBORN_TOOL_DISCLOSURE` values must no longer fall back
to `Bridged`. Update `from_raw` in `runtime.rs` so only an explicit bridged
setting maps to `Self::Bridged`, while unset/invalid inputs return the intended
non-disclosure default instead. Also adjust any related test coverage that
currently locks in the temporary default so it matches the restored behavior.

🤖 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.

Inline comments:
In `@crates/ironclaw_reborn/src/tool_disclosure_port.rs`:
- Around line 220-235: The validation fallback in the target registration flow
is too permissive: after a failed `validate_provider_tool_call` check,
`register_provider_tool_call` still runs and can admit invalid or
policy-rejected targets. Update the registration path in `tool_disclosure_port`
so `inner.register_provider_tool_call(...)` is only attempted when the target
has passed the inner validation, and ensure auth/approval/host-related failures
are propagated instead of downgraded; keep the bridge fallback only for
recoverable invalid input and use the existing `debug!`/validation branch to
gate the subsequent registration logic.

---

Duplicate comments:
In `@crates/ironclaw_reborn/src/runtime.rs`:
- Around line 131-144: The temporary default-on behavior in `from_raw` should be
reverted before merge: `None`, empty, and unrecognized `REBORN_TOOL_DISCLOSURE`
values must no longer fall back to `Bridged`. Update `from_raw` in `runtime.rs`
so only an explicit bridged setting maps to `Self::Bridged`, while unset/invalid
inputs return the intended non-disclosure default instead. Also adjust any
related test coverage that currently locks in the temporary default so it
matches the restored behavior.

In `@crates/ironclaw_reborn/src/tool_disclosure_port.rs`:
- Around line 165-183: The `provider_tool_call_capability_ids` fallback for
resolved targets is collapsing to `ProviderToolCallCapabilityIds::single(...)`,
which bypasses any broader `effective_capability_ids` the inner port may
produce. Update the `tool_disclosure_port` logic so the
`TOOL_CALL_NAME`/`allowed_tool_call_target` path delegates resolved target
capability resolution to `self.inner.provider_tool_call_capability_ids` (or
otherwise preserves the inner port’s expanded IDs) instead of forcing a single
capability id, while keeping the existing fail-closed gate for unresolved names.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c5538514-1a05-4d1d-b4a5-41a6489cd862

📥 Commits

Reviewing files that changed from the base of the PR and between 83ae258 and 519dbc7.

📒 Files selected for processing (2)

• crates/ironclaw_reborn/src/runtime.rs
• crates/ironclaw_reborn/src/tool_disclosure_port.rs

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@crates/ironclaw_loop_support/src/capability_surface_filter.rs`:
- Around line 202-208: CapabilitySurfaceDenyFilter is using
provider_capability_permitted, which preserves capability_info visibility and
makes explicit denies non-strict; update the deny paths in
CapabilitySurfaceDenyFilter (including tool_definitions and the other listed
surfaces) to use a raw self.permits(...) check or a strict deny-scope helper so
denied capabilities are fully hidden and fail closed. Make sure
capability_info::CAPABILITY_ID is actually excluded when denied, and add a
regression test that verifies capability_info cannot remain advertised or pass
scope checks under the deny filter.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2c476e60-aaa4-4bd4-adf6-9ac3054f2e4a

📥 Commits

Reviewing files that changed from the base of the PR and between 519dbc7 and 48fd811.

📒 Files selected for processing (5)

• crates/ironclaw_llm/src/nearai_chat.rs
• crates/ironclaw_loop_support/src/capability_surface_filter.rs
• crates/ironclaw_loop_support/src/lib.rs
• crates/ironclaw_reborn/src/runtime.rs
• crates/ironclaw_reborn_composition/src/runtime.rs

💤 Files with no reviewable changes (2)

• crates/ironclaw_reborn/src/runtime.rs
• crates/ironclaw_reborn_composition/src/runtime.rs

[coderabbitai]

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@crates/ironclaw_loop_support/src/capability_surface_filter.rs`:
- Around line 202-208: CapabilitySurfaceDenyFilter is using
provider_capability_permitted, which preserves capability_info visibility and
makes explicit denies non-strict; update the deny paths in
CapabilitySurfaceDenyFilter (including tool_definitions and the other listed
surfaces) to use a raw self.permits(...) check or a strict deny-scope helper so
denied capabilities are fully hidden and fail closed. Make sure
capability_info::CAPABILITY_ID is actually excluded when denied, and add a
regression test that verifies capability_info cannot remain advertised or pass
scope checks under the deny filter.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2c476e60-aaa4-4bd4-adf6-9ac3054f2e4a

📥 Commits

Reviewing files that changed from the base of the PR and between 519dbc7 and 48fd811.

📒 Files selected for processing (5)

• crates/ironclaw_llm/src/nearai_chat.rs
• crates/ironclaw_loop_support/src/capability_surface_filter.rs
• crates/ironclaw_loop_support/src/lib.rs
• crates/ironclaw_reborn/src/runtime.rs
• crates/ironclaw_reborn_composition/src/runtime.rs

💤 Files with no reviewable changes (2)

• crates/ironclaw_reborn/src/runtime.rs
• crates/ironclaw_reborn_composition/src/runtime.rs

🛑 Comments failed to post (1)

crates/ironclaw_loop_support/src/capability_surface_filter.rs (1)

202-208: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Make the deny filter strict; don’t reuse the capability-info-preserving helper.

CapabilitySurfaceDenyFilter promises explicit denies, but these paths call provider_capability_permitted, which existing profile tests rely on to keep capability_info visible outside the allowlist. If capability_info::CAPABILITY_ID is denied, it can still remain advertised/visible and pass scope checks. Use raw self.permits(...) or a strict deny-scope helper, and add a regression that denies capability_info.

As per path instructions, fail closed for authority/dispatch surfaces. As per coding guidelines, comments that promise guarantees across layers must be enforced by code/tests or softened.

Suggested shape

-        definitions.retain(|definition| {
-            provider_capability_permitted(&definition.capability_id, |capability_id| {
-                self.permits(capability_id)
-            })
-        });
+        definitions.retain(|definition| self.permits(&definition.capability_id));

-        validate_provider_tool_call_capability_scope(
+        validate_provider_tool_call_capability_scope_strict(
             capability_ids.clone(),
             |capability_id| self.permits(capability_id),
             "provider tool call targets a disabled capability",
         )?;

Also applies to: 223-227, 235-257, 267-271, 1628-1675

🤖 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_loop_support/src/capability_surface_filter.rs` around lines
202 - 208, CapabilitySurfaceDenyFilter is using provider_capability_permitted,
which preserves capability_info visibility and makes explicit denies non-strict;
update the deny paths in CapabilitySurfaceDenyFilter (including tool_definitions
and the other listed surfaces) to use a raw self.permits(...) check or a strict
deny-scope helper so denied capabilities are fully hidden and fail closed. Make
sure capability_info::CAPABILITY_ID is actually excluded when denied, and add a
regression test that verifies capability_info cannot remain advertised or pass
scope checks under the deny filter.

Sources: Coding guidelines, Path instructions

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/ironclaw_reborn/src/runtime.rs (1)

131-135: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Trim before matching the off escape hatch.

REBORN_TOOL_DISCLOSURE="off " currently hits the unrecognized branch and enables Bridged, so a padded env-file value fails open instead of disabling disclosure.

Suggested patch

 fn from_raw(raw: Option<&str>) -> Self {
-    match raw {
+    match raw.map(str::trim) {
         Some(value) if value.eq_ignore_ascii_case("off") => Self::Off,
         Some(value) if value.eq_ignore_ascii_case("bridged") => Self::Bridged,

As per coding guidelines, “Fail closed for ... runtime selection.” The PR objective also says REBORN_TOOL_DISCLOSURE=off remains the escape hatch.

🤖 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_reborn/src/runtime.rs` around lines 131 - 135, The
`DisclosureMode::from_raw` parsing in `runtime.rs` should fail closed by
recognizing the `off` escape hatch even when the env value has surrounding
whitespace. Trim the incoming `raw` string before matching in `from_raw` so
`REBORN_TOOL_DISCLOSURE="off "` is treated the same as `off`, and keep the
existing `Off`/`Bridged` matching logic in place for the `DisclosureMode` enum.

Source: Coding guidelines

♻️ Duplicate comments (2)

crates/ironclaw_reborn/src/model_gateway.rs (2)

1859-1908: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Test replay compaction through convert_messages.

These assertions would still pass if the caller stopped invoking collapse_repeated_failure_observations. Add a regression that drives the real convert_messages(...) path and asserts the interior duplicate carries repeated_error_elided. As per path instructions, “Test through the caller.”

🤖 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_reborn/src/model_gateway.rs` around lines 1859 - 1908, Add a
regression that exercises the real convert_messages(...) flow in
model_gateway.rs instead of calling collapse_repeated_failure_observations()
directly, so the replay-compaction behavior is verified through its caller.
Reuse the existing generic_error_observation(), error_tool_result_message(), and
tool_result_observation() helpers to build repeated failures plus a success,
then assert the interior duplicate produced by convert_messages(...) is marked
with detail.failure_kind = repeated_error_elided while the first/last failures
and success remain unchanged.

Source: Path instructions

---

1055-1063: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Propagate resolver failures; don’t collapse them into “unadvertised”.

This boolean gate still maps every provider_tool_call_capability_ids() failure to generic InvalidOutput, so internal host-state failures become indistinguishable from an actually outside-surface call. Make the helper return Result<bool, AgentLoopHostError> and only return Ok(false) for explicit invalid/outside-surface cases. As per path instructions, “Fail loud … Errors propagate with ? into thiserror types with context.”

🤖 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_reborn/src/model_gateway.rs` around lines 1055 - 1063, Update
the tool-call validation flow in model_gateway so resolver errors are not
collapsed into InvalidOutput: change provider_calls_are_advertised_or_resolvable
to return Result<bool, AgentLoopHostError>, have it return Ok(false) only for
explicit outside-surface cases, and propagate provider_tool_call_capability_ids
failures with ? so internal host-state issues surface with context. Then adjust
the caller in the model gateway validation block to handle the Result and only
construct HostManagedModelError::safe(InvalidOutput, ...) when the helper
returns Ok(false), using the existing HostManagedModelError and
AgentLoopHostError paths to preserve error detail.

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.

Inline comments:
In `@crates/ironclaw_reborn/src/model_gateway.rs`:
- Around line 1074-1078: The comment attached to the `reason =
error.safe_summary.as_str()` assignment in `model_gateway.rs` overstates an
unverified cross-layer guarantee. Keep the `safe_summary` field, but rewrite the
comment to describe it as sanitized rejection context rather than claiming it is
layer-distinct or that it identifies the rejecting port. Use the surrounding
`error.safe_summary`/`reason` log context to locate and soften the wording
without changing behavior.

---

Outside diff comments:
In `@crates/ironclaw_reborn/src/runtime.rs`:
- Around line 131-135: The `DisclosureMode::from_raw` parsing in `runtime.rs`
should fail closed by recognizing the `off` escape hatch even when the env value
has surrounding whitespace. Trim the incoming `raw` string before matching in
`from_raw` so `REBORN_TOOL_DISCLOSURE="off "` is treated the same as `off`, and
keep the existing `Off`/`Bridged` matching logic in place for the
`DisclosureMode` enum.

---

Duplicate comments:
In `@crates/ironclaw_reborn/src/model_gateway.rs`:
- Around line 1859-1908: Add a regression that exercises the real
convert_messages(...) flow in model_gateway.rs instead of calling
collapse_repeated_failure_observations() directly, so the replay-compaction
behavior is verified through its caller. Reuse the existing
generic_error_observation(), error_tool_result_message(), and
tool_result_observation() helpers to build repeated failures plus a success,
then assert the interior duplicate produced by convert_messages(...) is marked
with detail.failure_kind = repeated_error_elided while the first/last failures
and success remain unchanged.
- Around line 1055-1063: Update the tool-call validation flow in model_gateway
so resolver errors are not collapsed into InvalidOutput: change
provider_calls_are_advertised_or_resolvable to return Result<bool,
AgentLoopHostError>, have it return Ok(false) only for explicit outside-surface
cases, and propagate provider_tool_call_capability_ids failures with ? so
internal host-state issues surface with context. Then adjust the caller in the
model gateway validation block to handle the Result and only construct
HostManagedModelError::safe(InvalidOutput, ...) when the helper returns
Ok(false), using the existing HostManagedModelError and AgentLoopHostError paths
to preserve error detail.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b8cc017e-74b4-4fe9-9862-1aebdb425874

📥 Commits

Reviewing files that changed from the base of the PR and between 48fd811 and 7086bf6.

📒 Files selected for processing (2)

• crates/ironclaw_reborn/src/model_gateway.rs
• crates/ironclaw_reborn/src/runtime.rs

[coderabbitai]

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Soften the safe_summary guarantee.

This comment asserts a cross-layer guarantee that safe_summary is layer-distinct and names the rejecting port, but that uniqueness is not enforced here. Keep the log field; soften the comment to say it provides sanitized rejection context. As per coding guidelines, comments that promise guarantees across layers must be enforced or softened.

🤖 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_reborn/src/model_gateway.rs` around lines 1074 - 1078, The
comment attached to the `reason = error.safe_summary.as_str()` assignment in
`model_gateway.rs` overstates an unverified cross-layer guarantee. Keep the
`safe_summary` field, but rewrite the comment to describe it as sanitized
rejection context rather than claiming it is layer-distinct or that it
identifies the rejecting port. Use the surrounding `error.safe_summary`/`reason`
log context to locate and soften the wording without changing behavior.

Source: Coding guidelines

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@crates/ironclaw_agent_loop/src/executor/prompt.rs`:
- Around line 236-256: The call-time authorization path in prompt.rs now uses
callable_capability_ids directly, but the visible-capability filters only narrow
descriptors, so a tool can be hidden from the model-visible surface yet still
remain callable. Update CapabilitySurfaceProfileFilter::visible_capabilities and
CapabilitySurfaceDenyFilter::visible_capabilities to intersect or otherwise
constrain callable_capability_ids in the same way they constrain descriptors,
and add a regression test covering a surface with non-empty
callable_capability_ids to verify hidden tools are excluded from authorization.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4d1f2561-3a80-4e79-979a-753ac6b3726a

📥 Commits

Reviewing files that changed from the base of the PR and between 7ae4fef and 5b4e9be.

📒 Files selected for processing (20)

• crates/ironclaw_agent_loop/src/executor/prompt.rs
• crates/ironclaw_agent_loop/src/executor/tests/support.rs
• crates/ironclaw_agent_loop/src/test_support/mod.rs
• crates/ironclaw_hooks/src/middleware/capability_port.rs
• crates/ironclaw_hooks/src/middleware/tests/capability_port.rs
• crates/ironclaw_loop_support/src/capability_port.rs
• crates/ironclaw_loop_support/src/capability_surface_filter.rs
• crates/ironclaw_loop_support/src/lib.rs
• crates/ironclaw_loop_support/src/subagent_spawn_port/tests.rs
• crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs
• crates/ironclaw_product_workflow/tests/support/planned_agent_loop.rs
• crates/ironclaw_reborn/src/runtime.rs
• crates/ironclaw_reborn/src/subagent/flavors.rs
• crates/ironclaw_reborn/src/tool_disclosure.rs
• crates/ironclaw_reborn/src/tool_disclosure_port.rs
• crates/ironclaw_reborn/tests/hooks_integration.rs
• crates/ironclaw_reborn/tests/llm_gateway.rs
• crates/ironclaw_reborn_composition/src/extension_lifecycle.rs
• crates/ironclaw_turns/src/run_profile/host.rs
• crates/ironclaw_turns/tests/agent_loop_host_contract.rs

[coderabbitai]

🔒 Security & Privacy | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== prompt pipeline branch =="
rg -n -C4 'callable_capability_ids|visible_capability_ids|apply_capability_filter' \
  crates/ironclaw_agent_loop/src/executor/prompt.rs

echo
echo "== filter implementations touching the visible surface =="
rg -n -C4 'callable_capability_ids|descriptors|apply_capability_filter|CapabilitySurfaceProfileFilter|CapabilitySurfaceDenyFilter' \
  crates/ironclaw_loop_support/src

echo
echo "== tests that exercise non-empty callable_capability_ids =="
rg -n -C2 'callable_capability_ids:\s*vec!\[|callable_capability_ids:\s*Vec::new\(\)' \
  crates tests

Repository: nearai/ironclaw

Length of output: 50372

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== prompt.rs surface -> capability view path =="
sed -n '220,270p' crates/ironclaw_agent_loop/src/executor/prompt.rs

echo
echo "== capability_surface_filter.rs visible_capabilities implementations =="
sed -n '250,420p' crates/ironclaw_loop_support/src/capability_surface_filter.rs

echo
echo "== any non-empty callable_capability_ids constructors =="
rg -n -C3 'callable_capability_ids:\s*(?!Vec::new\(\)|vec!\[\])' \
  crates/ironclaw_loop_support/src crates/ironclaw_agent_loop/src \
  --pcre2

Repository: nearai/ironclaw

Length of output: 21215

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== prompt.rs surface -> capability view path =="
sed -n '220,270p' crates/ironclaw_agent_loop/src/executor/prompt.rs

echo
echo "== capability_surface_filter.rs visible_capabilities implementations =="
sed -n '250,420p' crates/ironclaw_loop_support/src/capability_surface_filter.rs

echo
echo "== any non-empty callable_capability_ids constructors =="
rg -n -C3 'callable_capability_ids:\s*(?!Vec::new\(\)|vec!\[\])' \
  crates/ironclaw_loop_support/src crates/ironclaw_agent_loop/src \
  --pcre2

Repository: nearai/ironclaw

Length of output: 21215

---

Intersect callable_capability_ids when filtering visible surfaces.
CapabilitySurfaceProfileFilter::visible_capabilities and CapabilitySurfaceDenyFilter::visible_capabilities only trim descriptors; crates/ironclaw_agent_loop/src/executor/prompt.rs now authorizes against callable_capability_ids. That leaves a path for an outer filter to drop a tool from the model-visible surface but still let it through call-time auth. Add a regression test with a non-empty callable_capability_ids surface.

🤖 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_agent_loop/src/executor/prompt.rs` around lines 236 - 256,
The call-time authorization path in prompt.rs now uses callable_capability_ids
directly, but the visible-capability filters only narrow descriptors, so a tool
can be hidden from the model-visible surface yet still remain callable. Update
CapabilitySurfaceProfileFilter::visible_capabilities and
CapabilitySurfaceDenyFilter::visible_capabilities to intersect or otherwise
constrain callable_capability_ids in the same way they constrain descriptors,
and add a regression test covering a surface with non-empty
callable_capability_ids to verify hidden tools are excluded from authorization.

Source: Path instructions

[serrrfirat]

/benchmark pinchbench --framework ironclaw-reborn

[github-actions]

🧪 Started pinchbench on ironclaw-reborn against ironclaw 5b4e9be044 — watch run.

[github-actions]

🧪 nearai-bench pinchbench — run complete (no baseline)

Ironclaw 5b4e9be044 on ironclaw-reborn: 0.7% pass, avg score 0.042 across 147 tasks. No baseline exists under baselines/pinchbench/ to compare against — add one (e.g. via the nightly refresh job) to enable regression detection.

🔍 browse run + per-task trajectories · download results