Skip to content

refactor(contracts): invert extension_host's product-facing ports onto product_contracts (WS2.1) - #6998

Merged
BenKurrek merged 11 commits into
mainfrom
ws2/extension-host-ports
Aug 1, 2026
Merged

BenKurrek merged 11 commits into
mainfrom
ws2/extension-host-ports

Conversation

@BenKurrek

@BenKurrek BenKurrek commented Aug 1, 2026 •

Copy link
Copy Markdown
Collaborator

Wave 2, slot 1 — CHECKLIST WS2 row 1. ironclaw_extension_host now implements ironclaw_product_contracts port definitions instead of ironclaw_product ones. Behavior-free: definitions move, every implementation stays with its owner (PROPOSAL §6.1.4).

This is the ordering-constrained half of §12.1c — port inversions land before the layer flip. The products → loops move for extension_host is a later WS2 slice and is deliberately not here.


Port inventory — each product dependency → the contracts port that replaced it → where the impl lives

Eleven port declarations moved across ten new modules (ironclaw_product_contracts 7 → 17). Nine are implemented by ironclaw_extension_host — those are the set the new architecture test pins as INVERTED_PORTS. The other two (AdminUserService, RebornOperatorToolCatalog) extension_host only consumes; their implementations are in ironclaw_reborn_composition, so they moved for the same reason — a port whose implementation sits outside product does not belong inside it.

Was (ironclaw_product) Now (ironclaw_product_contracts::) Implementation stayed in
delivery_coordinator::{ChannelDeliveryResolver, ResolvedChannelDelivery, DeliveryReplyContextSource} delivery extension_host (channel_delivery.rs, channel_dm_provisioning.rs); NoReplyContext + DeliveryCoordinator stay in product
extension_account_setup::{AccountConnectionStatusSource, AccountConnectionStatusError, ChannelConnectionNoticePolicy, ExtensionAccountSetupDescriptor, ExtensionAccountSetupError} account_setup extension_host::channel_pairing; ExtensionAccountSetupRegistry (mutable state) stays in product
reborn_services::ChannelConfigProductService channel_config extension_host::channel_config
reborn_services::views::{RebornViewProvider, RebornViewDescriptor, RebornViewQuery, RebornViewPage} views extension_host::admin_configuration + product's own providers; ProductView + UnavailableRebornViewProvider stay in product
command_admission::CommandActorRoleResolver + command_dispatch::ProductCommandContext command extension_host::channel_command_roles; DirectConversationCommandAdmission stays in product
action::{ProductActionId, ActionFingerprintKey, SourceBindingKey, ProductCommandName, AuthRequestRef, LinkedThreadActionId} action n/a (DTOs; the ProductInboundAction ledger record + saga stay in product)
run_delivery::{ApprovalPromptContextSource, BlockedAuthPromptSource} + auth_prompt::BlockedAuthPromptRequest prompt_source extension_host::run_delivery_ports; auth_prompt_view_for_blocked_auth (the renderer) stays in product
lifecycle::{LifecycleProductService, LifecycleProductContext, LifecycleProductSurfaceContext} lifecycle_service extension_host::lifecycle_product_service; UnsupportedLifecycleProductService stays in product
reborn_services::admin_users::{AdminUserService, AdminUserRecord/Role/Status/Error/SecretMeta, AdminCreateUserFields, AdminCreatedUser, ADMIN_USER_LIST_*} admin_users ironclaw_reborn_composition; RejectingAdminUserService + the RebornAdmin* wire DTOs stay in product
reborn_services::{RebornOperatorToolCatalog, RebornOperatorToolInfo} operator_tools ironclaw_reborn_composition

Alongside, the product re-export facade dissolved for extension_host: symbols it reached through ironclaw_product that already lived in ironclaw_host_api / ironclaw_extension_contracts / ironclaw_product_contracts now import from their owner — the sweep WS1.4's location-scan module doc explicitly deferred to "WS2/WS6, which repoint those crates for their own reasons". Every other crate got only the moved names repointed, so webui / openai_compat / operator keep their own flips intact.

Measured effect: extension_host's ironclaw_product:: production usage 146 → 62 distinct symbols across 46 → 35 files.


