feat(attachments): mirror outbox layout in inbox + richer transaction logging

[kitplummer]

Surfaced from a live two-node run: the receive log carried only distribution_id/blob_hash/size — no filename, no post-write validation.

• Receive (inbox sink): info log with filename, bytes, blob_hash, target path; validates the on-disk copy size against the distribution's blob_size before the tmp→target rename. On mismatch, drops the partial + returns Err so the watcher retries (no short file published). Content integrity is already iroh-content-verified on fetch — this is the local-write completeness gate.
• Send (outbox watcher): auto-distribute log now includes bytes + sha256 alongside filename + distribution_id.

Build + 59 attachment unit tests green.

[peat-bot]

Peat QA Review (SHA: 18ebdfd)

No findings.

Scope check: change is confined to src/attachments/inbox.rs, src/attachments/outbox.rs, and CHANGELOG.md. None of the gated surfaces are touched — no proto/sidecar.proto, no Connect RPC additions, no src/crypto.rs, no peat-mesh pin bump, no chart/peat-node/ or zarf.yaml, no src/watcher.rs, no non-Tokio runtime, no new cross-repo imports.

Receive-side post-write validation is correctly placed at the filesystem inbox sink (local-write completeness gate), not at the transport layer — content-hash verification remains upstream in iroh, as the inline comment notes. The Err-return-on-mismatch path relies on peat_protocol::storage::ReceiveSink::deliver's documented retry contract (the receive watcher owns retry per the module header), which matches the change's intent of "drop the partial, let the watcher retry" rather than publishing a short file.

Idiomatic Rust: anyhow::bail! for the failure path, let _ = tokio::fs::remove_file(&tmp).await for best-effort cleanup of the partial (correct — the bail is the load-bearing signal), no unwrap()/clone() overuse, no secret material in the new log fields (distribution_id, filename, bytes, blob_hash, sha256 hex are all non-sensitive transfer metadata). hex is already a direct dependency, so hex::encode(&sha256) adds no new crate.

Log-level lift from debug! → info! on the receive path is per-distribution (not per-byte / per-chunk), so the operator-visibility win does not produce a meaningful log-volume regression at sidecar scale.

[peat-bot]

Peat QA Review (SHA: b1e1b23)

Incremental review since 18ebdfd.

No findings.

Scope check: delta is confined to build.rs, src/main.rs, and CHANGELOG.md. None of the gated surfaces are touched — no proto/sidecar.proto, no Connect RPC additions, no src/crypto.rs, no peat-mesh pin bump (only a build-time read of the resolved version from Cargo.lock for telemetry), no chart/peat-node/ or zarf.yaml, no src/watcher.rs, no non-Tokio runtime, no new cross-repo imports.

Redaction completeness on --print-config: the two secret-bearing Args fields — shared_key and encryption_key — are both rewritten on the cloned struct before the info!("…{redacted:#?}") formatter runs. The Command::DeriveId arm also carries a shared_key, but its if let Some(Command::DeriveId { … }) = &args.command short-circuit returns before the print_config block, so the subcommand's shared key cannot leak via this path. TLS fields (agent_tls_cert/_key/_ca) are PathBuf, not contents. The flag is opt-in (default_value = "false") and gated behind both --print-config and PEAT_NODE_PRINT_CONFIG — operator-conscious.

build.rs::locked_version: the name = "<crate>" anchor matches only [[package]] headers — Cargo.lock's dependencies array uses bare string entries ("peat-mesh"), not name = …, so cross-section matches are not possible. unwrap_or_default() on read_to_string keeps the build resilient if Cargo.lock is unreadable; missing crates resolve to "unknown" rather than panicking. cargo:rerun-if-changed=Cargo.lock correctly invalidates the captured env vars on dependency bumps. Build-time only — no runtime exposure.

Idiomatic Rust: env!() macros for compile-time captures, args.clone()-before-mutating-redaction is the natural pattern for a Debug snapshot, no unwrap() on user-reachable inputs, if let Some(idx) = … / if let Some(line) = … rather than .unwrap() chains. Log-volume cost is one extra info! per process start (banner) plus the opt-in dump — no per-request impact.

Prior review (18ebdfd) was "No findings" — nothing to re-verify.

[peat-bot]

Peat QA Review (SHA: 1fededf)

Incremental review since b1e1b23.

No findings.

Scope check: the single new commit (fix(attachments): write inbox files at original relative path) is confined to src/attachments/inbox.rs and src/attachments/ingest.rs. None of the gated surfaces are touched — no proto/sidecar.proto, no Connect RPC additions or signature changes, no src/crypto.rs, no peat-mesh pin bump, no chart/peat-node/ or zarf.yaml, no src/watcher.rs TLS paths, no non-Tokio runtime, no new cross-repo imports.

