feat: add global and per-profile pre/post apply script hooks#577
Merged
Conversation
Profile switches in Thaw can now invoke user-supplied scripts before and after the apply, letting a sister app (the issue cites iStat Menus as the motivating use case) be kept in sync with Thaw's profile transitions on display auto-switch, hotkey, manual apply, menu bar context menu, and macOS Focus Filter activation. All five entry points funnel through `ProfileManager.applyProfile`, so wiring the hook stages into that single function covers every trigger. Two scopes of hook are supported. Global pre/post hooks live in `UserDefaults` (new `globalPreProfileHook` / `globalPostProfileHook` keys) and fire on every apply. Per-profile pre/post hooks live inside the profile's JSON via a new optional `automation: ProfileAutomation?` field on `Profile` and `ProfileContent`. The field uses `decodeIfPresent`, so existing profiles on disk decode unchanged. Profile export/import already serialises the whole `Profile`, so per-profile hooks travel with `exportProfile` / `importProfile` automatically. Hooks execute in nested order around the apply: `Global pre -> Profile pre -> [Thaw applies the snapshot, spacing offset, and layout] -> Profile post -> Global post`. Each scope's pre runs before its counterpart's post, mirroring a typical setup/teardown pairing. Hooks are blocking: the layout task awaits each one before proceeding, with a per-hook timeout (configurable 1-300 seconds, default 5). Failures and timeouts log to `DiagLog` but do not abort the apply, so a broken script can never lock the user out of switching profiles. Cancelled applies (e.g. a second profile switch fired before the first finishes) propagate `Task.isCancelled` through the hook stages. The new `HookRunner` (Thaw/Utilities/HookRunner.swift) wraps `Process()` and races process termination against a `Task.sleep` timeout via `withThrowingTaskGroup`. AppleScript files (`.scpt`, `.applescript`, `.scptd`) route through `/usr/bin/osascript <path>`; other files are launched directly and must carry the executable bit (`chmod +x`), enforced at run time so users can swap the file behind the scenes without re-picking it. Stdout and stderr are piped to `DiagLog` under category `HookRunner`. Scripts receive the following environment variables: `THAW_HOOK_PHASE` (`Pre` or `Post`), `THAW_HOOK_SCOPE` (`Global` or `Profile`), `THAW_PROFILE_ID`, `THAW_PROFILE_NAME`, `THAW_PREVIOUS_PROFILE_ID` (empty when starting from no active profile), `THAW_PREVIOUS_PROFILE_NAME`. `applyProfile` gained a `previousProfileID` parameter that callers populate by reading `activeProfileID` before they overwrite it; all seven call sites (manual Apply button, profile hotkey, menu bar context menu, control item context menu, Focus Filter activation, display auto-switch, and `reapplyActiveProfile`) were updated. The apply pipeline was restructured: `applyProfile`'s previously-synchronous snapshot push (general / advanced / hotkeys / display config / appearance / item names / new-items placement) now runs inside the `layoutTask` after the pre-hook stages, so a pre-hook can complete before Thaw mutates any state. Callers that previously awaited `layoutTask?.value` continue to do so and observe the same final state. With no hooks configured, the new code paths short-circuit and the apply timing matches today. The Automation settings pane (which previously housed only the Settings URI Scheme whitelist) gains a Hooks section with two subsections: Global Hooks (pre and post rows) and Per-Profile Hooks (a profile picker plus pre and post rows). Each hook row uses `NSOpenPanel` for script selection, a stepper for timeout, an enabled toggle, and surfaces validation warnings for missing or non-executable files. The section closes with the list of injected environment variables and an example showing how a bash pre-hook could push the new profile name into iStat Menus via `defaults write`. Closes #567 Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds global and per-profile pre/post hook configuration and UI, a HookRunner to execute scripts with timeout and env vars, stores hooks in profiles/user defaults, and threads previousProfileID through apply flows so hooks receive prior-profile context. ChangesProfile Automation Hooks
Sequence DiagramsequenceDiagram
participant User as User (Profile Switch)
participant Manager as ProfileManager
participant HookRunner as HookRunner
participant Hook as Shell/AppleScript
User->>Manager: applyProfile(newProfile, previousProfileID)
Manager->>Manager: Load pre/post hooks
Manager->>HookRunner: runIfEnabled(pre-hook, context)
HookRunner->>Hook: Execute with env vars
Hook-->>HookRunner: Exit status + stdout/stderr
Manager->>Manager: Apply profile snapshot
Manager->>Manager: Execute layout
Manager->>HookRunner: runIfEnabled(profile post-hook)
HookRunner->>Hook: Execute with env vars
Hook-->>HookRunner: Outcome
Manager->>HookRunner: runIfEnabled(global post-hook)
HookRunner->>Hook: Execute with env vars
Hook-->>HookRunner: Outcome
Manager-->>User: Profile applied with hooks completed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Thaw/Settings/SettingsPanes/AutomationSettingsPane.swift (1)
571-574:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPreview will crash —
AppStateenvironment object is missing.
AutomationSettingsPanedeclares@EnvironmentObject var appState: AppState(line 14), and the body unconditionally readsappState.profileManager.profiles/activeProfileIDin bothonAppearandprofileHooksGroup. The#Previewinstantiates the pane with no.environmentObject(...), so SwiftUI previews and any developer running this preview will hit a runtime trap before rendering.🔧 Inject AppState into the preview
`#Preview` { AutomationSettingsPane() .frame(width: 600, height: 500) + .environmentObject(AppState()) }If
AppState()has side effects that make it unsuitable for previews, wrap it in a#if DEBUGpreview helper or expose a.previewfactory.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Thaw/Settings/SettingsPanes/AutomationSettingsPane.swift` around lines 571 - 574, The SwiftUI preview for AutomationSettingsPane will crash because AutomationSettingsPane declares `@EnvironmentObject` var appState: AppState and its body reads appState.profileManager.profiles and activeProfileID, so update the `#Preview` that instantiates AutomationSettingsPane to inject a suitable AppState via .environmentObject(...), e.g. provide a default or debug AppState instance (or a preview factory like AppState.preview) so the preview has a valid environment object; ensure you reference AutomationSettingsPane and AppState when adding the .environmentObject to the preview.
🧹 Nitpick comments (3)
Thaw/Settings/SettingsPanes/AutomationSettingsPane.swift (3)
463-486: ⚡ Quick winTimeout controls lack accessible names.
Both the timeout
TextField(line 472) andStepper(line 477) useEmptyView()as their label and rely on the adjacent "Timeout" / "s"Textfor sighted users. VoiceOver users get no announced name for either control. TheToggleon line 467 is fine because "Enabled" is its label.♿ Suggested accessibility labels
HStack(spacing: 4) { Text("Timeout") TextField(value: timeoutBinding, format: .number) { EmptyView() } .textFieldStyle(.roundedBorder) .frame(width: 48) + .accessibilityLabel("Hook timeout in seconds") Stepper(value: timeoutBinding, in: 1...300) { EmptyView() } .labelsHidden() + .accessibilityLabel("Hook timeout in seconds") Text("s").foregroundStyle(.secondary) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Thaw/Settings/SettingsPanes/AutomationSettingsPane.swift` around lines 463 - 486, The timeout TextField and Stepper inside the hook != nil block lack accessible labels; update the TextField and Stepper that use timeoutBinding to supply an explicit accessibility label (or a non-empty label view) such as .accessibilityLabel("Timeout in seconds") or replace EmptyView() with a Label("Timeout", systemImage: "")/Text("Timeout") so VoiceOver announces them; ensure the Stepper and TextField labels match and keep the visible "Timeout" text for sighted users while using .accessibilityValue or .accessibilityLabel on the controls to expose the units ("seconds") if needed.
520-529: 💤 Low valueTextField accepts out-of-range values until commit.
Stepperis bounded by1...300, but theTextFieldis bound only throughtimeoutBinding, which clamps inside the setter. Until the user commits, the field can display9999or0, and the value only snaps back after focus loss. Consider using a bounded format style so the field rejects out-of-range entries inline, or at least display the value through a derived binding that mirrors the clamped result.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Thaw/Settings/SettingsPanes/AutomationSettingsPane.swift` around lines 520 - 529, The TextField shows out-of-range numbers until commit because timeoutBinding only clamps in its setter; change timeoutBinding so its getter also returns the clamped value (mirror the clamped timeoutSeconds) so the displayed value always reflects the bounded 1...300 range, and keep the setter clamping as-is; locate the timeoutBinding computed property and update its get to use hook?.timeoutSeconds clamped with max(1, min(...,300)) (or alternatively replace the TextField with a formatted/bounded NumberFormatter/FormatStyle tied to timeoutBinding) so the UI rejects or immediately corrects out-of-range entries.
550-555: 💤 Low valueRemove
UTType.itemor clarify the panel's intended filtering behavior.
UTType.itemis the root of the UTI hierarchy. When included inallowedContentTypes, it renders the more specific types (shellScript,appleScript,executable) ineffective at filtering—the panel will allow the same broad set regardless. Either remove.itemif the intent is to prefer specific script types, or remove the explicit types if the intent is to accept any file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Thaw/Settings/SettingsPanes/AutomationSettingsPane.swift` around lines 550 - 555, The allowedContentTypes array on the panel is including UTType.item, which overrides the specific filters (UTType.shellScript, UTType.appleScript, UTType.executable) and makes them ineffective; in AutomationSettingsPane.swift update the panel.allowedContentTypes assignment used when configuring the open panel (the line assigning to panel.allowedContentTypes) to either remove UTType.item so the panel actually filters to the specific script/executable types, or—if the intent is to accept any file—remove the explicit UTType.shellScript/UTType.appleScript/UTType.executable entries and keep only UTType.item to express a broad accept-all behavior, then adjust any related comments to reflect the chosen behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Thaw/Settings/Models/ProfileManager.swift`:
- Around line 382-397: After awaiting HookRunner.runIfEnabled(profilePost, ...)
add an explicit cancellation check and bail out before calling the global
post-hook: check Task.isCancelled (or call try Task.checkCancellation()) and
return/throw CancellationError if cancelled so that
HookRunner.runIfEnabled(globalPost, ...) is not invoked when the task was
cancelled; update the code immediately after the profile-post await (around
HookRunner.runIfEnabled(profilePost, ...) and before the
HookRunner.runIfEnabled(globalPost, ...)) to perform this guard using the
existing baseContext/HookRunner symbols.
In `@Thaw/Settings/SettingsPanes/AutomationSettingsPane.swift`:
- Around line 36-41: The Picker's selectedHookProfileID is only set in onAppear
and can become stale when profiles change; add an onChange(of:
appState.profileManager.profiles) handler that validates the current
selectedHookProfileID and, if it's not present in the updated profiles list, set
selectedHookProfileID = appState.profileManager.activeProfileID ??
appState.profileManager.profiles.first?.id so the Picker and HookRow references
always point to an existing profile; implement this adjacent to the existing
onAppear block and reference selectedHookProfileID,
appState.profileManager.profiles and activeProfileID in the handler.
In `@Thaw/Utilities/HookRunner.swift`:
- Around line 145-151: The wait for process exit is registered after calling
process.run(), which can miss very short-lived hooks; before calling
process.run() (in the same context where you use withThrowingTaskGroup and
compute exitStatus), spawn a background task that calls process.waitUntilExit()
(or set process.terminationHandler) so the wait is registered prior to launch —
update the code around process.run(), process.terminationHandler and the
withThrowingTaskGroup handling (the block returning process.terminationStatus)
to start the wait via process.waitUntilExit() before invoking process.run() to
avoid timeout false-positives.
- Around line 145-175: Wrap the existing withThrowingTaskGroup block in a
withTaskCancellationHandler so that when the parent task is cancelled you
explicitly terminate the spawned Process; in the onCancel closure call
process.terminate() and, after a short delay if needed, process.interrupt()
(mirroring your timeout cleanup), and ensure any child tasks are cancelled
(e.g., group.cancelAll()) — apply this around the withThrowingTaskGroup usage
that currently sets process.terminationHandler and returns
process.terminationStatus so that external Task cancellation also cleans up the
subprocess.
---
Outside diff comments:
In `@Thaw/Settings/SettingsPanes/AutomationSettingsPane.swift`:
- Around line 571-574: The SwiftUI preview for AutomationSettingsPane will crash
because AutomationSettingsPane declares `@EnvironmentObject` var appState:
AppState and its body reads appState.profileManager.profiles and
activeProfileID, so update the `#Preview` that instantiates AutomationSettingsPane
to inject a suitable AppState via .environmentObject(...), e.g. provide a
default or debug AppState instance (or a preview factory like AppState.preview)
so the preview has a valid environment object; ensure you reference
AutomationSettingsPane and AppState when adding the .environmentObject to the
preview.
---
Nitpick comments:
In `@Thaw/Settings/SettingsPanes/AutomationSettingsPane.swift`:
- Around line 463-486: The timeout TextField and Stepper inside the hook != nil
block lack accessible labels; update the TextField and Stepper that use
timeoutBinding to supply an explicit accessibility label (or a non-empty label
view) such as .accessibilityLabel("Timeout in seconds") or replace EmptyView()
with a Label("Timeout", systemImage: "")/Text("Timeout") so VoiceOver announces
them; ensure the Stepper and TextField labels match and keep the visible
"Timeout" text for sighted users while using .accessibilityValue or
.accessibilityLabel on the controls to expose the units ("seconds") if needed.
- Around line 520-529: The TextField shows out-of-range numbers until commit
because timeoutBinding only clamps in its setter; change timeoutBinding so its
getter also returns the clamped value (mirror the clamped timeoutSeconds) so the
displayed value always reflects the bounded 1...300 range, and keep the setter
clamping as-is; locate the timeoutBinding computed property and update its get
to use hook?.timeoutSeconds clamped with max(1, min(...,300)) (or alternatively
replace the TextField with a formatted/bounded NumberFormatter/FormatStyle tied
to timeoutBinding) so the UI rejects or immediately corrects out-of-range
entries.
- Around line 550-555: The allowedContentTypes array on the panel is including
UTType.item, which overrides the specific filters (UTType.shellScript,
UTType.appleScript, UTType.executable) and makes them ineffective; in
AutomationSettingsPane.swift update the panel.allowedContentTypes assignment
used when configuring the open panel (the line assigning to
panel.allowedContentTypes) to either remove UTType.item so the panel actually
filters to the specific script/executable types, or—if the intent is to accept
any file—remove the explicit
UTType.shellScript/UTType.appleScript/UTType.executable entries and keep only
UTType.item to express a broad accept-all behavior, then adjust any related
comments to reflect the chosen behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1dae0967-81f3-4d5e-92df-896d41d33612
📒 Files selected for processing (12)
Thaw/Hotkeys/Hotkey.swiftThaw/MenuBar/ControlItem/ControlItem.swiftThaw/MenuBar/MenuBarManager.swiftThaw/Resources/Localizable.xcstringsThaw/Settings/Models/AutomationHookSettings.swiftThaw/Settings/Models/HookScript.swiftThaw/Settings/Models/Profile.swiftThaw/Settings/Models/ProfileManager.swiftThaw/Settings/SettingsPanes/AutomationSettingsPane.swiftThaw/Settings/SettingsPanes/ProfileSettingsPane.swiftThaw/Utilities/Defaults.swiftThaw/Utilities/HookRunner.swift
Collaborator
Author
|
Here's a short video illustrating the change. This shows how you can have both global and profile hooks set. Cap.2026-05-13.at.15.26.18.mp4 |
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
|
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
2 participants
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.
What does this PR do?
Adds pre/post profile-apply hooks that run user-supplied shell scripts (any executable with a shebang) or AppleScript files (
.scpt,.applescript,.scptd) around every Thaw profile switch, in two scopes: Global (every apply) and Per-Profile (only when its owning profile is applied). All apply paths funnel throughProfileManager.applyProfile, so the hook stages cover the manual Apply button, the profile hotkey, the menu bar and control item context menus, the display auto-switch, and macOS Focus Filter activation.PR Type
Does this PR introduce a breaking change?
If yes, please describe the impact and migration path:
What is the current behavior?
There is no way to run a script when Thaw switches profiles. Users with sister menu bar apps that don't expose a profile-switch API (the issue cites iStat Menus, which responds to
defaults writeand AppleScript but has no native sync mechanism) have no programmatic hook to keep those apps' configurations in step with Thaw's display-driven or focus-filter-driven transitions.Issue Number: #567
What is the new behavior?
A new Hooks section appears in the Automation settings pane, with two subsections. Global Hooks holds pre and post rows that fire on every apply. Per-Profile Hooks holds a profile picker plus pre and post rows for the selected profile; switching the picker swaps the rows to that profile's hooks. Each hook row uses
NSOpenPanelto pick a script file, a stepper to set a wall-clock timeout (1-300 seconds, default 5), and an enable toggle. Hooks can be paused without losing the picked path by toggling enable off. Validation warnings surface in the row when the chosen file is missing or, for non-AppleScript files, lacks the executable bit.Hooks run in symmetric nested order around the apply:
Global pre -> Profile pre -> [Thaw applies the snapshot, spacing offset, and layout] -> Profile post -> Global post. With only one scope configured, the missing stages collapse out (Global pre -> [apply] -> Global post, orProfile pre -> [apply] -> Profile post). Profile hooks belong to the incoming profile, not the outgoing one; teardown for the outgoing profile is best expressed as a Global post that branches onTHAW_PREVIOUS_PROFILE_NAME. Hooks are blocking: the layout task awaits each stage before proceeding, capped at the configured timeout. Failures and timeouts log toDiagLogunder categoryHookRunnerbut never abort the apply, so a broken script can't lock the user out of switching profiles. Cancelled applies (rapid back-to-back profile switches) propagateTask.isCancelledthrough the hook stages.Every hook process inherits Thaw's environment merged with
THAW_HOOK_PHASE(PreorPost),THAW_HOOK_SCOPE(GlobalorProfile),THAW_PROFILE_ID(UUID of the incoming profile),THAW_PROFILE_NAME(user-visible name),THAW_PREVIOUS_PROFILE_ID(UUID of the previously active profile, empty if none), andTHAW_PREVIOUS_PROFILE_NAME(name or empty). For example, a one-line bash pre-hook that mirrors the switch into iStat Menus:defaults write com.bjango.istatmenus5 ActiveProfile -string "$THAW_PROFILE_NAME". An AppleScript post-hook reads the same values viasystem attribute "THAW_PROFILE_NAME"(osascript does not expose Unix env vars through the standard AppleScript environment accessor). A shell hook can branch on phase withcase "$THAW_HOOK_PHASE" in Pre) ...; Post) ...; esac.Implementation:
HookScriptandProfileAutomationare new inThaw/Settings/Models/HookScript.swift. Global hooks persist via two newDefaultskeys (globalPreProfileHook/globalPostProfileHook) as JSON-encodedData. Per-profile hooks live inside the profile's JSON via a new optionalautomation: ProfileAutomation?field onProfileandProfileContent, decoded withdecodeIfPresentso existing profile files on disk are unaffected; profile export and import already serialise the wholeProfile, so per-profile hooks round-trip throughexportProfileandimportProfileautomatically.HookRunnerinThaw/Utilities/HookRunner.swiftwrapsProcess()withPipes and races termination against aTask.sleeptimeout viawithThrowingTaskGroup; AppleScript files route through/usr/bin/osascript <path>, other paths launch directly and are checked for the executable bit at run time.ProfileManager.applyProfile's previously-synchronous snapshot push (general / advanced / hotkeys / display config / appearance / item names / new-items placement) now runs inside thelayoutTaskafter the pre-hook stages so a pre-hook can complete before Thaw mutates state, andapplyProfilegained apreviousProfileIDparameter that all seven call sites pass by readingactiveProfileIDbefore they overwrite it.PR Checklist
Other information
Manually verified the following scenarios on a multi-display setup, against the local debug build with diagnostic logging enabled:
await profileManager.layoutTask?.valuereturns.globalPre, profilePre, profilePost, globalPostin that order.sleep 60hook with timeout5logsHookError.timedOut; Apply button re-enables..shwithout+xlogsHookError.notExecutable; apply still completes..scpthook routes throughosascript(visible in Activity Monitor); no+xrequired on the script.env | grep THAW_confirms all six env vars are set, withTHAW_PROFILE_NAMEandTHAW_PREVIOUS_PROFILE_NAMEreflecting the transition.hook == nil || !hook.isEnabled; latency matches the pre-PR baseline.Notes for reviewers
HookRunner.runIfEnabled, so adding anabortOnFailureflag later is a small change.previousProfileIDis threaded throughapplyProfilerather than read fromactiveProfileIDinside the function, because every call site overwritesactiveProfileIDbefore invokingapplyProfile. Reading inside the function would always observe the new value.Process()inherits Thaw's full environment by design and merges theTHAW_*vars on top, so scripts see the user'sPATH, locale, and so on without extra configuration.Closes #567
Summary by CodeRabbit
New Features
Bug Fixes / Behavior
Validation