Row corrections (re-verified against this PR's base a50ad0638)

1. The wave milestone's "extension_host → product edge dies" is not reachable in this row — structurally, not as a shortfall

All 62 survivors are owned by later WS2 rows, and the mapping is exact:

Survivor class Files Owning WS2 row
Product's concrete assembly constructed inline (DefaultProductSurface, DefaultInboundTurnService, RebornFilesystemIdempotencyLedger, StaticProductInstallationResolver, DirectConversationCommandAdmission::new, ProductConversationBindingService::new, DeliveryCoordinator, RunDeliveryServices/Settings/Observer) channel_host.rs (26 symbols) not yet assigned — see the owner call below
extension_manager split inventory (admin_configuration*, operator_config_capability, skill_auto_activate_capability, webui_extension_credentials, available_extensions, product_lifecycle, extension_lifecycle_*) 9 files "Split ironclaw_extension_manager out of extension_host"
product::adapter_registry::{PRODUCT_ADAPTER_HOST_API_ID, product_adapter_sections, register_product_adapter_host_api_contract} channel_lifecycle.rs, available_extensions.rs, host_api_contracts.rs §6.9.1's shed of adapter_registry manifest parsing → extension_contracts/extension_registry
Named strays (projection::LiveProjectionPublisher, TriggeredRunDeliveryDriver) skill_learning.rs, channel_triggered_delivery.rs "Relocate extension_host strays"
ProductSurfaceFailure (19 files) see finding 3 blocked — no row owns it yet

§12.1c's own wording is the correct reading ("port inversions land before the layer flip"); PLAN's per-row milestone compressed a wave-level outcome into one row. CHECKLIST WS2 row 1 is amended with this.

2. extension_host carries no LAYER_MATRIX_EXCEPTIONS entry, and never did — the count stays 13

The brief expected "any exception entries it carries fall (the count on main is 13)". Every one of the 13 was re-read at this base: none names extension_host, product, or product_contracts. The mechanism explains why — both crates declare layer = "products", and layer_allows_dependency permits products → products, so this edge is legal by the matrix and invisible to it. That is exactly why it needed a purpose-built gate (below).

3. Six ports could not move, all for one mechanical reason — and ProductSurfaceFailure is the linchpin

product_contracts' enforced allowlist is ironclaw_host_api + ironclaw_extension_contracts and nothing else internal. A port whose signature names a type from ironclaw_auth, ironclaw_threads, ironclaw_turns, or ironclaw_conversations therefore cannot be declared there:

Port Blocking type
AuthChallengeProvider returns Result<_, ironclaw_auth::AuthProductError>; carries AuthProviderId, CredentialAccountLabel, OAuthAuthorizationUrl
ChannelConnectionService returns ChannelAuthAccountState { ironclaw_auth::AuthFlowStatus, CredentialAccountStatus }
ExtensionCredentialSetupService request/response are ironclaw_auth credential-account projections
ConversationBindingService errors with ProductSurfaceFailure
ProductActorUserResolver errors with ProductSurfaceFailure; resolves to ResolvedProductActorUser { ironclaw_conversations::ExternalActorBindingEpoch }
ProductConversationSubjectRouteResolver errors with ProductSurfaceFailure

The durable finding. ProductSurfaceFailure documents itself as "the internal error type used within the workflow crate", yet ironclaw_extension_host uses it as its own lifecycle error vocabulary across 19 production files, constructing InvalidBindingRequest / Transient / ProviderInstanceNotConfigured directly. It cannot follow the ports into contracts because two variants carry ironclaw_turns::TurnError (crates/ironclaw_product/src/error.rs:10,90,112). Until it is narrowed off ironclaw_turns or replaced by an extension-host-owned error, it blocks half the residue and is the largest single remaining term in the extension_host → product edge. Recommend a dedicated WS2 slice before the layer flip.

4. ProductCommandAdmissionService is a §6.1.3-vs-§6.9.1 conflict — recorded, not decided

§6.1.3 assigns "the ProductCommandAdmissionService shape" to product_contracts; its admit takes &ProductCommand, and §6.9.1 keeps the command grammar with product's frozen inventory. Neither this row nor WS1.4 had standing to choose. Left in ironclaw_product; recorded in ironclaw_product_contracts/CLAUDE.md.

5. Docs-truth correction (families/contracts.md)

The entry and PROPOSAL §6.1.3 both list ironclaw_common among product_contracts' dependencies. The crate has never held that edge, and the enforced allowlist (product_contracts_allowed) is the two crates it does hold. Amended in place with the date and the evidence.

Follow-up recorded from review (deliberately not in a move-shaped PR)

Three signatures this PR relocated verbatim are stringly-typed and should not stay that way. Retyping any of them changes the contract for its implementors, and PLAN's operating principle 2 keeps semantic changes out of move PRs — so each is recorded here with a named home: the same slice that has to narrow ProductSurfaceFailure off ironclaw_turns is already reopening these signatures.

  • prompt_source::BlockedAuthPromptRequest.gate_ref: &str → &TurnGateRef. The inconsistency is visible in one file: the sibling ApprovalPromptContextSource::approval_prompt_context already takes &TurnGateRef.
  • lifecycle_service::LifecycleProductService::installed_activation_errors → HashMap<ExtensionId, String>. The doc says "keyed by extension id"; the host implementation already holds typed ExtensionId and stringifies to satisfy the signature.
  • delivery::ResolvedChannelDelivery.{extension_id, installation_id}: String → distinct newtypes, so an identity mixup fails to compile.

A fourth, also verbatim and also recorded rather than fixed: LifecycleProductService::import_extension_bundle's default says "unavailable" but returns InvalidRequest/400 — different code, different meaning, and 400 blames the caller for an unwired capability. Changing it changes an HTTP status on a live WebUI route. The doc comment now describes what the code does and names the discrepancy, and the test comment says it pins today's behavior, so the flip cannot happen silently.

Owner call needed (skipped, not stalled)

crates/ironclaw_extension_host/src/channel_host.rs is composition-shaped assembly living in extension_host, and no WS2 row owns it. It constructs DefaultProductSurface, DefaultInboundTurnService, RebornFilesystemIdempotencyLedger, StaticProductInstallationResolver, DirectConversationCommandAdmission, and ProductConversationBindingService — product's concrete types, which by construction cannot be inverted into contracts. It alone is 26 of the 62 surviving symbols. Either it moves to composition, or the assembly inverts behind a factory port, or the layer flip cannot happen. §6.8.2's shed list does not name it. This needs the owner before the WS2 sequence reaches the flip.


New enforcement — crates/ironclaw_architecture/tests/reborn_extension_host_port_inversion.rs

The layer matrix cannot see this edge (finding 2), so the row gets its own gate. Three tests:

  1. extension_host_implements_only_the_frozen_residue_of_product_defined_traits — discovers every trait declared in ironclaw_product, scans extension_host's production source (#[cfg(test)] blocks and *_tests.rs stripped, same shape as reborn_registration_pipeline_boundary.rs) for impl … for, and requires the intersection to equal the six-entry residue exactly. A new one fails ("define the port in product_contracts instead of adding a row here"); a stale one fails too, so the row is deleted in the change that removes the edge. Plus a <= 6 shrink-only baseline.
  2. inverted_ports_are_declared_in_contracts_and_implemented_below_product — the nine moved ports are declared in product_contracts, not re-declared in product, and still implemented in extension_host. Makes a revert loud.
  3. impl_scanner_reads_the_trait_out_of_real_impl_shapes — self-test over plain / generic / path-qualified / inherent / commented-out / #[cfg(test)] impl shapes, so the scan cannot go quietly vacuous.

Zero-match check: the scanner asserts it walked >20 files and that both crates yield a non-empty trait set, so a broken path fails loudly rather than passing empty.

Enumerating gates touched (all update-never-relax)

Gate Change
docs/plans/composition-pubuse.snapshot 126 → 127. ChannelConnectionNoticePolicy + ExtensionAccountSetupDescriptor are re-sourced from product_contracts::account_setup and ChannelConnectionRequirement from package_lifecycle, so one four-name pub use splits into three. Same public name set — nothing added or removed.
reborn_extension_specificity.rs allowlist (128/130) untouched — no vendor term left or entered a listed file; the staleness half passes.
reborn_struct_test_support_ratchet.rs (79 paths / 276 members) untouched — the move carried no #[cfg(test)] struct members.
reborn_cross_crate_include_scan.rs (62 escaping / 19 cross-crate) untouched — swept every file I moved code out of for include_str!/include_bytes!; none carried one.
reborn_service_method_freeze_ratchet.rs untouched — ProductSurface still 3 methods; reborn_services.rs grows no local surface trait.
reborn_product_contract_location_scan.rs auto-enrolled — its one-import-path half discovers every trait product_contracts declares, so the nine new ports inherit the no-re-export rule with no edit. ironclaw_product re-exports none of them.
LAYER_MATRIX_EXCEPTIONS 13, unchanged (finding 2).
classify-test-scope.sh / reborn-crate-test-buckets.sh no edit needed — all four touched crates already have arms/buckets; this diff lights product-workflow + extension-operator.

New dependency: secrecy = "0.10" on product_contracts, with a manifest comment stating why (AdminUserService::put_secret takes the material, AdminCreatedUser carries the one-time token — both SecretString, so the port cannot be implemented or called with a self-Debug-printing String). It is a value wrapper, not on the contracts framework/driver/runtime-client denylist.

Un-masking

Full unfiltered --list inventory of all four touched crates on origin/main@a50ad0638 vs this branch, compared name by name:

  • 1976 → 1977 tests. Zero removed. One added.
  • The two typed-token tests moved with their code into product_contracts::action under their exact original names (typed_tokens_reject_empty_oversized_and_control_values, product_action_id_round_trips_display_and_uuid) — no content edited, which is why they do not show as a rename in the diff.
  • The one addition is action::tests::action_fingerprint_key_carries_every_dedup_component: ActionFingerprintKey moved into a new module with no direct test of its own, so it got one rather than landing bare.
  • Execution: 1977 passed / 0 failed / 0 ignored across 48 test binaries.

Verification

  • cargo fmt; cargo clippy -p ironclaw_product_contracts -p ironclaw_product -p ironclaw_extension_host -p ironclaw_reborn_composition -p ironclaw_webui -p ironclaw_architecture --all-targets --all-features -- -D warnings → clean.
  • cargo test -p ironclaw_architecture → 130 passed / 0 failed (all 25 gate files, including the new one).
  • cargo check --workspace --all-targets --all-features → 0 errors, 0 warnings.
  • cargo metadata --locked clean; git status clean after every cargo run; the Cargo.lock delta is the single secrecy line.
  • All 10 exact-test selectors in scripts/reborn-e2e-rust.sh executed under bash with < /dev/null; each matched exactly one test.
  • critical_mutation_gate.py --selection-only → true, so the full run was executed locally (cargo-mutants 27.1.0, the CI-pinned version). Both selected entries PASS: ironclaw_extension_host/src/channel_outbound_targets.rs (delivery routing registration) 1 caught / 0 survived, and ironclaw_product/src/run_delivery/observer.rs (delivery deduplication) 1 caught / 1 unviable / 0 survived. Neither function body changed — only import paths above them.
  • Workspace clippy in the CI Code Style shape (cargo clippy --all --benches --tests --examples --all-features -- -D warnings) → clean, so the lanes I did not lint per-crate are covered too.

Changed-coverage — CI ran it, and every hole is now a test

The move-shaped diff made the gate read relocated bodies as added production lines, exactly as predicted. CI's first run failed it; here is the whole list and what each became. Ten tests, one exemption.

Five relocated port modules had no LCOV record at all (delivery, channel_config, operator_tools, prompt_source, views) — pure declarations emit no source record, so the gate reported them absent. Each now carries a contract test rather than a waiver, pinning what these ports actually owe:

Property Why it is a contract, not a line-count exercise
object safety (all 7 traits) every consumer holds them as Arc<dyn _>; a signature change that breaks dyn-safety now fails at the contract, not at the far-away wiring site
argument order and pass-through (reply_context) extension id / installation id / conversation fingerprint are three bare strings — nothing but this test stops a transposition becoming a silent mis-delivery. This is review's identity-mixup concern, pinned without changing the types
absence without error an unresolved channel, an empty channel-config field set, an empty tool catalog and a missing approval context are all normal outcomes; a port that could only express them as failures would be wrong
caller scoping the operator catalog's caller parameter is the #5459 disclosure control. Two callers, one shared tool and one private tool each, asserting both directions of isolation — a double that ignored caller fails the test (verified by mutation). Scope, stated plainly: this pins that the port hands the implementation the caller and that its shape admits a per-caller answer; it cannot pin that the production catalog filters, which is composition's implementation and composition's test
next_cursor omission serializing null would make every unpaginated view look paginated to the browser

Two genuinely untested fail-closed error paths in extension_host, both on seams this move touched. AccountConnectionStatusSource::connected now proves it fails closed on a pairing-backend outage (activation must not proceed on an unknown connection state) and sanitized — the test asserts the driver, host and port do not appear in the product-facing error. The lifecycle output-serialization mapping moved out of an inline closure into a named lifecycle_output_decode_error so it is reachable from a test: the failure is defensive, but what it maps to is live — the model gets OutputDecode and never the serde error, which can quote projection contents.

A dead branch arm. validate_typed_token guards c == '\0' || c.is_control() and only the second arm ran. NUL has its own arm because an embedded NUL truncates at a C boundary rather than merely looking odd.

Diff shape. The remaining reports were an artifact of relocating types inline: a fully-qualified ironclaw_product_contracts::<mod>::<Item> in a signature turns an untouched line into a changed one. Seventeen files now import the symbol like every other — smaller diff, prevailing style restored, and the lines leave the gate's denominator because a use line is uninstrumentable by construction.

Exemptions: eleven lines, every one a place a test cannot reach. Nine are type positions — a struct field, a function parameter, a struct-literal field's enum path — where deleting ironclaw_product's re-export rewrote the line but LLVM emits no coverage region for it, so it can never be covered. Each entry names the exact construct; all nine were re-read against the source before the entry was written. The tenth is factory/test_support.rs:330, whose two integration callers are cited by file:line — the merged lcov does not attribute them back to the composition bucket build. The eleventh is a tracing::debug! message literal, and it is worth reading the entry: the event body demonstrably does execute — the DEBUG-subscriber test asserts the rendered log contains that exact message and passes — but tracing bakes the message into the callsite's static Metadata, so LLVM attributes the line's region to a static initializer and never counts it. The bucket's own tracefile shows lines 213/214/219/220 at 1 hit each with 217 at 0, which is what distinguishes "artifact" from "dead path". Every tracing::debug! in the workspace has this shape; they only escape the gate because their lines are not in a diff. All eleven are filed under the same #6963 lane-attribution lane as the four WS1 entries.

Verified, not hoped: the gate was replayed locally against CI's own merged lcov (reborn-integration-coverage-merged from run 30689416105) with these entries in place — changed line coverage 100.00% (147/147), changed branch coverage 100.00% (10/10).

What was not exempted is the point. CI's second run left one uncovered line: lifecycle_output_decode_error's tracing::debug! body, which never runs because tracing short-circuits on the null dispatcher with no subscriber installed. That is coverable, so it is covered — the test now installs a DEBUG-level subscriber over a shared writer (the pattern ironclaw_turns/tests/agent_loop_host_contract.rs established) and asserts both halves of the guard: the model gets OutputDecode and never the serde error, and the serde detail is not discarded — it reaches the debug log, which is where an operator diagnoses it. Without the subscriber a test cannot tell "logged it" from "dropped it", which is exactly why a waiver would have been the wrong answer. tracing-subscriber joins extension_host's dev-dependencies for it, with a manifest comment; the Cargo.lock delta is one line and the crate was already in the workspace.

Every one of these was verified red-then-green rather than assumed — each new double was mutated to drop the argument under test and the suite watched to fail. Result: ironclaw_product_contracts 73 → 94 tests, and a per-module llvm-cov replay of the crate-bucket lane's exact invocation shows zero uncovered added production lines across all ten relocated modules.

🤖 Generated with Claude Code

…o product_contracts (WS2.1)

`ironclaw_extension_host` sits below product in the target tree, so a
product-side port it satisfies must be declared at the product boundary and
implemented downward — never declared inside `ironclaw_product` and reached
upward. This moves every such port that `ironclaw_product_contracts` may
legally name, and dissolves the product re-export facade for the extension
host.

Nine port families move (definitions only; every implementation stays with its
owner, PROPOSAL §6.1.4): delivery resolution + reply context, account-connection
status + setup descriptors, channel config, the view-provider conduit, command
context + actor-role admission, gate-prompt enrichment, the lifecycle product
service, the admin-user directory, and the operator tool catalog. Product keeps
`DeliveryCoordinator`, `NoReplyContext`, `ExtensionAccountSetupRegistry`,
`UnsupportedLifecycleProductService`, `RejectingAdminUserService`,
`UnavailableRebornViewProvider`, `DirectConversationCommandAdmission`, the
frozen `Reborn*` wire DTOs, and the inbound-action ledger.

extension_host's product symbol usage drops 146 -> 62 across 46 -> 35
production files. The edge itself does not die here and could not: the
survivors are `channel_host.rs`'s construction of product's concrete assembly,
the `extension_manager` split inventory, `product::adapter_registry`, and the
named strays — each owned by a later WS2 row. Six ports also could not move,
all for one mechanical reason: `product_contracts` may depend only on
`host_api` + `extension_contracts`, so a signature naming `ironclaw_auth`,
`ironclaw_threads`, `ironclaw_turns`, or `ironclaw_conversations` cannot be
declared there. `ProductSurfaceFailure` is the linchpin — extension_host uses
product's *internal* workflow error as its own lifecycle error vocabulary in 19
files, and it carries `ironclaw_turns::TurnError`.

Regression cover: `reborn_extension_host_port_inversion.rs` pins the nine moved
ports where they landed and holds the six-entry residue shrink-only, with the
per-entry reason each could not move; a new product-declared port implemented
by extension_host fails the build. The moved typed-token tests travel with
their code and `ActionFingerprintKey` gains the coverage it lacked.

Enumerating gates, all update-never-relax: the composition pub-use snapshot
gains one line (two names re-sourced from `product_contracts`, so one `pub use`
splits into three); the extension-specificity allowlist, the struct/test-support
ratchet, the §11.2.7 include inventory, the `ProductSurface` method freeze, and
`LAYER_MATRIX_EXCEPTIONS` (13) are all untouched — extension_host carries no
layer-matrix exception and never did, since both crates are `products`-layer.

`secrecy` joins `product_contracts` with a manifest comment: `AdminUserService`
takes secret material and `AdminCreatedUser` carries a one-time token, both
`SecretString`. It is a value wrapper, not a framework/driver/runtime client.

CHECKLIST WS2 row 1 ticked with the four dispositions the lead sheet did not
predict.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@railway-app

railway-app Bot commented Aug 1, 2026 •

Copy link
Copy Markdown

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

Service Status Web Updated (UTC)
ironclaw 🕒 Building (View Logs) Web Aug 1, 2026 at 3:19 pm

@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6998 August 1, 2026 05:08 Destroyed
@github-actions github-actions Bot added size: XL 500+ changed lines scope: docs Documentation scope: dependencies Dependency updates risk: low Changes to docs, tests, or low-risk modules contributor: core 20+ merged PRs labels Aug 1, 2026
@coderabbitai

coderabbitai Bot commented Aug 1, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 5084f027-1e54-4f6a-b6dd-8599834cdf27

📥 Commits

Reviewing files that changed from the base of the PR and between ffc4b86 and b36c972.

📒 Files selected for processing (2)
  • crates/ironclaw_product_contracts/src/lifecycle_service.rs
  • docs/reborn/target-architecture/CHECKLIST.md

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added shared contracts for account setup, administration, lifecycle services, channel delivery, commands, prompts, operator tools, views, and action identity.
    • Added channel configuration management, caller-scoped operator tools, paginated views, and account connection status handling.
  • Bug Fixes

    • Improved protection of detailed lifecycle decoding errors while retaining diagnostic logging.
  • Tests

    • Expanded validation for contract behavior, serialization, security, input handling, and architecture boundaries.

Walkthrough

Changes

The PR moves product-side ports and DTOs into ironclaw_product_contracts. It updates product, extension-host, composition, WebUI, tests, documentation, and architecture enforcement. It also adds contract validation and lifecycle error-redaction coverage.

Product contract extraction and migration

Layer / File(s) Summary
Contract definitions and validation
crates/ironclaw_product_contracts/src/*, crates/ironclaw_product_contracts/Cargo.toml
Adds action, account setup, admin user, command, delivery, lifecycle, operator tool, prompt, and view contracts with focused tests.
Product boundary reduction
crates/ironclaw_product/src/*
Removes relocated declarations and narrows legacy public re-exports.
Extension-host and composition wiring
crates/ironclaw_extension_host/*, crates/ironclaw_reborn_composition/*
Updates implementations, trait objects, field types, and public re-exports to use contract modules.
Consumer and documentation updates
crates/ironclaw_webui/*, tests/*, docs/*, crates/AGENTS.md
Updates imports, fixtures, coverage exemptions, architecture records, and crate guidance.
Architecture enforcement
crates/ironclaw_architecture/tests/reborn_extension_host_port_inversion.rs
Scans production Rust code and checks contract placement, implementation coverage, parser behavior, and residue shrinkage.
Lifecycle error handling
crates/ironclaw_extension_host/src/extension_lifecycle_capabilities.rs, crates/ironclaw_extension_host/Cargo.toml
Maps serialization failures to OutputDecode, logs details at debug level, and tests error redaction.
Pairing status validation
crates/ironclaw_extension_host/src/channel_pairing.rs, crates/ironclaw_extension_host/src/channel_pairing/tests.rs
Uses the account-setup status contract and adds fail-closed tests for unavailable installations and unpaired callers.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers: ilblackdragon

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title follows Conventional Commits style and clearly describes the port inversion from product to product_contracts.
Description check ✅ Passed The description provides a detailed summary, scope, rationale, follow-ups, architecture details, and extensive validation evidence for the refactor.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

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

@ironloopai

ironloopai Bot commented Aug 1, 2026 •

Copy link
Copy Markdown
Contributor

🔎 Review · PR #6998

🟢 Completed · Review submitted

1 actionable findings →

The contract inversion preserves the moved DTOs, trait signatures, serialization shapes, authority context, secret wrappers, implementations, and consumer wiring across the complete trusted comparison. One non-blocking weakness remains in the new architecture ratchet: its hand-written impl parser can miss valid generic implementations.

Automatic · PR opened · attempt 1 of 3 · completed in 2m 39s

Run details
  • Repository: nearai/ironclaw
  • Base: main at a50ad06
  • Head: ws2/extension-host-ports at f4819bb
  • Created: Aug 1, 2026, 5:13 AM UTC
  • Updated: Aug 1, 2026, 5:15 AM UTC
  • Run: 35395aa6-f3bf-4bf7-9a3d-6a4b99b13d14
  • Latest attempt: 1 · Completed · 0cf25d09-6ffa-4436-9ec5-2cfd2fd85b21

@ironloopai ironloopai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Review complete · PR #6998

💬 1 finding

The contract inversion preserves the moved DTOs, trait signatures, serialization shapes, authority context, secret wrappers, implementations, and consumer wiring across the complete trusted comparison. One non-blocking weakness remains in the new architecture ratchet: its hand-written impl parser can miss valid generic implementations.

Findings

  1. 🟡 Low · Make the impl scanner handle nested generic bounds — crates/ironclaw_architecture/tests/reborn_extension_host_port_inversion.rs:229-234
    Details are attached to the relevant diff.
Validation and technical details
  • Reviewed the complete refs/ironloop/base (a50ad06) to refs/ironloop/head (f4819bb) comparison across all 130 changed files.
  • Compared relocated contract definitions directly against their base versions and traced updated consumers in product, extension_host, composition, WebUI, and integration tests.
  • git diff --check refs/ironloop/base..refs/ironloop/head completed without errors; the worktree was clean and the checkout merge tree had no delta from the trusted head.
  • Attempted targeted tests for product_contracts, product, extension_host, and architecture, but this review environment does not provide cargo (cargo: command not found), so runtime validation was limited to static inspection.
  • Base: main
  • Head: ws2/extension-host-ports at f4819bb
  • Run: 35395aa6-f3bf-4bf7-9a3d-6a4b99b13d14

Comment on lines +229 to +234
candidate = candidate[close + 1..].trim();
}
// Drop generic arguments on the trait itself, then the path qualifier.
let candidate = candidate.split('<').next().unwrap_or(candidate).trim();
let Some(last) = candidate.rsplit("::").next() else {
continue;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Low · Make the impl scanner handle nested generic bounds

implemented_trait_names removes the impl's generic parameter list by taking the first >. For valid declarations containing nested generics, such as impl<T: Iterator<Item = X>> NewProductPort for Host<T>, that delimiter closes Iterator, leaving > NewProductPort; the identifier check then rejects it. Consequently, extension_host could introduce a new product-defined trait implementation without tripping the shrink-only gate. Parse balanced angle brackets (or use a Rust syntax parser) and add this shape to the scanner self-test.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified and fixed in 0619f2b — the finding is correct.

I reproduced it in the scanner self-test before changing anything: impl<T: Iterator<Item = X>> NestedBound for Host<T> closes the parameter list at Iterator's >, leaves > NestedBound, fails is_rust_identifier, and the impl is dropped — so a new product-defined port could have entered extension_host with the shrink-only gate enforcing nothing for it.

balanced_angle_close now counts nesting rather than taking the first >. One extra case your example does not cover but the same walk hits: a -> return arrow inside a bound (impl<F: Fn(&str) -> bool> ArrowBound for Guard<F>) also puts a > in the list, so a > preceded by - is skipped. Both shapes are in impl_scanner_reads_the_trait_out_of_real_impl_shapes, and it fails without the fix.

Re-verified after widening the scan: the residue is still exactly the six frozen entries, so the wider walk surfaced no previously hidden implementation — the hole was latent, not live.

…nner bracket hole

Two follow-ups on the WS2.1 port inversion, both found by measuring rather
than assuming.

**Coverage of the surfaces this PR created.** `cargo llvm-cov` over
`ironclaw_product_contracts` showed the relocated bodies had no crate-tier
coverage of their own: `ProductCommandContext::from_envelope`,
`AdminUserRole::is_admin`, `AccountConnectionStatusError::new`,
`ChannelConnectionNoticePolicy::generic`, the bounded-token
`TryFrom`/`AsRef`/`Display` arms, and — the one that matters most — the two
`LifecycleProductService` **default** method bodies, which every production
implementor overrides, so nothing exercised the fail-closed defaults. Each is
now tested at its contract meaning, not for the line count: bundle import
defaults to `InvalidRequest` rather than silently succeeding; activation errors
default to none so the wire field stays absent; a non-command envelope is
rejected as an invalid request rather than an internal error; a token that
deserializes runs the same validation as its constructor; the generic notice
policy names the channel in all five notices and does not collapse them into
one string. Every added production line in the new modules is now covered.

**The scanner had a hole the review caught, and it was real.**
`implemented_trait_names` closed the impl's generic-parameter list at the first
`>`. For `impl<T: Iterator<Item = X>> Port for Host<T>` that `>` closes
`Iterator`, leaving `> Port` — not an identifier, so the impl was dropped and a
new product-defined port could have entered `extension_host` without tripping
the shrink-only gate. Now closed by balancing, with `->` inside a bound
(`impl<F: Fn(&str) -> bool>`) excluded from the count, and both shapes added to
the scanner self-test — which fails without the fix. Re-verified after the fix:
the residue is still exactly the six frozen entries, so the wider scan found no
previously hidden implementation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6998 August 1, 2026 05:21 Destroyed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

🤖 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_extension_host_port_inversion.rs`:
- Around line 132-157: Update rust_files to propagate read_dir failures and
individual directory-entry errors instead of returning or using flatten, and
update traits_implemented_by to propagate file-read errors instead of
continuing. Ensure the surrounding scan/test flow fails loudly on any filesystem
error rather than producing an incomplete result.
- Around line 139-144: Update the path exclusion logic in the inversion scan to
skip test_support directories and fixtures, including test_support.rs and
test_support/lifecycle.rs. Alternatively, extend strip_cfg_test_blocks() to
remove items guarded by #[cfg(feature = "test-support")], ensuring test-only
trait implementations cannot satisfy ports during the scan.
- Around line 163-217: Update implemented_trait_names and strip_cfg_test_blocks
so cfg(test) block detection uses source with comments and string literals
removed before scanning braces. Preserve the existing cfg(test) item removal
behavior, then continue extracting implemented traits from the cleaned result so
braces inside comments or strings cannot retain test-only impl edges.

In `@crates/ironclaw_product_contracts/src/delivery.rs`:
- Around line 25-51: Replace the raw String and &str extension_id and
installation_id values in ResolvedChannelDelivery,
ChannelDeliveryResolver::resolve_channel_delivery, and
DeliveryReplyContextSource::reply_context with distinct domain identifier
newtypes owned by the appropriate contracts module. Update the associated
fields, trait parameters, and implementations/callers to use the matching
newtypes while preserving existing behavior and preventing
extension/installation identity interchangeability.

In `@crates/ironclaw_product_contracts/src/lifecycle_service.rs`:
- Around line 54-67: The default
LifecycleProductService::import_extension_bundle implementation should represent
unsupported capability rather than invalid input. Replace its InvalidRequest/400
construction with ProductSurfaceError::unavailable(false), and add a
caller-level regression covering the absent-wiring path to verify the
unavailable contract is returned.
- Around line 80-85: Update installed_activation_errors and its associated
product port to return HashMap<ExtensionId, String> instead of String-keyed
maps. Propagate the typed ExtensionId key through the host implementation,
lookups, and tests, while retaining activation_error as the String wire/response
value.

In `@crates/ironclaw_product_contracts/src/prompt_source.rs`:
- Around line 25-35: The relocated contracts use raw string identifiers instead
of domain newtypes. In crates/ironclaw_product_contracts/src/prompt_source.rs
lines 25-35, update BlockedAuthPromptRequest::gate_ref to use TurnGateRef; in
crates/ironclaw_product_contracts/src/lifecycle_service.rs lines 80-85, change
the return type to HashMap<ExtensionId, String>, importing or reusing
ExtensionId as needed.

In `@crates/ironclaw_product/CLAUDE.md`:
- Around line 40-49: The hand-written contract inventories must be reconciled
with the authoritative architecture test and source modules. In
crates/ironclaw_product/CLAUDE.md lines 40-49, replace the eleven-port
relocation list and nine-port claim with the shrink-only enumeration from
reborn_extension_host_port_inversion.rs, including the correct treatment of
AdminUserService and RebornOperatorToolCatalog. In
crates/ironclaw_product_contracts/CLAUDE.md lines 19-39, change “sixteen
modules” to match src/lib.rs’s seventeen modules and update the line 120-126
enumeration to include its two missing ports.

In `@crates/ironclaw_product/src/action.rs`:
- Around line 7-11: Move the ProductCommandContext::from_envelope constructor
and envelope-admission coverage into crates/ironclaw_product_contracts, or add
equivalent contract-owned tests there. Cover preservation and validation of
requested_command and the envelope timestamp at this trust boundary; update the
related imports in crates/ironclaw_product/src/action.rs (anchor) and
crates/ironclaw_product/src/command_dispatch.rs (sibling) only as needed, with
no direct change required if the tests are relocated without affecting those
files.

In `@crates/ironclaw_product/src/delivery_coordinator.rs`:
- Around line 39-41: Remove executable ChannelAdapter and RestrictedEgress
handles from the contracts-side ResolvedChannelDelivery; keep that DTO limited
to neutral channel identifiers and move the resolved delivery-handle type or
equivalent resolution into the product crate. Update ChannelDeliveryResolver and
IronclawProductDeliveryCoordinator::drive_prepared() to cross the contracts seam
using identifiers, while retaining product-side adapter delivery and egress
enforcement.

In `@docs/reborn/target-architecture/CHECKLIST.md`:
- Around line 73-77: The checklist’s WS2.1 port-count wording is inconsistent:
it says “plus four more” and “nine ports total” while listing five additional
port entries. Update the paragraph to either use the correct total for
individually named ports or explicitly call them port families and document the
grouping, ensuring the count matches the listed symbols and architecture tests.

In `@docs/reborn/target-architecture/families/contracts.md`:
- Line 148: Update the dependency list in the contracts architecture
documentation to remove ironclaw_common, leaving only ironclaw_host_api and
ironclaw_extension_contracts as permitted dependencies. Check CLAUDE.md and
AGENTS.md for any stale documentation-reference checks, then run mint dev and
mint broken-links from the docs directory.
🪄 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: d49f8167-8840-4ad7-90f5-51a6dd688cbd

📥 Commits

Reviewing files that changed from the base of the PR and between a50ad06 and f4819bb.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock, !**/Cargo.lock
📒 Files selected for processing (129)
  • crates/AGENTS.md
  • crates/ironclaw_architecture/tests/reborn_extension_host_port_inversion.rs
  • crates/ironclaw_extension_host/src/admin_configuration.rs
  • crates/ironclaw_extension_host/src/available_extension_import.rs
  • crates/ironclaw_extension_host/src/available_extensions.rs
  • crates/ironclaw_extension_host/src/channel_command_roles.rs
  • crates/ironclaw_extension_host/src/channel_config.rs
  • crates/ironclaw_extension_host/src/channel_connection.rs
  • crates/ironclaw_extension_host/src/channel_delivery.rs
  • crates/ironclaw_extension_host/src/channel_dm_provisioning.rs
  • crates/ironclaw_extension_host/src/channel_host.rs
  • crates/ironclaw_extension_host/src/channel_host/e2e_auth_challenge.rs
  • crates/ironclaw_extension_host/src/channel_host/e2e_tests.rs
  • crates/ironclaw_extension_host/src/channel_lifecycle.rs
  • crates/ironclaw_extension_host/src/channel_outbound_targets.rs
  • crates/ironclaw_extension_host/src/channel_pairing.rs
  • crates/ironclaw_extension_host/src/channel_pairing/tests.rs
  • crates/ironclaw_extension_host/src/channel_subject_routes.rs
  • crates/ironclaw_extension_host/src/extension_credential_requirements.rs
  • crates/ironclaw_extension_host/src/extension_ingress.rs
  • crates/ironclaw_extension_host/src/extension_lifecycle_capabilities.rs
  • crates/ironclaw_extension_host/src/extension_lifecycle_command.rs
  • crates/ironclaw_extension_host/src/generic_host.rs
  • crates/ironclaw_extension_host/src/hosted_mcp_manifest.rs
  • crates/ironclaw_extension_host/src/hosted_mcp_preparation.rs
  • crates/ironclaw_extension_host/src/inbound_batches.rs
  • crates/ironclaw_extension_host/src/ingress/router.rs
  • crates/ironclaw_extension_host/src/install_policy.rs
  • crates/ironclaw_extension_host/src/ironhub/model.rs
  • crates/ironclaw_extension_host/src/ironhub/service.rs
  • crates/ironclaw_extension_host/src/lifecycle.rs
  • crates/ironclaw_extension_host/src/lifecycle_product_service.rs
  • crates/ironclaw_extension_host/src/lifecycle_restore.rs
  • crates/ironclaw_extension_host/src/operator_config_capability.rs
  • crates/ironclaw_extension_host/src/product_lifecycle.rs
  • crates/ironclaw_extension_host/src/provider_identity.rs
  • crates/ironclaw_extension_host/src/run_delivery_ports.rs
  • crates/ironclaw_extension_host/src/test_support.rs
  • crates/ironclaw_extension_host/src/test_support/lifecycle.rs
  • crates/ironclaw_extension_host/src/webui_extension_credentials.rs
  • crates/ironclaw_extension_host/tests/ingress_router_contract.rs
  • crates/ironclaw_product/CLAUDE.md
  • crates/ironclaw_product/src/action.rs
  • crates/ironclaw_product/src/auth_continuation.rs
  • crates/ironclaw_product/src/auth_prompt.rs
  • crates/ironclaw_product/src/command_admission.rs
  • crates/ironclaw_product/src/command_dispatch.rs
  • crates/ironclaw_product/src/communication_context.rs
  • crates/ironclaw_product/src/delivery_coordinator.rs
  • crates/ironclaw_product/src/extension_account_setup.rs
  • crates/ironclaw_product/src/fakes.rs
  • crates/ironclaw_product/src/filesystem_ledger.rs
  • crates/ironclaw_product/src/filesystem_ledger/path.rs
  • crates/ironclaw_product/src/in_memory_ledger.rs
  • crates/ironclaw_product/src/inbound_turn/tests.rs
  • crates/ironclaw_product/src/ledger.rs
  • crates/ironclaw_product/src/lib.rs
  • crates/ironclaw_product/src/lifecycle.rs
  • crates/ironclaw_product/src/policy.rs
  • crates/ironclaw_product/src/projection/turn_events.rs
  • crates/ironclaw_product/src/reborn_services.rs
  • crates/ironclaw_product/src/reborn_services/admin_configuration.rs
  • crates/ironclaw_product/src/reborn_services/admin_users.rs
  • crates/ironclaw_product/src/reborn_services/extensions.rs
  • crates/ironclaw_product/src/reborn_services/lifecycle_setup.rs
  • crates/ironclaw_product/src/reborn_services/llm_config.rs
  • crates/ironclaw_product/src/reborn_services/log_views.rs
  • crates/ironclaw_product/src/reborn_services/operator_command_views.rs
  • crates/ironclaw_product/src/reborn_services/operator_config_views.rs
  • crates/ironclaw_product/src/reborn_services/outbound_delivery_capability_surface.rs
  • crates/ironclaw_product/src/reborn_services/outbound_views.rs
  • crates/ironclaw_product/src/reborn_services/run_artifact.rs
  • crates/ironclaw_product/src/reborn_services/thread_artifact.rs
  • crates/ironclaw_product/src/reborn_services/trace_credits.rs
  • crates/ironclaw_product/src/reborn_services/views.rs
  • crates/ironclaw_product/src/run_delivery.rs
  • crates/ironclaw_product/src/run_delivery/observer.rs
  • crates/ironclaw_product/src/run_delivery/triggered.rs
  • crates/ironclaw_product/src/workflow.rs
  • crates/ironclaw_product/tests/durable_ledger_support/mod.rs
  • crates/ironclaw_product/tests/durable_scoped_ledger_contract.rs
  • crates/ironclaw_product/tests/extension_account_setup_contract.rs
  • crates/ironclaw_product/tests/outbound_delivery_contract.rs
  • crates/ironclaw_product/tests/product_command_surface_contract.rs
  • crates/ironclaw_product/tests/product_surface_contract.rs
  • crates/ironclaw_product/tests/prompt_projection_contract.rs
  • crates/ironclaw_product/tests/reborn_services_contract.rs
  • crates/ironclaw_product/tests/run_delivery_contract.rs
  • crates/ironclaw_product_contracts/CLAUDE.md
  • crates/ironclaw_product_contracts/Cargo.toml
  • crates/ironclaw_product_contracts/src/account_setup.rs
  • crates/ironclaw_product_contracts/src/action.rs
  • crates/ironclaw_product_contracts/src/admin_users.rs
  • crates/ironclaw_product_contracts/src/channel_config.rs
  • crates/ironclaw_product_contracts/src/command.rs
  • crates/ironclaw_product_contracts/src/delivery.rs
  • crates/ironclaw_product_contracts/src/lib.rs
  • crates/ironclaw_product_contracts/src/lifecycle_service.rs
  • crates/ironclaw_product_contracts/src/operator_tools.rs
  • crates/ironclaw_product_contracts/src/prompt_source.rs
  • crates/ironclaw_product_contracts/src/views.rs
  • crates/ironclaw_reborn_composition/src/admin_token.rs
  • crates/ironclaw_reborn_composition/src/admin_user_directory.rs
  • crates/ironclaw_reborn_composition/src/deployment.rs
  • crates/ironclaw_reborn_composition/src/extension_host_assembly.rs
  • crates/ironclaw_reborn_composition/src/factory.rs
  • crates/ironclaw_reborn_composition/src/factory/production_backend_assembly.rs
  • crates/ironclaw_reborn_composition/src/factory/production_build_assembly.rs
  • crates/ironclaw_reborn_composition/src/factory/test_support.rs
  • crates/ironclaw_reborn_composition/src/factory/tests.rs
  • crates/ironclaw_reborn_composition/src/input.rs
  • crates/ironclaw_reborn_composition/src/lib.rs
  • crates/ironclaw_reborn_composition/src/operator_tool_catalog.rs
  • crates/ironclaw_reborn_composition/src/product_surface/tests.rs
  • crates/ironclaw_reborn_composition/src/runtime.rs
  • crates/ironclaw_reborn_composition/src/runtime/tests/core.rs
  • crates/ironclaw_reborn_composition/tests/production_runtime_automations.rs
  • crates/ironclaw_reborn_composition/tests/webui_v2_serve.rs
  • crates/ironclaw_webui/src/webui_v2/handlers.rs
  • crates/ironclaw_webui/tests/support/product_surface.rs
  • crates/ironclaw_webui/tests/webui_v2_handlers_contract.rs
  • docs/plans/composition-pubuse.snapshot
  • docs/reborn/target-architecture/CHECKLIST.md
  • docs/reborn/target-architecture/families/contracts.md
  • tests/e2e/scenarios/test_admin_api.py
  • tests/integration/extension_delivery.rs
  • tests/integration/hosted_mcp_registration.rs
  • tests/integration/support/product_surface.rs
  • tests/integration/webui_v2_product_api.rs

Comment thread crates/ironclaw_architecture/tests/reborn_extension_host_port_inversion.rs Outdated
Comment thread crates/ironclaw_product_contracts/src/delivery.rs
Comment on lines +54 to +67
/// Import a standalone extension from an uploaded bundle (zip bytes) — the
/// WebUI "Install Tool" path. Default is unavailable; only the local runtime
/// service implements it.
async fn import_extension_bundle(
&self,
_context: LifecycleProductContext,
_bundle: Vec<u8>,
) -> Result<LifecycleProductResponse, ProductSurfaceError> {
Err(ProductSurfaceError::from_status(
ProductSurfaceErrorCode::InvalidRequest,
400,
false,
))
}

@coderabbitai coderabbitai Bot Aug 1, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Was this default body carried over verbatim, and what does the taxonomy offer?
rg -n -B4 -A12 'fn import_extension_bundle' crates/ --glob '*.rs'
rg -n 'Unavailable' crates/ironclaw_product_contracts/src/surface.rs | head -20

Repository: nearai/ironclaw

Length of output: 5643


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== lifecycle_service.rs =="
sed -n '1,90p' crates/ironclaw_product_contracts/src/lifecycle_service.rs

echo "== surface.rs relevant product surface error code definitions =="
sed -n '240,445p' crates/ironclaw_product_contracts/src/surface.rs

echo "== error construction helper usages around InvalidRequest/Unavailable =="
rg -n -B3 -A6 'ProductSurfaceError::from_status|ProductSurfaceErrorCode::InvalidRequest|ProductSurfaceErrorCode::Unavailable' crates/ironclaw_product/src crates/ironclaw_product_contracts/src --glob '*.rs' | head -120

echo "== product surface impl in ironclaw_product =="
rg -n -B8 -A12 'unavailable default|Unimplemented|ImportExtension|import_extension_bundle' crates/ironclaw_product/src crates/ironclaw_product/tests/reborn_services_contract.rs --glob '*.rs'

echo "== implementation of extension management result mapping =="
sed -n '458,490p' crates/ironclaw_extension_host/src/lifecycle_product_service.rs
rg -n -B20 -A40 'fn lifecycle_product_surface_error|ProductSurfaceFailure::InvalidBindingRequest|ServiceUnavailable' crates/ironclaw_product/src --glob '*.rs' | head -180

Repository: nearai/ironclaw

Length of output: 41481


Make the default bundle import response match “unavailable”.

LifecycleProductService::import_extension_bundle says the default is unavailable, but it returns ProductSurfaceErrorCode::InvalidRequest/400. That violates the product error contract mapping: 400 is used for malformed bindings, not omitted capability support. Change this method to return ProductSurfaceError::unavailable(false) and add a caller-level regression for the absent-wiring path.

🤖 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_product_contracts/src/lifecycle_service.rs` around lines 54 -
67, The default LifecycleProductService::import_extension_bundle implementation
should represent unsupported capability rather than invalid input. Replace its
InvalidRequest/400 construction with ProductSurfaceError::unavailable(false),
and add a caller-level regression covering the absent-wiring path to verify the
unavailable contract is returned.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed as a real defect, recorded rather than fixed here (f60f896).

Both the wording and the InvalidRequest/400 body arrived verbatim from ironclaw_product::lifecycle — the move did not introduce the mismatch and does not get to resolve it, because changing the code changes an HTTP status on a live WebUI route ("Install Tool"). What I did instead: the doc comment now describes what the code actually does, names the discrepancy and why 400 is the wrong meaning (an unwired capability is not the caller's fault), and points at the test; the test carries a comment saying it pins today's behavior, not the desired one, with the invariant that survives either way — a service without bundle support must refuse, never succeed. So the flip cannot happen silently and the next person to touch it has the whole argument in front of them.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== lifecycle service outline =="
ast-grep outline crates/ironclaw_product_contracts/src/lifecycle_service.rs --items all

echo "== relevant implementation and tests =="
rg -n -B6 -A22 'import_extension_bundle|today.s behavior|unwired capability|InvalidRequest|unavailable' \
  crates/ironclaw_product_contracts/src/lifecycle_service.rs \
  crates/ironclaw_product/tests \
  crates/ironclaw_reborn_composition \
  --glob '*.rs' || true

echo "== follow-up references in the changed documentation and source =="
rg -n -i -C2 'bundle import|install tool|unwired capability|invalid request|unavailable' \
  crates/ironclaw_product_contracts \
  crates/ironclaw_product \
  docs \
  --glob '*.rs' --glob '*.md' | head -240

Length of output: 201237


@BenKurrek, verified.

crates/ironclaw_product_contracts/src/lifecycle_service.rs now documents the mismatch at Lines 54-63. The test at Lines 176-180 pins the current InvalidRequest result and verifies that the default refuses the operation.

The behavior remains InvalidRequest/400. ProductSurfaceError::unavailable(false) remains Unavailable/503 in crates/ironclaw_product_contracts/src/surface.rs Lines 373-380.

The added test is a contract-level test. It is not the caller-level WebUI absent-wiring test from the original request. That is acceptable for this move-only PR, but the follow-up must change the error contract and test the “Install Tool” route.

The finding remains open. I will not create a follow-up issue unless you explicitly request it.

You are interacting with an AI system.

Comment thread crates/ironclaw_product/CLAUDE.md
Comment thread crates/ironclaw_product/src/action.rs
Comment thread crates/ironclaw_product/src/delivery_coordinator.rs
Comment thread docs/reborn/target-architecture/CHECKLIST.md Outdated
Comment thread docs/reborn/target-architecture/families/contracts.md

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/ironclaw_product_contracts/src/lifecycle_service.rs`:
- Around line 170-177: Change the lifecycle service contract so
installed_activation_errors returns HashMap<ExtensionId, String> instead of
HashMap<String, String>, updating the trait and
ExtensionHostLifecycleProductService implementation plus any affected callers,
imports, and tests. Preserve the existing empty/default behavior while using
ExtensionId as the map key throughout.
- Around line 161-168: Update the default import_extension_bundle implementation
to return ProductSurfaceError::unavailable(false) when bundle import is
unsupported, and change
bundle_import_defaults_to_an_invalid_request_rather_than_silently_succeeding to
expect ProductSurfaceErrorCode::Unavailable. Also update
ExtensionHostLifecycleProductService::import_extension_bundle so its
absent-extension-management branch returns the same unavailable error instead of
InvalidBindingRequest.
🪄 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: 67d48a27-1176-42d1-8324-05a9be4503cf

📥 Commits

Reviewing files that changed from the base of the PR and between f4819bb and 0619f2b.

📒 Files selected for processing (6)
  • crates/ironclaw_architecture/tests/reborn_extension_host_port_inversion.rs
  • crates/ironclaw_product_contracts/src/account_setup.rs
  • crates/ironclaw_product_contracts/src/action.rs
  • crates/ironclaw_product_contracts/src/admin_users.rs
  • crates/ironclaw_product_contracts/src/command.rs
  • crates/ironclaw_product_contracts/src/lifecycle_service.rs

Comment thread crates/ironclaw_product_contracts/src/lifecycle_service.rs
Comment on lines +170 to +177
#[tokio::test]
async fn activation_errors_default_to_none_so_the_wire_field_stays_absent() {
let errors = MinimalLifecycleService
.installed_activation_errors(surface_context())
.await
.expect("default reports no durable errors");
assert!(errors.is_empty());
}

@coderabbitai coderabbitai Bot Aug 1, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

installed_activation_errors still keys the map with String, not ExtensionId.

A prior review on this same file (lines 80-85) flagged that installed_activation_errors returns HashMap<String, String> while the doc comment says it is "keyed by extension id" — the port should return HashMap<ExtensionId, String> instead. That comment has no "Addressed" reply.

The downstream implementor ExtensionHostLifecycleProductService::installed_activation_errors (crates/ironclaw_extension_host/src/lifecycle_product_service.rs) still returns Result<std::collections::HashMap<String, String>, ProductSurfaceError>, confirming the signature is unchanged. This new test only checks is_empty(), so it does not lock in the wrong type, but the underlying contract still needs the typed key fixed to comply with the repository's strong-typing rule for domain identifiers.

🤖 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_product_contracts/src/lifecycle_service.rs` around lines 170
- 177, Change the lifecycle service contract so installed_activation_errors
returns HashMap<ExtensionId, String> instead of HashMap<String, String>,
updating the trait and ExtensionHostLifecycleProductService implementation plus
any affected callers, imports, and tests. Preserve the existing empty/default
behavior while using ExtensionId as the map key throughout.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct on the facts, and the disposition is unchanged from the sibling thread — but it now has a named owner instead of a PR-body sentence. a6ee90c.

Verified. LifecycleProductService::installed_activation_errors returns HashMap<String, String> while its own doc says "keyed by extension id", and ExtensionHostLifecycleProductService::installed_activation_errors (crates/ironclaw_extension_host/src/lifecycle_product_service.rs:492-503) still returns the same. Your read that the new test does not lock the wrong type in — it asserts is_empty() only — is also right.

Why not here. Same reason as the sibling: this signature is on origin/main unchanged (crates/ironclaw_product/src/lifecycle.rs:87-91), and WS2.1 relocated it verbatim. Retyping the key changes the contract for every implementor and every caller of the map, which is a semantic change in a move PR.

What changed. The follow-up now sits on a CHECKLIST WS2 row rather than in prose, with the observation that makes it cheap when it lands:

product_contracts::lifecycle_service::LifecycleProductService::installed_activation_errors → HashMap<ExtensionId, String>. The doc says "keyed by extension id"; ExtensionHostLifecycleProductService already holds typed ExtensionId and stringifies only to satisfy the signature.

and the method's doc comment now carries a ✎ note saying the same thing and pointing at the row, so the next reader of the port sees the debt without leaving the file. It is grouped with the other three stringly-typed signatures (BlockedAuthPromptRequest.gate_ref, ResolvedChannelDelivery.{extension_id,installation_id}) because they all re-open for their implementors in the same slice.

Verification as on the sibling thread: 225 tests passed / 0 failed across ironclaw_architecture + ironclaw_product_contracts, fmt and clippy clean.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BenKurrek, acknowledged. The finding remains valid, but the deferment is appropriate for this move-only extraction.

The CHECKLIST row and in-file note provide a concrete owner and migration target. This preserves the WS2.1 scope while recording the required HashMap<ExtensionId, String> contract change.

Would you like me to open a GitHub follow-up issue for the grouped typed-signature migration?


✏️ Learnings added
Learnt from: BenKurrek
Repo: nearai/ironclaw PR: 6998
File: crates/ironclaw_product_contracts/src/lifecycle_service.rs:192-199
Timestamp: 2026-08-01T14:20:17.234Z
Learning: For move-only WS2.1 contract extraction in nearai/ironclaw PR `#6998`, defer the semantic migration of `ironclaw_product_contracts::lifecycle_service::LifecycleProductService::installed_activation_errors` from `HashMap<String, String>` to `HashMap<ExtensionId, String>` to a named follow-up. The signature was relocated unchanged from `origin/main`; the migration must update implementors and callers together. `ExtensionHostLifecycleProductService` already has typed `ExtensionId` values and stringifies them only because of the current contract.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: henrypark133
Repo: nearai/ironclaw PR: 4953
File: crates/ironclaw_reborn_composition/src/slack_delivery.rs:2520-2520
Timestamp: 2026-06-17T04:13:18.772Z
Learning: When suppressing Clippy with `#[allow(clippy::too_many_arguments)]`, require an “architecture exemption” comment immediately above the attribute that explains why the many-arguments signature is justified (e.g., what bundled concepts the function needs) and provides a concrete rationale for the exemption. Avoid silent `#[allow(...)]` for this lint.

Learnt from: BenKurrek
Repo: nearai/ironclaw PR: 6873
File: docs/superpowers/plans/2026-07-29-pr1-role-gated-command-admission.md:621-624
Timestamp: 2026-07-29T18:55:20.993Z
Learning: When reviewing Rust changes in the IronClaw repo, follow the documented two-lane Clippy feature-matrix checks from `.claude/rules/review-discipline.md`: run Clippy with the default feature set first (i.e., *without* `--all-features`), because an all-features-only build can hide feature-gated dead code. Then run a second Clippy lane with `--all-features` to exercise all feature combinations. Use `cargo clippy --workspace --all-targets ... -D warnings` for each lane as appropriate; treat a single workspace-wide `cargo clippy --workspace --all-targets --all-features -- -D warnings` run as supplemental coverage, not a replacement for the required default-feature lane + `--all-features` lane.

Learnt from: BenKurrek
Repo: nearai/ironclaw PR: 6364
File: crates/ironclaw_product_adapters/src/lib.rs:0-0
Timestamp: 2026-07-29T20:58:08.708Z
Learning: When reviewing Rust code, ensure consumers import `WorkspaceFile` directly from `ironclaw_host_api` (i.e., `ironclaw_host_api::WorkspaceFile`). `WorkspaceFile` is owned by `ironclaw_host_api::attachment` and is re-exported from `ironclaw_host_api`; do not introduce or rely on imports from the retired `ironclaw_product_adapters` crate (and do not treat it as an active consumer or re-export facade). Verify that no `use ...::WorkspaceFile` (or similar) references `ironclaw_product_adapters`, and that any `pub use`/facade changes don’t reintroduce that dependency.

Learnt from: BenKurrek
Repo: nearai/ironclaw PR: 6967
File: crates/ironclaw_product/src/approval_interaction/gate_ref.rs:0-0
Timestamp: 2026-07-31T18:47:58.224Z
Learning: For canonical contract migrations in Rust code under crates/, update imports on lines already modified by the PR to reference the canonical contract owner (for example, `ironclaw_host_api::turn`). Do not change untouched consumer import lines solely as part of the migration; defer those updates to separate consumer migration PRs to keep scope focused.

Learnt from: BenKurrek
Repo: nearai/ironclaw PR: 6967
File: tools/ironclaw_stress/src/user_turn.rs:1680-1680
Timestamp: 2026-07-31T18:48:30.383Z
Learning: In Rust code using the turn vocabulary, validate `ironclaw_host_api::turn::TurnGateRef` only for non-empty content, maximum length, and absence of control characters; it does not require a prefix. Apply the `gate:` prefix requirement only to the separate `LoopGateRef` reference family. Do not apply `LoopGateRef` validation rules to `TurnGateRef` usages, including `tools/ironclaw_stress/src/user_turn.rs`.

Learnt from: BenKurrek
Repo: nearai/ironclaw PR: 6998
File: crates/ironclaw_product_contracts/src/prompt_source.rs:25-35
Timestamp: 2026-08-01T05:36:38.623Z
Learning: For explicitly move-only contract-extraction PRs in the ironclaw_product_contracts crate, preserve existing public Rust signatures. Do not require semantic type improvements in the same PR when they would require coordinated caller migrations across crates. Record any deferred signature changes as a named follow-up that reopens the affected contracts.

Learnt from: BenKurrek
Repo: nearai/ironclaw PR: 6998
File: crates/ironclaw_product/src/delivery_coordinator.rs:39-41
Timestamp: 2026-08-01T05:37:19.485Z
Learning: In the Rust Reborn architecture, `ironclaw_product_contracts` may reference `ironclaw_extension_contracts::channel_adapter::ChannelAdapter` and `ironclaw_extension_contracts::tool_adapter::RestrictedEgress` in contract DTOs such as `delivery::ResolvedChannelDelivery`, following the explicit §6.1.3 one-way dependency for channel-facing contract reuse. Do not implement these ports or delivery execution in the contracts crate. Keep `ChannelAdapter::deliver` implemented in `crates/ironclaw_product/src/delivery_coordinator.rs`.

You are interacting with an AI system.

…he doc counts

Review triage on #6998. Four findings taken, four rejected with evidence in the
thread; the taken ones are all about the gate telling the truth.

**The scanner could pass on an incomplete scan.** `rust_files` returned early on
a `read_dir` error and dropped per-entry errors through `.flatten()`, and
`traits_implemented_by` skipped any file it could not read. A permission or
transient I/O error in CI would have thinned the input and turned the ratchet
green while enforcing nothing — the exact failure class this file exists to
catch. Every I/O error is now fatal.

**`#[cfg(test)]` blocks were located by raw brace bytes.** A `{` inside a
comment or string literal in a gated block desynchronizes the depth count and
either leaks a test-only `impl` into the production set or swallows the
production code that follows it. Comments and strings are now stripped first;
`cfg_test_stripping_survives_braces_in_comments_and_strings` is the pin, and it
fails with the old composition (verified by reverting the order and watching it
go red). The doc comment now also states why `#[cfg(feature = "test-support")]`
is deliberately *not* stripped: that feature compiles into a real build, so an
`impl` behind it is a genuine normal-dependency edge, unlike `#[cfg(test)]`.

**The prose counts had drifted.** Eleven port declarations moved, not nine —
nine that `extension_host` implements (the pinned `INVERTED_PORTS`) plus
`AdminUserService` and `RebornOperatorToolCatalog`, which it only consumes and
composition implements. CHECKLIST, both CLAUDE files, and the module-count line
now agree and all defer to the architecture test as the enforced inventory.
`families/contracts.md` also still listed `ironclaw_common` in the family-level
dependency bullet; that is the second of the two places, now corrected too.

**One mismatch recorded rather than fixed.** `LifecycleProductService::
import_extension_bundle`'s default said "unavailable" while returning
`InvalidRequest`/400. The move carried both verbatim; changing the code changes
an HTTP status on a live route, which does not belong in a move-shaped PR. The
doc now describes what the code does, names the discrepancy, and points at the
test that pins today's behavior so a silent flip is impossible.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6998 August 1, 2026 05:35 Destroyed
The count line said 'seventeen modules' while `src/lib.rs` carries eighteen
`pub mod` declarations — the difference is `test_support`, which is gated
behind `#[cfg(any(test, feature = "test-support"))]` and is deliberately
absent from the table above it. Saying 'seventeen shipped modules plus the
dev-only test_support' makes the table and the manifest agree on inspection
instead of looking like drift.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6998 August 1, 2026 05:38 Destroyed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

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

⚠️ Outside diff range comments (1)
crates/ironclaw_product_contracts/CLAUDE.md (1)

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

Update the architecture-test inventory.

This section introduces reborn_extension_host_port_inversion.rs as an enforcement test for the nine inversions and six-entry residue. The earlier section still states that only three architecture tests hold the line and does not list this test.

Update the count and list, or explicitly scope the earlier statement to the three listed tests. As per coding guidelines, documentation promising guarantees must match code and tests.

Proposed documentation fix
-Three architecture tests hold the line, all runnable with
+Four architecture tests hold the line, all runnable with
...
+- `reborn_extension_host_port_inversion.rs` — the extension-host port-inversion rule and shrink-only residue.
🤖 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_product_contracts/CLAUDE.md` around lines 120 - 148, Update
the earlier architecture-test documentation to include
reborn_extension_host_port_inversion.rs and revise its count, or explicitly
scope the existing “three architecture tests” statement to only the three tests
it lists. Keep the documentation consistent with the enforcement tests described
in this section.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@crates/ironclaw_product_contracts/CLAUDE.md`:
- Around line 120-148: Update the earlier architecture-test documentation to
include reborn_extension_host_port_inversion.rs and revise its count, or
explicitly scope the existing “three architecture tests” statement to only the
three tests it lists. Keep the documentation consistent with the enforcement
tests described in this section.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4f34b45b-9144-4d01-86e7-97240d985722

📥 Commits

Reviewing files that changed from the base of the PR and between f60f896 and a4ce832.

📒 Files selected for processing (1)
  • crates/ironclaw_product_contracts/CLAUDE.md

@github-actions

github-actions Bot commented Aug 1, 2026 •

Copy link
Copy Markdown
Contributor

Coverage ratchet

Ratchet mode: ENFORCING

RATCHET PASS: global
  observed: 85.94% (323818 / 376802 lines)
  floor:    85.11% (tolerance 0.5pp -> effective floor 84.61%)
  denominator: 376802 lines now vs 375097 at floor capture (+1705 lines, +0.45%) — not a material change

RATCHET PASS: ironclaw_runner
  observed: 85.93% (14917 / 17359 lines)
  floor:    85.55% (tolerance 0.5pp -> effective floor 85.05%)
  floor_covered_lines: 14658 (tolerance 20 lines -> effective floor 14638)
  denominator: 17359 lines now vs 17133 at floor capture (+226 lines, +1.32%) — not a material change

RATCHET PASS: ironclaw_processes
  observed: 88.76% (5889 / 6635 lines)
  floor:    88.07% (tolerance 0.5pp -> effective floor 87.57%)
  floor_covered_lines: 5839 (tolerance 20 lines -> effective floor 5819)
  denominator: 6635 lines now vs 6630 at floor capture (+5 lines, +0.08%) — not a material change

RATCHET PASS: ironclaw_turns
  observed: 88.46% (3709 / 4193 lines)
  floor:    85.11% (tolerance 0.5pp -> effective floor 84.61%)

RATCHET PASS: ironclaw_authorization
  observed: 86.59% (723 / 835 lines)
  floor:    62.51% (tolerance 0.5pp -> effective floor 62.01%)
  floor_covered_lines: 612 (tolerance 20 lines -> effective floor 592)
  denominator: 835 lines now vs 979 at floor capture (-144 lines, -14.71%) — material change (>5%)

RATCHET PASS: ironclaw_approvals
  observed: 91.05% (1820 / 1999 lines)
  floor:    85.86% (tolerance 0.5pp -> effective floor 85.36%)
  floor_covered_lines: 1822 (tolerance 20 lines -> effective floor 1802)
  denominator: 1999 lines now vs 2122 at floor capture (-123 lines, -5.8%) — material change (>5%)

RATCHET PASS: ironclaw_secrets
  observed: 85.81% (2896 / 3375 lines)
  floor:    84.01% (tolerance 0.5pp -> effective floor 83.51%)
  floor_covered_lines: 2795 (tolerance 20 lines -> effective floor 2775)
  denominator: 3375 lines now vs 3327 at floor capture (+48 lines, +1.44%) — not a material change

RATCHET PASS: ironclaw_filesystem
  observed: 76.83% (5895 / 7673 lines)
  floor:    75.93% (tolerance 0.5pp -> effective floor 75.43%)
  floor_covered_lines: 5826 (tolerance 20 lines -> effective floor 5806)
  denominator: 7673 lines now vs 7673 at floor capture (+0 lines, +0%) — not a material change

RATCHET PASS: ironclaw_llm
  observed: 79.22% (20885 / 26364 lines)
  floor:    79.22% (tolerance 0.5pp -> effective floor 78.72%)
  floor_covered_lines: 20885 (tolerance 20 lines -> effective floor 20865)
  denominator: 26364 lines now vs 26364 at floor capture (+0 lines, +0%) — not a material change

RATCHET PASS: ironclaw_triggers
  observed: 94.68% (3134 / 3310 lines)
  floor:    86.04% (tolerance 0.5pp -> effective floor 85.54%)
  floor_covered_lines: 2804 (tolerance 20 lines -> effective floor 2784)
  denominator: 3310 lines now vs 3259 at floor capture (+51 lines, +1.56%) — not a material change

RATCHET PASS: ironclaw_product
  observed: 87.37% (22715 / 26000 lines)
  floor:    86.94% (tolerance 0.5pp -> effective floor 86.44%)
  floor_covered_lines: 21367 (tolerance 20 lines -> effective floor 21347)
  denominator: 26000 lines now vs 24576 at floor capture (+1424 lines, +5.79%) — material change (>5%)

RATCHET PASS: ironclaw_outbound
  observed: 94.68% (4271 / 4511 lines)
  floor:    93.49% (tolerance 0.5pp -> effective floor 92.99%)
  floor_covered_lines: 4105 (tolerance 20 lines -> effective floor 4085)
  denominator: 4511 lines now vs 4391 at floor capture (+120 lines, +2.73%) — not a material change

RATCHET PASS: ironclaw_extension_host
  observed: 85.03% (24382 / 28673 lines)
  floor:    83.82% (tolerance 0.5pp -> effective floor 83.32%)
  floor_covered_lines: 22271 (tolerance 20 lines -> effective floor 22251)
  denominator: 28673 lines now vs 26569 at floor capture (+2104 lines, +7.92%) — material change (>5%)

RATCHET PASS: ironclaw_events
  observed: 80.55% (1197 / 1486 lines)
  floor:    80.55% (tolerance 0.5pp -> effective floor 80.05%)
  floor_covered_lines: 1197 (tolerance 20 lines -> effective floor 1177)
  denominator: 1486 lines now vs 1486 at floor capture (+0 lines, +0%) — not a material change

RATCHET PASS: ironclaw_safety
  observed: 92.75% (4468 / 4817 lines)
  floor:    92.44% (tolerance 0.5pp -> effective floor 91.94%)
  floor_covered_lines: 3973 (tolerance 20 lines -> effective floor 3953)
  denominator: 4817 lines now vs 4298 at floor capture (+519 lines, +12.08%) — material change (>5%)

RATCHET PASS: ironclaw_host_runtime
  observed: 88.41% (21338 / 24135 lines)
  floor:    88.23% (tolerance 0.5pp -> effective floor 87.73%)
  floor_covered_lines: 20538 (tolerance 20 lines -> effective floor 20518)
  denominator: 24135 lines now vs 23277 at floor capture (+858 lines, +3.69%) — not a material change

Reborn integration-tier coverage

Line coverage (Reborn crates): 85.94% — 323818 / 376802 lines

Per-crate breakdown (63 crates, lowest-covered first)
Crate Line % Covered / Total
ironclaw_host_ingress 42.5% 17 / 40
ironclaw_memory 53.48% 630 / 1178
ironclaw_projects 72.36% 233 / 322
ironclaw_capabilities 74.59% 2876 / 3856
ironclaw_trust 75.79% 748 / 987
ironclaw_extractors 75.88% 538 / 709
ironclaw_reborn_cli 76.1% 11084 / 14566
ironclaw_observability 76.19% 32 / 42
ironclaw_filesystem 76.83% 5895 / 7673
ironclaw_wasm 78.84% 704 / 893
ironclaw_llm 79.22% 20885 / 26364
ironclaw_events 80.55% 1197 / 1486
ironclaw_loop_contracts 82.4% 5637 / 6841
ironclaw_first_party_extensions 82.57% 6784 / 8216
ironclaw_product_contracts 82.84% 2573 / 3106
ironclaw_memory_native 82.85% 2850 / 3440
ironclaw_libsql_runtime 83.3% 384 / 461
ironclaw_auth 83.95% 6699 / 7980
ironclaw_operator 84.47% 5309 / 6285
ironclaw_hooks 84.57% 9896 / 11702
ironclaw_event_projections 84.81% 854 / 1007
ironclaw_reborn_event_store 84.93% 1206 / 1420
ironclaw_extension_host 85.03% 24382 / 28673
ironclaw_reborn_config 85.29% 2110 / 2474
ironclaw_network 85.31% 894 / 1048
ironclaw_reborn_composition 85.51% 21791 / 25483
ironclaw_extension_contracts 85.64% 2546 / 2973
ironclaw_secrets 85.81% 2896 / 3375
ironclaw_runner 85.93% 14917 / 17359
ironclaw_host_api 86.05% 6367 / 7399
ironclaw_authorization 86.59% 723 / 835
ironclaw_webui 86.95% 11937 / 13729
ironclaw_wasm_limiter 87.06% 74 / 85
ironclaw_common 87.07% 1152 / 1323
ironclaw_product 87.37% 22715 / 26000
ironclaw_reborn_traces 87.61% 11720 / 13377
ironclaw_scripts 87.87% 420 / 478
ironclaw_threads 88.14% 5189 / 5887
ironclaw_host_runtime 88.41% 21338 / 24135
ironclaw_turns 88.46% 3709 / 4193
ironclaw_telegram_extension 88.55% 588 / 664
ironclaw_skills 88.61% 2785 / 3143
ironclaw_process_sandbox 88.64% 281 / 317
ironclaw_processes 88.76% 5889 / 6635
ironclaw_reborn_openai_compat 89.4% 3644 / 4076
ironclaw_telegram_v2_adapter 89.47% 1580 / 1766
ironclaw_extensions 89.55% 6249 / 6978
ironclaw_loop_host 90.47% 18043 / 19944
ironclaw_resources 90.76% 4084 / 4500
ironclaw_approvals 91.05% 1820 / 1999
ironclaw_reborn_identity 91.3% 451 / 494
ironclaw_mcp 92% 1426 / 1550
ironclaw_conversations 92.08% 2383 / 2588
ironclaw_event_streams 92.5% 1048 / 1133
ironclaw_safety 92.75% 4468 / 4817
ironclaw_agent_loop 93.52% 10430 / 11153
ironclaw_slack_extension 93.95% 3697 / 3935
ironclaw_first_party_extension_ports 94.66% 3758 / 3970
ironclaw_outbound 94.68% 4271 / 4511
ironclaw_triggers 94.68% 3134 / 3310
ironclaw_prompt_envelope 97.46% 192 / 197
ironclaw_runtime_policy 97.6% 855 / 876
ironclaw_attachments 98.23% 831 / 846

This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors.

Exemptions (18 entry/entries excluded from the accounting above)
Module / Crate Reason Issue
crate: ironclaw_gateway v1-only: consumed only by root ironclaw (src/channels/web/platform/static_files.rs, src/channels/web/handlers/frontend.rs); no crates/* dependents. Covered by "Tests (Legacy)". #5657
crate: ironclaw_tui v1-only: consumed only by root ironclaw (src/main.rs, src/channels/tui.rs); no crates/* dependents. Crate's own doc comment confirms it bridges INTO v1, not Reborn. Covered by "Tests (Legacy)". #5657
crates/ironclaw_attachments/src/lib.rs Declarative crate facade: module declarations, constants, and re-exports only; executable attachment modules remain covered. #6524
crates/ironclaw_extension_host/src/ingress/mod.rs Declarative ingress module facade and documentation only; executable router modules remain covered. #6524
crates/ironclaw_host_api/src/lib.rs Declarative crate facade: module declarations and re-exports only; executable host API modules remain covered. #6524
crates/ironclaw_host_api/src/product_adapter/mod.rs Declarative product-adapter facade: module declarations and re-exports only; executable adapter modules remain covered. #6524
crates/ironclaw_llm/src/rig_adapter/tests/finish_reason_tests.rs Test-only module stored under src/ for private adapter access; cargo-llvm-cov omits test harness source from production LCOV while the exercised rig_adapter.rs production lines remain coverage-gated. #6284
crates/ironclaw_loop_contracts/src/lib.rs Declaration-only public facade with no executable Rust statements; rustc emits no LCOV source record. Executable loop-contract behavior remains covered in the owned implementation modules. #6524
crates/ironclaw_outbound/src/error.rs Declarative error vocabulary only; variants have no LLVM-instrumentable production statements. #6524
crates/ironclaw_outbound/src/lib.rs Declarative crate facade: module declarations and re-exports only; executable outbound modules remain covered. #6524
crates/ironclaw_product/src/lib.rs Declaration-only public facade with no executable Rust statements; rustc emits no LCOV source record. Executable product behavior remains covered in the owned implementation modules. #6524
crates/ironclaw_product/src/lib.rs Declarative crate facade: module declarations and re-exports only; executable product modules remain covered. #6524
crates/ironclaw_product/src/scoped_fs/mod.rs Declarative scoped-filesystem facade and documentation only; executable scoped filesystem modules remain covered. #6524
crates/ironclaw_reborn_composition/src/support/fs/mod.rs Declarative composition support facade: module declarations and re-exports only; executable filesystem adapters remain covered. #6524
crates/ironclaw_slack_extension/src/lib.rs Declarative Slack crate facade: module declarations and re-exports only; executable Slack modules remain covered. #6524
crates/ironclaw_telegram_extension/src/lib.rs Declarative Telegram crate facade: module declarations and re-exports only; executable Telegram modules remain covered. #6524
crates/ironclaw_threads/src/lib.rs Declaration-only public facade with no executable Rust statements; rustc emits no LCOV source record. Executable thread behavior remains covered in the owned implementation modules. #6524
crates/ironclaw_webui/src/webui_v2/mod.rs Declaration-only WebUI v2 facade with no executable Rust statements; rustc emits no LCOV source record. Executable route behavior remains covered in the owned implementation modules. #6524

BenKurrek added a commit that referenced this pull request Aug 1, 2026
…its scanner hardening

The base branch moved with #6998's review triage (0619f2b, f60f896,
a4ce832). Conflicts were two, both mechanical unions:

- `product_contracts/src/admin_users.rs` — #6998 added a `mod tests`; this
  branch added the `Reborn*` wire DTOs above it. Kept both, tests last.
- `product_contracts/CLAUDE.md` — both slots amended the same "deferred by
  design" section; kept both amendments.

**Adopted, not just merged:** f60f896 found that the port-inversion scanner
could pass on an incomplete scan — `read_dir`/`read_to_string` errors were
skipped, so an unreadable directory reported a *smaller* residue than reality.
`reborn_transport_product_boundary.rs` was written from the same template and
had the same hole; it now panics on any I/O error instead of skipping.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI's changed-coverage gate failed on the WS2.1 move, exactly where a
move-shaped diff is expected to: relocated bodies read as added production
lines. Every hole is now closed with a test. One line is exempted, with its
callers named.

**Five relocated port modules had no LCOV record at all.** `delivery`,
`channel_config`, `operator_tools`, `prompt_source`, and `views` are pure
declarations, so rustc emitted no source record and the gate reported them
absent. Each now carries a contract test rather than a waiver, and the
properties they pin are the ones these ports actually owe:

- **object safety** for all seven traits — every consumer holds them as
  `Arc<dyn _>`, so a signature change that breaks dyn-safety now fails at the
  contract instead of at the far-away wiring site;
- **argument pass-through and ordering** for the delivery ports — `reply_context`
  takes extension id, installation id, and conversation fingerprint as three
  bare strings, so nothing but a test stops a transposition turning into a
  silent mis-delivery (this is the identity-mixup risk review raised; the types
  stay verbatim, the ordering is now pinned);
- **absence without error** — an unresolved channel, an empty channel-config
  field set, an empty operator tool catalog, and a missing approval-prompt
  context are all normal outcomes that must not be expressible only as failures;
- **caller scoping** on the operator catalog, whose `caller` parameter is the
  #5459 disclosure control;
- **`next_cursor` omission** on an unpaginated view page — serializing `null`
  would make every unpaginated view look paginated to the browser.

**Two genuinely untested error paths in `extension_host`, both fail-closed
seams the move touched.** `AccountConnectionStatusSource::connected` now has
coverage proving it fails *closed* on a pairing-backend outage (activation must
not proceed on an unknown connection state) and *sanitized* (the test asserts
the driver, host, and port do not appear in the product-facing error). The
lifecycle output-serialization mapping moved out of an inline closure into a
named `lifecycle_output_decode_error` so the mapping is reachable from a test:
the failure is defensive, but *what it maps to* is a live contract — the model
gets `OutputDecode` and never the serde error, which can quote projection
contents.

**A dead branch arm.** `validate_typed_token` guards `c == '\0' || c.is_control()`
and only the second arm was exercised. NUL has its own arm because a token with
an embedded NUL truncates at a C boundary rather than merely looking odd.

**Diff shape.** The remaining reports were an artifact of relocating types
inline: a fully-qualified `ironclaw_product_contracts::<mod>::<Item>` in a
signature turns an untouched line into a changed one. Those 17 files now import
the symbol like every other, which shrinks the diff, restores the crate's
prevailing style, and drops the lines out of the gate's denominator because a
`use` line is uninstrumentable by construction.

**One exemption, with evidence.** `factory/test_support.rs`'s
`channel_config_service` accessor: the repoint collapsed its signature onto one
line, and the merged lcov does not attribute its two integration callers back
to the composition bucket build. Both callers are named in the manifest, the
service and the port contract are covered by tests added here, and it is filed
under the same #6963 lane-attribution lane as the WS1 entries above it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6998 August 1, 2026 06:30 Destroyed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/ironclaw_product_contracts/src/operator_tools.rs`:
- Around line 59-68: Extend the test around
`the_catalog_is_caller_scoped_and_may_legitimately_be_empty` with an
implementation-backed catalog containing one tenant-shared tool and distinct
private tools for two callers. Invoke `list_operator_tools` for both `UserId`
values and assert each result includes the shared tool and only that caller’s
private tool, verifying private installations are isolated through the real
catalog implementation rather than `EmptyCatalog`.

In `@crates/ironclaw_product_contracts/src/views.rs`:
- Around line 77-85: Update OneRowView::query to use the caller and parameters
instead of ignoring them, returning both alongside the echoed cursor in the test
payload. Extend the corresponding test assertions to verify caller identity and
parameters in addition to cursor behavior, covering both referenced query paths.
🪄 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: 6a2cb5c3-f19d-43f5-a312-c9c501589d3c

📥 Commits

Reviewing files that changed from the base of the PR and between a4ce832 and 5144422.

📒 Files selected for processing (25)
  • crates/ironclaw_extension_host/src/channel_host.rs
  • crates/ironclaw_extension_host/src/channel_pairing/tests.rs
  • crates/ironclaw_extension_host/src/extension_lifecycle_capabilities.rs
  • crates/ironclaw_extension_host/src/extension_lifecycle_command.rs
  • crates/ironclaw_extension_host/src/hosted_mcp_preparation.rs
  • crates/ironclaw_extension_host/src/run_delivery_ports.rs
  • crates/ironclaw_extension_host/src/test_support/lifecycle.rs
  • crates/ironclaw_product_contracts/src/action.rs
  • crates/ironclaw_product_contracts/src/channel_config.rs
  • crates/ironclaw_product_contracts/src/delivery.rs
  • crates/ironclaw_product_contracts/src/operator_tools.rs
  • crates/ironclaw_product_contracts/src/prompt_source.rs
  • crates/ironclaw_product_contracts/src/views.rs
  • crates/ironclaw_reborn_composition/src/deployment.rs
  • crates/ironclaw_reborn_composition/src/extension_host_assembly.rs
  • crates/ironclaw_reborn_composition/src/factory.rs
  • crates/ironclaw_reborn_composition/src/factory/production_backend_assembly.rs
  • crates/ironclaw_reborn_composition/src/factory/production_build_assembly.rs
  • crates/ironclaw_reborn_composition/src/factory/test_support.rs
  • crates/ironclaw_reborn_composition/src/factory/tests.rs
  • crates/ironclaw_reborn_composition/src/input.rs
  • crates/ironclaw_reborn_composition/src/runtime.rs
  • crates/ironclaw_reborn_composition/tests/production_runtime_automations.rs
  • crates/ironclaw_webui/src/webui_v2/handlers.rs
  • tests/integration/changed-coverage-exemptions.toml

Comment thread crates/ironclaw_product_contracts/src/operator_tools.rs
Comment thread crates/ironclaw_product_contracts/src/views.rs
…eir arguments

Review caught two tests of mine that asserted the double's behavior rather
than the contract, and it was right about both.

`EmptyCatalog` ignored `caller` and always returned an empty vector, so
`the_catalog_is_caller_scoped...` would have passed against a production
catalog that disclosed every user's private installs — the exact leak the
`caller` parameter exists to close (#5459 P1). It is now backed by an
ownership-filtering double, two callers, one tenant-shared tool and one private
tool each, asserting both directions of isolation and that the answer *can*
differ by caller. `OneRowView::query` ignored `_caller` and `_params` and the
test only checked the cursor; the provider now echoes all three conduit
arguments and the test asserts all three.

Both were verified red-then-green rather than assumed: dropping the caller
filter fails the catalog tests, and dropping params from the echo fails the
view test. (My first attempt at the view mutation substituted the expected
literals and passed — a reminder that a mutation which doesn't fail proves
nothing about the mutation, only about the mutant.)

The over-claim went into the PR body too, and is corrected there: a contracts
crate can pin that the port *hands the implementation the caller* and that its
shape admits a per-caller answer. It cannot pin that production filters
correctly — that is composition's implementation and composition's test. The
doc comments now say so instead of implying the stronger claim.

Also lands the CHECKLIST note this PR earned for the rest of Wave 2/3: a
move-shaped PR fails the changed-coverage gate on its first CI run, in three
distinct shapes needing three different answers, with the two mechanical habits
that shrink all three.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6998 August 1, 2026 06:46 Destroyed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/reborn/target-architecture/CHECKLIST.md`:
- Around line 79-84: Split the combined ProductSurfaceFailure checklist entry
into separate items: one covering the six port relocations and residue baseline,
one for BlockedAuthPromptRequest.gate_ref, one for installed_activation_errors,
one for ResolvedChannelDelivery identity newtypes, and one for
import_extension_bundle’s default behavior and test. Mark only the six port
relocations as dependent on removing ironclaw_turns::TurnError; give each
independent change its own owner and caller-level tests.
🪄 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: 2c418c86-5e19-44a7-83e7-1710540e3d1c

📥 Commits

Reviewing files that changed from the base of the PR and between fe40cb2 and a6ee90c.

📒 Files selected for processing (2)
  • crates/ironclaw_product_contracts/src/lifecycle_service.rs
  • docs/reborn/target-architecture/CHECKLIST.md

Comment thread docs/reborn/target-architecture/CHECKLIST.md Outdated
…hpin row

Review was right that the row added in a6ee90c bundled a blocker with
work that is not blocked. Only the six unmovable ports depend on
`ProductSurfaceFailure` carrying `ironclaw_turns::TurnError`; `gate_ref`,
the `installed_activation_errors` key, and the delivery identities each
replace a type that already sits in a crate `product_contracts` may name,
so grouping them behind the linchpin would have held independent work
hostage. The grouping was scheduling convenience, not a dependency.

Now two rows: the linchpin (the six ports, and the 19-file use of
product's internal error type as extension_host's own vocabulary), and
the four verbatim-relocated signatures, marked independent of it and of
each other. `import_extension_bundle` is called out on that row as a
behavior change rather than a signature correction, with the live route
and the test assertion that moves with it.

Doc comments repointed to the row that now owns each item.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6998 August 1, 2026 14:50 Destroyed
BenKurrek added a commit that referenced this pull request Aug 1, 2026
…tion, doc drift

Review triage for #7002. Ten of the seventeen threads changed code or docs;
the rest are refuted or routed with evidence in their threads.

**The one real defect: the transport scanner's strip order was inverted.**
`product_symbols_in` ran `strip_cfg_test_blocks` on raw source, so a brace
inside a comment or string literal in a `#[cfg(test)]` block desynchronised the
byte-level depth walk and truncated the rest of the file. Regression test
first: the new case makes the scan return an *empty* set (proof it ran off the
end), then the order flips to match the sibling gate, which #6998's own triage
had already fixed for exactly this reason. The frozen residues are byte-for-byte
unchanged either way, so the bug was latent, not active.

**Four credential/payload DTOs kept a derived `Debug` through the move.**
`ProductInboundAttachment` carries a whole user upload and is a field of
`ProductSubmitTurnRequest`; `RebornAdminPutSecretRequest`/`…ProductRequest`
carry raw secret material in front of a port that takes `SecretString`;
`RebornAdminUserCreatedResponse` carries the one-time API token; and
`RebornAccountLoginLinkResponse.url` is a code-bearing credential whose own doc
comment promises it "must never be logged". Each now hand-writes `Debug`, the
same treatment `ProjectFsFile` already had in this crate, pinned by three tests.
Serde is untouched — no wire shape changes. Four mutations of the
implementations (not the expected literals) produce four assertion-level reds.

**Documentation that the move made false.**
- rustdoc: 8 broken intra-doc links → 0 (`cargo doc --no-deps --all-features`).
  The reviewer found 3; the other 5 are the same class.
- `product_contracts/CLAUDE.md`: nineteen → twenty-three shipped modules, the
  four new rows added, and the `admin_users`/`views` rows corrected — both still
  claimed their contents had stayed in product.
- `product/CLAUDE.md`: `ProductView` is no longer residue; the residue is the
  frozen operation inventory, which is the honest reason the dep survives.
- `Cargo.toml`: the `secrecy/serde` justification named a type carrying no
  secret. The three `Deserialize` bodies that actually force it are named now.
- `PROPOSAL.md` §6.1.3's allowed-deps line dropped `ironclaw_common`, so it
  agrees with `families/contracts.md` and with `product_contracts_allowed`.
- `product_wire.rs`'s `large_file` exemption is one line with `plan #7008`, so
  it satisfies the enforcer regex in `scripts/pre-commit-safety.sh`.
- `descriptors::query_on` states the one-payload conduit invariant.
- `fs_browse.rs` imports the project-fs DTOs from their owner instead of
  through the sibling module's re-export.
- CHECKLIST WS8 records `RebornSkillInfo`'s duplicate `source`/`source_kind`
  wire field — a real removal, deliberately not made inside a move.

Verification: fmt; clippy `-D warnings --all-targets --all-features` on the six
touched crates → clean; unfiltered tests for those six → 2841 passed / 0 failed
across 117 binaries; 4 tests added, none removed; rustdoc 0 warnings.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/reborn/target-architecture/CHECKLIST.md`:
- Line 79: Define the contract-facing error in an allowed crate, limited to
ironclaw_host_api or ironclaw_extension_contracts, and update affected ports to
use it without depending on ironclaw_product or ironclaw_extension_host. In
ironclaw_product, map that neutral error into ProductSurfaceFailure while
preserving existing product behavior; then revise this checklist entry and the
reborn_extension_host_port_inversion.rs residue test plan to track the new
boundary.
- Around line 80-83: Update the checklist section heading to “four independent
WS2.1 follow-ups” and revise its introductory text to state that the list
contains three type corrections and one behavior change, rather than four
unchanged relocations. Remove or qualify the claim that all four items were
relocated unchanged so it applies only to the three type-correction items, while
preserving the independence and dependency statements.
🪄 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: 525719bc-5aef-40d7-8079-777c2c286138

📥 Commits

Reviewing files that changed from the base of the PR and between a6ee90c and ffc4b86.

📒 Files selected for processing (2)
  • crates/ironclaw_product_contracts/src/lifecycle_service.rs
  • docs/reborn/target-architecture/CHECKLIST.md

Comment thread docs/reborn/target-architecture/CHECKLIST.md Outdated
Comment on lines +80 to +83
- [ ] **The four verbatim-relocated signatures WS2.1 deferred — independent of the linchpin row above, and of each other.** ✎ *Split out of that row 2026-08-01 after review pointed out that bundling them behind the blocker would hold independent work hostage; the original grouping was scheduling convenience ("the slice is already in these files"), not a dependency.* Each was relocated **unchanged** under PLAN operating principle 2 (no semantic change in a move PR) and each needs its typed replacement plus a caller-level test at the seam named. None of the four depends on `ProductSurfaceFailure`: every replacement type already sits in a crate `product_contracts` may name.
- `product_contracts::prompt_source::BlockedAuthPromptRequest.gate_ref: &str` → `&TurnGateRef`. `TurnGateRef` is `host_api::turn` vocabulary the module already imports, and the sibling `ApprovalPromptContextSource::approval_prompt_context` already takes `&TurnGateRef` — the inconsistency is visible in one file.
- `product_contracts::lifecycle_service::LifecycleProductService::installed_activation_errors` → `HashMap<ExtensionId, String>`. The doc says "keyed by extension id"; `ExtensionHostLifecycleProductService` already holds typed `ExtensionId` and stringifies only to satisfy the signature.
- `product_contracts::delivery::ResolvedChannelDelivery.{extension_id, installation_id}: String` → distinct newtypes, so an identity mixup fails to compile.

@coderabbitai coderabbitai Bot Aug 1, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Correct the follow-up classification.

The heading says “four verbatim-relocated signatures,” but the list contains three type corrections and one behavior change. The statement that each item was relocated unchanged also does not apply to the behavior change.

Rename this section to “four independent WS2.1 follow-ups” and state the three-to-one split.

Proposed wording
-- [ ] **The four verbatim-relocated signatures WS2.1 deferred
+- [ ] **The four independent WS2.1 follow-ups
🤖 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/CHECKLIST.md` around lines 80 - 83, Update
the checklist section heading to “four independent WS2.1 follow-ups” and revise
its introductory text to state that the list contains three type corrections and
one behavior change, rather than four unchanged relocations. Remove or qualify
the claim that all four items were relocated unchanged so it applies only to the
three type-correction items, while preserving the independence and dependency
statements.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taken on the classification, with one correction to the premise. b36c972.

Where you are right: the row mixed two kinds of fix under one heading. Three are type changes; the fourth flips a live HTTP status. "Signatures" covered the first three and not the fourth, whose defect is in a default body, not a signature.

Where the premise needs adjusting: "verbatim-relocated" is accurate for all four, including the behavior change. I checked before writing it — import_extension_bundle's default is byte-identical on origin/main at crates/ironclaw_product/src/lifecycle.rs:64-74:

Err(ProductSurfaceError::from_status(
    ProductSurfaceErrorCode::InvalidRequest,
    400,
    false,
))

WS2.1 moved it unchanged. So "relocated verbatim" is their shared provenance — which is exactly why all four are follow-ups of this PR rather than pre-existing debt someone else owns — while the fix differs. Dropping the word entirely would lose the reason they belong on a WS2.1 row at all.

Retitled to keep both facts:

The four WS2.1 follow-ups — three typed-signature corrections and one status-code flip — independent of the linchpin row above, and of each other. … Re-titled the same day: the first three are type changes, the fourth is a behavior change, and "signatures" covered only the first three. All four were relocated verbatim by WS2.1 under PLAN operating principle 2 (no semantic change in a move PR) — that is their shared provenance, not a shared fix. The first three need a typed replacement plus a caller-level test at the seam named; the fourth changes a live HTTP status and is sized separately below.

which is your "state the three-to-one split", in the title and in the body, without asserting something about the fourth that is not true.

Verification as on the sibling thread: fmt clean, clippy clean, 94 tests passed / 0 failed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rate Limit Exceeded

@BenKurrek have exceeded the limit for the number of chat messages per hour. Please wait 30 minutes and 14 seconds before sending another message.

Review found the resolution I wrote was wrong, and it was — inherited
verbatim from row 1's disposition 2, which carried the same error.

"Narrow `ProductSurfaceFailure` off `ironclaw_turns`, or replace it with
an extension-host-owned error" is insufficient either way.
`product_contracts`' allowlist is enforced as a whitelist of exactly
{product_contracts, extension_contracts, host_api}
(reborn_dependency_boundaries.rs:362-374, comment: "Never product, never
operator, never the extension host"), and `ProductSurfaceFailure` lives
in `ironclaw_product` (error.rs:46). A port erroring with it is
undeclarable in contracts whatever it carries; an extension-host-owned
error is barred by the same list.

The row now states the real work: define the port-facing error in an
allowed crate and map it to `ProductSurfaceFailure` inside product, with
narrowing off `ironclaw_turns` demoted to the sub-goal it is. Row 1's
disposition 2 carries a dated correction quoting the clause it replaces.

Also re-titled the sibling row: "four verbatim-relocated signatures" was
right about provenance and wrong about the fix — three are type changes
and one is a status-code flip. It is now "The four WS2.1 follow-ups —
three typed-signature corrections and one status-code flip", with the
shared-provenance/separate-fix distinction stated. Doc pointers follow.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6998 August 1, 2026 15:19 Destroyed
@BenKurrek
BenKurrek merged commit 569d8e4 into main Aug 1, 2026
58 of 63 checks passed
@BenKurrek
BenKurrek deleted the ws2/extension-host-ports branch August 1, 2026 15:26
BenKurrek added a commit that referenced this pull request Aug 1, 2026
…ated modules

Applied preemptively from #6998's CI experience, before the gate could fire.

(1) Relocated declaration modules emit no LCOV records. `workspace_views`,
`operator_llm`, `inbound_requests`, `descriptors`, and `product_wire` now carry
contract tests for their real executable content — chosen for the invariant, not
for the number: `ProjectFsFile`'s hand-written `Debug` (it keeps whole user files
out of diagnostics; re-deriving it compiles and silently re-opens the leak),
`FsMount::ALL` exhaustiveness, the `as_str`/`Display`/serde triples, and the
list-request builders.

(2) Two genuinely untested paths arrived with the DTOs and are tested, not
exempted: `validate_outbound_delivery_display_field` — the only fence between an
operator-supplied delivery-target label and the browser, now driven through both
the constructor and the `TryFrom` path for bidi-override, zero-width-joiner,
separator, control-character, whitespace and per-field byte-cap shapes — and the
`RebornOutboundPreferencesResponse` wire-shadow back-compat rule plus
`RebornAutomationState`'s degrade-to-`Unknown` deserializer.

(3) Diff-shape noise: two `webui_v2/handlers.rs` signatures carried inline
`ironclaw_product_contracts::product_wire::<Item>` paths; the longer one made
rustfmt reflow a one-line return type into three. Both are `use` imports now, so
that file's diff is imports plus exactly two production lines.

No test doubles are used, so the discard-the-argument vacuity mode has no
surface. Every test was red-green verified by mutating the implementation (nine
mutations, nine assertion-level reds); two earlier mutations that produced
compile errors were rewritten as behavior changes, because a compile error
proves the compiler caught it and not the test.

Regression tests: crates/ironclaw_product_contracts/src/{workspace_views,
operator_llm,inbound_requests,descriptors,product_wire}.rs

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BenKurrek added a commit that referenced this pull request Aug 1, 2026
…tion, doc drift

Review triage for #7002. Ten of the seventeen threads changed code or docs;
the rest are refuted or routed with evidence in their threads.

**The one real defect: the transport scanner's strip order was inverted.**
`product_symbols_in` ran `strip_cfg_test_blocks` on raw source, so a brace
inside a comment or string literal in a `#[cfg(test)]` block desynchronised the
byte-level depth walk and truncated the rest of the file. Regression test
first: the new case makes the scan return an *empty* set (proof it ran off the
end), then the order flips to match the sibling gate, which #6998's own triage
had already fixed for exactly this reason. The frozen residues are byte-for-byte
unchanged either way, so the bug was latent, not active.

**Four credential/payload DTOs kept a derived `Debug` through the move.**
`ProductInboundAttachment` carries a whole user upload and is a field of
`ProductSubmitTurnRequest`; `RebornAdminPutSecretRequest`/`…ProductRequest`
carry raw secret material in front of a port that takes `SecretString`;
`RebornAdminUserCreatedResponse` carries the one-time API token; and
`RebornAccountLoginLinkResponse.url` is a code-bearing credential whose own doc
comment promises it "must never be logged". Each now hand-writes `Debug`, the
same treatment `ProjectFsFile` already had in this crate, pinned by three tests.
Serde is untouched — no wire shape changes. Four mutations of the
implementations (not the expected literals) produce four assertion-level reds.

**Documentation that the move made false.**
- rustdoc: 8 broken intra-doc links → 0 (`cargo doc --no-deps --all-features`).
  The reviewer found 3; the other 5 are the same class.
- `product_contracts/CLAUDE.md`: nineteen → twenty-three shipped modules, the
  four new rows added, and the `admin_users`/`views` rows corrected — both still
  claimed their contents had stayed in product.
- `product/CLAUDE.md`: `ProductView` is no longer residue; the residue is the
  frozen operation inventory, which is the honest reason the dep survives.
- `Cargo.toml`: the `secrecy/serde` justification named a type carrying no
  secret. The three `Deserialize` bodies that actually force it are named now.
- `PROPOSAL.md` §6.1.3's allowed-deps line dropped `ironclaw_common`, so it
  agrees with `families/contracts.md` and with `product_contracts_allowed`.
- `product_wire.rs`'s `large_file` exemption is one line with `plan #7008`, so
  it satisfies the enforcer regex in `scripts/pre-commit-safety.sh`.
- `descriptors::query_on` states the one-payload conduit invariant.
- `fs_browse.rs` imports the project-fs DTOs from their owner instead of
  through the sibling module's re-export.
- CHECKLIST WS8 records `RebornSkillInfo`'s duplicate `source`/`source_kind`
  wire field — a real removal, deliberately not made inside a move.

Verification: fmt; clippy `-D warnings --all-targets --all-features` on the six
touched crates → clean; unfiltered tests for those six → 2841 passed / 0 failed
across 117 binaries; 4 tests added, none removed; rustdoc 0 warnings.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BenKurrek added a commit that referenced this pull request Aug 1, 2026
…cts (WS5) (#7002)

* refactor(contracts): invert webui + openai_compat onto product_contracts (WS5)

Both transports now compile against the product boundary instead of the
product crate: the wire DTOs they serialize, the request bodies they
deserialize, and the operation descriptor *types* they hold moved into
`ironclaw_product_contracts`, and every implementation stayed with its owner.

- `ironclaw_webui`: 228 -> 102 production `ironclaw_product::` symbols
- `ironclaw_reborn_openai_compat`: 23 -> 3 (7 -> 2 files)

New contracts modules: `descriptors`, `inbound_requests`, `product_wire`,
`workspace_views`; `admin_users` and `operator_llm` gained their wire DTOs.

The residue is structural, not shortfall: PROPOSAL §6.1.3 keeps product's
concrete command/view/capability constants in product as the frozen
inventory, and a route handler names the constant to call the surface. The
new `reborn_transport_product_boundary.rs` freezes the residue with per-entry
reasons, pins the moved vocabulary in contracts, and pins the inventory in
product so "shrinking" the residue by moving the inventory fails loudly.

Regression tests: `crates/ironclaw_architecture/tests/reborn_transport_product_boundary.rs`
(4 tests, incl. a scanner self-test and a non-vacuity assertion).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(contracts): resolve the ProductSurfaceFailure linchpin (WS2.2)

`ironclaw_extension_host` used `ironclaw_product`'s internal workflow error
as its own lifecycle error vocabulary across 19 production files — WS2.1's
recorded linchpin, blocking half the port-inversion residue and the layer
flip. Measured with `#[cfg(test)]` stripped, it constructs exactly six
variants (150 sites), all plain-`String` or unit, and none of the two
kernel-typed ones that kept the enum out of contracts.

The boundary half is now
`ironclaw_product_contracts::error::ProductOperationFailure`;
`ironclaw_product` keeps `ProductSurfaceFailure` unchanged in shape and
absorbs it with a total, payload-preserving `From`. The projection to
`ProductSurfaceError` is defined once, in contracts, and product's
`lifecycle_product_surface_error` delegates its six shared arms to it so the
two paths cannot drift. Only the logging stayed with each caller — contracts
may not log.

Narrowing the enum instead was rejected on evidence: `auth_continuation.rs`
matches all eight `TurnErrorCategory` values structurally and distinguishes
two the sanitized projection collapses, and constructs by matching
`TurnError` variants the projection cannot express — so narrowing is lossy
in a live auth path.

Unlocks `ProductConversationSubjectRouteResolver` (trait residue 6 -> 5, with
its route key and request type) and takes extension_host's files naming the
workflow error 19 -> 2. Corrects the two surviving residue reasons, which
named the error rather than the real blocker.

Regression coverage: nine crate-tier tests including the projection-agreement
pin and the `From` totality pin, plus two new architecture gates (frozen
residue files; the contract error names no kernel type), each verified by
negative probe. Extension-specificity allowlist shrinks 130 -> 129.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(product_wire): complete the residue list in the module header

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(workspace_views): state the real reason each read port stayed in product

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): close the three changed-coverage shapes on the relocated modules

Applied preemptively from #6998's CI experience, before the gate could fire.

(1) Relocated declaration modules emit no LCOV records. `workspace_views`,
`operator_llm`, `inbound_requests`, `descriptors`, and `product_wire` now carry
contract tests for their real executable content — chosen for the invariant, not
for the number: `ProjectFsFile`'s hand-written `Debug` (it keeps whole user files
out of diagnostics; re-deriving it compiles and silently re-opens the leak),
`FsMount::ALL` exhaustiveness, the `as_str`/`Display`/serde triples, and the
list-request builders.

(2) Two genuinely untested paths arrived with the DTOs and are tested, not
exempted: `validate_outbound_delivery_display_field` — the only fence between an
operator-supplied delivery-target label and the browser, now driven through both
the constructor and the `TryFrom` path for bidi-override, zero-width-joiner,
separator, control-character, whitespace and per-field byte-cap shapes — and the
`RebornOutboundPreferencesResponse` wire-shadow back-compat rule plus
`RebornAutomationState`'s degrade-to-`Unknown` deserializer.

(3) Diff-shape noise: two `webui_v2/handlers.rs` signatures carried inline
`ironclaw_product_contracts::product_wire::<Item>` paths; the longer one made
rustfmt reflow a one-line return type into three. Both are `use` imports now, so
that file's diff is imports plus exactly two production lines.

No test doubles are used, so the discard-the-argument vacuity mode has no
surface. Every test was red-green verified by mutating the implementation (nine
mutations, nine assertion-level reds); two earlier mutations that produced
compile errors were rewritten as behavior changes, because a compile error
proves the compiler caught it and not the test.

Regression tests: crates/ironclaw_product_contracts/src/{workspace_views,
operator_llm,inbound_requests,descriptors,product_wire}.rs

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(arch): apply the parent's scanner hardening to the WS2.2 half

The merge brought in WS2.1's review fixes (I/O errors fatal, comments and
strings stripped *before* `#[cfg(test)]` brace matching). Both apply verbatim
to `production_files_naming`, which this branch added after that review:

- An unreadable file was silently skipped, which is exactly how the frozen
  residue-file scan would go quietly vacuous. Now fatal, matching the three
  other readers in the file.
- The strip order was backwards. A `{` inside a comment or string literal can
  desynchronise the `#[cfg(test)]` brace matcher, so comments and strings go
  first. Re-probed both directions afterwards: a code reference still trips
  the gate, a comment mentioning the type (now with an unbalanced brace) still
  does not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): close the coverage-gate shapes on the WS2.2 slice

Applies the cross-slot lessons from WS2.1/WS2.3's coverage rounds to this
row's own new code, before the gate has to ask.

Pure-declaration modules gained real contract tests rather than waivers:

- `subject_route`: the port is held as `Arc<dyn _>` in five places, so object
  safety is a contract; a resolver is handed every field unswapped
  (`adapter_id`/`installation_id` are both string newtypes, so a swap would
  otherwise be silent); and an unconfigured route is absence, not failure.
  The double is **route-keyed, not fixed-answer** — two configured routes
  resolve to *different* subjects and a third resolves to `None`, so a
  resolver that ignored its argument could not pass. A fixed-answer double
  would have made all three assertions vacuous.
- `error`: `Display` is exercised for every variant, asserting each one keeps
  the text the LLM tool path forwards — `ProviderInstanceNotConfigured`
  carries the operator's exact `config set` remediation.
- `lifecycle_surface_error`: pinned against the contract's own projection
  (drift guard) *and* against absolute statuses (so both drifting together
  still fails).

`channel_config_unavailable` is extracted from a `map_err` closure because it
sat on the one path unreachable in test without fault-injecting the concrete
config service. Naming it makes the classification directly testable, and the
classification matters: a store failure is transient (retryable 503), never a
rejection (permanent 4xx) that would leave a correctly-configured channel
looking broken. The other 44 closures in this crate are pre-existing bodies
where only the type name changed (45 on the parent), so they are left alone
rather than churned on speculation.

Each new test was verified red-then-green by **mutating production code**, and
every mutation compiles cleanly so the red is an assertion failure rather than
the compiler catching the mutant:

- route key stops discriminating by conversation -> two routes collapse to one
  subject (`left: eng-subject, right: support-subject`)
- `Display` drops `{reason}` -> "rendered as ..., dropping ..."
- `lifecycle_surface_error` stops delegating -> "projection drifted for ..."
- store failure reclassified permanent -> "must be transient, got ..."

Scope is calibrated in the doc comments: the contracts-crate test pins the
port's shape and that it admits a per-route answer; it does not claim the
production resolver filters correctly — `channel_subject_routes`' own tests
(`foreign_adapter_or_installation_resolves_nothing`,
`malformed_config_json_fails_closed`) already own that.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(contracts): close the WS5 review round — scanner order, DTO redaction, doc drift

Review triage for #7002. Ten of the seventeen threads changed code or docs;
the rest are refuted or routed with evidence in their threads.

**The one real defect: the transport scanner's strip order was inverted.**
`product_symbols_in` ran `strip_cfg_test_blocks` on raw source, so a brace
inside a comment or string literal in a `#[cfg(test)]` block desynchronised the
byte-level depth walk and truncated the rest of the file. Regression test
first: the new case makes the scan return an *empty* set (proof it ran off the
end), then the order flips to match the sibling gate, which #6998's own triage
had already fixed for exactly this reason. The frozen residues are byte-for-byte
unchanged either way, so the bug was latent, not active.

**Four credential/payload DTOs kept a derived `Debug` through the move.**
`ProductInboundAttachment` carries a whole user upload and is a field of
`ProductSubmitTurnRequest`; `RebornAdminPutSecretRequest`/`…ProductRequest`
carry raw secret material in front of a port that takes `SecretString`;
`RebornAdminUserCreatedResponse` carries the one-time API token; and
`RebornAccountLoginLinkResponse.url` is a code-bearing credential whose own doc
comment promises it "must never be logged". Each now hand-writes `Debug`, the
same treatment `ProjectFsFile` already had in this crate, pinned by three tests.
Serde is untouched — no wire shape changes. Four mutations of the
implementations (not the expected literals) produce four assertion-level reds.

**Documentation that the move made false.**
- rustdoc: 8 broken intra-doc links → 0 (`cargo doc --no-deps --all-features`).
  The reviewer found 3; the other 5 are the same class.
- `product_contracts/CLAUDE.md`: nineteen → twenty-three shipped modules, the
  four new rows added, and the `admin_users`/`views` rows corrected — both still
  claimed their contents had stayed in product.
- `product/CLAUDE.md`: `ProductView` is no longer residue; the residue is the
  frozen operation inventory, which is the honest reason the dep survives.
- `Cargo.toml`: the `secrecy/serde` justification named a type carrying no
  secret. The three `Deserialize` bodies that actually force it are named now.
- `PROPOSAL.md` §6.1.3's allowed-deps line dropped `ironclaw_common`, so it
  agrees with `families/contracts.md` and with `product_contracts_allowed`.
- `product_wire.rs`'s `large_file` exemption is one line with `plan #7008`, so
  it satisfies the enforcer regex in `scripts/pre-commit-safety.sh`.
- `descriptors::query_on` states the one-payload conduit invariant.
- `fs_browse.rs` imports the project-fs DTOs from their owner instead of
  through the sibling module's re-export.
- CHECKLIST WS8 records `RebornSkillInfo`'s duplicate `source`/`source_kind`
  wire field — a real removal, deliberately not made inside a move.

Verification: fmt; clippy `-D warnings --all-targets --all-features` on the six
touched crates → clean; unfiltered tests for those six → 2841 passed / 0 failed
across 117 binaries; 4 tests added, none removed; rustdoc 0 warnings.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(ci): repoint the stale changed-coverage exemption the DTO move stranded

WS5 moved the `Reborn*` wire DTOs out of
`ironclaw_product/src/reborn_services/types.rs` (1890 -> 288 lines), which
left changed-coverage exemption #9 pointing at line 483 — past the new EOF.
The gate validates the manifest fail-closed before it scores anything, so a
single stranded entry aborted the whole run with
"exemption #9 names a line beyond current EOF (288)" and no coverage verdict
was produced at all.

The exempted line is the same struct field it always was — the non-executable
`pub gate_ref: Option<TurnGateRef>,` type position — which the deletions above
it moved to line 105. Repointed rather than deleted so the record of why that
line is uninstrumentable survives.

Replaying the manifest validation over all 71 entries against this tree now
reports zero problems (it was the only stranded entry; #7000's tree was
already clean).

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
BenKurrek added a commit that referenced this pull request Aug 2, 2026
…plit

The parent was force-pushed after its WS2.1 train squash-merged to main as
#6998, so the recorded merge base fell back to a50ad06 and 30 files
conflicted three-way against stale content. 16 of them this PR never touched
(blob-identical to the old parent tip) and take the parent verbatim; the rest
are the real reconciliation.

The parent's genuinely new work is #7002's WS5 inversion plus #6995/#6996 and
three WS2.2 follow-ups. Two of its edits landed on files this PR moves into
ironclaw_extension_manager, and both are ported to the new location rather
than dropped or resurrected at the old path.

One semantic conflict needed a decision. WS2.2 extracted the shared
post-install classifier `hosted_mcp_discovery_left_the_install_usable` so the
lifecycle service and the model-facing capability could not drift apart on
which activation failures still leave a usable install. It is `pub(crate)` in
a private module, and this PR moves *both* of its consumers out of the crate,
so `crate::hosted_mcp_manifest::...` stops resolving. Duplicating the two
string comparisons back into the manager would re-open exactly the drift the
parent closed, so the predicate is instead re-exported from
ironclaw_extension_host as a single named function — the crate's existing
private-module + selective `pub use` idiom — and both call sites reach the one
source of truth across the boundary.

Docs carrying dated amendments from both sides were unioned, not chosen
between: PROPOSAL keeps the Wave 1 truth audit's baseline bullet and this PR's
WS2.4 amendment, whose package figures were re-measured against the merged
tree (68 packages / 67 members entries, up one from the parent's 67 because
the rebase moved the baseline under it).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ilblackdragon added a commit that referenced this pull request Aug 2, 2026
…on_host (WS2.4) (#7003)

* refactor(contracts): invert extension_host's product-facing ports onto product_contracts (WS2.1)

`ironclaw_extension_host` sits below product in the target tree, so a
product-side port it satisfies must be declared at the product boundary and
implemented downward — never declared inside `ironclaw_product` and reached
upward. This moves every such port that `ironclaw_product_contracts` may
legally name, and dissolves the product re-export facade for the extension
host.

Nine port families move (definitions only; every implementation stays with its
owner, PROPOSAL §6.1.4): delivery resolution + reply context, account-connection
status + setup descriptors, channel config, the view-provider conduit, command
context + actor-role admission, gate-prompt enrichment, the lifecycle product
service, the admin-user directory, and the operator tool catalog. Product keeps
`DeliveryCoordinator`, `NoReplyContext`, `ExtensionAccountSetupRegistry`,
`UnsupportedLifecycleProductService`, `RejectingAdminUserService`,
`UnavailableRebornViewProvider`, `DirectConversationCommandAdmission`, the
frozen `Reborn*` wire DTOs, and the inbound-action ledger.

extension_host's product symbol usage drops 146 -> 62 across 46 -> 35
production files. The edge itself does not die here and could not: the
survivors are `channel_host.rs`'s construction of product's concrete assembly,
the `extension_manager` split inventory, `product::adapter_registry`, and the
named strays — each owned by a later WS2 row. Six ports also could not move,
all for one mechanical reason: `product_contracts` may depend only on
`host_api` + `extension_contracts`, so a signature naming `ironclaw_auth`,
`ironclaw_threads`, `ironclaw_turns`, or `ironclaw_conversations` cannot be
declared there. `ProductSurfaceFailure` is the linchpin — extension_host uses
product's *internal* workflow error as its own lifecycle error vocabulary in 19
files, and it carries `ironclaw_turns::TurnError`.

Regression cover: `reborn_extension_host_port_inversion.rs` pins the nine moved
ports where they landed and holds the six-entry residue shrink-only, with the
per-entry reason each could not move; a new product-declared port implemented
by extension_host fails the build. The moved typed-token tests travel with
their code and `ActionFingerprintKey` gains the coverage it lacked.

Enumerating gates, all update-never-relax: the composition pub-use snapshot
gains one line (two names re-sourced from `product_contracts`, so one `pub use`
splits into three); the extension-specificity allowlist, the struct/test-support
ratchet, the §11.2.7 include inventory, the `ProductSurface` method freeze, and
`LAYER_MATRIX_EXCEPTIONS` (13) are all untouched — extension_host carries no
layer-matrix exception and never did, since both crates are `products`-layer.

`secrecy` joins `product_contracts` with a manifest comment: `AdminUserService`
takes secret material and `AdminCreatedUser` carries a one-time token, both
`SecretString`. It is a value wrapper, not a framework/driver/runtime client.

CHECKLIST WS2 row 1 ticked with the four dispositions the lead sheet did not
predict.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): cover the moved port surfaces and close the impl-scanner bracket hole

Two follow-ups on the WS2.1 port inversion, both found by measuring rather
than assuming.

**Coverage of the surfaces this PR created.** `cargo llvm-cov` over
`ironclaw_product_contracts` showed the relocated bodies had no crate-tier
coverage of their own: `ProductCommandContext::from_envelope`,
`AdminUserRole::is_admin`, `AccountConnectionStatusError::new`,
`ChannelConnectionNoticePolicy::generic`, the bounded-token
`TryFrom`/`AsRef`/`Display` arms, and — the one that matters most — the two
`LifecycleProductService` **default** method bodies, which every production
implementor overrides, so nothing exercised the fail-closed defaults. Each is
now tested at its contract meaning, not for the line count: bundle import
defaults to `InvalidRequest` rather than silently succeeding; activation errors
default to none so the wire field stays absent; a non-command envelope is
rejected as an invalid request rather than an internal error; a token that
deserializes runs the same validation as its constructor; the generic notice
policy names the channel in all five notices and does not collapse them into
one string. Every added production line in the new modules is now covered.

**The scanner had a hole the review caught, and it was real.**
`implemented_trait_names` closed the impl's generic-parameter list at the first
`>`. For `impl<T: Iterator<Item = X>> Port for Host<T>` that `>` closes
`Iterator`, leaving `> Port` — not an identifier, so the impl was dropped and a
new product-defined port could have entered `extension_host` without tripping
the shrink-only gate. Now closed by balancing, with `->` inside a bound
(`impl<F: Fn(&str) -> bool>`) excluded from the count, and both shapes added to
the scanner self-test — which fails without the fix. Re-verified after the fix:
the residue is still exactly the six frozen entries, so the wider scan found no
previously hidden implementation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(arch): make the port-inversion scanner fail loud, and reconcile the doc counts

Review triage on #6998. Four findings taken, four rejected with evidence in the
thread; the taken ones are all about the gate telling the truth.

**The scanner could pass on an incomplete scan.** `rust_files` returned early on
a `read_dir` error and dropped per-entry errors through `.flatten()`, and
`traits_implemented_by` skipped any file it could not read. A permission or
transient I/O error in CI would have thinned the input and turned the ratchet
green while enforcing nothing — the exact failure class this file exists to
catch. Every I/O error is now fatal.

**`#[cfg(test)]` blocks were located by raw brace bytes.** A `{` inside a
comment or string literal in a gated block desynchronizes the depth count and
either leaks a test-only `impl` into the production set or swallows the
production code that follows it. Comments and strings are now stripped first;
`cfg_test_stripping_survives_braces_in_comments_and_strings` is the pin, and it
fails with the old composition (verified by reverting the order and watching it
go red). The doc comment now also states why `#[cfg(feature = "test-support")]`
is deliberately *not* stripped: that feature compiles into a real build, so an
`impl` behind it is a genuine normal-dependency edge, unlike `#[cfg(test)]`.

**The prose counts had drifted.** Eleven port declarations moved, not nine —
nine that `extension_host` implements (the pinned `INVERTED_PORTS`) plus
`AdminUserService` and `RebornOperatorToolCatalog`, which it only consumes and
composition implements. CHECKLIST, both CLAUDE files, and the module-count line
now agree and all defer to the architecture test as the enforced inventory.
`families/contracts.md` also still listed `ironclaw_common` in the family-level
dependency bullet; that is the second of the two places, now corrected too.

**One mismatch recorded rather than fixed.** `LifecycleProductService::
import_extension_bundle`'s default said "unavailable" while returning
`InvalidRequest`/400. The move carried both verbatim; changing the code changes
an HTTP status on a live route, which does not belong in a move-shaped PR. The
doc now describes what the code does, names the discrepancy, and points at the
test that pins today's behavior so a silent flip is impossible.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(contracts): state the module count as shipped-modules-plus-dev-seam

The count line said 'seventeen modules' while `src/lib.rs` carries eighteen
`pub mod` declarations — the difference is `test_support`, which is gated
behind `#[cfg(any(test, feature = "test-support"))]` and is deliberately
absent from the table above it. Saying 'seventeen shipped modules plus the
dev-only test_support' makes the table and the manifest agree on inspection
instead of looking like drift.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(contracts): resolve the ProductSurfaceFailure linchpin (WS2.2)

`ironclaw_extension_host` used `ironclaw_product`'s internal workflow error
as its own lifecycle error vocabulary across 19 production files — WS2.1's
recorded linchpin, blocking half the port-inversion residue and the layer
flip. Measured with `#[cfg(test)]` stripped, it constructs exactly six
variants (150 sites), all plain-`String` or unit, and none of the two
kernel-typed ones that kept the enum out of contracts.

The boundary half is now
`ironclaw_product_contracts::error::ProductOperationFailure`;
`ironclaw_product` keeps `ProductSurfaceFailure` unchanged in shape and
absorbs it with a total, payload-preserving `From`. The projection to
`ProductSurfaceError` is defined once, in contracts, and product's
`lifecycle_product_surface_error` delegates its six shared arms to it so the
two paths cannot drift. Only the logging stayed with each caller — contracts
may not log.

Narrowing the enum instead was rejected on evidence: `auth_continuation.rs`
matches all eight `TurnErrorCategory` values structurally and distinguishes
two the sanitized projection collapses, and constructs by matching
`TurnError` variants the projection cannot express — so narrowing is lossy
in a live auth path.

Unlocks `ProductConversationSubjectRouteResolver` (trait residue 6 -> 5, with
its route key and request type) and takes extension_host's files naming the
workflow error 19 -> 2. Corrects the two surviving residue reasons, which
named the error rather than the real blocker.

Regression coverage: nine crate-tier tests including the projection-agreement
pin and the `From` totality pin, plus two new architecture gates (frozen
residue files; the contract error names no kernel type), each verified by
negative probe. Extension-specificity allowlist shrinks 130 -> 129.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(arch): apply the parent's scanner hardening to the WS2.2 half

The merge brought in WS2.1's review fixes (I/O errors fatal, comments and
strings stripped *before* `#[cfg(test)]` brace matching). Both apply verbatim
to `production_files_naming`, which this branch added after that review:

- An unreadable file was silently skipped, which is exactly how the frozen
  residue-file scan would go quietly vacuous. Now fatal, matching the three
  other readers in the file.
- The strip order was backwards. A `{` inside a comment or string literal can
  desynchronise the `#[cfg(test)]` brace matcher, so comments and strings go
  first. Re-probed both directions afterwards: a code reference still trips
  the gate, a comment mentioning the type (now with an unbalanced brace) still
  does not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): close the changed-coverage holes the port move opened

CI's changed-coverage gate failed on the WS2.1 move, exactly where a
move-shaped diff is expected to: relocated bodies read as added production
lines. Every hole is now closed with a test. One line is exempted, with its
callers named.

**Five relocated port modules had no LCOV record at all.** `delivery`,
`channel_config`, `operator_tools`, `prompt_source`, and `views` are pure
declarations, so rustc emitted no source record and the gate reported them
absent. Each now carries a contract test rather than a waiver, and the
properties they pin are the ones these ports actually owe:

- **object safety** for all seven traits — every consumer holds them as
  `Arc<dyn _>`, so a signature change that breaks dyn-safety now fails at the
  contract instead of at the far-away wiring site;
- **argument pass-through and ordering** for the delivery ports — `reply_context`
  takes extension id, installation id, and conversation fingerprint as three
  bare strings, so nothing but a test stops a transposition turning into a
  silent mis-delivery (this is the identity-mixup risk review raised; the types
  stay verbatim, the ordering is now pinned);
- **absence without error** — an unresolved channel, an empty channel-config
  field set, an empty operator tool catalog, and a missing approval-prompt
  context are all normal outcomes that must not be expressible only as failures;
- **caller scoping** on the operator catalog, whose `caller` parameter is the
  #5459 disclosure control;
- **`next_cursor` omission** on an unpaginated view page — serializing `null`
  would make every unpaginated view look paginated to the browser.

**Two genuinely untested error paths in `extension_host`, both fail-closed
seams the move touched.** `AccountConnectionStatusSource::connected` now has
coverage proving it fails *closed* on a pairing-backend outage (activation must
not proceed on an unknown connection state) and *sanitized* (the test asserts
the driver, host, and port do not appear in the product-facing error). The
lifecycle output-serialization mapping moved out of an inline closure into a
named `lifecycle_output_decode_error` so the mapping is reachable from a test:
the failure is defensive, but *what it maps to* is a live contract — the model
gets `OutputDecode` and never the serde error, which can quote projection
contents.

**A dead branch arm.** `validate_typed_token` guards `c == '\0' || c.is_control()`
and only the second arm was exercised. NUL has its own arm because a token with
an embedded NUL truncates at a C boundary rather than merely looking odd.

**Diff shape.** The remaining reports were an artifact of relocating types
inline: a fully-qualified `ironclaw_product_contracts::<mod>::<Item>` in a
signature turns an untouched line into a changed one. Those 17 files now import
the symbol like every other, which shrinks the diff, restores the crate's
prevailing style, and drops the lines out of the gate's denominator because a
`use` line is uninstrumentable by construction.

**One exemption, with evidence.** `factory/test_support.rs`'s
`channel_config_service` accessor: the repoint collapsed its signature onto one
line, and the merged lcov does not attribute its two integration callers back
to the composition bucket build. Both callers are named in the manifest, the
service and the port contract are covered by tests added here, and it is filed
under the same #6963 lane-attribution lane as the WS1 entries above it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): make the catalog and view doubles discriminate on their arguments

Review caught two tests of mine that asserted the double's behavior rather
than the contract, and it was right about both.

`EmptyCatalog` ignored `caller` and always returned an empty vector, so
`the_catalog_is_caller_scoped...` would have passed against a production
catalog that disclosed every user's private installs — the exact leak the
`caller` parameter exists to close (#5459 P1). It is now backed by an
ownership-filtering double, two callers, one tenant-shared tool and one private
tool each, asserting both directions of isolation and that the answer *can*
differ by caller. `OneRowView::query` ignored `_caller` and `_params` and the
test only checked the cursor; the provider now echoes all three conduit
arguments and the test asserts all three.

Both were verified red-then-green rather than assumed: dropping the caller
filter fails the catalog tests, and dropping params from the echo fails the
view test. (My first attempt at the view mutation substituted the expected
literals and passed — a reminder that a mutation which doesn't fail proves
nothing about the mutation, only about the mutant.)

The over-claim went into the PR body too, and is corrected there: a contracts
crate can pin that the port *hands the implementation the caller* and that its
shape admits a per-caller answer. It cannot pin that production filters
correctly — that is composition's implementation and composition's test. The
doc comments now say so instead of implying the stronger claim.

Also lands the CHECKLIST note this PR earned for the rest of Wave 2/3: a
move-shaped PR fails the changed-coverage gate on its first CI run, in three
distinct shapes needing three different answers, with the two mechanical habits
that shrink all three.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(extensions): split ironclaw_extension_manager out of extension_host (WS2.4)

The extension host held two jobs: lifecycle authority (the only writer of
installation state, ingress verification, activation transactions) and the
extension-management product face that arrived with #6616/#6669. PROPOSAL
§6.8.3 splits the second into its own products-layer crate so the first can
move below product in WS2's layer flip.

Six of the nine inventory items moved; three are structurally blocked and
each is recorded with its measurement. extension_host production files
naming ironclaw_product: 20 -> 13. Port-inversion residue 5 -> 4.

Behavior-free: modules move, imports repoint, one 100-line product
projection is extracted from channel_config.rs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): close the coverage-gate shapes on the WS2.2 slice

Applies the cross-slot lessons from WS2.1/WS2.3's coverage rounds to this
row's own new code, before the gate has to ask.

Pure-declaration modules gained real contract tests rather than waivers:

- `subject_route`: the port is held as `Arc<dyn _>` in five places, so object
  safety is a contract; a resolver is handed every field unswapped
  (`adapter_id`/`installation_id` are both string newtypes, so a swap would
  otherwise be silent); and an unconfigured route is absence, not failure.
  The double is **route-keyed, not fixed-answer** — two configured routes
  resolve to *different* subjects and a third resolves to `None`, so a
  resolver that ignored its argument could not pass. A fixed-answer double
  would have made all three assertions vacuous.
- `error`: `Display` is exercised for every variant, asserting each one keeps
  the text the LLM tool path forwards — `ProviderInstanceNotConfigured`
  carries the operator's exact `config set` remediation.
- `lifecycle_surface_error`: pinned against the contract's own projection
  (drift guard) *and* against absolute statuses (so both drifting together
  still fails).

`channel_config_unavailable` is extracted from a `map_err` closure because it
sat on the one path unreachable in test without fault-injecting the concrete
config service. Naming it makes the classification directly testable, and the
classification matters: a store failure is transient (retryable 503), never a
rejection (permanent 4xx) that would leave a correctly-configured channel
looking broken. The other 44 closures in this crate are pre-existing bodies
where only the type name changed (45 on the parent), so they are left alone
rather than churned on speculation.

Each new test was verified red-then-green by **mutating production code**, and
every mutation compiles cleanly so the red is an assertion failure rather than
the compiler catching the mutant:

- route key stops discriminating by conversation -> two routes collapse to one
  subject (`left: eng-subject, right: support-subject`)
- `Display` drops `{reason}` -> "rendered as ..., dropping ..."
- `lifecycle_surface_error` stops delegating -> "projection drifted for ..."
- store failure reclassified permanent -> "must be transient, got ..."

Scope is calibrated in the doc comments: the contracts-crate test pins the
port's shape and that it admits a per-route answer; it does not claim the
production resolver filters correctly — `channel_subject_routes`' own tests
(`foreign_adapter_or_installation_resolves_nothing`,
`malformed_config_json_fails_closed`) already own that.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(ws2.4): date the two row corrections and quote the text they replace

The CHECKLIST disposition named the contradiction without quoting the
inventory line it corrects or carrying a date; PROPOSAL §6.8.3 pointed at
it without the verbatim text. Both now quote both sides.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(extension_host): cover the log-sanitization guard; exempt the type-position residue

CI's second changed-coverage run came back at 99.32% line / 100% branch, with
one uncovered line and six files reporting "contributed no instrumented lines".
Two different problems, two different answers.

**The uncovered line was coverable, so it is covered.**
`lifecycle_output_decode_error`'s `tracing::debug!` body never ran under test:
with no subscriber installed `tracing` short-circuits on the null dispatcher,
so the message literal is a region that cannot be reached. The fix is not a
waiver — it is the subscriber. The test now installs a DEBUG-level
`tracing_subscriber::fmt` over a shared writer (the pattern
`ironclaw_turns/tests/agent_loop_host_contract.rs` already uses) and asserts
*both* halves of the guard's contract: the model gets `OutputDecode` and never
the serde error, **and** the serde detail is not simply dropped — it reaches
the debug log, which is where an operator diagnoses it from. Without the
subscriber a test cannot tell "logged the detail" from "discarded it", which is
the whole point. `tracing-subscriber` joins this crate's dev-dependencies for
that, with a manifest comment saying why.

**The six files are the type-position residue, and it is precedented.**
Deleting `ironclaw_product`'s re-exports forced every signature naming a moved
symbol to be rewritten; where the name sits in a *type* position — a struct
field, a function parameter, a struct-literal field's enum path — the line
changes but LLVM emits no coverage region, so it can never be covered. Nine
exact lines across six files, each entry naming the construct, filed under the
same #6963 lane the four WS1 entries use. Every line was re-read against the
source before the entry was written; none is a guess.

The balance for the PR as a whole: ten exemption lines, all type positions or
one lane-attribution accessor, against ~30 tests written for surfaces that
genuinely lacked them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(coverage): exempt the tracing message literal, with the evidence that it is an artifact

Last line on the changed-coverage gate, and the obvious reading of it is wrong.

`extension_lifecycle_capabilities.rs:217` is the message string inside a
`tracing::debug!`. It reads as uncovered — but the event body demonstrably
executes: the DEBUG-subscriber test added in the previous commit asserts the
rendered log contains that exact message, and it passes, including in the
`extension-operator` bucket, which is green.

The proof it is an attribution artifact rather than a dead path comes from that
bucket's own tracefile (run 30689416105, `bucket-extension-operator.lcov`):

  line 213 (fn signature)       hits 1
  line 214 (macro invocation)   hits 1
  line 217 (message literal)    hits 0
  line 219 (error construction) hits 1
  line 220 (closing brace)      hits 1

The function ran, the macro ran, the error was built. What LLVM does not count
is the literal: `tracing` bakes the message into the callsite's `static`
`Metadata`, so the region on that line belongs to a static initializer and is
never attributed to an executed path. Nothing short of changing the log target
moves that counter, and changing a log target is a behavior change this
move-shaped PR will not make. Every `tracing::debug!` in the workspace has the
same shape; they only escape this gate because their lines are not in a diff.

Verified by replaying the gate locally against CI's own merged lcov with this
entry in place: changed line coverage 100.00% (147/147), changed branch
coverage 100.00% (10/10).

The test stays. It is what proves the 0 is an artifact, and it still pins the
guard's real contract: the model gets `OutputDecode` and never the serde error,
and the detail reaches the debug log rather than being dropped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(extension-host): prove the transient cause survives the sanitized 503

The lifecycle warning is the entire reason this crate kept a local projection
wrapper rather than calling the contract's `From` directly — and that claim
was asserted in a doc comment and nowhere else.

`tracing` short-circuits on the null dispatcher, so under a plain unit test the
macro body never runs and a test cannot distinguish "logged the cause" from
"dropped it" — which is exactly the distinction that matters when the 503 body
is sanitized. Installing a scoped subscriber (`with_default`, so parallel tests
are unaffected) over a shared writer, following the pattern
`ironclaw_turns/tests/agent_loop_host_contract.rs` established, makes both
halves of the guard's contract assertable, and both are asserted:

- the caller's 503 is sanitized — the cause appears nowhere in the serialized
  `ProductSurfaceError`; and
- the cause is not discarded — it reaches the warning, with its stable message.

A second test pins the other direction: a rejection carries no operational
cause and must not spend a warning, so "log everything" cannot satisfy the
first test.

Both verified red-then-green by mutating production code, compiling cleanly so
the red is an assertion:
- drop the warning -> "the transient cause must survive in the log, got \"\""
- warn on every variant -> "a rejection must not emit the transient warning,
  got ... invalid binding request: bad package ref"

`tracing-subscriber` joins `[dev-dependencies]` and the `Cargo.lock` delta is
**zero** — it was already resolved for the workspace.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(coverage): recapture the extension_host floor and ratchet the manager (WS2.4)

Both numbers come from this PR's own merged coverage artifact
(reborn-integration-coverage-merged, run 30689658637), read through the
same aggregation that enforces the file. extension_host regains its
covered-line floor at 19907/23467 = 84.83% (the ratio ROSE across the
split); the manager is ratcheted from birth at 4602/5440 = 84.60%.

Verified by running the enforcing ratchet against the artifact: both
entries PASS, 17 crates pass, exit 0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(contracts,extension-host): preserve the acquire cause and pin every HostApiError projection

Review triage for #7000.

- `import_bundle`'s decode-limiter `map_err(|_| ...)` discarded the
  `AcquireError`. The mapping is now a named `map_import_decode_acquire_error`
  that logs the bound source before mapping. Named rather than inlined so it is
  reachable from a test: nothing in the workspace calls `Semaphore::close`, so
  an inline closure would be a permanently uncovered branch that the
  changed-line coverage gate could only accept as a standing exemption. New
  regression test builds a genuine `AcquireError` from a closed semaphore and
  asserts the failure is `Transient` (retryable), not a client mistake.

- `From<HostApiError> for ProductOperationFailure` was pinned by one variant.
  It now enumerates all ten, asserts each carries its own rendering (so the
  cause cannot be flattened at the boundary) and projects to a 400, and adds an
  exhaustive `host_api_error_tag` match so a new `HostApiError` variant stops
  compiling the test instead of inheriting the blanket mapping silently.
  `InvariantViolation` is pinned as-is, not reclassified: the mapping mirrors
  product's pre-existing `From<HostApiError> for ProductSurfaceFailure` and
  changing it is a behavior change this slice does not own.

Red-then-green proved by mutating the code under test: InvariantViolation ->
Transient, flattening the reason text, and Transient -> InvalidBindingRequest
each fail the corresponding assertion.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(architecture,ci): close the review gaps on the extension_manager split

Review triage for #7003. All four are artifacts this PR introduced, not moved code.

- The new `ironclaw_extension_manager` boundary rule forbade
  `"ironclaw_reborn_cli"`, which is the crate DIRECTORY. `forbidden` entries are
  compared against `cargo metadata` package names and the CLI's package is
  `ironclaw`, so the entry could never fire — the edge it named was unguarded.
  Fixed, and pinned: `boundary_rule_names_are_package_names_not_crate_directories`
  flags any forbidden entry that is not a package but IS a directory under
  `crates/`. That discrimination matters — ~60 entries legitimately name retired
  v1 crates (`ironclaw_legacy`, `ironclaw_engine`, `ironclaw_gateway`,
  `ironclaw_tui`, `ironclaw_storage`) as reintroduction pins, and those have no
  directory. `ironclaw_reborn_cli` was the only entry in all 693 that had one.

- `production_files_naming` took a flat `files.len() >= 10` to accommodate the
  manager, which silently dropped the host's vacuous-scan guard from >20 to 10.
  The same diff had already parameterized `traits_implemented_by` for exactly
  this reason. Parameterized to match: host 21, manager 10.

- `classify-test-scope.sh` gained a `crates/ironclaw_extension_manager/*` arm
  with no self-test case, so a manager-only diff classifying
  `has_reborn_tests=false` would have gone unnoticed — the failure #6947 records
  for the stale `crates/ironclaw_product_*/*` arm. Case added.

- `coverage-floor.toml`'s "9.7k lines moved" explained an instrumented-line
  delta of 3,102 with a source-line figure. Both units are now stated with their
  measurements (source: 57,464 -> 47,794 in the host, 9,979 in the manager;
  instrumented: 26,569 -> 23,467 against 5,440) and why they do not reconcile.

Red-then-green proved by mutating the code under test: reverting the forbidden
entry to the directory spelling fails the new meta-test with the fix-it message;
removing the manager glob from the classifier fails the new self-test case
(has_reborn_tests=false); raising the manager's file floor to 40 fails only the
manager call site, proving the floor is per-call-site and consumed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(architecture,extensions): close the paranoid-architect review findings on the WS2.4 split

Review pass over #7003 (four parallel deep reviews; no Critical/High — the
move itself verified behavior-free). Everything found, fixed here:

Gate hardening (crates/ironclaw_architecture/tests):
- ratchet_support gains cfg_test_only_files: files reachable only through
  #[cfg(test)] mod chains (incl. #[path] overrides) are classified test code.
  channel_host/e2e_auth_challenge.rs — a fake AuthChallengeProvider impl
  wearing a production filename — no longer counts toward any residue row,
  implementor pin, or error-vocabulary floor. Pinned by a real-tree test that
  was red before the #[path] resolution landed.
- Trait matching is qualified by a whole-token crate reference (names_crate),
  so a name-colliding local trait can no longer satisfy an implementor pin,
  and a manifest rename of ironclaw_product can no longer blind the manager
  residue scan (metadata tie: dep exists iff the residue list is non-empty,
  never renamed).
- The manager gets its own product-defined-trait residue freeze (twin of the
  host's, frozen at ExtensionCredentialSetupService).
- each_half_of_the_split_kept_its_own_job: authority checks are symmetric
  across file/directory spellings and back every module with a content
  witness, so an empty stub cannot satisfy retention.
- untrusted_ingress_paths scan roots fail loudly on a missing root instead of
  silently dropping a tree from the guard.
- Fork-check message names its two-crate scope.
All new checks probed red-for-the-right-reason and reverted (hollow witness,
product alias, stale scan root, authority-as-directory, unguarded secret).

Manifest hygiene:
- extension_host drops the ed25519-dalek dep orphaned when ironhub moved.
- Ten manager deps used only by tests/the test_support fixture leave the
  production graph: fixture deps become test-support-gated optionals, pure
  test deps move to [dev-dependencies]. All three build shapes verified.

Manager/host code:
- channel_config: the pub resolved_manifest widening is narrowed to a
  declares_admin_configuration() boolean — the manifest read stays internal.
- admin_configuration view: secret field values are redacted in render_group
  (same defense-in-depth as render_state), with a sentinel regression test;
  the service-error table test now pins code/kind beside status/retryable.

Docs (single-source-of-truth):
- families/extensions.md confesses the direct auth/host_runtime deps and the
  transitional dep tail the four-crate target does not name.
- The residue characterization says what the list actually holds: DTOs,
  capability-id constants, and two port-inversion residues.
- 20 -> 13 becomes 20 -> 12 (the 13th was the cfg(test)-only fixture);
  coverage-floor/CHECKLIST stale "recapture owed" drafts corrected to the
  shipped recapture; line counts de-precisioned; stale exemption comment
  repointed to the manager.

Verification: architecture 143/0; manager 64/0 (--all-features);
extension_host 388/0 (--all-features); cargo check --workspace --all-targets
--all-features 0 errors / 0 warnings; clippy -D warnings clean on all three
touched crates; both CI script self-tests pass; cargo metadata --locked clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(coverage,architecture): close the human review on the WS2.4 split

Review findings from @serrrfirat on #7003. The first one was blocking CI
outright.

**The coverage exemption did not move with its file (HIGH).**
`extension_lifecycle_capabilities.rs` left `ironclaw_extension_host` for
`ironclaw_extension_manager` in this PR; its changed-coverage exemption kept
naming the old path. That is not cosmetic staleness — the manifest validator
is fail-closed on it, so the whole changed-coverage gate aborts with **no
verdict at all** rather than reporting a number. Reproduced on this branch
before the fix:

    GATE ERROR: exemption #71 names stale path:
      crates/ironclaw_extension_host/src/extension_lifecycle_capabilities.rs

exactly the entry index the reviewer named. Path repointed to the manager and
the line corrected 217 -> 218 (217 was the `?error,` field, not the message
literal the reason describes; the off-by-one was fixed on the parent). Whole
manifest re-validated: **71 entries, no stale paths, no lines past EOF.**

**Direct `#[cfg(test)]` module seeding was untested.** Confirmed empirically
rather than by reading: deleting the seeding loop from `cfg_test_only_files`
left the only in-tree pin green (9 passed), because its chain starts at
`e2e_tests.rs` — already seeded by the `*_tests.rs` name rule — and reaches
its child through an explicit `#[path]`. So neither the `cfg(test)` gate nor
default `<dir>/<name>.rs` resolution was exercised, and a production-named
file declared `#[cfg(test)] mod fixture;` could have become countable
silently. Added `direct_cfg_test_module_and_default_child_are_test_only` on a
synthetic tree covering both shapes plus the negative case; it goes red under
that same deletion.

**Crate contracts contradicted the move.** The CLI's exhaustive
`[dependencies]` inventory omitted `ironclaw_extension_manager` (and, found
while checking, `ironclaw_product_contracts` and `ironclaw_extension_contracts`
— all three added by this layer). The product-contract docs still said
`LifecycleProductService`, `ChannelConfigProductService` and
`RebornViewProvider` are implemented by `ironclaw_extension_host`, while this
branch's own `INVERTED_PORT_IMPLEMENTORS` says `ironclaw_extension_manager`.
Reconciled toward the enforced pin in `reborn_cli/AGENTS.md`,
`product_contracts/CLAUDE.md` (now a per-port implementor table, and citing
the constant by its real name), `lifecycle_service.rs`, `views.rs`,
`channel_config.rs`, and `crates/AGENTS.md` — the last of which the review did
not flag but was stale the same way.

**The production-source walker is centralized — for the two ratchets named.**
`ratchet_support::production_rust_files` now owns the fatal walk, the
name/directory exclusions and the `cfg_test_only_files` subtraction, and both
`reborn_extension_host_port_inversion.rs` and `reborn_extension_manager_split.rs`
delegate to it. The reviewer's concern was already realized rather than
hypothetical: the two walkers **had** drifted — one skipped `node_modules` and
the other did not. ~19 other ratchets still carry their own walk; migrating
them belongs in a dedicated change against `ratchet_support`, not in a crate
split, and that is recorded at the new helper and at the call site.

Verification: `cargo fmt --check` clean; `cargo clippy -p ironclaw_architecture
-p ironclaw_product_contracts -p ironclaw_extension_manager -p
ironclaw_extension_host --all-targets --all-features -- -D warnings` clean;
`cargo test -p ironclaw_architecture` 28 binaries green, 0 failed;
`cargo check --workspace --all-targets --all-features` clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: BenKurrek <benjaminkurrek@gmail.com>
Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com>
pull Bot pushed a commit to bryanwills/ironclaw that referenced this pull request Aug 3, 2026
….2, WS2.4, WS5) (nearai#7018)

* refactor(contracts): invert extension_host's product-facing ports onto product_contracts (WS2.1)

`ironclaw_extension_host` sits below product in the target tree, so a
product-side port it satisfies must be declared at the product boundary and
implemented downward — never declared inside `ironclaw_product` and reached
upward. This moves every such port that `ironclaw_product_contracts` may
legally name, and dissolves the product re-export facade for the extension
host.

Nine port families move (definitions only; every implementation stays with its
owner, PROPOSAL §6.1.4): delivery resolution + reply context, account-connection
status + setup descriptors, channel config, the view-provider conduit, command
context + actor-role admission, gate-prompt enrichment, the lifecycle product
service, the admin-user directory, and the operator tool catalog. Product keeps
`DeliveryCoordinator`, `NoReplyContext`, `ExtensionAccountSetupRegistry`,
`UnsupportedLifecycleProductService`, `RejectingAdminUserService`,
`UnavailableRebornViewProvider`, `DirectConversationCommandAdmission`, the
frozen `Reborn*` wire DTOs, and the inbound-action ledger.

extension_host's product symbol usage drops 146 -> 62 across 46 -> 35
production files. The edge itself does not die here and could not: the
survivors are `channel_host.rs`'s construction of product's concrete assembly,
the `extension_manager` split inventory, `product::adapter_registry`, and the
named strays — each owned by a later WS2 row. Six ports also could not move,
all for one mechanical reason: `product_contracts` may depend only on
`host_api` + `extension_contracts`, so a signature naming `ironclaw_auth`,
`ironclaw_threads`, `ironclaw_turns`, or `ironclaw_conversations` cannot be
declared there. `ProductSurfaceFailure` is the linchpin — extension_host uses
product's *internal* workflow error as its own lifecycle error vocabulary in 19
files, and it carries `ironclaw_turns::TurnError`.

Regression cover: `reborn_extension_host_port_inversion.rs` pins the nine moved
ports where they landed and holds the six-entry residue shrink-only, with the
per-entry reason each could not move; a new product-declared port implemented
by extension_host fails the build. The moved typed-token tests travel with
their code and `ActionFingerprintKey` gains the coverage it lacked.

Enumerating gates, all update-never-relax: the composition pub-use snapshot
gains one line (two names re-sourced from `product_contracts`, so one `pub use`
splits into three); the extension-specificity allowlist, the struct/test-support
ratchet, the §11.2.7 include inventory, the `ProductSurface` method freeze, and
`LAYER_MATRIX_EXCEPTIONS` (13) are all untouched — extension_host carries no
layer-matrix exception and never did, since both crates are `products`-layer.

`secrecy` joins `product_contracts` with a manifest comment: `AdminUserService`
takes secret material and `AdminCreatedUser` carries a one-time token, both
`SecretString`. It is a value wrapper, not a framework/driver/runtime client.

CHECKLIST WS2 row 1 ticked with the four dispositions the lead sheet did not
predict.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): cover the moved port surfaces and close the impl-scanner bracket hole

