Feature/p2p sync#143
Open
diegorv wants to merge 29 commits into
Open
Conversation
Context: The P2P vault sync design spec was just written and committed (39a440f). A self-review pass found two under-specified points. Problem: (1) Decision-table case 3 (only remote changed) did not state its effect on the persisted sync state, unlike cases 1 and 5. (2) The protocol section did not define what happens when a locally subscribed folder is no longer present in the peer's Shares list. Solution: State the case-3 state transition explicitly (synced = seen_remote = remote) and add a protocol rule: a session pulls only the intersection of local subscriptions and the peer's current Shares; dropped folders are skipped, reported in the summary, and never deleted locally. Behavior: Documentation only; no code changes. Files: - docs/superpowers/specs/2026-07-03-p2p-sync-design.md (decision table row 3; protocol bullet list) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: The P2P vault sync design was approved and specced in docs/superpowers/specs/2026-07-03-p2p-sync-design.md. Implementation needs a task-by-task plan with real code, tests, and commit gates. Solution: Add the full implementation plan (13 tasks, TDD steps with complete code for each) and the tasks/todo checklist file required by the Plan Mode Workflow. The plan pins verified dependency versions (snow 0.10.0, getrandom 0.4.3, serde_bytes 0.11.19), grounds every edit point in existing files (SettingsPanel registration, DEFAULT_SETTINGS, app-lifecycle init/teardown), and covers the spec's whole test matrix, including wrong-PSK handshake failure and hash-mismatch downloads writing nothing. Behavior: Documentation only; no code changes. Files: - docs/superpowers/plans/2026-07-03-p2p-sync.md (new; full plan) - tasks/todo/p2p-sync.md (new; task checklist per Plan Mode Workflow) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…plan Context: Pre-flight plan review for subagent execution flagged the Task 8 test local_lan_ip_does_not_panic as an assert-nothing test, which the review rubric treats as a defect. User chose to replace it. Solution: The test now asserts that, when local_lan_ip() returns Some, the string parses as std::net::IpAddr. None remains accepted so the test cannot flake on machines without a network route. Behavior: Documentation only; no code changes. Files: - docs/superpowers/plans/2026-07-03-p2p-sync.md (Task 8, step 1 test) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: P2P vault sync requires a wire protocol for message exchange between
peers over Noise-encrypted TCP. This is the foundation for all subsequent sync
tasks: message serialization, connection establishment, state management, and
the pull engine all depend on these protocol definitions and framing primitives.
Solution: Implemented the complete protocol layer as specified in
docs/superpowers/specs/2026-07-03-p2p-sync-design.md:
- Message types (Hello, HelloAck, ListShares, Shares, GetManifest, ManifestPage,
GetFile, FileChunk, FileEnd, Error, Bye) for all client-server interactions
- FileMeta struct (rel_path, size, sha256) for file metadata
- Constants: PROTOCOL_VERSION, MAX_FRAME_LEN, MAX_PLAINTEXT_LEN, FILE_CHUNK_LEN,
MANIFEST_PAGE_LEN
- MessagePack serialization (encode_msg, decode_msg) with serde_bytes support
- Length-prefixed framing (write_frame, read_frame) handling u32 LE length headers
with MAX_FRAME_LEN validation
- Added dependencies: snow (Noise protocol), getrandom, serde_bytes
- Extended tokio features: net, sync, time
Behavior: The sync module now exports all protocol primitives needed for:
- Message round-tripping through serialization (4 variants tested)
- Frame I/O over async streams with length validation
- Rejection of oversized frames and invalid headers
Tests verify correctness of encoding/decoding, framing, and boundary conditions.
Files:
- src-tauri/Cargo.toml:38-43: Added P2P sync dependencies (snow, getrandom,
serde_bytes) and extended tokio features (net, sync, time)
- src-tauri/src/lib.rs:6: Added pub mod sync after pub mod semantic
- src-tauri/src/sync/mod.rs:1-7: New module root with protocol submodule
- src-tauri/src/sync/protocol.rs:1-145: Full wire protocol implementation:
- 1-26: Module docstring and imports
- 27-35: Constants (PROTOCOL_VERSION, MAX_FRAME_LEN, MAX_PLAINTEXT_LEN,
FILE_CHUNK_LEN, MANIFEST_PAGE_LEN)
- 36-44: FileMeta struct with serde derive
- 45-69: Msg enum with 11 variants for sync session lifecycle
- 71-76: encode_msg using rmp_serde with named fields
- 78-80: decode_msg using rmp_serde
- 82-90: write_frame with length prefix and validation
- 92-104: read_frame with length validation and rejection
- 106-145: 4 unit tests covering message round-tripping, frame I/O,
oversized frame handling, and invalid header rejection
- tasks/todo/p2p-sync.md:10: Marked Task 1 complete
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: Task 2 of the P2P vault sync implementation. The sync protocol (from Task 1) provides message framing and serialization; this task adds the encryption layer to protect channel confidentiality and enforce pre-shared key (pairing key) authentication. Problem: No way to generate or parse pairing keys, no encrypted channel implementation, and no handshake to establish a Noise XXpsk3 session between sync initiator and responder. Solution: Implemented noise.rs with: - generate_pairing_key(): generates 32 random bytes, returns as 64 lowercase hex - parse_pairing_key(hex): parses hex string to 32-byte array, trims whitespace - NoiseChannel<S>: wraps any AsyncRead+AsyncWrite, implements send()/recv() - handshake_initiator()/handshake_responder(): run XXpsk3 handshake with PSK Behavior: - Each pairing key is 64 lowercase hex chars (32 bytes random) - Parsing trims whitespace and rejects non-hex or wrong-length input - Handshake uses transient per-session keypairs; identity is PSK-only - Wrong PSK fails handshake before any application data flows - All wire data is indistinguishable from random (no metadata leakage) - Each Msg encrypts to one Noise message via snow 0.10 Test results: 5 tests PASS (pairing key generation/parsing, roundtrip trim, bad input rejection, bidirectional encrypted message exchange, PSK mismatch detection). Full Rust suite: 119 tests PASS, 0 FAIL, 2 IGNORED. Files: - src-tauri/src/sync/noise.rs (lines 1-177): implementation + 5 test cases - src-tauri/src/sync/mod.rs (line 8): add pub mod noise Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: The NoiseChannel::send() method has a guard that rejects plaintext messages larger than MAX_PLAINTEXT_LEN, but this rejection path was not being exercised by any test. Problem: The size-check guard at send() line 49-51 was never validated by the test suite, leaving uncertainty about whether the error handling works correctly in practice. Solution: Added a new test `oversized_message_is_rejected_before_send` that constructs a FileChunk message whose data exceeds MAX_PLAINTEXT_LEN by 1 byte, performs a valid handshake on both sides, and verifies that send() returns an error with the expected "too large" message substring. Behavior: The test verifies that NoiseChannel::send() rejects oversized messages with an error containing "too large" before attempting encryption. Previously, this code path was untested; now all six noise tests pass. Files: - src-tauri/src/sync/noise.rs:177-184: Added oversized_message_is_rejected_before_send() test that creates a FileChunk exceeding MAX_PLAINTEXT_LEN and verifies rejection Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: The P2P sync feature (feature/p2p-sync) requires persistent state tracking of which content hash both sides last agreed on, and the last remote hash observed. This prevents unnecessary re-downloads and enables intelligent conflict-copy dedup logic. Task 3 of the plan adds the JSON persistence layer. Solution: Created sync/state.rs with FileSyncState struct (synced and seen_remote Option<String> fields), SyncStateMap type alias (peer device name to file state mapping), and three core functions: state_file_path() computes the location at <vault>/.kokobrain/sync-state.json, load_state() reads the JSON with graceful fallback to empty map on missing or corrupt files, and save_state() persists the map as pretty-printed JSON. Three inline tests cover missing file (returns empty), save-then-load roundtrip, and corrupt file handling. All tests pass; full Rust suite verified (119 passed). Behavior: Sync state is now persisted across sessions. A lost state file only triggers a one-time re-download/conflict-copy pass; never causes data loss. The state format is human-readable JSON for easy inspection and debugging. Files: - src-tauri/src/sync/state.rs:1-82: New file with FileSyncState, SyncStateMap, state_file_path, load_state, save_state, and three inline tests - src-tauri/src/sync/mod.rs:9: Added pub mod state; declaration - tasks/todo/p2p-sync.md:12: Marked Task 3 [x] complete Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lder
Context:
Task 4 in the P2P Vault Sync plan implements the module shared by the sync
server (reads exposed folders) and engine (writes downloads). Consumed by
future sync/server.rs and sync/engine.rs tasks.
Problem:
The sync infrastructure needs path validation, file hashing, and folder walking
to build manifests for replication. Without this, the server cannot enumerate
files for peer discovery, and the engine cannot verify integrity.
Solution:
Added sync/manifest.rs with four exported functions:
- validate_rel_path: Rejects empty, absolute, escaped, or hidden paths.
- hash_bytes: SHA-256 of content (lowercase hex, matches history.rs).
- hash_file: Reads file and returns its hash.
- build_manifest: Walks folder, sorts by rel_path, skips dot-prefixed entries
and symlinks, builds a Vec<FileMeta> with size and sha256.
The module uses a depth-first stack walk with tempfile for test isolation.
All functions integrate with protocol::FileMeta to match the wire format.
Behavior:
- Manifest validation: rejects "" (empty), "/abs" (absolute), "a/../b" (escape),
".." (parent), "a//b" (empty component), ".hidden/x" (hidden), "a/.git/c"
(hidden subdir), "a\\b" (backslash), "C:/x" (Windows drive).
- Hashing: sha256("abc") → "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad"
(matches external sha256sum).
- Manifest walk: lists nested files sorted by rel_path, with `/` separators.
Skips dot-prefixed files and directories, symlinks, and non-UTF-8 names.
Preserves file size (u64) and sha256 hash for integrity tracking.
Tests (6 total, all passing):
- validate_rel_path_accepts_normal_paths: "Notes", "Notes/sub dir/a b.md"
- validate_rel_path_rejects_escapes_and_hidden: "" "/" ".." "../" "a//b" ".hidden" ".git" "\\" "C:"
- hash_bytes_matches_known_sha256: matches known test vector
- build_manifest_lists_nested_files_sorted_and_skips_hidden: tempdir with
Notes/{b.md,sub/a.md,.dotfile,.hidden/c.md} → lists only sorted regular files
- build_manifest_skips_symlinks: /unix only; symlinks not in manifest
- build_manifest_missing_folder_errors: nonexistent folder → Err
Files:
- src-tauri/src/sync/manifest.rs (new, 146 lines)
Lines 1-80: validate_rel_path, hash_bytes, hash_file, build_manifest (implementations)
Lines 87-146: test module with 6 test functions
- src-tauri/src/sync/mod.rs (modified, +1 line)
Line 10: pub mod manifest; declaration
- tasks/todo/p2p-sync.md (modified, 1 checkbox)
Line 13: Task 4 marked [x]
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nifest Context: Security and reliability review of the manifest building logic revealed that build_manifest correctly skips symlinked directories and non-UTF-8 filenames, but the test suite lacked coverage for these important edge cases. Problem: The build_manifest function checks for symlinks and non-UTF-8 names (lines 56 and 61-62), but the test suite only covered symlinked files, not symlinked directories. Additionally, there was no test for the non-UTF-8 name skip behavior, creating a gap in coverage for security-relevant path handling. Solution: Added two new tests to src-tauri/src/sync/manifest.rs: - build_manifest_skips_symlinked_directories: verifies that symlinks pointing to directories are skipped, preventing directory traversal via symlinks - build_manifest_skips_non_utf8_names: verifies that files with non-UTF-8 filenames are skipped, with conditional logic for filesystems (like APFS) that don't support non-UTF-8 names Behavior: The two new tests now verify that build_manifest correctly filters both symlinked directories and non-UTF-8 names. All 8 sync::manifest tests pass. No changes to the build_manifest implementation. Files: - src-tauri/src/sync/manifest.rs:141-169: Added two new test functions covering symlinked directories and non-UTF-8 filename handling Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: Task 5 of the P2P sync implementation adds pure conflict-resolution logic that determines what action the engine should take for each file during a sync pull: whether to download the remote version, keep the local version unchanged, or handle a conflict by saving both versions. Problem: The sync engine needs a decision table that implements the "local wins" semantics from the spec, plus a utility to generate conflict-copy file names with peer device name and date information. Solution: Implemented the Action enum (Download, UpToDate, KeepLocal, Conflict) and the decide() function that exhaustively handles all five spec table rows plus edge cases (first sync with differing local content, repeat sync with unchanged remote). Also implemented conflict_copy_rel_path() to generate sanitized conflict copy names that prevent path injection via malicious peer names. Behavior: decide() takes local content hash (or None if file missing), remote hash, and optional sync state, returning the appropriate Action. The function correctly handles: (1) missing local file → Download; (2) identical hashes → UpToDate; (3) only remote changed → Download; (4) only local changed → KeepLocal; (5) both changed → Conflict with write_copy flag based on whether this remote hash was already seen. conflict_copy_rel_path() transforms "Notes/a.md" with peer="Studio" and date="2026-07-03" into "Notes/a (conflict from Studio 2026-07-03).md", sanitizing the peer name so "/" "\\" and ":" become "-". Files changed: - src-tauri/src/sync/decision.rs:1-137 (new, Action enum + decide + conflict_copy_rel_path + 8 tests) - src-tauri/src/sync/mod.rs:11 (added pub mod decision;) - tasks/todo/p2p-sync.md:14 (marked Task 5 complete) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: Task 6 of the P2P vault sync plan (docs/superpowers/plans/2026-07-03-p2p-sync.md).
Tasks 1-5 built the wire protocol, Noise channel, persisted sync state,
manifest/hashing, and conflict decision table. This task adds the
listening side: the machine being synced FROM must expose a read-only
TCP endpoint that a peer can connect to, authenticate via the shared
pairing key, and pull files from.
Solution: Added `sync/server.rs` with `start_server(ServerConfig, port)`,
which binds a TCP listener (port 0 = ephemeral) and spawns an accept loop
on a background task. Each connection runs the Noise XXpsk3 responder
handshake, exchanges `Hello`/`HelloAck` (checking `PROTOCOL_VERSION`),
then serves `ListShares`, `GetManifest`, and `GetFile` requests strictly
against `config.exposed_folders` — folders that no longer exist on disk
are silently dropped from `ListShares`, and any path failing
`validate_rel_path` or resolving outside a folder in `exposed_folders`
returns `Msg::Error` instead of touching disk. `GetFile` additionally
canonicalizes the resolved path and checks it stays under the
canonicalized vault root (defense in depth against symlink escapes,
beyond what `validate_rel_path`'s `..`-rejection already covers).
Connections are served one at a time (sequential await inside the
accept loop) since exposing folders is a low-throughput, occasional
operation. Shutdown uses a `tokio::sync::watch<bool>` channel:
`RunningServer::stop(self)` sends `true` and consumes `self`, so the
`TcpListener` (owned by the spawned task, not by the handle) drops as
soon as the accept loop observes the change and returns, closing the
socket.
Behavior: `start_server` returns a `RunningServer { port, .. }` handle
immediately after binding; `port` reports the actual bound port so
callers requesting port 0 can read back what the OS assigned. Peers
that complete the Noise handshake with the correct pairing key can list
shares, page through a folder's manifest (paginated at
`MANIFEST_PAGE_LEN` entries per `ManifestPage`), and download files in
`FILE_CHUNK_LEN` chunks followed by a `FileEnd { sha256 }` whose hash
the caller can verify against the received bytes. Wrong pairing keys
fail the handshake before any `Msg` is exchanged. A protocol version
mismatch on `Hello` gets an `Error` reply mentioning "version" and ends
the session. Calling `RunningServer::stop()` closes the listening
socket; new connection attempts to that port fail immediately after.
Files:
- src-tauri/src/sync/server.rs:1-176: New file. `ServerConfig` (Clone),
`RunningServer` (port + `stop()`), `SyncServerState` (Tauri managed
state wrapper, unused until Task 8 wires the Tauri commands),
`start_server()` accept loop with watch-channel shutdown,
`serve_connection()` per-session message loop (`ListShares`,
`GetManifest` w/ pagination, `GetFile`, `Bye`), `serve_file()` with
exposed-folder + canonicalized-path checks.
- src-tauri/src/sync/mod.rs:12: Added `pub mod server;`.
- src-tauri/tests/sync_server_test.rs:1-159: New integration test file.
Covers: filtering missing exposed folders out of `ListShares`;
manifest pagination + multi-chunk file download with hash
verification + empty-file zero-chunk download; refusal of
unshared/traversal/absolute paths; handshake failure on wrong PSK;
protocol version mismatch producing an `Error` containing "version";
and `stop()` actually closing the listening socket.
- tasks/todo/p2p-sync.md:15: Marked Task 6 done.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ions
Context: The sync listener's accept loop ran serve_connection(...).await
inside the outer select! arm, so while a session was in flight rx.changed()
was never polled. Neither the Noise handshake nor NoiseChannel::recv had
a read timeout.
Problem: Any device that could reach the port could open a bare TCP
connection, send nothing, and permanently freeze the listener: stop()
became unresponsive because the shutdown watch channel was never observed
during an in-flight session, and no new peer could ever connect while the
stalled connection held the single session slot. Zero authentication was
required to trigger this — the handshake itself was one of the unbounded
reads.
Solution: Restructured the accept loop to race each session against the
shutdown signal via a nested tokio::select! — dropping the session future
on shutdown closes the socket immediately, so a stalled peer no longer
blocks stop(). Added HANDSHAKE_TIMEOUT (10s) and RECV_TIMEOUT (30s)
constants and wrapped every blocking read in serve_connection (the Noise
handshake, the initial Hello recv, and the per-request recv loop) in
tokio::time::timeout so a silent peer can hold the session for at most
RECV_TIMEOUT before being dropped. The request-loop recv already treated
disconnects as a normal end-of-session; timeouts now fold into that same
path so existing hung-up semantics are unchanged.
Behavior: Previously, a peer that connected and sent nothing froze the
listener indefinitely — stop() would never take effect and no other peer
could connect. Now, shutdown always takes effect within one select poll
even mid-session, and any peer that goes silent during the handshake or
between requests is dropped after its respective timeout, freeing the
session slot for the next connection.
Files:
- src-tauri/src/sync/server.rs:8-21: Added tokio::time::{timeout, Duration}
import and HANDSHAKE_TIMEOUT (10s) / RECV_TIMEOUT (30s) constants
- src-tauri/src/sync/server.rs:68-82: Restructured the accept loop's
connection branch to race serve_connection against rx.changed() in a
nested select!, so shutdown is observed even while a session is active
- src-tauri/src/sync/server.rs:90-96: Wrapped the Noise handshake and the
initial Hello recv in timeout(), converting Elapsed into a session error
- src-tauri/src/sync/server.rs:114-117: Wrapped the per-request recv in
timeout(), folding timeout into the existing "peer hung up" early return
- src-tauri/tests/sync_server_test.rs:161-171: Added
stop_unblocks_while_a_session_is_stalled, which opens a raw TCP
connection that never sends bytes (stalling the handshake) and asserts
stop() still closes the listener promptly
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: Tasks 1-6 built the wire protocol, Noise-encrypted channel, persisted sync state, manifest/hashing, decision table, and read-only listener for P2P vault sync. This is the final piece of the sync core: the client side that actually connects to a peer and pulls files into the local vault. It is the only sync code that writes into the vault - the listener (server.rs) is strictly read-only. Solution: Added `sync/engine.rs` with `PeerTarget` (address, pairing key, local device name), `list_remote_shares` (connect, ListShares, Bye), and `run_sync` (full session: connect, ListShares, then per subscribed folder fetch the paginated manifest, diff against local hashes via `build_manifest`, and apply `decision::decide` per file). Downloads and conflict copies both go through `download_file`, which requests the file, accumulates `FileChunk`s bounded by a 1 GiB cap, and verifies the peer-reported `FileEnd` hash against the actual bytes before calling `write_atomic` - never the manifest hash, since the file can change between the manifest fetch and the file fetch. Verified bytes are written to a dot-prefixed temp file in the destination directory and renamed into place, so a crash or corrupted transfer never leaves a partial file in the vault. Per-file and per-folder failures are collected into `SyncSummary.errors` and the session keeps going rather than aborting the whole sync. Subscribed folders the peer no longer exposes are reported via `skipped_folders` instead of erroring. Session-level failures (unreachable peer, wrong pairing key, protocol mismatch) short-circuit `run_sync` with a descriptive error since there is nothing to sync without a channel. Behavior: New `list_remote_shares(&PeerTarget) -> Result<Vec<String>, String>` and `run_sync(vault_path, &PeerTarget, subscriptions) -> Result<SyncSummary, String>` are available for the Tauri command layer (Task 8) to call. A fresh sync downloads every remote file in each subscribed folder; a repeat sync with no changes downloads nothing. When only the remote side changed, the new version is downloaded; when only local changed, the local file is left untouched (peer picks it up on its own next pull). When both sides changed, the local version is kept and the remote version is written alongside as `<name> (conflict from <peer> <date>).<ext>` - written once per divergence, not duplicated on repeat syncs of the same divergence. Deletions on the peer never propagate: a file removed remotely stays in the local vault. A corrupted transfer (hash mismatch on `FileEnd`) is rejected before any bytes reach the vault and is surfaced as one entry in `SyncSummary.errors`. Files: - src-tauri/src/sync/engine.rs:1-276: New file. `SyncSummary`, `PeerTarget`, `connect`/`recv` helpers, `list_remote_shares`, `run_sync`, `sync_folder` (manifest fetch + per-file decision dispatch), `download_file` (fetch + hash verification), and `write_atomic` (temp-file-then-rename) - src-tauri/src/sync/mod.rs:13: Added `pub mod engine;` - src-tauri/tests/sync_e2e_test.rs:1-225: New e2e test file covering share listing, fresh pull + idempotent repeat, remote-only vs local-only changes, conflict-copy creation and dedup, non-propagating deletions with local-only file survival, unexposed-folder reporting, corrupted-transfer rejection via a fake malicious peer, wrong pairing key, and unreachable peer - tasks/todo/p2p-sync.md:16: Marked Task 7 complete Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nflict copies
Context: engine.rs is the only sync code that writes into the vault, so a
review pass focused on two correctness findings in the download/conflict
path before this branch merges.
Problem: (1) In the Action::Conflict { write_copy } arm, seen_remote was
set unconditionally after attempting a conflict-copy download. When the
download failed, seen_remote was still recorded as seen, so decide() would
return write_copy: false on the next sync and the copy would never be
retried — a permanently lost conflict copy. (2) download_file returned a
flat Result<String, String>, so every failure (timeout, size-cap overrun,
an unexpected message mid-transfer) was treated the same as a clean
single-message Msg::Error refusal. After a partial read or timeout the
Noise channel's position in the request/reply sequence is unknown, but the
old code kept reusing it for the next file — a stale reply for file A
could be misattributed to file B's request and written to B's path (the
hash check wouldn't catch this since the bytes are A's genuine content).
Solution: Added a DownloadError enum (Recoverable vs Fatal) to
download_file's error path so every failure is classified by whether the
Noise channel is still aligned with the request/reply sequence:
Msg::Error replies, post-transfer hash mismatches, and local write_atomic
failures are Recoverable (a full message was consumed, or the failure
never touched the wire); GetFile send failures, recv()/timeout failures,
size-cap overruns, and unexpected message variants are Fatal (the channel
state is unknown because the reply stream was abandoned mid-sequence).
sync_folder now returns bool instead of Result<(), String> so it can
report to run_sync whether the channel is still usable: Fatal errors push
to summary.errors and return false; Recoverable errors push and continue
the file loop. The manifest phase got the same treatment (GetManifest send
failure, recv failure, or an unexpected message variant are channel-fatal;
a clean Msg::Error reply to GetManifest or a local build_manifest error are
not). In the conflict arm, seen_remote is now set only when the copy
download actually succeeds (Ok) or when write_copy was already false —
never on a failed attempt — so a failed copy is retried on the next sync
instead of being silently and permanently suppressed. run_sync's folder
loop now breaks out (skipping remaining subscribed folders) as soon as
sync_folder returns false, then still runs save_state so already-confirmed
progress isn't lost.
Behavior: Previously, a failed conflict-copy download silently marked the
remote hash as seen, so the copy was never retried on subsequent syncs.
Now the copy is retried until it actually succeeds. Previously, a timeout
or malformed reply mid-download left the channel in an unknown state but
the session continued serving further requests over it, risking a stale
reply being written under the wrong file's path. Now any such error aborts
the remainder of the session (remaining folders are skipped, not attempted
over a possibly-desynced channel), while clean single-message refusals
(Msg::Error) and local I/O errors no longer abort the session since the
channel is provably still aligned.
Files:
- src-tauri/src/sync/engine.rs:98-142: run_sync's folder loop now checks
sync_folder's bool return and breaks (instead of only logging an Err)
when the channel is desynchronized; save_state still runs after the loop.
- src-tauri/src/sync/engine.rs:144-268: sync_folder rewritten to return
bool. Manifest-phase errors are classified (send/recv/unexpected-message
failures are channel-fatal -> push error + return false; a clean
Msg::Error reply or a local build_manifest error are not -> push error +
return true). The Download and Conflict arms now match on
DownloadError::Recoverable vs ::Fatal instead of a flat error string.
The Conflict arm's seen_remote update moved inside the Ok branch (and
the write_copy: false branch), so a failed copy attempt no longer
suppresses the retry.
- src-tauri/src/sync/engine.rs:270-312: Added the DownloadError enum
(Recoverable/Fatal) above download_file; download_file now returns
Result<String, DownloadError>, classifying GetFile send failures,
recv()/timeout failures, size-cap overruns, and unexpected replies as
Fatal, and Msg::Error replies, hash mismatches, and write_atomic
failures as Recoverable.
- src-tauri/tests/sync_e2e_test.rs:198-269: Added
malicious_manifest_path_is_rejected_and_session_survives. A fake peer
advertises a manifest with a legitimate Notes/a.md and a traversal entry
Notes/../evil.md, serves a.md normally, and replies Msg::Error to any
other GetFile. Asserts the traversal entry is rejected before any
GetFile is sent (never reaches the network), the recoverable Msg::Error
path does not abort the session (run_sync still returns Ok), a.md is
downloaded, and nothing is written for the traversal entry (checked at
its resolved escape target and by asserting Notes/ contains only a.md).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: The sync backend (engine, noise, server, state, manifest, decision, protocol) is complete. This task exposes it over Tauri IPC so the frontend can start/stop the listener, generate keys, and initiate sync sessions.
Problem: No IPC commands exist to invoke sync operations from the frontend.
Solution: Created `commands/sync.rs` with 6 async/sync command wrappers:
- sync_generate_pairing_key() - fresh 64-hex-char key
- sync_start_listener(vault_path, port, key, device_name, exposed_folders) - (re)start listener, return bound port
- sync_stop_listener() - stop listening if running
- sync_status() - return {listening, port, localIp} struct
- sync_list_remote_shares(address, key, device_name) - ask peer for its exposed folders
- sync_now(vault_path, address, key, device_name, subscriptions) - run one pull session
Each wraps the corresponding backend function in `crate::sync::*`. Added:
- SyncStatus struct (serializes to camelCase JSON)
- local_lan_ip() helper (UDP socket bind trick for best-effort LAN IP)
- State management: SyncServerState registered via .manage()
Behavior: Frontend can now generate pairing keys, start listeners on dynamic ports, query peer shares, and sync. All tests pass (627 unit + integration tests). No breaking changes.
Files with line ranges:
- src-tauri/src/commands/sync.rs: 1-129 (new file)
- src-tauri/src/commands/mod.rs: 8 (added pub mod sync)
- src-tauri/src/lib.rs: 281 (added .manage), 347-352 (added 6 commands to invoke_handler)
- tasks/todo/p2p-sync.md: 17 (marked Task 8 done)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: Frontend plumbing for P2P vault sync feature (backend tasks 1-8 complete). The app needs to persist and manage sync configuration: listener enabled/port, device name, peer address, pairing key, and folder subscriptions. Problem: No sync settings existed. Frontend could not read, update, or persist sync configuration. Settings load-on-startup did not merge sync defaults with persisted values. Solution: Added SyncSettings interface defining all sync config fields. Integrated into AppSettings with default values (listener disabled, random port 0, empty device/pairing/peer, no subscriptions). Added settingsStore.sync getter and updateSync() method for reactive access and partial updates. Updated loadSettings to merge sync defaults with disk values on startup. Behavior: Frontend can now read/write P2P sync configuration. Settings persisted to .kokobrain/settings.json survive restarts. Partial updates preserve unmodified fields. All 46 settings tests pass, including new sync defaults and merge tests. Files: - src/lib/core/settings/settings.types.ts:283-304: Added SyncSettings interface with 7 fields (exposeEnabled, listenPort, deviceName, pairingKey, peerAddress, exposedFolders, subscriptions) + sync key to AppSettings:345 - src/lib/core/settings/settings.store.svelte.ts:1: Imported SyncSettings type - src/lib/core/settings/settings.store.svelte.ts:126-133: Added DEFAULT_SETTINGS.sync with defaults (exposeEnabled=false, listenPort=0, empty strings, empty arrays) - src/lib/core/settings/settings.store.svelte.ts:163: Added get sync() getter - src/lib/core/settings/settings.store.svelte.ts:294-296: Added updateSync() method to merge partial SyncSettings with existing values - src/lib/core/settings/settings.service.ts:137: Added sync merge line in merged object inside loadSettings() to apply defaults + persisted overrides - src/tests/lib/core/settings/settings.store.test.ts:313-333: Added 2 new tests: sync defaults (all fields match expected zero/empty values) + updateSync merge behavior (partial updates preserve unmodified fields across multiple calls) - tasks/todo/p2p-sync.md:18: Marked Task 9 complete Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: Task 10 of the P2P sync feature implementation. The sync plugin needs reactive state to track listener status (port, IP, running), remote peer's exposed shares list, last sync session summary (downloads, conflicts, errors), and flags for syncing/busy operations. Solution: Implemented syncStore following the project's getter-based store pattern with Svelte 5 runes ($state). Added types file with SyncSummary and SyncListenerStatus interfaces (mirroring Rust types). Store provides state getters, individual setters, and a reset() method. Computed getter lastSyncClean returns true only when the last sync finished without errors, enabling UI to display a "clean" vs "error" state. Behavior: Frontend components can now subscribe to reactive listener status and sync state. The lastSyncClean computed getter drives conditional UI rendering (success vs error display). Reset is called on vault teardown. Files: - src/lib/plugins/sync/sync.types.ts:1-23: New file with SyncSummary and SyncListenerStatus interfaces (with JSDoc) - src/lib/plugins/sync/sync.store.svelte.ts:1-73: New store implementation with 7 getters (status, remoteShares, lastSummary, lastSyncAt, syncing, busy, lastSyncClean), 7 setters, and reset() method - src/tests/lib/plugins/sync/sync.store.test.ts:1-50: Test suite covering defaults, setter/reset, and lastSyncClean computed getter logic - tasks/todo/p2p-sync.md:19: Marked Task 10 complete Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…scan Context: The privacy workflow's "Scan for External Calls" job greps src-tauri/src for networking primitives (TcpStream::connect, UdpSocket::connect, hardcoded URLs) and fails on any match not on the reviewed `known` allowlist. The new P2P vault sync engine opens a direct TCP connection to a peer, so the scan blocked PR #143 (run 28747919240). Problem: sync/engine.rs:59 `TcpStream::connect(&target.address)` is a legitimate, reviewed external call — the address is the ip:port the user enters in Sync settings, never hardcoded, and there is no external server — but the scan has no way to know that and fails the build. Solution: Add a documented entry to the scan action's `known` allowlist in privacy.yml matching `sync/engine\.rs:.*TcpStream::connect`. The read-only listener uses TcpListener::bind (not a scanned pattern) and the local-IP helper uses `socket.connect` (not the `UdpSocket::connect` pattern), so neither needs allowlisting. Verified by locally replaying the action's pattern/skip/known classification: 0 unknown/blocking matches remain. Behavior: The privacy scan passes; the sync TCP connect surfaces as a reviewed `::notice::` instead of a blocking error. No source or runtime change. Files: - .github/workflows/privacy.yml: added the sync/engine.rs TcpStream entry (with rationale comment) to the Rust scan's `known` list Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: Task 11 of P2P Vault Sync feature. Implements the service layer that wraps Tauri sync IPC commands (sync_generate_pairing_key, sync_start_listener, sync_stop_listener, sync_status, sync_list_remote_shares, sync_now), integrates with syncStore and settingsStore, and provides error handling with user-facing toasts. Problem: The sync feature had Tauri backend commands and a reactive store (Task 10) but no TypeScript service layer to orchestrate them. Components need a clean async API to trigger sync operations with proper state management and error handling. Solution: Created sync.service.ts with six exported async functions: - generatePairingKey(): invoke Rust command, store key in settings - startListener(vaultPath): start listener, persist ephemeral port, refresh status - stopListener(): stop listener, refresh status - refreshStatus(): fetch and store listener status (no toast, used opportunistically) - listRemoteShares(): fetch peer's exposed folders, manage busy state - syncNow(vaultPath): run one pull session, store summary + timestamp, show result toast Each function follows the error handling pattern: try/catch, log via error(), show toast on failure, throw to propagate to caller. State updates happen via syncStore setters; port persistence uses saveSettings(vaultPath) when needed. Behavior: - generatePairingKey: stores 64-char hex key; rethrows on failure - startListener: adapts ephemeral port 0 picks; skips re-save if port unchanged - stopListener: always refreshes status - refreshStatus: stores SyncListenerStatus; throws on failure (no user-facing toast) - listRemoteShares: manages busy state in finally block; shows toast on failure - syncNow: manages syncing state in finally block; shows success/warning toasts Tests (10 total, all passing): - generatePairingKey: happy path (stores key), error path (rethrows, leaves settings) - startListener: ephemeral port save + refresh, configured port no re-save - stopListener: refreshes status - refreshStatus: stores status - listRemoteShares: success (stores folders, clears busy), error (clears busy, rethrows) - syncNow: success (stores summary + timestamp, shows toast), error (clears syncing, rethrows) Test coverage uses real syncStore and settingsStore (no mocks), mocks only @tauri-apps/api/core, svelte-sonner, $lib/utils/debug, and settings.service. All assertions on real store state, not .toHaveBeenCalled() alone. Files: - src/lib/plugins/sync/sync.service.ts: 126 lines (lines 1-126) - src/tests/lib/plugins/sync/sync.service.test.ts: 134 lines (lines 1-134) - tasks/todo/p2p-sync.md: marked Task 11 [x] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: The Task 10 sync store review found that the "setters update state and reset clears it" test mutated all six fields but only asserted three (status, remoteShares, syncing) after reset(). A regression that dropped busy, lastSyncAt, or lastSummary from reset() would pass the suite undetected, since those fields' defaults are otherwise only checked by the "has inert defaults" test before any mutation. Solution: Add the three missing post-reset assertions (lastSummary, lastSyncAt, busy) alongside the existing three so reset() is fully covered. Behavior: Test-only change; the store implementation was already correct. sync.store test: 3/3 pass; pnpm check: 0 errors. Files: - src/tests/lib/plugins/sync/sync.store.test.ts: three added assertions in the reset() test Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context:
The sync.service test suite had incomplete coverage for error handling
paths. Three functions (startListener, stopListener, refreshStatus) had
only happy-path tests; additionally, syncNow's warning-toast branch
(when errors are present) was not exercised.
Problem:
Project testing rule 11 (docs/TESTING.md) requires every service
function to include both happy-path and error-propagation tests. The
missing tests left rethrow behavior and error-state invariants
unverified.
Solution:
Added four new tests to src/tests/lib/plugins/sync/sync.service.test.ts:
1. startListener error path: Verify invoke rejection is rethrown and
saveSettings is NOT called (failure before port persistence).
2. stopListener error path: Verify invoke rejection is rethrown.
3. refreshStatus error path: Verify invoke rejection is rethrown and
syncStore.status remains at default { listening: false, port: null,
localIp: null }.
4. syncNow warning-branch: When invoke resolves with non-empty errors
array, verify lastSummary.errors has length 1,
lastSyncClean is false, and syncing clears to false.
Behavior:
All 14 tests in sync.service.test.ts pass (10 existing + 4 new).
Type checking passes (0 errors). All six service functions
(generatePairingKey, startListener, stopListener, refreshStatus,
listRemoteShares, syncNow) now have both happy-path and error-path
coverage.
Files:
- src/tests/lib/plugins/sync/sync.service.test.ts (lines 135–164,
new tests added after existing suite)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: Tasks 9-11 built the sync settings block, reactive store, and
Tauri-calling service for P2P vault sync, but there was no UI to
configure them and the listener never started automatically on vault
open. This task adds the last integration points: the SettingsPanel
section, the SettingsSection union entry (deliberately deferred from
Task 9 so the union and the panel's icon Record change together in one
commit), and vault init/teardown hooks for the expose-on-startup
listener.
Solution: Added SyncSection.svelte following the existing settings
section pattern (SettingItem rows, Input/Switch/Button from
$lib/components/ui, an untrack()-wrapped $effect for the initial
refreshStatus() call per docs/PATTERNS.md so the service call isn't
tracked as a reactive dependency). Registered 'sync' in the
SettingsSection union and in the Integrations settings group after
Todoist, and added the section's icon + content branch to
SettingsPanel.svelte. Wired app-lifecycle.service.ts to start the
listener on vault init when settingsStore.sync.exposeEnabled is true
(mirroring the existing auto-move enable/disable pattern), stop it
during teardown alongside the other background-process shutdowns, and
reset syncStore in the store-reset block alongside todoistStore.
Behavior: The Settings dialog now has a "Sync" entry under
Integrations with controls for device name, an expose-to-peer toggle,
pairing key generate/copy, peer address, exposed folders, peer
subscriptions, and a manual "Sync now" action with a last-sync
summary. Opening a vault with sync.exposeEnabled on now starts the
listener automatically; closing the vault stops it and resets the
sync store.
Files:
- src/lib/core/settings/sections/SyncSection.svelte:1-239: New section
component wiring syncStore/sync.service to the sync settings fields,
listener status, remote share listing, and sync-now.
- src/lib/core/settings/settings.types.ts:308: Added 'sync' to the
SettingsSection union (deferred from Task 9 per plan notes).
- src/lib/core/settings/settings.logic.ts:91-97: Added the sync
section entry to the Integrations group after Todoist.
- src/lib/core/settings/SettingsPanel.svelte:28,46,69,159-160: Added
the SyncSection import, ArrowLeftRightIcon import, sectionIcons
entry, and the {:else if 'sync'} content branch after Todoist.
- src/lib/core/app-lifecycle/app-lifecycle.service.ts:68-69,279-285,
395,450: Added sync.service/sync.store imports, a start-listener-on
-init block after the auto-move block, a stop-listener call in the
background-process teardown block, and syncStore.reset() in the
store-reset block.
- src/tests/lib/core/settings/settings.logic.test.ts:206,217-220:
Updated the pre-existing ordered-ids assertion to include 'sync' (a
direct consequence of registering the new section) and added a new
test asserting the sync section is registered under Integrations.
- tasks/todo/p2p-sync.md:21: Marked Task 12 complete.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: The Sync settings section has a "Copy" button that writes the pairing key to the clipboard via writeText() from @tauri-apps/plugin-clipboard-manager. Manual testing showed the button did nothing — the key never reached the clipboard and no error surfaced. Problem: The main-window capability (capabilities/default.json) granted only clipboard-manager:allow-read-text (used by deep-link/quick-capture paste), never allow-write-text. Tauri therefore denied writeText(), the promise rejected, and handleCopyKey() had no try/catch, so the failure was swallowed with no user feedback. Solution: Add the granular clipboard-manager:allow-write-text permission (matching the file's least-privilege style, not the broader :default). Harden handleCopyKey() with try/catch: a success toast confirms the copy (otherwise invisible to the user) and an error toast surfaces any future failure instead of swallowing it; also skip the no-op when the key is empty. Behavior: With the app rebuilt, clicking Copy places the pairing key on the clipboard and shows "Pairing key copied to clipboard". The capability change takes effect only after a Tauri rebuild (restart pnpm tauri dev), since capabilities are compiled into the binary. Files: - src-tauri/capabilities/default.json: add clipboard-manager:allow-write-text - src/lib/core/settings/sections/SyncSection.svelte: try/catch + toast feedback in handleCopyKey, empty-key guard, import toast Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: The Cargo Audit CI job (rustsec/audit-check in security.yml) started failing on PR #143 after the RUSTSEC database added two quick-xml advisories on 2026-07-05. This is unrelated to the P2P sync feature — it would fail on any branch, including main. Problem: quick-xml 0.39.4 has RUSTSEC-2026-0194 (O(N^2) duplicate-attribute check) and RUSTSEC-2026-0195 (unbounded namespace-declaration allocation), both DoS, patched in >=0.41.0. It is pulled only transitively: quick-xml <- plist <- tauri. plist 1.9.0 pins quick-xml "^0.39.2", so `cargo update` cannot select the patched 0.41.x without an upstream tauri/plist release — the fix is not available to this repo yet. Solution: Add src-tauri/.cargo/audit.toml ignoring exactly those two advisory IDs, with a documented justification: quick-xml is reached only through plist parsing app-controlled macOS property lists (bundle Info.plist), never untrusted network XML, so neither DoS vector is reachable in this app. The ignore is scoped to the two IDs — any other advisory still fails the audit. Verified locally that cargo-audit reads this file from src-tauri/.cargo/audit.toml and applies the ignore list. Behavior: Cargo Audit passes; the two quick-xml advisories are silenced with a revisit note (drop the ignores once tauri ships a plist allowing quick-xml >= 0.41). No dependency or source change. Files: - src-tauri/.cargo/audit.toml: new; ignore RUSTSEC-2026-0194 / -0195 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: All 13 tasks of the P2P vault sync plan are implemented, reviewed, and committed. The CLAUDE.md plugins list already lists `sync` (commit e1fec15), so this closeout only moves the plan checklist to done. Solution: Mark Task 13 complete and move the task file from tasks/todo to tasks/done per the project's Plan Mode Workflow. Behavior: Documentation/bookkeeping only. Files: - tasks/todo/p2p-sync.md -> tasks/done/p2p-sync.md (all 13 tasks checked) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t.toml Context: PR #143's Cargo Audit job kept failing on the two quick-xml DoS advisories even after src-tauri/.cargo/audit.toml was added (commit 7cf2176). The CI output showed "ignore":[] — the rustsec/audit-check action does NOT read .cargo/audit.toml; that file only affects a local `cargo audit` CLI run. Problem: The ignore list was in the wrong place for CI. The action (confirmed via its action.yml at the pinned SHA) exposes an `ignore` input — a comma-separated list of advisory IDs — which is the only mechanism it honors. Solution: Add `ignore: RUSTSEC-2026-0194,RUSTSEC-2026-0195` to the audit-check step's `with:` block in security.yml, with the same justification comment. Reword src-tauri/.cargo/audit.toml to state clearly that it is for LOCAL runs only and that CI reads the workflow input, so the two must be kept in sync — preventing the exact confusion that caused this second failure. Behavior: Cargo Audit passes in CI (the two quick-xml advisories are ignored with justification); local `cargo audit` from src-tauri also passes via audit.toml. Scoped to the two IDs — any other advisory still fails. Files: - .github/workflows/security.yml: add ignore input to the audit-check step - src-tauri/.cargo/audit.toml: clarify it is local-only, keep IDs in sync Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Context: Final security review of the P2P sync feature (docs/superpowers/specs/2026-07-03-p2p-sync-design.md
§Security) found that serve_file only validated the resolved path against
the vault root, not the specific exposed folder the request claimed to be
under.
Problem: A symlink inside an exposed folder that points elsewhere in the
vault passed validation. Example: vault exposes ["Notes"] and also
contains Private/secret.md; a symlink Notes/leak.md -> ../Private/secret.md
resolves inside the vault root, so the old check (resolved.starts_with(vault_root))
accepted it. build_manifest skips symlinks so the name is never advertised,
but an authenticated peer who guesses/knows the name via GetFile { rel_path:
"Notes/leak.md" } received Private/secret.md, a file outside the exposed
set. Additionally, serve_file's error replies distinguished "file not
shared" from "file not readable: {os error}" / "read failed: {os error}",
letting a peer probe file existence and leaking raw OS error text.
Solution: Replaced the boolean is_exposed helper with
matched_exposed_folder, which returns the specific exposed folder a
rel_path sits inside. serve_file now canonicalizes that matched folder
(not the vault root) and requires the resolved file to stay under it,
so a symlink can no longer escape into a sibling folder while staying
inside the vault. All failure branches (invalid path, no matching folder,
folder/file canonicalize failure, starts_with check, read failure) now
reply with one generic "file not available: {rel_path}" message, removing
the OS error text and the distinguishable "not shared" vs "not
readable"/"read failed" wording a peer could use to probe.
Behavior: Previously, GetFile for a symlink inside an exposed folder that
targeted a file outside it would succeed and stream the target file's
bytes. Now it is refused with a generic Error reply, verified end-to-end:
a new test spawns a server exposing only "Notes", creates
Notes/leak.md -> ../Private/secret.md, and asserts GetFile returns
Msg::Error (never FileChunk/FileEnd) while GetFile for the real
Notes/a.md still serves normally. Re-running the test against the
pre-fix code (via a temporary git stash of only server.rs) confirmed it
fails red, receiving a FileChunk containing the secret's bytes.
Files:
- src-tauri/src/sync/server.rs:159-205: Replaced is_exposed(bool) with
matched_exposed_folder(Option<&String>); rewrote serve_file to
canonicalize and confine to the matched folder instead of the vault
root, and unified every failure branch into one generic "file not
available: {rel_path}" reply with no OS error text.
- src-tauri/tests/sync_server_test.rs:173-240: Added
symlink_within_exposed_folder_is_refused (#[cfg(unix)]), a
self-contained test (own tempdir vault, since spawn_test_server's
fixed layout has no Private/ folder or symlink) covering the escape
scenario and a sanity check that normal in-folder files still serve.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.