refactor(traces): split the 17,470-line contribution.rs into chartered modules (WS6) by BenKurrek · Pull Request #7124 · nearai/ironclaw

BenKurrek · 2026-08-04T09:07:45Z

What

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.

It becomes a directory module: 13 production submodules + a mirrored test tree, each named for one owner in the pipeline (capture → redact → classify → score → queue → submit). Largest file is now 1,290 lines.

src/contribution/mod.rs carries the charter table saying which module a new item belongs to, plus the two rules that keep it honest:

Redaction splits by key, not by stage — privacy matches patterns over arbitrary text; tool_payloads matches tool-and-field names. A new rule belongs to whichever input it keys off.
queue owns state, remote owns the wire, submission is the only module allowed to call both — which is what stops a transport change from silently becoming a queue-semantics change.

No API change, 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 consumers (ironclaw_product, ironclaw_reborn_composition, ironclaw_host_runtime, ironclaw_reborn_cli). Zero files changed outside ironclaw_reborn_traces, one architecture test, and two docs. Items that newly cross a module line were widened to pub(crate) — never to pub.

The waiver is deleted, not carried forward

No replacement waiver was added. Every file clears the 1,500-line ARCH-SPRAWL threshold, which scripts/pre-commit-safety.sh enforces with exit 1 (not a warning). This is why remote.rs and the test module were each split a second time rather than exempted.

Evidence

Preservation was proved, not assumed:

Check	Result
Top-level item roster vs `origin/main`	501 before → 501 after, zero missing, zero extra
Unfiltered `cargo test --lib -- --list`	216 before → 216 after, leaf names identical
`cargo test -p ironclaw_reborn_traces`	216 lib + 2 integration pass
`cargo test -p ironclaw_architecture`	full suite green
`cargo clippy --benches --tests --examples --all-features`	clean on both touched crates

Test paths moved from contribution::tests::<name> to contribution::tests::<owner>::<name>; the leaf names are byte-identical and were diffed as such. 149 tests bucketed by owner, 28 shared fixtures into tests/support.rs.

A gate got narrower

reborn_extension_specificity.rs's PATH_TERM_COLLISIONS carried four whole-file vendor carve-outs (slack/telegram/gmail/github) for contribution.rs — permitting those names anywhere in 17,470 lines. They now resolve to tool_payloads.rs (the rule tables) and classification.rs (classify_tool_side_effect, slack only), so the gate now polices the other eleven production files.

Sabotage-tested both directions:

Point a carve-out at a file lacking the term → red with "stale PATH_TERM_COLLISIONS carve-outs (nothing matches any more — delete them)".
Delete a live carve-out → red naming the exact (path, term) pair as a new violation.

Restored, green (7/7).

Docs and guidance

Adds the crate's first guidance file. CHECKLIST WS6 and PROPOSAL §6.4.14 are amended with dated entries quoting the text they replace, including two corrections the work surfaced:

§6.4.14's "17,467-line contribution.rs" measured 17,470 — the file drifted after the entry was written.
⚠ The CHECKLIST's shorthand "ScopedFilesystem … dropped" is worded backwards. ScopedFilesystem is ironclaw_filesystem's type, used by ~170 files workspace-wide, and is absent from this crate entirely — there is nothing to drop. §6.4.14's actual instruction is adoption ("take a ScopedFilesystem instead of raw dirs/env access"), a persistence-plane change across ~91 raw fs call sites. Recorded rather than quietly reinterpreted.

Deliberately not in this PR

ScopedFilesystem adoption — a behavioral change; mixing it into a file move would have destroyed the roster/test-name evidence that makes this reviewable.
Dropping the recording/paths re-export shims — all three call sites are in ironclaw_reborn_cli, and recording additionally needs a decision (the CLI has no ironclaw_llm dependency to fall back on).

🤖 Generated with Claude Code

Follow-up commit: a real gap the move surfaced (`96f4181172`)