Two follow-ups on the WS2.1 port inversion, both found by measuring rather
than assuming.

**Coverage of the surfaces this PR created.** `cargo llvm-cov` over
`ironclaw_product_contracts` showed the relocated bodies had no crate-tier
coverage of their own: `ProductCommandContext::from_envelope`,
`AdminUserRole::is_admin`, `AccountConnectionStatusError::new`,
`ChannelConnectionNoticePolicy::generic`, the bounded-token
`TryFrom`/`AsRef`/`Display` arms, and — the one that matters most — the two
`LifecycleProductService` **default** method bodies, which every production
implementor overrides, so nothing exercised the fail-closed defaults. Each is
now tested at its contract meaning, not for the line count: bundle import
defaults to `InvalidRequest` rather than silently succeeding; activation errors
default to none so the wire field stays absent; a non-command envelope is
rejected as an invalid request rather than an internal error; a token that
deserializes runs the same validation as its constructor; the generic notice
policy names the channel in all five notices and does not collapse them into
one string. Every added production line in the new modules is now covered.

**The scanner had a hole the review caught, and it was real.**
`implemented_trait_names` closed the impl's generic-parameter list at the first
`>`. For `impl<T: Iterator<Item = X>> Port for Host<T>` that `>` closes
`Iterator`, leaving `> Port` — not an identifier, so the impl was dropped and a
new product-defined port could have entered `extension_host` without tripping
the shrink-only gate. Now closed by balancing, with `->` inside a bound
(`impl<F: Fn(&str) -> bool>`) excluded from the count, and both shapes added to
the scanner self-test — which fails without the fix. Re-verified after the fix:
the residue is still exactly the six frozen entries, so the wider scan found no
previously hidden implementation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(arch): make the port-inversion scanner fail loud, and reconcile the doc counts

