Harden Ghostty AppleScript integration#29
Merged
umputun merged 1 commit intoumputun:masterfrom Apr 5, 2026
Merged
Conversation
umputun
reviewed
Apr 5, 2026
Owner
umputun
left a comment
umputun
left a comment
There was a problem hiding this comment.
the AppleScript argv fix looks correct — proper use of on run argv + quoted heredoc to avoid injection.
two things from the PR description that aren't in the diff though:
- detection line is unchanged — still uses
osascript -e 'tell application "Ghostty" to get version'which you noted succeeds even withmacos-applescript = false. thecommand -v osascriptreplacement mentioned in the description isn't here. - EXIT trap for temp files (SENTINEL + LAUNCH_SCRIPT) isn't in the diff either.
are those planned for a follow-up, or were they accidentally left out?
- Pass shell variables via osascript argv instead of interpolating into AppleScript strings, preventing injection with special path characters - Replace unreliable get-version probe with command -v osascript check (get version succeeds even with macos-applescript=false) - Add EXIT trap for LAUNCH_SCRIPT and SENTINEL cleanup on interruption - Add explicit cleanup on success path for consistency with other backends Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tkukushkin
force-pushed
the
fix/ghostty-improvements
branch
from
360e553 to
2a46a23
Compare
Contributor
Author
|
Good catch — the detection fix and EXIT trap were accidentally left out during rebasing. Fixed in the force-pushed update: both |
umputun
approved these changes
Apr 5, 2026
Owner
umputun
left a comment
umputun
left a comment
There was a problem hiding this comment.
both fixes confirmed — detection guard and EXIT trap are in. lgtm, thx
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.
Summary
Follow-up to #28. Addresses Copilot review feedback and testing findings:
osascriptargv (on run argv) instead of interpolating into AppleScript string literals — prevents breakage with paths containing double quotes or backslashesget versionprobe withcommand -v osascript— the probe succeeds even withmacos-applescript = false, so it wasn't actually testing AppleScript availabilityLAUNCH_SCRIPTandSENTINELso they're cleaned up on signal interruption (Ctrl-C during sentinel polling)Test plan
launch-revdiff.sh HEAD~1from Ghostty — split, zoom, close on exitmacos-applescript = false— osascript fails with descriptive error🤖 Generated with Claude Code