CI's check-hermetic-env.sh flagged five raw std::env::set_var/remove_var sites in the moved test code. They are not new — all five are byte-identical to origin/main's contribution.rs at lines 10501/10513/10515/15648/15661. The gate is delta-scoped, so it had been grandfathering them; re-adding the file under a new path is what made them visible.

The finding was correct, so it is fixed rather than annotated. EnvVarRestore restored the previous value on drop but held no lock, so two tests mutating the environment on different threads still raced — UB on Rust 1.82+ regardless of variable names. Both sites now take ironclaw_common::env_helpers::lock_env(), the helper the gate's own message names; EnvVarRestore holds it as a field declared last, so the restore happens inside the critical section rather than after it.

Annotating with // env-hermetic: was rejected: that annotation asserts a genuinely single-threaded case, which is false here.

The gate's blind spot is filed separately as #7125 — pre-existing unguarded sites are invisible until someone happens to move them, and a pure move inherits the backlog of whatever it touches. Suggested fix there is a whole-tree shrink-only baseline, matching the pattern this repo already uses for LAYER_MATRIX_EXCEPTIONS and PATH_TERM_COLLISIONS.

Review triage (two CodeRabbit passes, 10 threads, all resolved)

Fixed — six were defects this PR introduced or carried:

Finding	Resolution
A second file-size waiver rode along into `queue.rs` ("already-oversized module", issue `#4088`) — contradicting this PR's own claim	Deleted; coupling rationale kept and reworded (`f788982cc7`)
`invite_code_gated_by_auth_mode` was inert — copied the production `match` and asserted against its own copy	Now calls the builder and asserts on the serialized request; sabotage-proved (`f788982cc7`)
Charter said "each stage owns one file"; `remote` is four	Reworded to module-level ownership in `mod.rs` + `CLAUDE.md`, with the `remote` mapping spelled out
Five policy-serde tests sat in `claims.rs`	Moved to a new `tests/policy.rs`, closing the one production owner with no test module
Three orphan section headers left by the split	Deleted (incl. one in `value.rs` not in the review)
`workload_token_env_mode_reads_env_unchanged` leaked its env var on panic	Uses `EnvVarRestore` RAII guard; drops both `unsafe` blocks and the clippy allow (`a04d4f3efa`)

Coverage gap closed: the workload bearer token had five unit tests on issuer_request_bearer and none proving it reaches the wire, while the caller attaches it conditionally. Added a caller-tier test with a mock issuer capturing the Authorization header; sabotage-proved by removing the attach.

Refuted-as-out-of-scope — three are real but pre-existing and need behavior changes, filed as #7127 rather than smuggled into a move PR: the case-sensitive remote status comparison that skips the local revocation record (a consent artifact), fetch_account_traces taking two adjacent &str where its sibling takes newtypes (its fix needs an edit in the occupied ironclaw_product), and the trace_scope_has_pending_queue doc/code mismatch (needs an intent decision, not a guess).

Final accounting

Item roster: 501 → 501, zero drift (re-verified after every commit).
Tests: 216 → 217. All 216 original leaf names still present, diffed against the origin/main baseline; the single addition is the new caller-tier bearer test.
Sabotage-proved this PR: the specificity carve-out (both directions), the de-inerted invite_code test, and the new bearer test.