Review triage on #6998. Four findings taken, four rejected with evidence in the
thread; the taken ones are all about the gate telling the truth.

**The scanner could pass on an incomplete scan.** `rust_files` returned early on
a `read_dir` error and dropped per-entry errors through `.flatten()`, and
`traits_implemented_by` skipped any file it could not read. A permission or
transient I/O error in CI would have thinned the input and turned the ratchet
green while enforcing nothing — the exact failure class this file exists to
catch. Every I/O error is now fatal.

**`#[cfg(test)]` blocks were located by raw brace bytes.** A `{` inside a
comment or string literal in a gated block desynchronizes the depth count and
either leaks a test-only `impl` into the production set or swallows the
production code that follows it. Comments and strings are now stripped first;
`cfg_test_stripping_survives_braces_in_comments_and_strings` is the pin, and it
fails with the old composition (verified by reverting the order and watching it
go red). The doc comment now also states why `#[cfg(feature = "test-support")]`
is deliberately *not* stripped: that feature compiles into a real build, so an
`impl` behind it is a genuine normal-dependency edge, unlike `#[cfg(test)]`.

**The prose counts had drifted.** Eleven port declarations moved, not nine —
nine that `extension_host` implements (the pinned `INVERTED_PORTS`) plus
`AdminUserService` and `RebornOperatorToolCatalog`, which it only consumes and
composition implements. CHECKLIST, both CLAUDE files, and the module-count line
now agree and all defer to the architecture test as the enforced inventory.
`families/contracts.md` also still listed `ironclaw_common` in the family-level
dependency bullet; that is the second of the two places, now corrected too.

