Skip to content

feat(launcher): macOS bundle detection, iTerm2 validation, PR #421 review fixes - #422

Merged
clcollins merged 4 commits into
mainfrom
srepd/macos-terminal-coverage
Aug 10, 2026
Merged

feat(launcher): macOS bundle detection, iTerm2 validation, PR #421 review fixes#422
clcollins merged 4 commits into
mainfrom
srepd/macos-terminal-coverage

Conversation

@clcollins

Copy link
Copy Markdown
Owner

Summary

  • macOS bundle detection: Detect kitty, alacritty, wezterm, ghostty installed as .app bundles in /Applications/ — the config wizard now offers them even when they're not on PATH
  • Conditional iTerm2: Only offer iTerm2 in detection results when /Applications/iTerm.app actually exists (previously unconditional on darwin)
  • validateTerminalExists enhancement: Check for iTerm2.app existence after confirming osascript is available
  • PR fix(launcher): fix macOS AppleScript terminal env var passing and escaping #421 review fixes: login() integration tests for AppleScript wrapper path, env var duplication fix in ocm+wrapper flow, extracted shared buildTerminalCommand helper, clarifying comment on AppleScript test

Detail

Detection (detect.go)

Added macOSBundleTerminals map and injectable statFn parameter to DetectTerminals. On darwin, after PATH probing, each known bundle path is stat'd. PATH-found terminals take precedence. iTerm2 now guarded by statFn("/Applications/iTerm.app"). Terminal.app stays unconditional.

PR #421 review items

  1. login() integration tests (commands_test.go): 3 tests proving the wrapper branch is taken for AppleScript (osascript error on Linux), wrapper scripts are created, and env vars aren't duplicated in the ocm+wrapper flow
  2. Env var duplication fix (commands.go): When ocm-container is present, env vars are now passed only as -e flags — export lines in the wrapper script are skipped
  3. Shared helper (launcher.go): Extracted buildTerminalCommand() — all three Build*Command methods delegate to it, removing ~50 lines of duplication
  4. Script path escaping: Already handled by existing appleScriptEscape in BuildCommand — no change needed

README

Added macOS terminal support section documenting bundle detection, wrapper scripts, and cleanup behavior.

Test plan

  • make fmt-check — clean
  • make vet — clean
  • make lint — 0 issues
  • make test — all pass
  • make test-race — all pass
  • make build + /tmp/srepd --dev — TUI renders correctly, no regressions
  • make plan-checkdocs/plans/416-macos-terminal-coverage.md present
  • make readme-check — README in diff (real update, not skip-readme)
  • macOS testing: no device available; validated by unit tests with injectable fakeStat/fakeLookPath

🤖 Generated with Claude Code

clcollins and others added 4 commits August 9, 2026 12:27
…view fixes

Add macOS .app bundle detection for kitty, alacritty, wezterm, ghostty
in /Applications/. Make iTerm2 conditional on installation. Enhance
validateTerminalExists for iterm2 app check. Fix env var duplication in
ocm-container + AppleScript wrapper flow. Extract shared buildTerminalCommand
helper. Add login() integration tests for AppleScript wrapper path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rop ghostty, test isolation

Bundle-detected terminals now emit the full binary path as Command
(e.g., /Applications/kitty.app/Contents/MacOS/kitty) so exec.Command
can find them even when not on PATH. Drop ghostty from bundle map —
its macOS CLI cannot reliably launch the terminal (needs open -na).
Tests now use t.TempDir() via SREPD_WRAPPER_DIR env override instead
of writing to the real ~/.cache/srepd/launch directory. Drop misleading
TestLogin_AppleScriptWrapperWriteFailureReturnsError test.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ER_DIR to SREPD_TEST_WRAPPER_DIR

Add docs/terminals.md covering all supported terminals across Linux,
Flatpak, macOS, and Toolbox — profiles, env var passing, wrapper scripts,
and known limitations. Rename SREPD_WRAPPER_DIR to SREPD_TEST_WRAPPER_DIR
to make clear it is a test hook, not user-facing configuration.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The example showed --env= flags combined with an ocm-container login
command, but those are mutually exclusive branches in login(). Switched
to a non-ocm login command. Also fixed INC_ID shorthand to use real
PAGERDUTY_* variable names and added missing cluster_login_command.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@clcollins
clcollins merged commit ca1f1f6 into main Aug 10, 2026
12 checks passed
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.

1 participant