feat(reborn): accept inline attachment uploads on the WebChat v2 send path (#4644)

[ilblackdragon]

Summary

End-to-end ingress wiring for #4644 — the call site for the land_inbound_attachments bridge (#4670). A browser can now attach files to a WebChat v2 message; the bytes land in project storage through the filesystem authority, and the user message persists attachment references.

Scope: crates/ only, no src/ changes. Crosses ironclaw_product_workflow (DTO + facade + port), ironclaw_reborn_composition (port impl + wiring), and ironclaw_webui_v2 (body limit).

Flow

1. DTO (webui_inbound.rs): WebUiSendMessageRequest gains attachments: Vec<WebUiInboundAttachment> (mime_type, filename, data_base64). decode_attachments() validates MIME against the shared format registry (Track 1), decodes base64, and enforces the v1 budgets (5 MiB/file, 10 MiB total, 10 files max). Kept separate from into_command so the serializable command never carries raw bytes.
2. Facade (reborn_services.rs::submit_turn): decodes attachments, lands them through a new InboundAttachmentLander port (using the stable per-message external_event_id for the storage path), and builds MessageContent::with_attachments before accept_inbound_message. With no lander wired, an attachment-bearing message is rejected (503) rather than silently dropped.
3. Port impl (composition attachment_landing.rs): ProjectScopedAttachmentLander writes through the project-scoped workspace ScopedFilesystem — the same authority the agent's file tools resolve through — via land_inbound_attachments, and is wired onto the facade in build_webui_services when a local runtime is present (mirrors the ApprovalInteractionService injection idiom).
4. Body limit: the send_message route descriptor goes 1 MiB → 14 MiB to carry base64 of the 10 MiB decoded cap. Descriptor/body-limit contract tests and the composition CLAUDE.md are updated to match.

Tests

• decode_attachments (7): metadata/kind/bytes + MIME normalization; unsupported MIME; malformed base64; per-file oversize; total oversize; too-many; empty.
• ProjectScopedAttachmentLander (2): lands + returns ref with storage_key; read-only workspace mount maps to an internal error.
• Facade, test-through-the-caller (2): submit_turn lands the attachment and the accepted user message carries the ref with storage_key; attachments without a wired lander are rejected with ServiceUnavailable.

cargo clippy -p ironclaw_attachments -p ironclaw_product_workflow --tests          # clean
cargo clippy -p ironclaw_reborn_composition --features webui-v2-beta --tests       # clean (one pre-existing
  #   feature-combo artifact in webui_serve.rs unrelated to this change; used under --all-features)
cargo test -p ironclaw_product_workflow                                            # all pass (+ 9 new)
cargo test -p ironclaw_reborn_composition --lib attachment_landing webui_body_limit # pass
cargo test -p ironclaw_webui_v2 --features webui-v2-beta --test webui_v2_descriptors_contract  # pass
cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold        # pass (new edges OK)

New dep edges product_workflow/composition → ironclaw_attachments, accepted by the reborn dependency-boundary test.

Stacking

Based on fix/4644-inbound-attachment-refs (#4670). Tip of the #4644 epic stack (registry → transcript refs → byte landing → bridge → this ingress wiring).

Deferred (remaining #4644 work)

• Model-visibility (the rest of Track 3): read bytes back from storage_key → ChatMessage.content_parts for images; run extraction/transcription to fill extracted_text; emit model-facing project_path; fix chat_workflow.rs's [non_text_content] drop. (Doc/audio extraction still needs the src/→crate move you flagged.)
• Per-attachment memory index note (memory_search discoverability); sandbox e2e; WASM store_attachment_data convergence.
• Static frontend "+ button" upload UI + in-thread attachment cards.

Summary by CodeRabbit

• New Features

  • Web messages can now include attachments when sending messages from the UI.
  • Attachments are decoded/validated, persisted, and associated with the submitted message for later use.

• Bug Fixes

  • Attachment sends fail gracefully when attachment support isn’t available.
  • WebUI attachment handling now returns stable validation errors for invalid data (e.g., type, size, count, or base64 issues).

• Documentation

  • Updated WebUI send-message route limits to reflect the increased maximum request size (14 MiB).

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds end-to-end inbound attachment support for the WebUI v2 send_message route. Introduces WebUiInboundAttachment DTO with decode_attachments() validation and base64 decoding, the InboundAttachmentLander trait, and ProjectScopedAttachmentLander backed by a project-scoped filesystem with UTC date partitioning. Wires landing into RebornServices::submit_turn with fail-closed semantics and the composition layer, and raises the body limit from 1 MiB to 14 MiB. Invariant check: CLAUDE.md body-limit alignment is maintained (14 MiB cap consistently documented across middleware, security bullets, and tests). Fail-closed wiring (503 when attachments present but no lander wired) enforces no accidental data drops.

Changes

WebUI Inbound Attachment Landing

Layer / File(s)	Summary
WebUiInboundAttachment DTO and decode_attachments validation crates/ironclaw_product_workflow/Cargo.toml, crates/ironclaw_product_workflow/src/webui_inbound.rs, crates/ironclaw_product_workflow/tests/webui_inbound_contract.rs	Adds base64/ironclaw_attachments deps. Introduces WebUiInboundAttachment struct with mime_type, optional filename, and data_base64 fields; defines budget constants (MAX_ATTACHMENTS=20, per-file and total decoded byte caps, max filename bytes). Implements decode_attachments() with MIME normalization via shared registry, count/size enforcement, base64 decode with stable WebUiInboundValidationError mapping, filename validation (trim + length), and full contract test coverage (unsupported types, malformed base64, per-file/total oversize, excess count).
InboundAttachmentLander trait and RebornServicesError::internal() crates/ironclaw_product_workflow/src/reborn_services.rs, crates/ironclaw_product_workflow/src/reborn_services/error.rs, crates/ironclaw_product_workflow/src/lib.rs	Defines public InboundAttachmentLander async trait with land(thread_scope, message_id, attachments) → Result<Vec<AttachmentRef>> contract. Adds RebornServicesError::internal() public constructor (500, non-retryable) for host-composition adapter use. Re-exports new symbols.
RebornServices attachment wiring and submit_turn integration crates/ironclaw_product_workflow/src/reborn_services.rs, crates/ironclaw_product_workflow/tests/reborn_services_contract.rs	Adds optional inbound_attachments: Arc<dyn InboundAttachmentLander> field and with_inbound_attachments builder to RebornServices. In submit_turn: decodes attachments up front; if present, requires lander (else 503 ServiceUnavailable) and lands via lander.land() to obtain AttachmentRef vector for attachment-inclusive MessageContent; if absent, builds text-only content. Passes computed content to accept_inbound_message. Adds RecordingLander test double (deterministic storage_key from message_id + index); two contract tests: one validates decode→land→persist with kind/mime_type/filename/storage_key assertions, one validates fail-closed (503) when attachments present without wired lander.
ProjectScopedAttachmentLander implementation crates/ironclaw_reborn_composition/Cargo.toml, crates/ironclaw_reborn_composition/src/attachment_landing.rs, crates/ironclaw_reborn_composition/src/lib.rs, crates/ironclaw_reborn_composition/src/local_dev_mounts.rs	Implements InboundAttachmentLander via ProjectScopedAttachmentLander<F: RootFilesystem>: routes writes through project-scoped ScopedFilesystem, computes UTC date partition, calls land_inbound_attachments with scoped fs/project alias/message_id/date/attachments/max bytes, maps failures to sanitized internal error with warning log (original error + message_id). Includes unit tests: success path asserts AttachmentRef::storage_key matches /workspace/attachments/…-<filename> pattern; read-only mount asserts RebornServicesErrorCode::Internal failure. Widens WORKSPACE_ALIAS to pub(crate) for attachment landing access. Adds ironclaw_attachments dependency.
Runtime accessor and composition-layer wiring crates/ironclaw_reborn_composition/src/runtime.rs, crates/ironclaw_reborn_composition/src/webui.rs	Adds RebornRuntime::webui_workspace_filesystem() accessor (returns ScopedFilesystem<LocalDevRootFilesystem> or None when local runtime absent). In build_webui_services_with_connectable_channels: conditionally attaches ProjectScopedAttachmentLander when workspace filesystem available. Patches two existing WebUI test fixtures with attachments: Vec::new() (maintains request shape consistency).
send_message body limit raised to 14 MiB crates/ironclaw_webui_v2/src/descriptors.rs, crates/ironclaw_webui_v2/tests/webui_v2_descriptors_contract.rs, crates/ironclaw_reborn_composition/src/webui_body_limit.rs, crates/ironclaw_reborn_composition/CLAUDE.md	Raises body_limit_kib for webui.v2.send_message from 1024 to 14×1024 in route descriptor; updates comment to reflect attachment/base64 budget rationale. Updates enforce_body_limit comments (largest cap, buffering assumptions). Updates body-limit contract test to assert 14 MiB resolves correctly. Updates webui_v2 descriptor contract test (expected policy). Propagates 14 MiB cap in CLAUDE.md middleware body-limit section and security-invariant bullets. Invariant: All three documentation layers (descriptor, enforcement, CLAUDE.md) now consistently report 14 MiB.

Sequence Diagram(s)

sequenceDiagram
    participant WebUI as WebUI Handler
    participant RS as RebornServices.submit_turn
    participant Lander as InboundAttachmentLander
    participant FS as ScopedFilesystem
    participant Thread as accept_inbound_message

    WebUI->>RS: WebUiSendMessageRequest (base64 attachments)
    RS->>RS: decode_attachments() → Vec<InboundAttachment>
    alt attachments present, lander wired
        RS->>Lander: land(thread_scope, message_id, attachments)
        Lander->>FS: land_inbound_attachments(date-partitioned path, max_bytes)
        FS-->>Lander: Vec<AttachmentRef>
        Lander-->>RS: Vec<AttachmentRef>
        RS->>RS: MessageContent with attachment refs
    else attachments present, no lander wired
        RS-->>WebUI: 503 ServiceUnavailable
    else no attachments
        RS->>RS: text-only MessageContent
    end
    RS->>Thread: accept_inbound_message(message_content)
    Thread-->>WebUI: turn accepted

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related issues

• Universal attachments across all channels: wire the legacy attachment pipeline into v1 + extensible format registry + polished web UX #4644: Directly implements the "Reborn webui ingress accepts base64 inline uploads" and "landing in agent filesystem" requirements, including InboundAttachmentLander trait, ProjectScopedAttachmentLander, and 14 MiB body limit.

Possibly related PRs

• nearai/ironclaw#4670: ProjectScopedAttachmentLander calls land_inbound_attachments and uses InboundAttachment/AttachmentRef types introduced in that PR.
• nearai/ironclaw#4655: The attachment-inclusive MessageContent path in submit_turn depends on ThreadMessageRecord/MessageContent attachment-ref plumbing in that PR.
• nearai/ironclaw#4668: The ScopedFilesystem-backed landing routine consumed by ProjectScopedAttachmentLander was added in that PR.

Suggested reviewers

• serrrfirat
• abbyshekit

Poem

📎 Base64 bytes knock at the gate,
MIME checked, decoded, held straight.
The lander swoops down to the disk,
AttachmentRef sealed—no egress risk.
14 MiB bold, the body grows wide,
Attachments persist, the thread confides. 🗂️

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main feature: adding inline attachment upload support for WebChat v2's send path, following Conventional Commits style with type(scope): summary format.
Description check	✅ Passed	The description comprehensively covers scope, implementation flow, test coverage, validation steps, and stacking context, addressing all key template sections despite non-standard formatting.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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

[Copilot]

Pull request overview

Wires WebChat v2 inline attachment uploads through the Reborn WebUI v2 send-message path so uploaded bytes are decoded, budget-checked, landed into project storage via the workspace filesystem authority, and persisted as transcript attachment references.

Changes:

• Added attachments support to WebUiSendMessageRequest, including base64 decoding + MIME validation + decoded-byte budgeting.
• Introduced an InboundAttachmentLander port in ironclaw_product_workflow and a composition implementation that lands bytes via the project-scoped ScopedFilesystem.
• Increased WebUI v2 send_message request body limit to 14 MiB and updated descriptor/body-limit contract tests + docs accordingly.

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
crates/ironclaw_webui_v2/tests/webui_v2_descriptors_contract.rs	Updates contract test to expect 14 MiB send_message body limit.
crates/ironclaw_webui_v2/src/descriptors.rs	Raises send_message descriptor body limit to 14 MiB with updated rationale.
crates/ironclaw_reborn_composition/src/webui.rs	Wires the inbound attachment lander into the WebUI services when a local runtime filesystem is available.
crates/ironclaw_reborn_composition/src/webui_body_limit.rs	Updates enforcement docs/tests for the new 14 MiB descriptor limit.
crates/ironclaw_reborn_composition/src/runtime.rs	Exposes the workspace scoped filesystem for WebUI attachment landing; updates test request DTOs to include empty attachments.
crates/ironclaw_reborn_composition/src/local_dev_mounts.rs	Makes WORKSPACE_ALIAS visible within the crate for reuse by attachment landing.
crates/ironclaw_reborn_composition/src/lib.rs	Registers the new attachment_landing module.
crates/ironclaw_reborn_composition/src/attachment_landing.rs	Implements InboundAttachmentLander using land_inbound_attachments via the workspace ScopedFilesystem.
crates/ironclaw_reborn_composition/CLAUDE.md	Updates composition documentation to reflect the 14 MiB send_message body limit.
crates/ironclaw_reborn_composition/Cargo.toml	Adds dependency on ironclaw_attachments for landing support.
crates/ironclaw_product_workflow/tests/webui_inbound_contract.rs	Adds decode/budget/MIME/base64 tests for inline attachments.
crates/ironclaw_product_workflow/tests/reborn_services_contract.rs	Adds facade contract tests covering landing + transcript persistence and the “no lander wired” rejection behavior.
crates/ironclaw_product_workflow/src/webui_inbound.rs	Adds WebUiInboundAttachment, attachments field, and decode_attachments() decode/validation/budget logic.
crates/ironclaw_product_workflow/src/reborn_services/error.rs	Adds a public RebornServicesError::internal() constructor for external port impls.
crates/ironclaw_product_workflow/src/reborn_services.rs	Adds InboundAttachmentLander port + wiring in submit_turn to land bytes and persist attachment refs.
crates/ironclaw_product_workflow/src/lib.rs	Re-exports WebUiInboundAttachment and InboundAttachmentLander.
crates/ironclaw_product_workflow/Cargo.toml	Adds base64 and ironclaw_attachments dependencies for decoding + landing.
Cargo.lock	Updates lockfile for the added dependencies.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ilblackdragon]

Fixed — replaced the local normalize_attachment_mime with ironclaw_common::normalize_mime_type (identical split(';').next().trim().to_ascii_lowercase() logic, but now the single canonical normalizer the registry / kind inference / transcription already route through, so no drift). Good catch.

[ilblackdragon]

Fixed the comment — it overclaimed determinism. The external_event_id is the path's message segment (stable across retries), but the lander also partitions by UTC day (Utc::now()), so a retry that crosses midnight UTC lands under the new day's directory, leaving the earlier bytes addressable-but-unreferenced. The comment now says exactly that, and notes idempotency is enforced at message acceptance (external_event_id dedup in the thread store), not by the storage path — so a cross-day retry is a dangling-bytes case, not a correctness bug.

[coderabbitai]

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (1)

crates/ironclaw_product_workflow/src/reborn_services.rs (1)

1647-1737: ⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

Add a pre-storage safety scan in the new attachment ingress lane.

submit_turn now accepts user attachment payloads and persists them via lander.land(...), but this path has no explicit safety_layer.* scan before workspace/storage write.

Suggested direction

 pub struct RebornServices {
     thread_service: Arc<dyn SessionThreadService>,
     turn_coordinator: Arc<dyn TurnCoordinator>,
     inbound_attachments: Option<Arc<dyn InboundAttachmentLander>>,
+    inbound_attachment_scanner: Option<Arc<dyn InboundAttachmentScanner>>,
     ...
 }

+#[async_trait]
+pub trait InboundAttachmentScanner: Send + Sync {
+    async fn scan_pre_storage(
+        &self,
+        scope: &ThreadScope,
+        attachments: &[InboundAttachment],
+    ) -> Result<(), RebornServicesError>;
+}

 ...
         let message_content = if attachments.is_empty() {
             MessageContent::text(content.clone())
         } else {
+            let scanner = self
+                .inbound_attachment_scanner
+                .as_ref()
+                .ok_or_else(|| RebornServicesError::service_unavailable(false))?;
+            scanner.scan_pre_storage(&thread_scope, &attachments).await?;
             let lander = self
                 .inbound_attachments
                 .as_ref()
                 .ok_or_else(|| RebornServicesError::service_unavailable(false))?;
             let refs = lander
                 .land(&thread_scope, &external_event_id, attachments)
                 .await?;
             MessageContent::with_attachments(content.clone(), refs)
         };

As per coding guidelines, "Every new ingress point ... must run the matching safety scan on the pre-transform, pre-injection payload" and "Memory and workspace writes must apply injection scan on the pre-storage value, never on the transformed or rendered value."

🤖 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_product_workflow/src/reborn_services.rs` around lines 1647 -
1737, The new attachment ingress path in the submit_turn method accepts user
attachment payloads via request.decode_attachments() and persists them via
lander.land() without running a safety scan. Before passing the attachments
variable to lander.land(), apply the appropriate safety_layer injection scan on
the pre-storage attachments payload to comply with the coding guidelines that
require every new ingress point to run matching safety scans on pre-transform
payloads and that memory/workspace writes must apply injection scans on
pre-storage values. Add the safety scan call immediately before the
lander.land() invocation where the message_content is being constructed with
attachments.

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_product_workflow/src/reborn_services.rs`:
- Around line 1286-1291: `InboundAttachmentLander::land` currently accepts
`message_id` as a raw `&str`, which should be replaced with a strong identifier
type at this public boundary. Update the `land` signature and all call sites to
use a dedicated newtype or existing identifier type such as `IdempotencyKey`,
and keep the typed identifier flowing through any related reborn service methods
instead of converting back to string.

In `@crates/ironclaw_product_workflow/src/webui_inbound.rs`:
- Around line 125-195: The decode_attachments method accepts and validates
user-controlled attachment payloads but does not perform a required safety_layer
scan on the pre-transform attachment data before returning them for downstream
storage. Add a call to the appropriate safety_layer scanning function on the
decoded attachment bytes or the original attachment payload before the method
returns the decoded attachments vector. Handle any safety scan errors by
converting them to WebUiInboundValidationError instances consistent with the
existing validation error handling pattern in the method, ensuring the error
includes an appropriate field identifier and validation code.

---

Outside diff comments:
In `@crates/ironclaw_product_workflow/src/reborn_services.rs`:
- Around line 1647-1737: The new attachment ingress path in the submit_turn
method accepts user attachment payloads via request.decode_attachments() and
persists them via lander.land() without running a safety scan. Before passing
the attachments variable to lander.land(), apply the appropriate safety_layer
injection scan on the pre-storage attachments payload to comply with the coding
guidelines that require every new ingress point to run matching safety scans on
pre-transform payloads and that memory/workspace writes must apply injection
scans on pre-storage values. Add the safety scan call immediately before the
lander.land() invocation where the message_content is being constructed with
attachments.

🪄 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: 48d690c4-edd9-4f09-8ad8-9d199f3bd42a

📥 Commits

Reviewing files that changed from the base of the PR and between faf182e and 3da460d.

⛔ Files ignored due to path filters (1)

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

📒 Files selected for processing (17)

• crates/ironclaw_product_workflow/Cargo.toml
• crates/ironclaw_product_workflow/src/lib.rs
• crates/ironclaw_product_workflow/src/reborn_services.rs
• crates/ironclaw_product_workflow/src/reborn_services/error.rs
• crates/ironclaw_product_workflow/src/webui_inbound.rs
• crates/ironclaw_product_workflow/tests/reborn_services_contract.rs
• crates/ironclaw_product_workflow/tests/webui_inbound_contract.rs
• crates/ironclaw_reborn_composition/CLAUDE.md
• crates/ironclaw_reborn_composition/Cargo.toml
• crates/ironclaw_reborn_composition/src/attachment_landing.rs
• crates/ironclaw_reborn_composition/src/lib.rs
• crates/ironclaw_reborn_composition/src/local_dev_mounts.rs
• crates/ironclaw_reborn_composition/src/runtime.rs
• crates/ironclaw_reborn_composition/src/webui.rs
• crates/ironclaw_reborn_composition/src/webui_body_limit.rs
• crates/ironclaw_webui_v2/src/descriptors.rs
• crates/ironclaw_webui_v2/tests/webui_v2_descriptors_contract.rs

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

crates/ironclaw_product_workflow/src/reborn_services.rs (1)

1722-1739: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Do not reuse a pre-accept landing key that ignores attachment content.

land() runs before accept_inbound_message() establishes the idempotent winner, but the landed path is reused for same-day retries by external_event_id. Two same-day submissions with the same client_action_id and different attachment bytes can overwrite the same files, so the accepted transcript refs end up pointing at whichever write won last. The same ordering also leaves orphaned files whenever landing succeeds and accept_inbound_message() fails afterward. Please either land behind a durable accepted-message id, or make the landed key/content-addressing reject mismatched replays before any overwrite.

As per coding guidelines, "A cache storing values dependent on input X must include a stable representation of X in the cache key; if get_or_create(a, b) uses only 'a' as key but 'b' affects the stored value, that is a bug."

🤖 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_product_workflow/src/reborn_services.rs` around lines 1722 -
1739, The attachment landing path in reborn_services::land should not key
storage only by external_event_id before accept_inbound_message() chooses the
durable winner. Update the flow so landing is tied to the accepted message
identity (or another content-addressed key) and rejects replay attempts whose
attachment bytes differ from the already-landed content; this prevents same-day
retries from overwriting files and avoids orphaned uploads when acceptance later
fails. Use the existing land() and accept_inbound_message() sequence in
reborn_services.rs as the place to introduce the stronger keying/check.

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.

Outside diff comments:
In `@crates/ironclaw_product_workflow/src/reborn_services.rs`:
- Around line 1722-1739: The attachment landing path in reborn_services::land
should not key storage only by external_event_id before accept_inbound_message()
chooses the durable winner. Update the flow so landing is tied to the accepted
message identity (or another content-addressed key) and rejects replay attempts
whose attachment bytes differ from the already-landed content; this prevents
same-day retries from overwriting files and avoids orphaned uploads when
acceptance later fails. Use the existing land() and accept_inbound_message()
sequence in reborn_services.rs as the place to introduce the stronger
keying/check.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 36260395-abce-4037-ab24-5a4d95e6bdd9

📥 Commits

Reviewing files that changed from the base of the PR and between 3da460d and 0005893.

📒 Files selected for processing (2)

• crates/ironclaw_product_workflow/src/reborn_services.rs
• crates/ironclaw_product_workflow/src/webui_inbound.rs

[ilblackdragon]

Re: CodeRabbit — "Do not reuse a pre-accept landing key that ignores attachment content" (outside-diff, reborn_services.rs:1722-1739)

Verified against the full function; the described overwrite-of-accepted-content is not reachable, because the outside-diff view missed two guards above land()/accept():

1. Per-thread operation lock — submit_turn holds lock_thread_operation(&scope) (a per-thread_operation_key(scope) async mutex) for the whole critical section, serializing every submit on the same thread. Retries of one send share a thread, so they cannot run land() concurrently.

2. Replay short-circuit before land() — the branch containing land() is the else of replay_webui_send_message(external_event_id). Once a message is accepted under an external_event_id (which equals client_action_id), every later same-id submission takes the replay branch and never re-lands.

So under the lock, submission A lands+accepts (recording the id) before B acquires it; B then hits the replay branch and returns A's accepted message without re-landing. An accepted message's bytes are written exactly once, in the same locked section that accepts them — there is no "last write wins" mismatch against an accepted ref.

Genuine residual: orphaned files when land() succeeds and accept() fails afterward — the dangling-bytes case the new comment already documents. Benign: the bytes are referenced by no message, and an honest retry re-lands + accepts consistently.

Narrow theoretical gap: a cross-thread path collision requires a client to reuse one client_action_id across two different threads of the same user/project (plus same date/filename/index) — a double idempotency-key contract violation. The robust fix (key the storage path on the durable accepted message-id rather than the pre-accept external_event_id) means reordering land/accept, and is deferred rather than bolted on for that narrow case.