**One mismatch recorded rather than fixed.** `LifecycleProductService::
import_extension_bundle`'s default said "unavailable" while returning
`InvalidRequest`/400. The move carried both verbatim; changing the code changes
an HTTP status on a live route, which does not belong in a move-shaped PR. The
doc now describes what the code does, names the discrepancy, and points at the
test that pins today's behavior so a silent flip is impossible.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(contracts): state the module count as shipped-modules-plus-dev-seam

The count line said 'seventeen modules' while `src/lib.rs` carries eighteen
`pub mod` declarations — the difference is `test_support`, which is gated
behind `#[cfg(any(test, feature = "test-support"))]` and is deliberately
absent from the table above it. Saying 'seventeen shipped modules plus the
dev-only test_support' makes the table and the manifest agree on inspection
instead of looking like drift.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(contracts): resolve the ProductSurfaceFailure linchpin (WS2.2)

`ironclaw_extension_host` used `ironclaw_product`'s internal workflow error
as its own lifecycle error vocabulary across 19 production files — WS2.1's
recorded linchpin, blocking half the port-inversion residue and the layer
flip. Measured with `#[cfg(test)]` stripped, it constructs exactly six
variants (150 sites), all plain-`String` or unit, and none of the two
kernel-typed ones that kept the enum out of contracts.

The boundary half is now
`ironclaw_product_contracts::error::ProductOperationFailure`;
`ironclaw_product` keeps `ProductSurfaceFailure` unchanged in shape and
absorbs it with a total, payload-preserving `From`. The projection to
`ProductSurfaceError` is defined once, in contracts, and product's
`lifecycle_product_surface_error` delegates its six shared arms to it so the
two paths cannot drift. Only the logging stayed with each caller — contracts
may not log.

Narrowing the enum instead was rejected on evidence: `auth_continuation.rs`
matches all eight `TurnErrorCategory` values structurally and distinguishes
two the sanitized projection collapses, and constructs by matching
`TurnError` variants the projection cannot express — so narrowing is lossy
in a live auth path.

Unlocks `ProductConversationSubjectRouteResolver` (trait residue 6 -> 5, with
its route key and request type) and takes extension_host's files naming the
workflow error 19 -> 2. Corrects the two surviving residue reasons, which
named the error rather than the real blocker.

Regression coverage: nine crate-tier tests including the projection-agreement
pin and the `From` totality pin, plus two new architecture gates (frozen
residue files; the contract error names no kernel type), each verified by
negative probe. Extension-specificity allowlist shrinks 130 -> 129.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(arch): apply the parent's scanner hardening to the WS2.2 half

The merge brought in WS2.1's review fixes (I/O errors fatal, comments and
strings stripped *before* `#[cfg(test)]` brace matching). Both apply verbatim
to `production_files_naming`, which this branch added after that review:

- An unreadable file was silently skipped, which is exactly how the frozen
  residue-file scan would go quietly vacuous. Now fatal, matching the three
  other readers in the file.
- The strip order was backwards. A `{` inside a comment or string literal can
  desynchronise the `#[cfg(test)]` brace matcher, so comments and strings go
  first. Re-probed both directions afterwards: a code reference still trips
  the gate, a comment mentioning the type (now with an unbalanced brace) still
  does not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): close the changed-coverage holes the port move opened

CI's changed-coverage gate failed on the WS2.1 move, exactly where a
move-shaped diff is expected to: relocated bodies read as added production
lines. Every hole is now closed with a test. One line is exempted, with its
callers named.

**Five relocated port modules had no LCOV record at all.** `delivery`,
`channel_config`, `operator_tools`, `prompt_source`, and `views` are pure
declarations, so rustc emitted no source record and the gate reported them
absent. Each now carries a contract test rather than a waiver, and the
properties they pin are the ones these ports actually owe:

- **object safety** for all seven traits — every consumer holds them as
  `Arc<dyn _>`, so a signature change that breaks dyn-safety now fails at the
  contract instead of at the far-away wiring site;
- **argument pass-through and ordering** for the delivery ports — `reply_context`
  takes extension id, installation id, and conversation fingerprint as three
  bare strings, so nothing but a test stops a transposition turning into a
  silent mis-delivery (this is the identity-mixup risk review raised; the types
  stay verbatim, the ordering is now pinned);
- **absence without error** — an unresolved channel, an empty channel-config
  field set, an empty operator tool catalog, and a missing approval-prompt
  context are all normal outcomes that must not be expressible only as failures;
- **caller scoping** on the operator catalog, whose `caller` parameter is the
  #5459 disclosure control;
- **`next_cursor` omission** on an unpaginated view page — serializing `null`
  would make every unpaginated view look paginated to the browser.

**Two genuinely untested error paths in `extension_host`, both fail-closed
seams the move touched.** `AccountConnectionStatusSource::connected` now has
coverage proving it fails *closed* on a pairing-backend outage (activation must
not proceed on an unknown connection state) and *sanitized* (the test asserts
the driver, host, and port do not appear in the product-facing error). The
lifecycle output-serialization mapping moved out of an inline closure into a
named `lifecycle_output_decode_error` so the mapping is reachable from a test:
the failure is defensive, but *what it maps to* is a live contract — the model
gets `OutputDecode` and never the serde error, which can quote projection
contents.

**A dead branch arm.** `validate_typed_token` guards `c == '\0' || c.is_control()`
and only the second arm was exercised. NUL has its own arm because a token with
an embedded NUL truncates at a C boundary rather than merely looking odd.

**Diff shape.** The remaining reports were an artifact of relocating types
inline: a fully-qualified `ironclaw_product_contracts::<mod>::<Item>` in a
signature turns an untouched line into a changed one. Those 17 files now import
the symbol like every other, which shrinks the diff, restores the crate's
prevailing style, and drops the lines out of the gate's denominator because a
`use` line is uninstrumentable by construction.

**One exemption, with evidence.** `factory/test_support.rs`'s
`channel_config_service` accessor: the repoint collapsed its signature onto one
line, and the merged lcov does not attribute its two integration callers back
to the composition bucket build. Both callers are named in the manifest, the
service and the port contract are covered by tests added here, and it is filed
under the same #6963 lane-attribution lane as the WS1 entries above it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(contracts): invert ironclaw_operator's product-facing ports onto product_contracts

Five operator ports and their wire vocabulary move from `ironclaw_product` to
`ironclaw_product_contracts` (PROPOSAL §6.1.3, §6.9.2): `LlmConfigService` +
`ActiveModelReader` (new `llm_config` module) and `OperatorStatusService` +
`OperatorLogsService` + `OperatorServiceLifecycleService` (new
`operator_service` module). Every implementation stays with its owner.

`ironclaw_operator`'s `ironclaw_product` dependency is dropped, not waived —
the ownership inversion §6.9.2 describes is now a Cargo fact.

Also: operator's duplicate route-mount carriers are deleted in favour of
`ironclaw_host_ingress::PublicRouteMount`, which dissolves the composition-side
repackaging shim that existed only to convert between them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): make the catalog and view doubles discriminate on their arguments

Review caught two tests of mine that asserted the double's behavior rather
than the contract, and it was right about both.

`EmptyCatalog` ignored `caller` and always returned an empty vector, so
`the_catalog_is_caller_scoped...` would have passed against a production
catalog that disclosed every user's private installs — the exact leak the
`caller` parameter exists to close (#5459 P1). It is now backed by an
ownership-filtering double, two callers, one tenant-shared tool and one private
tool each, asserting both directions of isolation and that the answer *can*
differ by caller. `OneRowView::query` ignored `_caller` and `_params` and the
test only checked the cursor; the provider now echoes all three conduit
arguments and the test asserts all three.

Both were verified red-then-green rather than assumed: dropping the caller
filter fails the catalog tests, and dropping params from the echo fails the
view test. (My first attempt at the view mutation substituted the expected
literals and passed — a reminder that a mutation which doesn't fail proves
nothing about the mutation, only about the mutant.)

The over-claim went into the PR body too, and is corrected there: a contracts
crate can pin that the port *hands the implementation the caller* and that its
shape admits a per-caller answer. It cannot pin that production filters
correctly — that is composition's implementation and composition's test. The
doc comments now say so instead of implying the stronger claim.

Also lands the CHECKLIST note this PR earned for the rest of Wave 2/3: a
move-shaped PR fails the changed-coverage gate on its first CI run, in three
distinct shapes needing three different answers, with the two mechanical habits
that shrink all three.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(arch): gate the operator port inversion, and shed skill_learning's product edge

New `reborn_operator_port_inversion.rs`. The layer matrix cannot see this edge —
`ironclaw_operator` and `ironclaw_product` are both `products`, so
`products -> products` is legal and invisible — which is why the row needs a
purpose-built gate. Four halves: the product-declared-trait residue is frozen
exact-match at zero and shrink-only; the manifest edge is proved gone through
`cargo metadata` (not a literal path, so a WS10 directory move fails loudly);
each inverted port is pinned declared-in-contracts / not-re-declared-in-product
/ implemented-by-its-owner; and the scanner is self-tested, fatal on every I/O
error, and asserts non-vacuity on every walk it performs.

Verified by negative probe rather than asserted — re-adding the manifest
dependency, a stale residue row, re-declaring a moved port in product, a
compat-alias DTO in product, and a renamed crate path each fail for their own
reason, the last with "cannot read ..." rather than a silent pass.

`ironclaw_operator` also gains AGENTS.md, CLAUDE.md, and a `BoundaryRule` — it
had none of the three, which is how its product dependency survived every
earlier sweep.

Separately, the `skill_learning.rs` stray: its entire `ironclaw_product`
dependency was one import behind a four-line adapter. `LiveSkillLearnedNotifier`
moves to composition, whose ownership the port's own doc already asserted, and
the file's product references go to zero.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(target-architecture): record the operator inversion and the strays re-verification

CHECKLIST WS5's operator row is checked with five dispositions the lead sheet
did not predict, and WS2's strays row is annotated item by item: one executed,
three corrected with the evidence that blocks them, one reassigned, one out of
scope. Two new `[decision]` rows — the contracts-family vendor-rule hole the
LLM-config port opened, and whether any live store still carries a `slack_user`
installation row.

PROPOSAL §6.1.3 and §6.9.2 carry dated amendments, including two corrections to
§6.9.2's own wording: the route clause was satisfied by deleting a duplicated
carrier rather than moving a route, and the missing guidance/boundary rule was
causal rather than cosmetic.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(extensions): split ironclaw_extension_manager out of extension_host (WS2.4)

The extension host held two jobs: lifecycle authority (the only writer of
installation state, ingress verification, activation transactions) and the
extension-management product face that arrived with #6616/#6669. PROPOSAL
§6.8.3 splits the second into its own products-layer crate so the first can
move below product in WS2's layer flip.

Six of the nine inventory items moved; three are structurally blocked and
each is recorded with its measurement. extension_host production files
naming ironclaw_product: 20 -> 13. Port-inversion residue 5 -> 4.

Behavior-free: modules move, imports repoint, one 100-line product
projection is extracted from channel_config.rs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): close the coverage-gate shapes on the WS2.2 slice

Applies the cross-slot lessons from WS2.1/WS2.3's coverage rounds to this
row's own new code, before the gate has to ask.

Pure-declaration modules gained real contract tests rather than waivers:

- `subject_route`: the port is held as `Arc<dyn _>` in five places, so object
  safety is a contract; a resolver is handed every field unswapped
  (`adapter_id`/`installation_id` are both string newtypes, so a swap would
  otherwise be silent); and an unconfigured route is absence, not failure.
  The double is **route-keyed, not fixed-answer** — two configured routes
  resolve to *different* subjects and a third resolves to `None`, so a
  resolver that ignored its argument could not pass. A fixed-answer double
  would have made all three assertions vacuous.
- `error`: `Display` is exercised for every variant, asserting each one keeps
  the text the LLM tool path forwards — `ProviderInstanceNotConfigured`
  carries the operator's exact `config set` remediation.
- `lifecycle_surface_error`: pinned against the contract's own projection
  (drift guard) *and* against absolute statuses (so both drifting together
  still fails).

`channel_config_unavailable` is extracted from a `map_err` closure because it
sat on the one path unreachable in test without fault-injecting the concrete
config service. Naming it makes the classification directly testable, and the
classification matters: a store failure is transient (retryable 503), never a
rejection (permanent 4xx) that would leave a correctly-configured channel
looking broken. The other 44 closures in this crate are pre-existing bodies
where only the type name changed (45 on the parent), so they are left alone
rather than churned on speculation.

Each new test was verified red-then-green by **mutating production code**, and
every mutation compiles cleanly so the red is an assertion failure rather than
the compiler catching the mutant:

- route key stops discriminating by conversation -> two routes collapse to one
  subject (`left: eng-subject, right: support-subject`)
- `Display` drops `{reason}` -> "rendered as ..., dropping ..."
- `lifecycle_surface_error` stops delegating -> "projection drifted for ..."
- store failure reclassified permanent -> "must be transient, got ..."

Scope is calibrated in the doc comments: the contracts-crate test pins the
port's shape and that it admits a per-route answer; it does not claim the
production resolver filters correctly — `channel_subject_routes`' own tests
(`foreign_adapter_or_installation_resolves_nothing`,
`malformed_config_json_fails_closed`) already own that.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(ws2.4): date the two row corrections and quote the text they replace

The CHECKLIST disposition named the contradiction without quoting the
inventory line it corrects or carrying a date; PROPOSAL §6.8.3 pointed at
it without the verbatim text. Both now quote both sides.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(extension_host): cover the log-sanitization guard; exempt the type-position residue

CI's second changed-coverage run came back at 99.32% line / 100% branch, with
one uncovered line and six files reporting "contributed no instrumented lines".
Two different problems, two different answers.

**The uncovered line was coverable, so it is covered.**
`lifecycle_output_decode_error`'s `tracing::debug!` body never ran under test:
with no subscriber installed `tracing` short-circuits on the null dispatcher,
so the message literal is a region that cannot be reached. The fix is not a
waiver — it is the subscriber. The test now installs a DEBUG-level
`tracing_subscriber::fmt` over a shared writer (the pattern
`ironclaw_turns/tests/agent_loop_host_contract.rs` already uses) and asserts
*both* halves of the guard's contract: the model gets `OutputDecode` and never
the serde error, **and** the serde detail is not simply dropped — it reaches
the debug log, which is where an operator diagnoses it from. Without the
subscriber a test cannot tell "logged the detail" from "discarded it", which is
the whole point. `tracing-subscriber` joins this crate's dev-dependencies for
that, with a manifest comment saying why.

**The six files are the type-position residue, and it is precedented.**
Deleting `ironclaw_product`'s re-exports forced every signature naming a moved
symbol to be rewritten; where the name sits in a *type* position — a struct
field, a function parameter, a struct-literal field's enum path — the line
changes but LLVM emits no coverage region, so it can never be covered. Nine
exact lines across six files, each entry naming the construct, filed under the
same #6963 lane the four WS1 entries use. Every line was re-read against the
source before the entry was written; none is a guess.

The balance for the PR as a whole: ten exemption lines, all type positions or
one lane-attribution accessor, against ~30 tests written for surfaces that
genuinely lacked them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(coverage): exempt the tracing message literal, with the evidence that it is an artifact

Last line on the changed-coverage gate, and the obvious reading of it is wrong.

`extension_lifecycle_capabilities.rs:217` is the message string inside a
`tracing::debug!`. It reads as uncovered — but the event body demonstrably
executes: the DEBUG-subscriber test added in the previous commit asserts the
rendered log contains that exact message, and it passes, including in the
`extension-operator` bucket, which is green.

The proof it is an attribution artifact rather than a dead path comes from that
bucket's own tracefile (run 30689416105, `bucket-extension-operator.lcov`):

  line 213 (fn signature)       hits 1
  line 214 (macro invocation)   hits 1
  line 217 (message literal)    hits 0
  line 219 (error construction) hits 1
  line 220 (closing brace)      hits 1

The function ran, the macro ran, the error was built. What LLVM does not count
is the literal: `tracing` bakes the message into the callsite's `static`
`Metadata`, so the region on that line belongs to a static initializer and is
never attributed to an executed path. Nothing short of changing the log target
moves that counter, and changing a log target is a behavior change this
move-shaped PR will not make. Every `tracing::debug!` in the workspace has the
same shape; they only escape this gate because their lines are not in a diff.

Verified by replaying the gate locally against CI's own merged lcov with this
entry in place: changed line coverage 100.00% (147/147), changed branch
coverage 100.00% (10/10).

The test stays. It is what proves the 0 is an artifact, and it still pins the
guard's real contract: the model gets `OutputDecode` and never the serde error,
and the detail reaches the debug log rather than being dropped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(extension-host): prove the transient cause survives the sanitized 503

The lifecycle warning is the entire reason this crate kept a local projection
wrapper rather than calling the contract's `From` directly — and that claim
was asserted in a doc comment and nowhere else.

`tracing` short-circuits on the null dispatcher, so under a plain unit test the
macro body never runs and a test cannot distinguish "logged the cause" from
"dropped it" — which is exactly the distinction that matters when the 503 body
is sanitized. Installing a scoped subscriber (`with_default`, so parallel tests
are unaffected) over a shared writer, following the pattern
`ironclaw_turns/tests/agent_loop_host_contract.rs` established, makes both
halves of the guard's contract assertable, and both are asserted:

- the caller's 503 is sanitized — the cause appears nowhere in the serialized
  `ProductSurfaceError`; and
- the cause is not discarded — it reaches the warning, with its stable message.

A second test pins the other direction: a rejection carries no operational
cause and must not spend a warning, so "log everything" cannot satisfy the
first test.

Both verified red-then-green by mutating production code, compiling cleanly so
the red is an assertion:
- drop the warning -> "the transient cause must survive in the log, got \"\""
- warn on every variant -> "a rejection must not emit the transient warning,
  got ... invalid binding request: bad package ref"

`tracing-subscriber` joins `[dev-dependencies]` and the `Cargo.lock` delta is
**zero** — it was already resolved for the workspace.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(coverage): recapture the extension_host floor and ratchet the manager (WS2.4)

Both numbers come from this PR's own merged coverage artifact
(reborn-integration-coverage-merged, run 30689658637), read through the
same aggregation that enforces the file. extension_host regains its
covered-line floor at 19907/23467 = 84.83% (the ratio ROSE across the
split); the manager is ratcheted from birth at 4602/5440 = 84.60%.

Verified by running the enforcing ratchet against the artifact: both
entries PASS, 17 crates pass, exit 0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): close the changed-coverage holes in the two new operator modules

Measured with the same `cargo llvm-cov --skip-functions -p … --all-targets`
shape the crate-bucket lane uses, rather than waiting for CI to report it.
Seven uncovered lines; each closed with a test, none with an exemption.

Two were real, and one of them is the kind a test can hide rather than find:

- `truncate_utf8_with_suffix`'s character-boundary back-up loop had **no**
  executing test. The multi-byte case looked covered, but the cut offset is
  256 - 16 = 240 and 2, 3, and 4 all divide 240 — so every homogeneous
  `glyph.repeat(n)` input lands exactly on a boundary and the loop body never
  runs. Driving it needs a shifted input (one ASCII byte then 3-byte
  characters), which is now the case, with an assertion that the kept prefix is
  strictly shorter than the naive offset so the loop having run is what is
  proven.
- The degenerate bound (a limit shorter than the truncation marker) is
  unreachable through the public entry point, whose bound is a constant, so it
  is exercised directly through the private helper. It is a fail-safe against
  the subtraction below it underflowing if that constant is ever lowered, and
  an untested fail-safe is how an arithmetic panic reaches a log-query path.

The other four were unexercised methods on the `LlmConfigService` double —
`delete_provider` and `complete_nearai_wallet_login`. A double method no test
calls is a contract the suite silently stopped covering, so both are now
driven, the first asserting its argument reaches the error it produces and the
second asserting both directions of its outcome.

Both modules are now at zero uncovered added production lines: `llm_config`
288/288 DA, `operator_service` 243/243.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(composition): cover LiveSkillLearnedNotifier through the real publisher

Review triage. The strays row introduced a six-argument forwarding
adapter with no test of its own, which is the shape that fails silently:
swapping `skill_name`/`feedback` compiles (both `&str`), and dropping the
`Some(owner)` wrapper compiles (the publisher takes `Option<&UserId>`)
while re-keying every learned-skill bubble onto the runtime operator's
stream instead of the user's. `skill_learning.rs`'s `StubNotifier` tests
stop at the port and cannot see either.

The new test drives the production trait object over a real
`LiveProjectionPublisher` — no double anywhere — with the runtime actor
deliberately different from the run owner, and reads the result back off
the product event stream the WebUI drains.

