Skip to content

(file-panel): ask before quit, close or reload drops unsaved file edits (#373) - #401

Merged
devsuitup merged 4 commits into
mainfrom
fix/373-unsaved-edits-guard
Oct 2, 2026
Merged

devsuitup merged 4 commits into
mainfrom
fix/373-unsaved-edits-guard

Conversation

@devsuitup

Copy link
Copy Markdown
Owner

Closes #373

What changed

Quitting, closing the window or reloading while a file tab of the file panel has unsaved edits now asks first. This covers the shown tab, tabs kept aside by #369, and tabs of other sessions.

  • Main (unsaved-guard.js): the window's close event (every app quit goes through it) is held while the renderer is asked over unsaved-check / unsaved-check-result. A will-prevent-unload (the renderer's beforeunload veto, which a reload hits) asks the same way and reloads on a yes. No answer within 2.5 s, a crashed or destroyed renderer, or a failed send all answer yes, so a hung renderer cannot keep the app from quitting.
  • Renderer (file-panel.js): an in-app dialog built on the add-project dialog's classes lists the dirty files with Save / Discard / Cancel. Save writes the shown tab through the viewer's own save (stale-disk confirm included) and the tabs kept aside through saveFileForPanel with their agreed base; a refused save keeps the dialog open with the reason. beforeunload vetoes an unload while a tab is dirty, unless the user just approved one.
  • ViewerPanel.saveNow() exposes the existing save.

Conventional default taken: warn, never persist drafts silently.

Not covered / decisions

  • Closing a session does not drop its file panel (destroySession leaves filePanelState alone), so nothing is lost there; the quit check sees those tabs. Deleting a session leaves its state in memory too, so its edits are asked about at quit. No change on those paths.
  • Memory and Work Files panels, MCP diff tabs and Changes buffers are not covered; this is the file-tab slice named in the issue.
  • A slow (not hung) renderer that misses the 2.5 s bound can lose its edits; chosen over any chance of a deadlock.

Tests

  • test/unsaved-guard.test.js (8, main side, fake window and injected timers): hold and approve, no keeps open, no duplicate question, timeout, late answer ignored, crashed renderer, blocked unload reload, approved close lets unload through. Written first; red with Cannot find module '../unsaved-guard'.
  • test/dom-file-panel-unsaved-guard.test.js (7, jsdom with the real ViewerPanel and file-panel.js): clean = immediate yes; Cancel; Discard + beforeunload; Save; refused save keeps the dialog; other session plus kept-aside tab; beforeunload while dirty. Written first; red with ctx.calls.check is not a function.
  • Mutations, each turning one test red: timeout answers no; the beforeunload approval flag ignored; Discard not setting the approval; the approved-close shortcut removed; Save reporting success on refusal; held tabs not collected.
  • Not verified: a real Electron window (no second Electron against the live app). The close / will-prevent-unload semantics follow the Electron docs and are exercised only against fakes.

Unsaved edits in the file panel were lost silently when the app quit, the
window closed or the page reloaded. Main now holds the window close and
a blocked unload until the renderer answers, with a bounded wait so a hung
renderer cannot prevent quitting. The renderer lists the dirty tabs (shown,
kept aside, any session) in a Save / Discard / Cancel dialog and vetoes
beforeunload while a tab is dirty.

Closes #373
@devsuitup

Copy link
Copy Markdown
Owner Author

Reviewing dead7ae (adversarial review in progress).

@devsuitup

Copy link
Copy Markdown
Owner Author

Adversarial review at dead7ae: changes needed. (1) The 2.5 s guard timer runs until the user clicks, so a user who takes longer than 2.5 s is answered "yes" and the window closes with the edits — the timeout must cover only receipt, not the human. (2) before-quit cleanup (PTYs killed, MCP, watchers, remote indexer) runs before the windows close, so Cancel leaves an open app with dead sessions — the check has to happen before that cleanup. (3) A timed-out reload re-arms the veto and loops. Checked and holding: will-prevent-unload preventDefault() semantics, Discard disarms once, destroySession keeps filePanelState, partial Save failure leaves a consistent state; 15 targeted tests pass, lint 0 errors.

…ter quit is confirmed

The renderer now acknowledges an unsaved-edits check on receipt; the bound
covers only send to ack, so a slow user, a slow save or a stale-disk confirm
no longer counts as yes. The check moves into before-quit, ahead of the PTY,
MCP and watcher cleanup, so Cancel leaves the app intact. An approved reload
allows exactly one unload, so a timeout yes cannot loop.

Refs #373
@devsuitup

Copy link
Copy Markdown
Owner Author

Reviewing b8380ef (adversarial review in progress).

@devsuitup

Copy link
Copy Markdown
Owner Author

Re-review at b8380ef: the blockers are closed — the bound now covers only send→ack (renderer acks on receipt), the check runs in before-quit before any cleanup and re-quits once on yes, an approved reload unloads exactly once; re-entrancy and a gone renderer resolve without loop. Being fixed: autoUpdater.quitAndInstall() spawns the installer before app.quit(), so a Cancel leaves the installer waiting — the check will run before it; Windows session end will pass through; a close check and a quit check will share one dialog.

… question at a time

electron-updater starts the installer before it quits, so the unsaved-edits
check now runs before quitAndInstall and a Cancel leaves the installer
unstarted. A Windows session end approves the quit so logoff does not wait
on a dialog. A window close, a quit and an install share one open question.

Refs #373
@devsuitup

Copy link
Copy Markdown
Owner Author

Re-review at 29ae117: closed — updater-install awaits confirmQuit(mainWindow) before quitAndInstall(), so a Cancel never starts the installer and a yes pre-approves the quit that follows; query-session-end/session-end approve the quit; a close, a quit and an install share one in-flight question. Ready to merge once CI is green on 29ae117.

@devsuitup
devsuitup merged commit 639694c into main Oct 2, 2026
10 checks passed
@devsuitup
devsuitup deleted the fix/373-unsaved-edits-guard branch October 2, 2026 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(file-panel): quitting, reloading or closing a session drops unsaved file edits without a warning

1 participant