feat: auto-save and full session restore on macOS system shutdown - #75
Conversation
macOS shutdown/restart/logout no longer gets cancelled by the save dialog: - powerMonitor 'shutdown' latches an explicit shutdown intent; before-quit joins the same quit-approval controller (user-initiated quit unchanged) - shutdown branch saves dirty path files without any dialog, keeps untitled content in the session snapshot (empty windows excluded), and commits cleanExit:false plus a one-shot restoreReason:'shutdown' marker atomically - next launch auto-applies the shutdown snapshot without the recovery banner; crash restore keeps the existing banner flow (no regression) - failed file saves fall back to the session snapshot (no data loss); a shutdown commit after an approved quit supersedes the quit fence instead of silently no-oping; a denied shutdown releases the latch so later Cmd+Q behaves exactly as before - close-dialog smoke: legacy 7 scenarios isolated per-scenario userData in the same order, plus shutdown-restore and file-failure-restore pairs driven through the real beginSystemShutdown() path
When an approved quit is superseded by a denied system-shutdown pass, fall back to the already-approved quit instead of retaining a dead latch/fence. Smoke runner: share one userData across the legacy seven scenarios (cold per-scenario isolation exposed a pre-existing save-close flake), bound shutdown first-phase waits, force-exit failed Electron workers, and focus CodeMirror before synthetic edits so shutdown pairs stay deterministic.
Keep the shutdown persist snapshot shape local to api-types so knip stays clean; only the Api surface needs to re-export consumer-facing types.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8117c19e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🤖 Psycho-842 [BOT] review
⚖️ Verdict: NEEDS REVISION — Codex empty-target power-off denial is real on HEAD; OS power-off can stick after preventDefault().
Summary
macOS power-off path still looks solid for the dirty-path / untitled / empty-window matrix covered by unit + close-dialog-smoke, and tip CI is green. Settlement re-measure cannot promote this PR: an unresolved Codex P1 on empty close targets is confirmed in CloseCoordinator.request + approveAllForQuit('shutdown') — empty target lists fall out of the decision loop as approved: false while the powerMonitor handler has already called preventDefault(), so the app can block OS power-off/restart/logout when there are no windows or only unready blanks.
Findings (by severity)
- 🔴 P1 High —
src/main/close-coordinator.ts(request, emptytargets) +src/main/app-windows.ts(approveAllForQuit('shutdown')) +src/main/main.ts(powerMonitor.on('shutdown')→preventDefault) — Codex thread confirmed. With zero close targets (no open windows, or every window filtered as unready blank),pending = []skips the decision loop and returns{ approved: false }. Because power-off already calledpreventDefault(), a denied approval leaves Notepad AI alive and can block the OS action. Fix: treat an empty shutdown transaction as approved and commit the empty/filtered session (then allow quit); add a unit/smoke case for zero-target shutdown. - 🟡 P3 Low —
src/main/app-windows.ts(createWindow→loadFile) — Short-lived empty third window in close-dialog smoke can be destroyed mid-loadFile, surfacingUnhandledPromiseRejectionWarning: ERR_FAILED. Smoke still passes; guard the load promise. - 💡 SUGGESTION —
.github/workflows/ci.yml—test:close-dialog-smoke(legacy 7 +shutdown-restore/file-failure-restore) is still outside theverifyjob. - 💡 SUGGESTION — Real-device matrix in the PR body (power-off / restart / logout) is still empty; latched smoke is not a live
powerMonitorevent.
Response to bot findings
- Codex P1 — Allow shutdown when there are no close targets (discussion): Accepted as P1 (unresolved). Measured on HEAD
c8117c1:CloseCoordinator.request:pending = [...targets];while (pending.length > 0 && …)never enters whentargetsis[], thenreturn { approved: false, intent }.approveAllForQuit('shutdown')builds targets asrecords.filter(not unreadyBlank); empty registry or all-unready-blank → empty targets → denied.main.tspower-off listener alwayspreventDefault()beforebeginSystemShutdown().- Minimal empty-path reproduction: empty targets →
approved: false. No test currently asserts zero-target shutdown approval.
- Not dismissed. Blocks Stage 2 SAFE/CAUTION until fixed or explicitly waived by user.
Findings delta (since Stage 1)
- Stage 1 on same HEAD had no unresolved P0–P2 (Codex thread landed in the same minute window and was not yet folded in).
- Settlement: Codex P1 new / still open → max-severity P1 → NEEDS REVISION. No
[FINAL]declaration.
Detailed analysis (CI, diff stats, verification log)
Meta (re-measured settlement)
- Repo:
project820/notepad-ai· PR #75 - HEAD:
c8117c19e471522e82785919f4913a8b14bddc70(unchanged since Stage 1) - Diff: +1289 / −153 · 24 files · mergeable=MERGEABLE · mergeStateStatus=CLEAN
- Unresolved review threads: 1 (Codex P1 empty close targets)
- No
[FINAL @…]on this HEAD prior to this update
Hotspots re-opened
src/main/close-coordinator.ts— empty-target control flowsrc/main/app-windows.ts—approveAllForQuit/ unready-blank filter /decideShutdownsrc/main/main.ts—powerMonitor+preventDefault+beginSystemShutdownsrc/main/lifecycle-flags.ts— latch / supersede / fallback (unchanged concern)
Caller tracing (empty-path)
- macOS
powerMonitor'shutdown' →preventDefault→beginSystemShutdown→ controller →approveAllForQuit('shutdown')→CloseCoordinator.request(intent, targets, …) - Empty
targetsnever reachescommitCloseTransaction/commitShutdownSession; denial bubbles to refused quit approval while the OS action is already deferred.
Adversarial pass (settlement — 4 angles)
- Async/ordering — In-flight quit + supersede/fallback still looks correct when targets exist (prior Stage 1 evidence). Empty-target path is the new hole.
- Boundaries/failure — File-save fail → dirty snapshot still OK when targets exist. Empty targets: fail-closed incorrectly for OS power-off (should approve + quit).
- Contract — Unready blanks are excluded from persist and only force-approved after
result.approved; if approval never comes, they never get the free pass either. - Claimed invariants — "power-off does not cancel the OS action" is broken for the no-window / all-blank cases.
CI (HEAD tip)
verifyonc8117c1: SUCCESS (completed 30+ min stable at settlement)- Local Stage 1 evidence still cited: focused lifecycle tests 106 pass;
test:close-dialog-smokepass including restore pairs (does not cover zero-window path)
Unverified
- Live macOS power-off / restart / logout matrix (PR table still empty)
- Full
npm run testre-run in this settlement session (tip CI already green)
Policy note
- This COMMENT is not an approval. User final sign-off still required after the P1 is fixed.
🛠️ NEEDS REVISION
🦀 Reviewed by Hermes agent BOT 🦞
CloseCoordinator treated empty target lists as denied. On macOS the powerMonitor handler has already preventDefault()'d, so a zero-window or all-unready-blank shutdown left the app alive and blocked the OS. Empty shutdown now commits the empty session (cleanExit:false + marker) and returns approved; empty quit/close still deny. Unit coverage for coordinator + approveAllForQuit zero-window / all-blank paths.
Response to Codex P1 / Psycho NEEDS REVISIONStatus: fixed on P1 — empty close targets block power-offAccepted and fixed. Root cause matched the review: empty Fix: Tests:
Other notes from Psycho
Verification (
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5624c3a6df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Restored or file-backed windows that are still loading (ready=false) cannot complete the renderer lease handshake, so decideShutdown used to cancel after powerMonitor already preventDefault()'d and strand OS power-off. Unready restorable windows now mint a main-owned lease and commit the durable main-side snapshot (restoreSnapshot / lastSnapshot / path-only disk fallback) without querying the renderer. Empty blanks stay excluded as before.
Follow-up: Codex P1s on
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fb0bcb1b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Ready crash-recovery windows can still show a blank renderer while restoreSnapshot holds the only recovered draft. Prefer that durable snapshot over an empty live shutdown persist so markShutdownRestoreQueued does not drop it. Path-only file-backed snapshots (doc empty) are now restorable so an unready loading window reopens after power-off instead of being filtered out of the shutdown session.
Follow-up: Codex P1/P2 on
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 367c8e83e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Prefer unanswered restoreSnapshot whenever the live renderer doc is empty (including path-backed recovery drafts) - Bound renderer shutdown save to 3.5s so slow disk cannot miss main's deadline; always return a post-save snapshot (dirty cleared on success) - Path-only shutdown restores reopen via openFileInCurrent
Follow-up: Codex round on
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a799a6126
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Prefer pending recovery when it has doc or chat and live has neither - session:clear clears in-memory restoreSnapshot/restoreReason after decline - Clean path-backed shutdown restores reopen from disk (dirty still applies snapshot)
Follow-up: Codex round on
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce5de2067a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…#73) - Clean path-backed shutdown restore reopens disk file and still applies chat/view metadata from the snapshot - Path-only ordinary crash restores reopen the file (no blank untitled) - Live session:write clears pending restoreSnapshot after apply
Follow-up: Codex round on
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8185337f13
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Identity-only Codex thread on tip
|
1 participant
Summary
Closes #73. Related backlog: #74.
On macOS system shutdown / restart / logout, Notepad AI no longer cancels the OS action with a dirty-document dialog. Instead it:
CloseIntent: 'shutdown'viapowerMonitor+createQuitApprovalController.beginSystemShutdown().cleanExit: falseplus a one-shotrestoreReason: 'shutdown'marker atomically.No new UI / i18n strings / settings toggle. Session schema stays v2 (optional fields only).
Commits
2e9190bfeat: auto-save and full session restore on macOS system shutdown (feat: macOS 시스템 종료 시 자동저장 및 전체 세션 복구 #73)7f8993dfix(close): supersede quit fallback and harden shutdown smoke (feat: macOS 시스템 종료 시 자동저장 및 전체 세션 복구 #73)c8117c1chore: unexport internal ShutdownSessionSnapshot type (feat: macOS 시스템 종료 시 자동저장 및 전체 세션 복구 #73)Commit branch matrix
cleanExit:false+restoreReason:'shutdown'(one-shot)Smoke isolation / order
mainand this branch; not a product regression).shutdown-restoreandfile-failure-restoreeach use pair-local userData.beginSystemShutdown()(NOTEPAD_AI_CLOSE_SMOKE_TRIGGER=shutdown); file-save fault viaNOTEPAD_AI_SMOKE_FAIL_FILE_SAVE=1.Verification (all green on HEAD
c8117c1)npm run preflight:tessdatanpm run typechecknpm run test— 149 files / 2422 testsnpm run buildnpm run test:security-e2enpm run test:converter-e2enpm run test:html-export-directnpm run test:roundtrip-smokenpm run test:close-dialog-smoke— 7 legacy + shutdown-restore + file-failure-restore (3 consecutive greens at residual fix + reconfirm after knip fix)npm run knipnpm audit --omit=dev --audit-level=high— high findings are pre-existing onorigin/main; this PR does not changepackage.json/package-lock.jsonArchitect re-review: CLEAR / CLEAR / CLEAR · APPROVE (prior HIGH supersede residue resolved).
Red-team adversarial cases a–e (empty-window exclusion, crash banner, one-shot marker, ⌘Q regression, file-failure dirty fallback): all passed.
PII 4-surface scan
Real-device manual QA (user-owned — leave blank until exercised)
Merge policy
Do not merge without an explicit user approval that names this PR number. No
--auto/ self-merge / admin merge.