Red-then-green proved by mutating the adapter, not the test, and both
mutations compile:
- swap `skill_name`/`feedback`  -> left: [(["picked this up summing a
  report column"], ["csv-column-sum"])]
- `Some(owner)` -> `None`       -> owner drain empty; with the first
  assertion neutralised, the negative assertion fires on its own with
  the bubble found on the runtime actor's stream.

Also from the same review:
- `llm_config.rs`'s comment claimed `assert_not_impl_any!` "would be the
  direct form" two lines above two live `assert_not_impl_any!` calls. It
  now says what the assertions enforce and why: both request types carry
  `api_key: Option<SecretString>`, so a `Serialize` impl is what would
  let the key ride back out.
- CHECKLIST's `reborn_extension_specificity.rs` pointer named `:1177-1180`,
  which in this PR's own tree is the `capability_surface.rs` pair; the
  `lifecycle_restore.rs`/`slack` entry sits at `:1202`. Replaced the line
  range with the allowlist entry itself, which cannot drift.

Verification: fmt clean; clippy -D warnings clean on
ironclaw_reborn_composition + ironclaw_product_contracts; 66 test
binaries, 1181 passed, 0 failed across composition, product_contracts,
and the full architecture suite.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(contracts,extension-host): preserve the acquire cause and pin every HostApiError projection

Review triage for #7000.

- `import_bundle`'s decode-limiter `map_err(|_| ...)` discarded the
  `AcquireError`. The mapping is now a named `map_import_decode_acquire_error`
  that logs the bound source before mapping. Named rather than inlined so it is
  reachable from a test: nothing in the workspace calls `Semaphore::close`, so
  an inline closure would be a permanently uncovered branch that the
  changed-line coverage gate could only accept as a standing exemption. New
  regression test builds a genuine `AcquireError` from a closed semaphore and
  asserts the failure is `Transient` (retryable), not a client mistake.

- `From<HostApiError> for ProductOperationFailure` was pinned by one variant.
  It now enumerates all ten, asserts each carries its own rendering (so the
  cause cannot be flattened at the boundary) and projects to a 400, and adds an
  exhaustive `host_api_error_tag` match so a new `HostApiError` variant stops
  compiling the test instead of inheriting the blanket mapping silently.
  `InvariantViolation` is pinned as-is, not reclassified: the mapping mirrors
  product's pre-existing `From<HostApiError> for ProductSurfaceFailure` and
  changing it is a behavior change this slice does not own.

Red-then-green proved by mutating the code under test: InvariantViolation ->
Transient, flattening the reason text, and Transient -> InvalidBindingRequest
each fail the corresponding assertion.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(arch): pin the rustfmt-wrapped impl header the operator scanner reads

Review argued `split_once(" for ")` misses a wrapped `impl` header and
that the frozen-empty residue half would therefore fail open. Measured:
it does not. rustfmt indents the continuation line, and that indent is
what keeps `" for "` intact as a substring — real rustfmt output for a
long header is `impl<'a> Trait<Arg>` / newline / `    for Type<'a>`, and
the scanner reads `Trait` from it.

Pinned rather than argued: `impl_scanner_reads_the_trait_out_of_real_impl_shapes`
now carries a wrapped-header case. Proved non-vacuous by mutating the
scanner to truncate each segment at its first newline, which compiles and
fails the test:

    WrappedHeaderPort was not read: {"ActiveModelReader", "LlmConfigService",
    "Local", "OperatorLogsService", "OperatorStatusService",
    "ReturnArrowInBound"}

Also dropped the `:933` line pointer from `ironclaw_operator/AGENTS.md`:
the `include_str!` is at `:934`, so it was already stale, and nothing
verifies it. Path plus `reborn_cross_crate_include_scan.rs` locate the
debt.

Verification: fmt clean; `cargo test -p ironclaw_architecture --test
reborn_operator_port_inversion` 7 passed / 0 failed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(architecture,ci): close the review gaps on the extension_manager split

Review triage for #7003. All four are artifacts this PR introduced, not moved code.

- The new `ironclaw_extension_manager` boundary rule forbade
  `"ironclaw_reborn_cli"`, which is the crate DIRECTORY. `forbidden` entries are
  compared against `cargo metadata` package names and the CLI's package is
  `ironclaw`, so the entry could never fire — the edge it named was unguarded.
  Fixed, and pinned: `boundary_rule_names_are_package_names_not_crate_directories`
  flags any forbidden entry that is not a package but IS a directory under
  `crates/`. That discrimination matters — ~60 entries legitimately name retired
  v1 crates (`ironclaw_legacy`, `ironclaw_engine`, `ironclaw_gateway`,
  `ironclaw_tui`, `ironclaw_storage`) as reintroduction pins, and those have no
  directory. `ironclaw_reborn_cli` was the only entry in all 693 that had one.

- `production_files_naming` took a flat `files.len() >= 10` to accommodate the
  manager, which silently dropped the host's vacuous-scan guard from >20 to 10.
  The same diff had already parameterized `traits_implemented_by` for exactly
  this reason. Parameterized to match: host 21, manager 10.

- `classify-test-scope.sh` gained a `crates/ironclaw_extension_manager/*` arm
  with no self-test case, so a manager-only diff classifying
  `has_reborn_tests=false` would have gone unnoticed — the failure #6947 records
  for the stale `crates/ironclaw_product_*/*` arm. Case added.

- `coverage-floor.toml`'s "9.7k lines moved" explained an instrumented-line
  delta of 3,102 with a source-line figure. Both units are now stated with their
  measurements (source: 57,464 -> 47,794 in the host, 9,979 in the manager;
  instrumented: 26,569 -> 23,467 against 5,440) and why they do not reconcile.

Red-then-green proved by mutating the code under test: reverting the forbidden
entry to the directory spelling fails the new meta-test with the fix-it message;
removing the manager glob from the classifier fails the new self-test case
(has_reborn_tests=false); raising the manager's file floor to 40 fails only the
manager call site, proving the floor is per-call-site and consumed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(contracts): resolve the ProductSurfaceFailure linchpin (WS2.2)

`ironclaw_extension_host` used `ironclaw_product`'s internal workflow error
as its own lifecycle error vocabulary across 19 production files — WS2.1's
recorded linchpin, blocking half the port-inversion residue and the layer
flip. Measured with `#[cfg(test)]` stripped, it constructs exactly six
variants (150 sites), all plain-`String` or unit, and none of the two
kernel-typed ones that kept the enum out of contracts.

The boundary half is now
`ironclaw_product_contracts::error::ProductOperationFailure`;
`ironclaw_product` keeps `ProductSurfaceFailure` unchanged in shape and
absorbs it with a total, payload-preserving `From`. The projection to
`ProductSurfaceError` is defined once, in contracts, and product's
`lifecycle_product_surface_error` delegates its six shared arms to it so the
two paths cannot drift. Only the logging stayed with each caller — contracts
may not log.

Narrowing the enum instead was rejected on evidence: `auth_continuation.rs`
matches all eight `TurnErrorCategory` values structurally and distinguishes
two the sanitized projection collapses, and constructs by matching
`TurnError` variants the projection cannot express — so narrowing is lossy
in a live auth path.

Unlocks `ProductConversationSubjectRouteResolver` (trait residue 6 -> 5, with
its route key and request type) and takes extension_host's files naming the
workflow error 19 -> 2. Corrects the two surviving residue reasons, which
named the error rather than the real blocker.

Regression coverage: nine crate-tier tests including the projection-agreement
pin and the `From` totality pin, plus two new architecture gates (frozen
residue files; the contract error names no kernel type), each verified by
negative probe. Extension-specificity allowlist shrinks 130 -> 129.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(arch): apply the parent's scanner hardening to the WS2.2 half

The merge brought in WS2.1's review fixes (I/O errors fatal, comments and
strings stripped *before* `#[cfg(test)]` brace matching). Both apply verbatim
to `production_files_naming`, which this branch added after that review:

- An unreadable file was silently skipped, which is exactly how the frozen
  residue-file scan would go quietly vacuous. Now fatal, matching the three
  other readers in the file.
- The strip order was backwards. A `{` inside a comment or string literal can
  desynchronise the `#[cfg(test)]` brace matcher, so comments and strings go
  first. Re-probed both directions afterwards: a code reference still trips
  the gate, a comment mentioning the type (now with an unbalanced brace) still
  does not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): close the coverage-gate shapes on the WS2.2 slice

Applies the cross-slot lessons from WS2.1/WS2.3's coverage rounds to this
row's own new code, before the gate has to ask.

Pure-declaration modules gained real contract tests rather than waivers:

- `subject_route`: the port is held as `Arc<dyn _>` in five places, so object
  safety is a contract; a resolver is handed every field unswapped
  (`adapter_id`/`installation_id` are both string newtypes, so a swap would
  otherwise be silent); and an unconfigured route is absence, not failure.
  The double is **route-keyed, not fixed-answer** — two configured routes
  resolve to *different* subjects and a third resolves to `None`, so a
  resolver that ignored its argument could not pass. A fixed-answer double
  would have made all three assertions vacuous.
- `error`: `Display` is exercised for every variant, asserting each one keeps
  the text the LLM tool path forwards — `ProviderInstanceNotConfigured`
  carries the operator's exact `config set` remediation.
- `lifecycle_surface_error`: pinned against the contract's own projection
  (drift guard) *and* against absolute statuses (so both drifting together
  still fails).

`channel_config_unavailable` is extracted from a `map_err` closure because it
sat on the one path unreachable in test without fault-injecting the concrete
config service. Naming it makes the classification directly testable, and the
classification matters: a store failure is transient (retryable 503), never a
rejection (permanent 4xx) that would leave a correctly-configured channel
looking broken. The other 44 closures in this crate are pre-existing bodies
where only the type name changed (45 on the parent), so they are left alone
rather than churned on speculation.

Each new test was verified red-then-green by **mutating production code**, and
every mutation compiles cleanly so the red is an assertion failure rather than
the compiler catching the mutant:

- route key stops discriminating by conversation -> two routes collapse to one
  subject (`left: eng-subject, right: support-subject`)
- `Display` drops `{reason}` -> "rendered as ..., dropping ..."
- `lifecycle_surface_error` stops delegating -> "projection drifted for ..."
- store failure reclassified permanent -> "must be transient, got ..."

Scope is calibrated in the doc comments: the contracts-crate test pins the
port's shape and that it admits a per-route answer; it does not claim the
production resolver filters correctly — `channel_subject_routes`' own tests
(`foreign_adapter_or_installation_resolves_nothing`,
`malformed_config_json_fails_closed`) already own that.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(extension-host): prove the transient cause survives the sanitized 503

The lifecycle warning is the entire reason this crate kept a local projection
wrapper rather than calling the contract's `From` directly — and that claim
was asserted in a doc comment and nowhere else.

`tracing` short-circuits on the null dispatcher, so under a plain unit test the
macro body never runs and a test cannot distinguish "logged the cause" from
"dropped it" — which is exactly the distinction that matters when the 503 body
is sanitized. Installing a scoped subscriber (`with_default`, so parallel tests
are unaffected) over a shared writer, following the pattern
`ironclaw_turns/tests/agent_loop_host_contract.rs` established, makes both
halves of the guard's contract assertable, and both are asserted:

- the caller's 503 is sanitized — the cause appears nowhere in the serialized
  `ProductSurfaceError`; and
- the cause is not discarded — it reaches the warning, with its stable message.

A second test pins the other direction: a rejection carries no operational
cause and must not spend a warning, so "log everything" cannot satisfy the
first test.

Both verified red-then-green by mutating production code, compiling cleanly so
the red is an assertion:
- drop the warning -> "the transient cause must survive in the log, got \"\""
- warn on every variant -> "a rejection must not emit the transient warning,
  got ... invalid binding request: bad package ref"

`tracing-subscriber` joins `[dev-dependencies]` and the `Cargo.lock` delta is
**zero** — it was already resolved for the workspace.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(contracts,extension-host): preserve the acquire cause and pin every HostApiError projection

Review triage for #7000.

- `import_bundle`'s decode-limiter `map_err(|_| ...)` discarded the
  `AcquireError`. The mapping is now a named `map_import_decode_acquire_error`
  that logs the bound source before mapping. Named rather than inlined so it is
  reachable from a test: nothing in the workspace calls `Semaphore::close`, so
  an inline closure would be a permanently uncovered branch that the
  changed-line coverage gate could only accept as a standing exemption. New
  regression test builds a genuine `AcquireError` from a closed semaphore and
  asserts the failure is `Transient` (retryable), not a client mistake.

- `From<HostApiError> for ProductOperationFailure` was pinned by one variant.
  It now enumerates all ten, asserts each carries its own rendering (so the
  cause cannot be flattened at the boundary) and projects to a 400, and adds an
  exhaustive `host_api_error_tag` match so a new `HostApiError` variant stops
  compiling the test instead of inheriting the blanket mapping silently.
  `InvariantViolation` is pinned as-is, not reclassified: the mapping mirrors
  product's pre-existing `From<HostApiError> for ProductSurfaceFailure` and
  changing it is a behavior change this slice does not own.

Red-then-green proved by mutating the code under test: InvariantViolation ->
Transient, flattening the reason text, and Transient -> InvalidBindingRequest
each fail the corresponding assertion.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(conversations): fix the conversations/threads naming trap (WS5)

Rename the five names `ironclaw_conversations` shared with
`ironclaw_threads` and unify the external actor/conversation pair onto its
one home in `ironclaw_extension_contracts`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(attachments): widen ironclaw_attachments to own its ports and ceilings (WS5)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(target-arch): record the WS5 naming-trap and attachments-widening outcomes

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(conversations): state the threads boundary in the crate doc

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(attachments,conversations,product): close the review gaps on the WS5 naming-trap slice

Review triage for #7005.

- `project_scoped.rs`: delete a stranded `///` block that described
  `ProjectScopedAttachmentReader`, ended mid-clause, and rustdoc was attaching
  to the `InboundAttachmentLander` impl. The module doc already records that the
  reader stays in `ironclaw_product`.

- `cleanup_stale`'s third pre-scan exit — an in-root reference whose relative
  depth is not `<date>/<message>/<file>` — had zero coverage anywhere in the
  tree. Extended the existing empty/unowned fail-closed test rather than adding
  a redundant one, asserting the `Internal` code and that the seeded batch
  survives the aborted pass.

- `stored_refs` / `ids`: state the rollback boundary. Compatibility is
  upgrade-only by decision — this build reads `thread_id`/`message_id` and
  writes only `topic_id`/`reply_target_message_id`, so a record written here and
  read by a pre-rename binary silently collapses every threaded route to its
  conversation root. Dual-writing is refused on the row's own type-placement
  rule, and is self-defeating besides: verified that a reader with
  `#[serde(alias)]` rejects a record carrying both spellings
  (`duplicate field \`topic_id\``).

- `gate_routes`: pass `None` for the source branch's reply target. Provably
  behavior-identical (`conversation_fingerprint` hashes space + conversation +
  topic and excludes the reply-target hint), but the previous spelling could
  only be read as correct together with the fingerprint body, and it reads as a
  per-message id baked into a stable route key.

- `run_delivery_contract`: the gate-route test could not see any of that. Its
  prompting event is now a threaded reply carrying both a topic and a reply
  target, which makes the source branch's key distinguishable from the
  delivered-message loop's, and it pins the fingerprint's reply-target
  independence directly.

- `inbound.rs`: rename the private `session_thread_service` field/param to
  `conversation_service`. `SessionThreadService` is the `ironclaw_threads` type
  this PR exists to stop colliding with.

- CHECKLIST WS5 sub-item 3: dated amendment quoting the sentence it annotates;
  the re-word/re-home obligation is now tracked in #7010.

No test was added and none removed — two were extended. Red-then-green proved
by mutating the code under test, not the tests: the malformed-reference branch
downgraded to `continue`; the fingerprint widened to include the reply target;
and three separate breaks of the source branch (topic keyed off the reply
target, topic dropped, branch records nothing). An earlier version of the
gate-route assertion passed under all three and was reworked until it failed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(extension-host): close the WS2.2 changed-coverage gate with tests, not waivers

The `ProductSurfaceFailure` -> `ProductOperationFailure` repoint put 137
already-uncovered error-path lines into the changed-line denominator: the new
name is two characters longer, so every construction site's first line changed
and rustfmt re-wrapped the arms that crossed 100 columns. The gate ran on this
PR for the first time (stacked PRs never triggered it) and reported 67.38%
line / 83.33% branch.

Measured, not assumed. Replaying CI's own merged lcov (run 30706965794) against
the base lcov from `main` @ 569d8e4895 (run 30705915898) shows 122 of the 137
lines are 1:1 rename-only replacements that each scored `DA:<line>,0` at their
pre-image, and the other 15 are rustfmt re-wraps of those same lines. Zero are
"no LLVM region" type positions -- all 137 carry a DA record, because the gate
intersects the changed set with DA records, so region-less lines never enter
the denominator at all.

60 of those lines get real tests here rather than a waiver
(ironclaw_extension_host 419 -> 438 tests), covering every pure boundary mapper
the repoint touched:

* the retryable-vs-caller-error split in `product_lifecycle`,
  `lifecycle_restore`, `active_publication`, `lifecycle_product_service`,
  `extension_activation_credentials` and `hosted_mcp_manifest`;
* `map_skill_error`'s `FilesystemDenied -> BindingAccessDenied` projection,
  which is an authorization outcome and must not read as retryable;
* both post-install activation fail-open classifiers (service tier and
  capability tier), which decide which activation failures are swallowed
  behind a successful install -- they must agree, and now both are pinned;
* `ensure_caller_may_mutate_tenant_installation`, the tenant-admin guard on
  shared installations, pinned on the denial and on both ways through;
* `UnavailableExtensionActivationCredentialGate`, pinned fail-closed;
* `pending_manifest`'s hosted-MCP name and client-profile input guards, which
  are what keep caller text out of interpolated manifest TOML;
* `prepare_install`'s refusal of a retained definition that disagrees with the
  catalog.

Every one was verified by mutating the code under test and confirming the
assertion went red -- not the compiler. 12/12 mutants killed.

The remaining 77 lines and 1 branch are exempted with per-site evidence in four
classes: map_err arms on argument-free infrastructure constructors that cannot
fail from any input; defensive arms dominated by the guard immediately above
them; paths gated behind `VerifiedAuthClaim`, which has no constructor outside
`ironclaw_host_api`; and pre-existing fault-injection paths inside async `&self`
service methods, each still scoring 0 hits at its pre-image in the base lcov.

Also corrects a stale entry inherited from WS2.1: the `tracing` message-literal
exemption named line 217 (`?error,`) instead of 218 (the literal), and its
evidence block was off by one throughout. Inert today because neither line is in
this PR's changed set, but it would have silently failed to apply the moment a
PR touched the real line -- the stranded-exemption failure mode the manifest is
supposed to prevent.

Local gate: 100.00% line (343/343), 100.00% branch (2/2), exit 0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(coverage): tighten the WS2.2 exemption evidence to what the lcovs actually show

Three reasons overstated their evidence. Corrected against the base lcov:

* `channel_subject_routes.rs` 231-233 have no 1:1 pre-image because the hunk
  is 1->3 (`@@ -217 +231,3 @@`); base line 217 held the whole closure and
  scored `DA:217,0`, so it is one uncovered closure re-wrapped, which the
  reason now says instead of claiming a per-line pre-image.
* `product_lifecycle.rs` 783-786 map to base 785-787, where the `.map_err(`
  call scored 136 hits and only the closure body scored 0. The reason now
  names both numbers rather than implying the whole span was cold.
* `test_support.rs` 633-634 are the only genuinely NEW lines in this PR -- a
  `.map_err(ProductSurfaceFailure::from)` conversion, not a rename. Calling
  them "rename-only" was wrong. The honest evidence is that every line of the
  enclosing `#[cfg(feature = "test-support")]` helper scored 0 hits at base
  (624-635), so the conversion was added to an already-dead seam.

The header block's "the remaining 15 are rustfmt re-wraps" is corrected to
13 re-wraps plus those 2 new lines. No line numbers changed; gate still
100.00% line (343/343), 100.00% branch (2/2), exit 0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(ws5): close the changed-coverage gate on the naming-trap slice

The gate attaches for the first time now that #7005 targets main. It
reported 21 uncovered changed lines, 8 uncovered changed branch arms, and
one file contributing no instrumented lines at all.

Every genuinely reachable hole is closed with a real test, driven through
the caller that owns the side effect rather than the helper:

- `ProjectScopedAttachmentLander::rollback` refuses malformed batch
  references. Rollback deletes a whole batch directory, so each guard in
  `attachment_batch_parent` is a delete-target check; the test lands a
  real batch first and asserts a refused rollback never removes it.
- `map_external_ref_error`'s non-`InvalidIdentifier` fallback keeps this
  crate's error vocabulary and carries the source message verbatim.
- The durable `RebornFilesystemConversationServices` forwards the
  inbound-message half of its contract (accept + replay), which only
  `InMemoryConversationServices` had ever exercised.
- `external_ref` maps `ProductAdapterError` to `InvalidMaterialization`
  without leaking a `RedactedString` detail, both directly and through
  `trigger_conversation_fields`.
- The two standalone attachment test-support accessors land bytes and
  read them back through both returned read views. They had no callers
  anywhere in the repo; `#[allow(dead_code)]` on the impl block hid it.
- `delivered_conversation_fingerprints` drops a vendor message ref that
  cannot key a route, covering the two reachable `Err` arms.

Two exemptions, both with per-site evidence, neither a shortcut:

- `types.rs`: seven `pub struct` / field declarations from the DTO
  rename. A serde round-trip contract test for all five types was
  written first to test the obvious hypothesis that the derives would
  instrument them; re-measuring showed the file still reports the
  identical 42 DA records over the identical 17..312 span, because
  derive-generated code is `#[automatically_derived]` and emits no
  region at the declaration site. The round-trip test is kept: it pins
  the persisted encoding across the rename, which is the risk the WS5
  CHECKLIST row actually cares about.
- `gate_routes.rs` branch arms 45/58/74: every argument is an accessor
  read off an already-validated `ExternalConversationRef`, whose fields
  are private and whose only value-producing paths all run
  `validate_external_id`. Re-validating a value that already passed a
  pure predicate cannot fail. The two sibling sites that also take the
  unvalidated vendor ref are tested, not exempted.

Each new test was mutation-verified red-then-green.

* fix(architecture,extensions): close the paranoid-architect review findings on the WS2.4 split

Review pass over #7003 (four parallel deep reviews; no Critical/High — the
move itself verified behavior-free). Everything found, fixed here:

Gate hardening (crates/ironclaw_architecture/tests):
- ratchet_support gains cfg_test_only_files: files reachable only through
  #[cfg(test)] mod chains (incl. #[path] overrides) are classified test code.
  channel_host/e2e_auth_challenge.rs — a fake AuthChallengeProvider impl
  wearing a production filename — no longer counts toward any residue row,
  implementor pin, or error-vocabulary floor. Pinned by a real-tree test that
  was red before the #[path] resolution landed.
- Trait matching is qualified by a whole-token crate reference (names_crate),
  so a name-colliding local trait can no longer satisfy an implementor pin,
  and a manifest rename of ironclaw_product can no longer blind the manager
  residue scan (metadata tie: dep exists iff the residue list is non-empty,
  never renamed).
- The manager gets its own product-defined-trait residue freeze (twin of the
  host's, frozen at ExtensionCredentialSetupService).
- each_half_of_the_split_kept_its_own_job: authority checks are symmetric
  across file/directory spellings and back every module with a content
  witness, so an empty stub cannot satisfy retention.
- untrusted_ingress_paths scan roots fail loudly on a missing root instead of
  silently dropping a tree from the guard.
- Fork-check message names its two-crate scope.
All new checks probed red-for-the-right-reason and reverted (hollow witness,
product alias, stale scan root, authority-as-directory, unguarded secret).

Manifest hygiene:
- extension_host drops the ed25519-dalek dep orphaned when ironhub moved.
- Ten manager deps used only by tests/the test_support fixture leave the
  production graph: fixture deps become test-support-gated optionals, pure
  test deps move to [dev-dependencies]. All three build shapes verified.

Manager/host code:
- channel_config: the pub resolved_manifest widening is narrowed to a
  declares_admin_configuration() boolean — the manifest read stays internal.
- admin_configuration view: secret field values are redacted in render_group
  (same defense-in-depth as render_state), with a sentinel regression test;
  the service-error table test now pins code/kind beside status/retryable.

Docs (single-source-of-truth):
- families/extensions.md confesses the direct auth/host_runtime deps and the
  transitional dep tail the four-crate target does not name.
- The residue characterization says what the list actually holds: DTOs,
  capability-id constants, and two port-inversion residues.
- 20 -> 13 becomes 20 -> 12 (the 13th was the cfg(test)-only fixture);
  coverage-floor/CHECKLIST stale "recapture owed" drafts corrected to the
  shipped recapture; line counts de-precisioned; stale exemption comment
  repointed to the manager.

Verification: architecture 143/0; manager 64/0 (--all-features);
extension_host 388/0 (--all-features); cargo check --workspace --all-targets
--all-features 0 errors / 0 warnings; clippy -D warnings clean on all three
touched crates; both CI script self-tests pass; cargo metadata --locked clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(conversations): keep the durable grammar so the rename survives a rollback

Human review on #7005 (serrrfirat, `stored_refs.rs:53`) and CodeRabbit
(`stored_refs.rs:38`) both found the same real defect, and the module's own
refutation was aimed at a different proposal than the one that fixes it.

`stored_refs` refuted DUAL-WRITING (emitting both spellings), correctly: a
reader with `#[serde(alias)]` rejects a record carrying both as a duplicate
field. But the ask was WRITE-LEGACY / READ-EITHER, which that objection does
not touch. Measured against `origin/main`, the released readers are
`RawExternalConversationIdentity` and the `ExternalConversationRef` wire struct
in `ironclaw_conversations/src/ids.rs`; both name `thread_id`/`message_id` and
carry no aliases. So a record this build wrote read back as `None` on a
rollback, with no error.

Worse than the reported "remaps to the conversation root": the identity keys
`BindingKey`, and `StoredConversationState::into_state` rebuilds the map with
`Vec<(K, V)>::into_iter().collect()`, so two threaded bindings in one
conversation collapse onto one key and the earlier one is dropped.

- `conversation_ref::serialize` now writes `{space_id, conversation_id,
  thread_id, message_id}` through a borrowed representation; both spellings
  still read, and a record carrying both still fails closed.
- `ExternalConversationIdentity` gets a matching hand-written `Serialize`.
- `stored_refs::actor_ref` deleted: the actor change was additive, so the
  canonical impls already do everything it did. `actor_serde_needs_no_adapter`
  pins that equivalence instead of asserting it.

Tested through the durable store, not a surrogate
(`filesystem_conversation_services_persist_external_refs_in_the_durable_grammar`
walks every key of the real persisted document). Both fixes verified
red-then-green by mutating the writers: reverting the ref writer fails the unit
AND store tests; making the identity emit `topic_id` fails only the store test,
naming all six sites — which is exactly the gap the review reported.

Also from the same review round:

- Rename `product/src/scoped_fs/attachment_landing.rs` -> `attachment_reader.rs`
  now that the lander moved out (serrrfirat, `attachment_landing.rs:1`).
- Reuse `ratchet_support::strip_comments_and_strings` instead of a third local
  copy; the extended self-test fixture proves the deleted line-based copy leaked
  block comments (serrrfirat, `reborn_conversations_threads_attachments.rs:135`).
- Amend `docs/reborn/contracts/conversation-binding.md` and the conversations
  CLAUDE.md for the renamed service, the moved ref pair, and `topic_id`
  (serrrfirat, `conversations/src/lib.rs:38`).
- Widen the `crates/AGENTS.md` attachments row and record the justified WebUI
  edge in `ironclaw_webui/AGENTS.md` (serrrfirat, `attachments/src/lib.rs:23`).
- Assert the topic participates in the fingerprint, so the route-membership
  check cannot pass vacuously (CodeRabbit, `run_delivery_contract.rs:1347`).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(operator,contracts): close the WS5 operator review findings

Human review on #7004 (serrrfirat). Five findings taken, one deferred with a
named home, one answered in place.

- Composition calls operator's route mount through the crate-root facade
  (`ironclaw_operator::nearai_login_callback_mount`) instead of naming the
  three-segment module path. The deep path was pre-existing — it lived in the
  composition shim this PR deleted — but the shim was what encapsulated it, so
  the facade re-export is this PR's to add. `llm_admin/mod.rs` already
  re-exports free functions (`apply_stored_api_key`, `resolve_reborn_runtime_llm`),
  so this follows the existing convention rather than inventing one.

- `map_llm_config_error` deleted; its 10 call sites across t…
pull Bot pushed a commit to Stars1233/ironclaw that referenced this pull request Aug 3, 2026
…ve-2 main (nearai#7032)

Audits docs/reborn/target-architecture/ (plus crates/AGENTS.md and the
crate guides Wave 2 touched) against merged main at 3be5f05, after
nearai#6996, nearai#6998, nearai#7002 and nearai#7018.

Docs-only: 13 .md files, no code, no tests. House style throughout —
dated amendments, prior text quoted verbatim wherever a clause is
corrected, nothing rewritten silently and no decision record deleted.

The two structural findings the wave produced and nobody had written
down: same-layer edges are invisible to the layer matrix by
construction, so the exception count could never have moved in Wave 2
and each removal needed its own purpose-built shrink-only gate
(PROPOSAL §8.1, §8.2, §11.1); and the changed-line coverage policy —
90% lines, branch coverage ungated since nearai#7013 — was recorded in no
document at all, alongside a stranded-exemption failure mode the new
pre-existing-uncovered exclusion creates (CHECKLIST WS10).

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
l3ocifer pushed a commit to l3ocifer/frick-ironclaw that referenced this pull request Sep 3, 2026
…o product_contracts (WS2.1) (nearai#6998)

* refactor(contracts): invert extension_host's product-facing ports onto product_contracts (WS2.1)

`ironclaw_extension_host` sits below product in the target tree, so a
product-side port it satisfies must be declared at the product boundary and
implemented downward — never declared inside `ironclaw_product` and reached
upward. This moves every such port that `ironclaw_product_contracts` may
legally name, and dissolves the product re-export facade for the extension
host.

Nine port families move (definitions only; every implementation stays with its
owner, PROPOSAL §6.1.4): delivery resolution + reply context, account-connection
status + setup descriptors, channel config, the view-provider conduit, command
context + actor-role admission, gate-prompt enrichment, the lifecycle product
service, the admin-user directory, and the operator tool catalog. Product keeps
`DeliveryCoordinator`, `NoReplyContext`, `ExtensionAccountSetupRegistry`,
`UnsupportedLifecycleProductService`, `RejectingAdminUserService`,
`UnavailableRebornViewProvider`, `DirectConversationCommandAdmission`, the
frozen `Reborn*` wire DTOs, and the inbound-action ledger.

extension_host's product symbol usage drops 146 -> 62 across 46 -> 35
production files. The edge itself does not die here and could not: the
survivors are `channel_host.rs`'s construction of product's concrete assembly,
the `extension_manager` split inventory, `product::adapter_registry`, and the
named strays — each owned by a later WS2 row. Six ports also could not move,
all for one mechanical reason: `product_contracts` may depend only on
`host_api` + `extension_contracts`, so a signature naming `ironclaw_auth`,
`ironclaw_threads`, `ironclaw_turns`, or `ironclaw_conversations` cannot be
declared there. `ProductSurfaceFailure` is the linchpin — extension_host uses
product's *internal* workflow error as its own lifecycle error vocabulary in 19
files, and it carries `ironclaw_turns::TurnError`.

Regression cover: `reborn_extension_host_port_inversion.rs` pins the nine moved
ports where they landed and holds the six-entry residue shrink-only, with the
per-entry reason each could not move; a new product-declared port implemented
by extension_host fails the build. The moved typed-token tests travel with
their code and `ActionFingerprintKey` gains the coverage it lacked.

Enumerating gates, all update-never-relax: the composition pub-use snapshot
gains one line (two names re-sourced from `product_contracts`, so one `pub use`
splits into three); the extension-specificity allowlist, the struct/test-support
ratchet, the §11.2.7 include inventory, the `ProductSurface` method freeze, and
`LAYER_MATRIX_EXCEPTIONS` (13) are all untouched — extension_host carries no
layer-matrix exception and never did, since both crates are `products`-layer.

`secrecy` joins `product_contracts` with a manifest comment: `AdminUserService`
takes secret material and `AdminCreatedUser` carries a one-time token, both
`SecretString`. It is a value wrapper, not a framework/driver/runtime client.

CHECKLIST WS2 row 1 ticked with the four dispositions the lead sheet did not
predict.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): cover the moved port surfaces and close the impl-scanner bracket hole

Two follow-ups on the WS2.1 port inversion, both found by measuring rather
than assuming.

**Coverage of the surfaces this PR created.** `cargo llvm-cov` over
`ironclaw_product_contracts` showed the relocated bodies had no crate-tier
coverage of their own: `ProductCommandContext::from_envelope`,
`AdminUserRole::is_admin`, `AccountConnectionStatusError::new`,
`ChannelConnectionNoticePolicy::generic`, the bounded-token
`TryFrom`/`AsRef`/`Display` arms, and — the one that matters most — the two
`LifecycleProductService` **default** method bodies, which every production
implementor overrides, so nothing exercised the fail-closed defaults. Each is
now tested at its contract meaning, not for the line count: bundle import
defaults to `InvalidRequest` rather than silently succeeding; activation errors
default to none so the wire field stays absent; a non-command envelope is
rejected as an invalid request rather than an internal error; a token that
deserializes runs the same validation as its constructor; the generic notice
policy names the channel in all five notices and does not collapse them into
one string. Every added production line in the new modules is now covered.

**The scanner had a hole the review caught, and it was real.**
`implemented_trait_names` closed the impl's generic-parameter list at the first
`>`. For `impl<T: Iterator<Item = X>> Port for Host<T>` that `>` closes
`Iterator`, leaving `> Port` — not an identifier, so the impl was dropped and a
new product-defined port could have entered `extension_host` without tripping
the shrink-only gate. Now closed by balancing, with `->` inside a bound
(`impl<F: Fn(&str) -> bool>`) excluded from the count, and both shapes added to
the scanner self-test — which fails without the fix. Re-verified after the fix:
the residue is still exactly the six frozen entries, so the wider scan found no
previously hidden implementation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(arch): make the port-inversion scanner fail loud, and reconcile the doc counts

Review triage on nearai#6998. Four findings taken, four rejected with evidence in the
thread; the taken ones are all about the gate telling the truth.

**The scanner could pass on an incomplete scan.** `rust_files` returned early on
a `read_dir` error and dropped per-entry errors through `.flatten()`, and
`traits_implemented_by` skipped any file it could not read. A permission or
transient I/O error in CI would have thinned the input and turned the ratchet
green while enforcing nothing — the exact failure class this file exists to
catch. Every I/O error is now fatal.

**`#[cfg(test)]` blocks were located by raw brace bytes.** A `{` inside a
comment or string literal in a gated block desynchronizes the depth count and
either leaks a test-only `impl` into the production set or swallows the
production code that follows it. Comments and strings are now stripped first;
`cfg_test_stripping_survives_braces_in_comments_and_strings` is the pin, and it
fails with the old composition (verified by reverting the order and watching it
go red). The doc comment now also states why `#[cfg(feature = "test-support")]`
is deliberately *not* stripped: that feature compiles into a real build, so an
`impl` behind it is a genuine normal-dependency edge, unlike `#[cfg(test)]`.

**The prose counts had drifted.** Eleven port declarations moved, not nine —
nine that `extension_host` implements (the pinned `INVERTED_PORTS`) plus
`AdminUserService` and `RebornOperatorToolCatalog`, which it only consumes and
composition implements. CHECKLIST, both CLAUDE files, and the module-count line
now agree and all defer to the architecture test as the enforced inventory.
`families/contracts.md` also still listed `ironclaw_common` in the family-level
dependency bullet; that is the second of the two places, now corrected too.

**One mismatch recorded rather than fixed.** `LifecycleProductService::
import_extension_bundle`'s default said "unavailable" while returning
`InvalidRequest`/400. The move carried both verbatim; changing the code changes
an HTTP status on a live route, which does not belong in a move-shaped PR. The
doc now describes what the code does, names the discrepancy, and points at the
test that pins today's behavior so a silent flip is impossible.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(contracts): state the module count as shipped-modules-plus-dev-seam

The count line said 'seventeen modules' while `src/lib.rs` carries eighteen
`pub mod` declarations — the difference is `test_support`, which is gated
behind `#[cfg(any(test, feature = "test-support"))]` and is deliberately
absent from the table above it. Saying 'seventeen shipped modules plus the
dev-only test_support' makes the table and the manifest agree on inspection
instead of looking like drift.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): close the changed-coverage holes the port move opened

CI's changed-coverage gate failed on the WS2.1 move, exactly where a
move-shaped diff is expected to: relocated bodies read as added production
lines. Every hole is now closed with a test. One line is exempted, with its
callers named.

**Five relocated port modules had no LCOV record at all.** `delivery`,
`channel_config`, `operator_tools`, `prompt_source`, and `views` are pure
declarations, so rustc emitted no source record and the gate reported them
absent. Each now carries a contract test rather than a waiver, and the
properties they pin are the ones these ports actually owe:

- **object safety** for all seven traits — every consumer holds them as
  `Arc<dyn _>`, so a signature change that breaks dyn-safety now fails at the
  contract instead of at the far-away wiring site;
- **argument pass-through and ordering** for the delivery ports — `reply_context`
  takes extension id, installation id, and conversation fingerprint as three
  bare strings, so nothing but a test stops a transposition turning into a
  silent mis-delivery (this is the identity-mixup risk review raised; the types
  stay verbatim, the ordering is now pinned);
- **absence without error** — an unresolved channel, an empty channel-config
  field set, an empty operator tool catalog, and a missing approval-prompt
  context are all normal outcomes that must not be expressible only as failures;
- **caller scoping** on the operator catalog, whose `caller` parameter is the
  nearai#5459 disclosure control;
- **`next_cursor` omission** on an unpaginated view page — serializing `null`
  would make every unpaginated view look paginated to the browser.

**Two genuinely untested error paths in `extension_host`, both fail-closed
seams the move touched.** `AccountConnectionStatusSource::connected` now has
coverage proving it fails *closed* on a pairing-backend outage (activation must
not proceed on an unknown connection state) and *sanitized* (the test asserts
the driver, host, and port do not appear in the product-facing error). The
lifecycle output-serialization mapping moved out of an inline closure into a
named `lifecycle_output_decode_error` so the mapping is reachable from a test:
the failure is defensive, but *what it maps to* is a live contract — the model
gets `OutputDecode` and never the serde error, which can quote projection
contents.

**A dead branch arm.** `validate_typed_token` guards `c == '\0' || c.is_control()`
and only the second arm was exercised. NUL has its own arm because a token with
an embedded NUL truncates at a C boundary rather than merely looking odd.

**Diff shape.** The remaining reports were an artifact of relocating types
inline: a fully-qualified `ironclaw_product_contracts::<mod>::<Item>` in a
signature turns an untouched line into a changed one. Those 17 files now import
the symbol like every other, which shrinks the diff, restores the crate's
prevailing style, and drops the lines out of the gate's denominator because a
`use` line is uninstrumentable by construction.

**One exemption, with evidence.** `factory/test_support.rs`'s
`channel_config_service` accessor: the repoint collapsed its signature onto one
line, and the merged lcov does not attribute its two integration callers back
to the composition bucket build. Both callers are named in the manifest, the
service and the port contract are covered by tests added here, and it is filed
under the same nearai#6963 lane-attribution lane as the WS1 entries above it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): make the catalog and view doubles discriminate on their arguments

Review caught two tests of mine that asserted the double's behavior rather
than the contract, and it was right about both.

`EmptyCatalog` ignored `caller` and always returned an empty vector, so
`the_catalog_is_caller_scoped...` would have passed against a production
catalog that disclosed every user's private installs — the exact leak the
`caller` parameter exists to close (nearai#5459 P1). It is now backed by an
ownership-filtering double, two callers, one tenant-shared tool and one private
tool each, asserting both directions of isolation and that the answer *can*
differ by caller. `OneRowView::query` ignored `_caller` and `_params` and the
test only checked the cursor; the provider now echoes all three conduit
arguments and the test asserts all three.

Both were verified red-then-green rather than assumed: dropping the caller
filter fails the catalog tests, and dropping params from the echo fails the
view test. (My first attempt at the view mutation substituted the expected
literals and passed — a reminder that a mutation which doesn't fail proves
nothing about the mutation, only about the mutant.)

The over-claim went into the PR body too, and is corrected there: a contracts
crate can pin that the port *hands the implementation the caller* and that its
shape admits a per-caller answer. It cannot pin that production filters
correctly — that is composition's implementation and composition's test. The
doc comments now say so instead of implying the stronger claim.

Also lands the CHECKLIST note this PR earned for the rest of Wave 2/3: a
move-shaped PR fails the changed-coverage gate on its first CI run, in three
distinct shapes needing three different answers, with the two mechanical habits
that shrink all three.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(extension_host): cover the log-sanitization guard; exempt the type-position residue

CI's second changed-coverage run came back at 99.32% line / 100% branch, with
one uncovered line and six files reporting "contributed no instrumented lines".
Two different problems, two different answers.

**The uncovered line was coverable, so it is covered.**
`lifecycle_output_decode_error`'s `tracing::debug!` body never ran under test:
with no subscriber installed `tracing` short-circuits on the null dispatcher,
so the message literal is a region that cannot be reached. The fix is not a
waiver — it is the subscriber. The test now installs a DEBUG-level
`tracing_subscriber::fmt` over a shared writer (the pattern
`ironclaw_turns/tests/agent_loop_host_contract.rs` already uses) and asserts
*both* halves of the guard's contract: the model gets `OutputDecode` and never
the serde error, **and** the serde detail is not simply dropped — it reaches
the debug log, which is where an operator diagnoses it from. Without the
subscriber a test cannot tell "logged the detail" from "discarded it", which is
the whole point. `tracing-subscriber` joins this crate's dev-dependencies for
that, with a manifest comment saying why.

**The six files are the type-position residue, and it is precedented.**
Deleting `ironclaw_product`'s re-exports forced every signature naming a moved
symbol to be rewritten; where the name sits in a *type* position — a struct
field, a function parameter, a struct-literal field's enum path — the line
changes but LLVM emits no coverage region, so it can never be covered. Nine
exact lines across six files, each entry naming the construct, filed under the
same nearai#6963 lane the four WS1 entries use. Every line was re-read against the
source before the entry was written; none is a guess.

The balance for the PR as a whole: ten exemption lines, all type positions or
one lane-attribution accessor, against ~30 tests written for surfaces that
genuinely lacked them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(coverage): exempt the tracing message literal, with the evidence that it is an artifact

Last line on the changed-coverage gate, and the obvious reading of it is wrong.

`extension_lifecycle_capabilities.rs:217` is the message string inside a
`tracing::debug!`. It reads as uncovered — but the event body demonstrably
executes: the DEBUG-subscriber test added in the previous commit asserts the
rendered log contains that exact message, and it passes, including in the
`extension-operator` bucket, which is green.

The proof it is an attribution artifact rather than a dead path comes from that
bucket's own tracefile (run 30689416105, `bucket-extension-operator.lcov`):

  line 213 (fn signature)       hits 1
  line 214 (macro invocation)   hits 1
  line 217 (message literal)    hits 0
  line 219 (error construction) hits 1
  line 220 (closing brace)      hits 1

The function ran, the macro ran, the error was built. What LLVM does not count
is the literal: `tracing` bakes the message into the callsite's `static`
`Metadata`, so the region on that line belongs to a static initializer and is
never attributed to an executed path. Nothing short of changing the log target
moves that counter, and changing a log target is a behavior change this
move-shaped PR will not make. Every `tracing::debug!` in the workspace has the
same shape; they only escape this gate because their lines are not in a diff.

Verified by replaying the gate locally against CI's own merged lcov with this
entry in place: changed line coverage 100.00% (147/147), changed branch
coverage 100.00% (10/10).

The test stays. It is what proves the 0 is an artifact, and it still pins the
guard's real contract: the model gets `OutputDecode` and never the serde error,
and the detail reaches the debug log rather than being dropped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(target-architecture): give WS2.1's four deferred signature fixes a real home

Review triage found the follow-ups this PR deliberately deferred were
recorded only in the PR body, pointing at "the same slice that has to
narrow `ProductSurfaceFailure`" — a slice with no CHECKLIST row. Adds
that WS2 row and names all four items on it with their exact seams,
including the `import_extension_bundle` 400-vs-503 flip and the test
assertion that has to move with it. The two doc comments under review
now point at the row, so the deferral is discoverable from the code.

No behavior change: markdown plus two doc comments.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(target-architecture): split the deferred signatures off the linchpin row

Review was right that the row added in a6ee90c bundled a blocker with
work that is not blocked. Only the six unmovable ports depend on
`ProductSurfaceFailure` carrying `ironclaw_turns::TurnError`; `gate_ref`,
the `installed_activation_errors` key, and the delivery identities each
replace a type that already sits in a crate `product_contracts` may name,
so grouping them behind the linchpin would have held independent work
hostage. The grouping was scheduling convenience, not a dependency.

Now two rows: the linchpin (the six ports, and the 19-file use of
product's internal error type as extension_host's own vocabulary), and
the four verbatim-relocated signatures, marked independent of it and of
each other. `import_extension_bundle` is called out on that row as a
behavior change rather than a signature correction, with the live route
and the test assertion that moves with it.

Doc comments repointed to the row that now owns each item.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(target-architecture): fix the linchpin row's stated resolution

Review found the resolution I wrote was wrong, and it was — inherited
verbatim from row 1's disposition 2, which carried the same error.

"Narrow `ProductSurfaceFailure` off `ironclaw_turns`, or replace it with
an extension-host-owned error" is insufficient either way.
`product_contracts`' allowlist is enforced as a whitelist of exactly
{product_contracts, extension_contracts, host_api}
(reborn_dependency_boundaries.rs:362-374, comment: "Never product, never
operator, never the extension host"), and `ProductSurfaceFailure` lives
in `ironclaw_product` (error.rs:46). A port erroring with it is
undeclarable in contracts whatever it carries; an extension-host-owned
error is barred by the same list.

The row now states the real work: define the port-facing error in an
allowed crate and map it to `ProductSurfaceFailure` inside product, with
narrowing off `ironclaw_turns` demoted to the sub-goal it is. Row 1's
disposition 2 carries a dated correction quoting the clause it replaces.

Also re-titled the sibling row: "four verbatim-relocated signatures" was
right about provenance and wrong about the fix — three are type changes
and one is a status-code flip. It is now "The four WS2.1 follow-ups —
three typed-signature corrections and one status-code flip", with the
shared-provenance/separate-fix distinction stated. Doc pointers follow.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
l3ocifer pushed a commit to l3ocifer/frick-ironclaw that referenced this pull request Sep 3, 2026
…cts (WS5) (nearai#7002)

* refactor(contracts): invert webui + openai_compat onto product_contracts (WS5)

Both transports now compile against the product boundary instead of the
product crate: the wire DTOs they serialize, the request bodies they
deserialize, and the operation descriptor *types* they hold moved into
`ironclaw_product_contracts`, and every implementation stayed with its owner.

- `ironclaw_webui`: 228 -> 102 production `ironclaw_product::` symbols
- `ironclaw_reborn_openai_compat`: 23 -> 3 (7 -> 2 files)

New contracts modules: `descriptors`, `inbound_requests`, `product_wire`,
`workspace_views`; `admin_users` and `operator_llm` gained their wire DTOs.

The residue is structural, not shortfall: PROPOSAL §6.1.3 keeps product's
concrete command/view/capability constants in product as the frozen
inventory, and a route handler names the constant to call the surface. The
new `reborn_transport_product_boundary.rs` freezes the residue with per-entry
reasons, pins the moved vocabulary in contracts, and pins the inventory in
product so "shrinking" the residue by moving the inventory fails loudly.

Regression tests: `crates/ironclaw_architecture/tests/reborn_transport_product_boundary.rs`
(4 tests, incl. a scanner self-test and a non-vacuity assertion).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(contracts): resolve the ProductSurfaceFailure linchpin (WS2.2)

`ironclaw_extension_host` used `ironclaw_product`'s internal workflow error
as its own lifecycle error vocabulary across 19 production files — WS2.1's
recorded linchpin, blocking half the port-inversion residue and the layer
flip. Measured with `#[cfg(test)]` stripped, it constructs exactly six
variants (150 sites), all plain-`String` or unit, and none of the two
kernel-typed ones that kept the enum out of contracts.

The boundary half is now
`ironclaw_product_contracts::error::ProductOperationFailure`;
`ironclaw_product` keeps `ProductSurfaceFailure` unchanged in shape and
absorbs it with a total, payload-preserving `From`. The projection to
`ProductSurfaceError` is defined once, in contracts, and product's
`lifecycle_product_surface_error` delegates its six shared arms to it so the
two paths cannot drift. Only the logging stayed with each caller — contracts
may not log.

Narrowing the enum instead was rejected on evidence: `auth_continuation.rs`
matches all eight `TurnErrorCategory` values structurally and distinguishes
two the sanitized projection collapses, and constructs by matching
`TurnError` variants the projection cannot express — so narrowing is lossy
in a live auth path.

Unlocks `ProductConversationSubjectRouteResolver` (trait residue 6 -> 5, with
its route key and request type) and takes extension_host's files naming the
workflow error 19 -> 2. Corrects the two surviving residue reasons, which
named the error rather than the real blocker.

Regression coverage: nine crate-tier tests including the projection-agreement
pin and the `From` totality pin, plus two new architecture gates (frozen
residue files; the contract error names no kernel type), each verified by
negative probe. Extension-specificity allowlist shrinks 130 -> 129.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(product_wire): complete the residue list in the module header

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(workspace_views): state the real reason each read port stayed in product

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): close the three changed-coverage shapes on the relocated modules

Applied preemptively from nearai#6998's CI experience, before the gate could fire.

(1) Relocated declaration modules emit no LCOV records. `workspace_views`,
`operator_llm`, `inbound_requests`, `descriptors`, and `product_wire` now carry
contract tests for their real executable content — chosen for the invariant, not
for the number: `ProjectFsFile`'s hand-written `Debug` (it keeps whole user files
out of diagnostics; re-deriving it compiles and silently re-opens the leak),
`FsMount::ALL` exhaustiveness, the `as_str`/`Display`/serde triples, and the
list-request builders.

(2) Two genuinely untested paths arrived with the DTOs and are tested, not
exempted: `validate_outbound_delivery_display_field` — the only fence between an
operator-supplied delivery-target label and the browser, now driven through both
the constructor and the `TryFrom` path for bidi-override, zero-width-joiner,
separator, control-character, whitespace and per-field byte-cap shapes — and the
`RebornOutboundPreferencesResponse` wire-shadow back-compat rule plus
`RebornAutomationState`'s degrade-to-`Unknown` deserializer.

(3) Diff-shape noise: two `webui_v2/handlers.rs` signatures carried inline
`ironclaw_product_contracts::product_wire::<Item>` paths; the longer one made
rustfmt reflow a one-line return type into three. Both are `use` imports now, so
that file's diff is imports plus exactly two production lines.

No test doubles are used, so the discard-the-argument vacuity mode has no
surface. Every test was red-green verified by mutating the implementation (nine
mutations, nine assertion-level reds); two earlier mutations that produced
compile errors were rewritten as behavior changes, because a compile error
proves the compiler caught it and not the test.

Regression tests: crates/ironclaw_product_contracts/src/{workspace_views,
operator_llm,inbound_requests,descriptors,product_wire}.rs

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(arch): apply the parent's scanner hardening to the WS2.2 half

The merge brought in WS2.1's review fixes (I/O errors fatal, comments and
strings stripped *before* `#[cfg(test)]` brace matching). Both apply verbatim
to `production_files_naming`, which this branch added after that review:

- An unreadable file was silently skipped, which is exactly how the frozen
  residue-file scan would go quietly vacuous. Now fatal, matching the three
  other readers in the file.
- The strip order was backwards. A `{` inside a comment or string literal can
  desynchronise the `#[cfg(test)]` brace matcher, so comments and strings go
  first. Re-probed both directions afterwards: a code reference still trips
  the gate, a comment mentioning the type (now with an unbalanced brace) still
  does not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): close the coverage-gate shapes on the WS2.2 slice

