terminal-helper: remove scripts after execution - #89
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The newly appended cleanup line uses bash-specific %q quoting even though -s allows changing shells; switching to rm -f -- "$0" (and using rm -f -- in the immediate cleanup path) would make the behavior robust and portable.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adjusts src/terminal-helper so the temporary script used to launch privileged commands is not deleted until the terminal-run shell completes, preventing races where pkexec (or the wrapper script) is handed a path that has already been removed (notably affecting kgx/GNOME Console scenarios).
Changes:
- Switch script content emission from
echotoprintffor predictable output/newline behavior. - Move temp-script cleanup into the generated script itself (self-cleaning after execution).
- Fix the launcher-failure conditional so a successful non-
kgxlaunch does not trigger cleanup/exit.
File summaries
| File | Description |
|---|---|
| src/terminal-helper | Writes a self-cleaning temp script and adjusts terminal-launch failure handling to avoid removing the script before it is executed. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Summary
This prevents
pkexecfrom receiving a path that has already been removed, notably with GNOME Console (kgx).Closes #87
Validation
bash -n src/terminal-helperalacritty; verified the command ran and its temporary script was removed afterwardsgit diff --check