feat(attachments): outbox watcher — bidirectional hands-off file sync#167
Conversation
…a root Symmetric counterpart to the receive-side inbox watcher: opt-in PEAT_NODE_ATTACHMENT_OUTBOX_WATCH polls the configured --attachment-root dirs and auto-distributes any stable, not-yet-sent file (AllNodes scope, default_priority) by synthesising the same SendAttachments request an app would send — no gRPC call. Drop a file in the outbox and it lands in every peer's inbox, the send-side half of "drop file -> syncs to peers -> written + verified." - src/attachments/outbox.rs: poll loop reusing handlers::send_attachments; pure stability/dedup decision (Send/Wait/Skip) is unit-tested; recursive walk skips dotfiles; stream-hashes sha256. - AttachmentConfig gains outbox_watch + outbox_poll_secs (default 2s); PEAT_NODE_ATTACHMENT_OUTBOX_WATCH / _POLL_SECS flags. Spawned in main after node construction (drives the gRPC path). Off by default; requires a root. Verified live: two deterministic-identity nodes, drop a 512 KiB file in the sender outbox (no SendAttachments) -> appears byte-identical in the receiver inbox in ~3s.
- test/attachment-outbox-watch-compose.sh + CI step: drop a file into the sender's outbox (no SendAttachments) and assert it lands byte-identical in the receiver inbox via the outbox watcher. Verified locally (PASS). - multi-host example + attachments README: document the hands-off PEAT_NODE_ATTACHMENT_OUTBOX_WATCH synced-folder mode. - CHANGELOG [Unreleased]: outbox watcher entry.
peat-bot
left a comment
There was a problem hiding this comment.
Peat QA Review (SHA: 2337716)
Opt-in send-side outbox watcher (PEAT_NODE_ATTACHMENT_OUTBOX_WATCH) that polls --attachment-root and auto-distributes stable new files via the existing handlers::send_attachments path. No proto/wire changes, no new RPCs, no Cargo.toml bump, no chart//zarf.yaml/src/watcher.rs/src/crypto.rs changes — pure Rust + Tokio, only depends on peat-mesh. The new functest (test/attachment-outbox-watch-compose.sh) exercises the end-to-end path with sha256 byte-identity assertion and is wired into the attachment-delivery.yml CI job.
[WARNING] Unbounded growth of watcher state maps when outbox files are deleted
src/attachments/outbox.rs:155-167 — last_seen and last_sent are HashMap<PathBuf, (u64, SystemTime)> accumulated for the lifetime of the watcher. Each poll iteration only ever calls .insert(...) for files currently found on disk; entries for files removed from the outbox root remain forever. In a long-running sidecar with steady outbox churn (the most natural "synced folder" usage pattern: drop, distribute, delete) the maps grow without bound. The rest of the attachments subsystem already bounds similar state via max_known_bundles — this watcher is the lone exception.
Action: at the end of each poll cycle, prune entries whose PathBuf was not observed by walk_files this iteration (e.g., collect the current set, retain only those keys). One-line fix. Not blocking — even at 1000 files/day churn the growth is ~36 MB/year — but worth fixing before this ships in 0.4.5.
Notes (non-findings, for context)
- No new RPCs on
PeatSidecar— the watcher drives the existinghandlers::send_attachmentsinternally, so the "new RPC ⇒tests/grpc_test.rsintegration test" rule does not trigger. The end-to-end functest covers the new behaviour. - The
Errbranch atoutbox.rs:188-195deliberately marks the version as sent so transient failures don't re-attempt every poll; the user must touch the file (bumping mtime) to retry. This is documented in the inline comment and matches the explicit-RPC semantics (a vacuous COMPLETED with zero peers, etc.). Acceptable as designed — flagged here only so reviewers know the trade-off is intentional, not a regression. - The compose functest mounts the sender outbox
:roand writes from the host, sidestepping any race between watcher polls and partial writes; combined with theWait-then-Sendstability check this is sound.
`last_seen`/`last_sent` only ever grew — entries for files deleted from the outbox were never removed, so the drop→distribute→delete synced-folder pattern leaked memory unbounded. Each poll now collects the observed paths and prunes both maps to that set via `prune_to_observed` (unit-tested).
peat-bot
left a comment
There was a problem hiding this comment.
Peat QA Review (SHA: 603abc3)
Incremental review since 2337716.
Delta: a single targeted fix for the prior [WARNING] on unbounded watcher state-map growth (src/attachments/outbox.rs). Adds a prune_to_observed helper, collects a HashSet<PathBuf> of paths seen during each poll cycle, and prunes both last_seen and last_sent to that set at the end of the cycle. New unit test prune_drops_vanished_files covers the helper. No proto changes, no new RPCs, no Cargo.toml/chart/Zarf/crypto/watcher-TLS changes, no language or runtime additions, still Tokio-only, still depends on peat-mesh only.
Prior findings
- Resolved: [WARNING] Unbounded growth of watcher state maps when outbox files are deleted —
src/attachments/outbox.rs:168-172, 213-216now collect anobserved: HashSet<PathBuf>during the poll'swalk_filestraversal and callprune_to_observedon bothlast_seenandlast_sentafter the cycle. Implementation uses idiomaticHashMap::retain, matches the bounding pattern used elsewhere in the attachments subsystem, and is covered by the newprune_drops_vanished_filesunit test (src/attachments/outbox.rs:270-280).
New findings
No findings.
`node_list_scope_only_delivers_to_listed_nodes` (and the other iroh_two_node serial tests) timed out at await_inbox_file's 30s budget on contended CI runners — the documented SKILL gotcha: spinning up multiple two-node iroh meshes under CPU contention stalls delivery past a tight budget. Delivery is <3s unloaded; 60s gives headroom without masking a real failure (a timeout at 60s is a genuine delivery bug). Not related to the outbox watcher (off by default in these tests); pre-existing flake surfaced by this PR's CI runs.
peat-bot
left a comment
There was a problem hiding this comment.
Peat QA Review (SHA: 23bccb4)
Incremental review since 603abc3.
Delta: test-only. tests/attachments_e2e_test.rs widens four await_inbox_file deadlines from 30s to 60s on the iroh two-node serial tests, and the await_inbox_file doc-comment is rewritten to explain the CI-contention rationale (peat-node SKILL gotcha — multiple two-node iroh meshes under CPU contention stall past a tight budget; <3s unloaded, 60s preserves a real-failure signal). No proto changes, no new/changed RPCs, no Cargo.toml/peat-mesh-pin changes, no chart/Zarf changes, no src/crypto.rs changes, no src/watcher.rs TLS changes, no language or runtime additions, still Tokio-only, still depends on peat-mesh only. Production code paths (src/attachments/outbox.rs pruning, the outbox watcher itself) are untouched.
Prior findings
- Resolved (still resolved): [WARNING] Unbounded growth of watcher state maps — fixed in 603abc3 via
prune_to_observedinsrc/attachments/outbox.rs. This delta does not touch that file; the bounding remains in place and covered byprune_drops_vanished_files.
New findings
No findings.
Completes the "watcher on both sides" model you asked for. The receive-side inbox watcher already existed (auto-fetch + write + integrity-verify); this adds the missing send-side outbox watcher, so a file dropped in a node's outbox auto-syncs to every peer's inbox with no
SendAttachmentsgRPC call.Why
--attachment-rootwas only an allowlist of readable dirs; distribution required an explicitSendAttachmentscall (confirmed: no source watcher existed). So the send side was manual while the receive side was automatic. This closes that asymmetry.What
PEAT_NODE_ATTACHMENT_OUTBOX_WATCH=true(opt-in;..._POLL_SECS, default 2s). Polls the configured roots; auto-distributes any stable (unchanged across a poll), not-yet-sent file via the samehandlers::send_attachmentspath (validate → ingest → sha256-verify → distribute → registry), scopeAllNodes, default priority. Idempotent.Send/Wait/Skip) is unit-tested; recursive walk skips dotfiles; sha256 is stream-hashed. Off by default (explicit RPC stays the safe default).Verified
cargo test --workspacegreen (the lone parallel-run failure is the knownsync_subprocesscontention flake — passes isolated).test/attachment-outbox-watch-compose.sh(+ CI step): same, in containers — PASS.Ships in the next release (0.4.5); the example/README note that.