Applies the cross-slot lessons from WS2.1/WS2.3's coverage rounds to this
row's own new code, before the gate has to ask.

Pure-declaration modules gained real contract tests rather than waivers:

- `subject_route`: the port is held as `Arc<dyn _>` in five places, so object
  safety is a contract; a resolver is handed every field unswapped
  (`adapter_id`/`installation_id` are both string newtypes, so a swap would
  otherwise be silent); and an unconfigured route is absence, not failure.
  The double is **route-keyed, not fixed-answer** — two configured routes
  resolve to *different* subjects and a third resolves to `None`, so a
  resolver that ignored its argument could not pass. A fixed-answer double
  would have made all three assertions vacuous.
- `error`: `Display` is exercised for every variant, asserting each one keeps
  the text the LLM tool path forwards — `ProviderInstanceNotConfigured`
  carries the operator's exact `config set` remediation.
- `lifecycle_surface_error`: pinned against the contract's own projection
  (drift guard) *and* against absolute statuses (so both drifting together
  still fails).

`channel_config_unavailable` is extracted from a `map_err` closure because it
sat on the one path unreachable in test without fault-injecting the concrete
config service. Naming it makes the classification directly testable, and the
classification matters: a store failure is transient (retryable 503), never a
rejection (permanent 4xx) that would leave a correctly-configured channel
looking broken. The other 44 closures in this crate are pre-existing bodies
where only the type name changed (45 on the parent), so they are left alone
rather than churned on speculation.

Each new test was verified red-then-green by **mutating production code**, and
every mutation compiles cleanly so the red is an assertion failure rather than
the compiler catching the mutant:

- route key stops discriminating by conversation -> two routes collapse to one
  subject (`left: eng-subject, right: support-subject`)
- `Display` drops `{reason}` -> "rendered as ..., dropping ..."
- `lifecycle_surface_error` stops delegating -> "projection drifted for ..."
- store failure reclassified permanent -> "must be transient, got ..."

Scope is calibrated in the doc comments: the contracts-crate test pins the
port's shape and that it admits a per-route answer; it does not claim the
production resolver filters correctly — `channel_subject_routes`' own tests
(`foreign_adapter_or_installation_resolves_nothing`,
`malformed_config_json_fails_closed`) already own that.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(contracts): close the WS5 review round — scanner order, DTO redaction, doc drift

Review triage for nearai#7002. Ten of the seventeen threads changed code or docs;
the rest are refuted or routed with evidence in their threads.

**The one real defect: the transport scanner's strip order was inverted.**
`product_symbols_in` ran `strip_cfg_test_blocks` on raw source, so a brace
inside a comment or string literal in a `#[cfg(test)]` block desynchronised the
byte-level depth walk and truncated the rest of the file. Regression test
first: the new case makes the scan return an *empty* set (proof it ran off the
end), then the order flips to match the sibling gate, which nearai#6998's own triage
had already fixed for exactly this reason. The frozen residues are byte-for-byte
unchanged either way, so the bug was latent, not active.

**Four credential/payload DTOs kept a derived `Debug` through the move.**
`ProductInboundAttachment` carries a whole user upload and is a field of
`ProductSubmitTurnRequest`; `RebornAdminPutSecretRequest`/`…ProductRequest`
carry raw secret material in front of a port that takes `SecretString`;
`RebornAdminUserCreatedResponse` carries the one-time API token; and
`RebornAccountLoginLinkResponse.url` is a code-bearing credential whose own doc
comment promises it "must never be logged". Each now hand-writes `Debug`, the
same treatment `ProjectFsFile` already had in this crate, pinned by three tests.
Serde is untouched — no wire shape changes. Four mutations of the
implementations (not the expected literals) produce four assertion-level reds.

**Documentation that the move made false.**
- rustdoc: 8 broken intra-doc links → 0 (`cargo doc --no-deps --all-features`).
  The reviewer found 3; the other 5 are the same class.
- `product_contracts/CLAUDE.md`: nineteen → twenty-three shipped modules, the
  four new rows added, and the `admin_users`/`views` rows corrected — both still
  claimed their contents had stayed in product.
- `product/CLAUDE.md`: `ProductView` is no longer residue; the residue is the
  frozen operation inventory, which is the honest reason the dep survives.
- `Cargo.toml`: the `secrecy/serde` justification named a type carrying no
  secret. The three `Deserialize` bodies that actually force it are named now.
- `PROPOSAL.md` §6.1.3's allowed-deps line dropped `ironclaw_common`, so it
  agrees with `families/contracts.md` and with `product_contracts_allowed`.
- `product_wire.rs`'s `large_file` exemption is one line with `plan nearai#7008`, so
  it satisfies the enforcer regex in `scripts/pre-commit-safety.sh`.
- `descriptors::query_on` states the one-payload conduit invariant.
- `fs_browse.rs` imports the project-fs DTOs from their owner instead of
  through the sibling module's re-export.
- CHECKLIST WS8 records `RebornSkillInfo`'s duplicate `source`/`source_kind`
  wire field — a real removal, deliberately not made inside a move.

Verification: fmt; clippy `-D warnings --all-targets --all-features` on the six
touched crates → clean; unfiltered tests for those six → 2841 passed / 0 failed
across 117 binaries; 4 tests added, none removed; rustdoc 0 warnings.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(ci): repoint the stale changed-coverage exemption the DTO move stranded

WS5 moved the `Reborn*` wire DTOs out of
`ironclaw_product/src/reborn_services/types.rs` (1890 -> 288 lines), which
left changed-coverage exemption #9 pointing at line 483 — past the new EOF.
The gate validates the manifest fail-closed before it scores anything, so a
single stranded entry aborted the whole run with
"exemption #9 names a line beyond current EOF (288)" and no coverage verdict
was produced at all.

The exempted line is the same struct field it always was — the non-executable
`pub gate_ref: Option<TurnGateRef>,` type position — which the deletions above
it moved to line 105. Repointed rather than deleted so the record of why that
line is uninstrumentable survives.

Replaying the manifest validation over all 71 entries against this tree now
reports zero problems (it was the only stranded entry; nearai#7000's tree was
already clean).

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
l3ocifer pushed a commit to l3ocifer/frick-ironclaw that referenced this pull request Sep 3, 2026
….2, WS2.4, WS5) (nearai#7018)

* refactor(contracts): invert extension_host's product-facing ports onto product_contracts (WS2.1)

`ironclaw_extension_host` sits below product in the target tree, so a
product-side port it satisfies must be declared at the product boundary and
implemented downward — never declared inside `ironclaw_product` and reached
upward. This moves every such port that `ironclaw_product_contracts` may
legally name, and dissolves the product re-export facade for the extension
host.

Nine port families move (definitions only; every implementation stays with its
owner, PROPOSAL §6.1.4): delivery resolution + reply context, account-connection
status + setup descriptors, channel config, the view-provider conduit, command
context + actor-role admission, gate-prompt enrichment, the lifecycle product
service, the admin-user directory, and the operator tool catalog. Product keeps
`DeliveryCoordinator`, `NoReplyContext`, `ExtensionAccountSetupRegistry`,
`UnsupportedLifecycleProductService`, `RejectingAdminUserService`,
`UnavailableRebornViewProvider`, `DirectConversationCommandAdmission`, the
frozen `Reborn*` wire DTOs, and the inbound-action ledger.

extension_host's product symbol usage drops 146 -> 62 across 46 -> 35
production files. The edge itself does not die here and could not: the
survivors are `channel_host.rs`'s construction of product's concrete assembly,
the `extension_manager` split inventory, `product::adapter_registry`, and the
named strays — each owned by a later WS2 row. Six ports also could not move,
all for one mechanical reason: `product_contracts` may depend only on
`host_api` + `extension_contracts`, so a signature naming `ironclaw_auth`,
`ironclaw_threads`, `ironclaw_turns`, or `ironclaw_conversations` cannot be
declared there. `ProductSurfaceFailure` is the linchpin — extension_host uses
product's *internal* workflow error as its own lifecycle error vocabulary in 19
files, and it carries `ironclaw_turns::TurnError`.

Regression cover: `reborn_extension_host_port_inversion.rs` pins the nine moved
ports where they landed and holds the six-entry residue shrink-only, with the
per-entry reason each could not move; a new product-declared port implemented
by extension_host fails the build. The moved typed-token tests travel with
their code and `ActionFingerprintKey` gains the coverage it lacked.

Enumerating gates, all update-never-relax: the composition pub-use snapshot
gains one line (two names re-sourced from `product_contracts`, so one `pub use`
splits into three); the extension-specificity allowlist, the struct/test-support
ratchet, the §11.2.7 include inventory, the `ProductSurface` method freeze, and
`LAYER_MATRIX_EXCEPTIONS` (13) are all untouched — extension_host carries no
layer-matrix exception and never did, since both crates are `products`-layer.

`secrecy` joins `product_contracts` with a manifest comment: `AdminUserService`
takes secret material and `AdminCreatedUser` carries a one-time token, both
`SecretString`. It is a value wrapper, not a framework/driver/runtime client.

CHECKLIST WS2 row 1 ticked with the four dispositions the lead sheet did not
predict.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): cover the moved port surfaces and close the impl-scanner bracket hole

Two follow-ups on the WS2.1 port inversion, both found by measuring rather
than assuming.

**Coverage of the surfaces this PR created.** `cargo llvm-cov` over
`ironclaw_product_contracts` showed the relocated bodies had no crate-tier
coverage of their own: `ProductCommandContext::from_envelope`,
`AdminUserRole::is_admin`, `AccountConnectionStatusError::new`,
`ChannelConnectionNoticePolicy::generic`, the bounded-token
`TryFrom`/`AsRef`/`Display` arms, and — the one that matters most — the two
`LifecycleProductService` **default** method bodies, which every production
implementor overrides, so nothing exercised the fail-closed defaults. Each is
now tested at its contract meaning, not for the line count: bundle import
defaults to `InvalidRequest` rather than silently succeeding; activation errors
default to none so the wire field stays absent; a non-command envelope is
rejected as an invalid request rather than an internal error; a token that
deserializes runs the same validation as its constructor; the generic notice
policy names the channel in all five notices and does not collapse them into
one string. Every added production line in the new modules is now covered.

**The scanner had a hole the review caught, and it was real.**
`implemented_trait_names` closed the impl's generic-parameter list at the first
`>`. For `impl<T: Iterator<Item = X>> Port for Host<T>` that `>` closes
`Iterator`, leaving `> Port` — not an identifier, so the impl was dropped and a
new product-defined port could have entered `extension_host` without tripping
the shrink-only gate. Now closed by balancing, with `->` inside a bound
(`impl<F: Fn(&str) -> bool>`) excluded from the count, and both shapes added to
the scanner self-test — which fails without the fix. Re-verified after the fix:
the residue is still exactly the six frozen entries, so the wider scan found no
previously hidden implementation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(arch): make the port-inversion scanner fail loud, and reconcile the doc counts

Review triage on #6998. Four findings taken, four rejected with evidence in the
thread; the taken ones are all about the gate telling the truth.

**The scanner could pass on an incomplete scan.** `rust_files` returned early on
a `read_dir` error and dropped per-entry errors through `.flatten()`, and
`traits_implemented_by` skipped any file it could not read. A permission or
transient I/O error in CI would have thinned the input and turned the ratchet
green while enforcing nothing — the exact failure class this file exists to
catch. Every I/O error is now fatal.

**`#[cfg(test)]` blocks were located by raw brace bytes.** A `{` inside a
comment or string literal in a gated block desynchronizes the depth count and
either leaks a test-only `impl` into the production set or swallows the
production code that follows it. Comments and strings are now stripped first;
`cfg_test_stripping_survives_braces_in_comments_and_strings` is the pin, and it
fails with the old composition (verified by reverting the order and watching it
go red). The doc comment now also states why `#[cfg(feature = "test-support")]`
is deliberately *not* stripped: that feature compiles into a real build, so an
`impl` behind it is a genuine normal-dependency edge, unlike `#[cfg(test)]`.

**The prose counts had drifted.** Eleven port declarations moved, not nine —
nine that `extension_host` implements (the pinned `INVERTED_PORTS`) plus
`AdminUserService` and `RebornOperatorToolCatalog`, which it only consumes and
composition implements. CHECKLIST, both CLAUDE files, and the module-count line
now agree and all defer to the architecture test as the enforced inventory.
`families/contracts.md` also still listed `ironclaw_common` in the family-level
dependency bullet; that is the second of the two places, now corrected too.

**One mismatch recorded rather than fixed.** `LifecycleProductService::
import_extension_bundle`'s default said "unavailable" while returning
`InvalidRequest`/400. The move carried both verbatim; changing the code changes
an HTTP status on a live route, which does not belong in a move-shaped PR. The
doc now describes what the code does, names the discrepancy, and points at the
test that pins today's behavior so a silent flip is impossible.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(contracts): state the module count as shipped-modules-plus-dev-seam

The count line said 'seventeen modules' while `src/lib.rs` carries eighteen
`pub mod` declarations — the difference is `test_support`, which is gated
behind `#[cfg(any(test, feature = "test-support"))]` and is deliberately
absent from the table above it. Saying 'seventeen shipped modules plus the
dev-only test_support' makes the table and the manifest agree on inspection
instead of looking like drift.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(contracts): resolve the ProductSurfaceFailure linchpin (WS2.2)

`ironclaw_extension_host` used `ironclaw_product`'s internal workflow error
as its own lifecycle error vocabulary across 19 production files — WS2.1's
recorded linchpin, blocking half the port-inversion residue and the layer
flip. Measured with `#[cfg(test)]` stripped, it constructs exactly six
variants (150 sites), all plain-`String` or unit, and none of the two
kernel-typed ones that kept the enum out of contracts.

The boundary half is now
`ironclaw_product_contracts::error::ProductOperationFailure`;
`ironclaw_product` keeps `ProductSurfaceFailure` unchanged in shape and
absorbs it with a total, payload-preserving `From`. The projection to
`ProductSurfaceError` is defined once, in contracts, and product's
`lifecycle_product_surface_error` delegates its six shared arms to it so the
two paths cannot drift. Only the logging stayed with each caller — contracts
may not log.

Narrowing the enum instead was rejected on evidence: `auth_continuation.rs`
matches all eight `TurnErrorCategory` values structurally and distinguishes
two the sanitized projection collapses, and constructs by matching
`TurnError` variants the projection cannot express — so narrowing is lossy
in a live auth path.

Unlocks `ProductConversationSubjectRouteResolver` (trait residue 6 -> 5, with
its route key and request type) and takes extension_host's files naming the
workflow error 19 -> 2. Corrects the two surviving residue reasons, which
named the error rather than the real blocker.

Regression coverage: nine crate-tier tests including the projection-agreement
pin and the `From` totality pin, plus two new architecture gates (frozen
residue files; the contract error names no kernel type), each verified by
negative probe. Extension-specificity allowlist shrinks 130 -> 129.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(arch): apply the parent's scanner hardening to the WS2.2 half

The merge brought in WS2.1's review fixes (I/O errors fatal, comments and
strings stripped *before* `#[cfg(test)]` brace matching). Both apply verbatim
to `production_files_naming`, which this branch added after that review:

- An unreadable file was silently skipped, which is exactly how the frozen
  residue-file scan would go quietly vacuous. Now fatal, matching the three
  other readers in the file.
- The strip order was backwards. A `{` inside a comment or string literal can
  desynchronise the `#[cfg(test)]` brace matcher, so comments and strings go
  first. Re-probed both directions afterwards: a code reference still trips
  the gate, a comment mentioning the type (now with an unbalanced brace) still
  does not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): close the changed-coverage holes the port move opened

CI's changed-coverage gate failed on the WS2.1 move, exactly where a
move-shaped diff is expected to: relocated bodies read as added production
lines. Every hole is now closed with a test. One line is exempted, with its
callers named.

**Five relocated port modules had no LCOV record at all.** `delivery`,
`channel_config`, `operator_tools`, `prompt_source`, and `views` are pure
declarations, so rustc emitted no source record and the gate reported them
absent. Each now carries a contract test rather than a waiver, and the
properties they pin are the ones these ports actually owe:

- **object safety** for all seven traits — every consumer holds them as
  `Arc<dyn _>`, so a signature change that breaks dyn-safety now fails at the
  contract instead of at the far-away wiring site;
- **argument pass-through and ordering** for the delivery ports — `reply_context`
  takes extension id, installation id, and conversation fingerprint as three
  bare strings, so nothing but a test stops a transposition turning into a
  silent mis-delivery (this is the identity-mixup risk review raised; the types
  stay verbatim, the ordering is now pinned);
- **absence without error** — an unresolved channel, an empty channel-config
  field set, an empty operator tool catalog, and a missing approval-prompt
  context are all normal outcomes that must not be expressible only as failures;
- **caller scoping** on the operator catalog, whose `caller` parameter is the
  #5459 disclosure control;
- **`next_cursor` omission** on an unpaginated view page — serializing `null`
  would make every unpaginated view look paginated to the browser.

**Two genuinely untested error paths in `extension_host`, both fail-closed
seams the move touched.** `AccountConnectionStatusSource::connected` now has
coverage proving it fails *closed* on a pairing-backend outage (activation must
not proceed on an unknown connection state) and *sanitized* (the test asserts
the driver, host, and port do not appear in the product-facing error). The
lifecycle output-serialization mapping moved out of an inline closure into a
named `lifecycle_output_decode_error` so the mapping is reachable from a test:
the failure is defensive, but *what it maps to* is a live contract — the model
gets `OutputDecode` and never the serde error, which can quote projection
contents.

**A dead branch arm.** `validate_typed_token` guards `c == '\0' || c.is_control()`
and only the second arm was exercised. NUL has its own arm because a token with
an embedded NUL truncates at a C boundary rather than merely looking odd.

**Diff shape.** The remaining reports were an artifact of relocating types
inline: a fully-qualified `ironclaw_product_contracts::<mod>::<Item>` in a
signature turns an untouched line into a changed one. Those 17 files now import
the symbol like every other, which shrinks the diff, restores the crate's
prevailing style, and drops the lines out of the gate's denominator because a
`use` line is uninstrumentable by construction.

**One exemption, with evidence.** `factory/test_support.rs`'s
`channel_config_service` accessor: the repoint collapsed its signature onto one
line, and the merged lcov does not attribute its two integration callers back
to the composition bucket build. Both callers are named in the manifest, the
service and the port contract are covered by tests added here, and it is filed
under the same #6963 lane-attribution lane as the WS1 entries above it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(contracts): invert ironclaw_operator's product-facing ports onto product_contracts

Five operator ports and their wire vocabulary move from `ironclaw_product` to
`ironclaw_product_contracts` (PROPOSAL §6.1.3, §6.9.2): `LlmConfigService` +
`ActiveModelReader` (new `llm_config` module) and `OperatorStatusService` +
`OperatorLogsService` + `OperatorServiceLifecycleService` (new
`operator_service` module). Every implementation stays with its owner.

`ironclaw_operator`'s `ironclaw_product` dependency is dropped, not waived —
the ownership inversion §6.9.2 describes is now a Cargo fact.

Also: operator's duplicate route-mount carriers are deleted in favour of
`ironclaw_host_ingress::PublicRouteMount`, which dissolves the composition-side
repackaging shim that existed only to convert between them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): make the catalog and view doubles discriminate on their arguments

Review caught two tests of mine that asserted the double's behavior rather
than the contract, and it was right about both.

`EmptyCatalog` ignored `caller` and always returned an empty vector, so
`the_catalog_is_caller_scoped...` would have passed against a production
catalog that disclosed every user's private installs — the exact leak the
`caller` parameter exists to close (#5459 P1). It is now backed by an
ownership-filtering double, two callers, one tenant-shared tool and one private
tool each, asserting both directions of isolation and that the answer *can*
differ by caller. `OneRowView::query` ignored `_caller` and `_params` and the
test only checked the cursor; the provider now echoes all three conduit
arguments and the test asserts all three.

Both were verified red-then-green rather than assumed: dropping the caller
filter fails the catalog tests, and dropping params from the echo fails the
view test. (My first attempt at the view mutation substituted the expected
literals and passed — a reminder that a mutation which doesn't fail proves
nothing about the mutation, only about the mutant.)

The over-claim went into the PR body too, and is corrected there: a contracts
crate can pin that the port *hands the implementation the caller* and that its
shape admits a per-caller answer. It cannot pin that production filters
correctly — that is composition's implementation and composition's test. The
doc comments now say so instead of implying the stronger claim.

Also lands the CHECKLIST note this PR earned for the rest of Wave 2/3: a
move-shaped PR fails the changed-coverage gate on its first CI run, in three
distinct shapes needing three different answers, with the two mechanical habits
that shrink all three.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(arch): gate the operator port inversion, and shed skill_learning's product edge

New `reborn_operator_port_inversion.rs`. The layer matrix cannot see this edge —
`ironclaw_operator` and `ironclaw_product` are both `products`, so
`products -> products` is legal and invisible — which is why the row needs a
purpose-built gate. Four halves: the product-declared-trait residue is frozen
exact-match at zero and shrink-only; the manifest edge is proved gone through
`cargo metadata` (not a literal path, so a WS10 directory move fails loudly);
each inverted port is pinned declared-in-contracts / not-re-declared-in-product
/ implemented-by-its-owner; and the scanner is self-tested, fatal on every I/O
error, and asserts non-vacuity on every walk it performs.

Verified by negative probe rather than asserted — re-adding the manifest
dependency, a stale residue row, re-declaring a moved port in product, a
compat-alias DTO in product, and a renamed crate path each fail for their own
reason, the last with "cannot read ..." rather than a silent pass.

`ironclaw_operator` also gains AGENTS.md, CLAUDE.md, and a `BoundaryRule` — it
had none of the three, which is how its product dependency survived every
earlier sweep.

Separately, the `skill_learning.rs` stray: its entire `ironclaw_product`
dependency was one import behind a four-line adapter. `LiveSkillLearnedNotifier`
moves to composition, whose ownership the port's own doc already asserted, and
the file's product references go to zero.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(target-architecture): record the operator inversion and the strays re-verification

CHECKLIST WS5's operator row is checked with five dispositions the lead sheet
did not predict, and WS2's strays row is annotated item by item: one executed,
three corrected with the evidence that blocks them, one reassigned, one out of
scope. Two new `[decision]` rows — the contracts-family vendor-rule hole the
LLM-config port opened, and whether any live store still carries a `slack_user`
installation row.

PROPOSAL §6.1.3 and §6.9.2 carry dated amendments, including two corrections to
§6.9.2's own wording: the route clause was satisfied by deleting a duplicated
carrier rather than moving a route, and the missing guidance/boundary rule was
causal rather than cosmetic.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(extensions): split ironclaw_extension_manager out of extension_host (WS2.4)

The extension host held two jobs: lifecycle authority (the only writer of
installation state, ingress verification, activation transactions) and the
extension-management product face that arrived with #6616/#6669. PROPOSAL
§6.8.3 splits the second into its own products-layer crate so the first can
move below product in WS2's layer flip.

Six of the nine inventory items moved; three are structurally blocked and
each is recorded with its measurement. extension_host production files
naming ironclaw_product: 20 -> 13. Port-inversion residue 5 -> 4.

Behavior-free: modules move, imports repoint, one 100-line product
projection is extracted from channel_config.rs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): close the coverage-gate shapes on the WS2.2 slice

Applies the cross-slot lessons from WS2.1/WS2.3's coverage rounds to this
row's own new code, before the gate has to ask.

Pure-declaration modules gained real contract tests rather than waivers:

- `subject_route`: the port is held as `Arc<dyn _>` in five places, so object
  safety is a contract; a resolver is handed every field unswapped
  (`adapter_id`/`installation_id` are both string newtypes, so a swap would
  otherwise be silent); and an unconfigured route is absence, not failure.
  The double is **route-keyed, not fixed-answer** — two configured routes
  resolve to *different* subjects and a third resolves to `None`, so a
  resolver that ignored its argument could not pass. A fixed-answer double
  would have made all three assertions vacuous.
- `error`: `Display` is exercised for every variant, asserting each one keeps
  the text the LLM tool path forwards — `ProviderInstanceNotConfigured`
  carries the operator's exact `config set` remediation.
- `lifecycle_surface_error`: pinned against the contract's own projection
  (drift guard) *and* against absolute statuses (so both drifting together
  still fails).

`channel_config_unavailable` is extracted from a `map_err` closure because it
sat on the one path unreachable in test without fault-injecting the concrete
config service. Naming it makes the classification directly testable, and the
classification matters: a store failure is transient (retryable 503), never a
rejection (permanent 4xx) that would leave a correctly-configured channel
looking broken. The other 44 closures in this crate are pre-existing bodies
where only the type name changed (45 on the parent), so they are left alone
rather than churned on speculation.

Each new test was verified red-then-green by **mutating production code**, and
every mutation compiles cleanly so the red is an assertion failure rather than
the compiler catching the mutant:

- route key stops discriminating by conversation -> two routes collapse to one
  subject (`left: eng-subject, right: support-subject`)
- `Display` drops `{reason}` -> "rendered as ..., dropping ..."
- `lifecycle_surface_error` stops delegating -> "projection drifted for ..."
- store failure reclassified permanent -> "must be transient, got ..."

Scope is calibrated in the doc comments: the contracts-crate test pins the
port's shape and that it admits a per-route answer; it does not claim the
production resolver filters correctly — `channel_subject_routes`' own tests
(`foreign_adapter_or_installation_resolves_nothing`,
`malformed_config_json_fails_closed`) already own that.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(ws2.4): date the two row corrections and quote the text they replace

The CHECKLIST disposition named the contradiction without quoting the
inventory line it corrects or carrying a date; PROPOSAL §6.8.3 pointed at
it without the verbatim text. Both now quote both sides.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(extension_host): cover the log-sanitization guard; exempt the type-position residue

CI's second changed-coverage run came back at 99.32% line / 100% branch, with
one uncovered line and six files reporting "contributed no instrumented lines".
Two different problems, two different answers.

**The uncovered line was coverable, so it is covered.**
`lifecycle_output_decode_error`'s `tracing::debug!` body never ran under test:
with no subscriber installed `tracing` short-circuits on the null dispatcher,
so the message literal is a region that cannot be reached. The fix is not a
waiver — it is the subscriber. The test now installs a DEBUG-level
`tracing_subscriber::fmt` over a shared writer (the pattern
`ironclaw_turns/tests/agent_loop_host_contract.rs` already uses) and asserts
*both* halves of the guard's contract: the model gets `OutputDecode` and never
the serde error, **and** the serde detail is not simply dropped — it reaches
the debug log, which is where an operator diagnoses it from. Without the
subscriber a test cannot tell "logged the detail" from "discarded it", which is
the whole point. `tracing-subscriber` joins this crate's dev-dependencies for
that, with a manifest comment saying why.

