Pulling a peer whose bucket hostname contains whitespace 400s and aborts the whole sync pass (activated by #685)

[ErikBjare]

Reproduced, and it interacts badly with #685 — worth deciding before v0.14.0 ships.

#658 made bucket_new reject hostnames containing whitespace, with a carve-out for buckets that already exist. On the pull path, get_or_create_sync_bucket creates the destination bucket with the source bucket's hostname, unchanged. For any peer whose bucket hostname column contains a space, that first create is a 400, and the error aborts the entire sync pass.

Reproduction

Against the shipped v0.14.0-beta.5 (rust) binary from the bundle, on a throwaway db:

$ curl -X POST "localhost:5699/api/0/buckets/aw-watcher-android-synced-from-POCO%20F8%20Ultra" \
    -H "Content-Type: application/json" \
    -d '{"id":"aw-watcher-android-synced-from-POCO F8 Ultra","type":"currentwindow",
         "client":"aw-sync","hostname":"POCO F8 Ultra","data":{}}'
{"message":"Invalid hostname \"POCO F8 Ultra\": hostname may not contain whitespace"}
HTTP 400

$ curl -X POST ".../aw-watcher-android-synced-from-poco_f8_ultra" \
    -d '{... "hostname":"poco_f8_ultra" ...}'
HTTP 200

The first request is exactly what a pull of my POCO F8 Ultra constructs.

Why the peer hostname has a space

ActivityWatch/aw-android#272: aw-android added sanitizeDeviceHostname and renamed the sync folder to poco_f8_ultra, but never migrated the hostname column on the buckets. Read from the live staging db in my sync folder:

aw-watcher-android        | POCO F8 Ultra | aw-android | currentwindow
aw-watcher-android-unlock | POCO F8 Ultra | aw-android | os.lockscreen.unlocks
aw-stopwatch              | POCO F8 Ultra | aw-webui   | general.stopwatch

And get_or_create_sync_bucket derives the destination from the bucket's hostname, not the folder name. So the sanitized name never reaches the create.

This is not an exotic device name. Any Android phone whose name had a space before the sanitization shipped is affected — "Pixel 8 Pro", "Galaxy S24 Ultra", and so on.

Blast radius

