fix(game-mode): unblock activation when a restore step can never succeed - #258
Conversation
A deactivation that leaves any item unrestored persists a residual snapshot with active:false, and activation refuses while one exists. Retry cleanup re-runs the same steps, so a step that fails deterministically - a vanished registry key, a removed service, a virtual network adapter gone after a reboot - locks Game Mode out permanently with no way back short of deleting game-mode-snapshot.json by hand. - Add a Discard escape hatch (IPC + confirm-on-second-click button) that drops a residual snapshot. Refuses while Game Mode is still active. - Surface the failure reason: persist it on the snapshot, expose it via GameModeStatus.pendingReason, and show it in the banner and retry toast instead of a bare count. - Scrub grouped steps per item, so one stuck registry value or network interface no longer keeps the healthy ones pending and re-applied forever. - Treat structurally unrestorable targets as restored: missing service, missing registry key, and a powerSaveBlocker ID left by a previous process. - Move the banner strings into the gameMode namespace (they were hardcoded). Fixes #241
![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: 44385dc81d
ℹ️ 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".
| deleteSnapshot() | ||
| return { discarded: true } |
There was a problem hiding this comment.
When the inactive snapshot is readable but its file cannot be unlinked, such as because of a user-data ACL or Windows file lock, deleteSnapshot() swallows the exception and this path still returns discarded: true. The renderer consequently hides the cleanup banner and reports success even though the snapshot remains and activation is still blocked; report failure unless the file was actually removed, while treating an absent file as success.
Useful? React with 👍 / 👎.
| residual.restoreErrors = errors.map((e) => ({ | ||
| optimizationId: e.optimizationId, | ||
| reason: (e.reason ?? 'Unknown error').slice(0, 500), | ||
| })) |
There was a problem hiding this comment.
When a valid snapshot produces more than 20 failed restore steps, this writes every error even though validateSnapshot rejects any restoreErrors array longer than 20. This is reachable because activation accepts up to 30 optimization entries without requiring uniqueness and the snapshot service array is unbounded; afterward readSnapshot() treats the residual as absent, clears the pending status, and permits a new activation to overwrite the original baseline. Limit the persisted array to 20 or align the validator with the maximum possible output.
Useful? React with 👍 / 👎.
1 participant
Fixes #241.
The deadlock
Game Mode can lock itself out permanently. It's reproducible by design, not machine-specific:
active: falseinstead of being deleted.deactivateGameModeon the same residual. A step that fails deterministically fails identically every time, and there was no escape hatch. The only fix was deletinggame-mode-snapshot.jsonfrom userData by hand.The reporter's "after restart PC from night" is the classic trigger: state captured before a reboot can reference things that no longer exist afterwards.
What changed
Escape hatch. New
game-mode:discard-pendingIPC and a Discard button (confirm on second click) in the cleanup banner. It drops the residual snapshot without restoring, and refuses while Game Mode is still active — discarding then would strand real changes. It also clears a snapshot file that fails validation, which previously sat on disk doing nothing.Say what failed.
deactivateGameModealready returnederrors[{optimizationId, reason}], but the UI only printed a count, so neither users nor we could tell which step was stuck. The reasons are now persisted on the snapshot (restoreErrors, bounded and validated like every other field), exposed asGameModeStatus.pendingReason, and shown in the banner, the retry toast, and the blocked-activation message — surviving an app restart.Per-item residual scrubbing.
sys-registry-tweaksandnet-disable-naglerestore N items but cleared all-or-nothing, so one bad entry kept all N pending and re-applied the healthy ones on every retry. Both now report which items failed and narrow the residual to just those.Targets that can't come back count as restored:
Get-Serviceguard instead ofSet-Service -ErrorAction Stopthrowing foreverTest-PathguardpowerSaveBlockerIdwritten by a previous process → dropped on load; it's process-scoped and meaningless after a restartThat last one means existing residual snapshots stuck only on a stale blocker ID self-heal on upgrade.
i18n. The banner strings were hardcoded English; they're now in the
gameModenamespace with the rest of the page.Testing
game-mode.ipc.test.tspreviously mirrored the validation rules rather than importing them. Added a mocked-electron/child_processharness (in-process fake snapshot file + scriptable PowerShell double) so the deactivation path is exercised for real — 17 new tests covering partial-failure scrubbing, reason persistence, retry clearing the snapshot, the missing-target guards, the stale blocker ID, and all three discard cases. The mirrored validator gained therestoreErrorsrules to stay honest.npm test— 2181 pass.npm run buildclean.Not verified on a real Windows box; the failure modes are reproduced through the PowerShell double rather than against live services and registry keys.