Repository navigation
feat(sync): automatic one-shot staging reset (#296) - #328
TimeToBuildBob wants to merge 3 commits into
Conversation
|
|
@greptileai review |
|
Ready for maintainer review: all three Greptile P1s are fixed in e0bcfec. The re-review on that head scores 5/5, CI is green (Test + E2E), and every thread is resolved. I can't merge this one myself (pull-only access on aw-android), so it's waiting on a maintainer. |
|
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. |
🤖 AI code reviewSafe to merge — no P0/P1 findings on latest reviewUpdated 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
Adds a one-shot staging-db reset for the full-sync path: a new StagingReset object (Android-free, unit-tested) runs a native resetStaging(hostname) call once per hostname, marking a SharedPreferences flag only on a JSON success response, and SyncInterface invokes it inside the syncBoth executor task before syncBoth. AWPreferences gains per-hostname getter/setter for the flag, and a new test file covers the runOnce logic. Needs a look — P2 onlyConfidence 4/5 1 finding ·
|
| commit | score | findings | engine | when |
|---|---|---|---|---|
e0bcfec8d72e |
3/5 | 1 | llm | 2026-10-07 09:32 UTC |
8ee56bef48c1 |
3/5 | 2 | llm | 2026-10-14 11:26 UTC |
8ee56bef48c1 |
4/5 | 4 | llm | 2026-10-14 12:28 UTC |
Reviewed 4e0745965b8e · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 101s · about this reviewer
Maintainer commands
@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.
|
The Rust side is now up in ActivityWatch/aw-server-rust#815. It adds |
Implement automatic one-shot reset of the phone's staging database to remove pre-ActivityWatch/aw-server-rust#713 duplicates. The reset: - Runs once per device (SharedPreferences flag) - Executes before first push after upgrade - Runs under syncInFlight guard (no concurrent access) - Requires corresponding Rust JNI implementation in aw-sync/src/android.rs Changes: - Add JNI binding for resetStaging(hostname) - Implement one-shot logic with device-specific preference tracking - Integrate into sync pre-operations (push and full-sync paths) - Add unit tests for one-shot behavior - Document Rust-side implementation requirements The reset removes staging files for own device only, preserving peer data. Subsequent push rebuilds staging from clean local source, and desktop pull will recognize re-imported records as duplicates (via #713 fingerprint fix). Closes: ActivityWatch#296 Relates-to: ActivityWatch/aw-server-rust#717 Git-Session-Id: e1f2
…ive symbol - Add AWPreferences.isStagingResetDone/setStagingResetDone (the previous getBoolean/setBoolean calls did not exist). - Extract the one-shot flag logic into StagingReset.runOnce, free of Android types, and replace the Mockito tests (no Mockito dependency; the tests asserted nothing) with JVM tests of that logic. - Catch LinkageError: until aw-server-rust ships resetStaging, the native call throws UnsatisfiedLinkError, which escaped both catch (Exception) blocks and would kill the sync executor before its callback cleared syncInFlight. - Run the reset only inside the full-sync path, which is the one that holds syncInFlight (push/pull-all wrappers have no callers and no guard). - Drop STAGING_RESET_IMPLEMENTATION.md from the repo root; the Rust spec lives in ActivityWatch#296. Git-Session-Id: ac9fc99f-bc9b-569f-b59a-2817d7711006
e0bcfec to
8ee56be
Compare
|
Rebased onto master (resolved conflict in |
…rows contract The sync executor's performSyncAsync only catches Exception; any Error escaping StagingReset.runOnce would leave syncInFlight permanently true, wedging all future syncs as "already in flight". runOnce's doc says "Never throws", but the previous catch(Exception)+catch(LinkageError) left non-LinkageError Errors (e.g. AssertionError) able to escape. Switch to catch(Throwable) to make the "Never throws" contract unconditional. Add a test covering the non-LinkageError path. Git-Session-Id: be171056-5acd-5fb9-aece-a62a90399bf0
Summary
Kotlin half of #296: an automatic one-shot reset of the phone's push staging db, so pre-ActivityWatch/aw-server-rust#713 duplicate copies stop being re-imported by desktops. Item 2 of ActivityWatch/aw-server-rust#717 (item 1, desktop dedupe, merged in ActivityWatch/aw-server-rust#718).
What it does
StagingReset.runOnce(Android-free, unit-tested): skips if this hostname's flag is set; otherwise calls the native reset and sets the flag only on{"success": true}. Failures leave the flag unset, so the reset retries on the next sync. It never throws, includingUnsatisfiedLinkError.SyncInterface: declaresexternal fun resetStaging(hostname), and runs the reset inside the full-sync executor task, after the legacy-folder migration and beforesyncBoth. That path holdssyncInFlight, so the reset cannot race a push writing the same staging db. (The push/pull-all wrappers have no callers and no guard, so they are left alone.)AWPreferences.isStagingResetDone/setStagingResetDone(hostname): the flag is keyed per hostname because staging lives under{sync_dir}/{hostname}/.Dependency: the Rust side is not written yet
resetStagingmust be added toaw-sync/src/android.rs(spec in #296: deletetest.db/-wal/-shmfor the own device dir only, refuse paths outside it). Until a build carries it, the native call throwsUnsatisfiedLinkError. That error is caught and logged, the flag stays unset, and sync carries on, so this PR is safe to merge before the Rust PR. The reset then fires on the first sync after the submodule bump.Verify before closing #296
android-test/android-unlockbuckets are not duplicated. If they are, re-staging reproduces the copies.Relates to #296, ActivityWatch/aw-server-rust#717, ActivityWatch/aw-server-rust#713, #295.