Skip to content

One-click start and sounds: land #63 as two switches - #156

Open
krishhgg wants to merge 45 commits into
integration/menu-bar-popoverfrom
integration/menu-bar-oneclick
Open

krishhgg wants to merge 45 commits into
integration/menu-bar-popoverfrom
integration/menu-bar-oneclick

Conversation

@krishhgg

@krishhgg krishhgg commented Oct 27, 2026 •

Copy link
Copy Markdown
Owner

Two new switches in Settings > General, both off by default:

  • Start a session with one click. In Pills, Compact and Popover, a click on the idle eye starts the default length (4 hours unless you changed it). A click during a session opens that style's usual controls, and a click never ends a session. Option-click on the idle eye opens the controls to pick another length, and in Popover a right-click or Control-click does too. Minimal greys the switch out, since its click already adds the default length.
  • Play a sound when you start or end a session. A short tone when a start or end you asked for in the menu bar succeeds. Nothing plays on an automatic end, on Quit, after a crash recovery, while the displays sleep or with the lid closed. Off, the sounds are never generated or loaded.

With both off, every style works as on #155.

This lands #63 by @vaststudios at e3840ca. The commit is merged here unchanged, so GitHub marks #63 merged when this one merges and its author keeps the credit. #63 replaces the right-click menu with a Liquid Glass quick menu, rebuilds Settings into toolbar panes and makes a click on the eye end a session, which conflicts with #147's one-page Settings, #93's press handling and the four menu bar styles, so it can't merge as it is. Here one-click start and the sounds ship as switches, and the rest is left out (listed below).

To @vaststudios: please don't push to your branch now. A new commit on it isn't in this pull request, so #63 would stay open instead of showing as merged.

The base is integration/menu-bar-popover (#155), which this stacks on. When #155 merges, this pull request's base moves down the stack and nothing else changes. #79 also contains #63's head; it is not part of this branch.

What ships from #63

  • One-click start, behind its switch, with Menu bar: add Liquid Glass quick actions and native settings #63's click tests adapted to it.
  • The start and end sounds (SessionSounds, Menu bar: add Liquid Glass quick actions and native settings #63's generated tones), behind their switch, with Menu bar: add Liquid Glass quick actions and native settings #63's sound tests.
  • The backstop relocation in scripts/backstop.sh: a copy of the script inside an app bundle resumes frozen processes with that bundle's own binary, which the LaunchAgent's codesign --verify --strict check already sealed, instead of the fixed ~/Applications path. Its test fails without it. It still runs under macOS's bash 3.2 and passes ShellCheck, and every restore it does is unchanged.
  • b180afa's approach of taking the whole Control-click before SwiftUI sees it, ported to the base's event monitor. The bug is on the base too: a Control-click opened the menu, and SwiftUI then fired the eye's tap on the release, which with one-click start on would start a session.

Not shipped: the click that ends a session; the Liquid Glass quick menu and glass style; the toolbar Settings panes, list editors and Command-number shortcuts (#147's one-page Settings stays); the animated countdown, the hours-and-minutes format and #63's Motion, PillView, EyeMark and hand cursor changes; the UI preview mode, its script and screenshots; docs/ui-refinement.md; RunningApps' icon changes; and the removal of the lid simulation line from the right-click menu. Presets and the 4 hour default are unchanged.

Conflicts resolved

git show --remerge-diff 9003a4e shows each one. Every file was resolved to keep the base's four styles and #147's Settings.

2458dfb merges #155's final head 499021c. Only run(mode:duration:) in the controller conflicted: the base added fromPopoverFields and this branch added showsRefusal, and both stay.

Maintainer commits

  • e516e71, 0d3c555, ff02323, 278a234: remove the toolbar Settings panes, the quick menu, glass style and hand cursor, the countdown animation and motion changes, and the preview mode, with their tests.
  • a011ebb: puts back the lid simulation line in the right-click menu and its test.
  • f923e61: the README and docs describe the base's clicks again.
  • a3162b5: DurationInput.from(seconds:) clamps before converting to Int, so a default length of 1e300 can't trap. A test helper runs each trap test in a child process.
  • d0095de: the one-click start checks the length as a Minimal click does and keeps the exact value for a retry.
  • 5c17d5a: one-click start becomes a switch, off by default. A refused start shows its reason without another click: the pills reopen with the length in the fields, the Compact panel opens with the error, or the popover opens with the reason.
  • 43d5ddb: the sounds become a switch, off by default. Compact and Minimal ends play too (Menu bar: add Liquid Glass quick actions and native settings #63 only hooked one end path), and a user end that lost the race to the timer's own end is silent.
  • 20a8143, b153b74: replace b180afa's tests, which were written for Menu bar: add Liquid Glass quick actions and native settings #63's button eye, and port its Control-click fix. 20a8143 said the fix wasn't needed before its test had run; b153b74 corrects that.
  • da29479: fixes the trap test helper's pass check and a tear-down that read NSApp in the helper's parent.
  • 7e2c53f: comments and docs say which binary the bundled backstop runs.
  • 94b4c4d: README, docs/spec.md and docs/releasing.md for both switches, with release checks.
  • 51881c0: the sound tests take down the key-catcher panel they open, which broke the next test class.
  • c98bf20: a refused one-click start opens the popover on empty fields. Before, a Custom draft from an earlier opening came back beside the reason, and Return started it.
  • 54b6ee1: the docs say the popover opens on the time buttons after a refusal.
  • 53ffa09: a test warning.
  • 1eb513e: a refused one-click start in Compact retries the click's length, to the second. The panel keeps a draft across closes, so Return started an older typed time instead.
  • f246b54: a one-click start in Compact that goes through with a warning, such as an App Nap setting that could not be written, clears the length it kept. Before, the next click opened the panel on that length, and Return added it again.
  • 3a7ee79: with the sound switch on, a cue reads the lid from the registry when it would play. The lid observer reports a close only after 2 s, so an end right after closing the lid on an external display played its tone. In debug builds, simulate-lid.sh no longer silences the tones, since the read sees the real lid.
  • 768b4ba: a refused one-click start in Pills and Compact opens on the default field from Settings, not the field focused last.
  • f4f89b5: in Popover, an Option-click or Control-click counts the keys held, as Pills and Compact do, not only the click's own flags. On a macOS 27.0.1 VM an Option-click on the idle eye started the default length and left the popover closed, 3 of 3. macOS 26.6.2 passed.
  • ea9f18d: Pills and Compact take an Option-click's keys from the press and the release. On a macOS 26.6.2 VM, Option let go 0.5 ms after the mouse-up started the default length in both styles, 2 of 2, because the tap read the keys held when it ran.
  • c77778e, b05534b: the status view takes the whole right click before SwiftUI sees it, as it already took a Control-click, and leaves the left button's release alone. Hosted CI on macOS 27 showed a right mouse-down and mouse-up sent to the status view firing the eye's tap, which with one-click start on starts a session. In the menu bar the menu's tracking takes the release, and a VM run on 27 saw real right-clicks start nothing, but the eye no longer depends on that.
  • 6c9d385: a right click or Control-click ends a press held on Minimal's eye before the menu opens. Since c77778e Minimal's own view no longer saw the right press, so a hold that outlived the menu could end the session.
  • b4c13ce: a right press keeps the keys of a left click still underway, and a tap reads only the keys of the last released click.
  • ca6c67e: VoiceOver's press on the eye in Pills and Compact reads only the keys held now, never keys kept from an earlier press.
  • 08ee3e0: a test comment.
  • 95a63e9: a test sends a right click's events back to back. SwiftUI makes no tap from a press held 0.6 s or more, and the test held one across three 0.2 s waits, so its macOS 27 cases failed on hosted CI.
  • 6dd3a82: on macOS 27 a status item's press and release reach the app with no modifier flags, so the status view keeps the keys held when it handles a left press on the eye. In a macOS 27.0.1 VM, 5 of 6 Option-clicks in Pills and Compact whose Option came up within 1 ms of the mouse-up started the default length. With this commit, 18 of 18 such trials opened the controls.
  • b6a36e0: on macOS 27, a click the app handles more than 50 ms after it happened, after a stall, counts as an Option-click in Pills, Compact and Popover, since the keys held by then may not be the click's.

Each behavior change has a test that fails without it.

Left for the owner

The safer choice is in place for each.

  1. Both switches are off by default, and a click never ends a session.
  2. With one-click start on, other lengths come from Option-click, and in Popover from a right-click or Control-click. The right-click menu in the other styles lists no lengths. Whether that is easy enough to find, or the menu should list presets, is open.
  3. A refused one-click start opens the style's own control with the reason, not a notification.
  4. In Minimal the switch is greyed out with a line saying why, not hidden.
  5. A VoiceOver press on the idle eye may count as an Option-click (VoiceOver holds Control and Option) and open the controls instead of starting a session. Which one VoiceOver users should get is open. In Pills and Compact it waits on the base's Accessibility press, which does nothing (below).
  6. Sounds play only for the menu bar's own starts and ends. --session start and --session end are silent.
  7. In Popover, retrying a refused one-click length with no matching chip means typing it, to the minute.

Found on the base and left for its own fix:

  • With every field empty, Return starts the default length, and a hand-edited default of 1e300 traps in the start log line (SessionManager.swift:1328).
  • A start typed in the Compact panel that goes through with a warning keeps the typed length in the panel, so the next Return adds it again. The same branch puts a refused extend's length back, which a test pins, so the fix changes the base's Compact path.
  • In Pills and Compact the status item's Accessibility press (AXPress, which VoiceOver uses) does nothing, with one-click start on or off, in macOS 26.6.2 and 27.0.1 VMs. The item's button has no action in those styles, as on main, and the eye's own SwiftUI button is not in the Accessibility tree. Popover's press works.

Limits

  • On macOS 27 a click carries no modifier keys, so the app reads the keys held as it handles the press, about 0.3 ms later in a VM. If a stall delays that by less than 50 ms and Option comes up in between, the click starts the default length. A longer stall opens the controls.
  • On macOS 27 a right press made while the left button is held on the item never reaches the app. In Minimal a right click during a hold opens no menu, and the hold ends the session as it would without the right click. On macOS 26 the menu opens and the session keeps running.

Tests

The full suite at 1eb513e: 1690 tests, 0 failures, on macOS 26.2. At b05534b the full suite passes on macOS 27.0.1 in a VM: 1697 tests, 0 failures. At b6a36e0 the click, one-click, hold and popover classes pass on macOS 26.2 (62 tests). The full suite there: 1704 tests, 1 failure, a race in main's settleAfterCommand that this stack doesn't touch; the class passed 10 of 10 runs alone. The release build is clean. Hosted CI passes on macOS 26 and 27.

Review

Clean at b6a36e0, round 9. Round 1 found a refused one-click start in Compact retrying an older typed draft, fixed in 1eb513e. Round 2 found a one-click start that went through with a warning leaving its length ready to extend the session, and a test a wrong fix would pass. f246b54 fixes both. Round 3 was clean. Greptile then found a tone right after a lid close and a refused start opening on the wrong field, fixed in 3a7ee79 and 768b4ba, and round 4 reviewed both. A VM run on macOS 27.0.1 then found an Option-click in Popover starting a session, fixed in f4f89b5, and round 5 was clean. A macOS 26.6.2 VM run and hosted CI on macOS 27 then found the Option release and right click problems fixed in ea9f18d to b05534b. Round 6 found that the swallowed right press no longer ended Minimal's press, that a right press dropped the Option of a left click still underway, and that kept keys could reach a VoiceOver press, fixed in 6c9d385 to 08ee3e0. Round 7 was clean. Hosted CI then found the test timing fixed in 95a63e9, and a macOS 27.0.1 VM run found the Option-clicks fixed in 6dd3a82. Round 8 found that a press handled late on macOS 27 still started a session and that a test rig read the Mac's keyboard, both fixed in b6a36e0. Round 9 was clean.

Evidence

VM runs on macOS 26.6.2 and 27.0.1 at this head pass every check except the Accessibility press, the base gap above. On 27 an Option-click with Option up right after the mouse-up now opens the controls 12 of 12, against 1 of 6 at 08ee3e0. A few checks need a real Mac and are in docs/releasing.md: Control-click through a real menu's tracking loop, a VoiceOver press on the eye, the sounds, and the backstop from a bundle moved out of ~/Applications.

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; no new blocking issue was found.

Summary

Adds two switches, both off by default: one-click session starts and session sounds. The bundled backstop also uses its own app binary after relocation.

Since the last review:

  • A click starts the default session in three menu bar styles.
  • Menu bar starts and ends can play short tones.
  • Bundled recovery scripts use their own app binary.

No new actionable issues found. Both earlier, unnumbered findings are fixed: sound playback reads the current lid state, and refused starts use the configured default field.

Tests and native UI checks were not run in this Linux environment.

Reviews (5) · Last reviewed commit: "Status view: a click handled late on mac..." · Reviewed by Greptile

vaststudios and others added 30 commits October 11, 2026 02:20
…DME and docs

main's README is now short (#65). Its Using it steps now describe this
PR's right-click start or stop, left-click quick actions and Command-drag,
keep the two Settings screenshots, and move the preview script note to
Development. docs/extras.md and docs/lid-close.md take this PR's Status
details and Paused apps wording, and the maintain-insomnia skill says
where the warnings are.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
The owner kept #147's one-page Settings. This removes the General, Lid
actions and Advanced panes, SettingsNavigation, the pane sizing, the
Command-number shortcuts, the list editors and app list that only those
panes used, and their tests (SettingsWindowTests and the freeze preview
test in UIStatusTests). SettingsWindow and SettingsView go back to the
base's versions; the two hand cursor lines the merge added there leave
with them.
The owner did not keep QuickMenuView or GlassStyle. Right-click goes
back to the base's menu (End session, the status rows and time left for
VoiceOver), and a click on the eye or countdown goes back to iconTapped
and customExtend, with the hold ring's onHoldEnd restored. StatusRootView
returns to the base's version, which also drops the Button wrapped
around the eye. HandCursor only served the removed controls.

The tests for quick menu items and the quick menu's Command-drag host go
with them. The one-click start in iconTapped and the sound hooks stay;
later commits put both behind their switches.
…hanges

Main keeps its rule of no per-second animation in the menu bar
(1cd9d3a): each animated frame re-renders the whole status item. The
owner did not keep #63's rolling countdown or its hours-and-minutes
format, so StatusCountdownView, CompactCountdown and their tests go.
Motion, PillView, EyeMark and EyeMarkGeometry return to the base's
versions, which keeps the base's blink, collapse and focus timings.
(The merge already kept the base's HoldToEndButton, so the end ring
stays in the label colour.) BrandingTests and the two motion tests in
UIStatusTests return to the base's expectations, and the controller's
focus glow delay goes back to 0.12 s.
The brief drops UIPreview, scripts/preview-app.sh, PreviewScriptTests,
docs/ui-refinement.md and the docs/assets/ui captures. The captures
show the toolbar Settings panes and quick menu, which are gone, and
the preview mode existed to inspect them. InsomniaApp and
ReminderScheduler return to the base's versions, except for the hook
that hands the status item #63's session sounds. The README loses its
paragraph on the preview.
The merge took #63's removal of the warning line that marks a build
with the scripts/simulate-lid.sh watcher compiled in, and of the test
that pins it. Main shows that line so such a build is never mistaken
for a release; #63 moved the notice to the logs only. That change came
with the dropped quick menu and is not part of what the owner kept, so
StatusMenu and LidSimulationTests go back to the base's versions.
The merge brought #63's wording for its quick menu: "Status details"
in docs/extras.md, the README and the maintain-insomnia skill, and a
session-flow picture of a left-click that starts and ends a four-hour
session with an hours-and-minutes countdown. The quick menu, the
click-to-end and that countdown are not kept, so these go back to the
base's text. A later commit documents the one-click switch.
from(seconds:) converted seconds / 60 to Int before clamping, so NaN or
a value out of Int's range traps. Settings cannot produce one, but a
hand-edited config can, and #63's one-click start passes the default
length straight to it. The seconds are now clamped to 0...30 days first,
and NaN reads as empty.

TrapProbe runs a test in a child xctest process, so a trap fails that
test instead of stopping the run. The three new tests (1e300, -1e300,
NaN) trap without the clamp.
#63's idle click passed config.defaultPreset straight to the manager,
which logs the requested seconds as an Int after sleep is already off.
A hand-edited 1e300 or -1e300 traps there. It also put the length in
the pills without keeping the exact value, so a refused 30 days and
1 minute came back as 30 days on the retry.

The click now checks the length with the same test as minimalTap
(now StatusItemController.canRequest) and keeps it with keepForRetry.
A length that fails the check opens the pills, as the base's click
does, so another length can be typed.

OneClickStartTests covers 1e300 and -1e300 (each traps without the
check, reported by TrapProbe) and the 30 days and 1 minute retry.
In #63 a click on the eye started the default length or ended the
running session, and its right-click quick menu held the other
lengths. With that menu dropped, the merge left the Pills idle click
starting the default length with no way to open the pills. The owner's
call is a switch, off by default: Settings > General, "Start a session
with one click", under the Menu bar picker.

Off, every style does what it did before. On, in Pills, Compact and
Popover, a plain left click with no session running, starting or
ending starts config.defaultPreset. Any other click does what it does
with the switch off: Option-click opens the pills, the Compact panel
or the popover, so every length stays reachable, and in Popover a
right click or Control-click opens the popover. A click never ends a
session. Minimal already adds the default length on every click, so
the switch is greyed out there and its line says so.

A refused one-click start shows why without another click, the way
each style shows a refusal: Pills reopens the pills with its label
(the base's path, which #63's own refusal branch skipped, Greptile's
finding at StatusItemController.swift:659), Compact opens the panel
and Popover the popover, each with the reason.

#63's three click tests in UIStatusTests now turn the switch on and
click the eye. The refusal test expects the pills back with the label,
and the running-session click opens the pills to add time instead of
ending the session.
b180afa made the status view take the whole right-click and
Control-click gesture before SwiftUI, because #63 turned the eye into
a SwiftUI Button that those clicks also pressed, which started or
ended a session. That eye is not kept: the base's eye takes its click
with onTapGesture, which a right click does not fire, and the host's
mouseDown takes a Control-click before SwiftUI sees it. b180afa's
filterMenuMouse never landed in this branch, so its tests no longer
build.

MenuClickEventTests sends real right clicks and Control-clicks to the
idle eye with one-click start on, on macOS 26 and on 27 (where the
host's pointer monitor sees every left click first). Each opens the
menu once, reaches no tap and starts nothing; a plain click on the
same eye starts a session. It passes without b180afa, so that fix is
not needed here.
#63 played a sound on every start and every user end, with no way to
turn it off. The owner's call is a switch, off by default: Settings >
General, "Play a sound when you start or end a session".

Off, nothing plays. The app's closure is the only thing that touches
SessionSounds, so nothing is generated or loaded either. On, a start
the menu bar asked for plays when the manager reports that this call
started the session (tryStart), and an end the menu bar asked for
plays when it found the session running and restored everything. The
Compact and Minimal ends (endPresentedSession) now play too; #63 only
hooked endNow.

Nothing plays while every display sleeps or the lid is closed, or for
an end the user did not ask for: the deadline, the recovery agent, a
quit, a floor or crash recovery. A user's end that waited behind the
timer's finds the session gone; SessionManager.lastEndReason (set by
the end that found the session) tells the controller that end was the
timer's.

#63's sound tests now turn both switches on, start with a click and
end with the hold, since a click no longer ends a session.
XCTest's name for a test here is "-[DurationInputTests testX]", without
the module, while the child's log line reads
"-[InsomniaTests.DurationInputTests testX]' passed". The probe looked
for the whole name, so every probe reported that its test did not run,
even when the child passed. It now matches from the class on.

OneClickStartTests' tearDown read NSApp.windows. In a probe's parent the
test body never runs, so nothing has made the application, NSApp is nil
and the run crashed. It now reads NSApplication.shared.
20a8143 said a Control-click never reaches the eye's tap, because the
host's mouseDown sends it to the menu without calling super, and that
b180afa was not needed. Its own test showed otherwise on its first
run: SwiftUI takes a press from the window before mouseDown runs, so a
Control-click opened the menu and the eye's onTapGesture still fired on
the release. With one-click start on, that tap starts a session. A right
click does not fire the tap.

The host's local monitor now runs on every macOS version. It takes a
Control-click's press, opens the menu and keeps the drags and release
from SwiftUI, the way b180afa (#63) took the whole menu gesture. The
next press ends the gesture, because the menu's own tracking usually
takes the release, so the click after a menu is not lost. Moving
relayed clicks and holding back early releases still happen on macOS 27
only. Popover style is unchanged: its clicks go to the status bar
button.

MenuClickEventTests sends a Control-click with its release, one whose
release the menu took followed by a stray release, and one followed by a
plain click. Without this change the stray tap starts a session on
macOS 26 and on 27.
#63 (e3840ca) made the copy of backstop.sh sealed in a bundle resume
frozen processes with that bundle's binary and Info.plist, so a bundle
moved out of ~/Applications finds its own binary instead of another
install's or none. A checkout's copy still uses ~/Applications. The
script's header and path comments, spec.md section 8, recovery.md and
ProcessIdentity's comment still said "the installed app binary". They
now say which binary each copy runs and why that is safe: the
LaunchAgent, install.sh and uninstall.sh each verify the whole bundle
with codesign --verify --strict before they run its copy, so the binary
is one the same check sealed.

Comments and docs only. The relocation test that came with #63,
RecoveryScriptTests.testBundledBackstopResumesWithItsSiblingBinaryAfterAppRelocation,
fails with the case block removed.
The README, spec.md and releasing.md described only the base's clicks.
They now cover the two switches this branch adds.

README: a "One click" item (what the switch does in each style, how to
reach another length, that a click never ends a session, Minimal greyed
out) and a "Sounds" item (off by default, what plays and what never
does).

spec.md: the two Settings entries in section 10 with their defaults, a
pointer from section 1, and a "One-click start" paragraph in section 11
with the clicks that still open the controls, the guarded default, the
refusal in each style and the Control-click the status view takes.

releasing.md: the Settings row check now lists both switches, and seven
new checks cover the switch off, the switch on in Pills, Compact and
Popover (on macOS 26 and 27, and a VoiceOver press), Minimal with the
switch on, a refused start's message, sounds off, sounds on, and an
automatic end with sounds on.
testFailedStartIsSilent makes a one-click start the recovery agent
refuses. In Pills a refused start reopens the pills with the reason,
and the key-catcher panel with them, and the test left that panel up.
The full suite runs UIStartupTests next, and its first test checks
that no key catcher is visible after a refused extend, so it failed
there and passed alone.

UIFeedbackTests now orders out any visible key-catcher panel after
each test, as UIStartupTests does after its own.
# Conflicts:
#	Sources/Insomnia/UI/StatusItemController.swift
A refused one-click start opens the closed popover to show why. The
popover keeps its fields once a start has been refused, so a Custom
draft left from an earlier opening, or a time refused then, came back
beside the reason with Custom out, and Return started that rather than
the default length. A closed popover's fields are now cleared before
it opens for the refusal. A popover opened during the start keeps what
was typed in it.
The spec and the release checklist said the reason shows under the
fields. Since c98bf20 the popover opens on the time buttons with its
fields empty. The checklist gains a step for a time left in Custom.
`await end.value` on a Task returning EndOutcome warned that the
result was unused.
A one-click start kept its length for a retry in the pills' input, but
the Compact panel's Return submits its own entry, and the panel keeps a
draft across closes. Option-click, 2 hours typed, the panel closed, then
a click for the 4 hour default: if that start was refused, the panel
opened on the 2 hours and Return started them. The click now writes its
length and the exact seconds into the panel's entry with the request.
The panel takes no keys until the start answers, and nothing writes the
entry after that, so an edit made after the refusal stays.

Pills and Popover had no such gap: the pills empty their input on every
open and take the click's length, and the popover opens on empty fields
(c98bf20), where Start and Return stay off until a time is typed. The
test covers all three styles.
1eb513e made a one-click start in Compact write its length into the
panel's entry, for Return to retry if the start was refused. A start
that went through cleared the entry only when the manager reported no
error. One that went through with an unrelated warning, such as an
agent app's App Nap setting that could not be written, kept the click's
4 hours and the exact seconds in the entry and turned editing on. The
next click opened the panel on those fields, and Return added another
4 hours.

A one-click start that the manager reports as started now clears the
entry, its kept request and editing, as a start with no error does. The
panel reads the warning from the manager, so it stays on screen. The
panel's own submit is unchanged: there an error during a session returns
to the duration that was typed, which a refused extend needs.

The 1eb513e test now checks what each style shows after the refusal and
changes the default before Return. A fix that only emptied Compact's
entry passed it before, since Return fell back to the same default.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-27T17:16:43.119838Z f246b54 PR opened
🔒 Security Review ✅ Completed 2026-10-27T17:23:59.126046Z f246b54 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Comment thread Sources/Insomnia/UI/StatusItemController.swift Outdated
Comment thread Sources/Insomnia/UI/StatusItemController.swift
The start and end tones checked `status.lidClosed`, which changes only
when the lid observer reports a close, 2 s after it. An end also stops
the observer before its tone. On an external display, a start or an end
from the menu bar within 2 s of closing the lid played its tone.

With the switch on, the controller now reads the lid from the registry
before a tone plays. `status.lidClosed` decides only when there is no
lid to read, as on a desktop. The read is injected, and the sound tests
pass their own, so no test reads the Mac's lid. With the switch off,
nothing reads it.

The new test has the registry say closed while `status.lidClosed` says
open, and checks that neither the start nor the end plays. The switch-off
test now checks that the lid is never read.
A refused one-click start in Pills reopens the pills with the click's
length, and `reopenAfterFailure` keeps the field focused as it was. Only
`expand` sets it from Settings. With Minutes as the default field, the
pills opened with Hours focused if Hours was focused when they last
closed, and a 5 typed for the retry made 1 hour into 15 hours.

Compact had the same gap since 1eb513e: the click's length replaces the
panel's draft, so the panel no longer resets the focus of an empty entry
on open, and kept the draft's field.

The click now sets the pills' focus to the default field with the
request, and copies it into the Compact entry with the length. The
popover's Custom row has its own focus, which starts in Hours by design,
so Popover is unchanged.

The test sets Minutes as the default and Hours as the last focus in
Pills and Compact, has the start refused, and types a 5.
With one-click start on, an Option-click on the idle eye in Popover
style started the default length and left the popover closed. A macOS
27.0.1 VM run saw this 3 of 3 times: with Option on the mouse events,
with the click wrapped in Option key events, and with Option held
through System Events. An event tap saw Option on the mouse-down and the
mouse-up. At the same build the check passed on macOS 26.6.2, and Pills
and Compact passed it on 27.

Pills and Compact read the keys held now (`NSEvent.modifierFlags`).
Popover read only the flags of the button's event (`NSApp.currentEvent`).
The session started, so the event that reached `popoverClicked` was a
left mouse-down without Option. Why that event lacks Option on macOS 27
is not established: no record shows the flags Insomnia received.

Popover now takes the event's flags together with the keys held, for
the Option check and the Control check. On macOS 26 the event already
carries them, so nothing changes there. A right click and a Control-click
still open the popover. The keys held are injected, and the one-click
tests pass none, so no test reads this Mac's keyboard.

The new test clicks with no flags on the event while the keys held are
Option, then Control: the popover opens and nothing starts. Without the
fix both start a session.
The eye's tap in Pills and Compact runs on the mouse-up and read only
NSEvent.modifierFlags, the keys held when the tap ran. A macOS 26.6.2 VM
run held Option on both mouse events of a 90 ms click and let go of it
0.5 ms after the mouse-up. Both styles found no Option, left their
controls closed and started the default 600 s session, 2 of 2. With
Option held 500 ms longer, both opened their controls.

The status view's local monitor sees each left mouse event before
SwiftUI. It now keeps the press's flags and adds the release's. The tap
takes that record once and adds the keys held now (the injected
heldModifiers), so Option at the press, the release or the tap keeps a
one-click start from taking the click. Each press on the view replaces
the record. A press elsewhere, a Control-click, a right click, a style
change and a move to another window drop it. A record left over can
only add keys, which opens the controls and starts nothing.

The test sends mouse events to the controller's own status view in
Pills and Compact, with the macOS 26 and 27 routing. Option on the press
alone, then on the release alone, with no key held, opens the controls
and starts nothing. An Option press dragged off the item and released
there is no tap, and the next plain click starts the default length.
The rig now clicks at the window's middle: the host only grows while
the pills open, so after a collapse its middle was off the item.
Hosted CI on macOS 27 failed testRightClickAndControlClickOnlyOpenTheMenu
at f4f89b5 (run 40555072359), in its os 26 and os 27 variants: one tap,
a running session, one backstop arm. In a 27.0.1 VM the tap fired on the
test's first step, a right mouse-down and mouse-up sent with
NSApp.sendEvent. So on macOS 27 the eye's onTapGesture fires for the
right button, which the host's comment said it doesn't. In the menu bar
the context menu's tracking takes the right mouse-up, and the 27 VM run
at f246b54 saw real right clicks start nothing in Pills and Compact, so
nothing reproduces it in the app. The app shouldn't depend on SwiftUI
ignoring the right button.

The status view's local monitor now takes a right click on the view as
it takes a Control-click. The press opens the menu with the event as it
came, so popUpContextMenu places the menu as before, and the press
leaves dispatch. The right drags and the release after it go nowhere. A
press taken out of dispatch never reaches rightMouseDown(with:), so the
menu opens once. In Popover style the monitor takes nothing, and the
right click goes on to the status bar button. On macOS 27 only left
events are moved to the pointer, as before.

The test now checks, through a window that records what it dispatches,
that no event of a right click or a Control-click reaches the window,
and so SwiftUI. This fails without the fix on macOS 26 too: the right
press, drag and release reach the window, though the tap doesn't fire
there. The right click in the test now has a drag between its press and
release; nothing else in it changed. A new test pins that in Popover
style the host opens no menu for a right click and lets it reach the
window, for the button under it.
c77778e gave a right click the Control-click's gesture flag, and while
that flag is set the monitor takes any drag or release, of either
button. It also stopped the release watcher on a right press. On macOS
27 a left press on the item holds its early mouse-up back and sends it
when the button really comes up, for the hold to end. With a right
click made while the left button was still down, that release was lost:
the watcher was stopped, and had it run, the flag would have taken the
release while the menu's tracking held the right one. Before c77778e
the monitor saw no right events, and the release went through. The hold
ring checks the button itself when it closes, so nothing ended, but
SwiftUI never saw the left button come up.

A menu gesture now takes only its own button's drags and release, and
only a Control-click stops the watcher.

The test, on the macOS 27 path, holds a left click's release back,
right-presses with no right release, and lets the left button up. The
left release must reach the window and the eye, the menu must open
once, and a right release that comes later must go nowhere. It fails at
c77778e, with the watcher stopped on a right press, and with the
gesture taking either button's release.
Review round 6 at b05534b. Minimal's eye tracks its press in a native
view, MinimalIconMouseView, whose rightMouseDown ends that press. Since
c77778e the status view's monitor takes the right press before the
view sees it, and showMenu does not end the press. A left press held on
the eye through a right click and the menu survived it: held past
0.6 s, it ended the session; released early, it started or extended
one.

The monitor now ends Minimal's press itself before it opens the menu,
for a right click and a Control-click. The release watcher keeps
running: on macOS 27 the held-back release still goes out when the
button comes up, and finds no press to act on.

The test sends the events through NSApp.sendEvent to Minimal's eye, on
the macOS 26 and 27 paths. A press held through a right click past the
hold's end, during a session, ends nothing. A press released right
after a right click, with no session, starts nothing. Without the fix
the hold ended the session and the release started one, on both paths.
Review round 6 at b05534b. With one-click start on: an Option press on
the idle eye, a right press with the left button still down, the menu
dismissed, Option let go, then the left release. The right press
dropped the left click's keys, and the release still reached the eye,
so its tap found no Option and started the default length.

The kept keys are now two records. A left press on the view keeps its
flags until its release is on its way to the window, which adds its own
and moves them to the record the tap reads once. A right press drops
only that second record, from a click already released; a left click
still underway keeps its keys for its release. The next press drops the
released record, so a tap that ran after another press would get none
of that press's keys. On macOS 27 the release counts when the watcher
sends it with the button up, not when it is held back: before, the
held-back release ended the click, and the right press then dropped
its keys.

Two tests. In Pills and Compact, on the macOS 26 and 27 paths: Option
on the left press, a right press, then the left release with no Option
on it or among the keys held, opens the controls and starts nothing. It
fails without the fix in all four, and on the 27 path alone when the
release counts before it is held back. SwiftUI runs the tap inside the
release's dispatch here, and a test cannot make it run later, so the
second test reads the host's keys as the tap does at the moments a
later tap would: after the release's dispatch it gets the click's keys
once, and after the next press none of that press's keys and none of
the click before it.
Review round 6 at b05534b. VoiceOver's press on the eye in Pills and
Compact went through the eye's tap, which reads the keys the status view
kept from a mouse click. An Option press dragged off the item and
released there kept Option, and a later VoiceOver press with no keys
held opened the controls instead of the one-click start. A hold, or a
click on a pill or the countdown, could leave the same keys.

The eye now has its own accessibility action, which reads only the keys
held now. A left release off the status view keeps no keys, since no
tap can follow it. A release on the view that makes no tap still keeps
them until the next press, but only the eye's tap reads them, and every
tap starts with a press that drops them.

The test, in Pills and Compact on the macOS 26 and 27 paths: an Option
press dragged off the item and released there keeps no keys. Then an
Option press released on the item beside the eye makes no tap. Then
VoiceOver's press starts the default length. A test cannot press the
eye as VoiceOver does (SwiftUI gives an in-process query no
accessibility elements), so it calls the closure the accessibility
action calls. With that closure sent through the tap's path, the press
opened the controls and started nothing; with keys kept for a release
off the item, the first check failed.
The comment on testInPopoverStyleARightClickGoesOnToTheButton said a
right click on the status bar button opens the menu. In Popover style
a right click and a Control-click reach the button, whose action
toggles the popover. The comment now says so.
Hosted CI on macOS 26.6.2 and 27.0.1 failed the macOS 27 cases of
testARightClickKeepsTheKeysOfALeftClickStillUnderway: no tap came, so
the controls never opened. The test waited 0.2 s after each event, and
on macOS 27 the left press lasted three of those waits before its
release. SwiftUI makes no tap from a press held about half a second: a
probe saw a tap after 0.3 s and none after 0.6 s. The events now go out
back to back, as the Minimal test already sends them.
On macOS 27 a status item's press and release reach the app with no
modifier flags, together as the button goes down. In a 27.0.1 VM, 5 of
6 Option-clicks in Pills and Compact whose Option came up 0.1 to 0.6 ms
after the mouse-up started the default length, and a diagnostic build
logged flags 0 on both events with Option held. The release waited for
the button to come up, so the tap found no keys in the click and none
held. macOS 26.6.2 passed 6 of 6: its events carry the flags.

The host now keeps the keys held when it handles a left press on it,
beside the press's own flags. The controller hands it the same reader
of held keys it uses itself, so tests set both.
On macOS 27 a status item's press carries no modifier flags, so the
keys held as the app handles it are all a click has (6dd3a82). A press
handled late, after a stall on the main thread, reads the keys of a
later moment, when Option and the button can be up, and the click
started the default length.

A press on the eye handled more than 50 ms after it happened now counts
as an Option-click in Pills and Compact, and so does Popover's button
event: the controls open and nothing starts. A 27.0.1 VM handled its
presses 0.3 ms after they happened. macOS 26 puts the keys in the
events, so it is unchanged. The hold tests' status view reads no keys
from the Mac's keyboard.
krishhgg added a commit that referenced this pull request Oct 27, 2026
@krishhgg

Copy link
Copy Markdown
Owner Author

VM runs at b6a36e0: one-click start on macOS 27.0.1 and 26.6.2

On macOS 27 an Option-click on the idle eye whose Option comes up right after the mouse-up now opens the controls and starts nothing, 12 of 12. At 08ee3e0 it passed 1 of 6, and 0 of 5 retries with the other way of posting. Every other check passed on both versions. The Accessibility press on the eye in Pills and Compact still does nothing, a gap the base has too. No trial logged the late-click line ("status item: a click handled late counts as an Option-click").

Setup. Two VMs on the maintainer's Mac mini, without a battery. All input was synthetic and posted inside the VM. Both ran one bundle, built from b6a36e0 on a MacBook with scripts/build-app.sh and installed with that commit's scripts/install.sh --app (CDHash a6e694ef). Each run started from default settings, turned on "Start a session with one click" and set the default length to 10 minutes.

  • macOS 27.0.1 (26A434) at 1440 × 900.
  • macOS 26.6.2 (25G83) at 1024 × 768.
Check macOS 27.0.1 macOS 26.6.2
Option-click in Pills and Compact, Option up within 1 ms of the mouse-up, 3 trials per style with each of two ways of posting 12/12 open the controls, nothing starts 12/12
The same with Option held 500 ms past the mouse-up, and with Option only in the mouse events' flags 4/4 4/4
Esc, then a plain click right after an Option-click 2/2 start 10 minutes and open nothing 2/2
Option-click in Popover, 3 ways, then a plain click 3/3 open the popover and start nothing; the plain click starts 10 minutes 3/3, then 1/1
2 right-clicks and a Control-click on the idle eye in Pills, Compact and Minimal 9/9 open the menu once, Esc closes it, nothing starts 9/9
A menu closed by a left click on the eye, Pills and Compact 2/2, nothing starts 2/2
Right-click and Control-click in Popover, a pair each 2/2 pairs open, then close, and start nothing 2/2 pairs
Minimal, a 0.8 s hold on the eye during a session 1/1 ends it 1/1
Minimal, a right-click during a left hold Skipped. macOS 27 never delivers that right press (Limits in the description) 2/2: the menu opens and the session keeps running
One-click start, then a 0.65 s hold on the end control, Pills and Compact 2/2 end it and restore sleep 2/2
Accessibility press (AXPress) on the idle eye, Pills and Compact 2/2 do nothing 2/2 do nothing

In the immediate trials Option came up 0.1 to 0.5 ms after the mouse-up on macOS 27 and 0.1 to 0.7 ms on 26. A synthetic click reaches the app well inside the 50 ms window, so the late-click rule didn't apply in any trial.

Pills, Compact and Popover after an Option-click on macOS 27, and the menu from a right-click on macOS 26:

Pills open beside the eye after an Option-click: Days, Hours and Minutes, with Hours selected

The Compact panel open under the eye after an Option-click: Days, Hours and Minutes

The popover after an Option-click: Not keeping your Mac awake, the preset chips and the idle rows

The menu from a right-click on the eye in Pills: the lid and Wi-Fi rows, Settings and Quit Insomnia

Folders: macOS 27.0.1 (report, run log) and macOS 26.6.2 (report, run log).

Not covered: a physical mouse or trackpad, VoiceOver, the sounds, a battery and a click the app handles late after a stall. The checks that need a real Mac are in docs/releasing.md.

@krishhgg

Copy link
Copy Markdown
Owner Author

Full suite at b6a36e0

On macOS 26.2 (Xcode 26.4.1) on the maintainer's MacBook: 1,704 tests, 1 failure, 0 skipped. The release build is clean. Hosted CI on macOS 26 and 27 passes at this head.

The failure is StillRunningCommandTests.testPassWaitingForTheJournalLeavesAPendingEndTheCleanup. The pass after a command ran one more pmset -g custom and lowpowermode 0 while an end was pending. Run alone, the class then passed 10 of 10 times (37 tests each). This stack doesn't change that test, and settleAfterCommand() is the same as on main. The same test failed the same way once today on another branch's full run, then passed 8 of 8 alone. The race is on main: the pass checks pendingEnd before it queues its exclusive step, so an end queued ahead of it can be pending by the time the Low Power check runs. Its own fix is left for the owner.

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.

2 participants