fix(aw-sync): do not stage a local db on pull-only sync_run by TimeToBuildBob · Pull Request #698 · ActivityWatch/aw-server-rust

TimeToBuildBob · 2026-09-16T08:28:07Z

Split out of #685 as requested: the daemon layout switch stays parked there. This is the piece that helps the paths that already pull today (Android, aw-sync sync).

Problem

sync_wrapper::pull walks a peer host folder via sync_run(..., Pull). sync_run called setup_local_remote unconditionally, so a pull wrote {peer_host}/{our_device_id}/test.db into a folder this device does not own.

Fix

Only open a staging datastore when the pass actually pushes (SyncMode::Push / Both). Pull-only returns None and does not create {peer}/{our_device_id}/.

No daemon-layout change. No walker change.

Tests

pull_does_not_create_local_staging_db
push_creates_local_staging_dir

Related: #682, #685, ActivityWatch/activitywatch#1445.

`sync_wrapper::pull` walks a peer host folder via `sync_run(..., Pull)`. That used to call `setup_local_remote` unconditionally, creating `{peer_host}/{our_device_id}/test.db` in a folder we do not own. Only open a staging datastore when the pass actually pushes. Split out of ActivityWatch#685; the daemon layout switch stays parked there. Git-Session-Id: 86fd4466-d56b-50f2-bff6-107033a71e12

TimeToBuildBob · 2026-09-16T08:28:09Z

Review guide (auto-posted)

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

Key files

aw-sync/src/sync.rs (+69/-4, Δ73)

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 #698.

greptile-apps · 2026-09-16T08:30:41Z

Confidence Score: 5/5

The PR appears safe to merge; the previous fresh-root pull failure is fully addressed without restoring the unwanted local staging directory.

Findings

Pull fails on fresh roots ▶

Summary

This PR prevents pull-only synchronization from creating a local-device staging datastore inside a peer-owned folder while preserving staging for push-capable modes.

Makes local staging conditional on Push or Both.
Creates only the sync root during pull-only operation, fixing fresh-root discovery without creating a device staging subdirectory.
Adds regression coverage for pull-only staging, missing sync roots, and push staging.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[sync_run] --> B{Sync mode}
    B -->|Pull| C[Create sync root only]
    C --> D[Discover and import remote databases]
    B -->|Push or Both| E[Open local staging datastore]
    E --> F[Export local first-hand buckets]
    B -->|Both| D

_{Reviews (2) · Last reviewed commit: "fix(aw-sync): create sync root on pull-o..."}

greptile-apps · 2026-09-16T08:30:44Z

+    } else {
+        Ok(None)
+    }


Pull fails on fresh roots

The advanced CLI and pull-only daemon can pass a sync root that does not exist yet because get_sync_dir() returns the path without creating it. This branch now skips all directory creation, so remote discovery returns NotFound; the one-shot command fails and the daemon exits instead of reaching the normal empty-directory warning. Please create the sync root here without creating the local device's staging subdirectory, and add a regression test that starts with a nonexistent root.

Suggested change

} else {

Ok(None)

}

} else {

fs::create_dir_all(path)?;

Ok(None)

}

Knowledge Base Used:

Synchronization

Sync workflows and transport

Fixed in 1f23dd7: pull-only now create_dir_alls the sync root without {path}/{our_device_id}/. Regression test pull_creates_missing_sync_root_without_local_staging starts from a nonexistent path and asserts discovery returns empty instead of NotFound.

ErikBjare · 2026-09-16T08:31:16Z

Exactly the split asked for on #685: maybe_setup_local_remote gated on mode, one file, no daemon switch. Merges clean against #699. This is the next thing Erik merges once windows/macOS finish — nothing else pending on it from me.

codecov · 2026-09-16T08:37:41Z

Codecov Report

❌ Patch coverage is 50.00000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.89%. Comparing base (656f3c9) to head (1f23dd7).
⚠️ Report is 115 commits behind head on master.

Files with missing lines	Patch %	Lines
aw-sync/src/sync.rs	50.00%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #698      +/-   ##
==========================================
+ Coverage   70.81%   79.89%   +9.07%     
==========================================
  Files          51       72      +21     
  Lines        2916     6823    +3907     
