refactor(llm): delete the verified-dead half of the reasoning module (WS8 closeout) - #6964
Merged
Merged
Conversation
…(WS8 closeout) `ironclaw_llm::reasoning` was half-live. `ironclaw_runner`'s model gateway calls `clean_response`, `contains_codex_text_tool_call_syntax`, and `recover_codex_text_tool_calls_from_tool_names` on the live model-response path; everything else in the module was a v1 engine remnant with no production consumer. PR #6943 excluded the module for exactly this reason and left an enumeration to re-verify. Re-verified all 20 enumerated names against this base: the module is private (`mod reasoning;`), so its entire external surface is the two `pub use` blocks in lib.rs, and no crate in the workspace imports any of the 20. All 20 deleted. Three private helpers — `truncate_at_tool_tags`, `closing_tag_for`, `TOOL_TAG_PATTERNS` — were not enumerated but are transitively dead: all 10 of their non-test call sites were inside the deleted `impl Reasoning` block. The three live helpers are untouched, byte-for-byte, and stay where they are. Placement is deferred to the Wave 3 runner shed. Un-masking: ironclaw_llm 1000 -> 884 (-116, all reasoning-module tests belonging to deleted code, each classified); ironclaw_runner 467 -> 467 with an identical roster. No surviving test was edited — the test-module diff has zero added lines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
🚅 Deployed to the ironclaw-pr-6964 environment in ironclaw-ci-preview
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6964
July 31, 2026 15:42
Destroyed
github-actions
Bot
added
size: XL
Records the outcome on that row only: 20/20 enumerated names re-verified dead and deleted with no exclusions, three transitively-dead helpers found beyond the enumeration, the un-masking counts, and the explicit "deferred to the Wave 3 runner shed" answer to the row's placement question. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe legacy reasoning engine and its public exports were removed. Response cleanup and textual tool-call recovery remain. Documentation and coverage baselines were updated to match the deletion. ChangesReasoning cleanup
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6964
July 31, 2026 15:43
Destroyed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/reborn/target-architecture/CHECKLIST.md`:
- Line 107: Reconcile the line-count statement in the checklist entry for
llm::reasoning with the actual diff accounting, replacing the conflicting “4,503
→ 1,299 lines, −3,204” claim with verified figures and a clearly defined scope,
or remove the file delta entirely. Keep the remainder of the landed-change
summary unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6d2e6757-fcbb-4980-b2af-15a107360afa
📒 Files selected for processing (5)
crates/ironclaw_host_api/src/channel.rscrates/ironclaw_llm/CLAUDE.mdcrates/ironclaw_llm/src/lib.rscrates/ironclaw_llm/src/reasoning.rsdocs/reborn/target-architecture/CHECKLIST.md
Contributor
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.77% — 318467 / 371284 lines Per-crate breakdown (60 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (17 entry/entries excluded from the accounting above)
|
Both floors are the documented legitimate-shrinkage case from coverage-floor.toml's own header (a code+test deletion lowering covered lines updates the entry in the same PR). Numbers copied verbatim from this PR's coverage-report run; no tests were added to chase the old floors. ironclaw_llm — caused by this PR. Deleting the verified-dead half of `llm::reasoning` removed 1,871 instrumented lines (28,235 -> 26,364, a material -6.63% denominator move). That dead half carried denser test coverage than the crate average — 116 of the crate's tests exercised it — so removing code and tests together lowered the percentage even though no live path lost coverage. Recaptured to observed: 79.22% / 20,885 covered. ironclaw_events — inherited from main, not caused by this PR. The previous floor was captured before #6943 deleted `events::{parse_jsonl, replay_jsonl}`, so main has been sitting under its own floor (-59 instrumented lines and their covered code); this branch is simply the first the ratchet caught. PR #6958, which touches no events file, fails identically. Recaptured to observed: 80.55% / 1,197 covered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6964
July 31, 2026 16:13
Destroyed
Collaborator
Author
|
Coordinator sign-off (Wave 0 closeout — the Review protocol ran: independent verification, one feedback round, fixes verified.
Ready for Ben to merge — recommend merging this one first to unblock the coverage lane for everything behind it. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/integration/coverage-floor.toml`:
- Around line 197-201: Remove the unrelated ironclaw_events coverage baseline
change from this PR, including the updated floor values, captured metadata, and
rationale in the coverage-floor configuration. If the change is required by
`#6964`, instead document that dependency explicitly in the relevant configuration
context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5518eadc-7377-4edd-946c-0e8192aa8eb1
📒 Files selected for processing (1)
tests/integration/coverage-floor.toml
Comment on lines
+197
to
+201
| floor_percent = 80.55 | ||
| floor_covered_lines = 1197 | ||
| captured_total_lines = 1486 | ||
| captured_date = "2026-07-31" | ||
| rationale = "Durable event envelopes are the typed redacted audit substrate; recalibrated after current-main covered-code shrinkage." | ||
| rationale = "Durable event envelopes are the typed redacted audit substrate; recalibrated because #6943's `events::{parse_jsonl, replay_jsonl}` deletion (-59 instrumented lines and their covered code) landed after the previous capture — inherited from main, not caused by #6964." |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Separate the unrelated ironclaw_events baseline change.
The rationale attributes this change to #6943 and states that it is not caused by #6964. Remove it from this PR, or document the dependency that requires it here.
As per coding guidelines, keep pull requests focused and avoid mixing unrelated concerns.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/coverage-floor.toml` around lines 197 - 201, Remove the
unrelated ironclaw_events coverage baseline change from this PR, including the
updated floor values, captured metadata, and rationale in the coverage-floor
configuration. If the change is required by `#6964`, instead document that
dependency explicitly in the relevant configuration context.
Source: Coding guidelines
BenKurrek
added a commit
that referenced
this pull request
Jul 31, 2026
#6943) The ironclaw_events floor was captured before #6943 deleted `events::{parse_jsonl, replay_jsonl}`, so main has been sitting under its own covered-lines floor ever since: observed 1197 covered / 1486 total against a floor of 1252 covered (effective 1232). Every branch that reaches the coverage job fails on it — PR #6958, which touches no events file, fails with the byte-identical block. Recaptured to the observed numbers per coverage-floor.toml's own same-PR recapture workflow for legitimate deletion-driven shrinkage. The entry is copied byte-for-byte from #6964's commit 7ca468d, which carries the same fix. Identical text on both branches means git merges them cleanly in either order. #6964's ironclaw_llm entry is deliberately NOT brought along — that shrinkage is caused by that PR's own deletion and belongs to it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BenKurrek
added a commit
that referenced
this pull request
Jul 31, 2026
…p, stale events floor recapture (#6966) * fix(ci): use histogram diff in the changed-coverage gate `reborn_changed_coverage.py` built its denominator from `git diff --unified=0` with no `--diff-algorithm`, so it inherited git's default (myers). Myers anchors greedily: on a deletion-shaped diff it shreds one large removal into interleaved -/+ hunks and re-emits surviving, unchanged text as added lines. The gate then demands 100% coverage for code the PR never touched. Discovered on #6964 (deleting the verified-dead half of `llm::reasoning`), where the gate saw 478 changed lines / 208 branch arms and failed at 83.89% / 65.87%. Measured on that same range with the gate's own parser: myers (old): 917 changed production lines histogram (new): 14 changed production lines All 14 are doc comments and imports — zero executable — so the true changed-testable denominator was zero and the 478 was entirely artifact. The regression test asserts the invocation rather than re-staging a myers pathology: the pathology depends on git's internal heuristics, so a fixture built around one can quietly stop reproducing on a future git and leave a vacuous green test. Verified red-then-green — removing the flag fails the new case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(deps): bump wasmtime 47.0.2 -> 47.0.3 (RUSTSEC-2026-0222, RUSTSEC-2026-0223) Two Wasmtime advisories published today fail `cargo deny check advisories` on every branch, which is the "Fast deterministic checks" red cascading into the required Code Style aggregate: RUSTSEC-2026-0222 — stores can mix up type indices between engines RUSTSEC-2026-0223 — preemption/traps during bulk operations can break internal VM state Both name `>=47.0.3` as the fix for the 47.x line. `cargo update -p wasmtime`. Cargo.lock-only. 28 packages move, every one of them on Wasmtime's own lockstep release train — wasmtime* 47.0.2 -> 47.0.3, cranelift* 0.134.2 -> 0.134.3 (its codegen backend), pulley* 47.0.2 -> 47.0.3 (its interpreter). Nothing outside that family changed; no package added or removed. Verified locally with cargo-deny 0.19.9: both advisories reproduce on the old lock and `advisories ok` after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(coverage): recapture the stale ironclaw_events floor (inherited from #6943) The ironclaw_events floor was captured before #6943 deleted `events::{parse_jsonl, replay_jsonl}`, so main has been sitting under its own covered-lines floor ever since: observed 1197 covered / 1486 total against a floor of 1252 covered (effective 1232). Every branch that reaches the coverage job fails on it — PR #6958, which touches no events file, fails with the byte-identical block. Recaptured to the observed numbers per coverage-floor.toml's own same-PR recapture workflow for legitimate deletion-driven shrinkage. The entry is copied byte-for-byte from #6964's commit 7ca468d, which carries the same fix. Identical text on both branches means git merges them cleanly in either order. #6964's ironclaw_llm entry is deliberately NOT brought along — that shrinkage is caused by that PR's own deletion and belongs to it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6964
July 31, 2026 17:27
Destroyed
9 tasks done
BenKurrek
added a commit
that referenced
this pull request
Jul 31, 2026
`reborn_changed_coverage.py` built its denominator from `git diff --unified=0` with no `--diff-algorithm`, so it inherited git's default (myers). Myers anchors greedily: on a deletion-shaped diff it shreds one large removal into interleaved -/+ hunks and re-emits surviving, unchanged text as added lines. The gate then demands 100% coverage for code the PR never touched. Discovered on #6964 (deleting the verified-dead half of `llm::reasoning`), where the gate saw 478 changed lines / 208 branch arms and failed at 83.89% / 65.87%. Measured on that same range with the gate's own parser: myers (old): 917 changed production lines histogram (new): 14 changed production lines All 14 are doc comments and imports — zero executable — so the true changed-testable denominator was zero and the 478 was entirely artifact. The regression test asserts the invocation rather than re-staging a myers pathology: the pathology depends on git's internal heuristics, so a fixture built around one can quietly stop reproducing on a future git and leave a vacuous green test. Verified red-then-green — removing the flag fails the new case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
serrrfirat
pushed a commit
that referenced
this pull request
Aug 3, 2026
…p, stale events floor recapture (#6966) * fix(ci): use histogram diff in the changed-coverage gate `reborn_changed_coverage.py` built its denominator from `git diff --unified=0` with no `--diff-algorithm`, so it inherited git's default (myers). Myers anchors greedily: on a deletion-shaped diff it shreds one large removal into interleaved -/+ hunks and re-emits surviving, unchanged text as added lines. The gate then demands 100% coverage for code the PR never touched. Discovered on #6964 (deleting the verified-dead half of `llm::reasoning`), where the gate saw 478 changed lines / 208 branch arms and failed at 83.89% / 65.87%. Measured on that same range with the gate's own parser: myers (old): 917 changed production lines histogram (new): 14 changed production lines All 14 are doc comments and imports — zero executable — so the true changed-testable denominator was zero and the 478 was entirely artifact. The regression test asserts the invocation rather than re-staging a myers pathology: the pathology depends on git's internal heuristics, so a fixture built around one can quietly stop reproducing on a future git and leave a vacuous green test. Verified red-then-green — removing the flag fails the new case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(deps): bump wasmtime 47.0.2 -> 47.0.3 (RUSTSEC-2026-0222, RUSTSEC-2026-0223) Two Wasmtime advisories published today fail `cargo deny check advisories` on every branch, which is the "Fast deterministic checks" red cascading into the required Code Style aggregate: RUSTSEC-2026-0222 — stores can mix up type indices between engines RUSTSEC-2026-0223 — preemption/traps during bulk operations can break internal VM state Both name `>=47.0.3` as the fix for the 47.x line. `cargo update -p wasmtime`. Cargo.lock-only. 28 packages move, every one of them on Wasmtime's own lockstep release train — wasmtime* 47.0.2 -> 47.0.3, cranelift* 0.134.2 -> 0.134.3 (its codegen backend), pulley* 47.0.2 -> 47.0.3 (its interpreter). Nothing outside that family changed; no package added or removed. Verified locally with cargo-deny 0.19.9: both advisories reproduce on the old lock and `advisories ok` after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(coverage): recapture the stale ironclaw_events floor (inherited from #6943) The ironclaw_events floor was captured before #6943 deleted `events::{parse_jsonl, replay_jsonl}`, so main has been sitting under its own covered-lines floor ever since: observed 1197 covered / 1486 total against a floor of 1252 covered (effective 1232). Every branch that reaches the coverage job fails on it — PR #6958, which touches no events file, fails with the byte-identical block. Recaptured to the observed numbers per coverage-floor.toml's own same-PR recapture workflow for legitimate deletion-driven shrinkage. The entry is copied byte-for-byte from #6964's commit 7ca468d, which carries the same fix. Identical text on both branches means git merges them cleanly in either order. #6964's ironclaw_llm entry is deliberately NOT brought along — that shrinkage is caused by that PR's own deletion and belongs to it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
pull Bot
pushed a commit
to Stars1233/ironclaw
that referenced
this pull request
Aug 3, 2026
* Instrument canary model and tool usage * ci: unblock the queue — histogram diff gate fix, wasmtime RUSTSEC bump, stale events floor recapture (nearai#6966) * fix(ci): use histogram diff in the changed-coverage gate `reborn_changed_coverage.py` built its denominator from `git diff --unified=0` with no `--diff-algorithm`, so it inherited git's default (myers). Myers anchors greedily: on a deletion-shaped diff it shreds one large removal into interleaved -/+ hunks and re-emits surviving, unchanged text as added lines. The gate then demands 100% coverage for code the PR never touched. Discovered on nearai#6964 (deleting the verified-dead half of `llm::reasoning`), where the gate saw 478 changed lines / 208 branch arms and failed at 83.89% / 65.87%. Measured on that same range with the gate's own parser: myers (old): 917 changed production lines histogram (new): 14 changed production lines All 14 are doc comments and imports — zero executable — so the true changed-testable denominator was zero and the 478 was entirely artifact. The regression test asserts the invocation rather than re-staging a myers pathology: the pathology depends on git's internal heuristics, so a fixture built around one can quietly stop reproducing on a future git and leave a vacuous green test. Verified red-then-green — removing the flag fails the new case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(deps): bump wasmtime 47.0.2 -> 47.0.3 (RUSTSEC-2026-0222, RUSTSEC-2026-0223) Two Wasmtime advisories published today fail `cargo deny check advisories` on every branch, which is the "Fast deterministic checks" red cascading into the required Code Style aggregate: RUSTSEC-2026-0222 — stores can mix up type indices between engines RUSTSEC-2026-0223 — preemption/traps during bulk operations can break internal VM state Both name `>=47.0.3` as the fix for the 47.x line. `cargo update -p wasmtime`. Cargo.lock-only. 28 packages move, every one of them on Wasmtime's own lockstep release train — wasmtime* 47.0.2 -> 47.0.3, cranelift* 0.134.2 -> 0.134.3 (its codegen backend), pulley* 47.0.2 -> 47.0.3 (its interpreter). Nothing outside that family changed; no package added or removed. Verified locally with cargo-deny 0.19.9: both advisories reproduce on the old lock and `advisories ok` after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(coverage): recapture the stale ironclaw_events floor (inherited from nearai#6943) The ironclaw_events floor was captured before nearai#6943 deleted `events::{parse_jsonl, replay_jsonl}`, so main has been sitting under its own covered-lines floor ever since: observed 1197 covered / 1486 total against a floor of 1252 covered (effective 1232). Every branch that reaches the coverage job fails on it — PR nearai#6958, which touches no events file, fails with the byte-identical block. Recaptured to the observed numbers per coverage-floor.toml's own same-PR recapture workflow for legitimate deletion-driven shrinkage. The entry is copied byte-for-byte from nearai#6964's commit 7ca468d, which carries the same fix. Identical text on both branches means git merges them cleanly in either order. nearai#6964's ironclaw_llm entry is deliberately NOT brought along — that shrinkage is caused by that PR's own deletion and belongs to it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * Render aggregate metrics in canary PR reports * test(reborn): allow instrumented runtime paths to settle * Map live QA harness in Reborn test planner * Preserve selected Reborn coverage mode * Ignore workspace MSRV in selected Reborn lanes --------- Co-authored-by: Benjamin Kurrek <57506486+BenKurrek@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
pull Bot
pushed a commit
to Stars1233/ironclaw
that referenced
this pull request
Aug 4, 2026
…te the eight doc-truth corrections (nearai#7155) * test(architecture): itemize extension_host's product references as a frozen ledger The products -> loops re-layer has been sized five times from proxies and was wrong five times (D-A's one-file seam, nearai#7092's twelve files, three more since). The trait residue is trait-shaped and cannot see constants, free functions, or inline concrete construction; the manifest biconditional sees the sum but only as a boolean. This adds the itemization: EXTENSION_HOST_PRODUCTION_FILES_STILL_NAMING_PRODUCT — exact-match in both directions, shrink-only under a baseline ceiling, one reason per file, on a whole-token crate matcher (the raw-substring helper would count ironclaw_product_contracts importers). A ledger<->manifest consistency assert keeps the itemization and the biconditional agreeing about whether the edge exists, so the ledger cannot read empty while the manifest still carries the dependency. Sabotage-verified before trusting it: a planted production file naming ironclaw_product reds the gate naming that file; a planted stale row reds the stale direction; comment and string-literal mentions do not register (channel_delivery.rs and skill_learning.rs are the standing comment-only exclusions, and channel_subject_routes.rs's usage is cfg(test)-only). Part of the WS2 re-layer re-scope (nearai#7145). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(target-architecture): amend D-A — re-cite its precedent, record unstarted execution The ruling's two measured legs stand. Its option-(b) refutation cited ChannelWorkflowStateFactory as the in-file precedent; measured, that trait is a sole-impl same-file convenience no architecture test names — the load-bearing precedent is the landed nearai#7004 operator inversion and the ten INVERTED_PORT_IMPLEMENTORS ports, so the amendment re-cites it. Also records that the factory port exists on no ref (decision, not partial execution; shape still open) and that the residue is now mechanically itemized by the reference ledger. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(target-architecture): correct the Wave 2 closeout's twelve-file sizing Six of the twelve were re-export repoints, executed in nearai#7143. The enforced remainder is four reference classes (trait residue, adapter-registry, product free functions, D-A assembly), now itemized mechanically by the reference ledger, with the inventory carried by nearai#7145. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(target-architecture): correct the Wave 3 milestone and the W7 label reading The milestone was wrong three ways: the start was 13 (live register is 6); the ratchet is ceiling-only and pins nothing; and zero is WS12's gate, not this wave's reachable exit — the lane edges are nearai#7067's (whose measurement refutes the WS3 mcp row's vocabulary premise) and conversations->turns is WS5's. Also records that removes_in=W7 is a retired July-train label (nearai#5852 era, introduced 2026-07-09), not Wave 5 or WS7. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(target-architecture): give conversations->turns an owning WS5 row; key the register to it The exception's removal condition lived only inside WS1's verify-row explanation — no row owned it, which is how a milestone silently expires (its removes_in=WS5 date already passed once without it falling, as PROPOSAL 8.3's 2026-08-02 amendment records while asking for exactly this re-milestone). Adds the owning WS5 slice row, re-keys the register entry to it, re-keys host_runtime->extension_support to WS3, documents that W7 is the retired July-train label (not Wave 5 or WS7), and marks the WS5 product-narrows adapter_registry clause as a prerequisite of the extension_host re-layer (nearai#7145). The two entries nearai#7141 deletes and the two it re-keys to nearai#7067 are deliberately left untouched here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(target-architecture): refute the stale 'delete reasoning.rs (dead)' claims Sections 9 (row 29) and 12.4 still said delete-it-outright while WS8's own execution (nearai#6964) deleted only the dead half and the surviving module is live on main (mod reasoning; + re-exports in llm/src/lib.rs). Acting on the rows as written would have deleted production surface. 6.4.13's own line is amended by the in-flight nearai#7128 and deliberately not touched here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(target-architecture): amend 8.3 — row 7's blocker refuted by nearai#7067; live register is 6 Row 7 promised the lane->resources edges dissolve as vocabulary; nearai#7067's measurement (raised on nearai#7065) shows the vocabulary is already in host_api::resource and imported from there — the real holders are ResourceGovernor (3 of 10 methods used) and the ResourceError cone, whose relocation is an authority carve-out. Also refreshes the live register to 6 post-nearai#7094 and notes the conversations->turns re-milestone landed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
elliotBraem
pushed a commit
to NEARBuilders/ironclaw
that referenced
this pull request
Aug 4, 2026
…earai#7117, nearai#7106, nearai#7099, nearai#7101, nearai#7128) (nearai#7139) * refactor(loop-host): move system-prompt content out of the composition root (WS6) CHECKLIST WS6 "Composition behavior evictions" — the `system-prompt content → owning prompt asset` clause. PROPOSAL §6.10.1 lists it among the items still resident in `ironclaw_reborn_composition`; `families/app.md` already says "prompt content of any kind" never belongs to the app family. The four assets move from `ironclaw_reborn_composition/assets/prompts/` to `ironclaw_loop_host/prompts/`, beside the five prompt assets that crate already ships and beside `identity_context.rs`, whose `HostIdentityContextSource` is what puts them in front of a model. `system_prompt_assets.rs` exports them as `pub const`; composition consumes the consts instead of `include_str!`. Resolved owner is the **loop** half of "loop/product owner": the port is loop_host's, and loop_host already owns `prompts/`. What deliberately did *not* travel: the seeding/validation of the on-disk, user-editable `SYSTEM.md`. That is boot-time `std::fs` work on a real host path and `ironclaw_loop_host` has zero `std::fs` uses — moving it would put host-path I/O into a loops crate. Composition keeps assembly + seeding. The runtime storage path `system/prompts/default-system.md` is unchanged; it is where existing installs' user-edited file lives, so renaming it would be a behavior change, not a move. Enforcement (new, in the same diff): `reborn_composition_boundaries.rs::composition_root_embeds_no_prompt_content` fails on either half of the debt — a re-added `include_str!("….md")` in composition source, or a re-added shipped `.md` asset under the crate that is not crate guidance. Sabotage-checked both halves independently. It is keyed on markdown, not on `include_str!`, so `builtin_capability_policy.toml` (config-as-data, composition's charter) is untouched. Un-masking: - `ironclaw_loop_host` 803 → 806 tests; the diff of the unfiltered `--list` rosters is exactly the three new `system_prompt_assets::tests::*`. - `ironclaw_reborn_composition` 928 → 928; roster diff is empty. - No existing test edited. Docs corrections, each quoting the text it replaces: - CHECKLIST WS6 + PROPOSAL §6.10.1: the `local_dev` misnomer's "one residue: the local variable at `runtime.rs:3016`" is wrong twice. The variable is at `runtime.rs:3095`, and `local_runtime` appears 191 times in composition's `src` — including six public API symbols, the public type `RebornLocalRuntimeIdentity`, and an assembly struct field. `reborn_standalone_typename_ratchet` stayed green because it governs *type* names only. Tracked as #7098 as a pure-rename PR, not folded in here. - PROPOSAL §2: `root/default_system_prompt.rs` is re-described as assembly + seeding now that its content assets are gone. - `families/loop.md` + loop_host `AGENTS.md`/`CLAUDE.md` record the new owner and the enforcing test. Refs #7098 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * review(ws6): fail-close the markdown ownership gate; fix two stale doc measurements Addresses both CodeRabbit threads on #7099. Both were right; verified before fixing, and each fix is sabotage-checked. **1. The markdown ownership gate had three false-negative paths.** - `include_str!` / `include_bytes!` were matched per *line*, so a `rustfmt`-wrapped invocation — `include_str!(\n "…/some-prompt.md"\n)`, which is what the formatter produces for a long path — evaded the scan entirely. Replaced with `markdown_include_sites()`, which scans complete invocations across line breaks, plus four unit tests including the multiline regression case. Verified by planting a multiline `include_str!("../../AGENTS.md")` in composition source: the gate now fails and names the flattened site. - `markdown_assets()` skipped unreadable directories and entries with `let Ok(..) else { continue }`, so "the walk could not see it" and "there is nothing there" looked identical to an ownership gate. It now panics on a failed `read_dir`, entry, or `file_type`. - Extensions were compared case-sensitively; `.MD` slipped past. Now `eq_ignore_ascii_case`, on both the extension and the guidance-file exemption. Also added a scanned-file floor (>= 50 sources) so a broken walk fails instead of reporting clean — the same "measured scan" idiom `reborn_registration_pipeline_boundary.rs` uses. **2. PROPOSAL §2.4 still carried the pre-correction `local_runtime` measurement.** Line 81 said `runtime.rs:3016` and "the local *variable* name survived" while §6.10.1 (line 670) already carried the correction — a document contradicting itself. §2.4 now cites `runtime.rs:3095`, states the 191-occurrence scope, and points at §6.10.1 and #7098. The one surviving `:3016` in the file is inside the verbatim quote of the text being replaced, which is deliberate. **Also in this commit — two WS6 rows re-measured, because they would otherwise have been redone.** `RebornRuntime` slimming, at `origin/main` @ `0f897e9366`: - "~40 `_for_test` accessors behind `test-support`" is **already done**: `runtime.rs` has 38 and zero are ungated; crate-wide 149, and all 13 without their own attribute sit in a module gated at its declaration site (`lib.rs:64-65`, `factory.rs:1388-1389`). No `_for_test` function compiles into a production build. - "delete the dead `product_live_adapters` export block" is **refuted**: it is live cross-crate test-support API. `ironclaw_product` declares `ironclaw_reborn_composition = { …, features = ["test-support"] }` as a dev-dependency and its `tests/support/planned_agent_loop.rs` imports seven of the eight names; composition has a suite dedicated to them. Deleting it would strand a sibling crate's test support. Only the third clause (re-export wall vs. snapshot) is still live. `crates/AGENTS.md`'s `ironclaw_loop_host` row now names the prompt assets and says the seeding stays in the composition root. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ci): stop the Reborn test planner failing closed on the crate-family map `crates/AGENTS.md`, `crates/Architecture.md` and `crates/README.md` sit directly under `crates/` and belong to no package directory. The planner skips markdown only at the repository root (`path.endswith(".md") and "/" not in path`), and `IGNORED_PREFIXES` does not include `crates/`, so all three fell through to the fail-closed package-resolution arm: Reborn PR test planner failed: unmapped crate path: crates/AGENTS.md That failed `Detect Reborn test scope`, which failed the `Tests (Reborn)` roll-up — on **any** PR that edited them. Hit while updating `crates/AGENTS.md` in this branch; filed as #7100 with the blast radius. It blocks the exact maintenance the house rule asks for: `crates/AGENTS.md` is the crate-level map WS11 requires updating when crate ownership changes, and `crates/Architecture.md` is already recorded in PROPOSAL §2 as carrying a stale `build_reborn_services` reference that WS11 has to fix. Fix: classify markdown *directly* under `crates/` as crate-family guidance with no test surface, ahead of the package-resolution arm. Deliberately narrow: - markdown *inside* a package directory is untouched and stays package-owned (`test_nested_crate_markdown_remains_package_owned` still passes); - anything non-markdown directly under `crates/` still falls through to the explicit-decision arm, which is the point of that arm. Two regression tests beside the existing nested-markdown one: all three family-map files plan to `mode=none` with no changed packages, and `crates/unexpected.txt` still raises `unmapped crate path`. Sabotage-checked by breaking the new arm's path-depth test — 3 errors, restored to green. Verified end to end: the planner run over this branch's own 14-file diff now succeeds and selects `ironclaw_architecture`, `ironclaw_loop_host`, `ironclaw_reborn_composition`. 44/44 planner tests pass. Fixes #7100 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * revert(ci): back out the planner fix — #7084 already carries it, better I hit `Reborn PR test planner failed: unmapped crate path: crates/AGENTS.md` after adding one line to the crate-family map, diagnosed it as an unhandled fail-closed arm, filed #7100 and fixed it. Then I checked whether other open PRs touch those files — #7084 and #7065 do — and expected them to be red for the same reason. **They are green**, which refuted the "any PR that edits them fails" framing and sent me to look at why. #7065 branched before the planner existed (#6952). **#7084 already modifies `scripts/ci/reborn_pr_test_plan.py` and already fixes this**, in the same function and the same arm I was editing: if package is None: # Markdown that belongs to no crate is prose, in the same class # as `docs/` and `.claude/` … Depth-independent by construction, # so it keeps holding for `crates/AGENTS.md` and for a future # `crates/<family>/AGENTS.md` after the WS7 family move. if path.endswith(".md"): continue with a regression test (`test_markdown_owned_by_no_crate_is_prose`) covering `crates/AGENTS.md`. Their rule is **strictly better than mine**: mine keyed on `path.count("/") == 1`, which would silently stop covering the file the moment WS7 moves crates under family directories. Theirs is depth-independent. So this reverts my planner change and its two tests, and drops the `crates/AGENTS.md` edit that provoked it — #7084 is on the do-not-disturb list and this would have collided with it line-for-line. The guidance follow-up is recorded on the CHECKLIST WS6 row with the exact text owed and the condition (#7084 landing) that unblocks it. #7100 is updated to say it is already fixed rather than left implying open work. Everything else on this branch is unchanged: the system-prompt asset eviction, the markdown ownership gate, and the doc corrections all stand. Refs #7100, #7084 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * review(ws6): statement-bounded include scan; fail-close the Rust-source walk Second CodeRabbit round on #7099. Both findings verified against the code before fixing; both were right. **1. `markdown_include_sites` missed a nested argument macro.** Confirmed: include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/prompt.md")) The first-`)` scan stopped at `(concat!(env!("CARGO_MANIFEST_DIR")` — before the path — and reported clean. Rather than teach the scan balanced-delimiter parsing (which then also owes string-literal, raw-string and comment handling — each an independent silent leak), the span is now bounded by the **statement**: from the macro-name occurrence to the next `;`. Whatever the nesting, spacing or line breaks, the path literal is inside that span. It also requires the name to be a whole identifier followed by optional whitespace and `!`, so `my_include_str!` and a plain `include_str_path` variable are not findings. It over-reports rather than under-reports — a comment mentioning `.md` inside an include statement is flagged — and says so. A false positive is a loud failure a human clears in one line; a false negative is prompt content silently back in the composition root. Seven scanner unit tests now: single-line, multiline, nested argument macro, whitespace before `!`, a comment inside the argument, uppercase `.MD`, non-markdown (`builtin_capability_policy.toml`, which must stay clean), and similar identifiers. Sabotage-checked against the real crate with the exact nested form above: the gate fails and prints the flattened site. **2. The file-count floor did not close the `rust_sources` hole.** Right — it only catches an empty-ish walk; an unreadable directory *after* 50 files still passed silently. `rust_sources` now panics on a failed `read_dir` and a failed entry, matching what it already did for unreadable file contents — this is consistency inside that function, not a new policy, and it hardens the three other tests in the file that share it. The floor is kept and re-justified for the case that stays silent even so: a walk that reads a perfectly good directory which is no longer the crate. After the WS7 family move relocates `crates/…` under family directories, a stale path can resolve to something small and readable rather than erroring. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(arch): restore four tests my previous commit silently deleted `fe641b7709` rewrote `reborn_composition_boundaries.rs` by replacing a *span* between two doc-comment anchors. The two anchors were at opposite ends of the file — `markdown_include_sites` near the top, `markdown_assets` near the bottom — so the replacement swallowed everything between them: - `composition_public_pub_use_surface_matches_snapshot` - `extension_host_cluster_stays_internal` - `reborn_binary_main_is_thin_bootstrap` - `composition_crate_installs_installed_tier_only_through_registrar` - helpers `composition_src_path`, `extract_pub_use_surface`, `has_module_decl`, `is_test_module_file`, `strip_test_module` It compiled and the file's own suite went green, because each deleted test left with the helpers only it used — which is exactly why "the suite passed" is not evidence. It was caught by diffing the function roster against `origin/main` rather than by a test, and by the commit's own −301/+114 line count. This restores the file from `origin/main` and re-applies the change with targeted edits instead of a span replacement. The roster is now **purely additive** against `origin/main` — 9 functions added, **0 removed**, verified with `comm -23`: - `composition_root_embeds_no_prompt_content` (the gate) - `markdown_include_sites`, `markdown_assets` (helpers) - 8 scanner unit tests 7 tests on `origin/main` -> 16 here. Both halves of the gate re-sabotage-checked after the restore: a nested `include_str!(concat!(env!(…), "…default_system.md"))` fails it, and a shipped `assets/prompts/s.MD` fails it. Also fixes what `Fast deterministic checks` caught on `fe641b7709`: clippy's `items after a test module` (the scan's test module now sits at the end of the file, after every helper) and two `doc list item without indentation` warnings (the doc comment is prose, not a list). `cargo clippy -p ironclaw_architecture --benches --tests --examples --all-features` is clean. The substance of `fe641b7709` is unchanged and still stands: statement-bounded include scanning, and `rust_sources` failing closed on unreadable directories and entries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * review(arch): skip Rust trivia when bounding the include statement Third CodeRabbit round on #7099. Both findings verified, both real, both fixed. **1. `.find(';')` could end the span before the path.** A semicolon inside a comment above the argument (`// see the note; below`) or inside the path literal itself (`"../a;b/prompt.md"`) terminated the scan early — and an ownership gate that ends early goes quiet, which is the failure mode this gate exists to prevent. `statement_end_after` now finds the first `;` that actually terminates a statement, skipping line comments, nestable block comments, normal strings with escapes, raw strings with any number of hashes, and char literals (while not mistaking a lifetime for one). It only has to locate a delimiter, not parse the expression, which keeps it ~50 lines. Three new tests, and the third is the one that keeps the fix honest: the span must still *stop*, or a markdown path in the **next** statement would make every non-markdown include a false positive. Sabotage-checked against the real crate with a semicolon-in-comment form — the gate fails. **2. `path.is_dir()` swallowed metadata errors in `rust_sources`.** Right: `Path::is_dir()` returns `false` on an error, so an unreadable directory left the walk silently. It now asks `entry.file_type()` and panics, matching `markdown_assets`. **Not done, with a reason rather than silently:** the suggested regression test for "an unreadable directory beneath an otherwise readable workspace". The only portable way to create one is `chmod 000`, which does not make a directory unreadable for `root` — and the CI containers run as root, so the test would pass locally and be vacuous in CI. A test that cannot fail where it matters is worse than none. The invariant is instead carried by construction: every read in both walks is `unwrap_or_else(panic!)`, with no `let Ok(..) else` and no `is_dir()` left in either. `reborn_composition_boundaries.rs` is 7 tests on `origin/main` -> 19 here, and the function roster is still purely additive (`comm -23` empty). Full `ironclaw_architecture` suite green; clippy `--all-features` clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * review(arch): reject symlinks in both composition ownership walks Fourth CodeRabbit round on #7099, and it is right. `DirEntry::file_type()` reports the **link's** type without following it, so a symlink pointing at a source directory is neither `is_dir()` nor an `.rs` file: both walks stepped over the entire subtree and the gate reported clean on source it never opened. Same "uninspected reads as absent" failure the fail-closed reads added in the previous round exist to prevent — one level further out. `reject_symlink` now panics for either walk, naming the path and the two ways forward. Rejecting is chosen over following deliberately: following needs canonical-root containment plus cycle detection to be safe, and neither scanned crate has ever contained a symlink (`find crates/ironclaw_reborn_composition/src -type l` is empty). The panic is where that decision gets made on purpose rather than silently. Regression test `a_symlinked_subtree_fails_the_walk_instead_of_being_skipped` builds a tempdir with a real source directory plus a symlink to it and asserts **both** `rust_sources` and `markdown_assets` panic. `#[cfg(unix)]`, since the workspace has a Windows lane and `std::os::unix::fs::symlink` is not portable. Sabotage-checked: commenting out both `reject_symlink` call sites turns the test red ("a symlinked subtree must fail the walk, not be skipped"); restoring them returns 20/20. `reborn_composition_boundaries.rs`: 7 tests on `origin/main` -> 20 here, roster still purely additive (`comm -23` empty). Full `ironclaw_architecture` suite green; clippy `--all-features` clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(event-store): stop leaking the Postgres driver in the public API (WS6) CHECKLIST WS6 / PROPOSAL §6.3.2: "stop leaking `deadpool_postgres::Pool` in the public API (wrap)". `ironclaw_reborn_event_store`'s public API now names `deadpool_postgres` zero times; the driver survives only inside its private `postgres_backed` module, which is where the TLS policy and pool construction §6.3.2 assigns this crate actually live. "Wrap" turned out to be three things, not one. **1. Half the leak was dead code, so it is deleted rather than wrapped.** `open_postgres_pool` and `open_postgres_pool_with_max_size` had exactly one caller each — composition's `open_reborn_postgres_pool` and `open_reborn_postgres_pool_with_max_size` — and those two had **zero** callers anywhere in `crates/`, `tests/`, `tools/` or `scripts/`. A four-function pass-through chain across two crates whose only remaining effect was to publish a third-party type in two public APIs. **2. The survivors take a carrier.** `open_postgres_pool_with_tls_options` returns `ironclaw_filesystem::PostgresConnectionPool` and `RebornEventStoreConfig::PostgresPool` holds one. The newtype lives in `ironclaw_filesystem`, not in event_store, for two reasons: it is the only crate `event_store`, `auth` and `composition` can all name without a new dependency edge, and that crate *is* the Postgres substrate, so the driver is chartered there (§11.2.6) rather than leaked. It is a carrier, not an abstraction — `driver()` / `into_driver()` exist for code that runs SQL — and it deliberately has no `Deref` (an implicit unwrap re-admits the driver into a signature unnoticed) and a hand-written `Debug` that renders nothing. The driver's own `Debug` prints its `tokio_postgres::Config`, which redacts the password (`tokio-postgres-0.7.16/src/config.rs:766-776`) but still prints `user`, `dbname`, `host`, `hostaddr`, `port` and `ssl_mode` — deployment topology that a derived `Debug` on any holder would inherit. **3. Stated residue: composition still names the driver, by charter.** §11.2.6 makes it "the one app-layer crate permitted a database driver", and it needs the raw pool for `PostgresRootFilesystem::new` and `CredentialRefreshLeaderLock::for_postgres`. It unwraps the carrier at exactly one site (`factory.rs`, `open_postgres_pool_from_source`). Pushing the carrier further down means changing `PostgresRootFilesystem::new`, which has **13 call sites across 5 crates plus `tests/integration/support/builder.rs`** — a separate test-wide slice, not this row. Recorded in both docs rather than left implied. **Enforcement (new file, lands with the change):** `crates/ironclaw_architecture/tests/reborn_persistence_driver_boundary.rs` - a shrink-only ratchet on which crates may hold a *normal* `deadpool-postgres` dependency (8 today, read from `cargo metadata`, not by eye), and - a scan proving event_store names the driver only below its private `postgres_backed` module — including that the module stays private, since a `pub mod` would silently defeat the scan. Both halves sabotage-checked: a planted `pub fn sabotage(p: deadpool_postgres::Pool)` fails the second and names the line; a planted `deadpool-postgres` dep on `ironclaw_projects` fails the first and names the crate. **Un-masking** (unfiltered `--list`, name-by-name, against `origin/main` in a clean baseline worktree): - `ironclaw_reborn_event_store` 71 → 71, roster identical - `ironclaw_reborn_composition` 928 → 928, roster identical - `ironclaw_filesystem` 296 → 296, roster identical - `ironclaw_architecture` 206 → 208, exactly the two new gate tests Deleting the four dead functions surfaced nothing, which is the evidence they were dead. No existing test edited. Guidance travels: `ironclaw_filesystem/CLAUDE.md` documents the carrier and its two deliberate omissions; `ironclaw_reborn_event_store/AGENTS.md` records that the driver cone is owned but not exported, and names the gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * review(arch): reject a symlink handed in as the walk root too Fifth CodeRabbit round on #7099, and right again — the previous fix closed the hole one level too late. `reject_symlink` only sees entries `read_dir` yields, but both walks push their **root** onto the stack before that ever runs, so a symlinked root was followed to its target silently. The regression test I added covered symlinked children only. `reject_symlink_root` now validates the root with `symlink_metadata` (which does not follow) before either walk starts, reusing the same rejection so the message and the policy stay in one place. The regression test is extended rather than duplicated: it now also symlinks a root and asserts **both** `rust_sources` and `markdown_assets` panic on it. Sabotage-checked — removing the two `reject_symlink_root` calls turns it red ("a symlinked walk root must fail rust_sources, not be followed"). Roster still purely additive against `origin/main` (`comm -23` empty); 20 tests in this file; full `ironclaw_architecture` suite green; clippy `--all-features` clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * review(arch): widen the driver-boundary scan past its two blind spots Three CodeRabbit threads on #7101, all naming the same real defect from different angles, and all correct: `take(module_start)` stopped the scan at the `mod postgres_backed` **header**, so the gate was strictly weaker than the three places documenting it claimed. Two blind spots, both now sabotage-fixtures rather than prose: - anything **after** the module body in `lib.rs` — a `pub fn` there naming `deadpool_postgres::Pool` kept the gate green; - **every sibling file** in the crate (`coalescing_sink.rs`, `durable_log.rs`), which the scan never opened at all. The scan now reads every `.rs` file under `crates/ironclaw_reborn_event_store/ src/` minus the brace-matched **body** of the private module. The brace match is trivia-aware (line comments, nestable block comments, strings, raw strings, char literals) so a `}` inside a literal cannot end the body early and silently drag the rest of the file into the exempt range — the same failure class one level down. It panics on an unterminated body rather than exempting to end-of-file, and asserts it saw at least two source files. Four unit tests on the brace matcher: a mention inside the body is exempt, a mention after the body is not, a brace in a literal does not end the body, and a file without the module has no exempt range. Sabotage-checked against the real crate for both former blind spots: - `pub fn sabotage_after_body(p: deadpool_postgres::Pool)` appended to `lib.rs` -> fails, naming `lib.rs:2215` - the same appended to `coalescing_sink.rs` -> fails, naming `coalescing_sink.rs:321` Also corrected the prose the reviewer flagged as over-claiming, in both places: `ironclaw_reborn_event_store/AGENTS.md` and the CHECKLIST WS6 row now say "module **body**" and state that the scan covers every file in the crate, with the earlier revision's blind spots recorded rather than quietly fixed. Clippy `--all-features` clean (the scan's test module moved to the end of the file for `items after a test module`); full `ironclaw_architecture` suite green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(extractors,observability): typed extraction failures and a one-dependency latency crate (WS6) CHECKLIST WS6 row "extractors: typed error across the boundary + delete caller-less `extract_text` (§6.4.10); observability: `json_value_bytes` eviction (§6.2.5)". Measurements from #7102. ## extractors (§6.4.10) Failures now cross the boundary as `ExtractionError`, not `String`, at both public sites (`DocumentExtraction::Failed` and `extract_document_text_by_filename`). Two variants: `UnsupportedType { mime }` (nothing was attempted) and `NotExtractable { detail }` (an extractor ran and could not produce text). `Display` renders the classification and nothing else; `Debug` carries the payload. That is not a shape change. The invariant — "carries the error reason for logging only; callers render a model-safe marker, never this string" — lived as a doc comment on one of the two boundary sites, and the *other* one leaked: `ironclaw_extension_support`'s `read_file` interpolated the raw extractor diagnostic into a model-facing safe summary (`coding/file.rs:325-329`) while carefully redacting the path one argument earlier. With `Display` content-free that call site is safe unchanged. Its regression test sits at the call site, not on `Display`, because the wrapper composing the summary is what leaked. `extract_text` and `TRUNCATION_MARKER` were both `pub` with zero external callers; both are private now. The row only named the first. The second mattered more: `ironclaw_agent_loop` and `ironclaw_mcp` each declare their own `TRUNCATION_MARKER` with a different value, so it must be resolved by crate, not by name. The census is exact — no crate writes `use ironclaw_extractors::…`, so a full-path grep is complete. The private ZIP-safety enum was renamed `ExtractionError` -> `ZipEntryError` to free the natural name. ## observability (§6.2.5) — delegated ruling, PROPOSAL §12.12 D-K `json_value_bytes` and its `JsonByteCounter` are localized into the two consumers; `serde_json` leaves the manifest with them, so the crate now holds exactly one dependency, `tracing`. The row's stated reason ("gravity-well hygiene") was wrong; the ruling survives on a measured one. Of five call sites in extension_support, three feed `ResourceUsage::set_output_bytes` — resource accounting, not a trace field — so "it is a latency helper, in charter" is false. And sharing bought no invariant: `output_bytes` is already computed three different ways in production (this counter, `output.stdout.len()` in `ironclaw_scripts`, `Value::to_string().len()` in `ironclaw_loop_host`), because each producer measures what it produced. `ironclaw_common` was rejected (the crate the restructure is actively narrowing) and `ironclaw_host_api` was rejected explicitly rather than by omission (behavior in the contracts leaf is the specific criticism already on record against it). Cost, stated: ~18 lines and 2 unit tests duplicated across two crates. ## Guidance and docs New `AGENTS.md` for both crates (both rows asked for one). PROPOSAL §6.4.10 and §6.2.5 amended with dated notes quoting what they replace; §12.12 opened as the Wave 4 delegated-decision log, continuing §12.11's lettering and marking discipline. `families/domains.md` and `families/substrates.md` updated, including a sharpened "never contains" test for observability and a corrected security role for extractors (its failure type is a redaction boundary; "none" was wrong). ## Tests Unfiltered per-crate `--list`, before -> after: extractors 26 -> 28, observability 2 -> 2, attachments 39 -> 39, host_runtime 1247 -> 1249, extension_support 152 -> 156, architecture 206 -> 206. Nothing deleted; nothing edited for content. Observability's two tests moved with the function and are now duplicated in both consumers (2 -> 4 workspace-wide); its two replacements pin what actually remains in the crate. Both new guards were sabotage-verified: break the invariant, confirm red with the right message, restore, confirm green. Coverage floors untouched and deliberately so: the source crate (`ironclaw_observability`) has no floor entry, and the destination `ironclaw_host_runtime` gains covered lines rather than losing them. Found and filed rather than patched: #7103 (the coding tool computes its JSON byte count before checking whether latency tracing is on) and #7104 ("no text found" classifies as `Failed` rather than `Empty`, so the model is told the wrong thing about a valid but text-free document). * fix(extractors): ASCII-only extension normalization + narrow the Debug-payload guidance Review triage for #7106. **CodeRabbit thread 2 — accepted.** `.claude/rules/types.md:170` and `review-discipline.md:45` require case-insensitive external values to be normalized with `to_ascii_lowercase()`, not Unicode case folding. Both extension registries in this crate used `to_lowercase()`; the sibling registry in `ironclaw_extension_support::coding::file` (`should_extract_document_before_text`) already got it right, so this is the outlier. Note it is a latent-hazard fix, not a live bug: the eight keys (pdf/docx/pptx/xlsx/doc/ppt/xls/rtf) contain none of the letters a Unicode fold can produce from a foreign codepoint, so I could not construct an input where the two differ today. It removes the hazard for the next key added. Test pins both halves: ASCII case-insensitivity still works, and a non-ASCII extension is not folded into an ASCII key. **CodeRabbit thread 1 — guidance tightened, code change refuted.** The reviewer is right that this crate's doc told callers to `tracing::debug!(?error, …)` without naming a ceiling, while `ironclaw_host_runtime/AGENTS.md:28` forbids unredacted user content in that crate's logs. Both docs now say the payload belongs in an operator log and nowhere else, and record what it actually carries. The proposed code change is refused with measurement in the PR thread: it would log strictly less than `main` does today. * fix(extractors): the Unicode extension fold was a live bug, not a latent one Correcting my own claim in 0e7d14e and in the #7106 review reply. I wrote that `to_lowercase()` vs `to_ascii_lowercase()` was observationally equivalent here and that I "could not construct an input where the two differ". That was measured against only ONE of the two extension registries. `try_extract_by_extension`'s key set is much larger than `extract_document_text_by_filename`'s eight, and it contains `markdown`: "MAR\u{212A}DOWN".to_lowercase() == "markdown" // U+212A KELVIN SIGN -> k "MAR\u{212A}DOWN".to_ascii_lowercase() == "MAR\u{212A}DOWN" So on `main`, a file named `notes.MAR<U+212A>DOWN` carrying an unrecognized MIME type took the filename fallback in `extract_text`, was UTF-8-decoded, and reached the model as markdown instead of being rejected as an unsupported type. `bash` and `zsh` are in the same key set for the same reason. Caught by CodeRabbit on #7106, which constructed the input I said did not exist. Recorded here rather than quietly repaired: the earlier reply's measurement was wrong and the switch at :707 is a behaviour fix. Regression test extends `extension_matching_is_ascii_case_insensitive_and_ nothing_more` with the `markdown` fold in both registries plus the public `extract_document` path that actually reaches the fallback. Sabotage-verified: reverting :707 to `to_lowercase()` turns it red on the named assertion. * fix(arch): make the driver-boundary visibility check reachable and the scan multi-line safe Review found this gate weaker than its docs for the third time. Both findings were real; both are fixed at the seam and pinned in both directions. 1. The `pub mod` assertion could never fire. The header was matched with `starts_with("mod postgres_backed {")`, so a line beginning `pub ` was not the matched header and the `!starts_with("pub ")` assertion below it was dead. A visible module was simply not found: the exempt range came back empty and the failure blamed whichever driver mention was reported first rather than the visibility change that broke containment. The header now keys on the `mod postgres_backed {` token and asserts on the captured visibility prefix, so `pub` and `pub(crate)` both fail by name. 2. String state did not survive a newline, and that was fail-open. Block comments were carried across lines; regular and raw strings were not, so the continuation lines of a multi-line literal were scanned as code. A `}` there truncated the body, and a `{` there stretched it past the module's real end and swallowed every driver mention after it. With an unbalanced `{` in a multi-line literal and a `deadpool_postgres::Pool` in a public signature after the body, the old scan reported ok; the new one fails on lib.rs:2217. The raw-string terminator is now searched over bytes, so a multi-byte character in a literal cannot leave the index off a char boundary and panic. Regression tests (all failed before the fix, except the last which had no fixture at all): multi-line literal boundary in both directions plus raw strings, `pub mod` and `pub(crate) mod` rejection, the widened header match not mistaking a comment or string for the declaration, and the unterminated-body panic that AGENTS.md and CHECKLIST.md both present as part of the guarantee. Both fixes sabotage-checked against the real event_store source, not only fixtures. The weakness is recorded in the CHECKLIST row and AGENTS.md rather than quietly repaired. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(config): retire the vendor config sections behind a generic window (WS6) `[slack]` and `[telegram]` were the last per-vendor sections in `ironclaw_reborn_config`. Nothing reads them: the enablement gate they fed was deleted with the unified extension runtime (#6116), so `config set slack.enabled true` printed "saved" for a value with no runtime consumer. Replaces the typed vendor schema with a generic retired-section table: - delete `SlackSection`, `SlackChannelRouteSection`, `TelegramSection`, their three builders, and `update_slack_enabled` - `RebornConfigFile` no longer names a vendor; retired sections are split off the raw document before the typed parse, so the schema stays `deny_unknown_fields` - `reject_legacy_slack_config` becomes `reject_retired_config_sections`, data-driven by the same table (PROPOSAL §12.2's "relocated shape") - `config set slack.enabled` now answers with migration guidance instead of writing a value nothing reads Compatibility window preserved and widened: an existing `config.toml` still parses, a retired *setup* key still fails the boot closed with the same message, an inert section still boots — and now says so instead of being silently ignored. Inline-secret rejection over retired sections goes from nine hardcoded keys to every string at any depth. Parse diagnostics: files with no retired section keep the line/column span on unknown-field errors (the split re-parses the original text); only files already carrying a retired section see the degraded form. Measured, and pinned by a test. Sabotage-testing the new guards found one of them inert: the scalar re-insert test only covered `slack = 1` alone, which takes the fast path and would catch it either way. Widened to `slack = 1` beside a genuine retired section, which is the case that actually bypasses `deny_unknown_fields` without the re-insert. The reachability-vs-fidelity limit of the table-driven key test is recorded in its doc rather than papered over. Extension-specificity allowlist 127 -> 125 (baseline lowered to match): the two surviving vendor tokens are the TOML table names, quarantined in `retired_sections.rs`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: correct the Slack/Telegram enablement gate that no longer exists The retired `[slack]`/`[telegram]` sections had a documentation half. Five operator-facing docs still taught a gate deleted by #6116 (2026-07-21): `setup-slack-for-reborn-binary.md` called it the binary's "one gate" and described `IRONCLAW_REBORN_SLACK_ENABLED=false` as a "deployment kill switch" (it is not — Slack stays mounted), and its troubleshooting step could never fix anything. README instructed a `config set slack.enabled` command that now fails. Replaces the gate story with the real one everywhere: the ingress route is compiled in and mounted unconditionally, answers 503 until the extension's signing secret is registered, and 401 on signature mismatch — Slack and Telegram go live by installing the extension and finishing setup at /extensions. Adds a migration note where an operator with an existing file would look. Also removes `IRONCLAW_REBORN_SLACK_PERSONAL_OAUTH_REDIRECT_URI` from `docs/channels/slack.mdx`: zero readers in `crates/`. The CLI already had a regression test asserting that variable must never be advertised in remediation text, so its retirement was known — only the docs kept saying it. Records amendments in the target-architecture docs (CHECKLIST WS6 rows, PROPOSAL §6.10.3 with the placement decision and rejected alternatives, §12.2's compat constraint) and corrects a phantom test citation in the extension-runtime checklist. Filed rather than patched: #7115 (docker entrypoint gates its migration on the dead env var, so following the docs skipped it) and #7116 (live-QA runner gates Slack cases on a value it writes itself). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci(planner): classify `.env.example` so a comment fix is not a full-matrix failure The Reborn PR test planner is fail-closed on unknown paths, and had no rule for `.env.example`. Repo-root `*.md` was classified; its non-`.md` sibling was not, so this PR's env-var comment correction aborted the planner with `unclassified pull-request path: .env.example` and failed the whole `Tests (Reborn)` roll-up on a change with no build surface. Nothing reads the file — no crate, test, or workflow; only doc comments name it by name. Classified rather than exempted, following the `.claude/` precedent added 2026-08-03, whose comment states the rule this follows: classify the path, do not loosen the arm that catches genuinely unknown ones. Regression test asserts all three halves: the path is accepted, it selects no Rust lane (so a future "classification" that turns a comment fix into a full matrix also fails), a real change riding along still selects its lane, and an unknown root file (`.env.local`) still raises. Verified by sabotage — removing the classification turns the new test red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(composition): gate three test-support-only imports so dependency builds lint clean `origin/main` already fails `Code Style` clippy for the package set `{ironclaw, ironclaw_reborn_config}` — verified on a clean detached checkout of `dfdd02b9fb`, exit 101, three unused imports in `composition/src/runtime.rs`. This PR is simply the first to produce that set, so it inherited the failure. Mechanism: the PR clippy lane derives `-p` from the diff and adds `--all-features`, which applies to *selected* packages only. All three imports are named solely by `#[cfg(any(test, feature = "test-support"))]` accessors, so when composition is a mere dependency its `test-support` is off, `--lib --bins` also drops `#[cfg(test)]`, and the imports go unused. With composition in the selected set, `--all-features` turns the gate on and the same command passes. Gating the imports to match their users is the minimal correct fix — they are used, so deleting them would be wrong and `#[allow]` would hide the real property. Verified both directions: the PR-lane invocation and `-p ironclaw_reborn_composition --all-targets --all-features` are now both exit 0. The class of bug — a lint gate whose verdict depends on which packages a PR happened to touch — is #7119; this commit only unblocks. Touching an otherwise-occupied crate deliberately kept to three `#[cfg]` attributes and a comment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: review fixes — google CLI path, Slack setup location, retired-key wording Three CodeRabbit findings, each verified before acting: - `capabilities/configuration.mdx`: `config set google.*` is still a supported path (README and `using/cli.mdx` both document it), so "configure it from the web interface rather than by hand" was wrong. Names both paths now. - `reborn/setup-slack-for-reborn-binary.md`: the 503 troubleshooting step pointed at `/extensions` generically and then called the same thing "Admin Configuration" — a third name for a place `docs/channels/slack.mdx` documents precisely (Extensions -> Channels tab -> Configure on the Slack card), including a warning that Extensions opens on the Registry tab, which is not it. Aligned to that wording, since it is the more specific of the two and matches the UI. - `using/cli.mdx`: "everything else is edited in config.toml directly" no longer holds for retired keys. The fourth finding is refuted in the thread: it asked for a "retired setup keys fail at serve" caveat on the `[telegram]` note, but `RETIRED_SECTIONS` gives telegram `rejected_keys: &[]` — it never had a setup field, so no `[telegram]` section can fail a boot. Adding the caveat would document behaviour that does not exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(slack): tie "Admin Configuration" to the Slack card once, in the guide The setup guide names the operator-facing concept ("Admin Configuration for Slack", 7 references) while docs/channels/slack.mdx names the UI path (Extensions -> Channels tab -> Configure on the Slack card). They are the same dialog, but nothing said so, and my earlier fix only rewrote the troubleshooting paragraph — leaving one place described two ways. Defines the equivalence once, next to the first use, and points the 503/401 steps back at it instead of restating the UI path a second time. Rewriting all seven references would churn a guide this PR is otherwise only correcting for the retired enablement gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(traces): split contribution.rs into chartered modules `crates/ironclaw_reborn_traces/src/contribution.rs` was 17,470 lines — the largest single file in the tree — and carried an `// arch-exempt: large_file` waiver from a 2026 mechanical rename (plan #6168). WS6's domain-internal cleanup row and PROPOSAL §6.4.14 both call for splitting it into chartered modules. It becomes a directory module of 13 production submodules plus a mirrored test tree, each named for one owner in the pipeline (capture → redact → classify → score → queue → submit). `src/contribution/mod.rs` carries the charter table that says which module a new item belongs to, plus the two rules that keep it honest: redaction is split by key (pattern vs tool-name), and `queue` owns state / `remote` owns the wire / `submission` is the only caller of both. The waiver is deleted rather than carried forward, and no new one is added: every file is under the 1,500-line ARCH-SPRAWL threshold (largest is 1,290). No public API change and no consumer edits. The submodules are private and `mod.rs` glob-re-exports them, so `contribution::X` remains the single public path for all four consumer crates. Items that newly cross a module line were widened to `pub(crate)`, never to `pub`. Verification: - Item roster diffed against origin/main: 501 top-level items before, 501 after, zero missing and zero extra. - Unfiltered `--list` before and after: 216 lib tests, leaf names identical. All 216 + 2 integration tests pass. - `cargo clippy --benches --tests --examples --all-features` clean on ironclaw_reborn_traces and ironclaw_architecture. The four `PATH_TERM_COLLISIONS` carve-outs that pinned the old file path are repointed and, in the process, narrowed: the vendor-name safety denylist now resolves to `tool_payloads.rs` (the rule tables) and `classification.rs` (external-write detection, `slack` only) instead of one 17k-line whole-file carve-out, so the specificity gate now polices the rest of the module. Those entries are staleness-checked, so the old path would have failed loudly. Adds the crate's first guidance file, recording the glob-re-export invariant and the three known gaps on §6.4.14's row that this PR does not close (ScopedFilesystem adoption, the two re-export modules, the crate rename). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(reborn): record the traces contribution.rs split and correct two stale clauses Amends CHECKLIST WS6's domain-internal-cleanups row and PROPOSAL §6.4.14 (plus the anti-pattern inventory and the crate-disposition table) with what landed, quoting the text each amendment replaces. Two corrections the work surfaced, recorded rather than silently fixed: - §6.4.14's "17,467-line contribution.rs" measured 17,470 on main; the file drifted after the entry was written. - The CHECKLIST's shorthand "`ScopedFilesystem` + re-export modules dropped" is worded backwards for the first clause. `ScopedFilesystem` is `ironclaw_filesystem`'s type, is used by ~170 files across the workspace, and is absent from `ironclaw_reborn_traces` entirely — there is nothing to drop. §6.4.14's actual instruction is adoption ("take a `ScopedFilesystem` instead of raw `dirs`/env access"), which is a persistence-plane change across ~91 raw fs call sites, not a deletion. Left as-is with the reason stated, so the next reader measures rather than inherits. Also records why the two remaining traces clauses did not land in this wave: dropping the `recording`/`paths` re-export shims needs edits in `ironclaw_reborn_cli`, and `recording` additionally needs a decision because the CLI has no `ironclaw_llm` dependency to fall back on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(traces): serialize test process-env mutation behind lock_env() The split re-surfaced five unguarded `std::env::set_var`/`remove_var` call sites that CI's `check-hermetic-env.sh` had been grandfathering: they are byte-identical pre-existing lines (contribution.rs:10501/10513/10515/15648/ 15661 on origin/main), and the gate only skipped them because it is delta-scoped and the file had not been re-added since it was written. This is a real gap, not a false positive, so it is fixed rather than annotated. `EnvVarRestore` restored the previous value on drop but took no lock, so two tests mutating the environment on different threads still raced — undefined behavior on Rust 1.82+ regardless of whether they name the same variable. `workload_token_env_mode_reads_env_unchanged` used a uniquely named variable, which avoids logical interference but not the setenv/getenv data race. Both now acquire `ironclaw_common::env_helpers::lock_env()`, the sanctioned helper the gate's message names. `EnvVarRestore` holds the guard as a field declared last, so it is released only after `Drop::drop` has restored the value — the restore is inside the critical section, not after it. The real process environment is kept (not `env_helpers::set_runtime_env`'s overlay) because the sidecar isolation test needs a value a child process would inherit, to prove `CommandPrivacyFilterAdapter` clears it. One `#[allow(clippy::await_holding_lock)]` on the async test, matching the precedent in `ironclaw_operator/src/llm_admin/llm_config_service.rs`: holding the lock across the await is the intent, and `#[tokio::test]` drives the future on a current-thread runtime so the guard never crosses threads. Verified: `check-hermetic-env.sh` exits 0, clippy clean, 216 + 2 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(traces): apply CodeRabbit review — carried waiver, inert test, charter drift Six findings verified against the code; four were defects this PR introduced or carried, and each is fixed. 1. **A second file-size waiver was carried forward after all.** `queue.rs` still held the in-body "File-size justification … already-oversized module … decomposition tracked in issue #4088" block, which contradicts a PR whose whole point is performing that decomposition. Deleted; the coupling rationale it was wrapped around (why credential resolution lives beside the policy/scope-dir helpers) is kept, since that still explains the layout. 2. **`invite_code_gated_by_auth_mode` was inert.** It re-implemented the `match policy.auth_mode` expression from `build_trace_upload_claim_issuer_request` and asserted against its own copy, so deleting the `DeviceKey => None` arm in production left it green. It now calls the production builder and asserts on the *serialized* request, so a field rename cannot hide a leak either. Sabotage-proved: removing that arm now fails with the leaked invite code visible in the body. 3. **The charter claimed "each stage owns one file"**, which `remote`'s four files contradict. Reworded to module-level ownership, naming `remote` as a directory module and why. `CLAUDE.md`'s test-layout paragraph gets the same correction plus the explicit `remote` → four-test-module mapping. 4. **Five policy-serde tests sat in `claims.rs`.** They verify `StandingTraceContributionPolicy`, whose owner is `policy.rs`, and the PR's own rule is that a test lives with its production owner. Moved to a new `tests/policy.rs`; leaf names unchanged. 5. **Three orphan section headers** left behind by the split, describing tests that now live in other modules (`credentials.rs`, `profile.rs`, `value.rs`). Deleted. The remaining two findings are real but pre-existing and need behavior changes, so they are filed as #7127 rather than fixed here: the case-sensitive remote `status` comparison that skips the local revocation record, and `fetch_account_traces` taking two adjacent `&str` where its sibling takes `&TenantId, &UserId` (its fix needs an edit in `ironclaw_product`). The issue also carries the `trace_scope_has_pending_queue` doc/code mismatch, which needs an intent decision rather than a guess. Re-verified: 501/501 production items, 216 tests with identical leaf names, clippy clean, hermetic-env clean, every file under 1,500 lines. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(traces): use the RAII env guard and cover the bearer at the caller Second CodeRabbit pass, both findings on the test this PR had already touched. 1. **RAII guard instead of manual cleanup.** `workload_token_env_mode_reads_env_unchanged` set the variable, awaited, asserted, then removed it — so any panic before the last line leaked the variable into every later test. It now uses `EnvVarRestore::set`, whose `Drop` restores during unwinding while holding the same process-env lock. That also deletes both `unsafe` blocks and the `#[allow(clippy::await_holding_lock)]`: the guard lives in a struct field, which the lint does not flag, so the suppression is no longer needed. 2. **The bearer token had no caller-tier coverage.** Five tests assert what `issuer_request_bearer` returns; none asserted the token reaches the wire. The direct issuer path attaches it conditionally (`if let Some(bearer) = issuer_bearer { request.bearer_auth(bearer) }`), so a helper regressing to `None` would send an unauthenticated request with every existing test green — the repo's "test through the caller" rule names exactly this shape. Adds `workload_token_reaches_the_issuer_request_as_a_bearer_header`: a mock issuer captures the `Authorization` header while `fetch_trace_upload_claim_from_issuer` drives the real path. Sabotage-proved — dropping the `bearer_auth` attach fails it with `left: None, right: Some("Bearer wire-bearer-xyz")`; restored, green. Test accounting: 216 → 217. All 216 original leaf names still present (diffed against the `origin/main` baseline); the one addition is the new caller-tier test. Clippy clean, hermetic-env clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(llm): add the enforced sub-owner map (WS6 module charters) PROPOSAL §6.4.13 asks `ironclaw_llm` for "internal module charters for its five sub-owners". This adds the map to `crates/ironclaw_llm/CLAUDE.md` and, because a charter nobody checks rots within a release, a test that pins it. **Five sub-owners were not enough, measured.** `providers` / `auth-sessions` / `registry` / `decorators` / `recording` own 28 of 48 files (79.6% of lines), leaving 20 unowned — including `lib.rs`, `provider.rs`, `error.rs` and `config.rs`. Five more are named, each with a stated reason rather than a residual bucket: `core-contract` (the trait, vocabulary, error taxonomy and config are *upstream* of every implementor, so charging them to `providers` would make providers own decorators' and recording's own dependencies), `normalization` (cross-provider wire hygiene, as opposed to the single-provider shims that stay beside their provider), `model-catalog` (facts about *models*, a different noun from registry's catalog of *providers*), `transcription` (`TranscriptionProvider` is a different trait; nothing there implements `LlmProvider`), and `test-support` (a published feature with its own compatibility obligation). **`tests/module_charter.rs` enforces it.** Every `src/**/*.rs` must appear in exactly one row, every path in a row must exist, and no file may be claimed twice. Sabotage-proved in all three directions — dropping `retry.rs` from the table, adding a phantom path, and double-claiming `registry.rs` each fail with the right message; restored green. The test also guards itself: it fails if the table parses to zero rows or if the source walk finds implausibly few files, so a table-shape change cannot silently turn it into a no-op. **§6.4.13's "Deletes: reasoning.rs (4.5k lines, zero external references)" is refuted.** The file is 1,299 lines after #6964 removed its dead half, and the survivor is live: `lib.rs:88-91` re-exports three helpers with five production call sites in `crates/ironclaw_loop_host/src/model_gateway.rs`. It is charted under `normalization`. `AGENTS.md` carried the same staleness ("legacy reasoning engine") and is corrected; it also now points at the map as authoritative so its informal buckets cannot quietly become a second source of truth. Four placement calls are recorded rather than left implicit: `token_refreshing.rs` is auth-sessions not decorators (CLAUDE.md and AGENTS.md disagreed); `runtime.rs` and `smart_routing.rs` force the decorator definition to widen from "reliability wrapper" to "wraps `dyn LlmProvider` and is not credential work"; `url_check.rs` is core-contract; and `gemini_oauth.rs` is genuinely two owners in one file, charged to the larger half with the split recorded as owed. CHECKLIST and PROPOSAL §6.4.13 carry dated amendments quoting the text they replace, including why the row's `providers.json` clause is blocked (its load-bearing include site is in `ironclaw_reborn_cli`, which is occupied, and it needs a new mechanism rather than a new path). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(traces): correct the claims/policy test-module docs after the move The script that moved the five policy-serde tests copied `claims.rs`'s preamble verbatim, so `policy.rs` ended up with two module docs — its own and a carried-over line describing claims. And `claims.rs`'s own doc still opened with "Standing-policy serde", which stopped being true the moment those tests left. `policy.rs` keeps only its own doc; `claims.rs` now describes what it actually covers (upload-claim cache keys, issuer error labels, the bearer the issuer request carries, device-key auth modes) and points at `policy.rs` for the policy serde contract. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(arch): lower the specificity ALLOWLIST baseline 125 -> 124 after the re-baseline #7117 measured `ALLOWLIST` 127 -> 125 against `origin/main` @ `1e2a294083`. #7094 then deleted one entry on `main` (127 -> 126), so this branch's two net removals now land on 124, not 125. The ratchet is `<=`, so it stayed green at 125 while carrying a unit of untracked slack — exactly what the constant's own doc forbids: "Lower it in the same PR that deletes entries so the new floor is locked in." Read off the ratchet's own failure message with the baseline temporarily set to `0` ("ALLOWLIST grew to 124 entries"), never counted by eye — a plain paren count over the literal answers 142, because the entries' comments contain parentheses too. Sabotage-verified in both directions: baseline 123 goes red naming 124, and 124 is green 7/7. The file's function roster is unchanged. * docs(checklist): map the WS6 "Domain-internal cleanups" row clause by clause The row bundles eight clauses and the Wave 4 part-1 consolidation closes one of them (the `traces` `contribution.rs` split). It stays open, correctly — but a reader of the row could not tell which of the remaining seven had been measured and which had not, and the `llm` `providers.json` measurement lived on the "Module charters" row two rows down because that is where the agent who made it was working. Adds item 7: a clause-by-clause status map — one done, three measured with the blocker named (including a pointer to where `providers.json` was measured), four untouched. No box is ticked; the row's real condition is unmet and stays unmet. Also fixes a stray space-semicolon left in the "Composition behavior evictions" row where the system-prompt clause was struck through. * review(ws6): fix seven findings on code this consolidation introduced CodeRabbit's pass over the consolidation raised 40 threads. 29 are on production code #7124 only *moved* and are filed as #7144. These seven are on code this program wrote, and all seven were correct. **A gate that was not scanning what its doc claimed.** The driver-boundary walk used a flat `read_dir` while its doc said it scans "**every** `.rs` file in the crate". `crates/ironclaw_reborn_event_store/src` is flat today, so nothing escaped — but `src/postgres/pool.rs` is exactly where a driver mention would go, and a skipped file is indistinguishable from a clean one. Now recursive and symlink-rejecting, matching the shape `reborn_composition_boundaries.rs` already uses in this same PR. Sabotage-proved against the real crate: a nested `postgres/pool.rs` naming `deadpool_postgres::Pool` now fails the gate naming `pool.rs:1`, and passed silently before. This is the third revision of this gate found weaker than its own docs; the doc now says why. **A charter gate that a table reformat would have broken.** `module_charter.rs` matched the separator row with `cells[0].starts_with("---")`, so an aligned separator (`|:---|:---|`) parsed as a *data* row: `:---` became an assigned path, `saw_row` went true so the shape guard stayed quiet, and the stale assertion reported `:---` instead of a diagnosis. Sabotage-proved both ways — with the fix reverted and the table rewritten in aligned form the test goes red on `:---`; with the fix it passes. Also: - `CONTRACT.MD` added to the composition guidance allowlist. The repo already ships it as crate-local guidance (`ironclaw_reborn_identity`, `ironclaw_trust`) and CLAUDE.md's module-spec table names it, so a composition `CONTRACT.md` would have been reported as prompt content and sent the author to the wrong fix. - `markdown_assets` gains its first real test: the case-insensitive `.md` match and the caller's guidance filter were both unpinned, and both drift quiet. - Two fixtures for comment-braced module bodies (line comment, nested block comment) — the scan handled them, nothing pinned it. - The symlink rationale doc block moved onto `reject_symlink`, which it describes; it was stacked above `reject_symlink_root` with no item between, so both attached to the wrong function and `reject_symlink` was undocumented. - The retired-section deprecation warn gains `target = "ironclaw::reborn::cli::serve"`, like every other warn on that path. Announcing an inert section is pointless if an operator filtering the documented startup target cannot see it. - `ironclaw_reborn_traces/CLAUDE.md` claimed a one-to-one test mapping that `tests/credentials.rs` breaks (it spans `queue.rs` and `remote/claim.rs`). The exception is now stated rather than left to be inferred. Rosters in both architecture test files are purely additive; no test removed. * docs: correct the extension-specificity allowlist numbers after the re-baseline Caught in review of #7139. Both ledgers still recorded #7117's measurement, `Extension-specificity allowlist **127 → 125**`, taken against `origin/main` @ `1e2a294083`. #7094 then deleted an entry on `main` (127 → 126), so the same two net removals land on **124**, which is what the shipped baseline says. This is the cross-slice-number failure mode the consolidation exists to catch, one layer down: the code was corrected in 811bfedeff and the prose was not. Both amendments quote the text they replace and record the method — read off the ratchet's own failure message with the baseline temporarily set to 0, never counted by eye. No checkbox state changed. * review(ws6): three more review findings, one of which broke my own fix **My `target =` fix did not work, and CodeRabbit was right to call it.** `tracing::warn!(target = "…")` records a *field* named `target`; it does not set the event's metadata target, which stays the module path. So the retired-section notice — given a target in #7117 precisely so operators would see an inert `[slack]`/`[telegram]` section announced — was still invisible to a subscriber filtering `ironclaw::reborn::cli::serve`. Measured with a capturing subscriber rather than argued: EQUALS-SYNTAX target = "target_probe" <- module path COLON-SYNTAX target = "ironclaw::reborn::cli::serve" <- correct Now `target:`, and pinned by `retired_section_notice_is_emitted_on_the_serve_target`, which asserts the emitted **metadata** target through the real `reject_retired_config_sections` call. Sabotage-proved: the `=` form makes it red with `observed targets: ["ironclaw::commands::serve"]`. This is repo-wide — **121 sites** use the `=` form against an `ironclaw::…` target, including the three sibling warns on this same serve path (`:318`, `:387`, `:454`). Filed as #7146 rather than fixed here; a consolidation should not carry a 121-site mechanical change. **The markdown gate's test was testing a copy of itself.** My new test carried its own duplicate of the guidance allowlist, so the production filter could drop `CONTRACT.MD` and the test would still pass — the "test through the caller" rule. Extracted `is_crate_guidance` / `shipped_non_guidance_markdown`; the gate and the test now share one path. Sabotage-proved by dropping `CONTRACT.MD` from the shared helper: red with `left: ["CONTRACT.md", "seed.MD"]`. **The separator fix had no committed regression test.** It was sabotage-proved by hand, which does not survive the session. `parse_sub_owner_table` is split out from the file read so a fixture can supply separator shapes the checked-in `CLAUDE.md` does not use, and `an_aligned_separator_row_is_not_parsed_as_data` covers unaligned, left-aligned and centred. Red when the fix is reverted. Rosters purely additive in all three files; no test remove…
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
…p, stale events floor recapture (nearai#6966) * fix(ci): use histogram diff in the changed-coverage gate `reborn_changed_coverage.py` built its denominator from `git diff --unified=0` with no `--diff-algorithm`, so it inherited git's default (myers). Myers anchors greedily: on a deletion-shaped diff it shreds one large removal into interleaved -/+ hunks and re-emits surviving, unchanged text as added lines. The gate then demands 100% coverage for code the PR never touched. Discovered on nearai#6964 (deleting the verified-dead half of `llm::reasoning`), where the gate saw 478 changed lines / 208 branch arms and failed at 83.89% / 65.87%. Measured on that same range with the gate's own parser: myers (old): 917 changed production lines histogram (new): 14 changed production lines All 14 are doc comments and imports — zero executable — so the true changed-testable denominator was zero and the 478 was entirely artifact. The regression test asserts the invocation rather than re-staging a myers pathology: the pathology depends on git's internal heuristics, so a fixture built around one can quietly stop reproducing on a future git and leave a vacuous green test. Verified red-then-green — removing the flag fails the new case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(deps): bump wasmtime 47.0.2 -> 47.0.3 (RUSTSEC-2026-0222, RUSTSEC-2026-0223) Two Wasmtime advisories published today fail `cargo deny check advisories` on every branch, which is the "Fast deterministic checks" red cascading into the required Code Style aggregate: RUSTSEC-2026-0222 — stores can mix up type indices between engines RUSTSEC-2026-0223 — preemption/traps during bulk operations can break internal VM state Both name `>=47.0.3` as the fix for the 47.x line. `cargo update -p wasmtime`. Cargo.lock-only. 28 packages move, every one of them on Wasmtime's own lockstep release train — wasmtime* 47.0.2 -> 47.0.3, cranelift* 0.134.2 -> 0.134.3 (its codegen backend), pulley* 47.0.2 -> 47.0.3 (its interpreter). Nothing outside that family changed; no package added or removed. Verified locally with cargo-deny 0.19.9: both advisories reproduce on the old lock and `advisories ok` after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(coverage): recapture the stale ironclaw_events floor (inherited from nearai#6943) The ironclaw_events floor was captured before nearai#6943 deleted `events::{parse_jsonl, replay_jsonl}`, so main has been sitting under its own covered-lines floor ever since: observed 1197 covered / 1486 total against a floor of 1252 covered (effective 1232). Every branch that reaches the coverage job fails on it — PR nearai#6958, which touches no events file, fails with the byte-identical block. Recaptured to the observed numbers per coverage-floor.toml's own same-PR recapture workflow for legitimate deletion-driven shrinkage. The entry is copied byte-for-byte from nearai#6964's commit 7ca468d, which carries the same fix. Identical text on both branches means git merges them cleanly in either order. nearai#6964's ironclaw_llm entry is deliberately NOT brought along — that shrinkage is caused by that PR's own deletion and belongs to it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
…(WS8 closeout) (nearai#6964) * refactor(llm): delete the verified-dead half of the reasoning module (WS8 closeout) `ironclaw_llm::reasoning` was half-live. `ironclaw_runner`'s model gateway calls `clean_response`, `contains_codex_text_tool_call_syntax`, and `recover_codex_text_tool_calls_from_tool_names` on the live model-response path; everything else in the module was a v1 engine remnant with no production consumer. PR nearai#6943 excluded the module for exactly this reason and left an enumeration to re-verify. Re-verified all 20 enumerated names against this base: the module is private (`mod reasoning;`), so its entire external surface is the two `pub use` blocks in lib.rs, and no crate in the workspace imports any of the 20. All 20 deleted. Three private helpers — `truncate_at_tool_tags`, `closing_tag_for`, `TOOL_TAG_PATTERNS` — were not enumerated but are transitively dead: all 10 of their non-test call sites were inside the deleted `impl Reasoning` block. The three live helpers are untouched, byte-for-byte, and stay where they are. Placement is deferred to the Wave 3 runner shed. Un-masking: ironclaw_llm 1000 -> 884 (-116, all reasoning-module tests belonging to deleted code, each classified); ironclaw_runner 467 -> 467 with an identical roster. No surviving test was edited — the test-module diff has zero added lines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(checklist): tick the WS8 llm::reasoning row as landed via nearai#6964 Records the outcome on that row only: 20/20 enumerated names re-verified dead and deleted with no exclusions, three transitively-dead helpers found beyond the enumeration, the un-masking counts, and the explicit "deferred to the Wave 3 runner shed" answer to the row's placement question. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(coverage): recapture ironclaw_llm and ironclaw_events ratchet floors Both floors are the documented legitimate-shrinkage case from coverage-floor.toml's own header (a code+test deletion lowering covered lines updates the entry in the same PR). Numbers copied verbatim from this PR's coverage-report run; no tests were added to chase the old floors. ironclaw_llm — caused by this PR. Deleting the verified-dead half of `llm::reasoning` removed 1,871 instrumented lines (28,235 -> 26,364, a material -6.63% denominator move). That dead half carried denser test coverage than the crate average — 116 of the crate's tests exercised it — so removing code and tests together lowered the percentage even though no live path lost coverage. Recaptured to observed: 79.22% / 20,885 covered. ironclaw_events — inherited from main, not caused by this PR. The previous floor was captured before nearai#6943 deleted `events::{parse_jsonl, replay_jsonl}`, so main has been sitting under its own floor (-59 instrumented lines and their covered code); this branch is simply the first the ratchet caught. PR nearai#6958, which touches no events file, fails identically. Recaptured to observed: 80.55% / 1,197 covered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
* Instrument canary model and tool usage * ci: unblock the queue — histogram diff gate fix, wasmtime RUSTSEC bump, stale events floor recapture (nearai#6966) * fix(ci): use histogram diff in the changed-coverage gate `reborn_changed_coverage.py` built its denominator from `git diff --unified=0` with no `--diff-algorithm`, so it inherited git's default (myers). Myers anchors greedily: on a deletion-shaped diff it shreds one large removal into interleaved -/+ hunks and re-emits surviving, unchanged text as added lines. The gate then demands 100% coverage for code the PR never touched. Discovered on nearai#6964 (deleting the verified-dead half of `llm::reasoning`), where the gate saw 478 changed lines / 208 branch arms and failed at 83.89% / 65.87%. Measured on that same range with the gate's own parser: myers (old): 917 changed production lines histogram (new): 14 changed production lines All 14 are doc comments and imports — zero executable — so the true changed-testable denominator was zero and the 478 was entirely artifact. The regression test asserts the invocation rather than re-staging a myers pathology: the pathology depends on git's internal heuristics, so a fixture built around one can quietly stop reproducing on a future git and leave a vacuous green test. Verified red-then-green — removing the flag fails the new case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(deps): bump wasmtime 47.0.2 -> 47.0.3 (RUSTSEC-2026-0222, RUSTSEC-2026-0223) Two Wasmtime advisories published today fail `cargo deny check advisories` on every branch, which is the "Fast deterministic checks" red cascading into the required Code Style aggregate: RUSTSEC-2026-0222 — stores can mix up type indices between engines RUSTSEC-2026-0223 — preemption/traps during bulk operations can break internal VM state Both name `>=47.0.3` as the fix for the 47.x line. `cargo update -p wasmtime`. Cargo.lock-only. 28 packages move, every one of them on Wasmtime's own lockstep release train — wasmtime* 47.0.2 -> 47.0.3, cranelift* 0.134.2 -> 0.134.3 (its codegen backend), pulley* 47.0.2 -> 47.0.3 (its interpreter). Nothing outside that family changed; no package added or removed. Verified locally with cargo-deny 0.19.9: both advisories reproduce on the old lock and `advisories ok` after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(coverage): recapture the stale ironclaw_events floor (inherited from nearai#6943) The ironclaw_events floor was captured before nearai#6943 deleted `events::{parse_jsonl, replay_jsonl}`, so main has been sitting under its own covered-lines floor ever since: observed 1197 covered / 1486 total against a floor of 1252 covered (effective 1232). Every branch that reaches the coverage job fails on it — PR nearai#6958, which touches no events file, fails with the byte-identical block. Recaptured to the observed numbers per coverage-floor.toml's own same-PR recapture workflow for legitimate deletion-driven shrinkage. The entry is copied byte-for-byte from nearai#6964's commit 7ca468d, which carries the same fix. Identical text on both branches means git merges them cleanly in either order. nearai#6964's ironclaw_llm entry is deliberately NOT brought along — that shrinkage is caused by that PR's own deletion and belongs to it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * Render aggregate metrics in canary PR reports * test(reborn): allow instrumented runtime paths to settle * Map live QA harness in Reborn test planner * Preserve selected Reborn coverage mode * Ignore workspace MSRV in selected Reborn lanes --------- Co-authored-by: Benjamin Kurrek <57506486+BenKurrek@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
…te the eight doc-truth corrections (nearai#7155) * test(architecture): itemize extension_host's product references as a frozen ledger The products -> loops re-layer has been sized five times from proxies and was wrong five times (D-A's one-file seam, nearai#7092's twelve files, three more since). The trait residue is trait-shaped and cannot see constants, free functions, or inline concrete construction; the manifest biconditional sees the sum but only as a boolean. This adds the itemization: EXTENSION_HOST_PRODUCTION_FILES_STILL_NAMING_PRODUCT — exact-match in both directions, shrink-only under a baseline ceiling, one reason per file, on a whole-token crate matcher (the raw-substring helper would count ironclaw_product_contracts importers). A ledger<->manifest consistency assert keeps the itemization and the biconditional agreeing about whether the edge exists, so the ledger cannot read empty while the manifest still carries the dependency. Sabotage-verified before trusting it: a planted production file naming ironclaw_product reds the gate naming that file; a planted stale row reds the stale direction; comment and string-literal mentions do not register (channel_delivery.rs and skill_learning.rs are the standing comment-only exclusions, and channel_subject_routes.rs's usage is cfg(test)-only). Part of the WS2 re-layer re-scope (nearai#7145). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(target-architecture): amend D-A — re-cite its precedent, record unstarted execution The ruling's two measured legs stand. Its option-(b) refutation cited ChannelWorkflowStateFactory as the in-file precedent; measured, that trait is a sole-impl same-file convenience no architecture test names — the load-bearing precedent is the landed nearai#7004 operator inversion and the ten INVERTED_PORT_IMPLEMENTORS ports, so the amendment re-cites it. Also records that the factory port exists on no ref (decision, not partial execution; shape still open) and that the residue is now mechanically itemized by the reference ledger. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(target-architecture): correct the Wave 2 closeout's twelve-file sizing Six of the twelve were re-export repoints, executed in nearai#7143. The enforced remainder is four reference classes (trait residue, adapter-registry, product free functions, D-A assembly), now itemized mechanically by the reference ledger, with the inventory carried by nearai#7145. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(target-architecture): correct the Wave 3 milestone and the W7 label reading The milestone was wrong three ways: the start was 13 (live register is 6); the ratchet is ceiling-only and pins nothing; and zero is WS12's gate, not this wave's reachable exit — the lane edges are nearai#7067's (whose measurement refutes the WS3 mcp row's vocabulary premise) and conversations->turns is WS5's. Also records that removes_in=W7 is a retired July-train label (nearai#5852 era, introduced 2026-07-09), not Wave 5 or WS7. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(target-architecture): give conversations->turns an owning WS5 row; key the register to it The exception's removal condition lived only inside WS1's verify-row explanation — no row owned it, which is how a milestone silently expires (its removes_in=WS5 date already passed once without it falling, as PROPOSAL 8.3's 2026-08-02 amendment records while asking for exactly this re-milestone). Adds the owning WS5 slice row, re-keys the register entry to it, re-keys host_runtime->extension_support to WS3, documents that W7 is the retired July-train label (not Wave 5 or WS7), and marks the WS5 product-narrows adapter_registry clause as a prerequisite of the extension_host re-layer (nearai#7145). The two entries nearai#7141 deletes and the two it re-keys to nearai#7067 are deliberately left untouched here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(target-architecture): refute the stale 'delete reasoning.rs (dead)' claims Sections 9 (row 29) and 12.4 still said delete-it-outright while WS8's own execution (nearai#6964) deleted only the dead half and the surviving module is live on main (mod reasoning; + re-exports in llm/src/lib.rs). Acting on the rows as written would have deleted production surface. 6.4.13's own line is amended by the in-flight nearai#7128 and deliberately not touched here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(target-architecture): amend 8.3 — row 7's blocker refuted by nearai#7067; live register is 6 Row 7 promised the lane->resources edges dissolve as vocabulary; nearai#7067's measurement (raised on nearai#7065) shows the vocabulary is already in host_api::resource and imported from there — the real holders are ResourceGovernor (3 of 10 methods used) and the ResourceError cone, whose relocation is an authority carve-out. Also refreshes the live register to 6 post-nearai#7094 and notes the conversations->turns re-milestone landed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
…earai#7117, nearai#7106, nearai#7099, nearai#7101, nearai#7128) (nearai#7139) * refactor(loop-host): move system-prompt content out of the composition root (WS6) CHECKLIST WS6 "Composition behavior evictions" — the `system-prompt content → owning prompt asset` clause. PROPOSAL §6.10.1 lists it among the items still resident in `ironclaw_reborn_composition`; `families/app.md` already says "prompt content of any kind" never belongs to the app family. The four assets move from `ironclaw_reborn_composition/assets/prompts/` to `ironclaw_loop_host/prompts/`, beside the five prompt assets that crate already ships and beside `identity_context.rs`, whose `HostIdentityContextSource` is what puts them in front of a model. `system_prompt_assets.rs` exports them as `pub const`; composition consumes the consts instead of `include_str!`. Resolved owner is the **loop** half of "loop/product owner": the port is loop_host's, and loop_host already owns `prompts/`. What deliberately did *not* travel: the seeding/validation of the on-disk, user-editable `SYSTEM.md`. That is boot-time `std::fs` work on a real host path and `ironclaw_loop_host` has zero `std::fs` uses — moving it would put host-path I/O into a loops crate. Composition keeps assembly + seeding. The runtime storage path `system/prompts/default-system.md` is unchanged; it is where existing installs' user-edited file lives, so renaming it would be a behavior change, not a move. Enforcement (new, in the same diff): `reborn_composition_boundaries.rs::composition_root_embeds_no_prompt_content` fails on either half of the debt — a re-added `include_str!("….md")` in composition source, or a re-added shipped `.md` asset under the crate that is not crate guidance. Sabotage-checked both halves independently. It is keyed on markdown, not on `include_str!`, so `builtin_capability_policy.toml` (config-as-data, composition's charter) is untouched. Un-masking: - `ironclaw_loop_host` 803 → 806 tests; the diff of the unfiltered `--list` rosters is exactly the three new `system_prompt_assets::tests::*`. - `ironclaw_reborn_composition` 928 → 928; roster diff is empty. - No existing test edited. Docs corrections, each quoting the text it replaces: - CHECKLIST WS6 + PROPOSAL §6.10.1: the `local_dev` misnomer's "one residue: the local variable at `runtime.rs:3016`" is wrong twice. The variable is at `runtime.rs:3095`, and `local_runtime` appears 191 times in composition's `src` — including six public API symbols, the public type `RebornLocalRuntimeIdentity`, and an assembly struct field. `reborn_standalone_typename_ratchet` stayed green because it governs *type* names only. Tracked as #7098 as a pure-rename PR, not folded in here. - PROPOSAL §2: `root/default_system_prompt.rs` is re-described as assembly + seeding now that its content assets are gone. - `families/loop.md` + loop_host `AGENTS.md`/`CLAUDE.md` record the new owner and the enforcing test. Refs #7098 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * review(ws6): fail-close the markdown ownership gate; fix two stale doc measurements Addresses both CodeRabbit threads on #7099. Both were right; verified before fixing, and each fix is sabotage-checked. **1. The markdown ownership gate had three false-negative paths.** - `include_str!` / `include_bytes!` were matched per *line*, so a `rustfmt`-wrapped invocation — `include_str!(\n "…/some-prompt.md"\n)`, which is what the formatter produces for a long path — evaded the scan entirely. Replaced with `markdown_include_sites()`, which scans complete invocations across line breaks, plus four unit tests including the multiline regression case. Verified by planting a multiline `include_str!("../../AGENTS.md")` in composition source: the gate now fails and names the flattened site. - `markdown_assets()` skipped unreadable directories and entries with `let Ok(..) else { continue }`, so "the walk could not see it" and "there is nothing there" looked identical to an ownership gate. It now panics on a failed `read_dir`, entry, or `file_type`. - Extensions were compared case-sensitively; `.MD` slipped past. Now `eq_ignore_ascii_case`, on both the extension and the guidance-file exemption. Also added a scanned-file floor (>= 50 sources) so a broken walk fails instead of reporting clean — the same "measured scan" idiom `reborn_registration_pipeline_boundary.rs` uses. **2. PROPOSAL §2.4 still carried the pre-correction `local_runtime` measurement.** Line 81 said `runtime.rs:3016` and "the local *variable* name survived" while §6.10.1 (line 670) already carried the correction — a document contradicting itself. §2.4 now cites `runtime.rs:3095`, states the 191-occurrence scope, and points at §6.10.1 and #7098. The one surviving `:3016` in the file is inside the verbatim quote of the text being replaced, which is deliberate. **Also in this commit — two WS6 rows re-measured, because they would otherwise have been redone.** `RebornRuntime` slimming, at `origin/main` @ `0f897e9366`: - "~40 `_for_test` accessors behind `test-support`" is **already done**: `runtime.rs` has 38 and zero are ungated; crate-wide 149, and all 13 without their own attribute sit in a module gated at its declaration site (`lib.rs:64-65`, `factory.rs:1388-1389`). No `_for_test` function compiles into a production build. - "delete the dead `product_live_adapters` export block" is **refuted**: it is live cross-crate test-support API. `ironclaw_product` declares `ironclaw_reborn_composition = { …, features = ["test-support"] }` as a dev-dependency and its `tests/support/planned_agent_loop.rs` imports seven of the eight names; composition has a suite dedicated to them. Deleting it would strand a sibling crate's test support. Only the third clause (re-export wall vs. snapshot) is still live. `crates/AGENTS.md`'s `ironclaw_loop_host` row now names the prompt assets and says the seeding stays in the composition root. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ci): stop the Reborn test planner failing closed on the crate-family map `crates/AGENTS.md`, `crates/Architecture.md` and `crates/README.md` sit directly under `crates/` and belong to no package directory. The planner skips markdown only at the repository root (`path.endswith(".md") and "/" not in path`), and `IGNORED_PREFIXES` does not include `crates/`, so all three fell through to the fail-closed package-resolution arm: Reborn PR test planner failed: unmapped crate path: crates/AGENTS.md That failed `Detect Reborn test scope`, which failed the `Tests (Reborn)` roll-up — on **any** PR that edited them. Hit while updating `crates/AGENTS.md` in this branch; filed as #7100 with the blast radius. It blocks the exact maintenance the house rule asks for: `crates/AGENTS.md` is the crate-level map WS11 requires updating when crate ownership changes, and `crates/Architecture.md` is already recorded in PROPOSAL §2 as carrying a stale `build_reborn_services` reference that WS11 has to fix. Fix: classify markdown *directly* under `crates/` as crate-family guidance with no test surface, ahead of the package-resolution arm. Deliberately narrow: - markdown *inside* a package directory is untouched and stays package-owned (`test_nested_crate_markdown_remains_package_owned` still passes); - anything non-markdown directly under `crates/` still falls through to the explicit-decision arm, which is the point of that arm. Two regression tests beside the existing nested-markdown one: all three family-map files plan to `mode=none` with no changed packages, and `crates/unexpected.txt` still raises `unmapped crate path`. Sabotage-checked by breaking the new arm's path-depth test — 3 errors, restored to green. Verified end to end: the planner run over this branch's own 14-file diff now succeeds and selects `ironclaw_architecture`, `ironclaw_loop_host`, `ironclaw_reborn_composition`. 44/44 planner tests pass. Fixes #7100 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * revert(ci): back out the planner fix — #7084 already carries it, better I hit `Reborn PR test planner failed: unmapped crate path: crates/AGENTS.md` after adding one line to the crate-family map, diagnosed it as an unhandled fail-closed arm, filed #7100 and fixed it. Then I checked whether other open PRs touch those files — #7084 and #7065 do — and expected them to be red for the same reason. **They are green**, which refuted the "any PR that edits them fails" framing and sent me to look at why. #7065 branched before the planner existed (#6952). **#7084 already modifies `scripts/ci/reborn_pr_test_plan.py` and already fixes this**, in the same function and the same arm I was editing: if package is None: # Markdown that belongs to no crate is prose, in the same class # as `docs/` and `.claude/` … Depth-independent by construction, # so it keeps holding for `crates/AGENTS.md` and for a future # `crates/<family>/AGENTS.md` after the WS7 family move. if path.endswith(".md"): continue with a regression test (`test_markdown_owned_by_no_crate_is_prose`) covering `crates/AGENTS.md`. Their rule is **strictly better than mine**: mine keyed on `path.count("/") == 1`, which would silently stop covering the file the moment WS7 moves crates under family directories. Theirs is depth-independent. So this reverts my planner change and its two tests, and drops the `crates/AGENTS.md` edit that provoked it — #7084 is on the do-not-disturb list and this would have collided with it line-for-line. The guidance follow-up is recorded on the CHECKLIST WS6 row with the exact text owed and the condition (#7084 landing) that unblocks it. #7100 is updated to say it is already fixed rather than left implying open work. Everything else on this branch is unchanged: the system-prompt asset eviction, the markdown ownership gate, and the doc corrections all stand. Refs #7100, #7084 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * review(ws6): statement-bounded include scan; fail-close the Rust-source walk Second CodeRabbit round on #7099. Both findings verified against the code before fixing; both were right. **1. `markdown_include_sites` missed a nested argument macro.** Confirmed: include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/prompt.md")) The first-`)` scan stopped at `(concat!(env!("CARGO_MANIFEST_DIR")` — before the path — and reported clean. Rather than teach the scan balanced-delimiter parsing (which then also owes string-literal, raw-string and comment handling — each an independent silent leak), the span is now bounded by the **statement**: from the macro-name occurrence to the next `;`. Whatever the nesting, spacing or line breaks, the path literal is inside that span. It also requires the name to be a whole identifier followed by optional whitespace and `!`, so `my_include_str!` and a plain `include_str_path` variable are not findings. It over-reports rather than under-reports — a comment mentioning `.md` inside an include statement is flagged — and says so. A false positive is a loud failure a human clears in one line; a false negative is prompt content silently back in the composition root. Seven scanner unit tests now: single-line, multiline, nested argument macro, whitespace before `!`, a comment inside the argument, uppercase `.MD`, non-markdown (`builtin_capability_policy.toml`, which must stay clean), and similar identifiers. Sabotage-checked against the real crate with the exact nested form above: the gate fails and prints the flattened site. **2. The file-count floor did not close the `rust_sources` hole.** Right — it only catches an empty-ish walk; an unreadable directory *after* 50 files still passed silently. `rust_sources` now panics on a failed `read_dir` and a failed entry, matching what it already did for unreadable file contents — this is consistency inside that function, not a new policy, and it hardens the three other tests in the file that share it. The floor is kept and re-justified for the case that stays silent even so: a walk that reads a perfectly good directory which is no longer the crate. After the WS7 family move relocates `crates/…` under family directories, a stale path can resolve to something small and readable rather than erroring. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(arch): restore four tests my previous commit silently deleted `fe641b7709` rewrote `reborn_composition_boundaries.rs` by replacing a *span* between two doc-comment anchors. The two anchors were at opposite ends of the file — `markdown_include_sites` near the top, `markdown_assets` near the bottom — so the replacement swallowed everything between them: - `composition_public_pub_use_surface_matches_snapshot` - `extension_host_cluster_stays_internal` - `reborn_binary_main_is_thin_bootstrap` - `composition_crate_installs_installed_tier_only_through_registrar` - helpers `composition_src_path`, `extract_pub_use_surface`, `has_module_decl`, `is_test_module_file`, `strip_test_module` It compiled and the file's own suite went green, because each deleted test left with the helpers only it used — which is exactly why "the suite passed" is not evidence. It was caught by diffing the function roster against `origin/main` rather than by a test, and by the commit's own −301/+114 line count. This restores the file from `origin/main` and re-applies the change with targeted edits instead of a span replacement. The roster is now **purely additive** against `origin/main` — 9 functions added, **0 removed**, verified with `comm -23`: - `composition_root_embeds_no_prompt_content` (the gate) - `markdown_include_sites`, `markdown_assets` (helpers) - 8 scanner unit tests 7 tests on `origin/main` -> 16 here. Both halves of the gate re-sabotage-checked after the restore: a nested `include_str!(concat!(env!(…), "…default_system.md"))` fails it, and a shipped `assets/prompts/s.MD` fails it. Also fixes what `Fast deterministic checks` caught on `fe641b7709`: clippy's `items after a test module` (the scan's test module now sits at the end of the file, after every helper) and two `doc list item without indentation` warnings (the doc comment is prose, not a list). `cargo clippy -p ironclaw_architecture --benches --tests --examples --all-features` is clean. The substance of `fe641b7709` is unchanged and still stands: statement-bounded include scanning, and `rust_sources` failing closed on unreadable directories and entries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * review(arch): skip Rust trivia when bounding the include statement Third CodeRabbit round on #7099. Both findings verified, both real, both fixed. **1. `.find(';')` could end the span before the path.** A semicolon inside a comment above the argument (`// see the note; below`) or inside the path literal itself (`"../a;b/prompt.md"`) terminated the scan early — and an ownership gate that ends early goes quiet, which is the failure mode this gate exists to prevent. `statement_end_after` now finds the first `;` that actually terminates a statement, skipping line comments, nestable block comments, normal strings with escapes, raw strings with any number of hashes, and char literals (while not mistaking a lifetime for one). It only has to locate a delimiter, not parse the expression, which keeps it ~50 lines. Three new tests, and the third is the one that keeps the fix honest: the span must still *stop*, or a markdown path in the **next** statement would make every non-markdown include a false positive. Sabotage-checked against the real crate with a semicolon-in-comment form — the gate fails. **2. `path.is_dir()` swallowed metadata errors in `rust_sources`.** Right: `Path::is_dir()` returns `false` on an error, so an unreadable directory left the walk silently. It now asks `entry.file_type()` and panics, matching `markdown_assets`. **Not done, with a reason rather than silently:** the suggested regression test for "an unreadable directory beneath an otherwise readable workspace". The only portable way to create one is `chmod 000`, which does not make a directory unreadable for `root` — and the CI containers run as root, so the test would pass locally and be vacuous in CI. A test that cannot fail where it matters is worse than none. The invariant is instead carried by construction: every read in both walks is `unwrap_or_else(panic!)`, with no `let Ok(..) else` and no `is_dir()` left in either. `reborn_composition_boundaries.rs` is 7 tests on `origin/main` -> 19 here, and the function roster is still purely additive (`comm -23` empty). Full `ironclaw_architecture` suite green; clippy `--all-features` clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * review(arch): reject symlinks in both composition ownership walks Fourth CodeRabbit round on #7099, and it is right. `DirEntry::file_type()` reports the **link's** type without following it, so a symlink pointing at a source directory is neither `is_dir()` nor an `.rs` file: both walks stepped over the entire subtree and the gate reported clean on source it never opened. Same "uninspected reads as absent" failure the fail-closed reads added in the previous round exist to prevent — one level further out. `reject_symlink` now panics for either walk, naming the path and the two ways forward. Rejecting is chosen over following deliberately: following needs canonical-root containment plus cycle detection to be safe, and neither scanned crate has ever contained a symlink (`find crates/ironclaw_reborn_composition/src -type l` is empty). The panic is where that decision gets made on purpose rather than silently. Regression test `a_symlinked_subtree_fails_the_walk_instead_of_being_skipped` builds a tempdir with a real source directory plus a symlink to it and asserts **both** `rust_sources` and `markdown_assets` panic. `#[cfg(unix)]`, since the workspace has a Windows lane and `std::os::unix::fs::symlink` is not portable. Sabotage-checked: commenting out both `reject_symlink` call sites turns the test red ("a symlinked subtree must fail the walk, not be skipped"); restoring them returns 20/20. `reborn_composition_boundaries.rs`: 7 tests on `origin/main` -> 20 here, roster still purely additive (`comm -23` empty). Full `ironclaw_architecture` suite green; clippy `--all-features` clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(event-store): stop leaking the Postgres driver in the public API (WS6) CHECKLIST WS6 / PROPOSAL §6.3.2: "stop leaking `deadpool_postgres::Pool` in the public API (wrap)". `ironclaw_reborn_event_store`'s public API now names `deadpool_postgres` zero times; the driver survives only inside its private `postgres_backed` module, which is where the TLS policy and pool construction §6.3.2 assigns this crate actually live. "Wrap" turned out to be three things, not one. **1. Half the leak was dead code, so it is deleted rather than wrapped.** `open_postgres_pool` and `open_postgres_pool_with_max_size` had exactly one caller each — composition's `open_reborn_postgres_pool` and `open_reborn_postgres_pool_with_max_size` — and those two had **zero** callers anywhere in `crates/`, `tests/`, `tools/` or `scripts/`. A four-function pass-through chain across two crates whose only remaining effect was to publish a third-party type in two public APIs. **2. The survivors take a carrier.** `open_postgres_pool_with_tls_options` returns `ironclaw_filesystem::PostgresConnectionPool` and `RebornEventStoreConfig::PostgresPool` holds one. The newtype lives in `ironclaw_filesystem`, not in event_store, for two reasons: it is the only crate `event_store`, `auth` and `composition` can all name without a new dependency edge, and that crate *is* the Postgres substrate, so the driver is chartered there (§11.2.6) rather than leaked. It is a carrier, not an abstraction — `driver()` / `into_driver()` exist for code that runs SQL — and it deliberately has no `Deref` (an implicit unwrap re-admits the driver into a signature unnoticed) and a hand-written `Debug` that renders nothing. The driver's own `Debug` prints its `tokio_postgres::Config`, which redacts the password (`tokio-postgres-0.7.16/src/config.rs:766-776`) but still prints `user`, `dbname`, `host`, `hostaddr`, `port` and `ssl_mode` — deployment topology that a derived `Debug` on any holder would inherit. **3. Stated residue: composition still names the driver, by charter.** §11.2.6 makes it "the one app-layer crate permitted a database driver", and it needs the raw pool for `PostgresRootFilesystem::new` and `CredentialRefreshLeaderLock::for_postgres`. It unwraps the carrier at exactly one site (`factory.rs`, `open_postgres_pool_from_source`). Pushing the carrier further down means changing `PostgresRootFilesystem::new`, which has **13 call sites across 5 crates plus `tests/integration/support/builder.rs`** — a separate test-wide slice, not this row. Recorded in both docs rather than left implied. **Enforcement (new file, lands with the change):** `crates/ironclaw_architecture/tests/reborn_persistence_driver_boundary.rs` - a shrink-only ratchet on which crates may hold a *normal* `deadpool-postgres` dependency (8 today, read from `cargo metadata`, not by eye), and - a scan proving event_store names the driver only below its private `postgres_backed` module — including that the module stays private, since a `pub mod` would silently defeat the scan. Both halves sabotage-checked: a planted `pub fn sabotage(p: deadpool_postgres::Pool)` fails the second and names the line; a planted `deadpool-postgres` dep on `ironclaw_projects` fails the first and names the crate. **Un-masking** (unfiltered `--list`, name-by-name, against `origin/main` in a clean baseline worktree): - `ironclaw_reborn_event_store` 71 → 71, roster identical - `ironclaw_reborn_composition` 928 → 928, roster identical - `ironclaw_filesystem` 296 → 296, roster identical - `ironclaw_architecture` 206 → 208, exactly the two new gate tests Deleting the four dead functions surfaced nothing, which is the evidence they were dead. No existing test edited. Guidance travels: `ironclaw_filesystem/CLAUDE.md` documents the carrier and its two deliberate omissions; `ironclaw_reborn_event_store/AGENTS.md` records that the driver cone is owned but not exported, and names the gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * review(arch): reject a symlink handed in as the walk root too Fifth CodeRabbit round on #7099, and right again — the previous fix closed the hole one level too late. `reject_symlink` only sees entries `read_dir` yields, but both walks push their **root** onto the stack before that ever runs, so a symlinked root was followed to its target silently. The regression test I added covered symlinked children only. `reject_symlink_root` now validates the root with `symlink_metadata` (which does not follow) before either walk starts, reusing the same rejection so the message and the policy stay in one place. The regression test is extended rather than duplicated: it now also symlinks a root and asserts **both** `rust_sources` and `markdown_assets` panic on it. Sabotage-checked — removing the two `reject_symlink_root` calls turns it red ("a symlinked walk root must fail rust_sources, not be followed"). Roster still purely additive against `origin/main` (`comm -23` empty); 20 tests in this file; full `ironclaw_architecture` suite green; clippy `--all-features` clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * review(arch): widen the driver-boundary scan past its two blind spots Three CodeRabbit threads on #7101, all naming the same real defect from different angles, and all correct: `take(module_start)` stopped the scan at the `mod postgres_backed` **header**, so the gate was strictly weaker than the three places documenting it claimed. Two blind spots, both now sabotage-fixtures rather than prose: - anything **after** the module body in `lib.rs` — a `pub fn` there naming `deadpool_postgres::Pool` kept the gate green; - **every sibling file** in the crate (`coalescing_sink.rs`, `durable_log.rs`), which the scan never opened at all. The scan now reads every `.rs` file under `crates/ironclaw_reborn_event_store/ src/` minus the brace-matched **body** of the private module. The brace match is trivia-aware (line comments, nestable block comments, strings, raw strings, char literals) so a `}` inside a literal cannot end the body early and silently drag the rest of the file into the exempt range — the same failure class one level down. It panics on an unterminated body rather than exempting to end-of-file, and asserts it saw at least two source files. Four unit tests on the brace matcher: a mention inside the body is exempt, a mention after the body is not, a brace in a literal does not end the body, and a file without the module has no exempt range. Sabotage-checked against the real crate for both former blind spots: - `pub fn sabotage_after_body(p: deadpool_postgres::Pool)` appended to `lib.rs` -> fails, naming `lib.rs:2215` - the same appended to `coalescing_sink.rs` -> fails, naming `coalescing_sink.rs:321` Also corrected the prose the reviewer flagged as over-claiming, in both places: `ironclaw_reborn_event_store/AGENTS.md` and the CHECKLIST WS6 row now say "module **body**" and state that the scan covers every file in the crate, with the earlier revision's blind spots recorded rather than quietly fixed. Clippy `--all-features` clean (the scan's test module moved to the end of the file for `items after a test module`); full `ironclaw_architecture` suite green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(extractors,observability): typed extraction failures and a one-dependency latency crate (WS6) CHECKLIST WS6 row "extractors: typed error across the boundary + delete caller-less `extract_text` (§6.4.10); observability: `json_value_bytes` eviction (§6.2.5)". Measurements from #7102. ## extractors (§6.4.10) Failures now cross the boundary as `ExtractionError`, not `String`, at both public sites (`DocumentExtraction::Failed` and `extract_document_text_by_filename`). Two variants: `UnsupportedType { mime }` (nothing was attempted) and `NotExtractable { detail }` (an extractor ran and could not produce text). `Display` renders the classification and nothing else; `Debug` carries the payload. That is not a shape change. The invariant — "carries the error reason for logging only; callers render a model-safe marker, never this string" — lived as a doc comment on one of the two boundary sites, and the *other* one leaked: `ironclaw_extension_support`'s `read_file` interpolated the raw extractor diagnostic into a model-facing safe summary (`coding/file.rs:325-329`) while carefully redacting the path one argument earlier. With `Display` content-free that call site is safe unchanged. Its regression test sits at the call site, not on `Display`, because the wrapper composing the summary is what leaked. `extract_text` and `TRUNCATION_MARKER` were both `pub` with zero external callers; both are private now. The row only named the first. The second mattered more: `ironclaw_agent_loop` and `ironclaw_mcp` each declare their own `TRUNCATION_MARKER` with a different value, so it must be resolved by crate, not by name. The census is exact — no crate writes `use ironclaw_extractors::…`, so a full-path grep is complete. The private ZIP-safety enum was renamed `ExtractionError` -> `ZipEntryError` to free the natural name. ## observability (§6.2.5) — delegated ruling, PROPOSAL §12.12 D-K `json_value_bytes` and its `JsonByteCounter` are localized into the two consumers; `serde_json` leaves the manifest with them, so the crate now holds exactly one dependency, `tracing`. The row's stated reason ("gravity-well hygiene") was wrong; the ruling survives on a measured one. Of five call sites in extension_support, three feed `ResourceUsage::set_output_bytes` — resource accounting, not a trace field — so "it is a latency helper, in charter" is false. And sharing bought no invariant: `output_bytes` is already computed three different ways in production (this counter, `output.stdout.len()` in `ironclaw_scripts`, `Value::to_string().len()` in `ironclaw_loop_host`), because each producer measures what it produced. `ironclaw_common` was rejected (the crate the restructure is actively narrowing) and `ironclaw_host_api` was rejected explicitly rather than by omission (behavior in the contracts leaf is the specific criticism already on record against it). Cost, stated: ~18 lines and 2 unit tests duplicated across two crates. ## Guidance and docs New `AGENTS.md` for both crates (both rows asked for one). PROPOSAL §6.4.10 and §6.2.5 amended with dated notes quoting what they replace; §12.12 opened as the Wave 4 delegated-decision log, continuing §12.11's lettering and marking discipline. `families/domains.md` and `families/substrates.md` updated, including a sharpened "never contains" test for observability and a corrected security role for extractors (its failure type is a redaction boundary; "none" was wrong). ## Tests Unfiltered per-crate `--list`, before -> after: extractors 26 -> 28, observability 2 -> 2, attachments 39 -> 39, host_runtime 1247 -> 1249, extension_support 152 -> 156, architecture 206 -> 206. Nothing deleted; nothing edited for content. Observability's two tests moved with the function and are now duplicated in both consumers (2 -> 4 workspace-wide); its two replacements pin what actually remains in the crate. Both new guards were sabotage-verified: break the invariant, confirm red with the right message, restore, confirm green. Coverage floors untouched and deliberately so: the source crate (`ironclaw_observability`) has no floor entry, and the destination `ironclaw_host_runtime` gains covered lines rather than losing them. Found and filed rather than patched: #7103 (the coding tool computes its JSON byte count before checking whether latency tracing is on) and #7104 ("no text found" classifies as `Failed` rather than `Empty`, so the model is told the wrong thing about a valid but text-free document). * fix(extractors): ASCII-only extension normalization + narrow the Debug-payload guidance Review triage for #7106. **CodeRabbit thread 2 — accepted.** `.claude/rules/types.md:170` and `review-discipline.md:45` require case-insensitive external values to be normalized with `to_ascii_lowercase()`, not Unicode case folding. Both extension registries in this crate used `to_lowercase()`; the sibling registry in `ironclaw_extension_support::coding::file` (`should_extract_document_before_text`) already got it right, so this is the outlier. Note it is a latent-hazard fix, not a live bug: the eight keys (pdf/docx/pptx/xlsx/doc/ppt/xls/rtf) contain none of the letters a Unicode fold can produce from a foreign codepoint, so I could not construct an input where the two differ today. It removes the hazard for the next key added. Test pins both halves: ASCII case-insensitivity still works, and a non-ASCII extension is not folded into an ASCII key. **CodeRabbit thread 1 — guidance tightened, code change refuted.** The reviewer is right that this crate's doc told callers to `tracing::debug!(?error, …)` without naming a ceiling, while `ironclaw_host_runtime/AGENTS.md:28` forbids unredacted user content in that crate's logs. Both docs now say the payload belongs in an operator log and nowhere else, and record what it actually carries. The proposed code change is refused with measurement in the PR thread: it would log strictly less than `main` does today. * fix(extractors): the Unicode extension fold was a live bug, not a latent one Correcting my own claim in 0e7d14e and in the #7106 review reply. I wrote that `to_lowercase()` vs `to_ascii_lowercase()` was observationally equivalent here and that I "could not construct an input where the two differ". That was measured against only ONE of the two extension registries. `try_extract_by_extension`'s key set is much larger than `extract_document_text_by_filename`'s eight, and it contains `markdown`: "MAR\u{212A}DOWN".to_lowercase() == "markdown" // U+212A KELVIN SIGN -> k "MAR\u{212A}DOWN".to_ascii_lowercase() == "MAR\u{212A}DOWN" So on `main`, a file named `notes.MAR<U+212A>DOWN` carrying an unrecognized MIME type took the filename fallback in `extract_text`, was UTF-8-decoded, and reached the model as markdown instead of being rejected as an unsupported type. `bash` and `zsh` are in the same key set for the same reason. Caught by CodeRabbit on #7106, which constructed the input I said did not exist. Recorded here rather than quietly repaired: the earlier reply's measurement was wrong and the switch at :707 is a behaviour fix. Regression test extends `extension_matching_is_ascii_case_insensitive_and_ nothing_more` with the `markdown` fold in both registries plus the public `extract_document` path that actually reaches the fallback. Sabotage-verified: reverting :707 to `to_lowercase()` turns it red on the named assertion. * fix(arch): make the driver-boundary visibility check reachable and the scan multi-line safe Review found this gate weaker than its docs for the third time. Both findings were real; both are fixed at the seam and pinned in both directions. 1. The `pub mod` assertion could never fire. The header was matched with `starts_with("mod postgres_backed {")`, so a line beginning `pub ` was not the matched header and the `!starts_with("pub ")` assertion below it was dead. A visible module was simply not found: the exempt range came back empty and the failure blamed whichever driver mention was reported first rather than the visibility change that broke containment. The header now keys on the `mod postgres_backed {` token and asserts on the captured visibility prefix, so `pub` and `pub(crate)` both fail by name. 2. String state did not survive a newline, and that was fail-open. Block comments were carried across lines; regular and raw strings were not, so the continuation lines of a multi-line literal were scanned as code. A `}` there truncated the body, and a `{` there stretched it past the module's real end and swallowed every driver mention after it. With an unbalanced `{` in a multi-line literal and a `deadpool_postgres::Pool` in a public signature after the body, the old scan reported ok; the new one fails on lib.rs:2217. The raw-string terminator is now searched over bytes, so a multi-byte character in a literal cannot leave the index off a char boundary and panic. Regression tests (all failed before the fix, except the last which had no fixture at all): multi-line literal boundary in both directions plus raw strings, `pub mod` and `pub(crate) mod` rejection, the widened header match not mistaking a comment or string for the declaration, and the unterminated-body panic that AGENTS.md and CHECKLIST.md both present as part of the guarantee. Both fixes sabotage-checked against the real event_store source, not only fixtures. The weakness is recorded in the CHECKLIST row and AGENTS.md rather than quietly repaired. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(config): retire the vendor config sections behind a generic window (WS6) `[slack]` and `[telegram]` were the last per-vendor sections in `ironclaw_reborn_config`. Nothing reads them: the enablement gate they fed was deleted with the unified extension runtime (#6116), so `config set slack.enabled true` printed "saved" for a value with no runtime consumer. Replaces the typed vendor schema with a generic retired-section table: - delete `SlackSection`, `SlackChannelRouteSection`, `TelegramSection`, their three builders, and `update_slack_enabled` - `RebornConfigFile` no longer names a vendor; retired sections are split off the raw document before the typed parse, so the schema stays `deny_unknown_fields` - `reject_legacy_slack_config` becomes `reject_retired_config_sections`, data-driven by the same table (PROPOSAL §12.2's "relocated shape") - `config set slack.enabled` now answers with migration guidance instead of writing a value nothing reads Compatibility window preserved and widened: an existing `config.toml` still parses, a retired *setup* key still fails the boot closed with the same message, an inert section still boots — and now says so instead of being silently ignored. Inline-secret rejection over retired sections goes from nine hardcoded keys to every string at any depth. Parse diagnostics: files with no retired section keep the line/column span on unknown-field errors (the split re-parses the original text); only files already carrying a retired section see the degraded form. Measured, and pinned by a test. Sabotage-testing the new guards found one of them inert: the scalar re-insert test only covered `slack = 1` alone, which takes the fast path and would catch it either way. Widened to `slack = 1` beside a genuine retired section, which is the case that actually bypasses `deny_unknown_fields` without the re-insert. The reachability-vs-fidelity limit of the table-driven key test is recorded in its doc rather than papered over. Extension-specificity allowlist 127 -> 125 (baseline lowered to match): the two surviving vendor tokens are the TOML table names, quarantined in `retired_sections.rs`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: correct the Slack/Telegram enablement gate that no longer exists The retired `[slack]`/`[telegram]` sections had a documentation half. Five operator-facing docs still taught a gate deleted by #6116 (2026-07-21): `setup-slack-for-reborn-binary.md` called it the binary's "one gate" and described `IRONCLAW_REBORN_SLACK_ENABLED=false` as a "deployment kill switch" (it is not — Slack stays mounted), and its troubleshooting step could never fix anything. README instructed a `config set slack.enabled` command that now fails. Replaces the gate story with the real one everywhere: the ingress route is compiled in and mounted unconditionally, answers 503 until the extension's signing secret is registered, and 401 on signature mismatch — Slack and Telegram go live by installing the extension and finishing setup at /extensions. Adds a migration note where an operator with an existing file would look. Also removes `IRONCLAW_REBORN_SLACK_PERSONAL_OAUTH_REDIRECT_URI` from `docs/channels/slack.mdx`: zero readers in `crates/`. The CLI already had a regression test asserting that variable must never be advertised in remediation text, so its retirement was known — only the docs kept saying it. Records amendments in the target-architecture docs (CHECKLIST WS6 rows, PROPOSAL §6.10.3 with the placement decision and rejected alternatives, §12.2's compat constraint) and corrects a phantom test citation in the extension-runtime checklist. Filed rather than patched: #7115 (docker entrypoint gates its migration on the dead env var, so following the docs skipped it) and #7116 (live-QA runner gates Slack cases on a value it writes itself). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci(planner): classify `.env.example` so a comment fix is not a full-matrix failure The Reborn PR test planner is fail-closed on unknown paths, and had no rule for `.env.example`. Repo-root `*.md` was classified; its non-`.md` sibling was not, so this PR's env-var comment correction aborted the planner with `unclassified pull-request path: .env.example` and failed the whole `Tests (Reborn)` roll-up on a change with no build surface. Nothing reads the file — no crate, test, or workflow; only doc comments name it by name. Classified rather than exempted, following the `.claude/` precedent added 2026-08-03, whose comment states the rule this follows: classify the path, do not loosen the arm that catches genuinely unknown ones. Regression test asserts all three halves: the path is accepted, it selects no Rust lane (so a future "classification" that turns a comment fix into a full matrix also fails), a real change riding along still selects its lane, and an unknown root file (`.env.local`) still raises. Verified by sabotage — removing the classification turns the new test red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(composition): gate three test-support-only imports so dependency builds lint clean `origin/main` already fails `Code Style` clippy for the package set `{ironclaw, ironclaw_reborn_config}` — verified on a clean detached checkout of `dfdd02b9fb`, exit 101, three unused imports in `composition/src/runtime.rs`. This PR is simply the first to produce that set, so it inherited the failure. Mechanism: the PR clippy lane derives `-p` from the diff and adds `--all-features`, which applies to *selected* packages only. All three imports are named solely by `#[cfg(any(test, feature = "test-support"))]` accessors, so when composition is a mere dependency its `test-support` is off, `--lib --bins` also drops `#[cfg(test)]`, and the imports go unused. With composition in the selected set, `--all-features` turns the gate on and the same command passes. Gating the imports to match their users is the minimal correct fix — they are used, so deleting them would be wrong and `#[allow]` would hide the real property. Verified both directions: the PR-lane invocation and `-p ironclaw_reborn_composition --all-targets --all-features` are now both exit 0. The class of bug — a lint gate whose verdict depends on which packages a PR happened to touch — is #7119; this commit only unblocks. Touching an otherwise-occupied crate deliberately kept to three `#[cfg]` attributes and a comment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: review fixes — google CLI path, Slack setup location, retired-key wording Three CodeRabbit findings, each verified before acting: - `capabilities/configuration.mdx`: `config set google.*` is still a supported path (README and `using/cli.mdx` both document it), so "configure it from the web interface rather than by hand" was wrong. Names both paths now. - `reborn/setup-slack-for-reborn-binary.md`: the 503 troubleshooting step pointed at `/extensions` generically and then called the same thing "Admin Configuration" — a third name for a place `docs/channels/slack.mdx` documents precisely (Extensions -> Channels tab -> Configure on the Slack card), including a warning that Extensions opens on the Registry tab, which is not it. Aligned to that wording, since it is the more specific of the two and matches the UI. - `using/cli.mdx`: "everything else is edited in config.toml directly" no longer holds for retired keys. The fourth finding is refuted in the thread: it asked for a "retired setup keys fail at serve" caveat on the `[telegram]` note, but `RETIRED_SECTIONS` gives telegram `rejected_keys: &[]` — it never had a setup field, so no `[telegram]` section can fail a boot. Adding the caveat would document behaviour that does not exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(slack): tie "Admin Configuration" to the Slack card once, in the guide The setup guide names the operator-facing concept ("Admin Configuration for Slack", 7 references) while docs/channels/slack.mdx names the UI path (Extensions -> Channels tab -> Configure on the Slack card). They are the same dialog, but nothing said so, and my earlier fix only rewrote the troubleshooting paragraph — leaving one place described two ways. Defines the equivalence once, next to the first use, and points the 503/401 steps back at it instead of restating the UI path a second time. Rewriting all seven references would churn a guide this PR is otherwise only correcting for the retired enablement gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(traces): split contribution.rs into chartered modules `crates/ironclaw_reborn_traces/src/contribution.rs` was 17,470 lines — the largest single file in the tree — and carried an `// arch-exempt: large_file` waiver from a 2026 mechanical rename (plan #6168). WS6's domain-internal cleanup row and PROPOSAL §6.4.14 both call for splitting it into chartered modules. It becomes a directory module of 13 production submodules plus a mirrored test tree, each named for one owner in the pipeline (capture → redact → classify → score → queue → submit). `src/contribution/mod.rs` carries the charter table that says which module a new item belongs to, plus the two rules that keep it honest: redaction is split by key (pattern vs tool-name), and `queue` owns state / `remote` owns the wire / `submission` is the only caller of both. The waiver is deleted rather than carried forward, and no new one is added: every file is under the 1,500-line ARCH-SPRAWL threshold (largest is 1,290). No public API change and no consumer edits. The submodules are private and `mod.rs` glob-re-exports them, so `contribution::X` remains the single public path for all four consumer crates. Items that newly cross a module line were widened to `pub(crate)`, never to `pub`. Verification: - Item roster diffed against origin/main: 501 top-level items before, 501 after, zero missing and zero extra. - Unfiltered `--list` before and after: 216 lib tests, leaf names identical. All 216 + 2 integration tests pass. - `cargo clippy --benches --tests --examples --all-features` clean on ironclaw_reborn_traces and ironclaw_architecture. The four `PATH_TERM_COLLISIONS` carve-outs that pinned the old file path are repointed and, in the process, narrowed: the vendor-name safety denylist now resolves to `tool_payloads.rs` (the rule tables) and `classification.rs` (external-write detection, `slack` only) instead of one 17k-line whole-file carve-out, so the specificity gate now polices the rest of the module. Those entries are staleness-checked, so the old path would have failed loudly. Adds the crate's first guidance file, recording the glob-re-export invariant and the three known gaps on §6.4.14's row that this PR does not close (ScopedFilesystem adoption, the two re-export modules, the crate rename). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(reborn): record the traces contribution.rs split and correct two stale clauses Amends CHECKLIST WS6's domain-internal-cleanups row and PROPOSAL §6.4.14 (plus the anti-pattern inventory and the crate-disposition table) with what landed, quoting the text each amendment replaces. Two corrections the work surfaced, recorded rather than silently fixed: - §6.4.14's "17,467-line contribution.rs" measured 17,470 on main; the file drifted after the entry was written. - The CHECKLIST's shorthand "`ScopedFilesystem` + re-export modules dropped" is worded backwards for the first clause. `ScopedFilesystem` is `ironclaw_filesystem`'s type, is used by ~170 files across the workspace, and is absent from `ironclaw_reborn_traces` entirely — there is nothing to drop. §6.4.14's actual instruction is adoption ("take a `ScopedFilesystem` instead of raw `dirs`/env access"), which is a persistence-plane change across ~91 raw fs call sites, not a deletion. Left as-is with the reason stated, so the next reader measures rather than inherits. Also records why the two remaining traces clauses did not land in this wave: dropping the `recording`/`paths` re-export shims needs edits in `ironclaw_reborn_cli`, and `recording` additionally needs a decision because the CLI has no `ironclaw_llm` dependency to fall back on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(traces): serialize test process-env mutation behind lock_env() The split re-surfaced five unguarded `std::env::set_var`/`remove_var` call sites that CI's `check-hermetic-env.sh` had been grandfathering: they are byte-identical pre-existing lines (contribution.rs:10501/10513/10515/15648/ 15661 on origin/main), and the gate only skipped them because it is delta-scoped and the file had not been re-added since it was written. This is a real gap, not a false positive, so it is fixed rather than annotated. `EnvVarRestore` restored the previous value on drop but took no lock, so two tests mutating the environment on different threads still raced — undefined behavior on Rust 1.82+ regardless of whether they name the same variable. `workload_token_env_mode_reads_env_unchanged` used a uniquely named variable, which avoids logical interference but not the setenv/getenv data race. Both now acquire `ironclaw_common::env_helpers::lock_env()`, the sanctioned helper the gate's message names. `EnvVarRestore` holds the guard as a field declared last, so it is released only after `Drop::drop` has restored the value — the restore is inside the critical section, not after it. The real process environment is kept (not `env_helpers::set_runtime_env`'s overlay) because the sidecar isolation test needs a value a child process would inherit, to prove `CommandPrivacyFilterAdapter` clears it. One `#[allow(clippy::await_holding_lock)]` on the async test, matching the precedent in `ironclaw_operator/src/llm_admin/llm_config_service.rs`: holding the lock across the await is the intent, and `#[tokio::test]` drives the future on a current-thread runtime so the guard never crosses threads. Verified: `check-hermetic-env.sh` exits 0, clippy clean, 216 + 2 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(traces): apply CodeRabbit review — carried waiver, inert test, charter drift Six findings verified against the code; four were defects this PR introduced or carried, and each is fixed. 1. **A second file-size waiver was carried forward after all.** `queue.rs` still held the in-body "File-size justification … already-oversized module … decomposition tracked in issue #4088" block, which contradicts a PR whose whole point is performing that decomposition. Deleted; the coupling rationale it was wrapped around (why credential resolution lives beside the policy/scope-dir helpers) is kept, since that still explains the layout. 2. **`invite_code_gated_by_auth_mode` was inert.** It re-implemented the `match policy.auth_mode` expression from `build_trace_upload_claim_issuer_request` and asserted against its own copy, so deleting the `DeviceKey => None` arm in production left it green. It now calls the production builder and asserts on the *serialized* request, so a field rename cannot hide a leak either. Sabotage-proved: removing that arm now fails with the leaked invite code visible in the body. 3. **The charter claimed "each stage owns one file"**, which `remote`'s four files contradict. Reworded to module-level ownership, naming `remote` as a directory module and why. `CLAUDE.md`'s test-layout paragraph gets the same correction plus the explicit `remote` → four-test-module mapping. 4. **Five policy-serde tests sat in `claims.rs`.** They verify `StandingTraceContributionPolicy`, whose owner is `policy.rs`, and the PR's own rule is that a test lives with its production owner. Moved to a new `tests/policy.rs`; leaf names unchanged. 5. **Three orphan section headers** left behind by the split, describing tests that now live in other modules (`credentials.rs`, `profile.rs`, `value.rs`). Deleted. The remaining two findings are real but pre-existing and need behavior changes, so they are filed as #7127 rather than fixed here: the case-sensitive remote `status` comparison that skips the local revocation record, and `fetch_account_traces` taking two adjacent `&str` where its sibling takes `&TenantId, &UserId` (its fix needs an edit in `ironclaw_product`). The issue also carries the `trace_scope_has_pending_queue` doc/code mismatch, which needs an intent decision rather than a guess. Re-verified: 501/501 production items, 216 tests with identical leaf names, clippy clean, hermetic-env clean, every file under 1,500 lines. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(traces): use the RAII env guard and cover the bearer at the caller Second CodeRabbit pass, both findings on the test this PR had already touched. 1. **RAII guard instead of manual cleanup.** `workload_token_env_mode_reads_env_unchanged` set the variable, awaited, asserted, then removed it — so any panic before the last line leaked the variable into every later test. It now uses `EnvVarRestore::set`, whose `Drop` restores during unwinding while holding the same process-env lock. That also deletes both `unsafe` blocks and the `#[allow(clippy::await_holding_lock)]`: the guard lives in a struct field, which the lint does not flag, so the suppression is no longer needed. 2. **The bearer token had no caller-tier coverage.** Five tests assert what `issuer_request_bearer` returns; none asserted the token reaches the wire. The direct issuer path attaches it conditionally (`if let Some(bearer) = issuer_bearer { request.bearer_auth(bearer) }`), so a helper regressing to `None` would send an unauthenticated request with every existing test green — the repo's "test through the caller" rule names exactly this shape. Adds `workload_token_reaches_the_issuer_request_as_a_bearer_header`: a mock issuer captures the `Authorization` header while `fetch_trace_upload_claim_from_issuer` drives the real path. Sabotage-proved — dropping the `bearer_auth` attach fails it with `left: None, right: Some("Bearer wire-bearer-xyz")`; restored, green. Test accounting: 216 → 217. All 216 original leaf names still present (diffed against the `origin/main` baseline); the one addition is the new caller-tier test. Clippy clean, hermetic-env clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(llm): add the enforced sub-owner map (WS6 module charters) PROPOSAL §6.4.13 asks `ironclaw_llm` for "internal module charters for its five sub-owners". This adds the map to `crates/ironclaw_llm/CLAUDE.md` and, because a charter nobody checks rots within a release, a test that pins it. **Five sub-owners were not enough, measured.** `providers` / `auth-sessions` / `registry` / `decorators` / `recording` own 28 of 48 files (79.6% of lines), leaving 20 unowned — including `lib.rs`, `provider.rs`, `error.rs` and `config.rs`. Five more are named, each with a stated reason rather than a residual bucket: `core-contract` (the trait, vocabulary, error taxonomy and config are *upstream* of every implementor, so charging them to `providers` would make providers own decorators' and recording's own dependencies), `normalization` (cross-provider wire hygiene, as opposed to the single-provider shims that stay beside their provider), `model-catalog` (facts about *models*, a different noun from registry's catalog of *providers*), `transcription` (`TranscriptionProvider` is a different trait; nothing there implements `LlmProvider`), and `test-support` (a published feature with its own compatibility obligation). **`tests/module_charter.rs` enforces it.** Every `src/**/*.rs` must appear in exactly one row, every path in a row must exist, and no file may be claimed twice. Sabotage-proved in all three directions — dropping `retry.rs` from the table, adding a phantom path, and double-claiming `registry.rs` each fail with the right message; restored green. The test also guards itself: it fails if the table parses to zero rows or if the source walk finds implausibly few files, so a table-shape change cannot silently turn it into a no-op. **§6.4.13's "Deletes: reasoning.rs (4.5k lines, zero external references)" is refuted.** The file is 1,299 lines after #6964 removed its dead half, and the survivor is live: `lib.rs:88-91` re-exports three helpers with five production call sites in `crates/ironclaw_loop_host/src/model_gateway.rs`. It is charted under `normalization`. `AGENTS.md` carried the same staleness ("legacy reasoning engine") and is corrected; it also now points at the map as authoritative so its informal buckets cannot quietly become a second source of truth. Four placement calls are recorded rather than left implicit: `token_refreshing.rs` is auth-sessions not decorators (CLAUDE.md and AGENTS.md disagreed); `runtime.rs` and `smart_routing.rs` force the decorator definition to widen from "reliability wrapper" to "wraps `dyn LlmProvider` and is not credential work"; `url_check.rs` is core-contract; and `gemini_oauth.rs` is genuinely two owners in one file, charged to the larger half with the split recorded as owed. CHECKLIST and PROPOSAL §6.4.13 carry dated amendments quoting the text they replace, including why the row's `providers.json` clause is blocked (its load-bearing include site is in `ironclaw_reborn_cli`, which is occupied, and it needs a new mechanism rather than a new path). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(traces): correct the claims/policy test-module docs after the move The script that moved the five policy-serde tests copied `claims.rs`'s preamble verbatim, so `policy.rs` ended up with two module docs — its own and a carried-over line describing claims. And `claims.rs`'s own doc still opened with "Standing-policy serde", which stopped being true the moment those tests left. `policy.rs` keeps only its own doc; `claims.rs` now describes what it actually covers (upload-claim cache keys, issuer error labels, the bearer the issuer request carries, device-key auth modes) and points at `policy.rs` for the policy serde contract. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(arch): lower the specificity ALLOWLIST baseline 125 -> 124 after the re-baseline #7117 measured `ALLOWLIST` 127 -> 125 against `origin/main` @ `1e2a294083`. #7094 then deleted one entry on `main` (127 -> 126), so this branch's two net removals now land on 124, not 125. The ratchet is `<=`, so it stayed green at 125 while carrying a unit of untracked slack — exactly what the constant's own doc forbids: "Lower it in the same PR that deletes entries so the new floor is locked in." Read off the ratchet's own failure message with the baseline temporarily set to `0` ("ALLOWLIST grew to 124 entries"), never counted by eye — a plain paren count over the literal answers 142, because the entries' comments contain parentheses too. Sabotage-verified in both directions: baseline 123 goes red naming 124, and 124 is green 7/7. The file's function roster is unchanged. * docs(checklist): map the WS6 "Domain-internal cleanups" row clause by clause The row bundles eight clauses and the Wave 4 part-1 consolidation closes one of them (the `traces` `contribution.rs` split). It stays open, correctly — but a reader of the row could not tell which of the remaining seven had been measured and which had not, and the `llm` `providers.json` measurement lived on the "Module charters" row two rows down because that is where the agent who made it was working. Adds item 7: a clause-by-clause status map — one done, three measured with the blocker named (including a pointer to where `providers.json` was measured), four untouched. No box is ticked; the row's real condition is unmet and stays unmet. Also fixes a stray space-semicolon left in the "Composition behavior evictions" row where the system-prompt clause was struck through. * review(ws6): fix seven findings on code this consolidation introduced CodeRabbit's pass over the consolidation raised 40 threads. 29 are on production code #7124 only *moved* and are filed as #7144. These seven are on code this program wrote, and all seven were correct. **A gate that was not scanning what its doc claimed.** The driver-boundary walk used a flat `read_dir` while its doc said it scans "**every** `.rs` file in the crate". `crates/ironclaw_reborn_event_store/src` is flat today, so nothing escaped — but `src/postgres/pool.rs` is exactly where a driver mention would go, and a skipped file is indistinguishable from a clean one. Now recursive and symlink-rejecting, matching the shape `reborn_composition_boundaries.rs` already uses in this same PR. Sabotage-proved against the real crate: a nested `postgres/pool.rs` naming `deadpool_postgres::Pool` now fails the gate naming `pool.rs:1`, and passed silently before. This is the third revision of this gate found weaker than its own docs; the doc now says why. **A charter gate that a table reformat would have broken.** `module_charter.rs` matched the separator row with `cells[0].starts_with("---")`, so an aligned separator (`|:---|:---|`) parsed as a *data* row: `:---` became an assigned path, `saw_row` went true so the shape guard stayed quiet, and the stale assertion reported `:---` instead of a diagnosis. Sabotage-proved both ways — with the fix reverted and the table rewritten in aligned form the test goes red on `:---`; with the fix it passes. Also: - `CONTRACT.MD` added to the composition guidance allowlist. The repo already ships it as crate-local guidance (`ironclaw_reborn_identity`, `ironclaw_trust`) and CLAUDE.md's module-spec table names it, so a composition `CONTRACT.md` would have been reported as prompt content and sent the author to the wrong fix. - `markdown_assets` gains its first real test: the case-insensitive `.md` match and the caller's guidance filter were both unpinned, and both drift quiet. - Two fixtures for comment-braced module bodies (line comment, nested block comment) — the scan handled them, nothing pinned it. - The symlink rationale doc block moved onto `reject_symlink`, which it describes; it was stacked above `reject_symlink_root` with no item between, so both attached to the wrong function and `reject_symlink` was undocumented. - The retired-section deprecation warn gains `target = "ironclaw::reborn::cli::serve"`, like every other warn on that path. Announcing an inert section is pointless if an operator filtering the documented startup target cannot see it. - `ironclaw_reborn_traces/CLAUDE.md` claimed a one-to-one test mapping that `tests/credentials.rs` breaks (it spans `queue.rs` and `remote/claim.rs`). The exception is now stated rather than left to be inferred. Rosters in both architecture test files are purely additive; no test removed. * docs: correct the extension-specificity allowlist numbers after the re-baseline Caught in review of #7139. Both ledgers still recorded #7117's measurement, `Extension-specificity allowlist **127 → 125**`, taken against `origin/main` @ `1e2a294083`. #7094 then deleted an entry on `main` (127 → 126), so the same two net removals land on **124**, which is what the shipped baseline says. This is the cross-slice-number failure mode the consolidation exists to catch, one layer down: the code was corrected in 811bfedeff and the prose was not. Both amendments quote the text they replace and record the method — read off the ratchet's own failure message with the baseline temporarily set to 0, never counted by eye. No checkbox state changed. * review(ws6): three more review findings, one of which broke my own fix **My `target =` fix did not work, and CodeRabbit was right to call it.** `tracing::warn!(target = "…")` records a *field* named `target`; it does not set the event's metadata target, which stays the module path. So the retired-section notice — given a target in #7117 precisely so operators would see an inert `[slack]`/`[telegram]` section announced — was still invisible to a subscriber filtering `ironclaw::reborn::cli::serve`. Measured with a capturing subscriber rather than argued: EQUALS-SYNTAX target = "target_probe" <- module path COLON-SYNTAX target = "ironclaw::reborn::cli::serve" <- correct Now `target:`, and pinned by `retired_section_notice_is_emitted_on_the_serve_target`, which asserts the emitted **metadata** target through the real `reject_retired_config_sections` call. Sabotage-proved: the `=` form makes it red with `observed targets: ["ironclaw::commands::serve"]`. This is repo-wide — **121 sites** use the `=` form against an `ironclaw::…` target, including the three sibling warns on this same serve path (`:318`, `:387`, `:454`). Filed as #7146 rather than fixed here; a consolidation should not carry a 121-site mechanical change. **The markdown gate's test was testing a copy of itself.** My new test carried its own duplicate of the guidance allowlist, so the production filter could drop `CONTRACT.MD` and the test would still pass — the "test through the caller" rule. Extracted `is_crate_guidance` / `shipped_non_guidance_markdown`; the gate and the test now share one path. Sabotage-proved by dropping `CONTRACT.MD` from the shared helper: red with `left: ["CONTRACT.md", "seed.MD"]`. **The separator fix had no committed regression test.** It was sabotage-proved by hand, which does not survive the session. `parse_sub_owner_table` is split out from the file read so a fixture can supply separator shapes the checked-in `CLAUDE.md` does not use, and `an_aligned_separator_row_is_not_parsed_as_data` covers unaligned, left-aligned and centred. Red when the fix is reverted. Rosters purely additive in all three files; no test remove…
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the last unlanded piece of the WS8 safe-deletion tranche: the dead half of
ironclaw_llm::reasoning. PR #6943 excluded this module after discovering ithalf-live and left an enumeration for a scoped PR to re-verify. This is that PR.
Deletion only. No moves, no renames, no behavior change.
How the surface was established
reasoningis a private module (mod reasoning;,lib.rs:32), so its entireexternal surface is the two
pub use reasoning::{…}blocks atlib.rs:88-98.Nothing outside
ironclaw_llmcan reach a symbol that is not in those blocks —which makes the verification exhaustive rather than best-effort.
Verification method, run against this PR's own base (
31f42b790):use ironclaw_llm::{…}statement in the workspace, flattened across linebreaks (77 import lines, all
.rsfiles,target/excluded);ironclaw_llm::<Name>path;use ironclaw_llm::*andpub use ironclaw_llm…. Two exist (tests/support/trace_llm.rs:25,ironclaw_reborn_traces/src/lib.rs:22); both are onrecording, notreasoning.Bare-name grep was deliberately not used as the deciding evidence:
Reasoning,TokenUsage, andResponseMetadataall collide with unrelated types elsewhere inthe tree (e.g.
rig::message::Reasoning), which is the trap that makes anenumeration a lead sheet rather than truth.
Per-symbol verification
Enumerated dead set — 20/20 confirmed dead, all deleted
ReasoningReasoningContextActionPlanPlannedActionToolSelectionTokenUsageResponseAnomalyResponseMetadataRespondResultRespondOutputCommunicationPresentationPolicySuccessEvaluationSILENT_REPLY_TOKENTOOL_INTENT_NUDGETRUNCATED_TOOL_CALL_NOTICEis_silent_replyllm_signals_tool_intentuser_signals_execution_intentrecover_tool_calls_from_contentrecover_codex_text_tool_calls_from_contentPlannedActionandSuccessEvaluationarepubinside a private module and werenever re-exported — unreachable from outside the crate by construction.
No exclusions. Every enumerated name re-verified dead on this base; none had a
production consumer outside the dead cluster.
Live helpers — kept, untouched
clean_responseironclaw_runner/src/model_gateway.rs:26contains_codex_text_tool_call_syntaxironclaw_runner/src/model_gateway.rs:26recover_codex_text_tool_calls_from_tool_namesironclaw_runner/src/model_gateway.rs:27Their whole private closure is kept and unmodified:
find_code_regions,is_inside_code,overlaps_code_region,line_bounds,is_recoverable_tool_call_segment,recover_codex_text_tool_calls(_from_name_set),parse_codex_text_tool_call_at,strip_codex_text_tool_calls,strip_markdown_fence_block,strip_bracket_tool_calls,strip_thinking_tags_regex,extract_final_content,strip_pipe_reasoning_tags,strip_xml_tag,strip_pipe_tag,collapse_newlines,CodeRegion,TOOL_TAGS,RECOVERED_TOOL_CALL_SEED, and the fourLazyLockregexes.Not enumerated, deleted as transitively dead
The checklist's list was a lead sheet; three more items fell out once their only
caller went. Each is recorded with the evidence that made it dead:
truncate_at_tool_tagsimpl Reasoning(base lines 645, 679, 727, 741, 806, 890, 909, 940, 970, 1005).clean_responsenever called itclosing_tag_fortruncate_at_tool_tagsTOOL_TAG_PATTERNStruncate_at_tool_tags+closing_tag_forThis is the one judgment call in the PR and it is worth stating plainly:
truncate_at_tool_tagsis real behavior (truncate a reply at an unclosed tool tag)with 40 tests behind it — but it had no production caller before this PR either.
The live gateway path reaches
clean_response, which strips tool tags viastrip_xml_tag/strip_pipe_tag; it never truncated. So no reachable behavior islost here; an already-unreachable helper stops being compiled. Leaving it would
have meant shipping
dead_codewarnings, which the WS8 exit criterion(flip
unreachable_pub+dead_codeon forcrates/**) exists to prevent.Private helpers that fell with their sole callers, compiler-verified:
strip_code_blocksandstrip_quoted_strings(called only fromllm_signals_tool_intent/user_signals_execution_intent),merge_system_messagesand
extract_json(called only fromimpl Reasoning), and the test-onlyTruncatingLlmmock (mockedReasoning::select_tools).Un-masking evidence
Full unfiltered suites, before and after, no
--filters:cargo test -p ironclaw_llmcargo test -p ironclaw_runnerironclaw_runneris the crate that consumes the live helpers: its roster isidentical before and after (
diffof the two sorted test-name lists is empty),so the surviving path is exercised exactly as before.
Within
ironclaw_llm, the loss is entirely inside the reasoning module(182 → 66 tests, −116) and the other 496 test names are byte-identical before
and after. Zero tests appeared.
Every one of the 116 lost tests was matched back to the deleted symbol it
exercised, by re-reading each test body from the base file — 0 unclassified:
truncate_at_tool_tags/closing_tag_for/TOOL_TAG_PATTERNSReasoning::*engine methods (respond / select_tools / plan / evaluate / build_system_prompt) incl. theTruncatingLlmmockrecover_tool_calls_from_content/recover_codex_text_tool_calls_from_contentllm_signals_tool_intent/user_signals_execution_intent/strip_code_blocks/strip_quoted_stringsextract_json/merge_system_messagesNo test was edited. Extracting the surviving test module from base and from HEAD
and diffing them yields zero added lines — every surviving test is byte-identical,
so nothing was reshaped to stay green.
No surviving production line changed. The same extraction over the pre-
#[cfg(test)]region yields exactly 8 added lines: 6 lines of new module doc (the old one
described the deleted planner) and 2 narrowed imports
(
use std::sync::{Arc, LazyLock}→LazyLock; the 8-namecrate::{…}block →crate::ToolCall). Everything else is deletion.LOC
crates/ironclaw_llm/src/reasoning.rscrates/ironclaw_llm/src/lib.rs(re-export block)Net −3,211 lines. (
git diff --statreports 912 insertions / 4,123 deletions —that is a diff re-anchoring artifact: removing a ~1,350-line block in the middle makes
git re-emit the unchanged tail as added. The extraction diffs above are the real
measurement.)
Ratchets
Nothing raised; nothing needed shrinking.
scripts/ci/check-composition-budget.sh, the one gate thatre-measures the tree): deleting production LOC shrinks the workspace denominator and
therefore raises composition's share — 646 bp → 650 bp, against a ceiling of
2398 + 30 tolerance. Passes with ~1,778 bp of slack.
reasoning.rscontained none of the terms pinnedfor
crates/ironclaw_llm/src/(google,github,private.near.ai,accounts.google.com,oauth2.googleapis.com), so this deletion cannot move thatratchet or stale an entry.
ceilings, not re-measured from the tree — unaffected.
The checklist's placement question
The WS8 row asks the scoped PR to "decide where those three live helpers belong."
Answer: deferred to the Wave 3 runner shed; the helpers are untouched. They stay in
ironclaw_llm::reasoningexactly where they are — no move, no rename, not even are-export change beyond dropping the dead names. Placement is a Wave 3 question
(the model-gateway shed) and answering it here would mean shipping a cross-crate move
inside a deletion PR, which is precisely the coupling that made #6943 defer this module
in the first place. A deletion PR whose diff is provably deletion-only is reviewable;
one that also relocates a live provider-quirk path is not.
Doc hygiene carried by this PR
Two docs cited symbols this PR deletes, so they are corrected here rather than left dangling:
crates/ironclaw_llm/CLAUDE.md— the crate's own spec (root CLAUDE.md Module Specs table). Itsreasoning.rsrow and "reasoning.rs Contents" section described the deleted planner, and claimedSILENT_REPLY_TOKEN/is_silent_reply()were "used by the dispatcher" —ironclaw_dispatcherwas itself deleted in the WS0 dead-crates PR. Rewritten to describe the three surviving functions.crates/ironclaw_host_api/src/channel.rs— a doc comment onChannelPresentationread "(CommunicationPresentationPolicyderives from this)". Prose only, no code dependency; reworded so it no longer names a deleted type.Left alone deliberately:
docs/reborn/extension-runtime/{checklist,implementation}.mdalso mentionCommunicationPresentationPolicy(OUT-11). Those are another workstream's historical/completed records — worth a follow-up from that workstream's owner, but not mine to rewrite inside a deletion PR. Worth flagging on its own: OUT-11 landed against the dead engine.CommunicationPresentationPolicyexisted only as a field of theReasoningstruct and its builder; nothing ever read it on a live path.Coverage-floor recapture (
7ca468d62)The coverage ratchet flagged two crates. Both are the legitimate-shrinkage case
that
tests/integration/coverage-floor.toml's own header prescribes ("a legitimatecode+test deletion lowering covered lines … updates captured_total_lines and
floor_percent/floor_covered_lines for the affected entry in the SAME PR"). Numbers are
copied verbatim from this PR's coverage-report run. No tests were added to chase the
old floors — that would defeat the point of a deletion PR.
ironclaw_llmironclaw_eventsironclaw_llmis mine, and legitimately so. The deletion removed 1,871 instrumentedlines (a material −6.63% denominator move). The dead half was more densely tested than
the crate average — 116 of the crate's 1,000 tests existed to cover it — so removing the
code and its tests together lowers the crate percentage even though no live path lost
any coverage. The un-masking evidence above is the check that matters:
ironclaw_runner,which owns the only live consumers, is 467 → 467 with an identical roster.
ironclaw_eventsis not mine. This PR touches zero files in that crate. Its floor wascaptured before #6943 deleted
events::{parse_jsonl, replay_jsonl}, so main has beensitting just under its own covered-lines floor since; this branch is simply the first the
ratchet caught. Independent confirmation: PR #6958 (progressive tool disclosure — no
ironclaw_eventsfiles) fails with the byte-identical block:Recaptured here so this PR lands green; flagging it so the reviewer reads that half as
inherited, not as a regression introduced by the deletion.
[global]is untouched (85.11%, still enforcing).Merge order — #6966 first, then this PR
This PR's remaining red is not a defect in the deletion. The coverage job now
clears every ratchet (all 16 PASS, including the two recaptured above) and fails one
step later, in
Gate changed Reborn lines and branches, which requires 100% coverageof changed production lines.
That gate built its denominator with git's default diff algorithm (myers), which
re-anchors deletion-shaped diffs and reports surviving, unchanged text as added.
On this PR it saw 478 changed lines / 208 branch arms; measured with the histogram
algorithm the real number is 14 lines across 3 files, all doc comments and imports,
zero executable. The 478 was entirely artifact.
Fixed in #6966 (one line + regression test), kept as its own reviewed PR rather
than folded in here, per the precedent that gate-behavior changes are never smuggled
into the PR they unblock.
So: merge #6966 first, then re-run this PR's failed jobs. This branch needs no
further change — once #6966 lands, these jobs re-run against the fresh merge ref, the
gate sees zero instrumentable changed lines ("Changed coverage: no Reborn production
lines added"), and this goes green with zero churn.
Verification gauntlet
cargo fmt --all --check— cleancargo clippy --all --benches --tests --examples --all-features— zero warnings, exit 0cargo test --workspace— delegated to this PR's CI, which is the arbiter for workspace-wide state (standing policy: full local workspace suites are reserved for debugging red CI)cargo test -p ironclaw_architecture— 94 passed, 0 failed, exit 0 (run locally in full)cargo test -p ironclaw_llm(unfiltered, before/after) — 1000 → 884cargo test -p ironclaw_runner(unfiltered, before/after) — 467 → 467, identical rosterNo
.unwrap()/.expect()introduced (none added at all — this PR only removes code).No imports touched outside
ironclaw_llm.🤖 Generated with Claude Code