refactor(contracts): consolidate the Wave 2 port-inversion stack (WS2.2, WS2.4, WS5) - #7018
Conversation
…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>
…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>
…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>
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>
`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>
Parent moved three commits (scanner made fail-loud, moved-port surface coverage, doc-count reconciliation). One conflict, in `ironclaw_product_contracts/CLAUDE.md`, where both sides rewrote the same three passages: - Module count: took the parent's shipped-modules-plus-dev-seam framing and set it to nineteen (WS2.2 adds `error` and `subject_route`). The parent independently fixed the pre-existing off-by-one, so WS2.2's note about it is dropped rather than duplicated. - Port inventory: took the parent's split framing (implemented-by-extension_host vs only-consumed) and folded WS2.2 in — ten implemented + two consumed = twelve relocated, residue five. - Residue reasons: kept WS2.2's corrections, which replace the `ProductSurfaceFailure` justification the parent's text still carried. `ironclaw_product/CLAUDE.md`'s eleven/nine counts updated to twelve/ten for the same reason. The scanner file auto-merged; both halves re-verified. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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>
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>
…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>
…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>
…'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>
…ys 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>
…on_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>
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>
…lace 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>
…pe-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>
…-surface-failure Parent landed its changed-coverage work: the exemptions file, the discriminating-double fixes, and a WS2.1 coverage-gate note for later slots. One conflict, in CHECKLIST.md, and it was an insertion collision rather than a disagreement — both sides appended to the same point in the WS2 section. Both kept: the parent's coverage-gate note first, because it annotates the WS2.1 row directly above it, then this row's linchpin and `[decision]` entries as new checklist rows. Checked rather than assumed, because `changed-coverage-exemptions.toml` is **exact-line** and this branch inserts two `pub mod` lines into `ironclaw_product_contracts/src/lib.rs`: the sole exemption for that file targets line 33 (`#![warn(unreachable_pub)]`) and both insertions land below it, so line 33 still resolves to the same source line on both sides. No exemption in the file names any of the four files this row adds code to, and none is made stale by the merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…o ws2/extension-manager-split # Conflicts: # crates/ironclaw_extension_host/src/channel_config.rs
…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>
One conflict, in CHECKLIST.md, where both sides rewrote adjacent rows: slot 4 checked the `extension_manager` split row and added its `[decision]`, this branch rewrote the strays row beneath it. Resolution takes both wholesale — neither side touched the other's row. Re-derived one of my own claims against the merged tree rather than carrying it forward. The strays row said the `nearai_mcp` fork's removal belonged to the `extension_manager` split; that row has now landed and deliberately did *not* take `available_extensions.rs` (the catalog is lifecycle authority, not management UX), so the claim was stale on arrival. Corrected to name the `include_str!` reach-in row, which owns the same mechanism and names nearai-mcp by name — and recorded the sharper finding the merge exposed: the fork is what keeps `extension_host` from depending on `ironclaw_operator`, which the `products` -> `loops` re-layer would make illegal. Deleting it in favour of operator's copy would block the flip, not help it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d 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>
…nager (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>
…uct-surface-failure Parent landed its own `tracing` coverage work: a DEBUG-subscriber test for the log-sanitization guard and an exemption for a message literal. One conflict, and it is a **convergence rather than a disagreement**: both sides independently added `tracing-subscriber = "0.3"` to `[dev-dependencies]` for the same underlying reason (the null dispatcher short-circuits, so a test cannot tell "logged it" from "dropped it"). Kept one entry with a comment covering both levels this crate now needs it at — DEBUG for the parent's lifecycle sanitization tests, WARN for this row's transient-cause test. Checked for collision with the parent's new exemption: it names `extension_lifecycle_capabilities.rs:217` and nothing in this row's slice. This branch needs no message-literal exemption of its own, and that was measured rather than assumed -- both `tracing::warn!`s here put the invocation and the message literal on a single source line, so the line carries the invocation's count. `cargo llvm-cov --lib` reads line 402 of `lifecycle_product_service.rs` at count 3, not 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ator 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>
…o ws2/extension-manager-split # Conflicts: # crates/ironclaw_extension_manager/src/extension_lifecycle_capabilities.rs # crates/ironclaw_reborn_composition/src/factory/production_backend_assembly.rs
…o tmp-7003 # Conflicts: # crates/ironclaw_extension_host/Cargo.toml
…blisher 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>
…ry 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>
…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>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/ci/test-reborn-changed-coverage.sh (1)
1101-1217: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMissing-
ghself-test is still not hermetic (unresolved from a prior review).This block's missing-binary case still does
rm -f "${fake_gh}/gh"before callingrun_gate_fetch. Deleting the stub leaves the runner's preinstalledghonPATH, so the assertioncheck_rc "a missing gh binary counts every changed line" 1can pass via the realghfailing against the fixture repo, not via the intended not-found/OSErrorfallback ingh_api. This does not validate the enforcement claim the test's own name makes.Replace the deletion with an unexecutable stub (e.g. write a non-executable
${fake_gh}/ghfile) soPATHnever falls through to a realgh, and add a focused Python-level test inscripts/ci/test_reborn_changed_coverage.pyfor the not-foundOSErrorpath ofgh_api/download_base_coverage.🐛 Proposed fix
-rm -f "${fake_gh}/gh" +cat >"${fake_gh}/gh" <<'EOF' +#!/usr/bin/env bash +exit 127 +EOF +chmod -x "${fake_gh}/gh" run_gate_fetch check_rc "a missing gh binary counts every changed line" 1 check_text "the missing binary is explained" "Base coverage: NOT APPLIED"As per path instructions: "these scripts gate merges (check_no_panics.py, check_gateway_boundaries.py, pr-labeler.sh), so behavior changes need matching workflow updates" — a self-test that claims to gate a fallback path must actually exercise it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/ci/test-reborn-changed-coverage.sh` around lines 1101 - 1217, Make the missing-gh case hermetic by replacing the executable stub with a non-executable ${fake_gh}/gh file instead of deleting it, ensuring PATH cannot fall through to a system gh binary while preserving the expected failure assertions. In scripts/ci/test_reborn_changed_coverage.py, add a focused test that exercises the not-found OSError handling in gh_api or download_base_coverage and verifies the fallback is enforced.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@scripts/ci/test-reborn-changed-coverage.sh`:
- Around line 1101-1217: Make the missing-gh case hermetic by replacing the
executable stub with a non-executable ${fake_gh}/gh file instead of deleting it,
ensuring PATH cannot fall through to a system gh binary while preserving the
expected failure assertions. In scripts/ci/test_reborn_changed_coverage.py, add
a focused test that exercises the not-found OSError handling in gh_api or
download_base_coverage and verifies the fallback is enforced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0588c2d6-1640-4247-9e5b-6824bb3bc815
📒 Files selected for processing (4)
.github/workflows/reborn-tests.ymlscripts/ci/test-reborn-changed-coverage.shscripts/ci/test_reborn_changed_coverage.pytests/integration/changed-coverage-exemptions.toml
…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>
…-tree coverage (WS2 + nearai#7083) (nearai#7094) * refactor(extensions): re-layer the extension registry to substrates (WS2) `ironclaw_extensions` moves `loops` -> `substrates`, which is PROPOSAL §6.8.1's assignment for the crate. Four `LAYER_MATRIX_EXCEPTIONS` fall out with it and the WS0 ratchet baseline drops 10 -> 6. The four were one edge under four names: `host_runtime`, `capabilities`, `mcp` and `scripts` — kernel/runtimes-tier crates — reaching the registry for its manifest DTOs. None is waived and none of those edges is deleted; the registry moved down to a layer every tier above may legally reach. §6.8.1 predicted two of them ("Layer substrates legalizes `capabilities -> extensions` and `host_runtime -> extensions`"); it undercounted, and the amendment records that. The one edge that blocked the move is gone rather than waived. At `substrates` a crate may name only `contracts`/`substrates`, and `ironclaw_extensions` named `ironclaw_trust` (kernel) for exactly one type, `TrustPolicyInput`, used by one method. Every field of that type is already `host_api` vocabulary — `PackageIdentity`, `RequestedTrustClass`, `BTreeSet<CapabilityId>` — and the type names no decision, ceiling, or provenance, so it is requested-trust vocabulary like everything else in `ironclaw_host_api::trust`. It moves there, which is §6.8.1's own prescription ("`trust`-vocabulary via `host_api`"), and every consumer's import is repointed rather than shimmed behind a re-export (.claude/rules/type-placement.md). Behavior-free: a type relocation, a layer declaration, and the exception entries the layer change makes unreachable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(extensions): source package manifests from the inventory, not include_str! (WS2) The WS2 `include_str!` row's named targets — gmail, github, nearai-mcp — plus the two cross-crate manifest reach-ins nearai#7018 added. **nearai-mcp gets its inventory module.** It was the one asset directory of twelve with no module in `ironclaw_extension_support::packages`, so `available_extensions.rs` embedded its manifest and three assets directly. Those embeds move to `packages/nearai.rs` beside every other package's, and `available_extensions.rs` consumes `nearai_bundle()`. It is deliberately **not** a `PACKAGES` entry, and the module says why at length: every other entry is a config-free `fn() -> PackageBundle`, but NEAR AI's shipped `[mcp].server` is a placeholder the host rewrites from the operator's LLM-admin bootstrap config. A config-free builder cannot produce that value, so the *embeds* live with the inventory and the *patch* stays with the endpoint authority. `PackageBundle::manifest_toml` is already a `Cow` precisely so the patched manifest is representable. This makes `ironclaw_extension_support` a normal dependency of `ironclaw_extension_host` instead of a `test-support`-only one. No new dependency cone: the binary already links it, and `extension_host` already built against it under that feature. **github and gmail fixtures are inlined.** Both were test-only reach-ins into a shipped product manifest. Each test needs one property — a v3 manifest asserting first-party trust; a no-channel manifest with an `[admin_configuration]` group — now spelled out in the test that needs it instead of borrowed from 200 lines it does not own. **The two cross-crate sites route through `bundled_packages()`.** slack and telegram carry adapter crates, so `extension_manager`'s manifest reach-ins were classified cross-crate. The tests' stated intent — project the *shipped* field set, not a drifting fixture — is unchanged; only the path changed, from a relative file path to the inventory that owns the bytes. Measured with the §11.2.7 scan: escaping sites 133 -> 128, cross-crate 19 -> 17. `REPORT_ONLY` stays `true` — the 17 survivors belong to three other owners (the support crate's own slack/telegram package crates, host_runtime's seven memory-provider embeds, and four test-only doc reach-ins in `operator`/`product`), none of which this row owns. The doc amendment names each. The `("…/available_extensions.rs", "nearai-mcp")` specificity carve-out is deleted with the embed it covered; the allowlist is shrink-only and staleness-checked, so leaving it would fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci: make Reborn coverage aggregation nested-tree-safe (nearai#7083) Coverage was structurally dark for every crate under `crates/extensions/`. `reborn_coverage_lcov.py` keyed on `crates/(ironclaw_[A-Za-z0-9_]+)/` — a literal path shape requiring `ironclaw_*` *directly* under `crates/` — and the `if match:` it gated guards the global aggregate as well as the per-crate table, so an unmatched record left both numerator and denominator. Five crate directories (~33.7k instrumented lines) contributed nothing to a gate reading `enforce = true`, and nothing said so: there is no `else`, no counter, no warning. This is a regression, not a never-worked condition. All five were flat `crates/ironclaw_*` until nearai#7037 colocated packages three days ago; the module has one commit in its history and predates the move. The `[global]` floor was captured 2026-07-30, before that, so the denominator has silently shrunk under the gate that enforces it. A better regex cannot fix it. Four of the five directory basenames contain no `ironclaw_` at all (`packages/slack`, `telegram`, `mem0`, `memory-native` — PROPOSAL §5.1 names package directories by extension identity), and a greedy nested pattern mis-attributes an in-crate `src/ironclaw_*/` module directory to a crate that does not exist. So the fix is the one the *merge* script one step upstream already applies: anchor on the discovered crate inventory (`crate_tree.py`). The data was always in the merged lcov; only the aggregator was blind, and the two disagreeing is what made the hole silent. Three consequences worth stating: - **The accounting key is the crate directory basename** — what `crate_tree.crate_directory()` resolves by and what `classify-test-scope.sh` keys on. Every existing floor and exemption key is already a basename, so none churns; basenames also survive the family moves still ahead (`crates/ironclaw_llm` -> `crates/substrates/ironclaw_llm`). - **Separate workspace roots are excluded explicitly**, checked *before* the crate pattern. "Outermost wins" would otherwise attribute `packages/slack/wasm-src/` to `packages/slack/`, putting never-compiled guest code in a denominator. Same precedence `reborn_changed_coverage.py` applies. - **It fails closed.** No discoverable crate tree is now a refusal, not a percentage computed over an empty inventory — the WS10 rule this whole class of bug violates. Regression proof: six new cases (A6b/A6c/A6d/A6e, R19/R19b) covering a nested crate in the table *and* the aggregate, a non-`ironclaw` basename, a separate-workspace guest, a vendored third-party `crates/` subtree, the fail-closed refusal, and a floored nested crate passing and failing its covered-lines floor. The suite gains a shared fixture crate tree, because the aggregator now needs one — the old shape needed no tree at all, which is exactly why every case stayed green while 11 crates went dark. Local: 166/178, with the same 12 pre-existing macOS bash-3.2 `mapfile` failures in the C section that the tree has today (148/160 before this change). Floors for the newly-visible crates are captured separately, from a real coverage run — floors invented without a measurement would bake the hole in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(extensions): restore behavioural coverage for the retired slack_user migration `remove_retired_internal_installation` has had **no behavioural coverage since nearai#6616**, which deleted `restore_removes_retired_slack_user_installation_without_catalog_entry` and replaced it with `assert_eq!(RETIRED_SLACK_USER_EXTENSION_ID, "slack_user")` — a constant compared to its own literal. That gap matters more than most: the branch runs on **every boot** and **destructively** deletes persisted installation rows, and its disposition is an open owner decision (PROPOSAL §12.11 D-I, escalated 2026-08-02, deliberately not ruled by the delegated-authority pass). D-I's recommended sequencing lists restoring this coverage as step (i), "required either way" — so it lands now, whichever way the owner rules, and the behavior is untouched. Extends the existing crate-integration suite rather than adding a file: it already drives `restore_extension_lifecycle_state` over a real `ExtensionInstallationStore` on a real `RootFilesystem`, which is the whole seam this branch lives on. Pins the ACTUAL behavior, including the parts that read as surprising: - Both port reads return `None` — `delete_installation` alone deliberately leaves the manifest projection authoritative, so the branch's second store call is load-bearing and the test says so. - **"Deleted" means tombstoned, not erased.** The v2 record survives with `removed_at` stamped, `removal_cleanup_pending` converged, and the embedded manifest retained; only the two legacy projections are hard-deleted. A test asserting erasure would pin a contract this code does not implement and would hide that the migration is recoverable evidence rather than data loss. - **The control**: a second, equally uncatalogued installation must survive. Deletion keys on the extension id, never on "the catalog could not resolve it". Both halves red-checked against the live tree: disabling the branch fails the removal assertions (and only this test); widening it to delete every catalog-miss row fails the control. Neither the branch nor any other production file is touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(target-architecture): record the Wave 2 closeout and its five corrections Amendments for the WS2 work in this PR, each quoting the text it replaces. **PLAN Wave 2 ✎ note** — its open-item list was stale within a day: strays landed as nearai#7040, and package colocation, the telegram merge and the memory-provider move all landed as nearai#7037. Four carry-forwards: the re-layer is two independent halves and only one was reachable; a *downward* re-layer is costed from the crate's own manifest, mirroring Wave 3's finding that an *upward* one is costed from its consumer set; a stale wave list costs a slot its first hour, so re-measure the list itself; and branch names collide across parallel worktrees — verify `git ls-remote` matches your tip before trusting a run attached to it. **PROPOSAL §6.8.1** — "two more W7 exceptions gone" is wrong by two. Four fall: `mcp` and `scripts` reach the registry for the same manifest DTOs `capabilities` and `host_runtime` do. Baseline 10 -> 6. The entry's own `Deps` line turned out to be the executable instruction for the one blocking edge. **PROPOSAL §12.11 D-A** — the ruling stands; its sizing does not. "The seam is narrow, which is why this is cheap" is true of `channel_host.rs` and false of the crate: twelve production files name `ironclaw_product`, and `ironclaw_host_ingress` is a second blocking edge D-A never named. Recorded as an amendment rather than a silent re-scope so the next slot costs it from evidence. Filed as nearai#7092. **CHECKLIST WS2** — the `include_str!` row ticks with the per-owner breakdown of the 17 surviving cross-crate sites and why `REPORT_ONLY` cannot flip on them (nearai#7093); the re-layer row records the half that landed and the half that did not, with the twelve-file measurement; the escalated §12.11 D-I row records that its own step (i) is done and the escalation is unaffected. **CHECKLIST WS10** — the path-keyed-gates row missed a sixth gate, one step down the same pipeline it audited (nearai#7083). Two corrections to how that row framed the risk: fix a path-keyed gate along its whole pipeline, not at the file the audit opened; and the dark-verdict failure is not only about `git mv` — package colocation broke this one first, because a gate keyed on a name shape fails for any tree change, not just the scheduled one. Crate guides travelling with the change: `ironclaw_trust`'s AGENTS/CLAUDE/ CONTRACT stop claiming `TrustPolicyInput`; `ironclaw_extensions`'s AGENTS records its `substrates` layer and that it must not regain `ironclaw_trust`; `ironclaw_extension_support`'s AGENTS records why `nearai` is a package module that is deliberately not a `PACKAGES` entry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci: floor the crates the coverage fix made visible (nearai#7083) Captured from this PR's own dispatch run 30865483401 at `4c841a4321`, **after** the aggregator fix. Capturing beforehand would have recorded zeros and pinned the hole shut, which is the one outcome nearai#7083 exists to prevent. Four new `[[crate]]` entries — `ironclaw_extension_support` (82.64%, 6826 / 8260), `slack` (93.95%, 3697 / 3935), `telegram` (90.31%, 1435 / 1589), `memory-native` (82.85%, 2850 / 3440). None of the four lacked a floor because anyone judged it unworthy of one; they were invisible to the gate. All four were compiled, instrumented, and present in the merged tracefile the whole time. Keys are crate **directory** basenames, which is what the aggregator keys on and what every pre-existing entry already is — they coincide with package names only for flat `crates/ironclaw_*` crates. `mem0` is deliberately absent and the file says why: it compiles only behind the `memory-mem0` feature, which no coverage lane enables, so it contributes no instrumented lines and a floor would enforce nothing. `[global]` recaptured 85.11% / 375097 → **86.96% / 386885**. The old denominator never described the tree it was enforcing: nearai#7037 landed on 2026-08-03 and four crate directories left both numerator and denominator silently, under `enforce = true`. Both numbers are read off the same `RATCHET PASS: global` line of the same `reborn-coverage-ratchet.sh` invocation that enforces this file, so the mapping is the enforcing mapping by construction — the existing comment's caution is about comparing *across* toolchains, which this does not do. `ironclaw_extension_host` recaptured 84.83% / 19907 / 23467 → **87.99% / 21605 / 24554**. It is the SOURCE side of a move (the NEAR AI embeds left for the package inventory), and a source floor is the one that silently stops describing its crate when code leaves it. Recorded honestly as a ratchet tightening rather than a repair: the denominator moved only −1.46%… +4.63%, below this file's own 5% materiality threshold, and both fields rose — partly because this PR also adds the retired-`slack_user` test the crate had been missing since nearai#6616. Verified locally against the run's own `reborn-integration-merged.lcov`: `reborn-coverage-ratchet.sh` exits 0 with 22 `RATCHET PASS` and zero `FAIL`, and every observed figure matches the CI job line-for-line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ci): refuse colliding crate basenames; tighten the tombstone assertion Review triage on nearai#7094. Two of three findings accepted; the third is declined with reasons, recorded in the PR thread rather than silently skipped. **Accepted — colliding crate basenames must be a refusal (Major).** `crate_key()` reduces a discovered directory to its basename, so two crate directories sharing one would silently fold into a single coverage bucket and a single ratchet floor. That is a *quieter* version of the bug this PR fixes: the merged number looks entirely plausible and nothing reports the merge. `crate_tree.crate_directory()` already refuses an ambiguous basename rather than picking one; this applies the same rule to the aggregation key, raising `CrateTreeError` so it lands on the existing fail-closed path. Unreachable on today's tree — all 65 basenames are distinct — and reachable the moment crates move under family directories, which is the next wave. Pinned by a new self-test case (A6f) with `crates/{domains,substrates}/ironclaw_threads`, asserting the refusal, that both colliding directories are named, and that the message says why a merged number is not offered. **Accepted — `removed_at` presence is not enough.** `Value::get` returns `Some(Value::Null)` for an explicit JSON null, so the tombstone claim would go vacuous if the serializer ever emitted one. Now rejects null explicitly, and the message says why presence alone was insufficient. **Declined — requiring absolute `SF:` paths to be contained under the resolved repo root.** Real in principle; wrong to apply here. `reborn-coverage-merge-lcov.sh` — which produces the tracefile this module reads, and which already filtered every record in it — anchors on the discovered inventory with the identical `(?:^|/)` form and no root containment. Adding containment to the consumer and not the producer re-creates exactly the producer/consumer divergence that made nearai#7083 silent. The documented real-world case (`.../wasmtime-46.0.1/crates/wasmtime/`) is already excluded by inventory anchoring in both, and the scenario the finding describes needs a vendored tree that reproduces a full IronClaw crate directory path *inside an already-filtered lcov*. It would also require rewriting every fixture path in the suite, since they use synthetic absolute prefixes. Self-test 178 -> 181 cases; same 12 pre-existing macOS bash-3.2 C-section failures. Ratchet re-verified against the run's own artifact: exit 0, 22 PASS, 0 FAIL — the captured floors are unaffected (a test-file assertion and a script-level guard change no instrumented line). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ci): correct the coverage-lib header's entry-point signature `aggregate()` gained a `repo_root` parameter with the nearai#7083 fix and the module header still advertised the three-argument form. Names the default resolution order and points at `crate_pattern()` for why the accounting scope is inventory-derived rather than a path shape. Comment only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(ci): pin producer/consumer agreement on a vendored crate-path collision Review catch (nearai#7094): the vendored fixtures guarding the nearai#7083 coverage fix (M5, A6d) pass because their `wasmtime` path matches no inventory entry — so they prove the inventory filter *runs*, not that it is contained. The adversarial input — a path outside the repository that repeats a discovered crate directory verbatim — was never in the suite. A fixture that passes because its input never reaches the code under test is exactly the failure mode this file exists to kill. A6g supplies that input (`.../foreign-1.0.0/crates/extensions/packages/slack/src/lib.rs`) and pins the property that actually protects the accounting: the producer (`reborn-coverage-merge-lcov.sh`, which filters every record the aggregator ever sees) and the consumer (`lib/reborn_coverage_lcov.py`) make the SAME call on it. Both are asked about the same fixture in parallel — chaining the consumer onto the merge's output would only ever compare it against a record the producer had already dropped, so a producer-only change would stay invisible. That mistake was made and caught here before the case landed. Restricting the consumer alone to paths contained under the resolved repo root was reviewed and not taken: the producer applies the identical `(?:^|/)` inventory anchor with no containment, and producer/consumer disagreement is what made nearai#7083 silent instead of loud. Whichever way the rule goes, it goes in both halves at once. This case is what turns a one-sided change red. Sabotage-probed in both directions rather than assumed green: consumer-only "repo-owned" filter -> FAIL A6g: producer and consumer make the same call ... producer-only "repo-owned" filter -> FAIL A6g: producer and consumer make the same call ... and unsabotaged: 174/186, the same 12 pre-existing macOS bash-3.2 C-section failures as before the change (181 -> 186 cases). Not reachable from the real pipeline as it stands, measured rather than asserted: `cargo llvm-cov` emits only workspace-member sources, so every `SF:` record in a lane tracefile already lives under the checkout root — 1067 of 1067 per lane and 1139 of 1139 merged, read off run 30865483401's own artifacts, with zero records dropped by the inventory filter and zero carrying a `/crates/` segment anywhere but the repo-root position. The guard is for the day that stops being true. Test-only. No production behavior changes and no instrumented line moves, so the coverage floors captured for this PR are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Two conflicts, both real. **`WS0_EXTENSION_SPECIFICITY_ALLOWLIST_BASELINE`: recounted, not picked.** #7143 set **125** measuring its own branch; this branch had **124** measuring its own. Neither is right for the union: #7143 also *deleted* an entry (`lifecycle_restore.rs`/`slack`) this branch never saw, so the merged list is lower than either side. 126 on `main` after #7094, minus this branch's two net vendor-config removals, minus #7143's one, is **123** — which is what the ratchet reports with the constant temporarily set to `0` (`ALLOWLIST grew to 123 entries`). Read off the compiler, never by eye: a paren count over the literal answers 142, because the entries' comments contain parens. Both sides' doc amendments are kept and a third records the union recount, flagging #7141/#7152 (#7147) so whichever merges last recounts rather than inheriting 123. **`PROPOSAL.md` was a pure append collision** (empty merge base): #7143 adds the D-I owner ruling, this branch adds §12.12 D-K. Both survive, in document order — the D-I blockquote attaches to the entry directly above it, §12.12 opens the new section before §13. Verified to the #7018 standard: **19 files differ from this branch's pre-merge tip, and all 19 are files `main` touched — zero unexplained.** Across both ledgers, 45 lines #7143 added and 90 lines this branch added are all present. `LAYER_MATRIX_EXCEPTIONS` is 6 on both sides and the file is untouched by us. The widened driver-boundary gate is **byte-identical to the pre-merge tip** — it did not regress to the `take(module_start)` form that let a nested `postgres/pool.rs` leak pass silently. Re-sabotaged after the merge: the nested leak still fails the gate naming `pool.rs:1`.
…earai#7323) reborn-tests.yml's coverage-report job requests job-level `actions: read` since nearai#7018 (it fetches the base commit's merged-lcov artifact for the changed-line coverage gate). GitHub validates called-workflow permissions at trigger time, and the nightly caller grants only `contents: read` + `pull-requests: write` — so every scheduled run since 2026-08-03 (first run at 3be5f05) has died as a startup_failure with zero jobs, and the run's own failure-reporting job dies with it. Add the missing scope and record why in the contract comment next to the call.
…earai#7323) reborn-tests.yml's coverage-report job requests job-level `actions: read` since nearai#7018 (it fetches the base commit's merged-lcov artifact for the changed-line coverage gate). GitHub validates called-workflow permissions at trigger time, and the nightly caller grants only `contents: read` + `pull-requests: write` — so every scheduled run since 2026-08-03 (first run at 3be5f05) has died as a startup_failure with zero jobs, and the run's own failure-reporting job dies with it. Add the missing scope and record why in the contract comment next to the call.
…earai#7323) reborn-tests.yml's coverage-report job requests job-level `actions: read` since nearai#7018 (it fetches the base commit's merged-lcov artifact for the changed-line coverage gate). GitHub validates called-workflow permissions at trigger time, and the nightly caller grants only `contents: read` + `pull-requests: write` — so every scheduled run since 2026-08-03 (first run at 3be5f05) has died as a startup_failure with zero jobs, and the run's own failure-reporting job dies with it. Add the missing scope and record why in the contract comment next to the call.
….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…
…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>
…-tree coverage (WS2 + nearai#7083) (nearai#7094) * refactor(extensions): re-layer the extension registry to substrates (WS2) `ironclaw_extensions` moves `loops` -> `substrates`, which is PROPOSAL §6.8.1's assignment for the crate. Four `LAYER_MATRIX_EXCEPTIONS` fall out with it and the WS0 ratchet baseline drops 10 -> 6. The four were one edge under four names: `host_runtime`, `capabilities`, `mcp` and `scripts` — kernel/runtimes-tier crates — reaching the registry for its manifest DTOs. None is waived and none of those edges is deleted; the registry moved down to a layer every tier above may legally reach. §6.8.1 predicted two of them ("Layer substrates legalizes `capabilities -> extensions` and `host_runtime -> extensions`"); it undercounted, and the amendment records that. The one edge that blocked the move is gone rather than waived. At `substrates` a crate may name only `contracts`/`substrates`, and `ironclaw_extensions` named `ironclaw_trust` (kernel) for exactly one type, `TrustPolicyInput`, used by one method. Every field of that type is already `host_api` vocabulary — `PackageIdentity`, `RequestedTrustClass`, `BTreeSet<CapabilityId>` — and the type names no decision, ceiling, or provenance, so it is requested-trust vocabulary like everything else in `ironclaw_host_api::trust`. It moves there, which is §6.8.1's own prescription ("`trust`-vocabulary via `host_api`"), and every consumer's import is repointed rather than shimmed behind a re-export (.claude/rules/type-placement.md). Behavior-free: a type relocation, a layer declaration, and the exception entries the layer change makes unreachable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(extensions): source package manifests from the inventory, not include_str! (WS2) The WS2 `include_str!` row's named targets — gmail, github, nearai-mcp — plus the two cross-crate manifest reach-ins nearai#7018 added. **nearai-mcp gets its inventory module.** It was the one asset directory of twelve with no module in `ironclaw_extension_support::packages`, so `available_extensions.rs` embedded its manifest and three assets directly. Those embeds move to `packages/nearai.rs` beside every other package's, and `available_extensions.rs` consumes `nearai_bundle()`. It is deliberately **not** a `PACKAGES` entry, and the module says why at length: every other entry is a config-free `fn() -> PackageBundle`, but NEAR AI's shipped `[mcp].server` is a placeholder the host rewrites from the operator's LLM-admin bootstrap config. A config-free builder cannot produce that value, so the *embeds* live with the inventory and the *patch* stays with the endpoint authority. `PackageBundle::manifest_toml` is already a `Cow` precisely so the patched manifest is representable. This makes `ironclaw_extension_support` a normal dependency of `ironclaw_extension_host` instead of a `test-support`-only one. No new dependency cone: the binary already links it, and `extension_host` already built against it under that feature. **github and gmail fixtures are inlined.** Both were test-only reach-ins into a shipped product manifest. Each test needs one property — a v3 manifest asserting first-party trust; a no-channel manifest with an `[admin_configuration]` group — now spelled out in the test that needs it instead of borrowed from 200 lines it does not own. **The two cross-crate sites route through `bundled_packages()`.** slack and telegram carry adapter crates, so `extension_manager`'s manifest reach-ins were classified cross-crate. The tests' stated intent — project the *shipped* field set, not a drifting fixture — is unchanged; only the path changed, from a relative file path to the inventory that owns the bytes. Measured with the §11.2.7 scan: escaping sites 133 -> 128, cross-crate 19 -> 17. `REPORT_ONLY` stays `true` — the 17 survivors belong to three other owners (the support crate's own slack/telegram package crates, host_runtime's seven memory-provider embeds, and four test-only doc reach-ins in `operator`/`product`), none of which this row owns. The doc amendment names each. The `("…/available_extensions.rs", "nearai-mcp")` specificity carve-out is deleted with the embed it covered; the allowlist is shrink-only and staleness-checked, so leaving it would fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci: make Reborn coverage aggregation nested-tree-safe (nearai#7083) Coverage was structurally dark for every crate under `crates/extensions/`. `reborn_coverage_lcov.py` keyed on `crates/(ironclaw_[A-Za-z0-9_]+)/` — a literal path shape requiring `ironclaw_*` *directly* under `crates/` — and the `if match:` it gated guards the global aggregate as well as the per-crate table, so an unmatched record left both numerator and denominator. Five crate directories (~33.7k instrumented lines) contributed nothing to a gate reading `enforce = true`, and nothing said so: there is no `else`, no counter, no warning. This is a regression, not a never-worked condition. All five were flat `crates/ironclaw_*` until nearai#7037 colocated packages three days ago; the module has one commit in its history and predates the move. The `[global]` floor was captured 2026-07-30, before that, so the denominator has silently shrunk under the gate that enforces it. A better regex cannot fix it. Four of the five directory basenames contain no `ironclaw_` at all (`packages/slack`, `telegram`, `mem0`, `memory-native` — PROPOSAL §5.1 names package directories by extension identity), and a greedy nested pattern mis-attributes an in-crate `src/ironclaw_*/` module directory to a crate that does not exist. So the fix is the one the *merge* script one step upstream already applies: anchor on the discovered crate inventory (`crate_tree.py`). The data was always in the merged lcov; only the aggregator was blind, and the two disagreeing is what made the hole silent. Three consequences worth stating: - **The accounting key is the crate directory basename** — what `crate_tree.crate_directory()` resolves by and what `classify-test-scope.sh` keys on. Every existing floor and exemption key is already a basename, so none churns; basenames also survive the family moves still ahead (`crates/ironclaw_llm` -> `crates/substrates/ironclaw_llm`). - **Separate workspace roots are excluded explicitly**, checked *before* the crate pattern. "Outermost wins" would otherwise attribute `packages/slack/wasm-src/` to `packages/slack/`, putting never-compiled guest code in a denominator. Same precedence `reborn_changed_coverage.py` applies. - **It fails closed.** No discoverable crate tree is now a refusal, not a percentage computed over an empty inventory — the WS10 rule this whole class of bug violates. Regression proof: six new cases (A6b/A6c/A6d/A6e, R19/R19b) covering a nested crate in the table *and* the aggregate, a non-`ironclaw` basename, a separate-workspace guest, a vendored third-party `crates/` subtree, the fail-closed refusal, and a floored nested crate passing and failing its covered-lines floor. The suite gains a shared fixture crate tree, because the aggregator now needs one — the old shape needed no tree at all, which is exactly why every case stayed green while 11 crates went dark. Local: 166/178, with the same 12 pre-existing macOS bash-3.2 `mapfile` failures in the C section that the tree has today (148/160 before this change). Floors for the newly-visible crates are captured separately, from a real coverage run — floors invented without a measurement would bake the hole in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(extensions): restore behavioural coverage for the retired slack_user migration `remove_retired_internal_installation` has had **no behavioural coverage since nearai#6616**, which deleted `restore_removes_retired_slack_user_installation_without_catalog_entry` and replaced it with `assert_eq!(RETIRED_SLACK_USER_EXTENSION_ID, "slack_user")` — a constant compared to its own literal. That gap matters more than most: the branch runs on **every boot** and **destructively** deletes persisted installation rows, and its disposition is an open owner decision (PROPOSAL §12.11 D-I, escalated 2026-08-02, deliberately not ruled by the delegated-authority pass). D-I's recommended sequencing lists restoring this coverage as step (i), "required either way" — so it lands now, whichever way the owner rules, and the behavior is untouched. Extends the existing crate-integration suite rather than adding a file: it already drives `restore_extension_lifecycle_state` over a real `ExtensionInstallationStore` on a real `RootFilesystem`, which is the whole seam this branch lives on. Pins the ACTUAL behavior, including the parts that read as surprising: - Both port reads return `None` — `delete_installation` alone deliberately leaves the manifest projection authoritative, so the branch's second store call is load-bearing and the test says so. - **"Deleted" means tombstoned, not erased.** The v2 record survives with `removed_at` stamped, `removal_cleanup_pending` converged, and the embedded manifest retained; only the two legacy projections are hard-deleted. A test asserting erasure would pin a contract this code does not implement and would hide that the migration is recoverable evidence rather than data loss. - **The control**: a second, equally uncatalogued installation must survive. Deletion keys on the extension id, never on "the catalog could not resolve it". Both halves red-checked against the live tree: disabling the branch fails the removal assertions (and only this test); widening it to delete every catalog-miss row fails the control. Neither the branch nor any other production file is touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(target-architecture): record the Wave 2 closeout and its five corrections Amendments for the WS2 work in this PR, each quoting the text it replaces. **PLAN Wave 2 ✎ note** — its open-item list was stale within a day: strays landed as nearai#7040, and package colocation, the telegram merge and the memory-provider move all landed as nearai#7037. Four carry-forwards: the re-layer is two independent halves and only one was reachable; a *downward* re-layer is costed from the crate's own manifest, mirroring Wave 3's finding that an *upward* one is costed from its consumer set; a stale wave list costs a slot its first hour, so re-measure the list itself; and branch names collide across parallel worktrees — verify `git ls-remote` matches your tip before trusting a run attached to it. **PROPOSAL §6.8.1** — "two more W7 exceptions gone" is wrong by two. Four fall: `mcp` and `scripts` reach the registry for the same manifest DTOs `capabilities` and `host_runtime` do. Baseline 10 -> 6. The entry's own `Deps` line turned out to be the executable instruction for the one blocking edge. **PROPOSAL §12.11 D-A** — the ruling stands; its sizing does not. "The seam is narrow, which is why this is cheap" is true of `channel_host.rs` and false of the crate: twelve production files name `ironclaw_product`, and `ironclaw_host_ingress` is a second blocking edge D-A never named. Recorded as an amendment rather than a silent re-scope so the next slot costs it from evidence. Filed as nearai#7092. **CHECKLIST WS2** — the `include_str!` row ticks with the per-owner breakdown of the 17 surviving cross-crate sites and why `REPORT_ONLY` cannot flip on them (nearai#7093); the re-layer row records the half that landed and the half that did not, with the twelve-file measurement; the escalated §12.11 D-I row records that its own step (i) is done and the escalation is unaffected. **CHECKLIST WS10** — the path-keyed-gates row missed a sixth gate, one step down the same pipeline it audited (nearai#7083). Two corrections to how that row framed the risk: fix a path-keyed gate along its whole pipeline, not at the file the audit opened; and the dark-verdict failure is not only about `git mv` — package colocation broke this one first, because a gate keyed on a name shape fails for any tree change, not just the scheduled one. Crate guides travelling with the change: `ironclaw_trust`'s AGENTS/CLAUDE/ CONTRACT stop claiming `TrustPolicyInput`; `ironclaw_extensions`'s AGENTS records its `substrates` layer and that it must not regain `ironclaw_trust`; `ironclaw_extension_support`'s AGENTS records why `nearai` is a package module that is deliberately not a `PACKAGES` entry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci: floor the crates the coverage fix made visible (nearai#7083) Captured from this PR's own dispatch run 30865483401 at `4c841a4321`, **after** the aggregator fix. Capturing beforehand would have recorded zeros and pinned the hole shut, which is the one outcome nearai#7083 exists to prevent. Four new `[[crate]]` entries — `ironclaw_extension_support` (82.64%, 6826 / 8260), `slack` (93.95%, 3697 / 3935), `telegram` (90.31%, 1435 / 1589), `memory-native` (82.85%, 2850 / 3440). None of the four lacked a floor because anyone judged it unworthy of one; they were invisible to the gate. All four were compiled, instrumented, and present in the merged tracefile the whole time. Keys are crate **directory** basenames, which is what the aggregator keys on and what every pre-existing entry already is — they coincide with package names only for flat `crates/ironclaw_*` crates. `mem0` is deliberately absent and the file says why: it compiles only behind the `memory-mem0` feature, which no coverage lane enables, so it contributes no instrumented lines and a floor would enforce nothing. `[global]` recaptured 85.11% / 375097 → **86.96% / 386885**. The old denominator never described the tree it was enforcing: nearai#7037 landed on 2026-08-03 and four crate directories left both numerator and denominator silently, under `enforce = true`. Both numbers are read off the same `RATCHET PASS: global` line of the same `reborn-coverage-ratchet.sh` invocation that enforces this file, so the mapping is the enforcing mapping by construction — the existing comment's caution is about comparing *across* toolchains, which this does not do. `ironclaw_extension_host` recaptured 84.83% / 19907 / 23467 → **87.99% / 21605 / 24554**. It is the SOURCE side of a move (the NEAR AI embeds left for the package inventory), and a source floor is the one that silently stops describing its crate when code leaves it. Recorded honestly as a ratchet tightening rather than a repair: the denominator moved only −1.46%… +4.63%, below this file's own 5% materiality threshold, and both fields rose — partly because this PR also adds the retired-`slack_user` test the crate had been missing since nearai#6616. Verified locally against the run's own `reborn-integration-merged.lcov`: `reborn-coverage-ratchet.sh` exits 0 with 22 `RATCHET PASS` and zero `FAIL`, and every observed figure matches the CI job line-for-line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ci): refuse colliding crate basenames; tighten the tombstone assertion Review triage on nearai#7094. Two of three findings accepted; the third is declined with reasons, recorded in the PR thread rather than silently skipped. **Accepted — colliding crate basenames must be a refusal (Major).** `crate_key()` reduces a discovered directory to its basename, so two crate directories sharing one would silently fold into a single coverage bucket and a single ratchet floor. That is a *quieter* version of the bug this PR fixes: the merged number looks entirely plausible and nothing reports the merge. `crate_tree.crate_directory()` already refuses an ambiguous basename rather than picking one; this applies the same rule to the aggregation key, raising `CrateTreeError` so it lands on the existing fail-closed path. Unreachable on today's tree — all 65 basenames are distinct — and reachable the moment crates move under family directories, which is the next wave. Pinned by a new self-test case (A6f) with `crates/{domains,substrates}/ironclaw_threads`, asserting the refusal, that both colliding directories are named, and that the message says why a merged number is not offered. **Accepted — `removed_at` presence is not enough.** `Value::get` returns `Some(Value::Null)` for an explicit JSON null, so the tombstone claim would go vacuous if the serializer ever emitted one. Now rejects null explicitly, and the message says why presence alone was insufficient. **Declined — requiring absolute `SF:` paths to be contained under the resolved repo root.** Real in principle; wrong to apply here. `reborn-coverage-merge-lcov.sh` — which produces the tracefile this module reads, and which already filtered every record in it — anchors on the discovered inventory with the identical `(?:^|/)` form and no root containment. Adding containment to the consumer and not the producer re-creates exactly the producer/consumer divergence that made nearai#7083 silent. The documented real-world case (`.../wasmtime-46.0.1/crates/wasmtime/`) is already excluded by inventory anchoring in both, and the scenario the finding describes needs a vendored tree that reproduces a full IronClaw crate directory path *inside an already-filtered lcov*. It would also require rewriting every fixture path in the suite, since they use synthetic absolute prefixes. Self-test 178 -> 181 cases; same 12 pre-existing macOS bash-3.2 C-section failures. Ratchet re-verified against the run's own artifact: exit 0, 22 PASS, 0 FAIL — the captured floors are unaffected (a test-file assertion and a script-level guard change no instrumented line). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ci): correct the coverage-lib header's entry-point signature `aggregate()` gained a `repo_root` parameter with the nearai#7083 fix and the module header still advertised the three-argument form. Names the default resolution order and points at `crate_pattern()` for why the accounting scope is inventory-derived rather than a path shape. Comment only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(ci): pin producer/consumer agreement on a vendored crate-path collision Review catch (nearai#7094): the vendored fixtures guarding the nearai#7083 coverage fix (M5, A6d) pass because their `wasmtime` path matches no inventory entry — so they prove the inventory filter *runs*, not that it is contained. The adversarial input — a path outside the repository that repeats a discovered crate directory verbatim — was never in the suite. A fixture that passes because its input never reaches the code under test is exactly the failure mode this file exists to kill. A6g supplies that input (`.../foreign-1.0.0/crates/extensions/packages/slack/src/lib.rs`) and pins the property that actually protects the accounting: the producer (`reborn-coverage-merge-lcov.sh`, which filters every record the aggregator ever sees) and the consumer (`lib/reborn_coverage_lcov.py`) make the SAME call on it. Both are asked about the same fixture in parallel — chaining the consumer onto the merge's output would only ever compare it against a record the producer had already dropped, so a producer-only change would stay invisible. That mistake was made and caught here before the case landed. Restricting the consumer alone to paths contained under the resolved repo root was reviewed and not taken: the producer applies the identical `(?:^|/)` inventory anchor with no containment, and producer/consumer disagreement is what made nearai#7083 silent instead of loud. Whichever way the rule goes, it goes in both halves at once. This case is what turns a one-sided change red. Sabotage-probed in both directions rather than assumed green: consumer-only "repo-owned" filter -> FAIL A6g: producer and consumer make the same call ... producer-only "repo-owned" filter -> FAIL A6g: producer and consumer make the same call ... and unsabotaged: 174/186, the same 12 pre-existing macOS bash-3.2 C-section failures as before the change (181 -> 186 cases). Not reachable from the real pipeline as it stands, measured rather than asserted: `cargo llvm-cov` emits only workspace-member sources, so every `SF:` record in a lane tracefile already lives under the checkout root — 1067 of 1067 per lane and 1139 of 1139 merged, read off run 30865483401's own artifacts, with zero records dropped by the inventory filter and zero carrying a `/crates/` segment anywhere but the repo-root position. The guard is for the day that stops being true. Test-only. No production behavior changes and no instrumented line moves, so the coverage floors captured for this PR are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…earai#7323) reborn-tests.yml's coverage-report job requests job-level `actions: read` since nearai#7018 (it fetches the base commit's merged-lcov artifact for the changed-line coverage gate). GitHub validates called-workflow permissions at trigger time, and the nightly caller grants only `contents: read` + `pull-requests: write` — so every scheduled run since 2026-08-03 (first run at 3be5f05) has died as a startup_failure with zero jobs, and the run's own failure-reporting job dies with it. Add the missing scope and record why in the contract comment next to the call.
Consolidates the four fully-reviewed Wave 2 port-inversion PRs into one branch onto
main, on the owner's instruction, replacing a four-step merge cascade that had already cost a rebase or reconciliation at every step.Important
This supersedes #7000, #7003, #7004 and #7005. Each was reviewed individually by CodeRabbit and by @serrrfirat and every thread on all four is answered. Read this as already-reviewed content, not as a fresh 152-file PR. The four originals are closed with a pointer here; closing is reversible.
What each slice contributed
#7000 —
ProductSurfaceFailurelinchpin (WS2.2) · #7000The WS2.2 test/coverage closure and reconciliation. The production
ProductSurfaceFailure→ProductOperationFailurechange had already reachedmainvia #7002, so what lands here is the follow-up: tests that close the changed-coverage gate with coverage rather than waivers, theHostApiErrorprojection pins, and the deletion of a ~250-line waiver tranche whose rationale had gone stale. Review threads complete.#7003 — split
ironclaw_extension_managerout ofextension_host(WS2.4) · #7003Carves the 9,671-line crate out of
ironclaw_extension_host. Its reviewability depends ongit diff -Mrename detection, which is preserved — see below. Review threads complete; it was approved and squash-merged into #7000's branch upstream, never ontomain.#7004 — invert
ironclaw_operator's product-facing ports (WS5) · #7004Operator port inversion onto
product_contracts, 27 product symbols → 0, the manifest dep deleted, and the non-webui strays. Leaves a genuinely empty residue — a first for this wave. Review threads complete.#7005 — conversations/threads naming trap + attachments widening (WS5) · #7005
The conversations/threads naming trap and the
ironclaw_attachmentswidening — including the rollback-grammar data-loss fix a human reviewer caught: the durable grammar keeps writing the legacy field names, so losing a topic drops a binding rather than merely misrouting it. Review threads complete.Method
Merged with
git mergein dependency order — not rebase, not squash. Rebase flattens merge commits and has already silently reverted a fail-closed scanner in this program; squashing would destroy #7003's rename detection. The individual commits survive, so slice boundaries are still visible ingit log.One repair was needed first. #7003 was squash-merged upstream into
ws2/product-surface-failureand its branch deleted, andws2/product-surface-failurehad been rebased since #7004 forked from it — so #7004's merge base had collapsed toa50ad06384, which made git 3-way merge all of WS2.1/WS2.2/WS2.4 twice (44 conflicted files). Recovering the deleted tip fromrefs/pull/7003/headand merging it first restored the true base3c088e68c5and cut that to 10. That merge is a proven content no-op:tree(refs/pull/7003/head)is byte-identical totree(ws2/product-surface-failure), and the merge resolved to the identical tree already atHEAD.Union verification
For every file each source branch touched, the consolidated blob was compared against that branch's blob:
mainalso editing the fileTwo genuine reconciliations
mainand #7004 independently moved the same types intoproduct_contractsunder different module names. Neither was a textual clash; both needed a decision.LLM config.
main(refactor(contracts): invert webui + openai_compat onto product_contracts (WS5) #7002) moved the DTOs tooperator_llm; refactor(contracts): invert ironclaw_operator's product-facing ports and execute the non-webui strays (WS5) #7004 moved the same DTOs plus theLlmConfigService/ActiveModelReaderports into a newllm_config. The DTO blocks were proved byte-identical, somain's home wins and refactor(contracts): invert ironclaw_operator's product-facing ports and execute the non-webui strays (WS5) #7004's ports,LlmConfigServiceErrorand itsFromprojection were ported intooperator_llm— which is whatmain's own module doc predicted: "the service ports … stay with their product-side implementation until the WS5operatorrow inverts them." The duplicate module is deleted and its 18 import sites repointed. One definition, one import path, asreborn_product_contract_location_scan.rsrequires.Operator control plane.
main'sproduct_wirealready held all twelve operator DTOs; refactor(contracts): invert ironclaw_operator's product-facing ports and execute the non-webui strays (WS5) #7004'soperator_serviceredeclared them and added three service ports. The delta was proved docs-only, so refactor(contracts): invert ironclaw_operator's product-facing ports and execute the non-webui strays (WS5) #7004's doc comments were ported ontomain's copies — they carry real invariants (the precedence roll-up; theUnsupported/Unknownavailability split) — andoperator_servicenow imports the DTOs and declares only the ports plusnormalize_operator_log_context_value.Rename detection
#7003's 16
ironclaw_extension_host → ironclaw_extension_managerrenames plus #7005'sattachment_landing.rs → ironclaw_attachments/src/project_scoped.rs.Two notes for reviewing this:
--stat=250. Plain--stattruncates paths and hides the=>entirely.-M40%. At git's default 50% threshold you get 16, becauseskill_auto_activate_capability.rsnow sits between 40% and 50% similarity — the cross-user state-leak fix and its tests roughly doubled that file, so git scores it as delete+add. The other 15 are 77–99% and detect at any threshold. Nothing moved that was not moved by refactor(extensions): split ironclaw_extension_manager out of extension_host (WS2.4) #7003; the file simply changed a lot after the move.Coverage-exemption manifest
All four slices touched it, so it was merged as a union and then validated through the real loader —
scripts/ci/reborn_changed_coverage.py'sload_manifest, the call that raisesGATE ERRORon a stale path:Line numbers were checked for meaning, not just validity: #7004's one manifest edit (a
3703 → 3701shift onreborn_composition/src/runtime.rs) still names the same source line it named on every branch, and #7005's ten exempted lines still name exactly the declarations their reasons describe.Docs
Unioned line-by-line with a survival audit — every added line from every branch survives and no dated
✎amendment was dropped.main's ✎ corrections on PROPOSAL §6.9.1/§6.9.3/§6.9.4 and #7004's ✎ "Landed 2026-08-01" note on §6.9.2 all stand. The contractsCLAUDE.mdmodule count was reconciled to the union's actual twenty-four — neither side's number was correct after merging.Combined diff and local gauntlet
cargo fmt --all --checkclippy -D warnings --all-targets --all-features× 12 touched cratescargo check --workspace --all-targets --all-featuresConflict-marker sweep (7- and 8-char,
<<<<<<</=======/>>>>>>>/|||||||) is clean in the tree and ingit diff --cached.🤖 Generated with Claude Code
Review findings from #7000, carried forward and fixed here
CodeRabbit's 2026-08-02 review on #7000 posted 19 actionable comments, but 12 of them failed to post as inline threads (
Inline review comments failed to post — GitHub's internal server error or limits) and existed only inside the review body, so they had never been triaged. Those plus the unanswered inline threads were re-read against live code and addressed here. #7003, #7004 and #7005 had every thread answered; nothing was outstanding on them.always_allowminted the persistentDispatchgrant before clearing the contradictingDisabledoverride — a partial failure left live auto-approval authority under a stale disableclearand was confirmed red against the old orderPermissionMode::Denytool throughhandler.dispatch, so a wrongmatches!arm would ship greenwarn!sites in a REPL-reachable dispatch handlerdebug!per CLAUDE.md's REPL/TUI rulePolicyDenied); the honest per-user fix is a read-side change inactivation.rs+ composition, recorded in the code. Test confirmed red beforeif let Ok(true)swallowed a manifest-read error, so a storage fault rendered "nothing to configure"error!linedebug!,internal_fromgets a fixed user-safe string. Test asserts every line carrying the sentinel starts withDEBUGmap_err(|_| …)closures discarded their cause#[cfg(test)] pub mod x;read as ungated — the attribute-run walk stopped atpub, so a test double in a production-named file could satisfy a residue row or an implementor pinstrip_trailing_visibility. There is a real in-tree instance (ironclaw_reborn_cli/src/runtime/mod.rs) that was counted as production; no pin flipped because no ratchet walks that crate yet. Fixtures cover all five visibility forms plus a negativeterminal_safe— which escapes untrusted extension text before it reaches a terminal — was duplicated verbatim, so a hardening fix to one copy would leave the other unescapedterminal_render; its test pins ESC/CR/LF/backspace literallycrates/AGENTS.mdstill credited the host with the lifecycle command WS2.4 movedOne finding was investigated and deliberately not "fixed".
webui_extension_credentialsmapsCrossScopeDeniedtoOk(None)on the status path, which reads like an auth denial failing open. It is not: the selection scope is built from the authenticated caller andCredentialAccountOwnerScope::matchescompares tenant and user for equality, so a foreign owner's account is filtered out before the requester gate runs. What survives is "the caller owns an account for this provider, but it is not granted to this extension" — a missing connection. Returning 403 would strand the user without the connect affordance, fail the whole extensions listing (product collects readiness withtry_collect), and act as an existence oracle. Enforcement lives on the runtime path, which maps the same variant toCredentialStageError::AuthRequired. The collapse is now logged so it is observable rather than silent, and the reasoning is recorded at the site.Deferred, with owners: the four architecture-test source scanners and the
parse_enabled/dispatch_error/resource_usagequadruplication both belong to the WS10 scanner-consolidation CHECKLIST row.