==========================================
+ Hits         2065     5451    +3386     
- Misses        851     1372     +521

☔ 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.

Pull-only skipped setup_local_remote, so a missing sync root made find_remotes NotFound and the advanced/daemon pull path exit instead of warning on an empty dir. Create the root; still do not create {root}/{our_device_id}/. Git-Session-Id: 657af204-9f0a-5a4c-b756-89c0f521ef53

TimeToBuildBob · 2026-09-16T08:43:28Z

Greptile's P1 was real: pull-only no longer created the sync root, so find_remotes NotFound on a fresh get_sync_dir() and the advanced/daemon pull path exited instead of the empty-dir warning.

Landed in 1f23dd7: create_dir_all(path) on Pull, still no {path}/{our_device_id}/, plus a regression test that starts from a missing root.

The #685 split is unchanged — one file, no daemon switch. CI will re-run on this SHA.

TimeToBuildBob · 2026-09-16T08:43:37Z

@greptileai review

The simple host-based sync path and the Android syncBoth JNI entry both did `report.merge(push(...)?)` — the `?` discarded the merged pull report when push returned Err, so last-sync-report.json (and the JNI error payload) reflected only the push-phase report from inside sync_run. Persist the pull-phase result with a push warning before propagating. Rebased onto master (ActivityWatch#698 landed); conflict in sync_run resolved by keeping maybe_setup_local_remote + selection-based remote discovery. Git-Session-Id: 2a2cd4fe-793a-5f03-bfbc-7da3c8ff9738

* feat(aw-sync): return and persist a SyncReport from each pass A successful pass used to return (). JNI, aw-sync status, and the daemon had nothing to report. SyncReport carries per-peer/per-bucket counts, including duplicate-device_id skips, and is persisted locally (not in the Syncthing folder). Closes #695 Git-Session-Id: 94e08188-0a05-5fed-b8c0-fd327ae1f803 * feat(aw-sync): persist discovery warnings on zero-peer SyncReport The #682 failure mode was silence: a pass that found nobody logged warnings and threw them away. Capture them on the report so `peers: []` plus the diagnosis survive in last-sync-report.json. Also persist pull_all's aggregate on a later-peer failure, and use a unique temp file for the atomic write. Git-Session-Id: 2a2cd4fe-793a-5f03-bfbc-7da3c8ff9738 * fix(aw-sync): persist pull-phase report when push fails The simple host-based sync path and the Android syncBoth JNI entry both did `report.merge(push(...)?)` — the `?` discarded the merged pull report when push returned Err, so last-sync-report.json (and the JNI error payload) reflected only the push-phase report from inside sync_run. Persist the pull-phase result with a push warning before propagating. Rebased onto master (#698 landed); conflict in sync_run resolved by keeping maybe_setup_local_remote + selection-based remote discovery. Git-Session-Id: 2a2cd4fe-793a-5f03-bfbc-7da3c8ff9738 * fix(aw-sync): record push failures on the persisted SyncReport The inner sync_run push-error path finished and persisted without a warning, so last-sync-report.json looked like a no-op success. Same string the simple-path and JNI callers already used. Git-Session-Id: 01a0aa45-67f2-7c32-9c50-447c12587192 * fix(aw-sync): surface warnings in SyncReport summary_message Daemon and JNI log this string. A push abort already lived in `warnings` and the Display impl, but summary_message still said "Pushed 0 new events" — a success-like line for a failed pass. Git-Session-Id: 01a0aa92-68eb-7062-9ad7-aca64833f8b1 * fix(aw-sync): treat empty local device_id as unknown in discovery warnings pull_all passed "" into pull_discovery_warnings, which wrapped it as Some(""). That does not unclassify entries: leftover 2-level own staging became a Peer and showed up as "peer db(s) that pull did not select". Pass None instead, and skip 2-level leftovers from the missed-peer warning when the local id is unknown. Do not call get_info() here — reqwest's timeout is 120s; empty-dir diagnosis must stay a filesystem check (#682). Git-Session-Id: 01a0aabd-c1af-70b2-807a-abd6ce4640cd * fix(aw-sync): close datastore workers on sync_run error returns Datastore::close() stops the sqlite worker thread. The success path already did that; the new persist-then-return error paths skipped it, so a long-lived daemon could leak connections across failed passes. Git-Session-Id: 01a0aabd-c1af-70b2-807a-abd6ce4640cd * fix(aw-sync): record incompatible-version peers as skipped on the SyncReport open_peer_datastore returned Ok(None) on a user_version mismatch, so a peer rejected for an incompatible schema silently vanished from the report — an all-incompatible pass looked like a clean empty one in status and JNI (Greptile P1 on #699). The mismatch reason is now carried back and each incompatible peer is recorded as skipped. Git-Session-Id: b1d605fd-5f81-5e6b-9293-7353ee952e7c * fix(aw-sync): persist aggregate report when a later host pull fails Simple host-based sync merges per-host pull reports. A later host's `?` dropped earlier hosts from last-sync-report.json, leaving only the failing host's inner persist. Catch, record, persist the aggregate, then propagate — same contract as the push-failure path. Git-Session-Id: ee496982-751b-50ab-a42d-742b7c76cf44

This was referenced Sep 16, 2026

Sync: tracking issue and v0.14.0 release triage ActivityWatch/activitywatch#1445

Open

fix(aw-sync): make default daemon use host-layout pull/push #685

Closed

greptile-apps Bot reviewed Sep 16, 2026

View reviewed changes

ErikBjare mentioned this pull request Sep 16, 2026

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

Merged

3 tasks

ErikBjare mentioned this pull request Sep 16, 2026

fix(aw-sync): open peer databases read-only; skip version mismatch #700

Merged

ErikBjare merged commit 5c12d3d into ActivityWatch:master Sep 16, 2026
8 checks passed

This was referenced Sep 16, 2026

fix(aw-sync): sanitize whitespace hostnames on import; make per-bucket errors non-fatal #697

Merged

feat(sync): reconcile owner-originated event edits #678

Merged

TimeToBuildBob mentioned this pull request Sep 16, 2026

fix(aw-sync): skip dot-directories when walking peer databases #702

Merged

ErikBjare mentioned this pull request Sep 16, 2026

fix(aw-sync): skip broken peers in sync_run instead of aborting the pass #703

Merged

ErikBjare mentioned this pull request Sep 17, 2026

aw-sync: daemon (the default subcommand) never pulls — two incompatible sync-folder layouts #682

Open

Navigation Menu

Uh oh!

fix(aw-sync): do not stage a local db on pull-only sync_run - #698

fix(aw-sync): do not stage a local db on pull-only sync_run#698
ErikBjare merged 2 commits into
ActivityWatch:masterfrom
TimeToBuildBob:fix/aw-sync-pull-only-no-stage

TimeToBuildBob commented Sep 16, 2026

Uh oh!

TimeToBuildBob commented Sep 16, 2026

Uh oh!

greptile-apps Bot commented Sep 16, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Sep 16, 2026

Uh oh!

TimeToBuildBob Sep 16, 2026

Uh oh!

ErikBjare commented Sep 16, 2026

Uh oh!

codecov Bot commented Sep 16, 2026 •

edited

Loading

Uh oh!

TimeToBuildBob commented Sep 16, 2026

Uh oh!

TimeToBuildBob commented Sep 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

TimeToBuildBob commented Sep 16, 2026

Problem

Fix

Tests

Uh oh!

TimeToBuildBob commented Sep 16, 2026

Review guide (auto-posted)

Key files

Suggested verification

Known risks / watch points

Suggested review focus

Uh oh!

greptile-apps Bot commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Findings

Summary

Diagram

Uh oh!

greptile-apps Bot Sep 16, 2026

Choose a reason for hiding this comment

Uh oh!

TimeToBuildBob Sep 16, 2026

Choose a reason for hiding this comment

Uh oh!

ErikBjare commented Sep 16, 2026

Uh oh!

codecov Bot commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

TimeToBuildBob commented Sep 16, 2026

Uh oh!

TimeToBuildBob commented Sep 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps Bot commented Sep 16, 2026 •

edited

Loading

codecov Bot commented Sep 16, 2026 •

edited

Loading