safaridriver: only write has-run semaphore on actual success#1209
Open
rcurranmoz wants to merge 2 commits into
Open
safaridriver: only write has-run semaphore on actual success#1209rcurranmoz wants to merge 2 commits into
rcurranmoz wants to merge 2 commits into
Conversation
Currently the script: 1. touch's the semaphore file at startup (before any work) 2. has no `set -e`, so osascript failures fall through silently 3. unconditionally writes the version string to the semaphore at the end The combined effect is that puppet's `unless => /bin/test -f <semaphore>` guard always sees the file (or sees it with the right version) after the first invocation, even when the underlying osascript click-dance failed because cltbld's TCC.db didn't exist yet. Result: the worker permanently lacks the Safari "Allow remote automation" entitlement and there's no mechanism for puppet to retry. This commit: - Drops the pre-emptive touch - Refactors `enable_remote_automation` to return non-zero on failure - Only writes the semaphore on confirmed osascript success - Aggregates per-app return codes so the script as a whole exits non-zero if Safari and/or Safari Tech Preview configuration failed, so puppet reports a visible error With this change, puppet's existing `unless` guard does the right thing: the file is created only when the configuration actually succeeded, so a failed run will be retried on the next puppet apply. Fixes #1207 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.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
1 participant
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.
Summary
Fixes #1207.
The script
modules/macos_safaridriver/files/safari-enable-remote-automation.shcurrently writes thesafari-(tech-preview-)enable-remote-automation-has-runsemaphore unconditionally, even when the underlying osascript "click the checkbox" sequence failed (most commonly because cltbld's TCC.db doesn't exist yet on first bootstrap). Puppet'sunless => /bin/test -f <semaphore>then sees the file on the next apply and skips the resource — the worker is permanently broken with no retry path.This PR:
touchof the semaphore file at script startupenable_remote_automationto return non-zero on failureWith this change, puppet's existing
unlessguard does the right thing: the semaphore is created only when configuration actually succeeded, so a failed run is retried on the next puppet apply.Test plan
unless🤖 Generated with Claude Code