fix(aw-sync): default sync dir to documented data dir, migrate legacy ~/ActivityWatchSync

[TimeToBuildBob]

Problem

aw-sync's default sync location was ~/ActivityWatchSync, created in the user's
home directory. A stock install makes this visible immediately: aw-tauri's
first-run autostart module list ships { name = "aw-sync", args = "daemon" }, so a
fresh install runs the sync daemon and writes ~/ActivityWatchSync/<uuid>/test.db
without the user opting into sync at all — clashing with the documented data
directories (ActivityWatch/activitywatch#1418).

What changed

• get_sync_dir() now defaults to data_dir()/activitywatch/aw-sync on desktop,
  matching aw-server's data_dir()/activitywatch/<component> convention
  (aw-server/src/dirs.rs::get_data_dir) and aw-sync's own config dir
  (config_dir()/activitywatch/aw-sync). Android keeps its app-scoped historical
  location, and the AW_SYNC_DIR / --sync-dir overrides are unchanged.
• If ~/ActivityWatchSync already has content, that path is kept. aw-sync's
  transport is an external folder synchronizer (Syncthing/Dropbox/etc) watching
  that directory; auto-renaming it would disconnect existing setups, and a failed
  cross-device rename would start a fresh empty tree beside live data. New
  installs with no legacy dir get the documented location. An empty leftover of
  ~/ActivityWatchSync does not displace live data already in the documented
  dir. Existing users can delete the old folder or set AW_SYNC_DIR to switch.

Testing

Unit tests in aw-sync/src/dirs.rs cover the resolution paths:

• prefers_existing_legacy_sync_dir
• uses_documented_dir_when_no_legacy
• prefers_legacy_even_if_documented_also_exists
• empty_legacy_does_not_displace_populated_documented

cargo test -p aw-sync --lib   # 17 passed (incl. 4 new)
cargo clippy -p aw-sync       # no new lints in aw-sync

Closes ActivityWatch/activitywatch#1418

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge.

Summary

The PR changes the desktop default sync root to the platform data directory while preserving existing legacy folder-sync setups.

• Resolves the sync root using legacy-directory existence, contents, and filesystem-error handling.
• Keeps Android’s historical app-scoped location and preserves explicit CLI and environment overrides.
• Updates CLI documentation, README guidance, and directory-selection tests.

_{Reviews (6) · Last reviewed commit: "style(aw-sync): fix extra blank line in ..."}

[codecov]

Codecov Report

❌ Patch coverage is 52.17391% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.06%. Comparing base (656f3c9) to head (9aa33e9).
⚠️ Report is 130 commits behind head on master.

Files with missing lines	Patch %	Lines
aw-sync/src/dirs.rs	52.17%	11 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #667      +/-   ##
==========================================
+ Coverage   70.81%   80.06%   +9.24%     
==========================================
  Files          51       74      +23     
  Lines        2916     8260    +5344     
==========================================
+ Hits         2065     6613    +4548     
- Misses        851     1647     +796     

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

@greptileai review

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

Greptile convergence adjudication

Hit the review-attempt cap. Not retriggering Greptile.

Verified against: edb18794be6f537118ec2fe4a66a9d5fb7531821 (current head; Greptile last-reviewed this SHA). Our own reviewer has no open findings.

CI: all green (ubuntu, windows, macOS, Android, clippy, format, coverage). mergeable=MERGEABLE, mergeStateStatus=CLEAN.

Convergence: round_convergence.status=new_blocking, stable_rounds=0 (required 2). Five rounds, each with a new P1 on resolve_sync_dir. Classic diminishing-returns loop, not a leftover bug.

Fixed (already on this head)

Finding	Class	Where
Failed fs::rename (EXDEV) abandoned live sync state	blocking	dropped auto-rename in 630caae
Rename disconnected Syncthing/Dropbox transport	blocking	same; keep existing ~/ActivityWatchSync
Empty leftover legacy dir displaced populated documented dir	blocking	68643b7
read_dir error treated as empty → silent switch	blocking	bcdd1ce (dir_has_entries → Option<bool>)
exists() mapped metadata error to absence	blocking	edb1879 (try_exists fail-closed)

Remaining (non-blocking)

Finding	Class	Dismissal
Empty ~/ActivityWatchSync still wins when documented is absent, empty, or unreadable	accepted trade-off / product call	Empty leftover already yields to a populated documented dir (empty_legacy_does_not_displace_populated_documented). Always-switch would disconnect a Syncthing/Dropbox root created from old docs and fail-open onto an unreadable documented dir. New installs with no leftover still get the platform data dir. Greptile withdrew this at 18:58Z (3907389302). Summary score stays 4/5 because the summary body was not re-edited after withdrawal.

This session: no code change. Resolved the six leftover Greptile threads (five outdated+fixed, one withdrawn). Did not retrigger review.

Domain risk (maintainer glance)

aw-sync default-dir selection:

1. Fresh install, no leftover folder → documented data_dir()/activitywatch/aw-sync.
2. Existing Syncthing/Dropbox watching ~/ActivityWatchSync → keep that path.
3. Empty leftover + populated documented dir → documented (covered by test).

The product call in (remaining) is whether an empty leftover with no documented content should keep the old folder. I would keep it. Switching is a silent transport break for anyone who created the folder from old docs before the first sync.

Not auto-merging (cross-repo, no merge rights). Waiting on maintainer judgment.

[TimeToBuildBob]

🤖 AI code review

This PR changes aw-sync's default sync directory resolution. On desktop, get_sync_dir() now prefers the platform data dir (data_dir()/activitywatch/aw-sync) unless a non-empty legacy ~/ActivityWatchSync exists, in which case the legacy path is kept. Android keeps the historical home-dir location. The PR adds a resolve_sync_dir() helper with unit tests covering legacy/documented precedence, and updates README and CLI help text accordingly.

Safe to merge — no P0/P1 findings

Confidence 5/5

✅ No findings in this PR's changed files. What follows is outside them — not about this change, and not scored.

1 out-of-scope finding (outside this PR's changed files)

Each item here is an observation about the PR as a whole — informational only, not scored.

⚠️ P2 medium — PR-level

The unit test metadata_error_on_legacy_keeps_legacy sets the parent directory's permissions to 0o000 and then calls resolve_sync_dir. On Linux, when the parent is inaccessible, legacy.try_exists() returns Err, so the function returns legacy. However, the test then restores permissions and asserts the result. The test is correct in that it verifies the fail-closed behavior. But there is a subtle issue: the test runs as root in some CI environments (e.g., Docker containers running as root), where permission bits are not enforced, so try_exists() may return Ok(true) and dir_has_entries may succeed, causing the assertion to fail. The test is not hermetic because it depends on the effective user. This is a test defect: it can fail in root environments, giving a false failure signal. The fix is to skip the test when running as root (e.g., check nix::unistd::geteuid() or `std::env::var(

1 advisory finding (summary-only, not scored)

These P2 guard, heuristic, trade-off, or documentation claims are retained for judgment without opening review threads.

⚠️ P2 medium — aw-sync/src/main.rs

This is a fix(...) PR but no test files are included in the diff. Erik's feedback: 'where is the repro & fixes they are supposed to catch' (gptme#3441), 'that measurement should come with a regression test' (gptme#3446). Add a test that would have caught this bug. (Advisory: Erik merged all such PRs but consistently requested tests.)

Add a test file that reproduces the bug before the fix and passes after it.

_{How this was verified: static preflight: fix-commit + touched-files scan (rule 7)}

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

• aw-sync/README.md — Updates documentation of the default sync directory and working paths to reflect the new data-dir default and legacy fallback.
• aw-sync/src/dirs.rs — Adds default_sync_dir(), legacy_sync_dir(), dir_has_entries(), and resolve_sync_dir(); changes get_sync_dir() to use the new resolution logic on desktop. Adds unit tests for the resolution paths.
• aw-sync/src/main.rs — Updates the --sync-dir help text to describe the new default resolution order.

Previous review passes

commit	score	findings	engine	when
edb18794be6f	3/5	1	llm	2026-09-16 08:43 UTC

_{Reviewed 0b93c1063a88 · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 105s · about this reviewer}

_{Maintainer commands}

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

[TimeToBuildBob]

Rebased onto current master (d1a61b1, after #697 landed). The only real conflict was aw-sync/README.md, where #697's one-shot-pass rewrite met this PR's default-dir paragraph — resolved by keeping the one-shot usage text and merging the default-dir semantics (platform data dir default, existing non-empty ~/ActivityWatchSync kept, empty leftover does not displace live data). cargo test -p aw-sync green locally.

[TimeToBuildBob]

Rebased again onto current master (70ba50d, after #711/#712/#713/#716 landed). One conflict in aw-sync/src/dirs.rs: the test module had both master's new daemon-pull-mode/sync-config tests and this PR's migration tests appended after the same shared test (rejects_non_data_leaf_and_root), causing a diff-alignment conflict rather than a logical one. Resolved by keeping both test groups intact. cargo build -p aw-sync, cargo test -p aw-sync --lib (68 passed), and cargo clippy -p aw-sync --all-targets all clean (no new lints from this change — remaining warnings are pre-existing and unrelated). Force-pushed 0b93c10 → cd2d3cb.

[TimeToBuildBob]

@greptileai review

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

[ErikBjare]

@TimeToBuildBob The reason for keeping this is that the folder is expected to be set under sync by the user (with Syncthing, rsync, Dropbox, or something else). Putting it under the data dir makes it difficult to see/understand/locate.

[ErikBjare]

@TimeToBuildBob But tauri shouldn't ship with sync enabled by default, it's still under development/preview.

[TimeToBuildBob]

Makes sense, thanks for the context — I hadn't weighed the "folder needs to be easy to find/point Syncthing/Dropbox at" requirement, and moving it into the OS data dir works against that on purpose.

Root cause per your second comment: aw-tauri's UserConfig::default() unconditionally pushes aw-sync daemon into the autostart module list (src-tauri/src/lib.rs around the modules.push(ModuleEntry::Full { name: "aw-sync", ... }) call), on every platform, regardless of whether sync has been configured. That's what makes a stock install create ~/ActivityWatchSync with zero opt-in — closing this PR and fixing it there instead: don't autostart aw-sync by default while it's still preview/under development.

Closing this PR (the sync-dir-under-Syncthing default should stay as-is) and opening the aw-tauri fix now.