`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>

… 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>

…on-split

gemini-code-assist · 2026-08-04T09:07:49Z

Caution

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

railway-app · 2026-08-04T09:07:54Z

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

Service	Status	Web	Updated (UTC)
ironclaw	✅ Success (View Logs)	Web	Aug 4, 2026 at 9:42 am

coderabbitai · 2026-08-04T09:08:18Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: dcedf5d8-a43b-456b-a1ee-bbb79a4c96f4

📥 Commits

Reviewing files that changed from the base of the PR and between a04d4f3 and b893e37.

📒 Files selected for processing (2)

crates/ironclaw_reborn_traces/src/contribution/tests/claims.rs
crates/ironclaw_reborn_traces/src/contribution/tests/policy.rs

💤 Files with no reviewable changes (1)

crates/ironclaw_reborn_traces/src/contribution/tests/policy.rs

📝 Walkthrough

Summary by CodeRabbit

New Features
- Added privacy-preserving trace capture, redaction, classification, scoring, queueing, submission, and credit tracking.
- Added secure account, community profile, authentication, and trace-status capabilities.
- Added configurable consent, upload, retention, eligibility, retry, and credit-notification controls.
- Added deterministic trace summaries for deduplication and embeddings.
Bug Fixes
- Strengthened sensitive-data protection, endpoint validation, credential handling, quarantine, and retry safety.
Documentation
- Updated architecture and implementation guidance.
Tests
- Expanded coverage across privacy, security, queueing, submissions, profiles, credits, and account operations.

Walkthrough

The PR splits the trace contribution implementation into focused modules. It adds schemas, capture, redaction, classification, queueing, remote operations, submission, notices, tests, and architecture guidance.

Changes

Trace contribution pipeline

Layer / File(s)	Summary
Contracts, capture, privacy, and value `crates/ironclaw_reborn_traces/src/contribution/{envelope,capture,classification,privacy,canonical,credit,tool_payloads}.rs`	Adds contribution schemas, capture conversion, consent and eligibility rules, redaction, canonical representations, value scoring, and credit reports.
Policy, queue, and maintenance `crates/ironclaw_reborn_traces/src/contribution/{policy,queue,maintenance}.rs`	Adds policy validation, enrollment resolution, pseudonymous scope handling, durable queue storage, retry holds, compaction, quarantine, diagnostics, and telemetry.
Remote credentials and transport `crates/ironclaw_reborn_traces/src/contribution/remote/*`	Adds upload claims, credential providers, pinned HTTP transport, account operations, and community-profile operations.
Submission and notices `crates/ironclaw_reborn_traces/src/contribution/{submission,notice}.rs`	Adds queue flushing, status synchronization, revocation, credit aggregation, notice scheduling, retries, and outbox persistence.
Validation and architecture guidance `crates/ironclaw_reborn_traces/src/contribution/tests/`, `crates/ironclaw_reborn_traces/CLAUDE.md`, `docs/reborn/`, `crates/ironclaw_architecture/tests/reborn_extension_specificity.rs`	Adds pipeline tests, module ownership guidance, updated contribution paths, and narrower architecture-test carve-outs.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

Three pre-existing defects in the trace contribution pipeline (surfaced by the #7124 split review) #7127 — The issue identifies defects in the same submission.rs and maintenance.rs areas, but this PR reorganizes those modules rather than changing the cited defects.

Possibly related PRs

nearai/ironclaw#4559 — Reorganizes the Trace Commons contribution logic into the modules updated here.
nearai/ironclaw#5280 — Introduces Trace Commons contribution functionality that this PR separates into capture, classification, privacy, queue, and remote modules.
nearai/ironclaw#5567 — Modifies the trace redaction APIs that this PR refactors into the contribution modules.

Suggested reviewers: think-in-universe

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description provides strong technical detail but does not follow the required template or explicitly complete several required sections.	Restructure the description with all template headings and complete Change Type, Linked Issue, Security Impact, Database Impact, Blast Radius, Rollback Plan, Review Follow-Through, and Review track.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title follows Conventional Commits style and accurately describes the contribution.rs module split.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

ironloopai · 2026-08-04T09:12:50Z

🔎 Review · PR #7124

⚫ Cancelled · Target changed

The target changed before this Run could finish.

_{Automatic · PR opened + CI failed · attempt 1 of 3 · cancelled after 1m 32s}

Run details

Repository: nearai/ironclaw
Base: main at 79435f4
Head: ws6/traces-contribution-split at 6af8fe4
Created: Aug 4, 2026, 9:12 AM UTC
Updated: Aug 4, 2026, 9:14 AM UTC
Run: 585f69af-5ce0-4808-92cb-d1ea33778642
Latest attempt: 1 · Cancelled · 2d948c56-a147-46fd-8897-0731b24b344c

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>

coderabbitai

Actionable comments posted: 8

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/ironclaw_reborn_traces/src/contribution/maintenance.rs`:
- Around line 803-821: Update trace_scope_has_pending_queue so an envelope file
counts as pending only when it is a .json entry without a corresponding
.held.json sidecar; continue excluding held files themselves and return false
when all remaining entries are held. Preserve the documented pending-queue
semantics rather than merely checking each filename’s suffix.

In `@crates/ironclaw_reborn_traces/src/contribution/mod.rs`:
- Around line 10-15: The contribution module documentation at
crates/ironclaw_reborn_traces/src/contribution/mod.rs:10-15 should describe
ownership at the module level, explicitly documenting the nested remote modules
instead of claiming each stage owns one file. Update the test guidance at
crates/ironclaw_reborn_traces/CLAUDE.md:36-38 to allow one or more test files
per owner or document the nested remote mapping; no other sites require changes.

In `@crates/ironclaw_reborn_traces/src/contribution/queue.rs`:
- Around line 626-635: Remove the stale file-size justification comment above
the trace credential resolution section in queue.rs, including references to the
oversized module and issue `#4088`. Retain only the coupling rationale if it
remains accurate for keeping credential resolution alongside the policy,
scope-directory, and credential-provider helpers.

In `@crates/ironclaw_reborn_traces/src/contribution/remote/account.rs`:
- Around line 361-366: Update the public account-trace fetch APIs, including
fetch_account_traces and fetch_account_traces_via_sink, to accept &TenantId and
&UserId in the same order as mint_account_login_link. Keep the typed identifiers
through the boundary and stringify only when invoking the
directory-parameterized internal cores, including resolve_trace_credentials_at
and trace_scope_key, so callers cannot transpose raw strings.

In `@crates/ironclaw_reborn_traces/src/contribution/submission.rs`:
- Around line 681-688: Update the status transition logic around update.status
to compare the external value case-insensitively, using eq_ignore_ascii_case or
a single to_ascii_lowercase normalization before the revoked, expired, and
purged checks. Ensure mixed-case server responses still set the corresponding
NodeTraceSubmissionStatus and preserve the existing revoked_at behavior.

In `@crates/ironclaw_reborn_traces/src/contribution/tests/claims.rs`:
- Line 1: Move the five StandingTraceContributionPolicy
tests—standing_policy_serde_back_compat_when_invite_code_missing,
standing_policy_serde_round_trips_invite_code_when_set,
standing_policy_serde_omits_invite_code_when_none,
legacy_policy_json_defaults_to_workload_token_env_auth, and
device_key_policy_round_trips—from claims.rs into the mirrored policy test
module for contribution/policy.rs, leaving claims.rs focused on its stated
claim-related coverage.
- Around line 407-445: Rewrite invite_code_gated_by_auth_mode to call the
production build_trace_upload_claim_issuer_request builder instead of
reproducing its auth-mode match locally. Supply the same production-relevant
inputs for both DeviceKey and WorkloadTokenEnv cases, then assert the returned
request has no invite_code for DeviceKey and forwards the trimmed configured
invite code for WorkloadTokenEnv.

In `@crates/ironclaw_reborn_traces/src/contribution/tests/credentials.rs`:
- Line 398: Remove the orphan section headers that describe tests moved to other
modules. In crates/ironclaw_reborn_traces/src/contribution/tests/credentials.rs
at line 398, delete the `// --- mint_account_login_link_via_sink tests ---`
header comment since those tests now live in tests/account.rs. In
crates/ironclaw_reborn_traces/src/contribution/tests/profile.rs at lines
416-421, delete the `// --- resolve_trace_credentials tests ---` header comment
and its associated isolation preamble since those tests now live in
tests/credentials.rs. These headers contradict the
one-test-module-per-production-owner layout established by the module split.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: bf4e7bc7-b6ee-42d1-9803-791b9ebff58f

📥 Commits

Reviewing files that changed from the base of the PR and between 79435f4 and 6af8fe4.

📒 Files selected for processing (38)

crates/ironclaw_architecture/tests/reborn_extension_specificity.rs
crates/ironclaw_reborn_traces/CLAUDE.md
crates/ironclaw_reborn_traces/src/contribution.rs
crates/ironclaw_reborn_traces/src/contribution/canonical.rs
crates/ironclaw_reborn_traces/src/contribution/capture.rs
crates/ironclaw_reborn_traces/src/contribution/classification.rs
crates/ironclaw_reborn_traces/src/contribution/credit.rs
crates/ironclaw_reborn_traces/src/contribution/envelope.rs
crates/ironclaw_reborn_traces/src/contribution/maintenance.rs
crates/ironclaw_reborn_traces/src/contribution/mod.rs
crates/ironclaw_reborn_traces/src/contribution/notice.rs
crates/ironclaw_reborn_traces/src/contribution/policy.rs
crates/ironclaw_reborn_traces/src/contribution/privacy.rs
crates/ironclaw_reborn_traces/src/contribution/queue.rs
crates/ironclaw_reborn_traces/src/contribution/remote/account.rs
crates/ironclaw_reborn_traces/src/contribution/remote/claim.rs
crates/ironclaw_reborn_traces/src/contribution/remote/client.rs
crates/ironclaw_reborn_traces/src/contribution/remote/mod.rs
crates/ironclaw_reborn_traces/src/contribution/remote/profile.rs
crates/ironclaw_reborn_traces/src/contribution/submission.rs
crates/ironclaw_reborn_traces/src/contribution/tests/account.rs
crates/ironclaw_reborn_traces/src/contribution/tests/claims.rs
crates/ironclaw_reborn_traces/src/contribution/tests/classification.rs
crates/ironclaw_reborn_traces/src/contribution/tests/credentials.rs
crates/ironclaw_reborn_traces/src/contribution/tests/flush.rs
crates/ironclaw_reborn_traces/src/contribution/tests/maintenance.rs
crates/ironclaw_reborn_traces/src/contribution/tests/mod.rs
crates/ironclaw_reborn_traces/src/contribution/tests/notice.rs
crates/ironclaw_reborn_traces/src/contribution/tests/privacy.rs
crates/ironclaw_reborn_traces/src/contribution/tests/profile.rs
crates/ironclaw_reborn_traces/src/contribution/tests/records.rs
crates/ironclaw_reborn_traces/src/contribution/tests/submission.rs
crates/ironclaw_reborn_traces/src/contribution/tests/support.rs
crates/ironclaw_reborn_traces/src/contribution/tests/value.rs
crates/ironclaw_reborn_traces/src/contribution/tool_payloads.rs
docs/reborn/extension-runtime/implementation.md
docs/reborn/target-architecture/CHECKLIST.md
docs/reborn/target-architecture/PROPOSAL.md

…arter 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>

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/ironclaw_reborn_traces/src/contribution/tests/claims.rs`:
- Around line 393-396: Replace the manual environment-variable setup and cleanup
in the test containing issuer_request_bearer with the RAII EnvVarRestore::set
helper from support.rs, while retaining the existing lock_env guard. Remove the
explicit cleanup at the end so EnvVarRestore restores the prior value during
normal completion and panic unwinding.
- Around line 384-388: Update workload_token_env_mode_reads_env_unchanged to
exercise the production issuer path via build_trace_upload_claim_issuer_request
or the issuer client, rather than calling issuer_request_bearer directly.
Configure the bearer token, invoke the real caller, and assert that the same
token is present on the outgoing request, preserving the existing
environment-lock setup.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c99e4590-5595-4f6b-94a1-3d403e2835a2

📥 Commits

Reviewing files that changed from the base of the PR and between 6af8fe4 and 96f4181.

📒 Files selected for processing (2)

crates/ironclaw_reborn_traces/src/contribution/tests/claims.rs
crates/ironclaw_reborn_traces/src/contribution/tests/support.rs

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/ironclaw_reborn_traces/src/contribution/tests/policy.rs`:
- Line 5: Update the module-level documentation comment in the policy test
module to describe only contribution/policy.rs serde compatibility and
device-key authentication modes; remove the references to upload-claim cache
keys and error labels, which are covered by tests/claims.rs.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b6881c25-7e66-402a-b0c9-033428045759

📥 Commits

Reviewing files that changed from the base of the PR and between 96f4181 and f788982.

📒 Files selected for processing (9)

crates/ironclaw_reborn_traces/CLAUDE.md
crates/ironclaw_reborn_traces/src/contribution/mod.rs
crates/ironclaw_reborn_traces/src/contribution/queue.rs
crates/ironclaw_reborn_traces/src/contribution/tests/claims.rs
crates/ironclaw_reborn_traces/src/contribution/tests/credentials.rs
crates/ironclaw_reborn_traces/src/contribution/tests/mod.rs
crates/ironclaw_reborn_traces/src/contribution/tests/policy.rs
crates/ironclaw_reborn_traces/src/contribution/tests/profile.rs
crates/ironclaw_reborn_traces/src/contribution/tests/value.rs

💤 Files with no reviewable changes (3)

crates/ironclaw_reborn_traces/src/contribution/tests/value.rs
crates/ironclaw_reborn_traces/src/contribution/tests/profile.rs
crates/ironclaw_reborn_traces/src/contribution/tests/credentials.rs

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>

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>

coderabbitai

Caution

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

⚠️ Outside diff range comments (1)

crates/ironclaw_reborn_traces/src/contribution/tests/claims.rs (1)
427-427: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Move the community-profile tests to the profile test module.

Line 427 starts public_attribution coverage in crates/ironclaw_reborn_traces/src/contribution/tests/claims.rs. This behavior belongs to crates/ironclaw_reborn_traces/src/contribution/remote/profile.rs; move the complete section to crates/ironclaw_reborn_traces/src/contribution/tests/profile.rs. Keep claims.rs focused on claim, cache-key, and issuer-auth coverage.

As per path instructions: “Place each contribution test in the test module corresponding to the production module owning the behavior under test.”

Based on learnings: contribution tests must mirror the production module they verify.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ironclaw_reborn_traces/src/contribution/tests/claims.rs` at line 427,
Move the complete community-profile/public_attribution test section beginning at
the “community profile” marker out of the claims tests and into the profile test
module. Preserve the tests and their behavior unchanged, and keep claims.rs
limited to claim, cache-key, and issuer-auth coverage while profile.rs contains
coverage for remote/profile.rs.
Sources: Path instructions, Learnings

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@crates/ironclaw_reborn_traces/src/contribution/tests/claims.rs`:
- Line 427: Move the complete community-profile/public_attribution test section
beginning at the “community profile” marker out of the claims tests and into the
profile test module. Preserve the tests and their behavior unchanged, and keep
claims.rs limited to claim, cache-key, and issuer-auth coverage while profile.rs
contains coverage for remote/profile.rs.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f795ea8f-0ea7-4997-b902-9597a97ee36e

📥 Commits

Reviewing files that changed from the base of the PR and between f788982 and a04d4f3.

📒 Files selected for processing (1)

crates/ironclaw_reborn_traces/src/contribution/tests/claims.rs

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.

Each of these is pre-existing and verbatim on `main`; #7124 only moved the file, so the move-only PR could not also change semantics. Working through them by consequence. **Privacy gate keyed on prose (finding 2).** Dataset eligibility was decided by scanning `warnings` for the substring `"quarantined"`, whose sole producer is one English sentence. Rewording, translating or localising it silently opened the gate — quietly, and in the permissive direction. `PrivacyMetadata` now carries a typed `quarantined` flag set by the producer beside that sentence, and the gate keys on it plus the typed `residual_pii_risk` (which covers envelopes persisted before the field existed, since `#[serde(default)]` gives them `false`). Prose is prose again: the sabotage test replaces the whole sentence with German and the gate still holds. **Sidecar security tests passing vacuously (finding 6).** The stderr suppression, environment scrubbing and oversized-stdout tests opened with `if !Path::new("/bin/sh").exists() { return; }` — success while asserting nothing. Now `#[cfg(unix)]`, so they do not exist on Windows rather than silently passing there (Windows runs `cargo check`, never this suite), and on unix a missing shell is a hard failure. Deliberately not the `IRONCLAW_REQUIRE_DOCKER_TESTS` shape: that flag is set nowhere in the repo, so the gate it guards is itself inert. **Redaction sidecar deadlock (finding 7).** The parent wrote up to 1 MiB into stdin with nothing draining stdout, and the timeout covered only `wait_with_output` — so a sidecar emitting more than one pipe buffer before reading wedged both ends under no timeout at all, leaking a live child per turn. Write and drain now run concurrently, both under the timeout. Every pre-existing sidecar test starts `cat >/dev/null` with 5 bytes of input, i.e. the one ordering that cannot deadlock; the new test inverts both. **Fabricated server receipt (finding 5).** A 2xx whose body did not parse was turned into `status: "submitted"` with a *locally estimated* credit, recorded as Submitted, and the queued envelope deleted — destroying the only retryable copy. Every receipt field has a serde default, so `{}` already parses; reaching that branch means the body was not JSON at all. It is now an error. **Compaction deleting a held envelope (finding 4).** The fail-loud hold read was swallowed by `.ok().flatten()`, so an unreadable sidecar ranked a held envelope as unheld and compaction deleted it — a consent artifact, lost silently, while every other IO failure in that function propagates. Now propagated with context. The existing telemetry test already built this exact fixture and asserted a *downstream* symptom; it now asserts the earlier, accurate failure. **Durable identifiers derived from `Debug` (finding 8).** `vector_key` addresses rows in a vector store; the credit fingerprint is persisted in `submissions.json` and compared on every load to keep an acknowledged notice suppressed. Both are now built from explicit `as_str`-style methods frozen at the values `Debug` produced, so nothing already persisted moves and a rename has to come to the `match`. Measured and *not* changed: adding `#[serde(rename_all = "snake_case")]` to `TraceCreditEventKind` for consistency with its 22 siblings would make every existing `submissions.json` fail to deserialize — that file has no schema version and no migration. The inconsistency is load-bearing and now says so. **Unbounded process-global maps (finding 9).** `TRACE_UPLOAD_CLAIM_CACHE` held one live-or-stale *bearer token* per user subject forever (expiry was filtered on read, never evicted); it now sweeps expired entries on write behind a `CREDIT_VIEW_CACHE_MAX_SCOPES`-shaped cap. `TRACE_SCOPE_MUTATION_LOCKS` sweeps entries with `Arc::strong_count == 1` — explicitly *not* the wholesale `clear()` that bounds the credit cache, because these `Arc`s are the mutual-exclusion identity and evicting a held one would hand the next caller a fresh uncontended mutex. **Smaller (finding 10).** `redaction_hash` no longer hashes zero bytes on a serialization failure, which gave every failing trace the same digest for dedupe and integrity — it is fallible now, and `rescrub_trace_envelope` carries it. `novelty_score` is clamped at both ends like its sibling. The trace card derives its retention policy from `allowed_uses` through the same ranking `retention_policy_for_trace` uses, instead of hardcoding `private_corpus_revocable` — they disagreed for three of five consent scopes, and the card is what crosses the wire. A malformed `tool_calls` payload still yields no calls but is no longer silent. "standaloneice key" corrected in both doc comments. Every fix above is sabotage-tested: the change is reverted, the new test observed failing with the right message, then restored and re-run green. Refs #7144 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…e merge queue The merge-queue run (Tests (Reborn) 30917327135) failed the changed-line coverage gate at 80.67% vs the 90% floor. 1,284 of the 1,311 uncovered changed lines are the #7124 contribution.rs split re-attributed as new code — the same queue run PASSED ironclaw_reborn_traces' per-crate covered-line floor, which is direct proof the split lost no coverage (the #6963 gate-vs-restructure collision class, same as the WS1.1 precedent entry). Exact-line exemptions per the manifest's policy; the 27 genuinely-new uncovered lines in other slices are deliberately NOT exempted (post-exemption aggregate ≈99.5%). Also merges main @ d06f804 (clean). 113/113 changed-coverage self-tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…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…

BenKurrek · 2026-08-05T11:37:20Z

Superseded: this slice's content is on main — the contribution/ split is on main (crates/ironclaw_reborn_traces/src/contribution/, landed via the #7139 consolidation). 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.

…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…

BenKurrek and others added 3 commits August 4, 2026 05:04

Merge remote-tracking branch 'origin/main' into ws6/traces-contributi…

6af8fe4

…on-split

railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-7124 August 4, 2026 09:07 Destroyed

github-actions Bot added scope: docs Documentation size: XL 500+ changed lines risk: low Changes to docs, tests, or low-risk modules contributor: core 20+ merged PRs labels Aug 4, 2026

railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-7124 August 4, 2026 09:14 Destroyed

BenKurrek mentioned this pull request Aug 4, 2026

check-hermetic-env.sh grandfathers pre-existing unguarded env mutation; a pure file move is what re-surfaces it #7125

Open

coderabbitai Bot reviewed Aug 4, 2026

View reviewed changes

BenKurrek mentioned this pull request Aug 4, 2026

Three pre-existing defects in the trace contribution pipeline (surfaced by the #7124 split review) #7127

Open

railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-7124 August 4, 2026 09:25 Destroyed

coderabbitai Bot reviewed Aug 4, 2026

View reviewed changes

Comment thread crates/ironclaw_reborn_traces/src/contribution/tests/claims.rs Outdated

Comment thread crates/ironclaw_reborn_traces/src/contribution/tests/claims.rs Outdated

coderabbitai Bot reviewed Aug 4, 2026

View reviewed changes

Comment thread crates/ironclaw_reborn_traces/src/contribution/tests/policy.rs Outdated

railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-7124 August 4, 2026 09:32 Destroyed

railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-7124 August 4, 2026 09:35 Destroyed

coderabbitai Bot reviewed Aug 4, 2026

View reviewed changes

This was referenced Aug 4, 2026

refactor(ws6): execute the 13 WS6 renames and close the remaining Wave 4 rows #7152

Merged

WS6: the rows still open after Wave 4 (measurements for the next slice) #7153

Open

BenKurrek mentioned this pull request Aug 4, 2026

WS6: evict the service cluster from composition (admin users, trace capture, route mounts, project reader, vendor stores) #7186

Closed

BenKurrek closed this Aug 5, 2026

coderabbitai Bot mentioned this pull request Aug 6, 2026

Program closure: the defect train, the await-edge ruling, and the WS12 100% gate #7263

Merged

Conversation

BenKurrek commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

No API change, no consumer edits

The waiver is deleted, not carried forward

Evidence

A gate got narrower

Docs and guidance

Deliberately not in this PR

Follow-up commit: a real gap the move surfaced (96f4181172)

Review triage (two CodeRabbit passes, 10 threads, all resolved)

Final accounting

Uh oh!

gemini-code-assist Bot commented Aug 4, 2026

Uh oh!

railway-app Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

ironloopai Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔎 Review · PR #7124

⚫ Cancelled · Target changed

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

BenKurrek commented Aug 5, 2026

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

BenKurrek commented Aug 4, 2026 •

edited

Loading

Follow-up commit: a real gap the move surfaced (`96f4181172`)

railway-app Bot commented Aug 4, 2026 •

edited

Loading

coderabbitai Bot commented Aug 4, 2026 •

edited

Loading

ironloopai Bot commented Aug 4, 2026 •

edited

Loading