**The six files are the type-position residue, and it is precedented.**
Deleting `ironclaw_product`'s re-exports forced every signature naming a moved
symbol to be rewritten; where the name sits in a *type* position — a struct
field, a function parameter, a struct-literal field's enum path — the line
changes but LLVM emits no coverage region, so it can never be covered. Nine
exact lines across six files, each entry naming the construct, filed under the
same #6963 lane the four WS1 entries use. Every line was re-read against the
source before the entry was written; none is a guess.

The balance for the PR as a whole: ten exemption lines, all type positions or
one lane-attribution accessor, against ~30 tests written for surfaces that
genuinely lacked them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(coverage): exempt the tracing message literal, with the evidence that it is an artifact

Last line on the changed-coverage gate, and the obvious reading of it is wrong.

`extension_lifecycle_capabilities.rs:217` is the message string inside a
`tracing::debug!`. It reads as uncovered — but the event body demonstrably
executes: the DEBUG-subscriber test added in the previous commit asserts the
rendered log contains that exact message, and it passes, including in the
`extension-operator` bucket, which is green.

The proof it is an attribution artifact rather than a dead path comes from that
bucket's own tracefile (run 30689416105, `bucket-extension-operator.lcov`):

  line 213 (fn signature)       hits 1
  line 214 (macro invocation)   hits 1
  line 217 (message literal)    hits 0
  line 219 (error construction) hits 1
  line 220 (closing brace)      hits 1

The function ran, the macro ran, the error was built. What LLVM does not count
is the literal: `tracing` bakes the message into the callsite's `static`
`Metadata`, so the region on that line belongs to a static initializer and is
never attributed to an executed path. Nothing short of changing the log target
moves that counter, and changing a log target is a behavior change this
move-shaped PR will not make. Every `tracing::debug!` in the workspace has the
same shape; they only escape this gate because their lines are not in a diff.

Verified by replaying the gate locally against CI's own merged lcov with this
entry in place: changed line coverage 100.00% (147/147), changed branch
coverage 100.00% (10/10).

The test stays. It is what proves the 0 is an artifact, and it still pins the
guard's real contract: the model gets `OutputDecode` and never the serde error,
and the detail reaches the debug log rather than being dropped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(extension-host): prove the transient cause survives the sanitized 503

The lifecycle warning is the entire reason this crate kept a local projection
wrapper rather than calling the contract's `From` directly — and that claim
was asserted in a doc comment and nowhere else.

`tracing` short-circuits on the null dispatcher, so under a plain unit test the
macro body never runs and a test cannot distinguish "logged the cause" from
"dropped it" — which is exactly the distinction that matters when the 503 body
is sanitized. Installing a scoped subscriber (`with_default`, so parallel tests
are unaffected) over a shared writer, following the pattern
`ironclaw_turns/tests/agent_loop_host_contract.rs` established, makes both
halves of the guard's contract assertable, and both are asserted:

- the caller's 503 is sanitized — the cause appears nowhere in the serialized
  `ProductSurfaceError`; and
- the cause is not discarded — it reaches the warning, with its stable message.

A second test pins the other direction: a rejection carries no operational
cause and must not spend a warning, so "log everything" cannot satisfy the
first test.

Both verified red-then-green by mutating production code, compiling cleanly so
the red is an assertion:
- drop the warning -> "the transient cause must survive in the log, got \"\""
- warn on every variant -> "a rejection must not emit the transient warning,
  got ... invalid binding request: bad package ref"

`tracing-subscriber` joins `[dev-dependencies]` and the `Cargo.lock` delta is
**zero** — it was already resolved for the workspace.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(coverage): recapture the extension_host floor and ratchet the manager (WS2.4)

Both numbers come from this PR's own merged coverage artifact
(reborn-integration-coverage-merged, run 30689658637), read through the
same aggregation that enforces the file. extension_host regains its
covered-line floor at 19907/23467 = 84.83% (the ratio ROSE across the
split); the manager is ratcheted from birth at 4602/5440 = 84.60%.

Verified by running the enforcing ratchet against the artifact: both
entries PASS, 17 crates pass, exit 0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): close the changed-coverage holes in the two new operator modules

Measured with the same `cargo llvm-cov --skip-functions -p … --all-targets`
shape the crate-bucket lane uses, rather than waiting for CI to report it.
Seven uncovered lines; each closed with a test, none with an exemption.

Two were real, and one of them is the kind a test can hide rather than find:

- `truncate_utf8_with_suffix`'s character-boundary back-up loop had **no**
  executing test. The multi-byte case looked covered, but the cut offset is
  256 - 16 = 240 and 2, 3, and 4 all divide 240 — so every homogeneous
  `glyph.repeat(n)` input lands exactly on a boundary and the loop body never
  runs. Driving it needs a shifted input (one ASCII byte then 3-byte
  characters), which is now the case, with an assertion that the kept prefix is
  strictly shorter than the naive offset so the loop having run is what is
  proven.
- The degenerate bound (a limit shorter than the truncation marker) is
  unreachable through the public entry point, whose bound is a constant, so it
  is exercised directly through the private helper. It is a fail-safe against
  the subtraction below it underflowing if that constant is ever lowered, and
  an untested fail-safe is how an arithmetic panic reaches a log-query path.

The other four were unexercised methods on the `LlmConfigService` double —
`delete_provider` and `complete_nearai_wallet_login`. A double method no test
calls is a contract the suite silently stopped covering, so both are now
driven, the first asserting its argument reaches the error it produces and the
second asserting both directions of its outcome.

Both modules are now at zero uncovered added production lines: `llm_config`
288/288 DA, `operator_service` 243/243.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(composition): cover LiveSkillLearnedNotifier through the real publisher

Review triage. The strays row introduced a six-argument forwarding
adapter with no test of its own, which is the shape that fails silently:
swapping `skill_name`/`feedback` compiles (both `&str`), and dropping the
`Some(owner)` wrapper compiles (the publisher takes `Option<&UserId>`)
while re-keying every learned-skill bubble onto the runtime operator's
stream instead of the user's. `skill_learning.rs`'s `StubNotifier` tests
stop at the port and cannot see either.

The new test drives the production trait object over a real
`LiveProjectionPublisher` — no double anywhere — with the runtime actor
deliberately different from the run owner, and reads the result back off
the product event stream the WebUI drains.

Red-then-green proved by mutating the adapter, not the test, and both
mutations compile:
- swap `skill_name`/`feedback`  -> left: [(["picked this up summing a
  report column"], ["csv-column-sum"])]
- `Some(owner)` -> `None`       -> owner drain empty; with the first
  assertion neutralised, the negative assertion fires on its own with
  the bubble found on the runtime actor's stream.

Also from the same review:
- `llm_config.rs`'s comment claimed `assert_not_impl_any!` "would be the
  direct form" two lines above two live `assert_not_impl_any!` calls. It
  now says what the assertions enforce and why: both request types carry
  `api_key: Option<SecretString>`, so a `Serialize` impl is what would
  let the key ride back out.
- CHECKLIST's `reborn_extension_specificity.rs` pointer named `:1177-1180`,
  which in this PR's own tree is the `capability_surface.rs` pair; the
  `lifecycle_restore.rs`/`slack` entry sits at `:1202`. Replaced the line
  range with the allowlist entry itself, which cannot drift.

Verification: fmt clean; clippy -D warnings clean on
ironclaw_reborn_composition + ironclaw_product_contracts; 66 test
binaries, 1181 passed, 0 failed across composition, product_contracts,
and the full architecture suite.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(contracts,extension-host): preserve the acquire cause and pin every HostApiError projection

Review triage for #7000.

- `import_bundle`'s decode-limiter `map_err(|_| ...)` discarded the
  `AcquireError`. The mapping is now a named `map_import_decode_acquire_error`
  that logs the bound source before mapping. Named rather than inlined so it is
  reachable from a test: nothing in the workspace calls `Semaphore::close`, so
  an inline closure would be a permanently uncovered branch that the
  changed-line coverage gate could only accept as a standing exemption. New
  regression test builds a genuine `AcquireError` from a closed semaphore and
  asserts the failure is `Transient` (retryable), not a client mistake.

- `From<HostApiError> for ProductOperationFailure` was pinned by one variant.
  It now enumerates all ten, asserts each carries its own rendering (so the
  cause cannot be flattened at the boundary) and projects to a 400, and adds an
  exhaustive `host_api_error_tag` match so a new `HostApiError` variant stops
  compiling the test instead of inheriting the blanket mapping silently.
  `InvariantViolation` is pinned as-is, not reclassified: the mapping mirrors
  product's pre-existing `From<HostApiError> for ProductSurfaceFailure` and
  changing it is a behavior change this slice does not own.

Red-then-green proved by mutating the code under test: InvariantViolation ->
Transient, flattening the reason text, and Transient -> InvalidBindingRequest
each fail the corresponding assertion.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(arch): pin the rustfmt-wrapped impl header the operator scanner reads

Review argued `split_once(" for ")` misses a wrapped `impl` header and
that the frozen-empty residue half would therefore fail open. Measured:
it does not. rustfmt indents the continuation line, and that indent is
what keeps `" for "` intact as a substring — real rustfmt output for a
long header is `impl<'a> Trait<Arg>` / newline / `    for Type<'a>`, and
the scanner reads `Trait` from it.

Pinned rather than argued: `impl_scanner_reads_the_trait_out_of_real_impl_shapes`
now carries a wrapped-header case. Proved non-vacuous by mutating the
scanner to truncate each segment at its first newline, which compiles and
fails the test:

    WrappedHeaderPort was not read: {"ActiveModelReader", "LlmConfigService",
    "Local", "OperatorLogsService", "OperatorStatusService",
    "ReturnArrowInBound"}

Also dropped the `:933` line pointer from `ironclaw_operator/AGENTS.md`:
the `include_str!` is at `:934`, so it was already stale, and nothing
verifies it. Path plus `reborn_cross_crate_include_scan.rs` locate the
debt.

Verification: fmt clean; `cargo test -p ironclaw_architecture --test
reborn_operator_port_inversion` 7 passed / 0 failed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(architecture,ci): close the review gaps on the extension_manager split

Review triage for #7003. All four are artifacts this PR introduced, not moved code.

- The new `ironclaw_extension_manager` boundary rule forbade
  `"ironclaw_reborn_cli"`, which is the crate DIRECTORY. `forbidden` entries are
  compared against `cargo metadata` package names and the CLI's package is
  `ironclaw`, so the entry could never fire — the edge it named was unguarded.
  Fixed, and pinned: `boundary_rule_names_are_package_names_not_crate_directories`
  flags any forbidden entry that is not a package but IS a directory under
  `crates/`. That discrimination matters — ~60 entries legitimately name retired
  v1 crates (`ironclaw_legacy`, `ironclaw_engine`, `ironclaw_gateway`,
  `ironclaw_tui`, `ironclaw_storage`) as reintroduction pins, and those have no
  directory. `ironclaw_reborn_cli` was the only entry in all 693 that had one.

- `production_files_naming` took a flat `files.len() >= 10` to accommodate the
  manager, which silently dropped the host's vacuous-scan guard from >20 to 10.
  The same diff had already parameterized `traits_implemented_by` for exactly
  this reason. Parameterized to match: host 21, manager 10.

- `classify-test-scope.sh` gained a `crates/ironclaw_extension_manager/*` arm
  with no self-test case, so a manager-only diff classifying
  `has_reborn_tests=false` would have gone unnoticed — the failure #6947 records
  for the stale `crates/ironclaw_product_*/*` arm. Case added.

- `coverage-floor.toml`'s "9.7k lines moved" explained an instrumented-line
  delta of 3,102 with a source-line figure. Both units are now stated with their
  measurements (source: 57,464 -> 47,794 in the host, 9,979 in the manager;
  instrumented: 26,569 -> 23,467 against 5,440) and why they do not reconcile.

Red-then-green proved by mutating the code under test: reverting the forbidden
entry to the directory spelling fails the new meta-test with the fix-it message;
removing the manager glob from the classifier fails the new self-test case
(has_reborn_tests=false); raising the manager's file floor to 40 fails only the
manager call site, proving the floor is per-call-site and consumed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(contracts): resolve the ProductSurfaceFailure linchpin (WS2.2)

`ironclaw_extension_host` used `ironclaw_product`'s internal workflow error
as its own lifecycle error vocabulary across 19 production files — WS2.1's
recorded linchpin, blocking half the port-inversion residue and the layer
flip. Measured with `#[cfg(test)]` stripped, it constructs exactly six
variants (150 sites), all plain-`String` or unit, and none of the two
kernel-typed ones that kept the enum out of contracts.

The boundary half is now
`ironclaw_product_contracts::error::ProductOperationFailure`;
`ironclaw_product` keeps `ProductSurfaceFailure` unchanged in shape and
absorbs it with a total, payload-preserving `From`. The projection to
`ProductSurfaceError` is defined once, in contracts, and product's
`lifecycle_product_surface_error` delegates its six shared arms to it so the
two paths cannot drift. Only the logging stayed with each caller — contracts
may not log.

Narrowing the enum instead was rejected on evidence: `auth_continuation.rs`
matches all eight `TurnErrorCategory` values structurally and distinguishes
two the sanitized projection collapses, and constructs by matching
`TurnError` variants the projection cannot express — so narrowing is lossy
in a live auth path.

Unlocks `ProductConversationSubjectRouteResolver` (trait residue 6 -> 5, with
its route key and request type) and takes extension_host's files naming the
workflow error 19 -> 2. Corrects the two surviving residue reasons, which
named the error rather than the real blocker.

Regression coverage: nine crate-tier tests including the projection-agreement
pin and the `From` totality pin, plus two new architecture gates (frozen
residue files; the contract error names no kernel type), each verified by
negative probe. Extension-specificity allowlist shrinks 130 -> 129.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(arch): apply the parent's scanner hardening to the WS2.2 half

The merge brought in WS2.1's review fixes (I/O errors fatal, comments and
strings stripped *before* `#[cfg(test)]` brace matching). Both apply verbatim
to `production_files_naming`, which this branch added after that review:

- An unreadable file was silently skipped, which is exactly how the frozen
  residue-file scan would go quietly vacuous. Now fatal, matching the three
  other readers in the file.
- The strip order was backwards. A `{` inside a comment or string literal can
  desynchronise the `#[cfg(test)]` brace matcher, so comments and strings go
  first. Re-probed both directions afterwards: a code reference still trips
  the gate, a comment mentioning the type (now with an unbalanced brace) still
  does not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(contracts): close the coverage-gate shapes on the WS2.2 slice

Applies the cross-slot lessons from WS2.1/WS2.3's coverage rounds to this
row's own new code, before the gate has to ask.

Pure-declaration modules gained real contract tests rather than waivers:

- `subject_route`: the port is held as `Arc<dyn _>` in five places, so object
  safety is a contract; a resolver is handed every field unswapped
  (`adapter_id`/`installation_id` are both string newtypes, so a swap would
  otherwise be silent); and an unconfigured route is absence, not failure.
  The double is **route-keyed, not fixed-answer** — two configured routes
  resolve to *different* subjects and a third resolves to `None`, so a
  resolver that ignored its argument could not pass. A fixed-answer double
  would have made all three assertions vacuous.
- `error`: `Display` is exercised for every variant, asserting each one keeps
  the text the LLM tool path forwards — `ProviderInstanceNotConfigured`
  carries the operator's exact `config set` remediation.
- `lifecycle_surface_error`: pinned against the contract's own projection
  (drift guard) *and* against absolute statuses (so both drifting together
  still fails).

`channel_config_unavailable` is extracted from a `map_err` closure because it
sat on the one path unreachable in test without fault-injecting the concrete
config service. Naming it makes the classification directly testable, and the
classification matters: a store failure is transient (retryable 503), never a
rejection (permanent 4xx) that would leave a correctly-configured channel
looking broken. The other 44 closures in this crate are pre-existing bodies
where only the type name changed (45 on the parent), so they are left alone
rather than churned on speculation.

Each new test was verified red-then-green by **mutating production code**, and
every mutation compiles cleanly so the red is an assertion failure rather than
the compiler catching the mutant:

- route key stops discriminating by conversation -> two routes collapse to one
  subject (`left: eng-subject, right: support-subject`)
- `Display` drops `{reason}` -> "rendered as ..., dropping ..."
- `lifecycle_surface_error` stops delegating -> "projection drifted for ..."
- store failure reclassified permanent -> "must be transient, got ..."

Scope is calibrated in the doc comments: the contracts-crate test pins the
port's shape and that it admits a per-route answer; it does not claim the
production resolver filters correctly — `channel_subject_routes`' own tests
(`foreign_adapter_or_installation_resolves_nothing`,
`malformed_config_json_fails_closed`) already own that.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(extension-host): prove the transient cause survives the sanitized 503

The lifecycle warning is the entire reason this crate kept a local projection
wrapper rather than calling the contract's `From` directly — and that claim
was asserted in a doc comment and nowhere else.

`tracing` short-circuits on the null dispatcher, so under a plain unit test the
macro body never runs and a test cannot distinguish "logged the cause" from
"dropped it" — which is exactly the distinction that matters when the 503 body
is sanitized. Installing a scoped subscriber (`with_default`, so parallel tests
are unaffected) over a shared writer, following the pattern
`ironclaw_turns/tests/agent_loop_host_contract.rs` established, makes both
halves of the guard's contract assertable, and both are asserted:

- the caller's 503 is sanitized — the cause appears nowhere in the serialized
  `ProductSurfaceError`; and
- the cause is not discarded — it reaches the warning, with its stable message.

A second test pins the other direction: a rejection carries no operational
cause and must not spend a warning, so "log everything" cannot satisfy the
first test.

Both verified red-then-green by mutating production code, compiling cleanly so
the red is an assertion:
- drop the warning -> "the transient cause must survive in the log, got \"\""
- warn on every variant -> "a rejection must not emit the transient warning,
  got ... invalid binding request: bad package ref"

`tracing-subscriber` joins `[dev-dependencies]` and the `Cargo.lock` delta is
**zero** — it was already resolved for the workspace.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(contracts,extension-host): preserve the acquire cause and pin every HostApiError projection

Review triage for #7000.

- `import_bundle`'s decode-limiter `map_err(|_| ...)` discarded the
  `AcquireError`. The mapping is now a named `map_import_decode_acquire_error`
  that logs the bound source before mapping. Named rather than inlined so it is
  reachable from a test: nothing in the workspace calls `Semaphore::close`, so
  an inline closure would be a permanently uncovered branch that the
  changed-line coverage gate could only accept as a standing exemption. New
  regression test builds a genuine `AcquireError` from a closed semaphore and
  asserts the failure is `Transient` (retryable), not a client mistake.

- `From<HostApiError> for ProductOperationFailure` was pinned by one variant.
  It now enumerates all ten, asserts each carries its own rendering (so the
  cause cannot be flattened at the boundary) and projects to a 400, and adds an
  exhaustive `host_api_error_tag` match so a new `HostApiError` variant stops
  compiling the test instead of inheriting the blanket mapping silently.
  `InvariantViolation` is pinned as-is, not reclassified: the mapping mirrors
  product's pre-existing `From<HostApiError> for ProductSurfaceFailure` and
  changing it is a behavior change this slice does not own.

Red-then-green proved by mutating the code under test: InvariantViolation ->
Transient, flattening the reason text, and Transient -> InvalidBindingRequest
each fail the corresponding assertion.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(conversations): fix the conversations/threads naming trap (WS5)

Rename the five names `ironclaw_conversations` shared with
`ironclaw_threads` and unify the external actor/conversation pair onto its
one home in `ironclaw_extension_contracts`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(attachments): widen ironclaw_attachments to own its ports and ceilings (WS5)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(target-arch): record the WS5 naming-trap and attachments-widening outcomes

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(conversations): state the threads boundary in the crate doc

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(attachments,conversations,product): close the review gaps on the WS5 naming-trap slice

Review triage for #7005.

- `project_scoped.rs`: delete a stranded `///` block that described
  `ProjectScopedAttachmentReader`, ended mid-clause, and rustdoc was attaching
  to the `InboundAttachmentLander` impl. The module doc already records that the
  reader stays in `ironclaw_product`.

- `cleanup_stale`'s third pre-scan exit — an in-root reference whose relative
  depth is not `<date>/<message>/<file>` — had zero coverage anywhere in the
  tree. Extended the existing empty/unowned fail-closed test rather than adding
  a redundant one, asserting the `Internal` code and that the seeded batch
  survives the aborted pass.

- `stored_refs` / `ids`: state the rollback boundary. Compatibility is
  upgrade-only by decision — this build reads `thread_id`/`message_id` and
  writes only `topic_id`/`reply_target_message_id`, so a record written here and
  read by a pre-rename binary silently collapses every threaded route to its
  conversation root. Dual-writing is refused on the row's own type-placement
  rule, and is self-defeating besides: verified that a reader with
  `#[serde(alias)]` rejects a record carrying both spellings
  (`duplicate field \`topic_id\``).

- `gate_routes`: pass `None` for the source branch's reply target. Provably
  behavior-identical (`conversation_fingerprint` hashes space + conversation +
  topic and excludes the reply-target hint), but the previous spelling could
  only be read as correct together with the fingerprint body, and it reads as a
  per-message id baked into a stable route key.

- `run_delivery_contract`: the gate-route test could not see any of that. Its
  prompting event is now a threaded reply carrying both a topic and a reply
  target, which makes the source branch's key distinguishable from the
  delivered-message loop's, and it pins the fingerprint's reply-target
  independence directly.

- `inbound.rs`: rename the private `session_thread_service` field/param to
  `conversation_service`. `SessionThreadService` is the `ironclaw_threads` type
  this PR exists to stop colliding with.

- CHECKLIST WS5 sub-item 3: dated amendment quoting the sentence it annotates;
  the re-word/re-home obligation is now tracked in #7010.

No test was added and none removed — two were extended. Red-then-green proved
by mutating the code under test, not the tests: the malformed-reference branch
downgraded to `continue`; the fingerprint widened to include the reply target;
and three separate breaks of the source branch (topic keyed off the reply
target, topic dropped, branch records nothing). An earlier version of the
gate-route assertion passed under all three and was reworked until it failed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(extension-host): close the WS2.2 changed-coverage gate with tests, not waivers

The `ProductSurfaceFailure` -> `ProductOperationFailure` repoint put 137
already-uncovered error-path lines into the changed-line denominator: the new
name is two characters longer, so every construction site's first line changed
and rustfmt re-wrapped the arms that crossed 100 columns. The gate ran on this
PR for the first time (stacked PRs never triggered it) and reported 67.38%
line / 83.33% branch.

Measured, not assumed. Replaying CI's own merged lcov (run 30706965794) against
the base lcov from `main` @ 569d8e4895 (run 30705915898) shows 122 of the 137
lines are 1:1 rename-only replacements that each scored `DA:<line>,0` at their
pre-image, and the other 15 are rustfmt re-wraps of those same lines. Zero are
"no LLVM region" type positions -- all 137 carry a DA record, because the gate
intersects the changed set with DA records, so region-less lines never enter
the denominator at all.

60 of those lines get real tests here rather than a waiver
(ironclaw_extension_host 419 -> 438 tests), covering every pure boundary mapper
the repoint touched:

* the retryable-vs-caller-error split in `product_lifecycle`,
  `lifecycle_restore`, `active_publication`, `lifecycle_product_service`,
  `extension_activation_credentials` and `hosted_mcp_manifest`;
* `map_skill_error`'s `FilesystemDenied -> BindingAccessDenied` projection,
  which is an authorization outcome and must not read as retryable;
* both post-install activation fail-open classifiers (service tier and
  capability tier), which decide which activation failures are swallowed
  behind a successful install -- they must agree, and now both are pinned;
* `ensure_caller_may_mutate_tenant_installation`, the tenant-admin guard on
  shared installations, pinned on the denial and on both ways through;
* `UnavailableExtensionActivationCredentialGate`, pinned fail-closed;
* `pending_manifest`'s hosted-MCP name and client-profile input guards, which
  are what keep caller text out of interpolated manifest TOML;
* `prepare_install`'s refusal of a retained definition that disagrees with the
  catalog.

Every one was verified by mutating the code under test and confirming the
assertion went red -- not the compiler. 12/12 mutants killed.

The remaining 77 lines and 1 branch are exempted with per-site evidence in four
classes: map_err arms on argument-free infrastructure constructors that cannot
fail from any input; defensive arms dominated by the guard immediately above
them; paths gated behind `VerifiedAuthClaim`, which has no constructor outside
`ironclaw_host_api`; and pre-existing fault-injection paths inside async `&self`
service methods, each still scoring 0 hits at its pre-image in the base lcov.

Also corrects a stale entry inherited from WS2.1: the `tracing` message-literal
exemption named line 217 (`?error,`) instead of 218 (the literal), and its
evidence block was off by one throughout. Inert today because neither line is in
this PR's changed set, but it would have silently failed to apply the moment a
PR touched the real line -- the stranded-exemption failure mode the manifest is
supposed to prevent.

Local gate: 100.00% line (343/343), 100.00% branch (2/2), exit 0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(coverage): tighten the WS2.2 exemption evidence to what the lcovs actually show

Three reasons overstated their evidence. Corrected against the base lcov:

* `channel_subject_routes.rs` 231-233 have no 1:1 pre-image because the hunk
  is 1->3 (`@@ -217 +231,3 @@`); base line 217 held the whole closure and
  scored `DA:217,0`, so it is one uncovered closure re-wrapped, which the
  reason now says instead of claiming a per-line pre-image.
* `product_lifecycle.rs` 783-786 map to base 785-787, where the `.map_err(`
  call scored 136 hits and only the closure body scored 0. The reason now
  names both numbers rather than implying the whole span was cold.
* `test_support.rs` 633-634 are the only genuinely NEW lines in this PR -- a
  `.map_err(ProductSurfaceFailure::from)` conversion, not a rename. Calling
  them "rename-only" was wrong. The honest evidence is that every line of the
  enclosing `#[cfg(feature = "test-support")]` helper scored 0 hits at base
  (624-635), so the conversion was added to an already-dead seam.

The header block's "the remaining 15 are rustfmt re-wraps" is corrected to
13 re-wraps plus those 2 new lines. No line numbers changed; gate still
100.00% line (343/343), 100.00% branch (2/2), exit 0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(ws5): close the changed-coverage gate on the naming-trap slice

The gate attaches for the first time now that #7005 targets main. It
reported 21 uncovered changed lines, 8 uncovered changed branch arms, and
one file contributing no instrumented lines at all.

Every genuinely reachable hole is closed with a real test, driven through
the caller that owns the side effect rather than the helper:

- `ProjectScopedAttachmentLander::rollback` refuses malformed batch
  references. Rollback deletes a whole batch directory, so each guard in
  `attachment_batch_parent` is a delete-target check; the test lands a
  real batch first and asserts a refused rollback never removes it.
- `map_external_ref_error`'s non-`InvalidIdentifier` fallback keeps this
  crate's error vocabulary and carries the source message verbatim.
- The durable `RebornFilesystemConversationServices` forwards the
  inbound-message half of its contract (accept + replay), which only
  `InMemoryConversationServices` had ever exercised.
- `external_ref` maps `ProductAdapterError` to `InvalidMaterialization`
  without leaking a `RedactedString` detail, both directly and through
  `trigger_conversation_fields`.
- The two standalone attachment test-support accessors land bytes and
  read them back through both returned read views. They had no callers
  anywhere in the repo; `#[allow(dead_code)]` on the impl block hid it.
- `delivered_conversation_fingerprints` drops a vendor message ref that
  cannot key a route, covering the two reachable `Err` arms.

Two exemptions, both with per-site evidence, neither a shortcut:

- `types.rs`: seven `pub struct` / field declarations from the DTO
  rename. A serde round-trip contract test for all five types was
  written first to test the obvious hypothesis that the derives would
  instrument them; re-measuring showed the file still reports the
  identical 42 DA records over the identical 17..312 span, because
  derive-generated code is `#[automatically_derived]` and emits no
  region at the declaration site. The round-trip test is kept: it pins
  the persisted encoding across the rename, which is the risk the WS5
  CHECKLIST row actually cares about.
- `gate_routes.rs` branch arms 45/58/74: every argument is an accessor
  read off an already-validated `ExternalConversationRef`, whose fields
  are private and whose only value-producing paths all run
  `validate_external_id`. Re-validating a value that already passed a
  pure predicate cannot fail. The two sibling sites that also take the
  unvalidated vendor ref are tested, not exempted.

Each new test was mutation-verified red-then-green.

* fix(architecture,extensions): close the paranoid-architect review findings on the WS2.4 split

Review pass over #7003 (four parallel deep reviews; no Critical/High — the
move itself verified behavior-free). Everything found, fixed here:

Gate hardening (crates/ironclaw_architecture/tests):
- ratchet_support gains cfg_test_only_files: files reachable only through
  #[cfg(test)] mod chains (incl. #[path] overrides) are classified test code.
  channel_host/e2e_auth_challenge.rs — a fake AuthChallengeProvider impl
  wearing a production filename — no longer counts toward any residue row,
  implementor pin, or error-vocabulary floor. Pinned by a real-tree test that
  was red before the #[path] resolution landed.
- Trait matching is qualified by a whole-token crate reference (names_crate),
  so a name-colliding local trait can no longer satisfy an implementor pin,
  and a manifest rename of ironclaw_product can no longer blind the manager
  residue scan (metadata tie: dep exists iff the residue list is non-empty,
  never renamed).
- The manager gets its own product-defined-trait residue freeze (twin of the
  host's, frozen at ExtensionCredentialSetupService).
- each_half_of_the_split_kept_its_own_job: authority checks are symmetric
  across file/directory spellings and back every module with a content
  witness, so an empty stub cannot satisfy retention.
- untrusted_ingress_paths scan roots fail loudly on a missing root instead of
  silently dropping a tree from the guard.
- Fork-check message names its two-crate scope.
All new checks probed red-for-the-right-reason and reverted (hollow witness,
product alias, stale scan root, authority-as-directory, unguarded secret).

Manifest hygiene:
- extension_host drops the ed25519-dalek dep orphaned when ironhub moved.
- Ten manager deps used only by tests/the test_support fixture leave the
  production graph: fixture deps become test-support-gated optionals, pure
  test deps move to [dev-dependencies]. All three build shapes verified.

Manager/host code:
- channel_config: the pub resolved_manifest widening is narrowed to a
  declares_admin_configuration() boolean — the manifest read stays internal.
- admin_configuration view: secret field values are redacted in render_group
  (same defense-in-depth as render_state), with a sentinel regression test;
  the service-error table test now pins code/kind beside status/retryable.

Docs (single-source-of-truth):
- families/extensions.md confesses the direct auth/host_runtime deps and the
  transitional dep tail the four-crate target does not name.
- The residue characterization says what the list actually holds: DTOs,
  capability-id constants, and two port-inversion residues.
- 20 -> 13 becomes 20 -> 12 (the 13th was the cfg(test)-only fixture);
  coverage-floor/CHECKLIST stale "recapture owed" drafts corrected to the
  shipped recapture; line counts de-precisioned; stale exemption comment
  repointed to the manager.

Verification: architecture 143/0; manager 64/0 (--all-features);
extension_host 388/0 (--all-features); cargo check --workspace --all-targets
--all-features 0 errors / 0 warnings; clippy -D warnings clean on all three
touched crates; both CI script self-tests pass; cargo metadata --locked clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(conversations): keep the durable grammar so the rename survives a rollback

Human review on #7005 (serrrfirat, `stored_refs.rs:53`) and CodeRabbit
(`stored_refs.rs:38`) both found the same real defect, and the module's own
refutation was aimed at a different proposal than the one that fixes it.

`stored_refs` refuted DUAL-WRITING (emitting both spellings), correctly: a
reader with `#[serde(alias)]` rejects a record carrying both as a duplicate
field. But the ask was WRITE-LEGACY / READ-EITHER, which that objection does
not touch. Measured against `origin/main`, the released readers are
`RawExternalConversationIdentity` and the `ExternalConversationRef` wire struct
in `ironclaw_conversations/src/ids.rs`; both name `thread_id`/`message_id` and
carry no aliases. So a record this build wrote read back as `None` on a
rollback, with no error.

Worse than the reported "remaps to the conversation root": the identity keys
`BindingKey`, and `StoredConversationState::into_state` rebuilds the map with
`Vec<(K, V)>::into_iter().collect()`, so two threaded bindings in one
conversation collapse onto one key and the earlier one is dropped.

- `conversation_ref::serialize` now writes `{space_id, conversation_id,
  thread_id, message_id}` through a borrowed representation; both spellings
  still read, and a record carrying both still fails closed.
- `ExternalConversationIdentity` gets a matching hand-written `Serialize`.
- `stored_refs::actor_ref` deleted: the actor change was additive, so the
  canonical impls already do everything it did. `actor_serde_needs_no_adapter`
  pins that equivalence instead of asserting it.

Tested through the durable store, not a surrogate
(`filesystem_conversation_services_persist_external_refs_in_the_durable_grammar`
walks every key of the real persisted document). Both fixes verified
red-then-green by mutating the writers: reverting the ref writer fails the unit
AND store tests; making the identity emit `topic_id` fails only the store test,
naming all six sites — which is exactly the gap the review reported.

Also from the same review round:

- Rename `product/src/scoped_fs/attachment_landing.rs` -> `attachment_reader.rs`
  now that the lander moved out (serrrfirat, `attachment_landing.rs:1`).
- Reuse `ratchet_support::strip_comments_and_strings` instead of a third local
  copy; the extended self-test fixture proves the deleted line-based copy leaked
  block comments (serrrfirat, `reborn_conversations_threads_attachments.rs:135`).
- Amend `docs/reborn/contracts/conversation-binding.md` and the conversations
  CLAUDE.md for the renamed service, the moved ref pair, and `topic_id`
  (serrrfirat, `conversations/src/lib.rs:38`).
- Widen the `crates/AGENTS.md` attachments row and record the justified WebUI
  edge in `ironclaw_webui/AGENTS.md` (serrrfirat, `attachments/src/lib.rs:23`).
- Assert the topic participates in the fingerprint, so the route-membership
  check cannot pass vacuously (CodeRabbit, `run_delivery_contract.rs:1347`).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(operator,contracts): close the WS5 operator review findings

Human review on #7004 (serrrfirat). Five findings taken, one deferred with a
named home, one answered in place.

- Composition calls operator's route mount through the crate-root facade
  (`ironclaw_operator::nearai_login_callback_mount`) instead of naming the
  three-segment module path. The deep path was pre-existing — it lived in the
  composition shim this PR deleted — but the shim was what encapsulated it, so
  the facade re-export is this PR's to add. `llm_admin/mod.rs` already
  re-exports free functions (`apply_stored_api_key`, `resolve_reborn_runtime_llm`),
  so this follows the existing convention rather than inventing one.

- `map_llm_config_error` deleted; its 10 call sites across t…
l3ocifer pushed a commit to l3ocifer/frick-ironclaw that referenced this pull request Sep 3, 2026
…ve-2 main (nearai#7032)

Audits docs/reborn/target-architecture/ (plus crates/AGENTS.md and the
crate guides Wave 2 touched) against merged main at 3be5f05, after
nearai#6996, nearai#6998, nearai#7002 and nearai#7018.

Docs-only: 13 .md files, no code, no tests. House style throughout —
dated amendments, prior text quoted verbatim wherever a clause is
corrected, nothing rewritten silently and no decision record deleted.

The two structural findings the wave produced and nobody had written
down: same-layer edges are invisible to the layer matrix by
construction, so the exception count could never have moved in Wave 2
and each removal needed its own purpose-built shrink-only gate
(PROPOSAL §8.1, §8.2, §11.1); and the changed-line coverage policy —
90% lines, branch coverage ungated since nearai#7013 — was recorded in no
document at all, alongside a stranded-exemption failure mode the new
pre-existing-uncovered exclusion creates (CHECKLIST WS10).

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

This branch was successfully deployed

No deployments
ironclaw-ci-preview / ironclaw-pr-6998 — b36c9726 Deployed Aug 1, 2026 by railway-app[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: low Changes to docs, tests, or low-risk modules scope: dependencies Dependency updates scope: docs Documentation size: XL 500+ changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant