* fix(store): plugin updates keep the old install until the new one succeeds
Both reinstall paths in update_plugin — the monorepo-migration remote
switch AND the routine archive update every store user hits — deleted
the installed plugin directory BEFORE downloading its replacement. A
mid-update failure (bad network, registry error) permanently destroyed
the plugin. Seen in the field: a Pi with broken DNS lost 12 plugins in
one update pass during the monorepo migration.
New _reinstall_with_rollback: rename the old install aside (using the
'.standalone-backup-' name pattern plugin discovery already excludes),
run install_plugin, remove the aside on success — restore it on ANY
failure, clearing partial-download debris first. A stale aside from a
previous crash is cleared before starting.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam
* fix(store): serialize concurrent updates per plugin, check cleanup results
CodeRabbit review on #405 flagged two things in
_reinstall_with_rollback, both verified against current code:
- Real race: the web UI runs Flask with threaded=True and there's a
single update route, so two overlapping requests for the same
plugin_id (double-click, two tabs) can interleave. The loser could
rename the winner's in-progress install aside mid-download, deleting
its own rollback safety net — worse than the bug this function
exists to fix. Added a lazy per-plugin_id lock dict (mirrors the
plugin_manager per-plugin lock pattern) held for the whole function.
- _safe_remove_directory's return value was ignored at both call
sites. Stale-aside cleanup failure now aborts cleanly instead of
falling through to a rename that would fail anyway with a less
useful error; post-success backup-removal failure now logs instead
of failing silently (still returns True — the update itself
succeeded, and the next update self-heals the leftover aside).
Left the third nitpick (test_stale_aside_from_previous_crash_is_cleared)
addressed by asserting the stale dir is actually gone and that
install_plugin was reached, rather than just the end-to-end result.
Added a concurrency regression test asserting install_plugin never
runs for the same plugin_id while another call is in flight.
---------
Co-authored-by: Chuck <chuck@example.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
