fix(gui): prefetch the asset index without devices and re-arm sync for new models#220
Conversation
…r new models The background asset sync only fired once a snapshot carried model info, so a machine whose receiver reads are flaky (#218) could miss its one chance: no model info, no sync, no index.json — every device stranded on the synthetic silhouette with nothing retrying the bootstrap. Split the trigger from the model gate: - The first agent snapshot — even a deviceless one — prefetches the index, so resolution works the moment a device first appears. - Depot fetches fire per model key and fold into a synced set on success, so a model set that grows later re-arms the sync instead of being latched off by the session-wide DONE state. - The AtomicU8 state machine becomes a single-flight flag plus an outcome channel into the select loop, which now also rebuilds the resolver after every successful run, not just the first. Failure backoff (1s doubling to 60s) is unchanged; success resets it so a newly paired device syncs immediately.
Greptile SummaryThis PR decouples the background asset-sync trigger from requiring an initial device snapshot with model info. The first agent snapshot — even a deviceless one — now fires an index prefetch, and per-depot syncs re-arm whenever new model keys appear in
Confidence Score: 4/5Safe to merge; the state-machine replacement is mechanically sound and correctly handles the select-loop exit, backoff, and re-arm edge cases. The core logic is correct: The re-arm logic in Important Files Changed
Sequence DiagramsequenceDiagram
participant A as Agent IPC
participant S as Select Loop
participant T as Sync Thread
participant FS as Filesystem
Note over S: startup: sync_enabled = should_run(...)
Note over S: index_refreshed=false, synced_keys={}
A->>S: GuiUpdate::Snapshot (no devices)
Note over S: pending=[], !index_refreshed → trigger sync
S->>T: spawn(run_asset_sync([]))
T->>FS: fetch index.json
T->>S: "SyncOutcome{ok:true, keys:[]}"
Note over S: index_refreshed=true, cache rebuilt, assets_dirty=true
A->>S: GuiUpdate::Snapshot (device_A appears)
Note over S: force_refresh=take(assets_dirty)=true
Note over S: pending=[model_A] → trigger sync
S->>T: spawn(run_asset_sync([model_A]))
T->>FS: re-fetch index.json, fetch depot for model_A
T->>S: "SyncOutcome{ok:true, keys:[key_A]}"
Note over S: synced_keys={key_A}, cache rebuilt, assets_dirty=true
A->>S: GuiUpdate::Snapshot (device_B paired)
Note over S: pending=[model_B] → trigger sync
S->>T: spawn(run_asset_sync([model_B]))
T->>FS: re-fetch index.json, fetch depot for model_B
T->>S: "SyncOutcome{ok:true, keys:[key_B]}"
Note over S: synced_keys={key_A, key_B}
Reviews (1): Last reviewed commit: "fix(gui): prefetch the asset index witho..." | Re-trigger Greptile |
| std::thread::spawn(move || { | ||
| let next = if sync_assets_if_needed(&inv) { | ||
| SYNC_DONE | ||
| } else { | ||
| SYNC_FAILED | ||
| }; | ||
| state.store(next, Ordering::Release); | ||
| let keys = pending.iter().map(model_key).collect(); | ||
| let ok = run_asset_sync(&pending); | ||
| let _ = tx.send(SyncOutcome { ok, keys }); | ||
| }); |
There was a problem hiding this comment.
sync() swallows per-depot errors (each sync_depot failure is logged and continued; sync() returns Ok(())). Because keys is pre-computed from pending before run_asset_sync runs, every model attempted in the run — including those whose depot download silently failed — gets folded into synced_keys on success. A device whose depot 404s or fails transiently will never be retried within the session, leaving it on the synthetic silhouette. The old SYNC_DONE latch had the same effect, so this is pre-existing behavior, but the re-arm design makes it worth noting: per-depot outcomes aren't currently distinguishable from the thread, so truly partial failures can't be singled out for retry without a more granular result type.
Resolve the IPC poll conflict against the startup-states rework and adapt the diagnostics adapter to master: - call store_agent_snapshot before the status snapshot moves into AgentLink::Ready; the inventory merge keeps the InventoryHealth gate (AprilNEA#213/AprilNEA#215/AprilNEA#220) - read accessibility from the retained AgentStatus; the AppState.accessibility_granted field no longer exists - map DeviceRoute::Unifying (AprilNEA#181) to a new ConnectionKind::UnifyingReceiver
- Compute the asset-cache size once when the Settings window opens and store it on the view, instead of re-walking the cache dir on the main thread every render. (Greptile) - Extract the macOS Finder reveal into a cfg-gated helper so the early return isn't the function's last statement on non-macOS — CI clippy flagged needless_return on the Linux build (local macOS clippy can't see that path). The original commit also skipped the manual sync on an empty inventory to keep the old SYNC_DONE latch from wedging the auto-sync gate; the rebased loop has no such latch (it re-arms per model set, #220) and an index-only refresh is first-class there (#218), so that guard is dropped.
- Compute the asset-cache size once when the Settings window opens and store it on the view, instead of re-walking the cache dir on the main thread every render. (Greptile) - Extract the macOS Finder reveal into a cfg-gated helper so the early return isn't the function's last statement on non-macOS — CI clippy flagged needless_return on the Linux build (local macOS clippy can't see that path). The original commit also skipped the manual sync on an empty inventory to keep the old SYNC_DONE latch from wedging the auto-sync gate; the rebased loop has no such latch (it re-arms per model set, #220) and an index-only refresh is first-class there (#218), so that guard is dropped.
- Compute the asset-cache size once when the Settings window opens and store it on the view, instead of re-walking the cache dir on the main thread every render. (Greptile) - Extract the macOS Finder reveal into a cfg-gated helper so the early return isn't the function's last statement on non-macOS — CI clippy flagged needless_return on the Linux build (local macOS clippy can't see that path). The original commit also skipped the manual sync on an empty inventory to keep the old SYNC_DONE latch from wedging the auto-sync gate; the rebased loop has no such latch (it re-arms per model set, #220) and an index-only refresh is first-class there (#218), so that guard is dropped.
Problem
The background asset sync only fired once an agent snapshot carried at least one device with HID++ model info. On a machine whose receiver reads are flaky (#218), that single gate can never open: no model info → no sync → no
index.json→ every device stays on the synthetic silhouette, and nothing ever retries the bootstrap. The reporter's GUI loggedno asset index found — using synthetic silhouetteon every poll with a permanently empty~/.local/share/openlogi/assets.Change
Decouple the index prefetch from the model gate, and make the sync re-armable:
sync()already fetchesindex.jsonbefore looking at models, so an empty model list now lands the registry on disk ready for the first device sighting.HashSet; a model set that grows later (device wakes up, new pairing) starts a new run instead of being latched off by the old session-wideSYNC_DONE.AtomicU8state machine. A single-flight flag plus aSyncOutcomechannel into the select loop; the resolver now rebuilds after every successful run (previously only the first), so renders landing in a later round still become visible.Failure backoff is unchanged (1s doubling to 60s, first attempt immediate); success resets it so a newly paired device syncs without waiting out a stale failure delay.
OPENLOGI_SYNCand the bundled-assets release gate (should_run) behave as before, checked once at startup.Notes
index.jsononce per session.Testing
cargo clippy -p openlogi-gui --all-targetsclean, full-workspace pre-push clippy clean.cargo test -p openlogi-gui: 27 passed.sync_retry_delayunit tests;sync()'s index-before-models order is pre-existing, covered by readingfetch_index_to_dirahead of thetargets.is_empty()early-return.