feat(llm): keep deferred tool promotion cache-stable - #7353
Closed
serrrfirat wants to merge 2 commits into
Closed
serrrfirat wants to merge 2 commits into
serrrfirat wants to merge 2 commits into
Conversation
|
🚅 Deployed to the ironclaw-pr-7353 environment in ironclaw-ci-preview
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7353
August 7, 2026 14:59
Destroyed
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds deferred provider tool surfaces across loop contracts, capability wrappers, and the model gateway. Anthropic API-key and OAuth providers now share native deferred-tool, tool-reference, cache-control, and streaming support. Provider wrappers forward the capability. ChangesDeferred tool loading
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size: XL
Contributor
🧭 IronLoop Run · ReviewThis comment updates in place as the Run moves through its stages. 🟩 Final result · Completed
Automatic trigger · attempt 1 of 3 · completed in 16m 3s IronLoop completed the review and posted it to GitHub. 🔗 Result |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/domains/ironclaw_llm/CONTRACT.md`:
- Line 343: Move the “Anthropic deferred loading is model-gated” bullet out of
the rig_adapter.rs Details section and into the Anthropic provider section owned
by anthropic_oauth.rs. Preserve the bullet’s wording and placement it alongside
the other Anthropic-specific behavior documentation, leaving the RigAdapter
section focused only on RigAdapter-owned facts.
In `@crates/domains/ironclaw_llm/src/anthropic_oauth.rs`:
- Around line 1740-1745: Update the test setup to construct the provider through
the public AnthropicProvider::new_api_key path instead of private new_with_auth,
supplying the required API key and configuration. Ensure the test exercises
production client creation and models_endpoint wiring rather than passing
api_key_clients: None.
- Around line 1377-1391: Update the tool-result conversion around
AnthropicToolResultContentBlock and convert_messages so reference blocks are
appended alongside the original msg.content rather than replacing it. Add the
enum’s serialized Text variant, preserve text for direct deferred tools
including tool_describe and capability_info, and add a regression test
confirming convert_messages retains the direct deferred-tool result text.
In `@crates/domains/ironclaw_llm/src/smart_routing.rs`:
- Around line 919-921: Update supports_deferred_tool_loading to return only
primary.supports_deferred_tool_loading(), matching the provider used by
complete_with_tools and complete_with_tools_streaming. Add a caller-level
regression test with a deferred-capable cheap provider and non-deferred primary
provider, asserting the gateway sends the non-deferred fallback to primary.
In `@crates/loop/ironclaw_loop_host/src/external_tool_capability.rs`:
- Around line 264-289: Update deferred_tool_surface and visible_capabilities to
reject external capabilities whose ProviderToolName matches any eager or
deferred host-tool definition, returning the existing authorization/error path
rather than appending or shadowing the host tool. Preserve capability_id
deduplication separately, and add a caller-level llm_gateway regression test
covering a provider-name collision and ensuring the host target remains
protected.
In `@crates/loop/ironclaw_loop_host/src/model_gateway.rs`:
- Around line 1725-1779: Update deferred_tool_references to resolve deferred
target names from assistant tool_call bridge entries, including successful
tool_call results whose provider name remains TOOL_CALL_NAME, while preserving
existing filtering. In
crates/loop/ironclaw_loop_host/src/model_gateway.rs:2964-2999, ensure the
caller-level native deferred-loading path uses these references. Add regression
coverage in crates/loop/ironclaw_loop_host/tests/llm_gateway.rs:284-376 that
exercises a tool_call bridge through the caller and verifies deferred arguments
are decoded after promotion.
🪄 Autofix
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: a25e80f2-a1e4-4802-9bb8-21c19a070563
📒 Files selected for processing (32)
crates/app/ironclaw_composition/src/runtime/capability_host/refreshing_capability_port.rscrates/contracts/ironclaw_loop_contracts/src/host/capability.rscrates/contracts/ironclaw_loop_contracts/src/host/mod.rscrates/contracts/ironclaw_loop_contracts/src/lib.rscrates/domains/ironclaw_llm/CONTRACT.mdcrates/domains/ironclaw_llm/src/anthropic_oauth.rscrates/domains/ironclaw_llm/src/circuit_breaker.rscrates/domains/ironclaw_llm/src/config.rscrates/domains/ironclaw_llm/src/error.rscrates/domains/ironclaw_llm/src/failover.rscrates/domains/ironclaw_llm/src/lib.rscrates/domains/ironclaw_llm/src/provider.rscrates/domains/ironclaw_llm/src/recording.rscrates/domains/ironclaw_llm/src/response_cache.rscrates/domains/ironclaw_llm/src/retry.rscrates/domains/ironclaw_llm/src/rig_adapter.rscrates/domains/ironclaw_llm/src/runtime.rscrates/domains/ironclaw_llm/src/smart_routing.rscrates/domains/ironclaw_llm/src/token_refreshing.rscrates/loop/ironclaw_hooks/src/middleware/capability_port.rscrates/loop/ironclaw_loop_host/README.mdcrates/loop/ironclaw_loop_host/src/capability_surface_filter.rscrates/loop/ironclaw_loop_host/src/external_tool_capability.rscrates/loop/ironclaw_loop_host/src/model_gateway.rscrates/loop/ironclaw_loop_host/src/subagent_spawn_port.rscrates/loop/ironclaw_loop_host/src/surface_disclosure.rscrates/loop/ironclaw_loop_host/src/synthetic_capability.rscrates/loop/ironclaw_loop_host/src/tool_disclosure.rscrates/loop/ironclaw_loop_host/src/tool_disclosure_port.rscrates/loop/ironclaw_loop_host/tests/llm_gateway.rscrates/loop/ironclaw_turn_runner/src/hook_gate_refs.rscrates/loop/ironclaw_turn_runner/src/loop_driver_host.rs
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🔍 IronLoop review
Found two correctness issues in the Anthropic compatibility paths.
Findings: 🟠 Medium 1 · 🟡 Low 1
🟠 Medium · Direct Anthropic client no longer disables caching for legacy models
Inline on crates/domains/ironclaw_llm/src/anthropic_oauth.rs:213. See the inline comment for details.
🟡 Low · Supported Claude Opus 5 is excluded from deferred tool loading
Inline on crates/domains/ironclaw_llm/src/anthropic_oauth.rs:99. See the inline comment for details.
Validation
- ✅ cargo fmt --all -- --check — Formatting check completed successfully.
- ✅ cargo test -p ironclaw_llm — Passed 922 unit tests, 2 module-charter tests, and doc tests.
- ✅ cargo test -p ironclaw_loop_host --test llm_gateway gateway_keeps_native_deferred_tool_surface_stable_across_promotion — Passed: 1 test succeeded; 85 were filtered out.
- ✅ git diff --check refs/ironloop/merge-base refs/ironloop/head — No whitespace errors found.
- ⚪ cargo clippy --all --benches --tests --examples --all-features -- -D warnings — Not run. Not run; validation focused on the changed LLM package and gateway integration path.
Review details
- Run:
fc96043d-689d-4e38-bb0f-6be2bdc276dc - Workflow: Review
- Attempts: 1
10 of 21 tasks
This was referenced Aug 9, 2026
Closed
21 of 29 tasks
Collaborator
Author
|
@ironloopai resolve |
Contributor
🧭 IronLoop Run · ResolveThis comment updates in place as the Run moves through its stages. 🟩 Final result · Completed
Manual command by serrrfirat · attempt 1 of 3 · completed in 46m 45s IronLoop updated the pull request with a verified resolution and resolved 8 captured review threads. 🔗 ResultRelated artifact: |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7353
August 11, 2026 17:15
Destroyed
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/domains/ironclaw_llm/src/anthropic_oauth.rs`:
- Around line 1731-1746: Extract the duplicated TcpListener
accept/read-headers/content-length/reply logic from
api_key_request_sends_stable_deferred_tools_and_tool_references and
api_key_legacy_model_disables_prompt_cache_control into a shared test helper
that returns the base URL and captured-request JoinHandle. Parameterize the
helper with each test’s canned response body, then update both tests to use it
while preserving their existing assertions.
- Around line 149-227: Move the #[cfg(test)] module from anthropic_oauth.rs into
a sibling tests.rs module, following the existing
rig_adapter/tests/finish_reason_tests.rs pattern, and wire it into the
production module without changing behavior. This should reduce
anthropic_oauth.rs below the applicable size threshold; if production additions
still exceed 200 lines, add the required inline justification.
In `@crates/domains/ironclaw_llm/src/lib.rs`:
- Line 469: Update AnthropicProvider::new_oauth and its call sites, including
create_anthropic_from_registry, to accept and forward the configured
request_timeout_secs. Use that value when constructing the buffered client via
new_with_auth instead of DEFAULT_REQUEST_TIMEOUT_SECS, while keeping the API-key
branch’s timeout behavior unchanged.
In `@crates/loop/ironclaw_loop_host/src/model_gateway.rs`:
- Around line 1729-1792: Update the bridge-reference parsing around the
tool-call target collection and match on bridge names using the imported
TOOL_CALL_NAME, TOOL_SEARCH_NAME, and TOOL_DESCRIBE_NAME constants from
tool_disclosure_port.rs instead of string literals. Add the parse_bridge_result
helper beside this function, replacing silent serde_json::from_str(...).ok()
handling so parse failures emit a debug warning with the bridge and error, then
continue without producing a reference.
- Around line 1444-1448: Update the repair-retry flow around
ToolCompletionRequest::from_completion_request so tool_references is recomputed
from the repaired/replayed messages immediately before the retry, rather than
retaining references from the original request. Preserve deferred_tool_names and
other request fields, and add a caller-level regression test under the “Test
through the caller” invariant covering replayed deferred tool results with a
native deferred provider.
In `@crates/loop/ironclaw_loop_host/src/tool_disclosure_port.rs`:
- Around line 355-388: Add a concise comment above the
select_active_set_for_mode call in deferred_tool_surface explaining that
PromotedSet::default() intentionally keeps the eager tool array stable and
independent of promotions, preserving the Anthropic prompt-prefix cache
invariant; warn against replacing it with the active promoted set.
In `@crates/loop/ironclaw_loop_host/tests/llm_gateway.rs`:
- Around line 382-428: Rename the test function
gateway_uses_primary_deferred_capability_when_smart_router_cheap_supports_it to
describe the asserted invariant: tool-capable calls use the non-deferred primary
with the dynamic full tool surface, while the deferred cheap provider is not
dispatched. Keep the test implementation and assertions unchanged.
🪄 Autofix
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: d843e255-2cc0-4912-afb8-45031cdbe211
📒 Files selected for processing (33)
crates/app/ironclaw_architecture_tests/tests/reborn_dependency_boundaries.rscrates/app/ironclaw_composition/src/runtime/capability_host/refreshing_capability_port.rscrates/contracts/ironclaw_loop_contracts/src/host/capability.rscrates/contracts/ironclaw_loop_contracts/src/host/mod.rscrates/contracts/ironclaw_loop_contracts/src/lib.rscrates/domains/ironclaw_llm/CONTRACT.mdcrates/domains/ironclaw_llm/src/anthropic_oauth.rscrates/domains/ironclaw_llm/src/circuit_breaker.rscrates/domains/ironclaw_llm/src/config.rscrates/domains/ironclaw_llm/src/error.rscrates/domains/ironclaw_llm/src/failover.rscrates/domains/ironclaw_llm/src/lib.rscrates/domains/ironclaw_llm/src/provider.rscrates/domains/ironclaw_llm/src/recording.rscrates/domains/ironclaw_llm/src/response_cache.rscrates/domains/ironclaw_llm/src/retry.rscrates/domains/ironclaw_llm/src/rig_adapter.rscrates/domains/ironclaw_llm/src/runtime.rscrates/domains/ironclaw_llm/src/smart_routing.rscrates/domains/ironclaw_llm/src/token_refreshing.rscrates/loop/ironclaw_hooks/src/middleware/capability_port.rscrates/loop/ironclaw_loop_host/README.mdcrates/loop/ironclaw_loop_host/src/capability_surface_filter.rscrates/loop/ironclaw_loop_host/src/external_tool_capability.rscrates/loop/ironclaw_loop_host/src/model_gateway.rscrates/loop/ironclaw_loop_host/src/subagent_spawn_port.rscrates/loop/ironclaw_loop_host/src/surface_disclosure.rscrates/loop/ironclaw_loop_host/src/synthetic_capability.rscrates/loop/ironclaw_loop_host/src/tool_disclosure.rscrates/loop/ironclaw_loop_host/src/tool_disclosure_port.rscrates/loop/ironclaw_loop_host/tests/llm_gateway.rscrates/loop/ironclaw_turn_runner/src/hook_gate_refs.rscrates/loop/ironclaw_turn_runner/src/loop_driver_host.rs
Comment on lines
+149
to
+227
| enum AnthropicAuth { | ||
| ApiKey(SecretString), | ||
| OAuth(std::sync::RwLock<SecretString>), | ||
| } | ||
|
|
||
| /// Direct Anthropic Messages API provider. | ||
| pub(crate) struct AnthropicProvider { | ||
| client: Client, | ||
| streaming_client: Client, | ||
| stream_idle_timeout: Duration, | ||
| /// OAuth token, wrapped in RwLock so it can be updated after a successful | ||
| /// Keychain refresh (fixes #1136: stale token reuse after expiry). | ||
| token: std::sync::RwLock<SecretString>, | ||
| auth: AnthropicAuth, | ||
| provider_id: String, | ||
| model: String, | ||
| base_url: Option<String>, | ||
| active_model: std::sync::RwLock<String>, | ||
| cache_retention: CacheRetention, | ||
| models_endpoint: Option<crate::rig_adapter::ModelsEndpoint>, | ||
| /// Parameter names that this provider does not support. | ||
| unsupported_params: HashSet<String>, | ||
| } | ||
|
|
||
| impl AnthropicOAuthProvider { | ||
| pub(crate) fn new(config: &RegistryProviderConfig) -> Result<Self, LlmError> { | ||
| impl AnthropicProvider { | ||
| pub(crate) fn new_oauth(config: &RegistryProviderConfig) -> Result<Self, LlmError> { | ||
| let token = config | ||
| .oauth_token | ||
| .clone() | ||
| .ok_or_else(|| LlmError::AuthFailed { | ||
| provider: "anthropic_oauth".to_string(), | ||
| })?; | ||
|
|
||
| let client = | ||
| crate::config::hardened_client_builder(crate::config::DEFAULT_REQUEST_TIMEOUT_SECS) | ||
| .build() | ||
| .map_err(|e| LlmError::RequestFailed { | ||
| provider: "anthropic_oauth".to_string(), | ||
| reason: format!("Failed to build HTTP client: {}", e), | ||
| })?; | ||
| let streaming_client = crate::config::hardened_streaming_client_builder() | ||
| .build() | ||
| .map_err(|e| LlmError::RequestFailed { | ||
| provider: "anthropic_oauth".to_string(), | ||
| reason: format!("Failed to build streaming HTTP client: {e}"), | ||
| })?; | ||
| Self::new_with_auth( | ||
| config, | ||
| AnthropicAuth::OAuth(std::sync::RwLock::new(token)), | ||
| None, | ||
| ) | ||
| } | ||
|
|
||
| pub(crate) fn new_api_key( | ||
| config: &RegistryProviderConfig, | ||
| request_timeout_secs: u64, | ||
| models_endpoint: crate::rig_adapter::ModelsEndpoint, | ||
| ) -> Result<Self, LlmError> { | ||
| let api_key = config.api_key.clone().ok_or_else(|| LlmError::AuthFailed { | ||
| provider: config.provider_id.clone(), | ||
| })?; | ||
| let client = crate::provider_http_client( | ||
| &config.provider_id, | ||
| &config.base_url, | ||
| request_timeout_secs, | ||
| )?; | ||
| Self::new_with_auth( | ||
| config, | ||
| AnthropicAuth::ApiKey(api_key), | ||
| Some((client, models_endpoint)), | ||
| ) | ||
| } | ||
|
|
||
| fn new_with_auth( | ||
| config: &RegistryProviderConfig, | ||
| auth: AnthropicAuth, | ||
| api_key_clients: Option<(Client, crate::rig_adapter::ModelsEndpoint)>, | ||
| ) -> Result<Self, LlmError> { | ||
| let provider_id = match &auth { | ||
| AnthropicAuth::ApiKey(_) => config.provider_id.clone(), | ||
| AnthropicAuth::OAuth(_) => "anthropic_oauth".to_string(), | ||
| }; | ||
| let client = match api_key_clients.as_ref() { | ||
| Some((client, _)) => client.clone(), | ||
| None => crate::url_check::build_http_client( | ||
| &config.provider_id, | ||
| &config.base_url, | ||
| crate::config::hardened_client_builder(crate::config::DEFAULT_REQUEST_TIMEOUT_SECS), | ||
| )?, | ||
| }; | ||
| let streaming_client = crate::url_check::build_http_client( | ||
| &config.provider_id, | ||
| &config.base_url, | ||
| crate::config::hardened_streaming_client_builder(), | ||
| )?; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
This file is over 1,500 lines and grew well past the 200-line inline-justification threshold.
anthropic_oauth.rs now carries the unified auth enum, both constructors, the full Messages encoder, deferred-tool and tool-reference wire formats, cache-control, streaming ingestion, and ~800 lines of tests. The repo rule: files over 1,500 lines should become shorter when touched unless the change explicitly expands a feature with no suitable alternative, and additions exceeding 200 lines require inline justification.
A suitable alternative exists and has precedent in this crate: move the #[cfg(test)] module into a sibling anthropic_oauth/tests.rs, the same split used for rig_adapter/tests/finish_reason_tests.rs. That removes ~815 lines without touching behavior. If the production growth stays, add the inline justification the rule requires.
As per coding guidelines: "Existing files over 1,500 lines should become shorter when touched unless explicitly expanding a feature with no suitable alternative; files over 3,000 lines require a decomposition tracking issue, and additions exceeding 200 lines require inline justification."
🤖 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/domains/ironclaw_llm/src/anthropic_oauth.rs` around lines 149 - 227,
Move the #[cfg(test)] module from anthropic_oauth.rs into a sibling tests.rs
module, following the existing rig_adapter/tests/finish_reason_tests.rs pattern,
and wire it into the production module without changing behavior. This should
reduce anthropic_oauth.rs below the applicable size threshold; if production
additions still exceed 200 lines, add the required inline justification.
Source: Coding guidelines
Comment on lines
+1731
to
+1746
| #[tokio::test] | ||
| async fn api_key_request_sends_stable_deferred_tools_and_tool_references() { | ||
| use tokio::io::{AsyncReadExt, AsyncWriteExt}; | ||
| use tokio::net::TcpListener; | ||
|
|
||
| let listener = TcpListener::bind("127.0.0.1:0") | ||
| .await | ||
| .expect("loopback listener"); | ||
| let base_url = format!( | ||
| "http://{}", | ||
| listener.local_addr().expect("loopback address") | ||
| ); | ||
| let server = tokio::spawn(async move { | ||
| let (mut socket, _) = listener.accept().await.expect("accept request"); | ||
| let mut request = Vec::new(); | ||
| loop { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Extract the duplicated loopback capture server.
api_key_request_sends_stable_deferred_tools_and_tool_references and api_key_legacy_model_disables_prompt_cache_control each inline the same ~35-line accept / read-headers / parse content-length / reply-200 block. The two copies are identical. Extract one helper that returns the base URL and the captured request JoinHandle, then let each test supply its own canned response body.
Also applies to: 1861-1878
🤖 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/domains/ironclaw_llm/src/anthropic_oauth.rs` around lines 1731 - 1746,
Extract the duplicated TcpListener accept/read-headers/content-length/reply
logic from api_key_request_sends_stable_deferred_tools_and_tool_references and
api_key_legacy_model_disables_prompt_cache_control into a shared test helper
that returns the base URL and captured-request JoinHandle. Parameterize the
helper with each test’s canned response body, then update both tests to use it
while preserving their existing assertions.
| "Using Anthropic OAuth API" | ||
| ); | ||
| let provider = anthropic_oauth::AnthropicOAuthProvider::new(config)?; | ||
| let provider = anthropic_oauth::AnthropicProvider::new_oauth(config)?; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The OAuth branch drops request_timeout_secs; the API-key branch honours it.
new_api_key receives request_timeout_secs and builds its client with it. new_oauth takes only config, so new_with_auth falls back to DEFAULT_REQUEST_TIMEOUT_SECS for the buffered client. Both branches now share one provider type, so an operator who sets a long LLM_REQUEST_TIMEOUT_SECS gets it for API-key Anthropic and the 60 s default for OAuth Anthropic.
create_anthropic_from_registry already has the value in scope. Thread it through new_oauth.
🔧 Proposed fix: pass the configured timeout on both branches
- let provider = anthropic_oauth::AnthropicProvider::new_oauth(config)?;
+ let provider = anthropic_oauth::AnthropicProvider::new_oauth(config, request_timeout_secs)?;
return Ok(Arc::new(provider));In anthropic_oauth.rs, take the timeout on new_oauth and use it for the buffered client instead of DEFAULT_REQUEST_TIMEOUT_SECS.
Also applies to: 520-524
🤖 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/domains/ironclaw_llm/src/lib.rs` at line 469, Update
AnthropicProvider::new_oauth and its call sites, including
create_anthropic_from_registry, to accept and forward the configured
request_timeout_secs. Use that value when constructing the buffered client via
new_with_auth instead of DEFAULT_REQUEST_TIMEOUT_SECS, while keeping the API-key
branch’s timeout behavior unchanged.
Comment on lines
+1444
to
+1448
| let mut tool_request = | ||
| ToolCompletionRequest::from_completion_request(completion, llm_tool_definitions); | ||
| tool_request.tool_references = | ||
| deferred_tool_references(&tool_request.messages, &deferred_tool_names); | ||
| tool_request.deferred_tool_names = deferred_tool_names; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep run --lang rust \
--pattern 'fn provider_tool_repair_messages($$$) { $$$ }' \
crates/loop/ironclaw_loop_host/src/model_gateway.rs
rg -nP -C6 'repair_request|tool_references' crates/loop/ironclaw_loop_host/src/model_gateway.rsRepository: nearai/ironclaw
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="crates/loop/ironclaw_loop_host/src/model_gateway.rs"
ast-grep outline "$file"
printf '\n--- relevant symbols and call sites ---\n'
rg -n -C12 'provider_tool_repair_messages|repair_request|tool_references|deferred_tool_references|deferred_tool_names' "$file"
printf '\n--- surrounding repair flow ---\n'
sed -n '1460,1575p' "$file"Repository: nearai/ironclaw
Length of output: 29212
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="crates/loop/ironclaw_loop_host/src/model_gateway.rs"
printf '%s\n' '--- deferred reference computation ---'
sed -n '1725,1798p' "$file"
printf '%s\n' '--- repair message construction ---'
sed -n '2430,2496p' "$file"
printf '%s\n' '--- provider/tool request definitions and deferred handling ---'
rg -n -C8 'struct ToolCompletionRequest|tool_references|deferred_tool_names|supports_deferred_tool_loading|complete_with_tools' crates --glob '*.rs'Repository: nearai/ironclaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- gateway helpers ---'
sed -n '1725,1798p' crates/loop/ironclaw_loop_host/src/model_gateway.rs
sed -n '2430,2496p' crates/loop/ironclaw_loop_host/src/model_gateway.rs
printf '%s\n' '--- request type owner ---'
rg -l 'struct ToolCompletionRequest' crates --glob '*.rs'
printf '%s\n' '--- deferred provider support declarations ---'
rg -n -C5 'supports_deferred_tool_loading|tool_references|deferred_tool_names' crates/domains crates/loop --glob '*.rs' --glob '!**/tests/**'Repository: nearai/ironclaw
Length of output: 42413
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Anthropic conversion of tool results and references ---'
sed -n '1353,1465p' crates/domains/ironclaw_llm/src/anthropic_oauth.rs
printf '%s\n' '--- native deferred request test assertions ---'
sed -n '1725,1865p' crates/domains/ironclaw_llm/src/anthropic_oauth.rs
printf '%s\n' '--- request contract ---'
sed -n '560,590p' crates/domains/ironclaw_llm/src/provider.rsRepository: nearai/ironclaw
Length of output: 12675
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
gateway = Path("crates/loop/ironclaw_loop_host/src/model_gateway.rs").read_text()
anthropic = Path("crates/domains/ironclaw_llm/src/anthropic_oauth.rs").read_text()
repair_start = gateway.index("let mut repair_request = tool_request;")
repair_end = gateway.index("provider.complete_with_tools(repair_request)", repair_start)
repair = gateway[repair_start:repair_end]
assert "repair_request.messages.extend(provider_tool_repair_messages" in repair
assert "repair_request.tool_references" not in repair
assert "repair_request.deferred_tool_names" not in repair
helper_start = gateway.index("fn provider_tool_repair_messages(")
helper_end = gateway.index("fn provider_tool_repair_result_content(", helper_start)
helper = gateway[helper_start:helper_end]
assert "ChatMessage::tool_result(" in helper
assert "tool_call.name.clone()" in helper
conversion_start = anthropic.index("fn convert_messages(")
conversion_end = anthropic.index("fn ", conversion_start + 4)
conversion = anthropic[conversion_start:conversion_end]
assert "tool_references.get(&tool_call_id)" in conversion
assert "AnthropicToolResultContent::Text(msg.content)" in conversion
print("repair request appends replay messages without recomputing tool_references")
print("repair helper replays the response tool-call IDs and names")
print("Anthropic conversion falls back to plain tool-result text when the ID is absent")
PY
printf '%s\n' '--- caller-level repair tests ---'
rg -n -C4 'repair|repairable|complete_model_request|tool_references' \
crates/loop/ironclaw_loop_host/src/model_gateway.rs \
crates/loop/ironclaw_loop_host/tests crates --glob '*test*.rs' 2>/dev/null \
| rg -m80 'repair|repairable|complete_model_request|tool_references'Repository: nearai/ironclaw
Length of output: 242
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
from pathlib import Path
gateway = Path("crates/loop/ironclaw_loop_host/src/model_gateway.rs").read_text()
anthropic = Path("crates/domains/ironclaw_llm/src/anthropic_oauth.rs").read_text()
repair_start = gateway.index("let mut repair_request = tool_request;")
repair_end = gateway.index("provider.complete_with_tools(repair_request)", repair_start)
repair = gateway[repair_start:repair_end]
normalized_repair = re.sub(r"\s+", " ", repair)
assert "repair_request .messages .extend ( provider_tool_repair_messages" in normalized_repair
assert "repair_request.tool_references" not in repair
assert "repair_request.deferred_tool_names" not in repair
helper_start = gateway.index("fn provider_tool_repair_messages(")
helper_end = gateway.index("fn provider_tool_repair_result_content(", helper_start)
helper = gateway[helper_start:helper_end]
assert "ChatMessage::tool_result(" in helper
assert "tool_call.name.clone()" in helper
conversion_start = anthropic.index("fn convert_messages(")
conversion_end = anthropic.index("\nfn ", conversion_start + 4)
conversion = anthropic[conversion_start:conversion_end]
assert "tool_references.get(&tool_call_id)" in conversion
assert "AnthropicToolResultContent::Text(msg.content)" in conversion
print("PASS: repair request appends replay messages without recomputing tool_references")
print("PASS: repair helper replays response tool-call IDs and names")
print("PASS: Anthropic conversion uses plain tool-result text when the ID is absent")
PY
printf '%s\n' '--- repair-related tests and caller seams ---'
rg -n -C4 'provider_tool_repair|repairable_provider|complete_model_request|tool_references' \
crates/loop/ironclaw_loop_host/src/model_gateway.rs \
crates/loop/ironclaw_loop_host --glob '*.rs' \
| head -n 240Repository: nearai/ironclaw
Length of output: 242
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
from pathlib import Path
text = Path("crates/loop/ironclaw_loop_host/src/model_gateway.rs").read_text()
start = text.index("let mut repair_request = tool_request;")
end = text.index("provider.complete_with_tools(repair_request)", start)
repair = text[start:end]
print("--- repair block ---")
print(repair)
print("--- normalized ---")
print(re.sub(r"\s+", " ", repair))
print("--- structural checks ---")
print("has repair_request assignment:", bool(re.search(r"let\s+mut\s+repair_request\s*=\s*tool_request\s*;", repair)))
print("extends repair messages:", bool(re.search(r"repair_request\s*\.messages\s*\.extend\s*\(\s*provider_tool_repair_messages", repair, re.S)))
print("recomputes references:", bool(re.search(r"repair_request\s*\.\s*tool_references\s*=", repair)))
print("recomputes deferred names:", bool(re.search(r"repair_request\s*\.\s*deferred_tool_names\s*=", repair)))
PYRepository: nearai/ironclaw
Length of output: 1061
Recompute tool_references before the repair retry. The repair messages replay deferred tool calls, but the request retains references from the original messages. A native deferred provider can reject the replayed tool result. Add a caller-level regression test under the “Test through the caller” invariant.
🤖 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/loop/ironclaw_loop_host/src/model_gateway.rs` around lines 1444 -
1448, Update the repair-retry flow around
ToolCompletionRequest::from_completion_request so tool_references is recomputed
from the repaired/replayed messages immediately before the retry, rather than
retaining references from the original request. Preserve deferred_tool_names and
other request fields, and add a caller-level regression test under the “Test
through the caller” invariant covering replayed deferred tool results with a
native deferred provider.
Comment on lines
+1729
to
+1792
| let tool_call_targets = messages | ||
| .iter() | ||
| .filter(|message| message.role == Role::Assistant) | ||
| .filter_map(|message| message.tool_calls.as_ref()) | ||
| .flatten() | ||
| .filter(|tool_call| tool_call.name == "tool_call") | ||
| .filter_map(|tool_call| { | ||
| tool_call | ||
| .arguments | ||
| .get("name") | ||
| .and_then(serde_json::Value::as_str) | ||
| .map(|name| (tool_call.id.clone(), name.to_string())) | ||
| }) | ||
| .collect::<HashMap<_, _>>(); | ||
| let mut references = std::collections::BTreeMap::new(); | ||
| for message in messages { | ||
| if message.role != Role::Tool { | ||
| continue; | ||
| } | ||
| let Some(tool_call_id) = message.tool_call_id.as_ref() else { | ||
| continue; | ||
| }; | ||
| let names = match message.name.as_deref() { | ||
| Some("tool_search") => serde_json::from_str::<serde_json::Value>(&message.content) | ||
| .ok() | ||
| .and_then(|value| { | ||
| value | ||
| .get("results") | ||
| .and_then(serde_json::Value::as_array) | ||
| .cloned() | ||
| }) | ||
| .into_iter() | ||
| .flatten() | ||
| .filter_map(|result| { | ||
| result | ||
| .get("name") | ||
| .and_then(serde_json::Value::as_str) | ||
| .map(str::to_string) | ||
| }) | ||
| .collect::<Vec<_>>(), | ||
| Some("tool_describe" | "capability_info") => { | ||
| serde_json::from_str::<serde_json::Value>(&message.content) | ||
| .ok() | ||
| .and_then(|value| { | ||
| value | ||
| .get("name") | ||
| .and_then(serde_json::Value::as_str) | ||
| .map(str::to_string) | ||
| }) | ||
| .into_iter() | ||
| .collect() | ||
| } | ||
| Some("tool_call") => tool_call_targets | ||
| .get(tool_call_id) | ||
| .cloned() | ||
| .into_iter() | ||
| .collect(), | ||
| Some(name) if deferred_tool_names.contains(name) => vec![name.to_string()], | ||
| _ => Vec::new(), | ||
| }; | ||
| let names = names | ||
| .into_iter() | ||
| .filter(|name| deferred_tool_names.contains(name)) | ||
| .collect::<Vec<_>>(); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use the bridge name constants, and log the discarded parse error.
Two problems in this match block.
-
The bridge names are hard-coded as
"tool_call","tool_search","tool_describe". This crate already ownsTOOL_CALL_NAME,TOOL_SEARCH_NAMEandTOOL_DESCRIBE_NAMEintool_disclosure_port.rs, and that module matches on them at lines 412, 448 and 452. If a constant is renamed, this function silently stops emitting references, deferred tools stay unreferenced, and the promotion path degrades with no compile error. Repo rule: "Use enums instead of matching on string literals". -
serde_json::from_str(...).ok()drops the parse error. The fallback is silent: a malformed bridge result yields no reference and the promoted tool becomes unreachable for the rest of the run. This is the "fail loud" rule's warn-and-continue shape. Adebug!line is enough here, matching the diagnostics style used elsewhere in this file.
♻️ Proposed fix
- let names = match message.name.as_deref() {
- Some("tool_search") => serde_json::from_str::<serde_json::Value>(&message.content)
- .ok()
+ let names = match message.name.as_deref() {
+ Some(TOOL_SEARCH_NAME) => parse_bridge_result(TOOL_SEARCH_NAME, &message.content)
.and_then(|value| {
value
.get("results")
.and_then(serde_json::Value::as_array)
.cloned()
})
@@
- Some("tool_describe" | "capability_info") => {
- serde_json::from_str::<serde_json::Value>(&message.content)
- .ok()
+ Some(TOOL_DESCRIBE_NAME | CAPABILITY_INFO_NAME) => {
+ parse_bridge_result(TOOL_DESCRIBE_NAME, &message.content)
.and_then(|value| {
@@
- Some("tool_call") => tool_call_targets
+ Some(TOOL_CALL_NAME) => tool_call_targetsAdd the helper next to the function:
fn parse_bridge_result(bridge: &str, content: &str) -> Option<serde_json::Value> {
match serde_json::from_str::<serde_json::Value>(content) {
Ok(value) => Some(value),
Err(error) => {
debug!(
bridge,
error = %error,
"reborn model gateway could not parse a bridge result; deferred tool reference is omitted"
);
None
}
}
}The tool_call_targets filter at line 1734 needs the same constant:
- .filter(|tool_call| tool_call.name == "tool_call")
+ .filter(|tool_call| tool_call.name == TOOL_CALL_NAME)TOOL_CALL_NAME and friends must be imported from the disclosure module rather than redeclared here.
🤖 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/loop/ironclaw_loop_host/src/model_gateway.rs` around lines 1729 -
1792, Update the bridge-reference parsing around the tool-call target collection
and match on bridge names using the imported TOOL_CALL_NAME, TOOL_SEARCH_NAME,
and TOOL_DESCRIBE_NAME constants from tool_disclosure_port.rs instead of string
literals. Add the parse_bridge_result helper beside this function, replacing
silent serde_json::from_str(...).ok() handling so parse failures emit a debug
warning with the bridge and error, then continue without producing a reference.
Sources: Coding guidelines, Path instructions
Comment on lines
+355
to
+388
| fn deferred_tool_surface( | ||
| &self, | ||
| ) -> Result<Option<DeferredProviderToolSurface>, AgentLoopHostError> { | ||
| let state = self.turn_state()?; | ||
| let Some(state) = state.as_ref() else { | ||
| return Ok(None); | ||
| }; | ||
| let eager = select_active_set_for_mode( | ||
| &state.catalog, | ||
| &PromotedSet::default(), | ||
| self.caps, | ||
| &self.policy, | ||
| self.mode, | ||
| ); | ||
| if !eager.deferred { | ||
| return Ok(None); | ||
| } | ||
| let eager_names = eager | ||
| .definitions | ||
| .iter() | ||
| .map(|definition| definition.name.as_str().to_string()) | ||
| .collect::<BTreeSet<_>>(); | ||
| let deferred = state | ||
| .catalog | ||
| .effective_definitions(&self.policy) | ||
| .filter(|definition| !eager_names.contains(definition.name.as_str())) | ||
| .cloned() | ||
| .collect(); | ||
| Ok(Some(DeferredProviderToolSurface { | ||
| eager: eager.definitions, | ||
| deferred, | ||
| })) | ||
| } | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Document why the eager set is recomputed with PromotedSet::default().
This call is the load-bearing part of the cache-stability invariant: the eager array must not vary with promotions, otherwise the Anthropic prompt prefix breaks. A future reader can easily "fix" this to pass the real PromotedSet and silently reintroduce the cache break the PR removes. The rest of this file comments comparable non-obvious decisions; this one deserves the same treatment.
♻️ Proposed comment
+ // Promotion-independent by construction: the provider-visible array must
+ // stay byte-stable for the whole run (Anthropic cached prefix), so the
+ // eager selection is recomputed from an empty promoted set. Promotions
+ // surface through `tool_reference` results, never by mutating this array.
let eager = select_active_set_for_mode(
&state.catalog,
&PromotedSet::default(),📝 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
| fn deferred_tool_surface( | |
| &self, | |
| ) -> Result<Option<DeferredProviderToolSurface>, AgentLoopHostError> { | |
| let state = self.turn_state()?; | |
| let Some(state) = state.as_ref() else { | |
| return Ok(None); | |
| }; | |
| let eager = select_active_set_for_mode( | |
| &state.catalog, | |
| &PromotedSet::default(), | |
| self.caps, | |
| &self.policy, | |
| self.mode, | |
| ); | |
| if !eager.deferred { | |
| return Ok(None); | |
| } | |
| let eager_names = eager | |
| .definitions | |
| .iter() | |
| .map(|definition| definition.name.as_str().to_string()) | |
| .collect::<BTreeSet<_>>(); | |
| let deferred = state | |
| .catalog | |
| .effective_definitions(&self.policy) | |
| .filter(|definition| !eager_names.contains(definition.name.as_str())) | |
| .cloned() | |
| .collect(); | |
| Ok(Some(DeferredProviderToolSurface { | |
| eager: eager.definitions, | |
| deferred, | |
| })) | |
| } | |
| fn deferred_tool_surface( | |
| &self, | |
| ) -> Result<Option<DeferredProviderToolSurface>, AgentLoopHostError> { | |
| let state = self.turn_state()?; | |
| let Some(state) = state.as_ref() else { | |
| return Ok(None); | |
| }; | |
| // Promotion-independent by construction: the provider-visible array must | |
| // stay byte-stable for the whole run (Anthropic cached prefix), so the | |
| // eager selection is recomputed from an empty promoted set. Promotions | |
| // surface through `tool_reference` results, never by mutating this array. | |
| let eager = select_active_set_for_mode( | |
| &state.catalog, | |
| &PromotedSet::default(), | |
| self.caps, | |
| &self.policy, | |
| self.mode, | |
| ); | |
| if !eager.deferred { | |
| return Ok(None); | |
| } | |
| let eager_names = eager | |
| .definitions | |
| .iter() | |
| .map(|definition| definition.name.as_str().to_string()) | |
| .collect::<BTreeSet<_>>(); | |
| let deferred = state | |
| .catalog | |
| .effective_definitions(&self.policy) | |
| .filter(|definition| !eager_names.contains(definition.name.as_str())) | |
| .cloned() | |
| .collect(); | |
| Ok(Some(DeferredProviderToolSurface { | |
| eager: eager.definitions, | |
| deferred, | |
| })) | |
| } |
🤖 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/loop/ironclaw_loop_host/src/tool_disclosure_port.rs` around lines 355
- 388, Add a concise comment above the select_active_set_for_mode call in
deferred_tool_surface explaining that PromotedSet::default() intentionally keeps
the eager tool array stable and independent of promotions, preserving the
Anthropic prompt-prefix cache invariant; warn against replacing it with the
active promoted set.
Comment on lines
+382
to
+428
| #[tokio::test] | ||
| async fn gateway_uses_primary_deferred_capability_when_smart_router_cheap_supports_it() { | ||
| let primary = Arc::new(ToolAwareProvider::tool_stop_reply("primary response")); | ||
| let cheap = | ||
| Arc::new(ToolAwareProvider::tool_stop_reply("cheap response").with_deferred_tool_loading()); | ||
| let provider = Arc::new(SmartRoutingProvider::new( | ||
| primary.clone(), | ||
| cheap.clone(), | ||
| SmartRoutingConfig::default(), | ||
| )); | ||
| let gateway = LlmProviderModelGateway::with_provider_identity( | ||
| STATIC_PROVIDER_ID, | ||
| provider, | ||
| LlmModelProfilePolicy::new() | ||
| .allow_model_profile(interactive_model(), Some("host-selected-model".to_string())), | ||
| ); | ||
|
|
||
| gateway | ||
| .stream_model_with_capabilities( | ||
| model_request(interactive_model()), | ||
| Arc::new(GatewayCapabilityPort::with_native_deferred_surface(false)), | ||
| ) | ||
| .await | ||
| .expect("primary tool completion"); | ||
|
|
||
| let primary_requests = primary.tool_requests.lock().expect("primary requests"); | ||
| assert_eq!(primary_requests.len(), 1); | ||
| assert!(primary_requests[0].deferred_tool_names.is_empty()); | ||
| assert!(primary_requests[0].tool_references.is_empty()); | ||
| assert_eq!( | ||
| primary_requests[0] | ||
| .tools | ||
| .iter() | ||
| .map(|tool| tool.name.as_str()) | ||
| .collect::<Vec<_>>(), | ||
| vec!["demo__echo"], | ||
| "the non-deferred primary must receive its dynamic fallback surface" | ||
| ); | ||
| assert!( | ||
| cheap | ||
| .tool_requests | ||
| .lock() | ||
| .expect("cheap requests") | ||
| .is_empty(), | ||
| "tool-capable calls always dispatch through the primary model" | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Rename this test to match what it asserts.
The name says the gateway "uses primary deferred capability when smart router cheap supports it". The assertions prove the opposite and more valuable invariant: the primary is not deferred-capable, so the request carries empty deferred_tool_names, empty tool_references, and the dynamic full surface. The cheap deferred provider is never reached for tool-capable calls.
That distinction matters. Sending defer_loading fields to a provider that cannot parse them is exactly the failure this guards. A reader who trusts the current name will draw the wrong conclusion about routing.
♻️ Proposed rename
-async fn gateway_uses_primary_deferred_capability_when_smart_router_cheap_supports_it() {
+async fn gateway_ignores_cheap_model_deferred_support_and_keeps_primary_dynamic_surface() {📝 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
| #[tokio::test] | |
| async fn gateway_uses_primary_deferred_capability_when_smart_router_cheap_supports_it() { | |
| let primary = Arc::new(ToolAwareProvider::tool_stop_reply("primary response")); | |
| let cheap = | |
| Arc::new(ToolAwareProvider::tool_stop_reply("cheap response").with_deferred_tool_loading()); | |
| let provider = Arc::new(SmartRoutingProvider::new( | |
| primary.clone(), | |
| cheap.clone(), | |
| SmartRoutingConfig::default(), | |
| )); | |
| let gateway = LlmProviderModelGateway::with_provider_identity( | |
| STATIC_PROVIDER_ID, | |
| provider, | |
| LlmModelProfilePolicy::new() | |
| .allow_model_profile(interactive_model(), Some("host-selected-model".to_string())), | |
| ); | |
| gateway | |
| .stream_model_with_capabilities( | |
| model_request(interactive_model()), | |
| Arc::new(GatewayCapabilityPort::with_native_deferred_surface(false)), | |
| ) | |
| .await | |
| .expect("primary tool completion"); | |
| let primary_requests = primary.tool_requests.lock().expect("primary requests"); | |
| assert_eq!(primary_requests.len(), 1); | |
| assert!(primary_requests[0].deferred_tool_names.is_empty()); | |
| assert!(primary_requests[0].tool_references.is_empty()); | |
| assert_eq!( | |
| primary_requests[0] | |
| .tools | |
| .iter() | |
| .map(|tool| tool.name.as_str()) | |
| .collect::<Vec<_>>(), | |
| vec!["demo__echo"], | |
| "the non-deferred primary must receive its dynamic fallback surface" | |
| ); | |
| assert!( | |
| cheap | |
| .tool_requests | |
| .lock() | |
| .expect("cheap requests") | |
| .is_empty(), | |
| "tool-capable calls always dispatch through the primary model" | |
| ); | |
| } | |
| #[tokio::test] | |
| async fn gateway_ignores_cheap_model_deferred_support_and_keeps_primary_dynamic_surface() { | |
| let primary = Arc::new(ToolAwareProvider::tool_stop_reply("primary response")); | |
| let cheap = | |
| Arc::new(ToolAwareProvider::tool_stop_reply("cheap response").with_deferred_tool_loading()); | |
| let provider = Arc::new(SmartRoutingProvider::new( | |
| primary.clone(), | |
| cheap.clone(), | |
| SmartRoutingConfig::default(), | |
| )); | |
| let gateway = LlmProviderModelGateway::with_provider_identity( | |
| STATIC_PROVIDER_ID, | |
| provider, | |
| LlmModelProfilePolicy::new() | |
| .allow_model_profile(interactive_model(), Some("host-selected-model".to_string())), | |
| ); | |
| gateway | |
| .stream_model_with_capabilities( | |
| model_request(interactive_model()), | |
| Arc::new(GatewayCapabilityPort::with_native_deferred_surface(false)), | |
| ) | |
| .await | |
| .expect("primary tool completion"); | |
| let primary_requests = primary.tool_requests.lock().expect("primary requests"); | |
| assert_eq!(primary_requests.len(), 1); | |
| assert!(primary_requests[0].deferred_tool_names.is_empty()); | |
| assert!(primary_requests[0].tool_references.is_empty()); | |
| assert_eq!( | |
| primary_requests[0] | |
| .tools | |
| .iter() | |
| .map(|tool| tool.name.as_str()) | |
| .collect::<Vec<_>>(), | |
| vec!["demo__echo"], | |
| "the non-deferred primary must receive its dynamic fallback surface" | |
| ); | |
| assert!( | |
| cheap | |
| .tool_requests | |
| .lock() | |
| .expect("cheap requests") | |
| .is_empty(), | |
| "tool-capable calls always dispatch through the primary model" | |
| ); | |
| } |
🤖 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/loop/ironclaw_loop_host/tests/llm_gateway.rs` around lines 382 - 428,
Rename the test function
gateway_uses_primary_deferred_capability_when_smart_router_cheap_supports_it to
describe the asserted invariant: tool-capable calls use the non-deferred primary
with the dynamic full tool surface, while the deferred cheap provider is not
dispatched. Keep the test implementation and assertions unchanged.
17 of 29 tasks
Collaborator
Author
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
2 participants
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.
Summary
defer_loadingdefinitions and host-generatedtool_referenceresults for supported models; non-Anthropic providers retain the existing dynamic resend behavior, while unsupported Anthropic models receive the stable full list without native deferred fields.Change Type
Linked Issue
Closes #6986
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warnings— used the narrower affected-package quality gate listed below.cargo build— affected crates were built by clippy and test commands.ironclaw_llmandironclaw_loop_hostsuites; focused stable-surface, gateway, and architecture-ratchet tests.cargo test -p <owning-crate> --features integrationif database-backed or runtime-integration behavior changed — Not applicable: no database or runtime-integration behavior changed.review-prorpr-shepherd --fixwas run before requesting review — Not run; the scoped test, clippy, architecture, and diff audits below were run directly.Test Strategy
User behavior:
After
tool_search,tool_describe, orcapability_infodiscovers a deferred tool, Anthropic can call that tool on the next turn without the advertised top-level tool array changing. That keeps the cache prefix reusable while preserving the existing promotion and dispatch behavior.Risk areas:
Tests added or updated:
defer_loadingandtool_referenceJSON encoding; supported-model gating; API-key header and payload loopback contract; wrapper delegation and existing provider error conformance.What the tests prove:
tool_referenceblocks; denied or unknown names cannot be smuggled into provider-visible references.Commands run:
cargo fmt --all -- --checkcargo test -p ironclaw_llm(922 unit tests, 2 module-charter tests)cargo test -p ironclaw_loop_host(full package suite)cargo test -p ironclaw_loop_host --lib deferred_tool_references_only_include_advertised_deferred_toolscargo test -p ironclaw_loop_host --lib search_discloses_tool_call_dispatches_target_and_promotes_next_turncargo test -p ironclaw_loop_host --test llm_gateway gateway_keeps_native_deferred_tool_surface_stable_across_promotioncargo test -p ironclaw_hookscargo test -p ironclaw_turn_runnercargo test -p ironclaw_architecture_tests(the initial run identified the obsolete rig default-token seam; after removal, its focused ratchet passed)cargo test -p ironclaw_architecture_tests --test reborn_struct_test_support_ratchetcargo clippy -p ironclaw_llm -p ironclaw_loop_host -p ironclaw_hooks -p ironclaw_turn_runner -p ironclaw_composition --all-targets -- -D warningsgit diff origin/main...HEAD --checkSecurity Impact
No authority is added or widened. Deferred references are derived only from host-generated tool-result messages and intersected with the exact policy-authorized deferred surface before provider encoding. Credentials remain host-side; API keys use
x-api-key, OAuth uses bearer auth plus its beta header, and the loopback test asserts those modes do not cross-contaminate.Reborn Trust-Boundary Checklist
production_adapters_conform_to_provider_error_fixture_matrixand the full LLM suite.serde(default)fields fail closed or have migration tests: no durable or security-bearing serde fields were added; request metadata defaults to empty and cannot widen authorization.Database Impact
None.
Blast Radius
Touches capability-surface disclosure and its production decorators, LLM request metadata/wrappers, the model gateway, and Anthropic API-key/OAuth request encoding. Non-Anthropic providers keep the existing dynamic tool-definition path. The main compatibility risk is Anthropic model support, mitigated by model-gating native fields and retaining the stable full-list fallback.
Rollback Plan
Revert this PR. That restores mid-run promotion/resend behavior and the rig-backed API-key Anthropic adapter. No schema, persisted data, migration, or external state cleanup is required.
Review Follow-Through
Reviewer judgment is especially welcome on the supported Anthropic model allowlist and the decision to use the direct Messages adapter for API-key Anthropic until rig-core can express
defer_loadingandtool_reference.Review track: B (feature / maintainer-approved issue)