Add change-storage-location (move / adopt / warn + restart)#30
Merged
Conversation
Settings → Storage location gains a "Change…" button. Picking a folder classifies the change (Core StorageRelocationService): - target already has a store -> adopt it (current store left in place; this is the conflict/warn case, surfaced in the confirm copy) - target empty, current has a store -> move store + icons there - neither -> just adopt the empty target On confirm, the new path is persisted and the app restarts (IAppRestartService -> AppInstance.Restart). The storage path is only read at startup, so apply-and-restart keeps the immutable startup singletons consistent and avoids the running app writing snips to the old location after the switch (a live-repoint would need a store-indirection refactor + Windows runtime verification — deferred deliberately). New Core: IStorageRelocationService/StorageRelocationService, IFolderPickerService, IAppRestartService; SettingsViewModel gains ChangeStoragePathCommand and an observable StorageDirectory. App: WindowsFolderPickerService, WindowsAppRestartService, DI + XAML wiring. 20 new Core tests (relocation outcomes + move; VM move/adopt/cancel/picker-cancel). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Picking a folder inside the current storage directory (e.g. its icons/ subfolder) would make MoveStore copy a directory into itself and could delete the just-copied store. Inspect now returns a new Invalid outcome for either-way nesting, and the change command notifies and aborts. Found by codex review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
In the move case, copy the store/icons, persist the new StoragePath, and only then remove the originals. Previously the old store was deleted before the config was saved, so a failed PersistAsync (locked settings file, full disk) could leave the app pointing at an emptied old location — snips appearing lost. Split MoveStore into CopyStore + RemoveStore to allow the safe ordering. Found by codex review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AppInstance.Restart returns a failure reason rather than guaranteeing termination, so IAppRestartService.Restart now returns bool. The change-path flow never deletes the old store from the running process (left as a backup); on a failed restart it notifies the user to restart manually, so the app stays consistent against the still-intact old store. Removed the now-unused RemoveStore. Found by codex review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
If AppInstance.Restart can't restart, the running app stays bound to the old store while the persisted setting would point future launches at the new one — so edits before a manual restart would be lost. On restart failure, revert the persisted StoragePath to the previous value so this session and future launches stay consistent on the old store, and notify the user it didn't take. The copied target files remain as a harmless backup. Found by codex review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the "Storage path: move / adopt / warn-on-conflict" carried-over item (task #8). Settings → Storage location becomes editable via a "Change…" button.
Behaviour (the three flows)
Picking a folder is classified by
StorageRelocationService.Inspect:store.json+icons/are moved there.Each is confirmed first; cancelling (or cancelling the folder picker) is a clean no-op.
The storage path is only read at startup (Bootstrap resolves it once;
ISnipStore/BackupService/IconAssetStorageare immutable singletons). Rather than a risky live-repoint refactor I couldn't runtime-verify, a confirmed change persists the new path and restarts the app (IAppRestartService→ WinAppSDKAppInstance.Restart).This is a correctness choice, not just convenience: if the app kept running against the old store after the switch, edits made before a manual restart would silently land in the old location and appear lost. Restarting eliminates that window. Trade-offs / alternatives I considered are in the commit message; happy to switch to live-reload if you'd prefer (bigger refactor).
What's where
IStorageRelocationService+StorageRelocationService(classify + move),IFolderPickerService,IAppRestartService;SettingsViewModel.ChangeStoragePathCommand+ observableStorageDirectory.WindowsFolderPickerService(FolderPicker),WindowsAppRestartService(AppInstance.Restart), DI registration, the "Change…" button.Tests
StorageRelocationServiceTests(5: all Inspect outcomes + a real move of store+icons against temp dirs) andSettingsViewModelTestsstorage cases (4: move, adopt-leaves-current, cancel, picker-cancel). Full suite: 157 passing.Needs your manual verification (can't be done from Linux)
AppInstance.Restartcall actually restarts the unpackaged app, and that after a real Move/Adopt the relaunched app loads from the new folder. IfRestartmisbehaves, the config is already persisted, so a manual relaunch still picks up the new path correctly.🤖 Generated with Claude Code