Skip to content

fix(sync): start local server before background sync - #379

Open
joecom7 wants to merge 2 commits into
ActivityWatch:masterfrom
joecom7:fix/background-sync-server-upstream
Open

joecom7 wants to merge 2 commits into
ActivityWatch:masterfrom
joecom7:fix/background-sync-server-upstream

Conversation

@joecom7

@joecom7 joecom7 commented Oct 22, 2026 •

Copy link
Copy Markdown

Background sync can cold-start through the alarm fallback without BackgroundService running. SyncWorker then calls aw-sync while its required loopback server is absent, producing Pull phase failed: all 1 peers failed and a ten-second timeout. Opening the app starts the server and makes sync work again. I reproduced this on a Galaxy S23 running Android 16 with ActivityWatch 0.14.3.

This change starts and polls the embedded server before native sync, sharing the foreground startup path's sync-folder migration lock. Readiness polling is bounded and interruptible, and failed background passes receive at most two WorkManager retries. Each attempt rechecks whether sync is enabled before initializing JNI. Native loading errors on the sync executor become reported failures through the existing callback; the separate server executor also catches missing JNI entry points and shuts down when startup exits.

The cold-start regression test runs in its own instrumentation process through make test-e2e-sync. Its setup temporarily disables the boot receiver and restores it on exit: Android 16 can deliver a deferred BOOT_COMPLETED when an app is unstopped, which would otherwise start the foreground service and mask this bug. The test checks that the foreground service is absent before and after the worker runs. Two additional device tests check that disabled sync skips both a queued first attempt and a pending retry.

Validation:

  • Current master: all 335 JVM tests pass; instrumentation Kotlin compilation passes.
  • On-device A/B against v0.14.3 with unchanged native libraries: original startup behavior reproduces the exact ten-second failure; patched cold sync succeeds without the foreground service.
  • Repeated cold sync while the phone remained dozing, and another cold sync with the persisted SAF folder grant, both pass. Ordinary scheduled export, external folder upload and downstream import were exercised successfully.
  • Both disabled-worker device tests pass. The installed v0.14.3-based patch also passes the cold-start and both disabled-worker tests together.
  • Local fault injection using a separate test APK with aw_sync present and aw_server deliberately omitted passes: two consecutive attempts report the native loading failure through the callback, without crashing or leaving the shared sync guard stuck. The temporary fault-injection harness and native binaries are not included in this PR.

The full upstream emulator suite has not been run locally.

@greptile-apps

greptile-apps Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 3/5

[Medium impact] The changes affect Android’s background and foreground sync paths.

Fix the post-disable retries and uncaught server library errors before merging.

Findings

  1. P1 Sync runs after being disabled ▶
  2. P1 Server library errors escape ▶

Summary

Starts the embedded server before native sync so alarm-triggered workers can recover without BackgroundService.

  • Cold background sync starts its local server before aw-sync runs.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Alarm-triggered worker] --> B[Sync executor]
    B --> C{Local server ready?}
    C -- No --> D[Wait for datastore rewrite]
    D --> E[Migrate sync folders under shared lock]
    E --> F[Start embedded server]
    F --> G[Poll for readiness]
    C -- Yes --> H[Native sync]
    G --> H
    H --> I[Copy files to selected folder]
    I --> J{Succeeded?}
    J -- Yes --> K[Finish worker]
    J -- No --> L[Retry up to twice]
    L --> A
Loading

Reviews (1) · Last reviewed commit: "fix(sync): start local server before bac..." · Reviewed by Greptile

} else {
Log.w(TAG, "Automatic sync failed: $message")
continuation.resume(Result.failure())
continuation.resume(if (runAttemptCount < 2) Result.retry() else Result.failure())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Sync runs after being disabled

If the user turns sync off while a retry is pending, that retry can still pull, push, and copy files. The new retry starts doWork() again, but that method checks only recent scheduler activity, not isSyncEnabled(). Turning sync off stops the scheduler without cancelling queued workers.

Check isSyncEnabled() at the start of each attempt so retries respect the user's choice.

Knowledge Base Used:

val url = URL(buildDashboardUrl("http://127.0.0.1:${BuildConfig.SERVER_PORT}/api/0/info", apiKey))
awaitSyncServer(
start = {
val rust = RustInterface(context)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Server library errors escape

If aw_server cannot load or lacks a required JNI method, the new RustInterface(context) call throws UnsatisfiedLinkError on the sync executor. Its handler catches only Exception. The handler in SyncWorker cannot catch errors from this other thread, so cold sync can crash the app instead of reporting failure.

Catch this error on the sync executor and report it through the callback.

Knowledge Base Used: Rust interface integration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant