Skip to content

fix(desktop): release session runtimes before trash moves#4776

Closed
GTC2080 wants to merge 3 commits into
esengine:main-v2from
GTC2080:GTC/fix-session-trash-4768
Closed

fix(desktop): release session runtimes before trash moves#4776
GTC2080 wants to merge 3 commits into
esengine:main-v2from
GTC2080:GTC/fix-session-trash-4768

Conversation

@GTC2080

@GTC2080 GTC2080 commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

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

DeleteSession and TrashTopic removed the tab binding, then moved the .jsonl session artifacts while the removed controller still carried the original SessionPath() 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

  • Clear removed controllers' session paths after the final snapshot so later snapshots become no-ops.
  • Quiesce the tab autosave loop before moving trashed session artifacts.
  • Wait session destroy handles, close the matching removed controller before moving its files, then finish the destroy window after the move.
  • Track closed controllers so multi-session topic cleanup does not double-close shared runtimes.
  • Add regression coverage for both DeleteSession and TrashTopic on 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.go
  • git diff --check

@github-actions github-actions Bot added v2 Go rewrite (1.x) — main-v2 branch, active development desktop Wails desktop app (desktop/**) labels Jun 18, 2026

@SivanCola SivanCola left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 SivanCola left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@SivanCola

Copy link
Copy Markdown
Collaborator

Integrated into #4942, which combines the related desktop sidebar/session runtime fixes on top of the latest main-v2.

I kept the relevant contribution from this PR credited in the #4942 body and commit co-author trailers. Please use #4942 as the integration path for this set of changes.

@SivanCola

Copy link
Copy Markdown
Collaborator

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.

@SivanCola SivanCola closed this Jun 20, 2026
@GTC2080 GTC2080 deleted the GTC/fix-session-trash-4768 branch June 22, 2026 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

desktop Wails desktop app (desktop/**) v2 Go rewrite (1.x) — main-v2 branch, active development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: 会话无法移至回收站

2 participants