feat(aw-sync): return and persist a SyncReport from each pass

[TimeToBuildBob]

Summary

A sync pass used to return (). JNI always said "Successfully pulled from all hosts", the daemon had no status except dying, and aw-sync status could only describe the folder, not the last pass. That is why a daemon could run 102 empty cycles with nothing to look at (#682 / #684).

This returns a SyncReport from sync_run / pull_all / push, persists the latest one locally (not in the Syncthing folder), and surfaces it in three places:

• aw-sync status prints the last pass (peers imported/skipped/failed, event counts)
• JNI returns real counts in the existing {success, message} object, plus events_pulled, peers_skipped, etc. — Sync status is a timestamp and a boolean — the model cannot answer what synced, when next, or from whom aw-android#274 becomes a rendering job
• The daemon writes the same file, so a later status can show what the last pass did even if the process has since exited

PeerOutcome::Skipped is how the duplicate-device_id dedupe from #686 reports itself, instead of a log line.

Closes #695.

Persistence

{data_dir}/aw-sync/last-sync-report.json (override with AW_SYNC_LAST_REPORT). Atomic write. A persist failure is a warning, not a failed pass.

Out of scope, as specified: per-device folder metadata ("which device is this peer", "when did it last push") — that is the manifest in ActivityWatch/activitywatch#302 / #691.

Does not change error policy: a per-peer failure still aborts the pass (#688). The partial report is persisted on that path so the failure is visible afterwards.

Test plan

• cargo test -p aw-sync --lib --bins --tests (40 passed)
• cargo clippy --workspace -- -D warnings
• CI

[TimeToBuildBob]

Review guide (auto-posted)

Structured warm-up for reviewers — what changed, what to run, where to look.

Key files

• aw-sync/src/report.rs (+429/-0, Δ429)
• aw-sync/src/util.rs (+97/-12, Δ109)
• aw-sync/src/sync.rs (+76/-28, Δ104)
• aw-sync/src/sync_wrapper.rs (+41/-19, Δ60)
• aw-sync/src/android.rs (+18/-28, Δ46)
• aw-sync/src/main.rs (+26/-7, Δ33)
• aw-sync/tests/sync.rs (+33/-0, Δ33)
• aw-sync/src/status.rs (+15/-0, Δ15)

Suggested verification

• Run the repo's usual CI-equivalent checks locally

Known risks / watch points

• No automatic high-risk tags; use file list + diff for judgment.

Suggested review focus

• Confirm behavior matches the PR description acceptance criteria.
• Skim the largest diffs first (listed above).
• If CI is green, spot-check the highest-risk paths called out here.

Generated by scripts/github/pr-warmup-review-guide.py for #699.

[ErikBjare]

@TimeToBuildBob Structure matches #695 point for point — PeerReport.device_id, PeerOutcome::Skipped { reason } giving the #686 dedupe somewhere to report, BucketReport, persisted to {data_dir}/aw-sync/last-sync-report.json (outside the synced folder, correctly), JNI fixed string gone. Test-merged against #697, #678 and #698: all clean, so the return-type change does not force rebases on the v0.14.x queue.

Sequencing: this goes after the v0.14.x fixes (#698 → #697 → #678 → #693-minimal), not gating the cut. It is additive and larger (+743), so it gets a full read when it reaches the front rather than a structural pass now. Please keep it rebased as those land.

One ask for when I read it properly: a test that a pass which finds zero peers produces a report saying so (peers: [] with the discovery warnings captured) — that is the #682 failure mode, and the whole point of #695 is that it can no longer be silent.

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge; the latest change correctly makes incompatible peers visible without introducing a new reporting failure.

Summary

This PR adds durable, structured reporting for each synchronization pass.

• Returns SyncReport values from pull, push, and combined sync workflows.
• Persists the latest complete or partial report for later inspection through aw-sync status.
• Exposes event and peer outcome counts through Android JNI.
• Records duplicate and incompatible peers as skipped, while preserving failure details and completed work.
• Uses unique temporary files for atomic report persistence.

Diagram

sequenceDiagram
    participant Caller as CLI / Daemon / Android
    participant Sync as Sync pass
    participant Peer as Peer databases
    participant Server as Local server
    participant Report as Last-report file

    Caller->>Sync: Start pull, push, or both
    Sync->>Peer: Discover and open peers
    Peer-->>Sync: Imported, skipped, or failed outcomes
    Sync->>Server: Import peer events
    Sync->>Peer: Export local events
    Sync-->>Caller: SyncReport or error
    Caller->>Report: Persist complete report
    Sync->>Report: Persist partial report on failure

Loading

_{Reviews (5) · Last reviewed commit: "fix(aw-sync): record incompatible-versio..."}

[codecov]

Codecov Report

❌ Patch coverage is 48.61878% with 186 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.80%. Comparing base (656f3c9) to head (2fab2f1).
⚠️ Report is 116 commits behind head on master.

Files with missing lines	Patch %	Lines
aw-sync/src/sync.rs	6.94%	67 Missing ⚠️
aw-sync/src/report.rs	74.84%	41 Missing ⚠️
aw-sync/src/sync_wrapper.rs	0.00%	35 Missing ⚠️
aw-sync/src/main.rs	0.00%	33 Missing ⚠️
aw-sync/src/status.rs	0.00%	7 Missing ⚠️
aw-sync/tests/sync.rs	89.47%	2 Missing ⚠️
aw-sync/src/util.rs	96.96%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #699      +/-   ##
==========================================
+ Coverage   70.81%   78.80%   +7.98%     
==========================================
  Files          51       73      +22     
  Lines        2916     7222    +4306     
==========================================
+ Hits         2065     5691    +3626     
- Misses        851     1531     +680     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TimeToBuildBob]

Added in 42946f6.

A zero-peer pull now persists peers: [] plus the discovery warnings (Found 0 remote db files / empty-dir diagnosis) on SyncReport. Test: zero_peer_pass_reports_empty_peers_and_captures_discovery_warnings. Those lines were warn!()-only before, which is how #682 stayed silent after 102 empty cycles.

Will keep this rebased as #698 → #697 → #678 → #693-minimal land. Not gating the cut.

Same commit also persists pull_all's aggregate when a later peer fails, and writes the report through a unique temp file.

[ErikBjare]

@TimeToBuildBob CONFLICTING against master after #698 (same sync_run region). Needs a rebase, but no rush — this is queued after the v0.14.0 cut, so wait until #700/#678/#697 have landed and rebase once over all of them rather than three times.

[TimeToBuildBob]

Understood — will rebase once, after #700/#678/#697 all land, not per-PR.

[TimeToBuildBob]

🤖 AI code review

Safe to merge — 5 findings disposed (other; rejected; fixed in 91255d6)

_{Updated after inline dispositions on finding threads — this is the current state; the verdict below is frozen at review time and is kept as the historical record of that pass.}

Finding disposition

Finding	Severity	State
aw-sync/src/main.rs:285	P1	superseded by latest review (not reproduced)
aw-sync/src/report.rs:357	P1	rejected — Reviewer's verification assumed Win32 MoveFile semantics (fail if destination exists); Rust std fs::rename is not raw Mo
aw-sync/src/sync.rs:159	P1	other
aw-sync/src/sync.rs:100	P1	rejected — Reviewer verified: find_remotes is 2-level, list_remote_dbs is 3-level, so sync_run against a 3-level folder finds 0 rem
aw-sync/src/sync.rs:177	P1	fixed in 91255d6
aw-sync/src/sync_wrapper.rs:48	P1	rejected — Reviewer's verification assumed pull_all merges sync_run's in-memory report on Err, producing two PeerReport::failed ent
aw-sync/src/main.rs:296	P2	fixed in 49f0ce1
aw-sync/src/main.rs:365	P2	wontfix
aw-sync/src/sync.rs:177	P2	superseded by latest review (not reproduced)
aw-sync/src/sync.rs:65	P2	accepted-tradeoff
aw-sync/src/sync.rs:122	P2	accepted-tradeoff
aw-sync/src/sync.rs:65	P2	accepted-tradeoff
aw-sync/src/sync.rs:135	P2	accepted-tradeoff
aw-sync/src/sync.rs:133	P2	accepted-tradeoff
aw-sync/src/sync_wrapper.rs:46	P2	rejected

This PR introduces a SyncReport type that is returned from sync_run, pull_all, push, and the JNI entry points, persisted to {data_dir}/aw-sync/last-sync-report.json, and printed by aw-sync status. It adds PeerOutcome::Skipped/Failed reporting for duplicate device_id and incompatible database versions, changes pull_discovery_warnings to accept Option<&str> for the local device id, and adds a close_opened_datastores helper for error paths. The diff also updates Cargo.toml to enable serde derive, adds tests for report round-tripping and event counts, and updates README/status output.

Needs a look — P2 only

Confidence 4/5

1 finding · ⚠️ 1 P2 — non-blocking (round cap reached)

⚠️ P2 medium · ℹ️ non-blocking — aw-sync/src/sync.rs:133

In sync_run, when open_peer_datastore returns Err, the code pushes a PeerReport::failed, persists the report, closes datastores, and returns Err. However, the report is only persisted inside the if mode == SyncMode::Pull || mode == SyncMode::Both block. If mode is Push, the error path still closes datastores and returns Err, but the report is not persisted and no PeerReport is added. This means a push-only pass that fails to open a remote db (which is not even used in push mode) will not leave a failure record, contradicting the PR's stated contract that a failed pass is visible afterwards. The observable consequence is that aw-sync status shows the previous successful report or 'none' after a push-only failure, making the failure invisible.

Move the persist and PeerReport::failed push outside the mode check, or at least persist the report with a warning in Push mode.

_{How this was verified: Checked the code path: the Err branch at line 133-147 only persists when mode is Pull or Both. In Push mode, ds_remotes is empty (no pull phase), so this branch is only reachable if find_remotes_nonlocal_selection returns a remote that fails to open, which is possible even in push mode. The report is created at line 64 but never persisted on this path for Push mode.}

Files changed (11) — the diff as I read it

• aw-sync/Cargo.toml — Enables the serde derive feature for the aw-sync crate.
• aw-sync/README.md — Adds a line to the status command documentation about printing the last persisted pass.
• aw-sync/src/android.rs — Changes JNI sync entry points to return and persist SyncReport JSON instead of fixed success messages.
• aw-sync/src/lib.rs — Adds the report module and re-exports its public types and functions.
• aw-sync/src/main.rs — Adds report persistence and summary logging to daemon and sync command paths, including failure handling.
• aw-sync/src/report.rs — New module defining SyncReport, PeerReport, BucketReport, persistence functions, and tests.
• aw-sync/src/status.rs — Adds loading and printing of the last persisted SyncReport to status output.
• aw-sync/src/sync.rs — Changes sync_run and sync_datastores to return SyncReport/BucketReport, adds error-path datastore closing, and records skipped/failed peers.
• aw-sync/src/sync_wrapper.rs — Changes pull_all, pull, push, and push_with_hostname to return SyncReport and persist partial reports on failure.
• aw-sync/src/util.rs — Adds select_remote_dbs_detailed and find_remotes_nonlocal_selection returning skipped remotes, and changes pull_discovery_warnings to take Option<&str>.
• aw-sync/tests/sync.rs — Adds a test asserting sync_datastores returns new event counts and that a second pull is a no-op.

Previous review passes

commit	score	findings	engine	when
42946f6494d4	2/5	3	llm	2026-09-16 09:28 UTC
49f0ce1726a2	2/5	3	llm	2026-09-16 12:44 UTC
91255d66a998	4/5	3	llm	2026-09-16 13:22 UTC
592c7b3ac0cb	4/5	1	llm	2026-09-16 14:20 UTC
a28035904771	3/5	2	llm	2026-09-16 15:28 UTC
e4e92ecb53ab	3/5	4	llm	2026-09-16 16:22 UTC

_{Reviewed 2fab2f1452bf · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 607s · about this reviewer}

_{Maintainer commands}

_{@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.}

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

Rebased onto master (#698 landed): the sync_run conflict is resolved by keeping maybe_setup_local_remote plus the selection-based remote discovery, so the pull-only no-write invariant holds.

Also fixed the two real findings from the latest review in 49f0ce1: the simple host-based path and the Android syncBoth entry now capture the push result — on push failure the merged pull report (with a push warning) is finished and persisted before the error propagates, so last-sync-report.json no longer shows a push-only pass. The Windows fs::rename P1 was dismissed as a false positive (MoveFileExW + MOVEFILE_REPLACE_EXISTING). Re-review triggered; CI running on the new head.

[TimeToBuildBob]

CI-green and mergeable (Greptile 5/5) — waiting only on a maintainer click.

This PR is ready to merge, but the bot has pull-only access to this repo and can't self-merge — surfacing it here so it isn't lost. The monitoring loop will stop re-flagging it now that this note is posted.

[TimeToBuildBob]

12:48 AI-review on 49f0ce1 (score 2/5): one real hole, two not.

• P1 push-abort silent report — real. Inner sync_run persisted a push failure with no warning, so last-sync-report.json looked like a no-op. Fixed in 91255d6 (record_push_failure before persist; same helper on the simple-path and JNI callers). Test: push_failure_persisted_report_is_not_a_silent_success.
• P1 two walkers — rejected. pull_all is the 3-level path; sync_run is 2-level relative to the directory it is given (a host folder from pull, or the legacy root). Broadening find_remotes at the root would import the aw-sync: daemon (the default subcommand) never pulls — two incompatible sync-folder layouts #682 orphan. Daemon layout is fix(aw-sync): make default daemon use host-layout pull/push #685.
• P2 daemon exit-on-error — no new defect. aw-sync: per-peer and per-bucket errors abort the whole sync pass (and burn the supervisor's restart budget) #688 / fix(aw-sync): skip broken peers in sync_run instead of aborting the pass #703.

Zero-peer report test is still in (Erik's ask). Next rebase still waits on #700 / #678 / #697, not a merge click now.

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

13:24 AI-review on 91255d6 (score 4/5): one real hole, two not.

• P2 summary_message hides push abort — real. Daemon/JNI logged Pushed 0 new events even when warnings already had push failed: …. Fixed in 592c7b3 (append warnings to the summary; same test now asserts it).
• P2 early setup does not persist — trade-off. get_info / find_remotes abort before a pass exists; overwriting the last completed report with "couldn't talk to the server" would hide a useful last-pass. Partial persist stays on failed-peer paths.
• P2 pull_all double persist — rejected. No distinct bug; the aggregate overwrite is intended.

#700 landed; still waiting on #678 and #697 before the one rebase Erik asked for. Not mergeable yet (CONFLICTING).

[TimeToBuildBob]

15:38 in-band review on 592c7b3 (score 4/5) plus DIRTY vs master after #700.

Pushed a280359:

• Empty local device_id — pull_all passed "" into pull_discovery_warnings, which is Some(""), not unclassified. Leftover 2-level own staging then showed up as "peer db(s) that pull did not select". Now None, and 2-level leftovers are not missed-peers when the id is unknown. Still no get_info() on this path (120s HTTP timeout; aw-sync: daemon (the default subcommand) never pulls — two incompatible sync-folder layouts #682 stays a filesystem check). Test: unknown_local_id_does_not_report_two_level_leftover_as_unselected_peer.
• Close datastore workers on error returns — Datastore::close() stops the sqlite worker; the new persist-then-return paths skipped it.

Did not rebase. #678 and #697 are still open; one rebase after both land, as asked.

In-band P1 "double-recorded failed peer" rejected: pull_all overwrites the nested persist, it does not merge it. P2 "persist push-mode remote-open failure" left as the same last-pass trade-off.

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

Rebased onto master (f8374cd) after #700/#697 landed. The sync.rs conflict was the read-only peer-open loop: sync_run now walks selection.selected and opens each peer with open_peer_datastore (read-only, no migration/WAL flips), keeping the SyncReport machinery — open failures record a failed PeerReport + persist, version-mismatch skips with the existing warning. cargo test -p aw-sync green (33 lib + 11 sync + 3 roundtrip). Greptile re-triggered.

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

16:41 AI-review on e4e92ec (score 3/5): one real hole, two not.

• P1 later-host pull drops earlier hosts — real. --host a --host b used ?, so a later host's failure skipped aw_sync_persist of the aggregate and last-sync-report.json kept only the failing host. Fixed in 2fab2f1 (record_pull_failure + persist before propagate). Test: later_host_pull_failure_keeps_earlier_hosts_on_disk.
• P2 get_info does not persist — trade-off. Early setup aborts before a pass exists; overwriting the last completed report would hide a useful last-pass.
• P2 push-mode open failure does not persist — same trade-off as 23be81b9f2e5 after the fix(aw-sync): sanitize whitespace hostnames on import; make per-bucket errors non-fatal #697 rebase. Skipping remotes in push mode is a separate change.

cargo test -p aw-sync --lib --bins --tests green (34 lib + 11 sync + 3 roundtrip). In-band review timed out at 1200s; push landed on 2fab2f1. Sweep will re-review the new head.

[TimeToBuildBob]

Local re-review on 2fab2f1 (the new head): score 4/5, no P0/P1. The later-host pull P1 is confirmed fixed and no new blocking findings reproduce. Remaining is the known P2 (push-mode remote open failure leaves the last completed report intact) — same accepted trade-off as dispositioned above.

Finding threads all carry replies now; leaving resolution to the sweep on this head.

[TimeToBuildBob]

Merge verified, not just "merged": full CI is green on the merge commit 37aa0b0 (ubuntu/macOS/windows/clippy/format/coverage/Android all success), and ActivityWatch/aw-server-rust#695 is closed.

Downstream consumer is already wired: ActivityWatch/aw-android#285 bumps the aw-server-rust submodule 97f51e3 → 37aa0b0 (this merge commit) and parses the to_jni_json() counts into SyncStatus, so the app-side half of ActivityWatch/aw-android#274 no longer discards the report.

Deliberately unchanged, as scoped here: a per-peer failure still aborts the pass. That remainder is tracked at ActivityWatch/aw-server-rust#688, with ActivityWatch/aw-server-rust#703 in flight.

No further action needed from me on this PR.