Skip to content

Align skill guidance with user-run demos and Jamf/root-run workflows#13

Closed
dan-snelson wants to merge 3 commits into
swiftDialog:mainfrom
dan-snelson:codex-homebrew-root-run-example
Closed

Align skill guidance with user-run demos and Jamf/root-run workflows#13
dan-snelson wants to merge 3 commits into
swiftDialog:mainfrom
dan-snelson:codex-homebrew-root-run-example

Conversation

@dan-snelson
Copy link
Copy Markdown
Contributor

Summary

  • remove unresolved merge-marker fallout from the skill pack docs/templates
  • preserve the original demo default of user-run command-file flows
  • explicitly document and template the Jamf/root-run /var/tmp ownership-handoff pattern for MDM execution

Validation

  • zsh -n skills/codex-swiftdialog-builder/assets/templates/progress-dialog.zsh
  • zsh -n skills/claude-swiftdialog-builder/templates/progress-dialog.zsh

Signed-off-by: Dan K. Snelson <24623109+dan-snelson@users.noreply.github.com>
Codex-syncronized examples with current 'skills/'

Signed-off-by: Dan K. Snelson <24623109+dan-snelson@users.noreply.github.com>
Codex-syncronized examples and updated 'skills/'

Signed-off-by: Dan K. Snelson <24623109+dan-snelson@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 10, 2026 17:11
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR cleans up skill-pack documentation/templates after a merge conflict and aligns the guidance with the repo’s intended execution models: user-run demos using mktemp -t command files, and Jamf/root-run workflows using /var/tmp + ownership handoff so the logged-in GUI user can receive swiftDialog updates.

Changes:

  • Remove leftover merge-conflict markers across skill READMEs, skill instructions, and reference docs.
  • Clarify/standardize command-file guidance: mktemp -t for user-run flows; /var/tmp + chown/chmod for Jamf/root-run handoff flows.
  • Add an internal “Homebrew uninstall confirmation” playground prompt + generated Jamf/root-run swiftDialog script example.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
skills/README.md Removes merge-marker fallout; keeps the intended Jamf/root-run handoff guidance.
skills/codex-swiftdialog-builder/SKILL.md Restores intended command-file/Jamf handoff guidance in Codex skill instructions.
skills/codex-swiftdialog-builder/references/authoring-patterns.md Resolves merge markers; documents user-run vs root-run command-file patterns explicitly.
skills/codex-swiftdialog-builder/references/advanced-patterns.md Resolves merge markers; keeps expanded workflow steps including Jamf/root-run handoff.
skills/codex-swiftdialog-builder/README.md Resolves merge markers; reiterates the two command-file workflows.
skills/codex-swiftdialog-builder/assets/templates/progress-dialog.zsh Removes merge markers; preserves root-run /var/tmp handoff starter template.
skills/claude-swiftdialog-builder/templates/progress-dialog.zsh Same as above for Claude template.
skills/claude-swiftdialog-builder/references/authoring-patterns.md Resolves merge markers; documents the two command-file workflows.
skills/claude-swiftdialog-builder/references/advanced-patterns.md Resolves merge markers; keeps Jamf/root-run handoff guidance.
skills/claude-swiftdialog-builder/README.md Resolves merge markers; reiterates the two command-file workflows.
skills/claude-swiftdialog-builder/CLAUDE.md Resolves merge markers; keeps Jamf/root-run handoff guidance.
playground/README.md Resolves merge-marker fallout in the command-file workflow notes.
playground/homebrew-delete-confirmation.md Adds an internal prompt/spec for generating a Jamf/root-run uninstall confirmation workflow.
playground/homebrew-delete-confirmation.zsh Adds an internal Jamf/root-run swiftDialog uninstall script with hash verification + progress UI.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +5 to +9
UNINSTALL_URL="https://raw.githubusercontent.com/Homebrew/install/HEAD/uninstall.sh"
EXPECTED_SHA256="58e8ea576b9f9682c4740c0e4c286dfea8d15271f33d8504006132a634f25b01"
HOMEBREW_DIRECTORY="/opt/homebrew"
BREW=""
LOGGED_IN_USER=""
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HOMEBREW_DIRECTORY is hard-coded to /opt/homebrew, but brew may be installed under /usr/local on Intel (or another prefix). This can leave the actual Homebrew install behind (or delete an unexpected /opt/homebrew directory if it exists). Consider setting this from "$BREW" --prefix (captured before uninstall) or deriving it from the detected BREW path, and reflect that same value in the warning/progress messages.

Copilot uses AI. Check for mistakes.
Comment on lines +132 to +135
if ! /bin/chmod 644 "$target_file" 2>/dev/null; then
return 1
fi

Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prepare_log_file sets the uninstall log to mode 644 in /var/tmp, which makes it world-readable. Since the log can contain sensitive system details, it would be safer to keep it 600 and hand ownership to the logged-in user (similar to the command-file handoff), or otherwise restrict permissions while still allowing the GUI user to open it.

Suggested change
if ! /bin/chmod 644 "$target_file" 2>/dev/null; then
return 1
fi
if ! /bin/chmod 600 "$target_file" 2>/dev/null; then
return 1
fi
if current_logged_in_user; then
if ! /usr/sbin/chown "$LOGGED_IN_USER" "$target_file" 2>/dev/null; then
return 1
fi
fi

Copilot uses AI. Check for mistakes.
Comment on lines +142 to +150
if [[ -n "$LOG_FILE" && -e "$LOG_FILE" ]]; then
/bin/rm -f "/var/tmp/dialog.log"

if current_logged_in_user; then
logged_in_user_id=$(/usr/bin/id -u "$LOGGED_IN_USER" 2>/dev/null)

if [[ "$logged_in_user_id" =~ ^[0-9]+$ ]]; then
/bin/launchctl asuser "$logged_in_user_id" /usr/bin/sudo -u "$LOGGED_IN_USER" /usr/bin/open -R "$LOG_FILE" >/dev/null 2>&1 || \
/usr/bin/sudo -u "$LOGGED_IN_USER" /usr/bin/open -R "$LOG_FILE" >/dev/null 2>&1 || \
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reveal_log_file unconditionally deletes /var/tmp/dialog.log, which is not created by this script and could remove an unrelated file or another run’s log. This line should be removed, or replaced with cleanup of only files this script created (e.g., LOG_FILE/CMD_FILE/TEMP_SCRIPT).

Copilot uses AI. Check for mistakes.
send_command "button1text: Close"
send_command "button1: enable"

wait $DIALOG_PID 2>/dev/null || true
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait $DIALOG_PID will behave like wait with no arguments if DIALOG_PID is ever empty (e.g., if the dialog fails to launch), which can block waiting on unrelated background jobs. Guard this with a non-empty/numeric PID check (or track whether the progress dialog was successfully started) before calling wait.

Suggested change
wait $DIALOG_PID 2>/dev/null || true
if [[ -n "$DIALOG_PID" && "$DIALOG_PID" == <-> ]]; then
wait "$DIALOG_PID" 2>/dev/null || true
fi

Copilot uses AI. Check for mistakes.
@dan-snelson dan-snelson deleted the codex-homebrew-root-run-example branch April 10, 2026 17:22
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