Skip to content

terminal-helper: Honor $TERMINAL and harden terminal launching - #289

Open
lenitain wants to merge 1 commit into
CachyOS:developfrom
lenitain:develop
Open

lenitain wants to merge 1 commit into
CachyOS:developfrom
lenitain:develop

Conversation

@lenitain

Copy link
Copy Markdown

Summary

terminal-helper documented $TERMINAL as the first-priority terminal (see the invariants comment), but never actually read it. The script always fell back to the hardcoded term_order list, so users' preferred terminal was ignored.

Honoring $TERMINAL makes every entry in the terminals map reachable, which exposed several latent issues in those previously untested code paths. This PR fixes them all.

Changes

  • Honor $TERMINAL — checked first, but only when the value is a known terminal from the terminals map (guarantees the correct invocation syntax) and the binary exists. Invalid/stale values fall back to term_order as before.
  • ptyxis: ptyxis -e … → ptyxis -- … — ptyxis has no -e option; -- PROGRAM is the documented form and implies standalone mode (no detached client).
  • kitty: added --wait-for-single-instance-window-close — previously, with a running kitty instance, the client rocess exited immediately and the temp script could be deleted before the new window read it.
  • Launch fallback chain — candidates are tried in order ($TERMINAL first, then installed terminals in term_order). If a launch fails, the next candidate is tried; retrying stops on exit codes 126/127 (e.g. the user dismissed the pkexec prompt), and if no terminal launches at all a desktop notification is shown and the script exits 2.
  • Exit-code normalization — exit 0 is appended to the temp script. Terminals like foot and st propagate the child's exit status, which must not be mistaken for a launch failure (previously this would have re-run the command in the next terminal).
  • kgx — the kill command is now also appended when $TERMINAL=kgx, guarded against $PPID == 1, and exit status 131 (killed by SIGQUIT after completion) is treated as a successful launch instead of a failure.
  • Robustness — explicit aborts on mktemp/write failures (replacing the removed set -e), and the temp file is cleaned up on every path, including the "no terminal installed" path that previously leaked it.

Notes

  • With escalate=true, real pkexec forks, so kgx's kill line targets the pkexec wrapper rather than the terminal — the window may stay open after the command finishes. Cosmetic only; the command itself has already completed.

- Check $TERMINAL before falling back to the hardcoded term_order list;
  only honor it when the terminal is in the known terminals array, so the
  correct invocation syntax is always used.
- Fix invocations exposed by honoring $TERMINAL: ptyxis takes
  "-- PROGRAM" instead of -e, and kitty needs
  --wait-for-single-instance-window-close so the temp file isn't removed
  while a single-instance window is still opening it.
- Try the ordered candidates until one launches; skip retrying on 126/127
  (e.g. the user dismissed pkexec), notify and exit 2 when none work.
- Append "exit 0" to the command script: terminals like foot and st
  propagate the child exit status, which must not be mistaken for a
  failed launch.
- kgx: append the kill command for $TERMINAL=kgx too, guard it against
  PPID 1, and treat exit 131 (killed after completion) as success.
- Remove set -e but abort on mktemp/echo failures and always clean up
  the temp file.
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