Repository navigation
Conversation
…s a typed name The Settings window was one long scrolling form with bundle IDs, SIGSTOP and NSAppSleepDisabled on screen, and two "Add running app" menus that meant opposite things; during testing, Comet landed on the freeze list that way. Settings is now one page that fits without scrolling. Edit Apps keeps one list with a rule per app, so each app sits on exactly one of agentList and freezeList. The hotspot name suggests networks this Mac has joined and still takes a typed name, matching either apostrophe (#87). Every config.json key stays. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Greptile's review of #104 found two ways to lose a setting. The window was as tall as its form, 896 pt, and not resizable. AppKit fits a window taller than the screen to it only when the window is resizable (constrainFrameRect(_:to:)), so on a 13-inch MacBook Air with the Dock at the bottom, Open at login was below the screen. The window is now resizable with zoom still dimmed, and the form has one width and no fixed height, so it scrolls on a short screen. The window refits to the form each time it opens. The fixed width also stops the one-time lid-close notice from widening the window to 738 pt. Choose… saved the app's bundle ID, but an app Launch Services hasn't seen, such as one just unzipped, has no URL for that ID, so its row was left out and could not be changed or removed. Choose… now registers the app (LSRegisterURL) before reading its ID. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Greptile's review of 44cb790 found no test for the Launch Services registration Choose… now does: the existing test reads Calculator's bundle ID, which passed before the fix too. testChoosingAnAppLaunchServicesHasNotSeenListsIt builds a stand-in app with a new bundle ID, checks it isn't found by that ID, runs bundleID(ofAppAt:) and checks isInstalled now finds it. Without the LSRegisterURL call the last check fails. Launch Services ignores apps in temporary folders and bundles with no executable, so the stand-in is a copy of /usr/bin/true next to the test bundle. The test unregisters it with lsregister -u and deletes it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The owner's review of #104, and Greptile's open thread on SettingsView, found that an app an older config.json names on both agentList and freezeList showed under both summary lines on the Settings page. Edit Apps showed it once, as Keep running, which is what the freeze planner does with it. Picking Keep running didn't clean it up either: setRule returned early when the effective rule was unchanged, so the app stayed on the freeze list. The summaries now come from the effective rules (Config.apps(with:), built on rule(for:)), the same rows Edit Apps shows. setRule takes the app off every list but the one for its rule even when that rule is unchanged, and an app already on the right list keeps its place there. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The owner's VM run of #104 found that with 30 tmux targets and the tmux disclosure open, the hotspot sheet was 870 pt tall and Done was below the bottom of the screen. The sheet's vertical fixedSize held the form at the height of all its rows, so it could not scroll. Without it, macOS sizes the sheet itself: a sheet's default sizing on macOS fits the content's height, up to a maximum it takes from the height of the window presenting it. In a scratch copy on a 16-inch MacBook Pro, a sheet like this one with 60 rows in a 929 pt window stopped at 857 pt with its bottom on screen, while with fixedSize it was 1084 pt and ran 32 pt off the bottom. Short sheets kept their height: 295 pt collapsed and 596 pt with 12 rows open, either way. testTheHotspotSheetCanBeShorterThanItsRows offers the sheet less height than its rows need and checks it takes no more; with the fixedSize back it fails. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The owner's review of #104 noted that testSessionLengthsReadAsWords expects English text while lengthLabel formats in the Mac's current locale, so on a Mac set to French it fails ("1 heure") although the formatting is right. lengthLabel takes a locale, Locale.autoupdatingCurrent by default, which is the locale the units style used before, so Settings reads the same. The test passes en_US_POSIX. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The release checklist's Settings items now cover an app config.json names on both lists and the hotspot sheet with 30 tmux targets, and docs/spec.md says how Settings treats an app on both lists. The validation record has today's checks of 544b7c9 on a MacBook Pro with macOS 27.0.1: the summaries, the Edit Apps cleanup, and the hotspot sheet's height in a scratch copy with the tmux section open. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Greptile's review of ac6e05e found that an older config.json can name an Apple app or Docker Desktop on freezeList alone. Edit Apps then showed "Freeze when lid closes" in a disabled picker, and the summary on the page listed the app under it, though FreezePlanner.isDenied keeps such an app running. rule(for:) now returns what the planner does: Keep running for any listed app it denies (the agent list, Apple apps, Insomnia, Docker Desktop), Freeze otherwise. Edit Apps and the summaries both read it. The row's Remove button already clears the stale entry. In a release build with TextEdit and Docker Desktop on freezeList alone, the page read "Keep running: Terminal, TextEdit, Docker" and "Freeze when lid closes: Discord", both rows read Keep running, and Remove took TextEdit off freezeList. testAppsThePlannerNeverFreezesShowAsKeepRunning fails with the previous rule(for:). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Conflicts: - SettingsView.swift: main added a Default field picker (#116), the version line and the update check (#117) to the old layout. They sit in General on the one page: Default field after Default session length, the version and the update check after Open at login, as main ordered them. releaseCheck is main's, unchanged. - InsomniaApp.swift: SettingsView without AnyView, as this branch has it, with main's ReleaseChecker passed in. - docs/spec.md: both edits to the presets line. Also from the merge: the Settings window test passes a ReleaseChecker, and main's "Settings > App" in README.md, docs and ReleaseCheck.swift reads "Settings > General", the section's name on the one page. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Lands #104 at 0baa658, by nsd97. Conflicts with #84 and #132, resolved here: #104 rebuilds the Settings page, and #84 (show the countdown in the menu bar) and #132 (countdown style) each added a control to the old one. Both controls move into #104's General section after Default field, with #132's onChange that applies a style change to a running session at once. The spec keeps #104's presets wording, #132's end time text with the maintainer's time change fix, and #84's toggle as its own bullet. The release checklist keeps both sides' bullets. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The app summary test expected the literal "Finder", but the summary uses the system's localized app name, so the test failed on a Mac set to another language. It now compares with RunningApps.displayName. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Brings in 54f3c7e, which keeps the pills open when a pill is clicked on macOS 27. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings in 732de03: one refocus watch per press, and the pills return focus to the app the user was in. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings in 85576e5, a test seam for the focus-loss test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…in the battery link Greptile's three findings on #104's code in this PR: - Choose... dropped Apple apps, so Terminal could not come back to Keep running, or to its App Nap cover, after a Remove. Config.addApp adds any app as Keep running; the running-apps menu still leaves Apple apps out. - Done stayed enabled while a password save waited on the keychain. A save that failed after the sheet closed showed its notice nowhere. Done and interactive dismissal now wait for the save. - The battery pickers lost main's caption saying the two levels move each other, so Never for Low Power Mode turned the battery end off without a word. The caption is back under End the session below. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
Author
VM run on macOS 27.0.1 at b892819: the three Settings fixes workAll three fixes in Setup. macOS 27.0.1 (26A434) with Xcode 27.0, in a Tart VM on the maintainer's Mac mini. Each commit was installed from a clean checkout with
The rest of the page at
|
| Step | What happened |
|---|---|
| The page, top to bottom | Five sections: When the lid closes, Apps, Battery, If Wi-Fi drops, General. General has Default session length, Default field, Countdown, "Show the countdown in the menu bar", Open at login, Version and the update buttons |
| Edit Apps: Slack from Freeze when lid closes to Keep running, then back | config.json moves Slack from freezeList to agentList, then back. Terminal's Freeze choice stays dimmed |
| A 4 hour session, Countdown set to End time in Settings | Within 2 seconds the menu bar changes from 3:59:53 to Sun 03:06, the session's end on the VM's clock |
| "Show the countdown in the menu bar" off | The status item shrinks from 134 to 34 points: only the eye is left |
| The switch on again, then Countdown back to Time left | 134 points with the switch on, then 3:59:12 at 120 points |
The hotspot sheet's list of joined networks was not checked: the VM has no Wi-Fi.
Evidence: t147-head.log (battery and Edit Apps), t147-head3b.log (hotspot), t147-control.log, t147s-head.log (the rest of the page), all screenshots and the README on which runs count, scripts.
Limits
- macOS 26 is not covered here. The lab has no macOS 26 VM now. Hosted CI runs the unit tests on macOS 26 and 27.
- The hotspot save was held by a locked login keychain. Other keychain dialogs were not tried.
- The first head run's hotspot part does not count: the script's own Terminal handling stalled it.
head3breran that part on the same install. The folder's README explains which runs count.
🤖 Generated with Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Settings becomes one page grouped by what happens (When the lid closes, Apps, Battery, If Wi-Fi drops, General), with one app list and a hotspot picker that suggests the networks this Mac has joined. The two controls #145 adds, Countdown (time left or end time) and "Show the countdown in the menu bar", sit in its General section.
This lands #104 by @nsd97 at 0baa658. The commit is merged here unchanged, so GitHub marks #104 merged when this one merges and its author keeps the credit. It sits on #145 (menu controls), because #145 adds two Settings controls to the old page that #104 replaces. Review and merge #145 first; this pull request's base then moves to main.
To @nsd97: please don't push to your branch now. A new commit on it isn't in this pull request, so #104 would stay open instead of showing as merged.
Conflicts resolved
onChangethat applies a style change to a running session at once.Maintainer fixes
SettingsTests.testAppSummaryNamesUpToThreeInstalledAppsexpected the literal "Finder", but the summary uses the system's localized app name, so it failed on a Mac set to another language. It now compares withRunningApps.displayName(for:).Config.addApp). The app list is also the App Nap list, so Terminal, on it by default, could not come back after a Remove.Tests
Focused on macOS 26.2 (Xcode 26.4.1): SettingsTests, ConfigTests, FailoverMachineTests, LidCloseCopyTests, SettingsPasswordNoticeTests, MenuBarCountdownOptionTests, TimeChangeTests, DisplaySleepTests and StatusMenuWarningTests pass. Full suite at b892819 on the same Mac: 1,267 tests, 0 failures. Two new ConfigTests cover the fixes:
testAddingAnAppKeepsItRunningAndAnAppleAppCanComeBackandtestNeverForLowPowerModeTurnsTheEndOff. Putting the old Apple app guard back makes the first one fail. Hosted CI on macOS 26 and 27 passes at b892819. UIStatusTests, UIStartupTests and KeychainStoreTests are skipped on the maintainer's Mac; hosted CI runs them.Codex review
At 1b60b15 and at b892819 (gpt-6.1-sol, xhigh, read-only): no findings. Greptile's three findings at e98c38d are fixed in b892819, and their threads are resolved.
Evidence
🤖 Generated with Claude Code
The PR appears safe to merge, with all three previous findings addressed.Summary
Settings becomes one page, with separate sheets for apps and hotspot setup.
Reviews (5) · Last reviewed commit: "Settings: re-add Apple apps, hold the ho..." · Reviewed by Greptile