Conversation
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>
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-7128 environment in ironclaw-ci-preview
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7128
August 4, 2026 09:49
Destroyed
github-actions
Bot
added
scope: docs
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
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 |
Contributor
🔎 Review · PR #7128
Submitted review →The complete trusted base-to-head comparison was reviewed. The module charter covers all 48 Rust source files exactly once, its enforcement test is logically sound, and the documentation amendments match the live code. No concrete actionable issues were found. Automatic · PR opened · attempt 1 of 3 · completed in 1m 13s Run details
|
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🔍 Review complete · PR #7128
✅ No actionable findings
The complete trusted base-to-head comparison was reviewed. The module charter covers all 48 Rust source files exactly once, its enforcement test is logically sound, and the documentation amendments match the live code. No concrete actionable issues were found.
Validation and technical details
- Inspected all five changed files and their complete diff between refs/ironloop/base and refs/ironloop/head.
- Confirmed crates/ironclaw_llm/src contains exactly 48 Rust source files and every file is represented once in the charter table.
- Confirmed reasoning.rs is 1,299 lines and the three re-exported helpers have the five cited production call sites in ironclaw_loop_host/src/model_gateway.rs.
- Confirmed 21 providers.json include_str! references and that reborn_cross_crate_include_scan.rs remains report-only.
- git diff --check completed successfully.
- cargo test -p ironclaw_llm --test module_charter could not run because cargo is unavailable in the review environment; the test and parser were inspected directly.
- Base:
main - Head:
ws6/module-chartersat5853d7c - Run:
c06f3c4f-51b7-4d37-bc2f-743688d8bfc2
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…
Collaborator
Author
|
Superseded: this slice's content is on main — the llm sub-owner charter map is on main (crates/ironclaw_llm/CLAUDE.md). It merged via the Wave 3–4 trains and batches (#7139/#7141 era and the Waves 0–4 batches #7170/#7181); the branch predates those squashes so ancestry can't show it, but the on-main artifact does. Closing as merged-via-train, not abandoned. |
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.
What
PROPOSAL §6.4.13 asks
ironclaw_llmfor "internal module charters for its five sub-owners (providers/auth-sessions/registry/decorators/recording)". This adds the map tocrates/ironclaw_llm/CLAUDE.md— and, because a charter nobody checks rots within a release, a test that pins it.This is one of the four items on CHECKLIST WS6's "Module charters" bullet.
Five sub-owners were not enough — measured, not guessed
Against the tree, the five named in §6.4.13 own 28 of 48 files (79.6% of lines). The other 20 fit none of them, including
lib.rs,provider.rs,error.rsandconfig.rs. Five more are named, each with a reason rather than being a residual bucket:core-contractproviderswould make providers the owner ofdecorators' andrecording's own dependencies — both wrapdyn LlmProvider.normalizationtool_schemahas five provider consumers.model-catalogregistryowns the catalog of providers. Different noun, different lifecycle, zero code overlap.transcriptionTranscriptionProvideris a different trait; nothing there implementsLlmProvider. It reuses OpenAI-compatible transports, nothing more.test-supporttesting/is a published cargo feature consumed by downstream harnesses — public API with a compatibility obligation, not a decorator.Rejected alternative: folding the 20 into the nearest of the five. That produces buckets whose stated charter does not describe their contents, which is the exact failure a charter exists to prevent.
The map is enforced, and proven so
crates/ironclaw_llm/tests/module_charter.rsasserts everysrc/**/*.rsappears in exactly one row, every path in a row exists, and no file is claimed twice. Sabotage-proved in all three directions:retry.rsfrom the decorators row1 source file(s) have no sub-owner … retry.rsdeleted_yesterday.rs1 sub-owner entr(y/ies) name a file that no longer existsregistry.rsunder two ownersregistry.rs -> registry, model-catalogEach restored green. The test also guards itself: it fails if the table parses to zero rows, and if the source walk finds implausibly few files — so a table-shape change cannot silently turn it into a no-op.
⚠ Two §6.4.13 claims are refuted
reasoning.rsis not dead and must not be deleted. §6.4.13 says "Deletes:reasoning.rs(4.5k lines, zero external references —SUPERSEDEDv1 engine remnant)". Measured onorigin/main:67088a426f) already deleted the dead half.lib.rs:88-91re-exportsclean_response,contains_codex_text_tool_call_syntaxandrecover_codex_text_tool_calls_from_tool_names, called at five sites incrates/ironclaw_loop_host/src/model_gateway.rs(:1617,:1619,:1634,:1807,:2156).crates/ironclaw_runner/src/model_gateway.rsno longer exists.It is charted under
normalization.AGENTS.mdcarried the same staleness ("legacy reasoning engine") and is corrected; it now also points at the map as authoritative, so its informal buckets cannot quietly become a second source of truth.Placement calls recorded (delegated authority)
Four files whose shape suggests one owner and whose purpose is another, stated rather than left implicit:
token_refreshing.rs→auth-sessions, notdecorators. A decorator by construction, but its whole body is OAuth refresh.CLAUDE.mdcalled it a decorator andAGENTS.mdfiled it under auth — they disagreed, and now do not.runtime.rsandsmart_routing.rsstaydecorators, which forces that owner's definition to widen from "reliability wrapper" to "wrapsdyn LlmProviderand is not credential work" — otherwise both fall out of the taxonomy.url_check.rs→core-contract, on the strength of the sharedbuild_http_client.gemini_oauth.rsis genuinely two owners in one file (CredentialManageris auth-sessions,GeminiOauthProvideris providers). A file-granular map must pick one; charged to the larger half with the split recorded as owed work rather than papered over.Not in this PR
providers.json(the row above) is blocked, not skipped. Of its 21include_str!sites the load-bearing one iscrates/ironclaw_reborn_cli/src/commands/config/init.rs:311, which reaches five levels up because the CLI may not depend onironclaw_llm— so it needs a new mechanism, not a new path. Plus twoDockerfileCOPY lines and two CI scripts. Worth knowing: its §11.2 gate is stillREPORT_ONLYinreborn_cross_crate_include_scan.rs.mcpis in an occupied crate;auth's two-engine split is a code change, not a map;webui handlers.rsis measured but not built — §6.9.4 contains no charter-map clause at all (the definition has to be read off §6.9.1 and §6.4.15's "module-charter work, not a split"), and itsarch-exempt: large_filewaiver must stay, since unlike the one deleted fromcontribution.rsit names a live pending plan (feat(reborn): route caller-requested model on OpenAI-compatible API (Phase 2) #5985).Note for merge order
Stacked conceptually on #7124. Both amend
PROPOSAL.mdat adjacent entries (§6.4.13 here, §6.4.14 there), so this branch will need a one-hunkgit merge origin/mainonce #7124 lands.CHECKLIST.mdauto-merges cleanly — verified by a trial merge.🤖 Generated with Claude Code