refactor(conversations,attachments): fix the conversations/threads naming trap and widen attachments (WS5)

[BenKurrek]

Stacked on #7002 → #6998 → main; merges after them. Base is ws2/webui-openai-ports.

Wave 2, slot 6 — CHECKLIST WS5, the conversations/threads naming-trap row and the attachments widened row. Vocabulary renames plus one crate widening; behavior-free apart from one recorded, self-healing format change (finding 4).

The naming trap is the one the audit called the worst in the tree, and it was costing real money: exactly one file had to hold both vocabularies at once — composition's trusted-trigger submit — and it carried four as Canonical… aliases to say which accept_inbound_message it meant, plus a fifth in extension_host. All five are gone; nothing aliases these names anywhere in the workspace now.

---

1. The rename table

Every name renamed on the conversations side. ironclaw_threads — the canonical transcript, and the stronger claim to thread/message vocabulary — is untouched.

Old (ironclaw_conversations)	New	Why	Consumers repointed
SessionThreadService	InboundConversationService	collided with ironclaw_threads::SessionThreadService, same method name and shape (accept_inbound_message(Request) -> Accepted), different store	composition::trigger_poller_assembly (the one caller)
AcceptInboundMessageRequest	AcceptConversationMessageRequest	collided	none outside the crate
AcceptedInboundMessage	AcceptedConversationMessage	collided	composition::automation::trigger_poller_trusted_submit
AcceptedInboundMessageReplay	AcceptedConversationMessageReplay	collided	none outside the crate
ThreadMessageRecord	ConversationMessageRecord	collided, and no row named it — finding 1	none outside the crate
AcceptedInboundMessageLookup	AcceptedConversationMessageLookup	never collided; renamed for family coherence	none outside the crate

The consumer blast radius is two files. Measured, not assumed: a scan of the 72 files in crates/, tests/, tools/ that name any of the six found that 70 of them mean ironclaw_threads — which is exactly why the collision was dangerous and why it stayed invisible.

Alongside, the external-ref pair unified onto its one home:

Was	Now	Deleted
ironclaw_conversations::{ExternalActorRef, ExternalConversationRef} (own definitions)	consumed from ironclaw_extension_contracts::external	both definitions; not re-exported, so conversations never becomes a second import path
ironclaw_product::conversation_binding::{conversation_actor_ref, conversation_conversation_ref}	the value passes through	both field-by-field translators
ironclaw_product::workflow::delivered_route_conversation_ref	envelope.external_conversation_ref().conversation_fingerprint()	the rebuild and its error branch — the fingerprint already excludes the reply target
ExternalConversationRef::without_message_id (conversations)	ExternalConversationRef::without_reply_target (contracts)	one method, one home

14 files repointed to ironclaw_extension_contracts::external.

2. Attachments widening

ironclaw_attachments gained two modules and now declares:

Moved in	From	Note
InboundAttachmentLander, InboundAttachmentReader, AttachmentCleanupReport	ironclaw_product::reborn_services	§6.4.9's "the landing routine plus its ports"
ProjectScopedAttachmentLander (+ its 6 tests, bodies unedited)	ironclaw_product::scoped_fs::attachment_landing	the default impl over ScopedFilesystem
AttachmentCapabilities / attachment_capabilities() (was ProductAttachmentCapabilities / product_attachment_capabilities)	ironclaw_product::product_surface_inbound	"one home for size ceilings (webui/openai import them)"

ProjectScopedAttachmentReader stayed in product — finding 6.

---