Path-traversal guard (inbox_relpath): only Component::Normal is accepted; ParentDir, RootDir, and Prefix all short-circuit to None, and the caller falls back to a flat <distribution_id>.bin at the inbox root — inbox_root.join(...) cannot escape because no .. or absolute components ever reach it. CurDir is silently dropped, and an all-./ name folds to an empty PathBuf which is filtered to None. The unit tests cover the three meaningful traversal vectors (../../etc/passwd, /etc/passwd, a/../../b), the empty/missing/./ fallback cases, and the end-to-end deliver_traversal_name_stays_inside_inbox integration check that confirms a hostile ../../../../tmp/pwned lands at inbox/dist-evil.bin.

Post-write validation tightened from "trust the tokio::fs::copy byte-count" to a true metadata(&tmp).await?.len() re-read before the publishing rename; on mismatch the partial is removed and the watcher retries. The size check is the only completeness gate needed here — iroh content-verifies the bytes on fetch.

Behavioral change to flag for awareness, not as a finding: re-delivery is now latest-wins on the mirrored path (previously namespaced under {distribution_id}/). The PR documents this explicitly and adds an idempotency assertion that the partial marker is cleaned up on overwrite.

Test coverage tracks the surface change: the removed inbox_filename_* tests are replaced by relpath_* tests at the new boundary, and the already_delivered_* tests now key on the mirrored path. Subdirectory creation via create_dir_all is exercised by deliver_mirrors_relative_path_and_is_idempotent.

Idiomatic Rust: Component match with explicit reject-on-non-Normal is the standard Rust path-sanitisation pattern, unwrap_or_else(|| ...) for the fallback PathBuf is correct (lazy allocation), and as_deref()? / ? propagation in inbox_relpath is clean. No unwrap() chains on user-reachable inputs.

Prior review (b1e1b23) was "No findings" — nothing to re-verify.

[peat-bot]

Peat QA Review (SHA: 409a546)

Incremental review since 1fededf.

No findings.

Scope check: the single new commit (test(attachments): assert mirrored inbox layout (no distribution_id dir)) is confined to tests/attachments_e2e_test.rs and test/attachment-delivery-compose.sh — both test-only. No gated surfaces touched: no proto/sidecar.proto, no Connect RPC additions or signature changes, no src/crypto.rs, no peat-mesh pin bump, no chart/peat-node/ or zarf.yaml, no src/watcher.rs TLS paths, no non-Tokio runtime, no cross-repo imports.

Test alignment is correct for the inbox layout change shipped in 1fededf: attachments now land at inbox/<relative_path> (no distribution_id wrapper dir), so the prior await_inbox_file(inbox, distribution_id, expected, deadline) signature and the bash poll path $WORK/inbox/$DIST/payload.bin were stale. The new helper drops the distribution_id parameter and walks the inbox tree by content via find_matching_file, which is the right shape now that the on-disk path no longer encodes the distribution id.

find_matching_file is sound: synchronous std::fs is fine in a test helper (explicitly justified in the doc comment), the stack-based walk avoids recursion bounds concerns, .flatten() on read_dir entries skips unreadable entries cleanly, and dotfile-prefix filtering preserves the in-flight .partial marker exclusion. Byte-exact equality (actual == expected) is a strictly stronger signal than the prior size-match heuristic.

Negative-case tightening in node_list_scope_only_delivers_to_listed_nodes: the assertion that C must NOT receive is now a byte-exact scan over C's entire inbox tree rather than a size check inside the (no-longer-existing) dist_dir. Stronger signal, correct shape post-layout-change. The change also preserves the contract-shape assertion that SendAttachments returns a distributionId even though the value is no longer needed for the on-disk lookup — appropriate.

Idiomatic Rust: let Ok(...) = ... else { continue }, let Some(found) = ... early-return, .is_some_and for the dotfile predicate, and entries.flatten() are all standard 2024-edition patterns. No unwrap() on user-reachable inputs; no clone-fights with the borrow checker.

Surface-tier coverage unchanged on the Connect RPC layer (no new RPCs, no signature changes). The e2e file exercises SendAttachments / GetAttachmentDistribution end-to-end across two nodes as before; only the on-disk verification path was updated.

PR-description claim ("all 4 two-node e2e tests pass, and the compose delivery script passes against the fix image") is consistent with the diff — the test helper is the only mechanical block that needed updating for the layout change.

Prior review (1fededf) was "No findings" — nothing to re-verify.