refactor(loop): shed the model gateway and tool disclosure into loop_host (WS3/WS4) - #7064
Conversation
…host (WS3/WS4) Moves two clusters out of `ironclaw_runner` into `ironclaw_loop_host` and re-layers the two loop-tier crates, clearing three `LAYER_MATRIX_EXCEPTIONS`. Model gateway + port adapters -> loop_host (PROPOSAL §6.7.2 "gains: runner's model-gateway adapter"): `model_gateway.rs` (+ `prompt_cache_activity`), `model_gateway_error_mapping.rs`, `model_routes.rs`, the driver-host model gateway and port adapters, and their two integration targets. `model_routes` had to travel (the gateway names eight of its types, so leaving it behind would make `loop_host -> runner` a cycle); `model_failure_mapping.rs` had to stay (its only callers are the two drivers that stay). Tool disclosure -> loop_host with zero new dependencies: it is a `LoopCapabilityPort` decorator, which `families/loop.md` already assigns to loop_host. The row's `/product` alternative is refuted, not skipped — `loops -> products` is an illegal upward edge, so its ~160 lines of prompt content cannot relocate there, and need not: `crates/ironclaw_loop_host/prompts/` already holds five prompt assets. Net: `ironclaw_runner` sheds `ironclaw_llm`, `ironclaw_common`, `base64` and `jsonschema` outright — the provider cone is out of the turn runner — and drops 33.2k -> 22.0k source lines. `LAYER_MATRIX_EXCEPTIONS` 13 -> 10 (`runner -> agent_loop`, `runner -> loop_host`, `hooks -> wasm_limiter`), with the baseline lowered in the same change. Enforcement: `reborn_runner_sheds.rs` pins the moved items at their new home and absent from the old, proves the manifest edges through `cargo metadata`, holds a reasoned shrink-only residue list, and pins the two `loops` layer declarations so a revert cannot silently need the deleted exceptions back. Un-masking: runner 458 -> 259, loop_host 572 -> 771 — 199 moved by identical name, 3 changed module path only, 0 lost, 0 edited for content. Deferred with measurements (see CHECKLIST WS4): `runtime.rs` `build_*` -> composition costs seven `pub(crate)` -> `pub` widenings in the crate the row narrows and moves decorator-chain ownership into the app layer; the `production_readiness` deletion is callerless as claimed but cascades into five `driver_registry.rs` types. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
🚅 Deployed to the ironclaw-pr-7064 environment in ironclaw-ci-preview
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR moves model gateway, model-route, host-port adapter, and progressive tool-disclosure ownership from ChangesWS3 runner shed
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
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 |
…d home Fixes the live references the WS3 move invalidated: the trace command's model-call row, the engine-v2 parity map's test paths, the integration test's path + visibility note, and one scenario doc comment. Also corrects `model_gateway.rs`'s module doc, which claimed the adapter lives "in the standalone Reborn composition crate" — never true of any tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
🔎 Review · PR #7064
Execution result is invalid The structured result could not be verified. Automatic · PR opened · attempt 1 of 3 · failed after 2m 11s Failure details
|
# Conflicts: # docs/reborn/target-architecture/CHECKLIST.md
|
Two follow-up commits on top of the shed:
The only true overlap with main's two new commits is Re-verified post-merge: |
`scripts/ci/reborn_pr_test_plan.py` had no rule for `.claude/`, so its fail-closed arm raised `unclassified pull-request path` on any PR that edited a skill, a command, or a rule — failing the `Detect Reborn test scope` job and skipping every downstream Reborn lane, on a documentation-only change. This PR hit it by repointing `.claude/commands/trace.md`'s model-call row at the moved gateway. Agent guidance is prose with no Rust or E2E surface any Reborn lane can exercise — the same class as `docs/`, which is already ignored. Classifying it is the fix; loosening the fail-closed arm is not, and the arm is untouched. Two regression tests, both red without the classification (verified by reverting it): guidance paths are accepted and select no lane, and a guidance edit riding along with a crate change still selects that crate's lane, so the ignore stays per-path rather than per-PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`check_no_panics.py`'s `has_cfg_test_module_declaration` only recognises a FLAT `#[path = "x.rs"]`; a module declared in a non-`mod.rs` file must spell the directory (`#[path = "loop_driver_host/x.rs"]`), which the regex misses. The carved file therefore read as production to the delta scan, and its six fixture `.unwrap()`s failed `Fast deterministic checks`. Inlining the module is both the fix and the local convention — `loop_driver_host.rs` already carries four inline `#[cfg(test)]` modules — and it keeps the three test paths identical (`loop_driver_host::thread_scope_tests::*`). The scanner gap is pre-existing and latent for the two sibling files declared the same way (`tests.rs`, `compaction_tests.rs`); neither has ever tripped it because their panics sit under item-level `#[cfg(test)]` attributes the scanner does track. Reported rather than fixed here: widening that regex changes a security-adjacent gate's classification and has baseline implications. Verified: `--base origin/main --head HEAD`, `--reborn-baseline`, and `--self-test` all clean; runner test roster unchanged at 259. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 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_architecture/tests/reborn_runner_sheds.rs`:
- Around line 54-57: The MOVED_ITEMS documentation and entries disagree about
whether module-private disclosure helpers are pinned. Update the inventory
around MOVED_ITEMS and the entries for CapabilityCatalog, PromotedSet,
DisclosureCaps, ActiveSet, and ToolTier so the documented contract is
consistent: either describe the tool-disclosure cluster as an intentionally
pinned package or retain only package-level types in the inventory.
- Around line 471-486: Hoist the loop-invariant crate_directory(&root, RUNNER)
call out of the per-file loop and compute the crate directory once before
iterating over runner entries. Reuse that value in the relative(path, ...) call
while preserving the existing matching and insertion behavior.
- Around line 582-583: Update the negative assertion in the relevant self-test
to query the fixture’s actual impl-header name, LlmProviderModelGateway, rather
than the absent SomethingNeverDefined name. Keep the assertion verifying that
defines does not treat the impl header as a definition, while preserving the
existing positive definition coverage.
- Around line 405-415: Update the documentation above dependency_names to
accurately describe cargo metadata --no-deps output: dependency["name"] is the
package identity, while rename is the optional manifest alias; alternatively,
change the helper to assert the intended package-name field. Keep the
implementation and renamed-dependency behavior consistent with the corrected
documentation.
In `@crates/ironclaw_loop_host/src/driver_host_port_adapters.rs`:
- Around line 480-525: Add tests in turn_error_to_host_error_tests for the
missing mappings: verify TurnError::Unauthorized produces
AgentLoopHostErrorKind::Unauthorized, and verify both TurnError::InvalidRequest
and TurnError::InvalidRunOriginAdapter produce
AgentLoopHostErrorKind::InvalidInvocation.
In `@crates/ironclaw_loop_host/src/model_gateway.rs`:
- Around line 17-23: Add a linked decomposition tracking issue for
model_gateway.rs, documenting planned boundaries for provider conversion, route
handling, error mapping, and tests. Keep this decomposition separate from the
current behavior-preserving change and reference the issue using the
repository’s established tracking convention.
In `@crates/ironclaw_loop_host/src/model_routes.rs`:
- Around line 50-117: Remove the identity-mirror struct ActiveModelRouteSettings
and its constructors/accessors, including from_active_settings on ModelRoute.
Add ModelRoute::from_llm_config accepting ironclaw_llm::LlmConfig and construct
the route directly from active_provider_id() and active_model_name(), preserving
existing validation through ModelRoute::new.
- Around line 415-443: Update validate_route_component and ModelRouteError so
validation failures preserve the underlying reason from
validate_model_route_component_value, including whether the value exceeds the
limit or contains a disallowed character. Add or populate a reason field when
constructing ModelRouteError instead of mapping the error to a reasonless
InvalidRoute; ensure Display or equivalent error reporting exposes that
preserved detail for validate_provider_id, validate_model_id, and
validate_version_token.
In `@crates/ironclaw_loop_host/src/thread_resolving_model_gateway.rs`:
- Around line 18-40: Keep the fields of ThreadResolvingLoopModelGateway private
and replace external struct-literal construction with a public owning-crate
factory, preferably ThreadResolvingLoopModelGateway::new accepting a typed
configuration object. Update ironclaw_runner’s loop_driver_host.rs construction
site to call that factory while preserving all existing initialization values
and invariants.
In `@docs/reborn/target-architecture/PROPOSAL.md`:
- Line 84: Reconcile the two pre-move ironclaw_runner line counts in this bullet
by selecting the authoritative 33.2k baseline used by the loop_host arithmetic,
or explicitly label 33.1k as the older measurement. Update the opening count and
related “was” wording so §2.4 and the checklist derive from one consistent
baseline.
In `@tests/integration/coverage-floor.toml`:
- Around line 71-95: Restore artifact-derived absolute coverage ratchets in both
the ironclaw_runner entry and the ironclaw_loop_host entry: add
floor_covered_lines and captured_total_lines values from the merged coverage
artifact, without estimating them. Preserve the existing percentage floors and
rationale, or document an explicit bounded exception if artifact values cannot
be added.
🪄 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: 90d555a9-a0a0-4878-928f-ee31d5754a70
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (55)
.github/workflows/code_style.ymlcrates/ironclaw_architecture/tests/reborn_dependency_boundaries.rscrates/ironclaw_architecture/tests/reborn_extension_specificity.rscrates/ironclaw_architecture/tests/reborn_runner_sheds.rscrates/ironclaw_architecture/tests/reborn_struct_test_support_ratchet.rscrates/ironclaw_hooks/Cargo.tomlcrates/ironclaw_loop_host/AGENTS.mdcrates/ironclaw_loop_host/CLAUDE.mdcrates/ironclaw_loop_host/Cargo.tomlcrates/ironclaw_loop_host/src/context_shadow.rscrates/ironclaw_loop_host/src/driver_host_port_adapters.rscrates/ironclaw_loop_host/src/driver_host_port_adapters/tests.rscrates/ironclaw_loop_host/src/lib.rscrates/ironclaw_loop_host/src/model_gateway.rscrates/ironclaw_loop_host/src/model_gateway/prompt_cache_activity.rscrates/ironclaw_loop_host/src/model_gateway_error_mapping.rscrates/ironclaw_loop_host/src/model_routes.rscrates/ironclaw_loop_host/src/thread_resolving_model_gateway.rscrates/ironclaw_loop_host/src/tool_disclosure.rscrates/ironclaw_loop_host/src/tool_disclosure_mode.rscrates/ironclaw_loop_host/src/tool_disclosure_port.rscrates/ironclaw_loop_host/tests/llm_gateway.rscrates/ironclaw_loop_host/tests/model_routes.rscrates/ironclaw_product/tests/inbound_turn_contract.rscrates/ironclaw_product/tests/support/planned_agent_loop.rscrates/ironclaw_reborn_composition/src/factory/production_backend_assembly.rscrates/ironclaw_reborn_composition/src/lib.rscrates/ironclaw_reborn_composition/src/model_gateway_assembly.rscrates/ironclaw_reborn_composition/src/root/product_live_adapters.rscrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime/tests/default_system_prompt.rscrates/ironclaw_reborn_composition/src/runtime_input.rscrates/ironclaw_reborn_composition/tests/product_live_adapters.rscrates/ironclaw_reborn_composition/tests/trigger_poller_e2e.rscrates/ironclaw_runner/AGENTS.mdcrates/ironclaw_runner/CLAUDE.mdcrates/ironclaw_runner/Cargo.tomlcrates/ironclaw_runner/src/lib.rscrates/ironclaw_runner/src/loop_driver_host.rscrates/ironclaw_runner/src/loop_driver_host/thread_scope_tests.rscrates/ironclaw_runner/src/runtime.rscrates/ironclaw_runner/src/tool_disclosure_bridge.rsdocs/reborn/target-architecture/CHECKLIST.mddocs/reborn/target-architecture/PLAN.mddocs/reborn/target-architecture/PROPOSAL.mddocs/reborn/target-architecture/families/loop.mdscripts/ci/ws12_workflow_contracts.pyscripts/no_panics_reborn_baseline.txtscripts/reborn-e2e-rust.shtests/integration/coverage-floor.tomltests/integration/support/builder.rstests/integration/support/group.rstests/integration/support/group_options.rstests/integration/support/triggered_submit.rstests/support/reborn_parity_qa/qa_trace.rs
💤 Files with no reviewable changes (1)
- crates/ironclaw_runner/src/tool_disclosure_bridge.rs
| use crate::{ | ||
| HostManagedModelError, HostManagedModelErrorKind, HostManagedModelGateway, | ||
| HostManagedModelMessage, HostManagedModelMessageRole, HostManagedModelRequest, | ||
| HostManagedModelResponse, HostManagedModelRouteSnapshot, HostManagedModelStreamSink, | ||
| HostManagedToolResultContent, ModelCost, StaticModelCostTable, ThreadBackedLoopContextPort, | ||
| ThreadBackedLoopModelPort, ThreadContextWindowCache, | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add a decomposition tracking issue for the relocated gateway file.
crates/ironclaw_loop_host/src/model_gateway.rs reaches at least Line [3604]. The file is therefore over 3,000 lines at its new owner.
Add a linked decomposition issue and identify boundaries for provider conversion, route handling, error mapping, and tests. Keep the split separate if this PR must remain behavior-preserving.
As per coding guidelines, files over 3,000 lines require a decomposition tracking issue.
🤖 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_host/src/model_gateway.rs` around lines 17 - 23, Add a
linked decomposition tracking issue for model_gateway.rs, documenting planned
boundaries for provider conversion, route handling, error mapping, and tests.
Keep this decomposition separate from the current behavior-preserving change and
reference the issue using the repository’s established tracking convention.
Source: Coding guidelines
There was a problem hiding this comment.
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: 11
🤖 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_architecture/tests/reborn_runner_sheds.rs`:
- Around line 54-57: The MOVED_ITEMS documentation and entries disagree about
whether module-private disclosure helpers are pinned. Update the inventory
around MOVED_ITEMS and the entries for CapabilityCatalog, PromotedSet,
DisclosureCaps, ActiveSet, and ToolTier so the documented contract is
consistent: either describe the tool-disclosure cluster as an intentionally
pinned package or retain only package-level types in the inventory.
- Around line 471-486: Hoist the loop-invariant crate_directory(&root, RUNNER)
call out of the per-file loop and compute the crate directory once before
iterating over runner entries. Reuse that value in the relative(path, ...) call
while preserving the existing matching and insertion behavior.
- Around line 582-583: Update the negative assertion in the relevant self-test
to query the fixture’s actual impl-header name, LlmProviderModelGateway, rather
than the absent SomethingNeverDefined name. Keep the assertion verifying that
defines does not treat the impl header as a definition, while preserving the
existing positive definition coverage.
- Around line 405-415: Update the documentation above dependency_names to
accurately describe cargo metadata --no-deps output: dependency["name"] is the
package identity, while rename is the optional manifest alias; alternatively,
change the helper to assert the intended package-name field. Keep the
implementation and renamed-dependency behavior consistent with the corrected
documentation.
In `@crates/ironclaw_loop_host/src/driver_host_port_adapters.rs`:
- Around line 480-525: Add tests in turn_error_to_host_error_tests for the
missing mappings: verify TurnError::Unauthorized produces
AgentLoopHostErrorKind::Unauthorized, and verify both TurnError::InvalidRequest
and TurnError::InvalidRunOriginAdapter produce
AgentLoopHostErrorKind::InvalidInvocation.
In `@crates/ironclaw_loop_host/src/model_gateway.rs`:
- Around line 17-23: Add a linked decomposition tracking issue for
model_gateway.rs, documenting planned boundaries for provider conversion, route
handling, error mapping, and tests. Keep this decomposition separate from the
current behavior-preserving change and reference the issue using the
repository’s established tracking convention.
In `@crates/ironclaw_loop_host/src/model_routes.rs`:
- Around line 50-117: Remove the identity-mirror struct ActiveModelRouteSettings
and its constructors/accessors, including from_active_settings on ModelRoute.
Add ModelRoute::from_llm_config accepting ironclaw_llm::LlmConfig and construct
the route directly from active_provider_id() and active_model_name(), preserving
existing validation through ModelRoute::new.
- Around line 415-443: Update validate_route_component and ModelRouteError so
validation failures preserve the underlying reason from
validate_model_route_component_value, including whether the value exceeds the
limit or contains a disallowed character. Add or populate a reason field when
constructing ModelRouteError instead of mapping the error to a reasonless
InvalidRoute; ensure Display or equivalent error reporting exposes that
preserved detail for validate_provider_id, validate_model_id, and
validate_version_token.
In `@crates/ironclaw_loop_host/src/thread_resolving_model_gateway.rs`:
- Around line 18-40: Keep the fields of ThreadResolvingLoopModelGateway private
and replace external struct-literal construction with a public owning-crate
factory, preferably ThreadResolvingLoopModelGateway::new accepting a typed
configuration object. Update ironclaw_runner’s loop_driver_host.rs construction
site to call that factory while preserving all existing initialization values
and invariants.
In `@docs/reborn/target-architecture/PROPOSAL.md`:
- Line 84: Reconcile the two pre-move ironclaw_runner line counts in this bullet
by selecting the authoritative 33.2k baseline used by the loop_host arithmetic,
or explicitly label 33.1k as the older measurement. Update the opening count and
related “was” wording so §2.4 and the checklist derive from one consistent
baseline.
In `@tests/integration/coverage-floor.toml`:
- Around line 71-95: Restore artifact-derived absolute coverage ratchets in both
the ironclaw_runner entry and the ironclaw_loop_host entry: add
floor_covered_lines and captured_total_lines values from the merged coverage
artifact, without estimating them. Preserve the existing percentage floors and
rationale, or document an explicit bounded exception if artifact values cannot
be added.
🪄 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: 90d555a9-a0a0-4878-928f-ee31d5754a70
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (55)
.github/workflows/code_style.ymlcrates/ironclaw_architecture/tests/reborn_dependency_boundaries.rscrates/ironclaw_architecture/tests/reborn_extension_specificity.rscrates/ironclaw_architecture/tests/reborn_runner_sheds.rscrates/ironclaw_architecture/tests/reborn_struct_test_support_ratchet.rscrates/ironclaw_hooks/Cargo.tomlcrates/ironclaw_loop_host/AGENTS.mdcrates/ironclaw_loop_host/CLAUDE.mdcrates/ironclaw_loop_host/Cargo.tomlcrates/ironclaw_loop_host/src/context_shadow.rscrates/ironclaw_loop_host/src/driver_host_port_adapters.rscrates/ironclaw_loop_host/src/driver_host_port_adapters/tests.rscrates/ironclaw_loop_host/src/lib.rscrates/ironclaw_loop_host/src/model_gateway.rscrates/ironclaw_loop_host/src/model_gateway/prompt_cache_activity.rscrates/ironclaw_loop_host/src/model_gateway_error_mapping.rscrates/ironclaw_loop_host/src/model_routes.rscrates/ironclaw_loop_host/src/thread_resolving_model_gateway.rscrates/ironclaw_loop_host/src/tool_disclosure.rscrates/ironclaw_loop_host/src/tool_disclosure_mode.rscrates/ironclaw_loop_host/src/tool_disclosure_port.rscrates/ironclaw_loop_host/tests/llm_gateway.rscrates/ironclaw_loop_host/tests/model_routes.rscrates/ironclaw_product/tests/inbound_turn_contract.rscrates/ironclaw_product/tests/support/planned_agent_loop.rscrates/ironclaw_reborn_composition/src/factory/production_backend_assembly.rscrates/ironclaw_reborn_composition/src/lib.rscrates/ironclaw_reborn_composition/src/model_gateway_assembly.rscrates/ironclaw_reborn_composition/src/root/product_live_adapters.rscrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime/tests/default_system_prompt.rscrates/ironclaw_reborn_composition/src/runtime_input.rscrates/ironclaw_reborn_composition/tests/product_live_adapters.rscrates/ironclaw_reborn_composition/tests/trigger_poller_e2e.rscrates/ironclaw_runner/AGENTS.mdcrates/ironclaw_runner/CLAUDE.mdcrates/ironclaw_runner/Cargo.tomlcrates/ironclaw_runner/src/lib.rscrates/ironclaw_runner/src/loop_driver_host.rscrates/ironclaw_runner/src/loop_driver_host/thread_scope_tests.rscrates/ironclaw_runner/src/runtime.rscrates/ironclaw_runner/src/tool_disclosure_bridge.rsdocs/reborn/target-architecture/CHECKLIST.mddocs/reborn/target-architecture/PLAN.mddocs/reborn/target-architecture/PROPOSAL.mddocs/reborn/target-architecture/families/loop.mdscripts/ci/ws12_workflow_contracts.pyscripts/no_panics_reborn_baseline.txtscripts/reborn-e2e-rust.shtests/integration/coverage-floor.tomltests/integration/support/builder.rstests/integration/support/group.rstests/integration/support/group_options.rstests/integration/support/triggered_submit.rstests/support/reborn_parity_qa/qa_trace.rs
💤 Files with no reviewable changes (1)
- crates/ironclaw_runner/src/tool_disclosure_bridge.rs
🛑 Comments failed to post (2)
crates/ironclaw_loop_host/src/model_routes.rs (2)
50-117: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Check whether ActiveModelRouteSettings has an independent contract # (e.g. persisted/serialized on its own) or is only ever bridged into ModelRoute. rg -n -C3 'ActiveModelRouteSettings' --type=rustRepository: nearai/ironclaw
Length of output: 153
🏁 Script executed:
#!/bin/bash set -u echo "== Locate model_routes.rs and related symbols ==" fd -a 'model_routes\.rs$' . || true rg -n "ActiveModelRouteSettings|ModelRoute|LlmConfig|LlmProvider" crates --type=rust || true echo "== File tree snippets ==" for f in $(rg -l "ActiveModelRouteSettings|struct ModelRoute|impl ModelRoute" crates --type=rust || true); do echo "--- $f" sed -n '1,180p' "$f" done echo "== Dependency references ==" rg -n "ironclaw_llm|ModelRouter|LlmConfig|active_provider_id|active_model_name" crates/ironclaw_loop_host --type=rust || trueRepository: nearai/ironclaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash set -u echo "== exact ActiveModelRouteSettings occurrences outside generated output ==" rg -n "ActiveModelRouteSettings" crates --type=rust || true echo "== model_routes.rs source ==" sed -n '1,180p' crates/ironclaw_loop_host/src/model_routes.rs echo "== model_gateway_assembly LlmConfig dependency source ==" sed -n '1,190p' crates/ironclaw_reborn_composition/src/model_gateway_assembly.rsRepository: nearai/ironclaw
Length of output: 13411
Remove
ActiveModelRouteSettingsas an identity mirror.
ActiveModelRouteSettingsandModelRouteboth holdprovider_id/model_id, validate identically, and only bridge into each other. Per the repo rule: mirror structs are allowed only when wire/API and internal representations evolve independently; field-for-field identity mirrors without that rationale are a violation. AddModelRoute::from_llm_configdirectly fromironclaw_llm::LlmConfigand removeActiveModelRouteSettings.The
ActiveModelRouteSettings::from_llm_configdependency is the only current directIronClaw LlmConfigusage fromironclaw_loop_host, so eliminating it keeps that dependency from spreading beyond where the model gateway is assembled.🤖 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_host/src/model_routes.rs` around lines 50 - 117, Remove the identity-mirror struct ActiveModelRouteSettings and its constructors/accessors, including from_active_settings on ModelRoute. Add ModelRoute::from_llm_config accepting ironclaw_llm::LlmConfig and construct the route directly from active_provider_id() and active_model_name(), preserving existing validation through ModelRoute::new.Sources: Coding guidelines, Learnings
415-443: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
validate_route_componentdrops the validation cause.
validate_model_route_component_value(...).map_err(|_| ModelRouteError::new(ModelRouteErrorKind::InvalidRoute))discards the underlying reason (length vs. disallowed character) from all three callers (validate_provider_id,validate_model_id,validate_version_token).ModelRouteErrorhas no reason field, so the detail is unrecoverable;Displayonly ever prints"invalid_route".Preserve the cause: add a
reason: Stringfield toModelRouteError(or log the source error before mapping) instead of discarding it withmap_err(|_| ...).🛠️ Proposed fix to preserve the cause
#[derive(Debug, Clone, PartialEq, Eq)] pub struct ModelRouteError { kind: ModelRouteErrorKind, + reason: String, } impl ModelRouteError { - fn new(kind: ModelRouteErrorKind) -> Self { - Self { kind } + fn new(kind: ModelRouteErrorKind, reason: impl Into<String>) -> Self { + Self { kind, reason: reason.into() } } } impl fmt::Display for ModelRouteError { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter.write_str(self.kind.as_str()) + write!(formatter, "{}: {}", self.kind.as_str(), self.reason) } } fn validate_route_component( label: &'static str, value: String, max_bytes: usize, allowed: impl Fn(char) -> bool, ) -> Result<String, ModelRouteError> { let trimmed = value.trim().to_string(); validate_model_route_component_value(label, &trimmed, max_bytes, allowed) - .map_err(|_| ModelRouteError::new(ModelRouteErrorKind::InvalidRoute))?; + .map_err(|error| ModelRouteError::new(ModelRouteErrorKind::InvalidRoute, error.to_string()))?; Ok(trimmed) }🤖 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_host/src/model_routes.rs` around lines 415 - 443, Update validate_route_component and ModelRouteError so validation failures preserve the underlying reason from validate_model_route_component_value, including whether the value exceeds the limit or contains a disallowed character. Add or populate a reason field when constructing ModelRouteError instead of mapping the error to a reasonless InvalidRoute; ensure Display or equivalent error reporting exposes that preserved detail for validate_provider_id, validate_model_id, and validate_version_token.Source: Coding guidelines
CI triage — both failures were gates catching real gaps, and both are fixedThe first run had two red lanes; 1. Not caused by this PR's moves: Fix: classify 2.
Fix: inline the module, which is also this file's own convention ( Reported, not fixed here: that scanner gap is pre-existing and latent for the two sibling files declared the same way ( Re-verified locally on |
…oud inventory Both pre-existing on main, both found by this PR: the PR test planner's missing `.claude/` classification (fixed here) and `check_no_panics.py`'s flat-only `#[path]` recognition (reported, sidestepped by inlining). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Round 2 surfaced the next `unmapped test or CI path`: `scripts/no_panics_reborn_baseline.txt` and `scripts/reborn-e2e-rust.sh`. That arm is deliberate — repo-root `scripts/` is not prefix-classified, so each file gets a decision rather than a blanket ignore — and both decisions are recorded beside the constant: the panic baseline is owned end-to-end by Code Style's `check_no_panics.py --reborn-baseline`, and the E2E selector script is driven by the `Reborn E2E` workflow, which has its own scope detector and which this planner does not schedule. The self-test asserts both halves: the two decided paths are accepted and select no lane, AND an undecided sibling still refuses — so the fix cannot drift into the blanket prefix the arm exists to prevent. Process note: discovering these one CI round at a time is avoidable. Running the planner locally over the PR's own changed-path set finds every unclassified path in one pass; the whole set now plans as `selected` over 3 buckets, root partition 0, and integration lanes 0 and 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-vacuous scan self-test Five findings from the CodeRabbit pass, all accepted: - **`ThreadResolvingLoopModelGateway` fields go back to private.** The shed had turned eleven `pub(super)` fields into `pub` so the runner's struct literal kept compiling — a real widening, and against root `CLAUDE.md`'s "module-specific initialization must live in the owning crate as a public factory". Replaced by `ThreadResolvingLoopModelGatewayParts` + a `new` that destructures it: the caller still gets a compile error when a field is added (the property the `pub`-fields shape had) without any downstream crate being able to assemble a gateway outside the host's construction path. - **The definition scanner's impl-header self-test was vacuous.** It asserted on a name absent from the fixture, so a regression that accepted `impl X for Y` headers as definitions would have stayed green. The fixture now carries a name that appears ONLY as an impl target. - **The `cargo metadata` rename comment described behaviour the code does not have.** It claimed the helper resolves through `rename`; it reads `name`, which under `--no-deps` is the package identity — which is exactly why a renamed edge cannot hide. Comment corrected to the real mechanism. - **`MOVED_ITEMS`' doc contradicted its own contents** on the five `pub(crate)` tool-disclosure types. They are pinned deliberately: visibility is not the criterion, membership in the moved unit's contract is, and a half-move that left one behind would compile. - **`turn_error_to_host_error` gained the two uncovered arms**, `Unauthorized` and the two request-shaped variants. The function moved crates and became `pub` in this PR, so the security-relevant arm was newly reachable and untested. Red-then-green verified by reclassifying `Unauthorized` to `InvalidInvocation`: that test alone fails, and only it. Also hoisted a loop-invariant `crate_directory` walk out of the residue scan's per-file loop. Rosters: runner 259 (unchanged), loop_host 771 -> 773 (the two new tests). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review pass — five findings, all accepted and fixed (
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
docs/reborn/target-architecture/PROPOSAL.md (1)
84-84: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse one authoritative
ironclaw_runnerbaseline.The bullet still opens with
33.1klines, but the same paragraph states that the post-shed crate was22.0k, was33.2k. The loop-host arithmetic also uses33.2k. Change the opening figure to33.2kor label33.1kas an older measurement.Proposed wording
-- **ironclaw_runner** (✎ **33.1k** lines, was 38.3k): +- **ironclaw_runner** (✎ **33.2k** lines, was 38.3k):🤖 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/reborn/target-architecture/PROPOSAL.md` at line 84, Update the opening ironclaw_runner size in this bullet from 33.1k to the authoritative 33.2k baseline, keeping the existing post-shed 22.0k figure and loop-host arithmetic consistent.crates/ironclaw_loop_host/src/model_gateway.rs (1)
25-31: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winKeep the decomposition issue linked for the relocated gateway file.
crates/ironclaw_loop_host/src/model_gateway.rsstill exceeds 3,000 lines; the file reaches at least Line [3604]. This PR changes the owner but does not show a decomposition tracking issue for the new owner. Add a linked issue with boundaries for provider conversion, route handling, error mapping, and tests.As per coding guidelines, files over 3,000 lines require a decomposition tracking issue.
🤖 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_host/src/model_gateway.rs` around lines 25 - 31, Add a decomposition-tracking issue reference for the relocated model gateway owner, covering separate boundaries for provider conversion, route handling, error mapping, and tests. Place the reference near the module’s existing ownership or documentation context in model_gateway.rs, without changing the imported symbols or implementation 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.
Inline comments:
In `@scripts/ci/test_reborn_pr_test_plan.py`:
- Around line 366-387: Update both
`scripts/ci/classify-test-scope.sh:is_docs_only_path()` and
`scripts/ci/test-classify-test-scope.sh` so guidance files under `.claude/**`,
including nested Markdown and `CLAUDE.md`/`AGENTS.md` paths, are consistently
classified as docs-only. Preserve the resulting no-test-lane behavior validated
by `test_agent_guidance_is_classified_and_selects_no_rust_lane`.
In `@tests/integration/group_triggers/scenario_trigger_self_create_denied.rs`:
- Line 13: Update the module reference in the test documentation around
validate_provider_tool_call to point to its defining module,
ironclaw_loop_host::capability_port::provider_validation, or the corresponding
provider_validation.rs file, instead of ironclaw_loop_host::model_gateway.
---
Duplicate comments:
In `@crates/ironclaw_loop_host/src/model_gateway.rs`:
- Around line 25-31: Add a decomposition-tracking issue reference for the
relocated model gateway owner, covering separate boundaries for provider
conversion, route handling, error mapping, and tests. Place the reference near
the module’s existing ownership or documentation context in model_gateway.rs,
without changing the imported symbols or implementation behavior.
In `@docs/reborn/target-architecture/PROPOSAL.md`:
- Line 84: Update the opening ironclaw_runner size in this bullet from 33.1k to
the authoritative 33.2k baseline, keeping the existing post-shed 22.0k figure
and loop-host arithmetic consistent.
🪄 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: 60fc246b-c3ef-4215-a6ab-3375acebb1c3
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (10)
.claude/commands/trace.mdcrates/ironclaw_loop_host/src/model_gateway.rscrates/ironclaw_runner/src/loop_driver_host.rsdocs/reborn/engine-v2-to-reborn-parity.mddocs/reborn/target-architecture/CHECKLIST.mddocs/reborn/target-architecture/PROPOSAL.mdscripts/ci/reborn_pr_test_plan.pyscripts/ci/test_reborn_pr_test_plan.pytests/integration/group_triggers/scenario_trigger_self_create_denied.rstests/integration/tool_disclosure.rs
| def test_agent_guidance_is_classified_and_selects_no_rust_lane(self) -> None: | ||
| """`.claude/**` is prose, like `docs/**`. | ||
|
|
||
| Regression for the gap #7064 hit: the planner had no rule for | ||
| `.claude/`, so its fail-closed arm rejected any PR editing a skill, a | ||
| command, or a rule — failing the whole `Tests (Reborn)` roll-up on a | ||
| documentation-only change. The assertion is deliberately paired: the | ||
| path must be *accepted* AND must select no Rust lane, so a future | ||
| "classification" that quietly turns guidance edits into a full matrix | ||
| fails here too. | ||
| """ | ||
| for path in ( | ||
| ".claude/commands/trace.md", | ||
| ".claude/rules/testing.md", | ||
| ".claude/skills/reborn-feature/SKILL.md", | ||
| ): | ||
| with self.subTest(path=path): | ||
| plan = self.plan("pull_request", [path]) | ||
| self.assertEqual(plan["mode"], "none", path) | ||
| self.assertEqual(plan["crate_buckets"], [], path) | ||
| self.assertEqual(plan["root_partitions"], [], path) | ||
| self.assertEqual(plan["integration_lanes"], [], path) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)(test_reborn_pr_test_plan\.py|classify-test-scope\.sh|.*no_rust_lane|.*reborn|.*scope|claude)' || true
echo
echo "== outline target =="
ast-grep outline scripts/ci/test_reborn_pr_test_plan.py --view expanded || true
echo
echo "== relevant lines test =="
sed -n '330,420p' scripts/ci/test_reborn_pr_test_plan.py
echo
echo "== classifier outline/lines =="
sed -n '1,240p' scripts/ci/classify-test-scope.shRepository: nearai/ironclaw
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== search classify-test-scope usage =="
rg -n "classify-test-scope|is_docs_only_path|TEST_PLAN|reborn|scope" scripts .github . -g '!**/*.csv' -g '!**/*.md' | head -n 250 || true
echo
echo "== deterministic shell path function probe =="
python3 - <<'PY'
from pathlib import Path
script = Path('scripts/ci/classify-test-scope.sh').read_text()
print('classify-test-scope exists:', Path('scripts/ci/classify-test-scope.sh').exists())
if 'is_docs_only_path' not in script:
print('no is_docs_only_path found')
else:
# Extract relevant shell control around is_docs_only_path until next top-level function or final fi? Use simple brace/line.
lines = script.splitlines()
start = next(i for i,l in enumerate(lines) if 'is_docs_only_path' in l)
# Print with line numbers
for i,l in enumerate(lines[max(0,start-20):start+120], start=max(0,start-20)-19):
print(f'{i:04d}: {l}')
PYRepository: nearai/ironclaw
Length of output: 30795
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,140p' scripts/ci/classify-test-scope.shRepository: nearai/ironclaw
Length of output: 5237
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== classify-test-scope tests relevant =="
sed -n '1,260p' scripts/ci/test-classify-test-scope.sh
echo
echo "== workflow references to classify-test-scope =="
rg -n "classify-test-scope|TEST_PLAN|changed_paths|only|workflow_dispatch|pull_request|merge_group|reborn-tests" .github/workflows scripts/ci -g '*.{yml,yaml,sh,py}' | head -n 220 || true
echo
echo "== shell is_docs_only_path behavior for nested paths =="
cat >/tmp/probe_shell.sh <<'BASH'
set -euo pipefail
is_docs_only_path() {
local path="$1"
case "$path" in
docs/*|.github/ISSUE_TEMPLATE/*|.github/pull_request_template.md)
return 0
;;
*.md)
case "$path" in
*/*) return 1 ;;
*) return 0 ;;
esac
;;
*).
return 1
;;
esac
}
for path in .claude/commands/trace.md .claude/rules/testing.md .claude/skills/reborn-feature/SKILL.md docs/reborn/README.md README.md; do
if is_docs_only_path "$path"; then echo "$path docs_only"; else echo "$path !docs_only"; fi
done
BASH
bash /tmp/probe_shell.sh
rm /tmp/probe_shell.shRepository: nearai/ironclaw
Length of output: 32418
Cover nested .claude/** in the live scope classifier.
scripts/ci/test-classify-test-scope.sh uses .claude/commands/trace.md as docs-only, but scripts/ci/classify-test-scope.sh:is_docs_only_path() treats nested Markdown as code and exits non-docs. Add the CLAUDE.md / AGENTS.md invariants for .claude/** guidance paths to both classifiers.
🤖 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 `@scripts/ci/test_reborn_pr_test_plan.py` around lines 366 - 387, Update both
`scripts/ci/classify-test-scope.sh:is_docs_only_path()` and
`scripts/ci/test-classify-test-scope.sh` so guidance files under `.claude/**`,
including nested Markdown and `CLAUDE.md`/`AGENTS.md` paths, are consistently
classified as docs-only. Preserve the resulting no-test-lane behavior validated
by `test_agent_guidance_is_classified_and_selects_no_rust_lane`.
Source: Path instructions
| //! | ||
| //! Traced, not assumed: denial happens at the model-gateway seam | ||
| //! (`ironclaw_runner::model_gateway`'s `validate_provider_tool_call`, via | ||
| //! (`ironclaw_loop_host::model_gateway`'s `validate_provider_tool_call`, via |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
bash scripts/codebase-graph.sh status || true
rg -n -C 2 '\bfn validate_provider_tool_call\b|mod provider_validation' \
crates/ironclaw_loop_host/srcRepository: nearai/ironclaw
Length of output: 5564
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a 'scenario_trigger_self_create_denied.rs' . | sed 's#^\./##'
file="$(fd 'scenario_trigger_self_create_denied.rs' . | head -n1)"
echo "FILE=$file"
wc -l "$file"
sed -n '1,80p' "$file"
rg -n 'validate_provider_tool_call|provider_validation|model_gateway' .Repository: nearai/ironclaw
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== module declarations =="
sed -n '44,52p' crates/ironclaw_loop_host/src/capability_port.rs
sed -n '240,250p' crates/ironclaw_loop_host/src/capability_port.rs
echo
echo "== definition =="
sed -n '1,35p' crates/ironclaw_loop_host/src/capability_port/provider_validation.rs
echo
echo "== public symbols in crate lib =="
sed -n '1,220p' crates/ironclaw_loop_host/src/lib.rs | grep -n 'pub (super|crate|mod)|capability_port|model_gateway' | head -n 80
rg -n 'pub (use|mod|struct|enum|type|trait|fn)\b.*validate_provider_tool_call|\bvalidate_provider_tool_call\b' crates/ironclaw_loop_host/src/lib.rs crates/ironclaw_loop_host/src
echo
echo "== call sites in loop_host =="
rg -n --glob '*.rs' 'validate_provider_tool_call|provider_validation' crates/ironclaw_loop_host/src -C 2Repository: nearai/ironclaw
Length of output: 2564
Correct the validate_provider_tool_call reference.
This integration test violates the “Module specs win ties” repo invariant by pointing validate_provider_tool_call at ironclaw_loop_host::model_gateway. The function is defined under ironclaw_loop_host::capability_port::provider_validation; use the defining module/filename or crates/ironclaw_loop_host/src/capability_port/provider_validation.rs.
🤖 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 `@tests/integration/group_triggers/scenario_trigger_self_create_denied.rs` at
line 13, Update the module reference in the test documentation around
validate_provider_tool_call to point to its defining module,
ironclaw_loop_host::capability_port::provider_validation, or the corresponding
provider_validation.rs file, instead of ironclaw_loop_host::model_gateway.
Round 3: the two fixed lanes are green. The one red lane is #7066's defect, reproduced on this tree — with local evidence that it is not this PR.At head
I am not duplicating the one-token fix — #7066 is MERGEABLE with 24/24 checks passing. This PR needs a merge of What I did instead: ran the affected lanes locally, so this PR's evidence does not depend on another PR. Both lane-0 and lane-1 suite lists, computed from
All four failures are the same environment gate, not behaviour: The suite that most directly exercises the moved code — One more thing worth recording for the next slice: |
Reconciles this lane with #7064, which landed the parallel WS3/WS4 runner sheds and edited the same coordination files. Five conflicts, all in shared coordination surfaces — no code move in this PR was altered (all 117 PR-only files are byte-identical to the pre-merge tip; the 5 apparent diffs are deletions absent on both sides). - `reborn_dependency_boundaries.rs`: the array auto-merged to the union of both sides' removals; only `WS0_LAYER_MATRIX_EXCEPTION_BASELINE` conflicted. Recomputed as `len()` of the merged list — 13 base, minus #7064's three (`hooks -> wasm_limiter`, `runner -> agent_loop`, `runner -> loop_host`) and this PR's three (`mcp -> extensions`, `scripts -> extensions`, `scripts -> resources`), plus this PR's justified `ironclaw_sandbox -> ironclaw_resources` = **8**. Counted by parsing only the entries between the const and its closing `];`, so the struct definition and the four test fixtures are excluded. - `loop_host/Cargo.toml`: both sides added a `[dev-dependencies]` line; kept both (`http` from main, `ironclaw_sandbox` from this PR). - `CHECKLIST.md`: kept both dated amendments in date order — #7064's `13 -> 10` and this PR's, with its count corrected from the authored-in-isolation `11` to the merged `8` exactly as the §11.2.2 row instructs. Also kept this PR's two new coverage-gate rows beside main's amended loud-inventory row. - `reborn_pr_test_plan.py`: both sides made the same `.claude/` fix; took main's landed wording (`startswith` makes tuple order irrelevant). - `test_reborn_pr_test_plan.py`: union of both sides' new tests, no name collisions — 46 tests pass. Also repointed the one PR-authored `changed-coverage-exemptions.toml` entry the merge shifted: `hosted_mcp.rs` moved +1 because main added a doc-comment line, so its line-keyed exemption now resolves to byte-identical source lines. The other stale entries in that manifest are inherited and already stale on main; left untouched. Verified: architecture suite 206 passed / 0 failed, `cargo check --all-targets` clean, `cargo fmt` a no-op, zero conflict markers, and both sides' `coverage-floor.toml` recaptures intact (runner 82.53 from #7064; host_runtime 88.65 and the new ironclaw_sandbox 87.09 from this PR). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reconciles this lane with #7064, which landed the parallel WS3/WS4 runner sheds and edited the same coordination files. One conflict: `WS0_LAYER_MATRIX_EXCEPTION_BASELINE` in `reborn_dependency_boundaries.rs`. The array itself auto-merged to the union of both sides' removals — #7064's three (`hooks -> wasm_limiter`, `runner -> agent_loop`, `runner -> loop_host`) and this PR's one (`host_runtime -> skills`). Recomputed the constant as `len()` of the merged list: main is at 10, so this slice takes it to **9**. Counted by parsing only the entries between the const and its closing `];`, which excludes the struct definition and the four test fixtures. This slice was authored off 13 and computed `13 -> 12` in isolation; the ratchet narrative and the two doc rows that quoted that figure (PLAN's "First-party tools" bullet and CHECKLIST's W7-progress row) now read `10 -> 9` and record why, per the union rule on the CHECKLIST §11.2.2 row. The edge deleted is unchanged; only the total moved. Everything else auto-merged and was verified rather than assumed: both sides' `coverage-floor.toml` recaptures are intact (runner 82.53 and the new `ironclaw_loop_host` 90.89 from #7064; `host_runtime` 88.59 from this PR), and both sides' dated amendments survive in CHECKLIST, PROPOSAL and PLAN. All 14 PR-only files are byte-identical to the pre-merge tip, so no code move was altered. Verified: architecture suite 206 passed / 0 failed, `cargo check --all-targets` clean, `cargo fmt` a no-op, zero conflict markers, and the changed-coverage manifest validates with no exemption stranded or shifted by this merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`Detect Reborn test scope` exits 1 on any pull request whose diff holds a path `reborn_pr_test_plan.py` has no rule for, which made this PR unmergeable: it must edit `Dockerfile` (the moved directory's `COPY wit/ wit/` no longer resolves) and `scripts/check-version-bumps.sh` (the ABI gate would otherwise grep dead paths and silently stop enforcing). 18 of its 46 paths were unclassified. Same class as the `.claude/` gap #7064 fixed, and classified the same way — one rule per class, recorded beside the constant: * `Dockerfile` / `.dockerignore` — `platform-and-compat.yml` keys `has_docker_risk` off exactly this pair and owns the image build. * `.githooks/**` — Code Style triggers on the tree and lints its contents (`test-ci-comm-locale-pin.sh`); no Reborn lane runs a hook. * `scripts/{build-wasm-extensions,check-version-bumps}.sh` — `platform-and-compat.yml`'s `has_direct_wasm_abi_risk` classifier both scopes and runs them. * markdown owned by no crate (`crates/AGENTS.md`, `test-tools/README.md`) — prose, like `docs/` and `.claude/`. A crate-resident doc still selects its own crate's lane. The first-party extension package assets are deliberately NOT ignored. `crates/extensions/packages/*/wasm/*.wasm` is a shipped artifact that `ironclaw_extension_support` embeds with `include_bytes!`, and `test-tools/*/manifest.toml` is `include_str!`d by `ironclaw_extension_host`. Calling either prose would convert today's loud failure into a silent under-schedule of a change to production output — the WS10 failure mode. `EMBEDDED_ASSET_OWNERS` routes each tree to the crate that compiles it instead, so this PR now additionally schedules `ironclaw_extension_{support,host,manager}`: the crates that consume the six rebuilt WASM artifacts. Also fixes #7085 in a file this PR already touches. The WIT version extractors used the GNU-only BRE `\+`, so on BSD sed (macOS) they matched nothing, and because the `WIT_TOOL_VERSION` cross-check is guarded on a non-empty version the hook printed "All version checks passed" having compared nothing. `[[:space:]][[:space:]]*` is identical under GNU sed, so the enforced Linux CI lane is unchanged; verified on BSD sed that both `wit/tool.wit` (0.3.0) and `wit/channel.wit` (0.3.1) now extract. Regression tests: every classified class gets a case in `test_reborn_pr_test_plan.py`, including the paired assertion that the embedded assets *select a lane* rather than merely being accepted (the inverse of the `.claude/` prose test), and a staleness pin that fails if an asset tree or its owning crate moves. All ten new cases fail against the planner on `main`. `test_unclassified_build_input_fails_fast` moves off `Dockerfile` onto a still-undecided input so the fail-closed arm stays exercised. Refs #7087, #7085 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two review findings, both correct, both artifacts of this PR's own renames.
1. engine-v2-to-reborn-parity.md note 4 read "a native script/software
execution lane (`ironclaw_sandbox`, `RuntimeKind::Script`) sandboxed via
`ironclaw_sandbox`" -- self-referential after the merge collapsed
ironclaw_scripts and ironclaw_process_sandbox into one crate, and it
contradicts note 5 four paragraphs down ("no production execution backend
is wired for it"). Re-stated as the typed runtime contract it is, citing
the measurement: `with_script_runtime` has zero production callers
(`rg` finds only the builder itself, docs, and 30 test call sites).
2. CHECKLIST WS10 ratchet note 2 said "raise the percentage floor ...; only
the line count should fall". That generalises WS3's sandbox merge, where
observed coverage happened to rise. It is wrong as guidance for WS7, and
the counterexample is in this same file: the 2026-08-03 entry from #7064
records ironclaw_runner falling 85.55% -> 82.53% because the shed removed
the crate's better-covered half, holding the floor, and RATCHET FAILing in
the merge queue. Note 2 now says re-capture from the merged artifact, and
lower only with that entry's move-not-regression counterfactual (add the
moved files back, confirm the union clears the old floor, plus a zero-tests-
lost name set-diff).
cargo test -p ironclaw_architecture: 32 targets, 206 passed, 0 failed
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Wave 3, WS3 runner sheds + the WS4 re-layer. Move-only plus two
layer =lines — no behaviour change, and the un-masking below shows zero tests lost or edited.What moved, and the measurement behind each call
Model gateway + port adapters →
ironclaw_loop_host(DONE)PROPOSAL §6.7.2's "Gains: runner's model-gateway adapter (a host-port adapter by charter) and runner's port adapters, so the port-implementer census shrinks", executed.
ironclaw_runner/srcironclaw_loop_host/srcmodel_gateway.rs(+model_gateway/prompt_cache_activity.rs)model_gateway_error_mapping.rs,model_routes.rsloop_driver_host/model_gateway.rsthread_resolving_model_gateway.rsloop_driver_host/port_adapters.rs(+ its tests)driver_host_port_adapters.rs(+ tests)tests/llm_gateway.rs,tests/model_routes.rsTwo dispositions the row did not predict, both forced by the call graph rather than the filename prefix:
model_routes.rshad to travel and is not optional. It reads as separable route policy vocabulary with its own runner and composition consumers — butmodel_gateway.rsnames eight of its types, so leaving it behind makesloop_host → runnera cycle against the pre-existingrunner → loop_hostedge.model_failure_mapping.rsmust NOT travel, though its name puts it in the cluster. Its only callers areplanned_driver.rsandtext_loop_driver.rs, which stay, and its test needs runner-privateretry_disposition. Moving it would create a cross-crate call in the wrong direction for no benefit.Also moved:
turn_error_to_host_error(theTurnError → AgentLoopHostErrormapper the checkpoint port calls — every arm isironclaw_turns/loop_contractsvocabulary, no runner concept in it).Tool disclosure →
ironclaw_loop_host(DONE; the/producthalf is refuted)tool_disclosure.rs,tool_disclosure_port.rs,context_shadow.rs, andToolDisclosureMode/REBORN_TOOL_DISCLOSURE_ENV(lifted out ofruntime.rsintotool_disclosure_mode.rs).tool_disclosure_bridge.rs— a 13-line one-function delegate that existed only because the catalog was private — is deleted, its test folded intotool_disclosure.rsverbatim.Zero new dependencies. Every crate the cluster needs was already a loop_host dependency; its
ironclaw_loop_host::imports simply becamecrate::. It belongs there by charter: it is aLoopCapabilityPortdecorator, andfamilies/loop.mdgives loop_host "the base capability-port adapter and its capability-surface-filtering decorators".§6.7.3's "→ loop_host / product prompt policy" cannot mean product, measured. ~160 of the cluster's 2,314 production lines are prompt content (the catalog-index prose, the three bridge tool descriptions,
CORE_TOOL_NAMES, the describe-first note). Moving them toironclaw_productneedsloop_host → product;loops → productsis upward and illegal. It would take an injection seam (composition supplying product-owned policy data) — a design change, not a move — and it is unnecessary: unlike a contracts crate (§6.1.4), nothing forbids prompt content in the loop tier, andcrates/ironclaw_loop_host/prompts/already holds fiveinclude_str!assets. Read the clause as "→ loop_host". PROPOSAL and CHECKLIST carry dated amendments quoting the text they replace.What stayed, and why
runtime.rs's decorator-chain composition,planned_driver*,text_loop_driver,driver_registry,loop_exit_applier,turn_scheduler,turn_run_executor,subagent/**,failure_categories,model_failure_mapping.families/loop.mdassigns the ordering of the capability-port decorator chain to the turn runner; this PR moves the pieces, not the ordering.Manifest result
ironclaw_runnershedsironclaw_llm,ironclaw_common,base64,jsonschema— the provider (reqwest/rig-core/Bedrock) cone is out of the turn runner entirely.ironclaw_loop_hostgainsironclaw_llm(default-features = false),ironclaw_common,base64, plushttp/reqwestdev-deps for the moved test targets.ironclaw_runnersrc: 33,209 → 22,039.ironclaw_loop_hostsrc: 38,496 → 49,727.Exception delta: 13 → 10, with per-edge evidence
ironclaw_runner → ironclaw_agent_looplayer = "loops";loops → loopsis matrix-legalironclaw_runner → ironclaw_loop_hostironclaw_hooks → ironclaw_wasm_limiterlayer = "loops";loops → runtimesis matrix-legalWS0_LAYER_MATRIX_EXCEPTION_BASELINE13 → 10 in the same change.The re-layer row read as if it were gated on the sheds; measured, it was not. Both moves are strictly permissive (
kernel's allowed set ⊂loops's;substrates's ⊂loops's), so they can only break consumers — and the complete consumer sets areironclaw_reborn_composition(app) plus each other. The PROPOSAL preconditions were already met onmain: #6696's supervisor inversion (§6.7.3) and WS1.2'sloop_contractsdependency (§6.7.4). It is twolayer =lines. This is the first exception movement since WS0; the register only responds to a crate changing layer, which is why Wave 2 moved it zero times.Enforcement —
crates/ironclaw_architecture/tests/reborn_runner_sheds.rs(8 tests)Four halves, each failing for its own reason:
cargo metadata, not literal paths, so the WS7 family move cannot blind it. Both directions asserted, with a non-vacuity control proving the reader found the right package.loopsdeclarations are pinned — reverting one would need three deleted exceptions back, and that register is shrink-only, so the revert must fail at the declaration rather than as an undeclared-edge message three crates away.Discipline: production-only via
ratchet_support::production_rust_files(subtracts#[cfg(test)] modchains, not just test-shaped filenames); comments/strings stripped before#[cfg(test)]; every I/O error fatal; non-vacuity asserted with a positive control. Self-tested over 9 definition shapes including theModelRouteError/ModelRouteErrorKindprefix trap, and the ordering guard proves the naive order loses production code rather than merely documenting it.Negative probes (each restored afterwards, tree byte-clean):
LlmProviderModelGatewayin the runner:361(left-behind) +:495(residue)ironclaw_llmto the runner manifest:440(manifest):495(residue)ironclaw_hookstolayer = "substrates":544(layer)Gates updated, never relaxed
reborn_dependency_boundaries.rs:reborn_runner_llm_wiring_is_isolatedrepointed and strengthened (it now also asserts the file is gone from the runner); thedefault-features = falseallowlist follows the edge to loop_host; threeprovider_tool_namesallowlist paths repointed; two deadpub useentries removed from the runner surface lock.reborn_extension_specificity.rs: 5 rows repointed — count unchanged, so the shrink-only baseline is untouched.reborn_struct_test_support_ratchet.rs: oneFROZEN_PATH_COUNTSrow repointed.scripts/no_panics_reborn_baseline.txt: 2 entries repointed (scanner re-run: OK, 1189 files / 51 invariants).scripts/reborn-e2e-rust.sh: exact selector feat: Sandbox jobs #4-p ironclaw_runner→-p ironclaw_loop_host— all 10 selectors verified to match exactly one test underbash … < /dev/null.tests/integration/coverage-floor.toml:ironclaw_runner's absolute numerator is now structurally unreachable (~10.5k lines and two test targets left), so it drops to a percentage floor for one release withfloor_percentretained unchanged, not weakened — the exact resolution WS1.2 used for theironclaw_turnssplit. A[[crate]]entry forironclaw_loop_hostis added in the same change so the moved lines keep a ratchet — the gap WS1.2 left open forironclaw_loop_contractsand did not close. Both numerators are owed from this PR's own merged artifact, per the file's same-PR workflow.code_style.yml+ws12_workflow_contracts.py:has_reborn_cliwidened toironclaw_loop_host. Without it, the move would have narrowed CI scope as a side effect — a PR touching only the new home would no longer light the lane that runscargo test -p ironclaw_architecture reborn.Registration traps verified empirically, not assumed:
classify-test-scope.shreportshas_reborn_tests=truefor files in all three crates and for the new arch test;reborn-crate-test-buckets.shstill mapsironclaw_loop_host→agent-runtime,ironclaw_runner→reborn-core,ironclaw_architecture→architecture-misc;discover-reborn-package-crates.shstill carriesironclaw_loop_hostthrough the binary's cargo-tree closure. The new arch test is namedreborn_*(file and every test fn) socode_style.yml's name-filtered lane picks it up.Un-masking
ironclaw_runnerport_adapter_tests::→thread_scope_tests::, same names)ironclaw_loop_hostironclaw_reborn_composition1030 → 1030. Zero unaccounted, zero lost, zero new tests smuggled in. The only content edit any moved test carries is one self-referential path literal in
tool_disclosure_mode_non_unicode_env_fails_closed, which re-runs itself by module path; it is called out in a comment at the line.Deferred, with measurements — the two clauses this PR did not execute
runtime.rsbuild_*→ composition. Payload is ~465 production lines. Executing it as written costs elevenpub(crate)→pubwidenings inironclaw_runner, four of which this PR already dissolved by moving tool disclosure out, leaving seven:subagent::capability_surface(module + resolver +::new),loop_driver_host::{capability_resolve_error_to_agent_host_error, apply_capability_surface_profile, with_resolved_profiled_capability_port_factory},SchedulerWakeWiring::start. That grows the public API of the crate the row exists to narrow. It also relocatesRuntimeProfiledCapabilityPortFactoryandSubagentSpawnCapabilityDecorator— the ordering of the capability-port decorator chain — into anapp-layer crate, contradictingfamilies/loop.md. The design that resolves both is one runner-ownedpub fn build_profiled_capability_port_factory(...), collapsing seven widenings to one — a semantic change PLAN principle 2 keeps out of a move PR. Worth noting for whoever takes it:build_product_live_planned_runtimehas zero production callers, so part of this row is a test-seam question.production_readinessdeletion. "No production caller" re-verified: onlytests/production_readiness.rsand fiveproduction_readiness_*tests intests/driver_registry.rs(composition's same-named test-support module is unrelated — it wrapsironclaw_host_runtime::ProductionWiringReport). But it cascades:DriverReadinessMode,HostGraphReadiness,DriverReadinessDiagnosticCode,ConfiguredRunProfile,PersistedRunDriverIdentityhave no other consumer, so the deletion propagates into a 589-line file that otherwise stays. That is an un-masking slice under PLAN principle 4. WS8's row now carries the cascade inventory.Row corrections landed in the docs
[~]with per-clause status and both refutations.[~]; the "absorb runner's model-gateway/port adapters" half is done.llm::reasoningplacement question answered and closed:clean_response/contains_codex_text_tool_call_syntax/recover_codex_text_tool_calls_from_tool_namesstay inironclaw_llm. Their sole caller moved to loop_host, so the coupling that made the placement ambiguous (akernelcrate reaching asubstratesprovider crate) is now an ordinaryloops → substratesedge.24-with_*figure was 28; the model-gateway and tool-disclosure lines are retired).first_party_extensions, a crate refactor(extensions): colocate packages under crates/extensions/ (WS2) #7037 renamed toextension_support. Now 10, with the current names.Verification
fmt clean · per-crate clippy
-D warnings --all-targets --all-featuresclean on all six touched crates · per-crate unfiltered tests: loop_host 771, runner 259, hooks 344, composition 928, product 1032, architecture 196 — 0 failures ·cargo test -p ironclaw_architecture32/32 binaries green · workspacecargo check --all-targets --all-featuresclean · per-cratecargo build -p <crate>alone for the three re-layered/changed crates ·cargo metadata --lockedclean · composition budget + include-str + panic-baseline + ws12-contract + classify-test-scope + crate-bucket self-tests all green.🤖 Generated with Claude Code
CI status (kept current — detail in the comments)
At head
dc190de227: Code Style ✅ · Platform & Compat ✅ · Reborn E2E ✅ · IronClaw Stress ✅ · Regression Test Check ✅ · PR: History Check ✅ · Tests (Reborn) ❌.The single red lane is #7066's
llvm-covenvironment defect, reproduced on this tree and not caused by this PR:run-hermetic-test-process.shstripsREBORN_COV_COLLECT, whichreborn-coverage-lane-run.shthen defaults back totrue, so a lane with coverage off calls acargo llvm-covthat was correctly never installed. This PR needs amainmerge once #7066 lands; its one-token fix is deliberately not duplicated here.Local evidence that the lanes themselves are fine: both failing lanes' complete suite lists, computed with the workflow's own modulo-4 partitioning — 27 suites, 461 passed, 4 failed, and all four failures carry the explicit "StorageMode::Postgres requires a reachable Docker daemon" gate.
reborn_integration_tool_disclosure(in failing lane 1, the suite that most directly exercises the moved cluster) is 26/26.Three CI-plumbing commits ride along, all fixing pre-existing defects this PR was the first to hit — two in the Reborn PR test planner (
.claude/and repo-rootscripts/were unclassified, so its fail-closed arm skipped the entire Reborn suite on a docs-only path) and one worked around incheck_no_panics.py(flat-only#[path]recognition). Both planner gaps are fixed with regression tests verified red by reverting; the panic-scanner gap is reported on CHECKLIST WS10 rather than fixed, because widening that regex touches a security-adjacent gate's classification and has panic-baseline implications.One evidence gap, stated plainly:
Rust ReborninReborn E2Ewas skipped by scope, so the repointedllm_gatewayselector has not run in CI. All 10 selectors were verified locally underbash … < /dev/null, each matching exactly one test.Merge-queue rejection and the floor recapture (
b75bdacc10)This PR jammed the merge queue and took #7040 down with it. Run 30855460733, job Reborn integration-tier coverage report:
Why the PR page said 30/30 green:
scripts/ci/reborn_pr_test_plan.py:21'sFULL_EVENTSomitspull_request, so PR runs getcoverage_mode: "none"and the ratchet never executes on a PR.merge_groupruns it for the first time. Tracked as #7036; not fixed here (CI policy frozen).The held floor was an unverified prediction, and it was wrong
This entry held
floor_percent = 85.55across the shed on the reasoning — now deleted from the file — that "the moved half is adapter code with roughly the crate's own coverage profile, so the ratio is the invariant that survives a split". Measured, the moved files score 6,108/6,577 = 92.87% at their new home — 10.33pp above the 82.53% of what stayed. The shed removed the crate's better-covered half and un-masked a weaker remainder that had been riding on it.Move, not regression — proven, not asserted
origin/mainvs branchrequest_shaped_errors_map_to_invalid_invocation,unauthorized_maps_to_unauthorized), zero names lostHad the moved code stayed, the runner would have passed. That is the definition of a numerator/denominator shift.
Both entries recaptured from this PR's own merged artifact
Read from run 30855460733's
reborn-integration-coverage-mergedthrough the gate's ownaggregate()(scripts/ci/lib/reborn_coverage_lcov.py) — never estimated:ironclaw_runnerironclaw_loop_hostThe destination is raised, not left inheriting the source's 85.55 — that would have handed the absorbed code 5.34pp of arrival slack (~1.4k covered lines it could have lost silently). This is the WS2.4
extension_host/extension_managerpattern: both halves captured in the same PR, so no release runs on a half-captured entry.Verification — the exact failing artifact, replayed
The ratchet does not run on
pull_request, so this PR's own checks cannot prove the fix — confirmed directly on this PR: theReborn integration-tier coverage reportcheck isskipping(run 30857632335, job 91836054560), even though this PR now editscoverage-floor.toml. Editing a coverage floor does not escalate a PR to the full plan:coverage-floor.tomlis in the planner'sPR_STATIC_CONTROL_PATHS(reborn_pr_test_plan.py:43), whose branch at:353de-escalates ("static CI or workspace-policy checks own"). Measured,coverage-floor.toml,coverage-exemptions.tomlandchanged-coverage-exemptions.tomleach yieldcoverage_mode: noneon--event pull_request. Real signal instead:Local replay of the exact CI input. Downloaded run 30855460733's
reborn-integration-coverage-mergedartifact (unexpired, single file, md5efc5ccbf1502f8277dd50008120c7379) and ranscripts/ci/reborn-coverage-ratchet.shover it:RATCHET FAIL: ironclaw_runner, 82.53%, 9470/11474) — confirming the harness is faithful;RATCHET FAIL, all 19 crates + global PASS.Cross-check against the stacked variant. refactor(contracts): close the WS2 strays and WS2.1 follow-ups #7040-on-refactor(loop): shed the model gateway and tool disclosure into loop_host (WS3/WS4) #7064 (run 30855963015) measured 82.55% (9472/11474) — a 2-line wobble. Against the new floors: 82.55% ≥ 82.03% effective, 9472 ≥ 9450 effective. Passes with the tolerance ~10× the observed wobble.
Full
workflow_dispatchrun — CONFIRMED GREEN. Run 30857389891 at headb75bdacc10(workflow_dispatch∈FULL_EVENTS, so the full plan including the coverage ratchet ran). Per-job tally: 28 success, 1 skipped, 0 failures — and the job that rejected this PR in the queue, Reborn integration-tier coverage report (job 91834469469), issuccesswith zeroRATCHET FAIL:Note this run measured 9470/11474 and 24598/27063 — byte-identical to the queue artifact the floors were captured from, on a fresh run against a different base. The capture is stable, not a fluke. (The dispatch roll-up also came back
successhere; reborn-tests.yml: workflow_dispatch runs structurally fail the Tests (Reborn) roll-up (critical-mutation skipped but disallowed) #6978's structural-red caveat did not bite, but the per-job tally above is the signal either way.)cargo fmtclean;cargo test -p ironclaw_architecture --test reborn_restructure_baselinesgreen (it parses this file); no stranded entry inchanged-coverage-exemptions.toml(every path resolves on HEAD — a stranded one makes the fail-closed validator abort with no verdict).#7040's exposure: none of its own
#7040 touches
ironclaw_extension_host,ironclaw_extension_manager,ironclaw_product,ironclaw_product_contracts,ironclaw_reborn_composition,ironclaw_webui. Its queue entry on the un-stacked base (run 30856287488) was green; stacked behind this PR it inherited the identicalRATCHET FAIL: ironclaw_runner. It was pure collateral — no change needed there.Rule recorded
docs/reborn/target-architecture/CHECKLIST.md(WS10 ratchet row): a shed/move re-captures the SOURCE crate's floor, not just the destination's — a held source floor is not conservative, it is an unverified prediction about a ratio, and the merge queue is where it gets checked. The note carries the counterfactual recipe so the next shed proves move-vs-regression before touching a number.