3. Row corrections (re-verified against this PR's base ba009786d)

1. The "same-named DTO trio" is a quartet

The row names SessionThreadService "+ its same-named DTO trio" — four names. Comparing the two crates' pub use sets found five. The fifth is ThreadMessageRecord (crates/ironclaw_conversations/src/types.rs:243 against crates/ironclaw_threads/src/contract.rs), and nothing in CHECKLIST or PROPOSAL names it. It is renamed with the others, and the new gate discovers collisions rather than enumerating them, so the next one fails at introduction instead of at the next audit.

2. "unify … with host_api" is stale by one wave

host_api does not declare the pair any more: WS1.4 moved external.rs into ironclaw_extension_contracts (crates/ironclaw_extension_contracts/src/external.rs:69,144; the location scan's own module doc records the arrival at reborn_extension_contract_location_scan.rs:107-110). The unification landed on that copy. ironclaw_conversations gained the dep — substrates → contracts, a downward edge, no layer exception (LAYER_MATRIX_EXCEPTIONS stays 13).

3. The duplicate was field-divergent, and the divergence that mattered was equality

Not two copies of one shape. Conversations' copy spelled the last two fields thread_id / message_id where the canonical type says topic_id / reply_target_message_id, and carried no display_name. But the live difference was PartialEq / Hash: conversations' were derived, so they included the per-event message id; the canonical type excludes it deliberately ("the reply-target hint is deliberately excluded"). The same conversation route therefore compared equal or unequal depending on which copy the caller happened to hold, and ironclaw_product's two hand-written translators were the only bridge. That is the concrete argument for this row: a duplicated type is not just duplication, it is two answers to ==.

4. Delivered gate-route fingerprints change format — recorded, not mitigated

The two copies also had different conversation_fingerprint() algorithms (length-prefixed "{len}:{part}|" versus "space:{len}:{v};…"). Unifying picks the canonical one, so DeliveredGateRouteRecord.delivered_conversation_fingerprints is written and read in a new spelling.

Both sides traced: the writer (product::run_delivery::gate_routes) and the reader (product::workflow) both used the conversations copy and both flip together, so there is no read/write skew within a build. The exposure is across an upgrade and is bounded by DELIVERED_GATE_ROUTE_TTL = 48 hours (crates/ironclaw_outbound/src/delivered_gate_routes.rs:43): a gate prompt delivered before the upgrade and answered after it misses the delivered-route fallback, and the index self-heals inside the TTL. Dual-writing both formats is exactly the compatibility shim the type-placement rule forbids, so this is recorded rather than papered over.

5. …but the durable state that is not self-healing is preserved, and it would have failed silently

BindingRecord and ReplyTargetRecord embed the ref permanently, and serde fills a missing Option field with None without an error. Renaming thread_id → topic_id on the wire would therefore have loaded every existing threaded route as a conversation-root route — replies landing in the channel instead of the thread — with no log line anywhere.

ironclaw_conversations::stored_refs keeps that grammar, in the crate that owns the records, not in the contracts crate that owns the type.

✎ Corrected 2026-08-02 on review (@serrrfirat, stored_refs.rs:53; CodeRabbit, :38) — commit 4cd166f727. This finding originally shipped the compatibility one-way: write the canonical spelling, read either, with a # Rollback boundary section calling that deliberate. It was wrong, and the argument recorded for it refuted a proposal nobody made — it rejected dual-writing (emitting both spellings, which #[serde(alias)] makes self-defeating) when the fix is write-legacy / read-either, which that objection does not touch. Measured against origin/main, the released readers name thread_id/message_id with no aliases, so a record this build wrote read back as None on a rollback.

The severity was understated too. ExternalConversationIdentity keys BindingKey, and StoredConversationState::into_state rebuilds that map with Vec<(K, V)>::into_iter().collect() — so two threaded bindings in one conversation that both lose their topic become the same key and the earlier one is dropped. Binding loss, not a remap.

Now: conversation_ref::serialize writes {space_id, conversation_id, thread_id, message_id} through a borrowed representation, ExternalConversationIdentity has a matching hand-written Serialize, both readers keep their aliases, and a record carrying both spellings still fails closed. stored_refs::actor_ref was deleted as a genuine no-op (the actor change was additive, so the canonical impls already did the job).

The evidence moved tiers as well: filesystem_conversation_services_persist_external_refs_in_the_durable_grammar drives the real StoredConversationState through the services and walks every key of the persisted document. Under the mutation that makes the identity emit topic_id, the stored_refs surrogate tests stay green and only the store-level test fails — naming all six affected sites. That gap was @serrrfirat's separate memory.rs:1528 finding, and it was correct.

6. attachments — "their composition impls move in" is wrong twice

The impls were never in composition: both adapters lived in ironclaw_product::scoped_fs::attachment_landing, and composition only constructs them. And only one could move. ProjectScopedAttachmentReader cannot, for two independent reasons: it implements ironclaw_loop_host::LoopAttachmentReadPort as well as InboundAttachmentReader, so ironclaw_attachments (substrates) would need ironclaw_loop_host (loops) — an illegal upward edge — and even granted that, once the struct moved that impl would have neither side in ironclaw_product, so the orphan rule (#7002's finding) forbids leaving it behind too. The gate pins the reader in product and asserts ironclaw_attachments never acquires a loop_host dep, so "finishing the row" cannot quietly mean dragging the loop tier into a substrate. The row wants re-wording to exclude the read adapter, or the read port re-homed first.

7. [decision] — ironclaw_attachments now names ironclaw_product_contracts

The two ports keep erroring with ProductSurfaceError because the caller that fails is a product surface and the WebUI bytes endpoint maps NotFound/Forbidden straight onto 404/403. Layer-legal (contracts is the lowest layer, and this is the neutral product-tier contract crate, not ironclaw_product), but it does put product-tier error vocabulary in a substrate. The alternative — narrow the ports onto an attachments-owned error and map at the product boundary — moves an HTTP status mapping, which is behavior, not placement, and deserves its own slice the way WS2.2 gave ProductSurfaceFailure one. Recorded, not taken; carried on the CHECKLIST row.

---

4. New enforcement — crates/ironclaw_architecture/tests/reborn_conversations_threads_attachments.rs

Five tests, three rules:

1. conversations_and_threads_declare_no_name_in_common — discovered, not enumerated: every type/trait/enum either crate declares, compared against the other's. Also pins that the five historic collisions stayed in ironclaw_threads and left ironclaw_conversations, so a later "fix" that renames the transcript crate instead (≈70 files, and the weaker claim) fails loudly. Non-vacuity: both sets asserted > 20.
2. the_external_ref_pair_is_declared_only_by_the_extension_contracts_crate — the pair is declared by its owner and not by conversations, the two product translators are not re-declared, and conversations does not re-export the pair (the §11.2.4 re-export trap). The source-reading halves read stripped source, so a name surviving in prose does not count.
3. the_attachment_ports_and_size_ceilings_live_in_the_attachments_crate — six names declared in ironclaw_attachments and not in ironclaw_product, attachment_capabilities() present, product_attachment_capabilities() absent, and the carve-out pinned in both directions (reader in product; no loop_host dep in the manifest).
4. the_source_strip_removes_prose_and_literals_but_not_code — self-test over the four traps (doc comment, line comment, string literal, trailing comment).
5. the_declaration_walk_finds_declarations_not_mentions — self-test: a type conversations really declares is found; ThreadScope, which it only names, is not.

Mutation-checked: re-adding pub struct ThreadMessageRecord + pub struct ExternalActorRef to conversations and pub trait InboundAttachmentLander to product made tests 1, 2 and 3 fail with the intended messages (declare 1 name(s) in common: ["ThreadMessageRecord"], declares its own ExternalActorRef again, is declared in ironclaw_product again); restoring returned 5/5 green.

5. Enumerating gates touched (all update-never-relax)

Gate	Change
reborn_extension_contract_location_scan.rs COLLISION_EXEMPT	4 → 2, deleted not repointed. Its entries said the pair was "the same concept, declared twice … unifying them changes the conversations record grammar — a domain call". The call is taken, so the carve-out goes. Added in its place: the blind spot that remains — AdapterInstallationId/ExternalEventId are bounded_string_id! expansions no pub struct walk can resolve.
reborn_transport_product_boundary.rs	WebUI residue 102 → 100, two rows deleted. They named this row as their owner ("CHECKLIST WS5 attachments widened … one home for size ceilings owns the move"), so the gate fired the moment the move landed — working exactly as #7002 designed it.
reborn_extension_specificity.rs allowlist	untouched at 129. Its two reborn_services.rs entries still match after the ports left that file (verified by execution, not by reading).
reborn_struct_test_support_ratchet.rs	untouched. Its one attachment_landing.rs entry is ProjectScopedAttachmentReader::with_max_bytes, which stayed with the reader.
reborn_service_method_freeze_ratchet.rs	untouched — ProductSurface still 3 methods; reborn_services.rs shed a trait, it did not gain one.
LAYER_MATRIX_EXCEPTIONS	13, unchanged. Every new dep is downward (substrates → contracts, products → substrates).
docs/plans/composition-pubuse.snapshot	untouched — composition's public surface is unchanged.
CLI exact-dep allowlist	untouched — no CLI dependency changed.
reborn_cross_crate_include_scan.rs	untouched — all seven files code was moved out of were swept for include_str!/include_bytes!; none carried one.
scripts/no_panics_reborn_baseline.txt	untouched — the one baselined entry in a file this PR touches (conversation_state_store.rs, index_key_tenant_ids) is pre-existing and unmodified.

6. Un-masking

Full unfiltered --list inventory of all 18 crates this PR touches or could touch, --lib --tests, taken on a quiescent tree in this worktree at ba009786d and at this tip, compared name by name:

6702 → 6722. Six removed, twenty-six added, every one accounted for.

• The six absent names are the six ProjectScopedAttachmentLander tests, which reappear as project_scoped::tests::<the same name>: the lander moved crates, so its module path changed and the diff cannot render it as a rename. All six bodies verified byte-identical by extracting each function from the base file and the destination file and comparing (6/6).
• Twenty genuinely new, none written to make something pass:
  • 5 — the new architecture gate (§4).
  • 5 — conversations::stored_refs::tests: the durable record grammar. ✎ Reworked by the 2026-08-02 review correction above (4cd166f727), and grown to 7 plus 2 store-tier tests. A legacy conversation record keeps its topic and its reply target; the current record is written in the durable grammar and round-trips; a canonically spelled record still loads (the alias is not dead code); a record carrying both spellings is rejected; the actor pair proves no adapter is needed; all re-validate on load.
  • 4 — conversations::ids::tests: the route identity drops the reply target but keeps the topic (asserted both ways), a legacy thread_id binding key still resolves to the same route, deserialization re-validates a corrupted record, and the named error mapping keeps this crate's error vocabulary.
  • 4 — attachments::ports::tests: object safety for both Arc<dyn _> ports; per-method argument pass-through (scope + message id reach the impl, and rollback receives exactly the batch land returned); the cleanup snapshot; and a read whose answer differs by thread scope in both directions, against a double keyed on (scope, key) — a double that ignored the scope would have passed a one-directional check.
  • 2 — attachments::budgets::tests: the advertised ceiling is the enforced ceiling, the tokens come from the shared registry with no wildcards, and the budgets stay flattened on the wire (a nested budgets key would silently break every client reading max_file_bytes at the top level).
• No test was deleted, disabled, or weakened. Exactly one existing body was edited, disclosed here: conversations::inbound_contract::serde_deserialization_revalidates_external_ref_invariants now spells its two JSON fixtures in the canonical field names, because that is what the unified type deserializes — and it gains an assertion that the route identity still reads the pre-unification thread_id key. All other test-file churn is mechanical (ExternalActorRef::new(a, b) → (a, b, None::<String>), qualified paths becoming imports); the largest such file, product_surface_contract.rs, was classified line by line: 20 identical call rewrites plus import churn, zero argument or assertion changes.

7. Verification

• cargo fmt; cargo clippy -p ironclaw_conversations -p ironclaw_extension_contracts -p ironclaw_attachments -p ironclaw_product -p ironclaw_extension_host -p ironclaw_reborn_composition -p ironclaw_webui -p ironclaw_architecture --all-targets --all-features -- -D warnings → clean.
• cargo test -p ironclaw_architecture → 142 passed / 0 failed (137 at the fork point + this PR's 5).
• Per-touched-crate unfiltered --all-features tests → 2262 passed / 0 failed / 0 ignored across those eight crates, plus ironclaw_reborn_composition green on its own run.
• cargo check --workspace --all-targets --all-features → 0 errors, 0 warnings (run last).
• cargo metadata --locked clean; git status clean after every cargo run; the Cargo.lock delta is 8 lines (the four new path deps).
• All 10 exact selectors in scripts/reborn-e2e-rust.sh executed under bash with < /dev/null; each matched exactly one test (10/10, SELECTOR-FAIL=0).

Changed-coverage: this is a move-shaped diff, so relocated bodies read as added production lines. Entries will be derived from this PR's own merged lcov via the replay method once CI publishes it — not guessed here. Written defensively for it: the pure-declaration module that moved (attachments::ports) carries real contract tests rather than a waiver — object safety for both Arc<dyn _> ports, argument pass-through recorded per method, and a read whose answer differs by thread scope in both directions against a double that keys on (scope, key); and the error re-wraps this PR introduces are named functions (map_external_ref_error, external_ref), not inline closures.

🤖 Generated with Claude Code

[gemini-code-assist]

Caution

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

[coderabbitai]

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: 2f38e922-64e6-497d-80a8-0068ce857941

📥 Commits

Reviewing files that changed from the base of the PR and between 5b85634 and 5f1867a.

📒 Files selected for processing (1)

• crates/ironclaw_product/src/scoped_fs/attachment_reader.rs

---

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Added centralized attachment capabilities, size limits, scoped landing and reading, rollback, and stale cleanup.
  • WebUI attachment responses now include shared capability metadata.
  • Added compatibility for canonical and legacy conversation identity formats.

• Refactor

  • Unified conversation terminology and service contracts.
  • Centralized external references and attachment ownership.

• Documentation

  • Updated architecture and contract documentation.

• Tests

  • Expanded coverage for attachment safety, persistence, routing, compatibility, and architectural boundaries.

Walkthrough

Changes

The PR renames conversation contracts, centralizes external references, and adds backward-compatible identity persistence. It moves attachment ports, capabilities, and project-scoped landing into ironclaw_attachments, while retaining the product-side reader. Architecture tests and documentation enforce the new boundaries.

Conversation and attachment migration

Layer / File(s)	Summary
Conversation contracts and durable identity crates/ironclaw_conversations/*, crates/ironclaw_extension_contracts/src/external.rs	Conversation services and DTOs use conversation-specific names. Route identity uses topic_id. Durable storage accepts legacy field names.
Attachment contracts and filesystem flow crates/ironclaw_attachments/*, crates/ironclaw_product/src/scoped_fs/*	Attachment capabilities, ports, scoped landing, rollback, cleanup, and size ceilings are centralized. Product retains the loop-host reader.
Boundary wiring and validation crates/ironclaw_product/*, crates/ironclaw_extension_host/*, crates/ironclaw_reborn_composition/*, crates/ironclaw_webui/*, tests/integration/*	Consumers use canonical external-reference and attachment APIs. Tests cover routing, persistence, attachment reads, and composition wiring.
Architecture enforcement and records crates/ironclaw_architecture/tests/*, docs/reborn/target-architecture/*, crates/AGENTS.md, tests/integration/changed-coverage-exemptions.toml	Source scanners, dependency checks, ratchets, coverage policy, and architecture records encode the new ownership rules.

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

Possibly related issues

• Universal attachments across all channels: wire the legacy attachment pipeline into v1 + extensible format registry + polished web UX #4644 — Adds shared attachment ports, capability metadata, scoped landing, and filesystem support for the Reborn attachment pipeline.

Possibly related PRs

• nearai/ironclaw#6364 — Introduces the shared attachment budgets and durable attachment flow refined by this PR.
• nearai/ironclaw#5851 — Modifies the same conversation contracts, external references, and binding APIs.
• nearai/ironclaw#6977 — Enforces canonical ownership for shared extension-contract types.

Suggested reviewers: ilblackdragon, think-in-universe

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is detailed and relevant but omits most required template sections, including change type, linked issue, security, database, rollback, and review follow-through.	Add all required template headings and complete each field, using “None” or “N/A: ” where applicable.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title follows Conventional Commits style and accurately describes the conversation naming refactor and attachment ownership changes.
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

• X
• Mastodon
• Reddit
• LinkedIn

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

[railway-app]

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

Service	Status	Web	Updated (UTC)
ironclaw	✅ Success (View Logs)	Web	Aug 2, 2026 at 2:44 pm

[ironloopai]

🔎 Review · PR #7005

🔴 Failed

Execution result is invalid

The structured result could not be verified.

_{Automatic · PR opened · attempt 1 of 3 · failed after 2m 33s}

Failure details

• Repository: nearai/ironclaw
• Base: ws2/webui-openai-ports at 256163c
• Head: ws2/naming-traps-attachments at 2f78d5d
• Created: Aug 1, 2026, 8:18 AM UTC
• Updated: Aug 1, 2026, 8:21 AM UTC
• Run: 118c97e0-ecde-4630-b7e1-f3d9896ecfd3
• Latest attempt: 1 · Completed · 10dbe533-be1a-4ea0-8a5d-792ae770345b
• Failed during: Verification
• Retryable: No
• Failure: 4b3f5df8-3142-4089-a0a0-406a8dd3ad5b

[BenKurrek]

Coordinator sign-off (Wave 2 slot 6 — WS2 naming traps + attachments widening).

This slice's author was terminated mid-verification by a session limit, and its own last message flagged that its test tallies came from an awk pattern later proven to silently under-count failures. I re-verified the load-bearing claims myself rather than accept them, with a regex-based parser and a name-by-name inventory comparison:

• Zero failures across the crates this PR changes: ironclaw_conversations + ironclaw_attachments — 130 passed / 0 failed / 0 ignored (6 result lines); ironclaw_architecture + ironclaw_extension_contracts — 234 passed / 0 failed across 32 binaries, so the boundary rules and location scans hold.
• Un-masking confirmed by inventory diff, not by counting: base (ws2/webui-openai-ports) 109 tests → branch 130. comm against sorted name lists shows 0 removed, 21 added. No test was dropped or renamed away.
• CI on the stacked base: 12 checks, 0 failures.

Ready to merge in stack order (after #7002, its base). Two notes for whoever merges: the full 64-check set only attaches once it retargets to main, and its remaining un-run lanes are the usual stacked-gap ones rather than anything red.

[BenKurrek]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

Actionable comments posted: 7

Caution

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

⚠️ Outside diff range comments (2)

crates/ironclaw_conversations/src/inbound.rs (1)

39-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Field name still carries the collision the PR removes.

The bound is now InboundConversationService, but the field and constructor parameter are still session_thread_service (lines 42-46, and again at 309-319). SessionThreadService is the ironclaw_threads type this rename exists to stop colliding with, so the old name now points at the wrong crate's vocabulary. It is private, so this is readability only.

Rename to conversation_service in this crate while you are in the file.

🤖 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_conversations/src/inbound.rs` around lines 39 - 48, Rename
the inbound service field and constructor parameter from session_thread_service
to conversation_service throughout the inbound conversation service
implementation, including the constructor near new and the later initialization
around the referenced lines. Update all internal references while preserving
behavior and the InboundConversationService type bound.

crates/ironclaw_reborn_composition/src/trigger_poller_assembly.rs (1)

59-67: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Do not silence the unused-field warning with a cfg-gated no-op read.

TriggerPollerServices::pairing_service is assigned in production but only read under #[cfg(any(test, feature = "test-support"))]; compile-time unused-field lints are not suppressed by that block. Per CLAUDE.md, avoid cfg-gated no-op field reads that hide lints: either gate the field off in production build shape or add the intended production reader.

🤖 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_composition/src/trigger_poller_assembly.rs` around
lines 59 - 67, Remove the cfg-gated no-op read of
TriggerPollerServices::pairing_service and resolve the unused-field warning by
either conditionally excluding the field from the production build shape or
adding the intended production code path that reads it, while preserving its
test and test-support behavior.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@crates/ironclaw_attachments/src/project_scoped.rs`:
- Around line 56-59: Delete the stranded `///` documentation block immediately
before the `#[async_trait]` implementation of `InboundAttachmentLander`; it
incorrectly describes `ProjectScopedAttachmentReader` and is incomplete.
- Around line 464-508: Extend
stale_cleanup_with_empty_or_unowned_snapshot_deletes_nothing with an in-root
attachment reference whose relative path depth is not 3, then call cleanup_stale
and assert it returns the expected error and leaves the seeded attachment
intact. Reuse the existing fixture and verify one malformed reference aborts
reclamation for the entire thread.

In `@crates/ironclaw_conversations/src/ids.rs`:
- Around line 124-140: Choose and document one rollback policy for both
persistence adapters: either dual-write canonical and legacy field names during
a transition, or declare rollback unsupported after this commit. If
dual-writing, update crates/ironclaw_conversations/src/ids.rs:124-140 to replace
derived serialization and alias-based deserialization with explicit two-field
raw serialization/deserialization that prefers topic_id, and update
crates/ironclaw_conversations/src/stored_refs.rs:24-56 to cover both
topic_id/thread_id and reply_target_message_id/message_id; revise its test at
lines 133-136 accordingly. If rollback is unsupported, add the rollback boundary
to the ids module documentation and apply the same policy to stored_refs.rs
without changing the test to require legacy writes.

In `@crates/ironclaw_product/src/run_delivery/gate_routes.rs`:
- Around line 63-73: Update the source-conversation branch that builds
`ExternalConversationRef` to pass `None` for the reply target, preserving a
stable fingerprint independent of per-event message IDs. Also register the
topic-less variant used by the delivered-message loop, and add a caller-level
contract test covering recording with a reply target followed by resolution
without one.

In `@crates/ironclaw_product/tests/run_delivery_contract.rs`:
- Around line 1287-1289: Update the import used by the fixture around
source_fingerprint to import ExternalConversationRef from
ironclaw_extension_contracts::external instead of ironclaw_conversations, while
leaving the fingerprint assertion behavior unchanged.

In `@crates/ironclaw_webui/Cargo.toml`:
- Around line 36-39: Update
crates/ironclaw_webui/tests/reborn_dependency_boundaries.rs to explicitly allow
the ironclaw_attachments workspace dependency and document the existing
rationale about sharing the attachment size ceiling. Verify the WebUI imports
only the capability DTO and size constants from ironclaw_attachments, with no
attachment landing or filesystem types referenced under the WebUI source.

In `@docs/reborn/target-architecture/CHECKLIST.md`:
- Line 139: Add an explicit tracked follow-up for the
ProjectScopedAttachmentReader row’s wording or LoopAttachmentReadPort re-homing,
including an issue or plan link in the checklist. Keep the existing dependency
and orphan-rule blockers, but ensure the [~] row surfaces this obligation before
closure.

---

Outside diff comments:
In `@crates/ironclaw_conversations/src/inbound.rs`:
- Around line 39-48: Rename the inbound service field and constructor parameter
from session_thread_service to conversation_service throughout the inbound
conversation service implementation, including the constructor near new and the
later initialization around the referenced lines. Update all internal references
while preserving behavior and the InboundConversationService type bound.

In `@crates/ironclaw_reborn_composition/src/trigger_poller_assembly.rs`:
- Around line 59-67: Remove the cfg-gated no-op read of
TriggerPollerServices::pairing_service and resolve the unused-field warning by
either conditionally excluding the field from the production build shape or
adding the intended production code path that reads it, while preserving its
test and test-support behavior.

🪄 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: 6a61e139-6105-4db4-b1bd-81d8f78dcd00

📥 Commits

Reviewing files that changed from the base of the PR and between 256163c and 2f78d5d.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock, !**/Cargo.lock

📒 Files selected for processing (57)

• crates/ironclaw_architecture/tests/reborn_conversations_threads_attachments.rs
• crates/ironclaw_architecture/tests/reborn_extension_contract_location_scan.rs
• crates/ironclaw_architecture/tests/reborn_transport_product_boundary.rs
• crates/ironclaw_attachments/Cargo.toml
• crates/ironclaw_attachments/src/budgets.rs
• crates/ironclaw_attachments/src/lib.rs
• crates/ironclaw_attachments/src/ports.rs
• crates/ironclaw_attachments/src/project_scoped.rs
• crates/ironclaw_conversations/Cargo.toml
• crates/ironclaw_conversations/src/conversation_state_store.rs
• crates/ironclaw_conversations/src/ids.rs
• crates/ironclaw_conversations/src/inbound.rs
• crates/ironclaw_conversations/src/lib.rs
• crates/ironclaw_conversations/src/memory.rs
• crates/ironclaw_conversations/src/stored_refs.rs
• crates/ironclaw_conversations/src/traits.rs
• crates/ironclaw_conversations/src/types.rs
• crates/ironclaw_conversations/tests/conversation_state_store_contract.rs
• crates/ironclaw_conversations/tests/inbound_contract.rs
• crates/ironclaw_extension_contracts/src/external.rs
• crates/ironclaw_extension_host/Cargo.toml
• crates/ironclaw_extension_host/src/channel_host.rs
• crates/ironclaw_extension_host/src/channel_host/e2e_tests.rs
• crates/ironclaw_extension_host/src/channel_pairing.rs
• crates/ironclaw_extension_host/src/channel_pairing/tests.rs
• crates/ironclaw_product/src/conversation_binding.rs
• crates/ironclaw_product/src/inbound_turn.rs
• crates/ironclaw_product/src/inbound_turn/tests/attachments.rs
• crates/ironclaw_product/src/lib.rs
• crates/ironclaw_product/src/product_surface_inbound.rs
• crates/ironclaw_product/src/reborn_services.rs
• crates/ironclaw_product/src/run_delivery/gate_routes.rs
• crates/ironclaw_product/src/scoped_fs/attachment_landing.rs
• crates/ironclaw_product/src/scoped_fs/mod.rs
• crates/ironclaw_product/src/workflow.rs
• crates/ironclaw_product/tests/product_surface_contract.rs
• crates/ironclaw_product/tests/reborn_services_contract.rs
• crates/ironclaw_product/tests/run_delivery_contract.rs
• crates/ironclaw_reborn_composition/src/automation/trigger_poller_trusted_submit.rs
• crates/ironclaw_reborn_composition/src/extension_host_assembly.rs
• crates/ironclaw_reborn_composition/src/factory.rs
• crates/ironclaw_reborn_composition/src/factory/test_support.rs
• crates/ironclaw_reborn_composition/src/factory/trigger_creation_assembly.rs
• crates/ironclaw_reborn_composition/src/product_surface.rs
• crates/ironclaw_reborn_composition/src/runtime.rs
• crates/ironclaw_reborn_composition/src/runtime/tests/core.rs
• crates/ironclaw_reborn_composition/src/trigger_poller_assembly.rs
• crates/ironclaw_reborn_composition/tests/production_runtime_trigger_poller.rs
• crates/ironclaw_reborn_composition/tests/trigger_poller_e2e.rs
• crates/ironclaw_reborn_composition/tests/trigger_webui_timeline_e2e.rs
• crates/ironclaw_webui/Cargo.toml
• crates/ironclaw_webui/src/webui_v2/handlers.rs
• crates/ironclaw_webui/tests/webui_v2_handlers_contract.rs
• docs/reborn/target-architecture/CHECKLIST.md
• docs/reborn/target-architecture/PROPOSAL.md
• tests/integration/support/harness/mod.rs
• tests/integration/support/triggered_submit.rs

[coderabbitai]

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

New workspace dependency requires an update to the WebUI dependency-boundary test.

ironclaw_webui/**/Cargo.toml states: "Keep workspace dependencies limited to the explicitly allowed crates; any other workspace-crate dependency requires updating tests/reborn_dependency_boundaries.rs with explicit rationale." This adds ironclaw_attachments, and crates/ironclaw_webui/tests/reborn_dependency_boundaries.rs is not in the change set.

The rationale in the manifest comment is sound — one home for the size ceiling beats a duplicated constant. Record it in the allowlist so the boundary stays enforced instead of silently widening. Confirm the dependency is import-limited to the capability DTO and the size constants, and does not pull attachment landing or filesystem behavior into the gateway.

#!/bin/bash
# Is ironclaw_attachments on the WebUI allowlist, and what does the gateway import from it?
set -uo pipefail

echo "=== the boundary test: allowlist entries ==="
fd -g 'reborn_dependency_boundaries.rs' crates/ironclaw_webui --exec cat -n {}

echo
echo "=== is ironclaw_attachments named anywhere in webui tests? ==="
rg -n 'ironclaw_attachments' crates/ironclaw_webui/

echo
echo "=== everything the gateway pulls from the attachments crate ==="
rg -n 'use ironclaw_attachments|ironclaw_attachments::' crates/ironclaw_webui/src/

echo
echo "=== must NOT appear: landing / filesystem / port types in the gateway ==="
rg -n 'InboundAttachmentLander|ProjectScopedAttachmentLander|ScopedFilesystem|RootFilesystem' \
  crates/ironclaw_webui/src/

🤖 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_webui/Cargo.toml` around lines 36 - 39, Update
crates/ironclaw_webui/tests/reborn_dependency_boundaries.rs to explicitly allow
the ironclaw_attachments workspace dependency and document the existing
rationale about sharing the attachment size ceiling. Verify the WebUI imports
only the capability DTO and size constants from ironclaw_attachments, with no
attachment landing or filesystem types referenced under the WebUI source.

Source: Path instructions

[BenKurrek]

Refuted on the ask; the verification you requested was run and comes back clean.

The file you name does not exist. crates/ironclaw_webui/tests/ contains 20 test files; reborn_dependency_boundaries.rs is not among them. The only file by that name in the tree is crates/ironclaw_architecture/tests/reborn_dependency_boundaries.rs.

And it is a blocklist, not an allowlist. The webui rule there (:3063-3098) is BoundaryRule { crate_name: "ironclaw_webui", forbidden: vec![...] } — 33 named crates that must not appear. ironclaw_attachments is not on it, so the boundary did not "silently widen": there is no allowlist for ironclaw_webui to add to, and nothing was relaxed. (The allowlist-shaped checks in that file cover memory, reborn_identity, loop_contracts, extension_contracts, product_contracts only. The exact-dependency assertion is for the CLI crate, at :777.)

The substantive check — is the dependency import-limited? — passes. Exactly one import under src/, and it is the DTO plus the constants:

crates/ironclaw_webui/src/webui_v2/handlers.rs:33:
use ironclaw_attachments::{AttachmentCapabilities, attachment_capabilities};

InboundAttachmentLander, ProjectScopedAttachmentLander, ScopedFilesystem and RootFilesystem return zero matches across the whole crates/ironclaw_webui/ tree — no landing routine and no filesystem type reaches the gateway. Worth noting that ironclaw_filesystem is on webui's forbidden list (:3072), so the shape you were checking for would have failed CI on its own.

The rationale you call sound is already recorded where it takes effect, in the manifest comment above the dependency, and the size-ceiling clause is separately cashed out by reborn_transport_product_boundary.rs, whose two ProductAttachmentCapabilities residue rows this PR deletes (102 → 100) precisely because the move landed.

[coderabbitai]

Rate Limit Exceeded

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

[coderabbitai]

📐 Maintainability & Code Quality | 🔵 Trivial

Track the row re-wording obligation.

This item states the row "should be re-worded to exclude the read adapter, or the read port re-homed first". The row stays [~] on that basis, but the obligation has no tracking issue or plan link, so nothing surfaces it when someone next tries to close the row. The two blockers are recorded well: the illegal substrates → loops edge and the orphan rule.

Do you want me to open an issue to track re-homing LoopAttachmentReadPort or re-wording §6.4.9?

🤖 Prompt for AI Agents

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

In `@docs/reborn/target-architecture/CHECKLIST.md` at line 139, Add an explicit
tracked follow-up for the ProjectScopedAttachmentReader row’s wording or
LoopAttachmentReadPort re-homing, including an issue or plan link in the
checklist. Keep the existing dependency and orphan-rule blockers, but ensure the
[~] row surfaces this obligation before closure.

[BenKurrek]

Valid — done, and thank you for asking rather than assuming. You are right that the two blockers are recorded and the obligation was not: the sentence stated work with no home, so nothing would have surfaced it when someone next tried to close the [~].

Opened #7010 and linked it from the row as a dated amendment that quotes the sentence it annotates, per this program's docs convention (the target-architecture docs are amended, never silently rewritten). The issue carries the two options — re-word §6.4.9 to exclude the read adapter, or re-home LoopAttachmentReadPort first — and records the one way it must not be closed: adding a loop_host dep to ironclaw_attachments, which the arch gate asserts never appears. The box stays [~] until #7010 closes.

Landed in 69dc88ac74.

[coderabbitai]

Rate Limit Exceeded

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

[BenKurrek]

Review triage — the two outside-diff findings

CodeRabbit's review body carries two findings with no inline thread. Triaged here. The seven inline threads are answered in place; 69dc88ac74 carries the fixes.

1. crates/ironclaw_conversations/src/inbound.rs:39-48 — field still named session_thread_service — fixed

Correct, and squarely on this PR's charter rather than "readability only": SessionThreadService is the ironclaw_threads type the whole slice exists to stop colliding with, so a field named after it is exactly the residue the rename was meant to remove. The new gate (conversations_and_threads_declare_no_name_in_common) discovers declarations, so it could never have caught a field name.

13 occurrences renamed to conversation_service, all in inbound.rs, all private — no API surface, no consumer change. rg session_thread_service crates/ironclaw_conversations/ is now empty. (The remaining tree-wide hits are legitimate ironclaw_threads vocabulary: runtime.rs:1565's session_thread_service() accessor returns Arc<dyn ironclaw_threads::SessionThreadService>, which is the correct crate.)

2. crates/ironclaw_reborn_composition/src/trigger_poller_assembly.rs:59-67 — cfg-gated no-op read — substance agreed, deferred; two corrections

The cfg direction in the finding is inverted. The line is

#[cfg(not(any(test, feature = "test-support")))]
let _ = &services.pairing_service;

— gated on not(...), i.e. it exists only in production builds. The field is genuinely read under test/test-support; the no-op exists to stop the production build warning about it. The finding describes the opposite arrangement.

The rule cited is not in CLAUDE.md. I searched root CLAUDE.md, .claude/rules/, and the composition CLAUDE.md/AGENTS.md. The rule that actually applies is in .claude/rules/cargo-features.md:

Never gate on a feature to make a lint go away. #[cfg_attr(not(feature = "x"), allow(dead_code))] means the item is dead in some build — fix the build shape, don't silence it.

Different file, same substance, and it does condemn this shape.

Deferred because it is not this PR's, and neither remedy is a rename. git log -S 'let _ = &services.pairing_service' dates it to #6691; this PR's entire diff to that file is one trait bound:

-        + ironclaw_conversations::SessionThreadService
+        + ironclaw_conversations::InboundConversationService

Both remedies the finding offers — cfg the field out of the production build shape, or add the intended production reader — are composition-assembly changes to TriggerPollerServices' shape or to what the trigger poller does with a pairing service. Doing either inside a vocabulary-rename slice would smuggle a behavior or build-shape change into a PR whose claim is that it has none.

Named home: the composition god-crate row in docs/reborn/target-architecture/CHECKLIST.md, which already owns trigger_poller_assembly.rs — same pass that decides whether the poller should be reading a pairing service in production at all. That is the question underneath the lint, and it is not answerable from the lint.

[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_conversations/src/stored_refs.rs`:
- Around line 19-38: Preserve rollback compatibility for durable route records
by updating the stored record representation and its readers/writers around the
relevant serialization types and conversion methods to accept both legacy
thread_id/message_id and canonical topic_id/reply_target_message_id spellings,
while rejecting conflicting duplicate values. Add a cross-version regression
test covering records written by each representation and verified by the other,
ensuring threaded routes retain their original targets.

In `@crates/ironclaw_product/tests/run_delivery_contract.rs`:
- Around line 1321-1347: Add an independent assertion in the test around
source_fingerprint that compares the fingerprint for the topic-bearing
conversation with the same conversation created without topic_id, and requires
them to differ. Place this assertion before the
delivered_conversation_fingerprints membership check, while keeping the existing
reply-target invariance assertion unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f6d5e597-848f-46bd-ac24-300fc98884dd

📥 Commits

Reviewing files that changed from the base of the PR and between 2f78d5d and bbdf1e8.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock, !**/Cargo.lock

📒 Files selected for processing (57)

• crates/ironclaw_architecture/tests/reborn_conversations_threads_attachments.rs
• crates/ironclaw_architecture/tests/reborn_extension_contract_location_scan.rs
• crates/ironclaw_architecture/tests/reborn_transport_product_boundary.rs
• crates/ironclaw_attachments/Cargo.toml
• crates/ironclaw_attachments/src/budgets.rs
• crates/ironclaw_attachments/src/lib.rs
• crates/ironclaw_attachments/src/ports.rs
• crates/ironclaw_attachments/src/project_scoped.rs
• crates/ironclaw_conversations/Cargo.toml
• crates/ironclaw_conversations/src/conversation_state_store.rs
• crates/ironclaw_conversations/src/ids.rs
• crates/ironclaw_conversations/src/inbound.rs
• crates/ironclaw_conversations/src/lib.rs
• crates/ironclaw_conversations/src/memory.rs
• crates/ironclaw_conversations/src/stored_refs.rs
• crates/ironclaw_conversations/src/traits.rs
• crates/ironclaw_conversations/src/types.rs
• crates/ironclaw_conversations/tests/conversation_state_store_contract.rs
• crates/ironclaw_conversations/tests/inbound_contract.rs
• crates/ironclaw_extension_contracts/src/external.rs
• crates/ironclaw_extension_host/Cargo.toml
• crates/ironclaw_extension_host/src/channel_host.rs
• crates/ironclaw_extension_host/src/channel_host/e2e_tests.rs
• crates/ironclaw_extension_host/src/channel_pairing.rs
• crates/ironclaw_extension_host/src/channel_pairing/tests.rs
• crates/ironclaw_product/src/conversation_binding.rs
• crates/ironclaw_product/src/inbound_turn.rs
• crates/ironclaw_product/src/inbound_turn/tests/attachments.rs
• crates/ironclaw_product/src/lib.rs
• crates/ironclaw_product/src/product_surface_inbound.rs
• crates/ironclaw_product/src/reborn_services.rs
• crates/ironclaw_product/src/run_delivery/gate_routes.rs
• crates/ironclaw_product/src/scoped_fs/attachment_landing.rs
• crates/ironclaw_product/src/scoped_fs/mod.rs
• crates/ironclaw_product/src/workflow.rs
• crates/ironclaw_product/tests/product_surface_contract.rs
• crates/ironclaw_product/tests/reborn_services_contract.rs
• crates/ironclaw_product/tests/run_delivery_contract.rs
• crates/ironclaw_reborn_composition/src/automation/trigger_poller_trusted_submit.rs
• crates/ironclaw_reborn_composition/src/extension_host_assembly.rs
• crates/ironclaw_reborn_composition/src/factory.rs
• crates/ironclaw_reborn_composition/src/factory/test_support.rs
• crates/ironclaw_reborn_composition/src/factory/trigger_creation_assembly.rs
• crates/ironclaw_reborn_composition/src/product_surface.rs
• crates/ironclaw_reborn_composition/src/runtime.rs
• crates/ironclaw_reborn_composition/src/runtime/tests/core.rs
• crates/ironclaw_reborn_composition/src/trigger_poller_assembly.rs
• crates/ironclaw_reborn_composition/tests/production_runtime_trigger_poller.rs
• crates/ironclaw_reborn_composition/tests/trigger_poller_e2e.rs
• crates/ironclaw_reborn_composition/tests/trigger_webui_timeline_e2e.rs
• crates/ironclaw_webui/Cargo.toml
• crates/ironclaw_webui/src/webui_v2/handlers.rs
• crates/ironclaw_webui/tests/webui_v2_handlers_contract.rs
• docs/reborn/target-architecture/CHECKLIST.md
• docs/reborn/target-architecture/PROPOSAL.md
• tests/integration/support/harness/mod.rs
• tests/integration/support/triggered_submit.rs

[github-actions]

Coverage ratchet

Ratchet mode: ENFORCING

RATCHET PASS: global
  observed: 86% (325158 / 378092 lines)
  floor:    85.11% (tolerance 0.5pp -> effective floor 84.61%)
  denominator: 378092 lines now vs 375097 at floor capture (+2995 lines, +0.8%) — not a material change

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

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

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

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

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

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

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

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

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

RATCHET PASS: ironclaw_product
  observed: 87.79% (22195 / 25281 lines)
  floor:    86.94% (tolerance 0.5pp -> effective floor 86.44%)
  floor_covered_lines: 21367 (tolerance 20 lines -> effective floor 21347)
  denominator: 25281 lines now vs 24576 at floor capture (+705 lines, +2.87%) — not a material change

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

RATCHET PASS: ironclaw_extension_host
  observed: 85.06% (24454 / 28748 lines)
  floor:    83.82% (tolerance 0.5pp -> effective floor 83.32%)
  floor_covered_lines: 22271 (tolerance 20 lines -> effective floor 22251)
  denominator: 28748 lines now vs 26569 at floor capture (+2179 lines, +8.2%) — material change (>5%)

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

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

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

Reborn integration-tier coverage

Line coverage (Reborn crates): 86% — 325158 / 378092 lines

Per-crate breakdown (63 crates, lowest-covered first)

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

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

Exemptions (18 entry/entries excluded from the accounting above)

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

[serrrfirat]

Code Review (multi-agent)

Intent: Resolve conversations/threads naming collisions, centralize external reference contracts, and expand attachment landing APIs while preserving compatibility except for a bounded fingerprint transition.

Mode: normal — the shape preflight selected the standard eight-lens path for this 59-file PR. The required review packet is present in the PR body.

Coverage: diff source github; 59 files (production=33, tests=19, docs=2, generated_vendor=1, config=4, ci=0); diff_truncated=true. Each reviewer inspected the full detached worktree because the bundled unified diff was truncated to 40,000 characters. Limitations: the codebase graph was unavailable, and preflight's rename-aware summary could not resolve the PR branch names locally.

Stats: 12 findings (from 14 raw reviewer findings; 12 after false-positive filtering; 12 after overlap/same-line dedup) across 10 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 5.

Tests

1. Medium Legacy serde adapters are not tested through the durable store (crates/ironclaw_conversations/src/memory.rs:1528-1601, confidence 100) — anchor: crates/ironclaw_conversations/src/memory.rs:1528
   The compatibility adapters are tested through surrogate structs, but no test reopens a real persisted conversation state using the legacy thread_id / message_id spelling, so misplaced record annotations could silently collapse a threaded route.

Conventions

1. Medium Update the owning conversation contract after moving its API (crates/ironclaw_conversations/src/lib.rs:38-43, confidence 100) — anchor: AGENTS.md:159-163,193-194
   The owning contract and crate-local guidance still assign the external refs and old SessionThreadService vocabulary to ironclaw_conversations.

2. Medium Reconcile guidance with the widened attachments boundary (crates/ironclaw_attachments/src/lib.rs:13-20, confidence 100) — anchor: AGENTS.md:159-163,193-194
   The crate map, product-contracts note, and WebUI dependency guidance still describe the pre-move attachments ownership boundary.

Local patterns

1. Medium Rename the reader-only attachment_landing module (crates/ironclaw_product/src/scoped_fs/attachment_landing.rs:1, confidence 100) — anchor: crates/ironclaw_product/src/scoped_fs/project_filesystem_reader.rs:1
   The lander moved out, but the remaining reader-only module retains a landing-oriented path that now misdirects source discovery.

2. Low Restore the reader documentation truncated by the move (crates/ironclaw_product/src/scoped_fs/attachment_landing.rs:24, confidence 100) — anchor: crates/ironclaw_product/src/scoped_fs/attachment_landing.rs:24 (no diff position — body only)
   The public reader rustdoc was reduced to a lowercase sentence fragment and no longer explains mount re-scoping or the loop-model use.

3. Low Repoint the attachment lander rustdoc link (crates/ironclaw_product/src/workflow.rs:489, confidence 100) — anchor: crates/ironclaw_product/src/lib.rs:322 (no diff position — body only)
   The explicit link still targets the removed crate::InboundAttachmentLander re-export instead of ironclaw_attachments::InboundAttachmentLander.

4. Medium Update the conversations guardrail to the new vocabulary (crates/ironclaw_conversations/CLAUDE.md:3-8, confidence 100) — anchor: crates/ironclaw_conversations/src/ids.rs:3 (no diff position — body only)
   The guide still claims ownership of the external refs and uses thread_id for the external sub-conversation identity.

5. Medium Remove the obsolete attachment-capabilities ownership note (crates/ironclaw_product_contracts/src/inbound_requests.rs:15-17, confidence 100) — anchor: crates/ironclaw_product/src/product_surface_inbound.rs:14 (no diff position — body only)
   The comment says the deleted ProductAttachmentCapabilities remains in product instead of naming its new attachments owner.

6. Low Refresh the attachments routing entry after widening (crates/AGENTS.md:87, confidence 100) — anchor: AGENTS.md:15 (no diff position — body only)
   The primary routing entry still describes only the landing routine and omits the ports, cleanup contract, default lander, and advertised/enforced budgets.

Maintainability

1. Low Delete the no-op actor serde adapter (crates/ironclaw_conversations/src/stored_refs.rs:82-113, confidence 100) — anchor: crates/ironclaw_extension_contracts/src/external.rs:109
   The adapter duplicates the canonical actor serializer/deserializer, which already accepts a missing optional display name and revalidates through the constructor.

2. Low Reuse the shared ratchet source lexer (crates/ironclaw_architecture/tests/reborn_conversations_threads_attachments.rs:135-170, confidence 100) — anchor: crates/ironclaw_architecture/tests/ratchet_support/mod.rs:215
   The new local lexer is weaker than the existing shared helper and can make architecture gates disagree on block comments, raw strings, byte strings, or character literals.

Approach

1. Medium Keep emitting the rollback-compatible persisted field names (crates/ironclaw_conversations/src/stored_refs.rs:47-55, confidence 88) — anchor: crates/ironclaw_conversations/src/stored_refs.rs:47-55; crates/ironclaw_conversations/src/ids.rs:92-106; .claude/rules/type-placement.md:60-67
   Writing only the new field names creates a permanent downgrade boundary. The persistence adapters can emit the legacy grammar while accepting both spellings, preserving rollback compatibility without retaining duplicate domain types or dual-writing duplicate fields.

Security, bugs, and performance/concurrency returned no findings.

[serrrfirat]

Medium — Legacy serde adapters are not tested through the durable store.

The compatibility tests deserialize surrogate structs declared inside stored_refs.rs, but no test reopens an actual StoredConversationState containing pre-unification thread_id/message_id fields. Omitting or misplacing either new serde adapter annotation could leave current tests green while an upgrade silently loads a threaded route as its conversation root.

Fix: Add filesystem_conversation_services_reopen_pre_unification_external_refs covering a persisted binding and reply-target snapshot rewritten to legacy thread_id/message_id spelling, then reopen and assert the topic and reply target survive.

[BenKurrek]

Valid, and the gap was real — the surrogate tests could not see a missing annotation. Fixed in 4cd166f727.

Two tests now drive the actual StoredConversationState through RebornFilesystemConversationServices, in crates/ironclaw_conversations/tests/conversation_state_store_contract.rs:

• filesystem_conversation_services_persist_external_refs_in_the_durable_grammar — pairs an actor, resolves a threaded binding, accepts a message, then reads /conversations/state.json back and walks every key at every depth, asserting no canonical spelling appears anywhere and that the topic and reply target are present under the durable names. Because it is discovered rather than enumerated, a missing or misplaced annotation at any site fails it, which is the failure mode you described.
• filesystem_conversation_services_reopen_pre_unification_external_refs (your name) — rewrites the persisted document's ref objects to the other spelling and reopens, asserting the topic survives as a topic: a second topic in the same conversation must still key a different binding, so the test cannot pass with the topic read as None.

That the surrogates were insufficient is not inference — it is measured. Mutating ExternalConversationIdentity::serialize to emit topic_id leaves every stored_refs.rs unit test green and fails only the store-level test, which names all six affected sites:

found the canonical spelling at ["topic_id", "topic_id", "topic_id",
                                "topic_id", "topic_id", "topic_id"]

Those six are BindingKey, BindingRecord.external_conversation_identity (twice — bindings and source_bindings), two external_event_routes entries, and StoredAcceptedMessageReplay. Exactly the "current tests green while an upgrade silently loads a threaded route as its conversation root" case.

This is the repo's own "test through the caller, not just the helper" rule, and the suite was on the wrong side of it. Thank you for catching it.

[serrrfirat]

Medium — Update the owning conversation contract after moving its API.

The diff removes the conversations re-export of ExternalActorRef/ExternalConversationRef and renames SessionThreadService plus its DTOs, but docs/reborn/contracts/conversation-binding.md and the crate-local AGENTS/CLAUDE guidance still assign those refs and the old service name to ironclaw_conversations. This contradicts the new one-home contract and directs future callers back to deleted names.

Fix: Update the conversation contract and crate-local AGENTS/CLAUDE files to name InboundConversationService, the renamed DTO family, topic_id, and ironclaw_extension_contracts::external as the external-ref owner.

[BenKurrek]

Valid — the contract did still name the deleted API. Amended in 4cd166f727.

Confirmed each claim against the tree before editing:

• docs/reborn/contracts/conversation-binding.md:34 — ownership table row keyed on SessionThreadService.
• :45 — "ConversationBindingService, SessionThreadService, and InboundTurnService traits/DTOs".
• :44 — assigned ExternalActorRef/ExternalConversationRef to this crate's "implemented semantic slice".
• :63 — invariant 8: binding identity is (space_id, conversation_id, thread_id).
• crates/ironclaw_conversations/CLAUDE.md:5 — "the SessionThreadService/TranscriptStore storage boundary".

All five corrected. The contract carries a dated amendment block at the head rather than a silent in-place rewrite, per this program's docs convention, because contract freeze makes these three of the answers an engineer is entitled to rely on (_contract-freeze-index.md §1: "which public traits/types are stable enough to implement against"). It records the service rename plus its DTO trio, the ref pair's move to ironclaw_extension_contracts::external, and topic_id; the table row and the slice list are corrected in place, and invariant 8 now reads (space_id, conversation_id, topic_id).

One thing I added beyond your list, because leaving it out would have made the contract newly misleading in the other direction. Invariant 8 now says the durable record still spells that field thread_id. That is deliberate and is the subject of @serrrfirat's stored_refs.rs:53 finding on this same PR: only the Rust spelling changed, so a rollback can still read persisted routes. A contract that said topic_id with no qualifier would have sent the next reader looking for a key that is not in the document.

The two rules the amendment points at are enforced, not just written down: reborn_conversations_threads_attachments.rs pins the no-shared-name rule by discovery rather than enumeration, and pins the ref pair's single home.

[serrrfirat]

Medium — Reconcile guidance with the widened attachments boundary.

The diff makes ironclaw_attachments own and export the ports, default lander, and advertised capabilities, while existing crate-map, product-contracts, and WebUI guidance still describes the previous ownership/dependency boundary. Those instructions conflict with the refactor's new boundary.

Fix: Update the crate map, product-contracts module documentation, and WebUI AGENTS/CLAUDE dependency guidance to describe the new attachment owner and explicitly allow the justified direct WebUI edge.

[BenKurrek]

Two of the three were stale and are fixed; the third was already updated in this PR. 4cd166f727.

Fixed — the crate map. crates/AGENTS.md:87 described ironclaw_attachments as "the single inbound-attachment landing routine" and nothing else, which is the pre-widening boundary. It now names what the crate actually owns after this PR: the ports (InboundAttachmentLander, InboundAttachmentReader, AttachmentCleanupReport), the default ProjectScopedAttachmentLander, and the size ceilings + advertised AttachmentCapabilities that WebUI and the OpenAI-compatible adapter read. The "does not own" column now carries the reader carve-out, since that is the part of the row a reader is most likely to get wrong.

Fixed — the WebUI dependency guidance. crates/ironclaw_webui/AGENTS.md:68-75 enumerated the allowed workspace edges and closed with "any other workspace-crate edge requires an ironclaw_architecture boundary-test update plus explicit PR rationale". ironclaw_attachments was not in that list, so the guidance contradicted the manifest. It is now listed with the justification scoped to what the edge is actually for — the advertised ceilings only, so the transport reads the same constants the landing routine enforces instead of keeping a second copy.

Already correct — the product-contracts module documentation. crates/ironclaw_product_contracts/src/inbound_requests.rs:15-17 already points at the new owner:

decode_attachments and ProductAttachmentCapabilities stay there too, because the byte budgets are ironclaw_attachments types. (CHECKLIST WS5 "attachments widened … one home for size ceilings" owns that move.)

I re-read the crate's other attachment-touching modules (inbound.rs, product_wire.rs, workspace_views.rs, surface.rs) and found no ownership claim that survived the move, so no edit there. If you were pointing at a specific line I have not found, say which and I will take it.

Note this is a different ask from the earlier CodeRabbit thread on Cargo.toml:39, which I refuted: that one named a crates/ironclaw_webui/tests/reborn_dependency_boundaries.rs that does not exist and treated a blocklist as an allowlist. Yours is about the guidance being inconsistent with the new boundary, and it was.

[serrrfirat]

Medium — Rename the reader-only attachment_landing module.

After moving the lander to ironclaw_attachments, this module contains only ProjectScopedAttachmentReader, but its path still says attachment_landing. Path-based discovery now sends readers looking for landing behavior to the wrong crate.

Fix: Rename the file/module to attachment_reader or project_scoped_attachment_reader, then update the declaration, re-export, tests, docs, and exact-path ratchets.

[BenKurrek]

Valid — renamed. 4cd166f727.

The file's own module doc already opened with "Project-scoped inbound attachment read-back", so the path was the last thing still claiming otherwise. git mv to attachment_reader.rs, and every reference repointed:

• crates/ironclaw_product/src/scoped_fs/mod.rs:14,17 — the pub mod and the pub use.
• crates/ironclaw_architecture/tests/reborn_struct_test_support_ratchet.rs:456 — the exact-path ratchet entry you flagged.
• docs/reborn/target-architecture/CHECKLIST.md:58 and PROPOSAL.md:66,611 — the three places that describe the current consumer of the LoopAttachmentReadPort/LoopAttachmentReadError pair, each now carrying "(renamed from attachment_landing.rs when the lander moved)" so the WS5/WS6 severing work still resolves.

Left alone deliberately: CHECKLIST.md:144 and PROPOSAL.md:546 say the impls "were never in composition, they were in ironclaw_product::scoped_fs::attachment_landing". That is a statement about where they were, and it is true with the old name. Rewriting it would make the record wrong.

The module doc now leads with why the name changed rather than only with what moved out, so the next reader gets the answer without having to reconstruct it from the CHECKLIST.

One thing worth flagging, since it is a cost your finding could not have known about. The rename moves this file's body into the changed-coverage gate's candidate set — git pairs the deleted attachment_landing.rs with ironclaw_attachments/src/project_scoped.rs (the lander's real destination) and then sees attachment_reader.rs as new. Replayed through the gate's own git_diff with CI's exact flags (--diff-algorithm=histogram --diff-filter=AMR, per-crate pathspecs), the net moves 2006 → 1745 candidate lines, so the rename reduces the total; the risk is only that it redistributes ~238 lines onto a file that was previously uncandidate. That file has an inline #[cfg(test)] module the gate excludes, and tests/integration/attach.rs drives the reader, so I expect it to hold — but I am watching that lane and will derive any needed exemption from CI's artifact rather than guessing at it.

[BenKurrek]

Follow-up on the coverage risk I flagged above: it did not hold, and chasing it down turned your rename into a real find.

The changed-coverage gate failed on 4cd166f727 at 90.87% (209/230), naming 21 lines — attachment_reader.rs:63, 101-117, 120, 121, 123. The mechanism is the one I predicted: git pairs the deleted attachment_landing.rs with ironclaw_attachments/src/project_scoped.rs (the lander's real destination), so the surviving reader reads as a new file and its pre-existing lines become changed-code candidates.

But they were not an attribution artifact. They were genuinely untested. Every test in that module drives read_attachment_bytes — the LoopAttachmentReadPort half. Nothing called InboundAttachmentReader::read, so the entire map_err taxonomy under it was dead to the suite:

LoopAttachmentReadError::NotFound  => 404 NotFound
LoopAttachmentReadError::Forbidden => 403 ParticipantDenied
LoopAttachmentReadError::Backend   => sanitized 500

That taxonomy is the bytes endpoint's contract — a caller separates "gone" from "not yours" from "broken" only by the status this closure picks — and none of it was pinned. So the rename did not merely move lines into a gate; it exposed that the reader's product-facing half had no coverage at all, which is exactly the thing a path-name that lied about the module's contents helps hide.

Fixed in 5b85634272 by covering them, not by exempting them — an exemption is for lines that cannot execute, and these can. Four tests appended to the existing inline module (extended, not a new file): the success path through the thread scope, 404, 403 (a denied mount must not answer 404 — that would tell a caller the attachment does not exist), and a malformed storage_key that must fail closed at ScopedPath::new and become a 500 that does not echo the rejected value back.

Verified with the same instrumentation the gate uses rather than by assertion — cargo llvm-cov over that test binary reports non-zero hits on all 21 lines CI listed, none still uncovered:

   63: hits=1     101: hits=3    102-117: hits=1    120: hits=1
  121: hits=1     123: hits=3
STILL UNCOVERED: NONE

Product crate 1026 → 1030 tests, 0 removed. The exemption manifest is untouched and re-validated (73 entries, 316 coordinates, zero stranded).

[BenKurrek]

Correcting myself on one detail of the comment above, because it does not meet the bar I applied to the rest of this round.

I wrote that the malformed-key test asserts the 500 "does not echo the rejected value back". That assertion was vacuous — it could never have failed. ProductSurfaceError carries only {code, kind, status_code, retryable, field, validation_code}; there is no reason-bearing field, and internal_from logs its source through tracing::error! and then returns Self::internal(), discarding it. So !format!("{error:?}").contains("evil.example.com") was decoration, not coverage. The non-leak is structural, not behavioral.

Replaced in 5f1867ac87 with assert_eq!(error, ProductSurfaceError::internal()), which pins the entire sanitized shape — and unlike the string check, it would fail if a future variant added a detail-bearing field. The comment now states which property is structural and why no assertion is made for it, so the next reader does not re-add the vacuous form.

Mutation-proved this time rather than asserted: making ScopedPath::new's failure classify as NotFound instead of Backend fails exactly this test and no other. I went back and did the same for the sibling — collapsing Forbidden into the NotFound shape fails only the 403 test. The other three lines of the taxonomy were already pinned that way.

The coverage result is unchanged: the test executes the same paths, so all 21 lines stay covered. Product crate still 1030 tests, 0 removed.

[serrrfirat]

Low — Delete the no-op actor serde adapter.

The actor_ref adapter delegates serialization and reconstructs the same kind/id/display_name shape through the same constructor as ExternalActorRef's existing Deserialize implementation. Missing Option already deserializes as None, so the annotations add a second compatibility path without changing legacy behavior.

Fix: Delete stored_refs::actor_ref and remove its serde(with) annotations from ActorKey and AcceptedMessageReplayKey; use ExternalActorRef's canonical Serialize/Deserialize implementations.

[BenKurrek]

Valid — deleted. 4cd166f727.

Checked rather than taken on faith, because the claim rests on ExternalActorRef's canonical impls being exactly equivalent, and they are:

• Deserialize (ironclaw_extension_contracts/src/external.rs:116) reads ExternalActorRefWire { kind, id, display_name: Option<String> } and reconstructs through ExternalActorRef::new(...).map_err(serde::de::Error::custom) — the same three fields, the same constructor, the same error mapping the adapter used. Serde defaults a missing Option to None, so a released {kind, id} payload loads with no annotation at all.
• Serialize is derived, and the adapter's serialize did nothing but value.serialize(serializer).

So both halves were a restatement. #[serde(with = "crate::stored_refs::actor_ref")] is gone from ActorKey and AcceptedMessageReplayKey, and the module is now conversation-ref only.

The two tests were retargeted, not deleted — they are the evidence for the deletion, so they now run against a record that carries no #[serde(with)]: actor_serde_needs_no_adapter and stored_actor_ref_revalidates_on_load.

Worth stating explicitly, because it is the asymmetry that decides which ref needs an adapter and which does not: the actor change was additive ({kind,id} → {kind,id,display_name}), and an added field is safe in both directions — serde ignores unknown fields, and a missing Option reads as None. The conversation ref's change was a rename, which is safe in neither. That contrast is now the # Scope: no actor adapter section of the module doc.

[serrrfirat]

Low — Reuse the shared ratchet source lexer.

This test introduces a second strip_comments_and_strings implementation even though ratchet_support exposes the canonical helper. The local version handles only line comments and ordinary strings, while the shared lexer also handles nested block comments, raw/byte strings, and character literals, so architecture gates can disagree.

Fix: Import ratchet_support::strip_comments_and_strings and delete the local implementation; keep the self-test against the shared helper if desired.

[BenKurrek]

Valid — and the local copy was not merely redundant, it was wrong in a way that mattered. Fixed in 4cd166f727.

ratchet_support::strip_comments_and_strings is imported and the local implementation is deleted. reborn_authorized_seal_ratchet.rs:28 already imported the shared one, so this file was the outlier, not the precedent.

The gap was real, not theoretical. Ran the deleted line-based implementation against a fixture containing a block comment:

OLD local lexer -> assertion `!stripped.contains("fn conversation_actor_ref")`: FAIL
leaked lines:
    '/* fn conversation_actor_ref in a block comment'
    '   /* fn conversation_actor_ref in a NESTED block comment */'

It breaks a line at // and tracks " per line, so it has no concept of a block comment at all. A prose mention of a type name inside /* … */ anywhere in ironclaw_conversations or ironclaw_threads would have registered as a declaration and failed rule 1 with a name nobody declared — the exact "architecture gates can disagree" outcome you named, arriving as a false positive on the most confusing possible failure message.

The self-test is kept and extended rather than dropped, per your "if desired" — it now covers the three shapes only the shared lexer handles (nested block comments, a raw string, a char literal holding a quote), so this file pins the behavior it actually depends on instead of trusting that another gate's self-test covers it.

All five tests in the file still pass, which is the non-vacuity check that matters here: the frozen name sets are unchanged, so the swap is behavior-preserving for this gate while removing the latent divergence.

One correction to the count, for the record: this was the third copy, not the second — reborn_deployment_mode_branching_ratchet.rs:89 carries one too. I left that one alone because it is pre-existing and outside this PR's diff; flagging it here so it is on the record rather than silently unaddressed.

[BenKurrek]

Follow-up, so the third copy I flagged above does not sit homeless: it now has one.

The same consolidation point was raised on #7003 and #7004 for the other scanner helpers, and #7004 (791c2379aa) records a WS10 CHECKLIST row that owns the whole family, with the measurement: rust_files ×3, strip_cfg_test_blocks ×4 plus a fifth spelling, balanced_angle_close ×2, implemented_trait_names ×2 — and it explicitly names reborn_deployment_mode_branching_ratchet.rs's shadow of ratchet_support::strip_comments_and_strings to be folded in the same pass.

So the split is: the copy this PR introduced is deleted here (it was mine to remove), and the pre-existing one is on that row rather than left as an unowned observation.

[serrrfirat]

Medium — Keep emitting the rollback-compatible persisted field names.

The compatibility adapter delegates serialization to the canonical ref, so subsequent state writes replace thread_id/message_id with topic_id/reply_target_message_id; ExternalConversationIdentity similarly emits the new spelling. Older binaries then silently load durable threaded routes as conversation-root routes. Dual-writing is unnecessary: the persistence adapter can keep writing the legacy grammar while accepting either spelling, preserving rollback without retaining a duplicate domain type.

Fix: Serialize conversation_ref through a borrowed stored representation that emits thread_id/message_id, and give ExternalConversationIdentity a custom serializer that emits thread_id; keep the aliases so both spellings remain readable.

[BenKurrek]

You are right, and the reasoning in the module doc was wrong — it refuted a proposal you did not make. Fixed in 4cd166f727.

Where our argument failed. stored_refs.rs refused dual-writing — emitting both spellings — and that refusal is sound: #[serde(alias)] makes a dual-written record self-defeating, because a reader that accepts both spellings rejects a document carrying both as a duplicate field. But you asked for write-legacy / read-either, which that objection does not touch at all. One field goes out, thread_id; the alias accepts it. The module talked itself out of the fix by arguing against a different design.

Worse, the module's own opening sentence is the argument for your version — "a record grammar outlives the type that shaped it" — and then the code wrote the new type's spelling into the record anyway.

Your factual premises, verified against origin/main. The released readers are crates/ironclaw_conversations/src/ids.rs — RawExternalConversationIdentity { space_id, conversation_id, thread_id } and the ExternalConversationRef wire struct {space_id, conversation_id, thread_id, message_id}. Neither carries an alias, and neither sets deny_unknown_fields. So a document this build wrote read back as topic_id ignored + thread_id absent + None, silently.

And the consequence is one notch worse than "conversation-root routes". ExternalConversationIdentity keys BindingKey, and StoredConversationState::into_state rebuilds the map with Vec<(K, V)>::into_iter().collect() (conversation_state_store.rs:190). Two threaded bindings in one conversation that both lose their topic become the same key, and the later one silently wins. That is binding loss, not misrouting.

What landed — exactly your prescription.

• conversation_ref::serialize now writes through a borrowed Written<'a> { space_id, conversation_id, thread_id, message_id }.
• ExternalConversationIdentity gets a matching hand-written Serialize emitting thread_id (it already had a hand-written Deserialize, so it is now symmetric).
• Both readers keep the aliases, so either spelling loads.
• A record carrying both spellings still fails closed — kept deliberately, and now pinned by a_record_carrying_both_spellings_is_rejected.

Proof it is not just asserted. Reverting serialize to the canonical delegate fails both the unit test and the store-level test. Making the identity emit topic_id fails the store-level test with the six sites named:

durable records must keep the released grammar so a rollback can still read them;
found the canonical spelling at ["topic_id" x6]

and the fixed document reads:

"external_conversation_ref":{"conversation_id":"chat-threaded","message_id":"msg-1700.1",
                             "space_id":"space-1","thread_id":"topic-1"}

The # Rollback boundary section that declared the one-way compatibility "by decision" is gone; the module now documents why the write side matters as much as the read side.

[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_conversations/CLAUDE.md`:
- Line 5: Update the binding identity guardrail in CLAUDE.md to use topic_id for
the external channel sub-conversation route identity, while reserving thread_id
for durable record grammar. Keep the surrounding
InboundConversationService/TranscriptStore storage-boundary guidance unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f54fe063-db74-4b56-bfb5-7d917a051745

📥 Commits

Reviewing files that changed from the base of the PR and between 7fd3998 and 4cd166f.

📒 Files selected for processing (15)

• crates/AGENTS.md
• crates/ironclaw_architecture/tests/reborn_conversations_threads_attachments.rs
• crates/ironclaw_architecture/tests/reborn_struct_test_support_ratchet.rs
• crates/ironclaw_conversations/CLAUDE.md
• crates/ironclaw_conversations/src/ids.rs
• crates/ironclaw_conversations/src/memory.rs
• crates/ironclaw_conversations/src/stored_refs.rs
• crates/ironclaw_conversations/tests/conversation_state_store_contract.rs
• crates/ironclaw_product/src/scoped_fs/attachment_reader.rs
• crates/ironclaw_product/src/scoped_fs/mod.rs
• crates/ironclaw_product/tests/run_delivery_contract.rs
• crates/ironclaw_webui/AGENTS.md
• docs/reborn/contracts/conversation-binding.md
• docs/reborn/target-architecture/CHECKLIST.md
• docs/reborn/target-architecture/PROPOSAL.md

💤 Files with no reviewable changes (1)

• crates/ironclaw_conversations/src/memory.rs

[BenKurrek]

Superseded by #7018

This work is now carried by #7018 — consolidate the Wave 2 port-inversion stack (WS2.2, WS2.4, WS5), which merges #7000, #7003, #7004 and this PR into a single PR onto main.

What carried over: the conversations/threads naming trap and the ironclaw_attachments widening — including the rollback-grammar data-loss fix the human review caught, where the durable grammar keeps writing the legacy field names so that losing a topic drops a binding rather than merely misrouting it. Both files that carry your coverage exemptions (ironclaw_conversations/src/types.rs, run_delivery/gate_routes.rs) are byte-identical to this branch in the consolidated one, and all ten exempted lines were re-verified to still name exactly the declarations their reasons describe.

Three of the five merge conflicts were import lists where both this PR and #7004 removed different names — #7004 moving types into product_contracts, this PR moving them into ironclaw_attachments. Taking either side alone would have resurrected the other's deletions, so they were resolved by three-way set merge, (ours & theirs) | (ours - base) | (theirs - base), which honours both removal sets. Your PROPOSAL §6.9.1 edit survives, as does your LoopAttachmentReadPort re-homing row (still [~], tracked by #7010).

The coverage-exemption manifest was unioned rather than taking a side: main's WS2.2 "no entries, deliberately" rationale block is kept verbatim alongside your two new entries, and the result was validated through scripts/ci/reborn_changed_coverage.py's own load_manifest — 73 exemptions, zero stale paths, zero lines past EOF, zero duplicate claims.

Closing here so review continues in one place — closing is reversible.