Skip to content

Add an explicit End session action to the menu - #74

Open
ethduke wants to merge 2 commits into
krishhgg:mainfrom
ethduke:codex/end-session-menu
Open

ethduke wants to merge 2 commits into
krishhgg:mainfrom
ethduke:codex/end-session-menu

Conversation

@ethduke

@ethduke ethduke commented Oct 11, 2026 •

Copy link
Copy Markdown

Summary

Adds End session to the right-click menu during an active session, giving users a fallback for the hold-control problem reported in #60. It uses the existing restoration path and leaves Insomnia open. The reported gesture failure itself has not been reproduced or changed.

Refs #60.

Validation

  • Hosted macOS 26 CI passed on 73cfa73: 1,228 tests, 10 skipped, zero failures; release build with warnings treated as errors, shell checks and workflow lint passed. Greptile passed with no review findings.
  • 90 focused local tests passed, including native menu dispatch, journal cleanup, stale actions, in-flight extension and restoration failure. These used injected system dependencies and private journals in a documented macOS 15 compatibility copy; the deployment/arithmetic workarounds are not included in this PR.
  • Contributor device report: macOS 27.0 (26A428), M2 Max, Xcode 27.1, with macOS 27: allow the DisplayServices calls and read Low Power Mode from powermode #58, Menu bar: route status item clicks by the cursor on macOS 27 #76 and a local Comet-list edit also applied. The contributor reported 1,256 passing tests and a real 5-minute session ended through the menu, with sleep/App Nap restored, a clean journal and Insomnia still running. This is evidence for that combined tree, not a device test of this PR alone.

Limitations

The local native preview used fake power dependencies; it did not reproduce the original hold gesture failure or verify a live power-restoration click on this laptop. The implementation provides an explicit fallback rather than changing the hold control.

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; no blocking issues were found.

Summary

Adds End session to the right-click menu while a session runs. It uses the existing restoration path and leaves Insomnia open.

  • The right-click menu can end an active session without closing Insomnia.

No new actionable issues were found. No previous review threads were supplied.

Reviews (5) · Last reviewed commit: "fix: wait for session restoration before..." · Reviewed by Greptile

@nsd97

nsd97 commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Tested on macOS 27.0 (26A428), MacBook Pro (Mac14,6) M2 Max, Xcode 27.1, with #58, #76 and a local Comet list change also applied. The README conflict with #76 was kept as #76's text. On that tree swift test ran 1,256 tests with 0 failures, EndSessionMenuTests included.

During a 5-minute session, right-click, then End session, ended it: "session end (user)", "sleep restored", "app nap restored for 20 app(s)". The journal was clean afterwards and Insomnia kept running. Together with #76 this gives macOS 27 two ways to end early.

@krishhgg

Copy link
Copy Markdown
Owner

End-to-end test in a VM on macOS 27.0.1

End session shows in the right-click menu while a session runs, and clicking it ends the session the same way the ring does: sleep is restored, App Nap is restored, and the app keeps running.

Setup

A Tart VM on the maintainer's Mac mini, macOS 27.0.1 (26A434) with Xcode 27.0. Each version was built from source and installed with scripts/install.sh, then driven by a script inside the VM using Accessibility and posted mouse events.

Main (76e517d) against this PR's head (73cfa73). The script (t74.sh) started a session by clicking the eye and pressing Enter, right-clicked the eye, read the menu items, and clicked End session.

Results

main this PR
Menu items during a session status lines, Settings…, Quit Insomnia status lines, End session, Settings…, Quit Insomnia
After clicking End session (no item) log: session end (user), sleep restored, app nap restored for 20 app(s)
Notification text (from the log) Session ended - Ended by you. Sleep is back to normal.
Afterwards no session.json, SleepDisabled 0, Insomnia still running

The menu during a session, main on top and this PR below:

menu main

menu this PR

The orange "Sleep still held with no session" line in main's menu comes from main, not this PR. The menu opened during main's 5 s start check, before the log said the session had started. The PR's menu opened after it.

Not covered

  • The notification banner. Notifications are off for the app in the VM, so only its text in the log was checked.
  • macOS 26, real hardware, and the lid.

The full log is run-full-lab27.log.

🤖 Generated with Claude Code

@krishhgg krishhgg closed this Oct 18, 2026
@krishhgg krishhgg reopened this Oct 18, 2026
@krishhgg
krishhgg marked this pull request as draft October 18, 2026 03:41
@krishhgg
krishhgg marked this pull request as ready for review October 18, 2026 03:41
@krishhgg
krishhgg marked this pull request as draft October 18, 2026 05:35
@krishhgg
krishhgg marked this pull request as ready for review October 18, 2026 05:35
@krishhgg

Copy link
Copy Markdown
Owner

Thanks for this. One thing to fix before it can merge: the new test can fail on CI even when the app does the right thing.

The test stops waiting before the end finishes

EndSessionMenuTests.swift:125 waits until the session is no longer active and the UI shows idle. But performEnd clears the session before it awaits restoreAll. So the UI can look idle while sleep restoration, the journal cleanup or the extension's completion is still running, and the asserts on lines 127 to 131 then run too early.

This happened on CI. With this head merged into main ad96f8b, the macOS 27 job failed at line 129:

EndSessionMenuTests.swift:129: error: -[InsomniaTests.EndSessionMenuTests testMenuEndWhileExtensionWaitsLeavesNoSessionOrExtensionUI] : XCTAssertFalse failed

The same job passed on two reruns, so it is a timing race in the test, not a bug in the menu item.

Fix: have the test wait for an explicit signal that the end finished. Or extend the wait condition so it also waits for a clean journal on disk (h.store.loadState() not dirty) and pendingCountdown == nil.

Push the fix as a new commit on this branch (no rebase or force-push). CI, Greptile and the Codex review then run again on the new head. My VM test above covered 73cfa73, so I'll rerun it on the new head.

🤖 Generated with Claude Code

@krishhgg

Copy link
Copy Markdown
Owner

End-to-end test in a VM on macOS 27.0.1, rerun at 43f7523

End session still works at this head. It shows in the right-click menu during a session, and clicking it ends the session: sleep is restored, App Nap is restored, and the app keeps running. The results match the run at 73cfa73.

Setup

The same VM and method as the earlier run: macOS 27.0.1 (26A434) with Xcode 27.0, each version built from source and installed with scripts/install.sh. Main (ad96f8b) against this PR's head (43f7523), with the same script, t74.sh, run by run-fix.sh.

Results

main this PR
Menu items during a session status lines, Settings…, Quit Insomnia status lines, End session, Settings…, Quit Insomnia
After clicking End session (no item) log: session end (user), sleep restored, app nap restored for 20 app(s)
Notification text (from the log) Session ended - Ended by you. Sleep is back to normal.
Afterwards no session.json, SleepDisabled 0, Insomnia still running

The menu during a session, main on top and this PR below:

menu main

menu this PR

As in the earlier run, main's orange "Sleep still held with no session" line comes from main: its menu opened during the 5 s start check.

Not covered

  • The notification banner. Notifications are off for the app in the VM, so only its text in the log was checked.
  • macOS 26, real hardware, and the lid.

The full log is run-fix-lab27.log.

🤖 Generated with Claude Code

@ethduke
ethduke marked this pull request as draft October 19, 2026 20:48
@ethduke
ethduke marked this pull request as ready for review October 19, 2026 20:48
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.

3 participants