The failure chain: create_bucket → 400 → map_err(…)? in get_or_create_sync_bucket → aborts sync_datastores → aborts sync_run → ? in pull_all → whole pass aborts, every peer after it skipped → daemon returns Err → exits → supervisor restarts → identical failure → budget exhausted → dialog, sync stopped (see #688).

Scope, stated precisely:

• Only when the destination server is aw-server-rust. aw-server-python has no whitespace check — I grepped aw_server/api.py / rest.py and there is none.
• The desktop bundle's default server is aw-server-python, so a default desktop install is unaffected.
• Android is affected — it embeds aw-server-rust. An Android device pulling a whitespace-hostname peer fails.
• Desktop users who opt into aw-server-rust are affected.

Why it is latent today and why that changes

Nothing on desktop currently pulls — that is #682. #685 turns pulling on. So merging #685 activates this for every aw-server-rust destination that has such a peer in its sync folder. The fix for #682 is what makes this reachable.

Options

Naive sanitize-on-import is wrong — see the correction note below before implementing.

1. Sanitize on import, reusing the legacy ID. get_or_create_sync_bucket must first look up the existing unsanitized destination ID (…-synced-from-POCO F8 Ultra) and keep using it if present; only new buckets get a sanitized ID. The raw hostname is preserved in bucket metadata either way. Without the lookup this forks (below).
2. Exempt sync-created buckets from the whitespace check. Narrower, but re-admits the values fix(server): reject bucket creation with whitespace in client hostname #658 was added to keep out.
3. Make it per-bucket non-fatal (aw-sync: per-peer and per-bucket errors abort the whole sync pass (and burn the supervisor's restart budget) #688). Necessary regardless, but on its own it silently drops the peer's data rather than failing loudly — better than dying, still a loss.
4. Migrate bucket hostnames on Android (Sanitized device hostname forks the sync identity and never migrates bucket hostnames aw-android#272). Correct long-term, ships on Android's cadence, does nothing for peers already in folders.

Suggest 1 (with the legacy-ID lookup) + 3 for v0.14.0, with 4 following on Android's own schedule.

Correction: why plain sanitizing forks

Anyone who pulled this device before #658 landed (Aug 26) already has a local bucket named …-synced-from-POCO F8 Ultra. If import starts sanitizing, it constructs …-synced-from-POCO_F8_Ultra, get_or_create_sync_bucket misses the existing bucket, and resume-from-newest starts from nothing — a full re-import, so every event appears twice in /timeline. That is the exact symptom of discussions#1373 that #648 was filed to fix.

The same fork hits aw-server-python destinations, which never needed sanitizing in the first place and would be broken by a fix aimed at a rust-only problem.

So the destination ID must be resolved as: existing legacy ID if one exists → otherwise sanitized ID. Sanitize the hostname field and new IDs, never an established one.

Credit: derived from a design review of aw-sync; I reproduced it against the shipped binary before filing.

Related: #685, #688, #682, ActivityWatch/aw-android#272, #658, ActivityWatch/activitywatch#1445.

cc @TimeToBuildBob

[ErikBjare]

Correcting my own recommendation above: plain sanitize-on-import is wrong and I've rewritten the Options section.

Anyone who pulled an affected device before #658 landed already has …-synced-from-POCO F8 Ultra locally. Sanitizing produces a different destination ID, get_or_create_sync_bucket misses the existing bucket, and resume-from-newest starts from zero — a full re-import, every event twice in /timeline. That is precisely the discussions#1373 symptom #648 exists to prevent. It would also fork aw-server-python destinations, which never had the whitespace problem.

The fix has to resolve the destination ID as: existing legacy ID if present → otherwise sanitized. Sanitize the hostname field and newly-created IDs only, never an established one.

Credit: caught in design review of this issue, not by me.

[TimeToBuildBob]

Implemented both options from above in #697:

Part 1 — sanitize on import with legacy-ID lookup (Option 1):

• get_or_create_sync_bucket checks the unsanitized ID first, so any device pulled before this fix keeps its established bucket (no re-import fork per v0.14.0b3 activitywatch#1373).
• If the bucket doesn't exist yet, whitespace → _ in both the ID and hostname field before create_bucket. New tests: test_whitespace_hostname_pull_creates_sanitized_bucket and test_whitespace_hostname_pull_reuses_legacy_unsanitized_bucket.

Part 2 — per-bucket non-fatal errors (Option 3 / #688):

• The ? in the sync_datastores bucket loop replaced with warn + continue. A broken or invalid bucket is skipped; healthy buckets still sync. Updated two existing tests whose assertions relied on the old fatal-error semantics (no-panic invariant preserved).

All 12 aw-sync tests pass.

[TimeToBuildBob]

Post-merge verification — fully resolved via #697 (merged 09:15:50Z, merge commit d1a61b12), which is what this issue's close tracks.

What is on master (b0fab735, after #703):

• sanitize_hostname() in aw-sync/src/sync.rs — byte-identical to aw-android's sanitizeDeviceHostname.
• Destination-ID resolution implements the corrected requirement: get_or_create_sync_bucket looks up the unsanitized ID first and reuses it when that bucket exists, so any device pulled before fix(server): reject bucket creation with whitespace in client hostname #658 keeps its established bucket (no re-import fork, per v0.14.0b3 activitywatch#1373). Only newly-created buckets get the sanitized ID and sanitized hostname field.
• The ? in the sync_datastores bucket loop is now warn + continue, so an invalid bucket no longer aborts the whole pass (Option 3).

Remaining, tracked on their own threads:

• Android-side hostname migration: Sanitized device hostname forks the sync identity and never migrates bucket hostnames aw-android#272 (Android's cadence).
• The per-peer error half landed separately as fix(aw-sync): skip broken peers in sync_run instead of aborting the pass #703; leftover tracking is on aw-sync: per-peer and per-bucket errors abort the whole sync pass (and burn the supervisor's restart budget) #688.

No further action from my side on this issue.