Skip to content

docs: spec — update dialog must never block work - #255

Open
oxedom wants to merge 1 commit into
mainfrom
docs/spec-update-dialog-never-block
Open

docs: spec — update dialog must never block work#255
oxedom wants to merge 1 commit into
mainfrom
docs/spec-update-dialog-never-block

Conversation

@oxedom

@oxedom oxedom commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Design spec only. No code changes.

The report

"When there is a new version update, the window doesn't close."

Root cause

app-shell.tsx:788-800 derives the dialog's open state from three inputs:

const effectiveUpdateDialogOpen =
  updateDialogOpen || hasPersistentUpdateState || shouldPromptForUpdate;

Every close path (onOpenChange :1206-1212, onLater :1221) routes to handleUpdateLater() (:718-729), which clears updateDialogOpen and records dismissedUpdateVersion — but never touches hasPersistentUpdateState. Once the status is restart-required (the normal state after a download), the second term stays true forever and Later / X / Esc are all no-ops.

What verification changed

The spec was written first and then checked line-by-line against the code. Four claims turned out to be wrong, and the reality is worse than the draft:

  • The trap is total, not partial. At restart-required the dialog has zero forward actions: update-summary.tsx:195 explicitly hides "Update now" in that state, preload.cjs:44-127 exposes no restart channel, and quitAndInstall() (main.cjs:607) is unreachable from the renderer. The user gets a title saying "Restart Cabinet to finish updating" and no way to do it.
  • UpdateState has nine members, not seven (src/types/update.ts:7-16). Six latch.
  • The reminder pill already exists (status-bar.tsx:777-795) — it just misses failed and in-progress. This is an extension, not new work.
  • The two update mechanisms are not independent. electron/main.cjs:143 and src/lib/storage/path-utils.ts:11 write the same DATA_DIR/.cabinet-state/update-status.json. On macOS, update-downloaded writes restart-required before showing the native box; the renderer polls, sees it, and latches the React dialog behind it. Dismissing the native box leaves the stuck one. This is the likely shape of the report.

The fix

Dismissal keyed on version + state, so every state collapses to the existing non-blocking pill and a state change legitimately re-surfaces the dialog.

State Behaviour
restart-required Dismissible → pill. Needs a genuinely new restart action (none exists today).
failed Dismissible + "Try again" (only valid for source-managed installs; packaged macOS returns 409).
in-progress Minimizable, never blocking — a hung applying must not trap anyone.

Follow-ups flagged (not fixed here)

  • autoUpdater.on("error") writes failed (main.cjs:569-577) on any updater error, including routine unsigned/dev-build failures and transient network blips on the 4-hour check. That latches the dialog with no user action at all. Arguably the real bug.
  • restart-required needs a real action, not just dismissibility, or the pill points at nothing.

See ADR-0002.

🤖 Generated with Claude Code

Root-causes the "window doesn't close" report to the derived-open latch
in app-shell.tsx:788-800, where handleUpdateLater() clears two of the
three open-conditions and never touches hasPersistentUpdateState.

Verified against the code: the trap is total, not partial. At
restart-required the dialog renders zero forward actions, so the user
gets a title telling them to restart and no way to do it. The two update
mechanisms also share one status file, so on macOS the native box and the
React dialog both fire, which is the likely shape of the report.

Adds ADR-0002 recording that no update state may hard-lock the dialog.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@oxedom
oxedom requested a review from a team as a code owner July 16, 2026 11:30
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.

1 participant