fix(desktop): release session runtimes before trash moves#4776
Conversation
SivanCola
left a comment
There was a problem hiding this comment.
Approved after resolving the main-v2 conflict in commit ab7237e.
The resolution keeps the latest cleanup-pending / delayed-trash behavior from main-v2, while preserving this PRs core fix: removed session runtimes now snapshot, clear their session path, quiesce autosave, and release the matching runtime before the session artifacts are moved.
Verified locally:
- git diff --check
- cd desktop && go test . -run Test(DeleteSessionClearsRemovedRuntimeSessionPath|TrashTopicClearsRemovedRuntimeSessionPath) -count=1
The new PR CI run is still pending after the push.
SivanCola
left a comment
There was a problem hiding this comment.
Approved after the follow-up lint fix in commit 6f0b179.
The main-v2 conflict is resolved, the PR is mergeable, and the follow-up removed the unused helper reported by the desktop lint job.
Verified locally after the follow-up:
- git diff --check
- cd desktop && go test . -run Test(DeleteSessionClearsRemovedRuntimeSessionPath|TrashTopicClearsRemovedRuntimeSessionPath) -count=1
- cd desktop && go test ./...
Remote CI: all current checks are passing except the desktop job, which is still in progress on GitHub Actions and has not produced logs yet.
|
Closing this in favor of the merged integration PR #4942. The Windows session-trash fix from this PR was kept/adapted in #4942: removed runtimes are released before session artifacts move to trash, autosave is drained, and the regression coverage is included. @GTC2080 is credited in #4942's contribution table and commit co-author trailers. Thank you for the fix and tests. |
Problem
On Windows, moving an open desktop conversation to the trash could appear to do nothing after the user confirmed the action.
Fixes #4768.
Root Cause
DeleteSessionandTrashTopicremoved the tab binding, then moved the.jsonlsession artifacts while the removed controller still carried the originalSessionPath()and its tab autosave loop could still snapshot. Unix allows renaming an open file, but Windows can reject the move when the session file is still held or rewritten by the same process.Fix
DeleteSessionandTrashTopicon open runtimes.Verification
cd desktop && go test . -run 'Test(DeleteSessionClearsRemovedRuntimeSessionPath|TrashTopicClearsRemovedRuntimeSessionPath)'cd desktop && go test . -run 'Test(DeleteSessionClearsRemovedRuntimeSessionPath|TrashTopicClearsRemovedRuntimeSessionPath|DeleteSession|TrashTopic|Restore.*Topic|CloseTab.*Autosave|CloseTabSurvivorKeepsAutosave)'go test ./...cd desktop && go test ./...go vet ./...cd desktop && go vet ./...gofmt -l desktop/app.go desktop/tabs.go desktop/app_autosave_test.gogit diff --check