Prevent sudo timeouts during long installs#6
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a sudo “keep-alive” mechanism to the installer so long-running installs don’t fail due to sudo credential timeout, and documents the behavior for users.
Changes:
- Add a background sudo heartbeat plus EXIT/INT/TERM cleanup traps in
run.sh. - Start the heartbeat near the beginning of
run.shbefore privileged install steps. - Document the new keep-alive behavior in
README.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| run.sh | Introduces sudo heartbeat process management and signal/exit cleanup; starts heartbeat at script startup. |
| README.md | Documents the new sudo keep-alive behavior during long installs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| cleanup_sudo_heartbeat() { | ||
| if [ -n "${SUDO_HEARTBEAT_PID:-}" ]; then | ||
| kill "$SUDO_HEARTBEAT_PID" >/dev/null 2>&1 || true |
There was a problem hiding this comment.
cleanup_sudo_heartbeat blindly kills whatever process currently has SUDO_HEARTBEAT_PID. If the heartbeat background job exits early (e.g., sudo -n -v fails due to sudo config) and the PID gets reused, this could terminate an unrelated process during cleanup. Consider guarding the kill by verifying the PID still belongs to a live background job started by this shell (e.g., check against jobs -p / kill -0 plus job identity) and/or wait+unset the PID after stopping the job to avoid PID reuse hazards.
| kill "$SUDO_HEARTBEAT_PID" >/dev/null 2>&1 || true | |
| # Only attempt to kill the process if it is still a background job of this shell. | |
| if jobs -p 2>/dev/null | grep -q "^${SUDO_HEARTBEAT_PID}\$"; then | |
| kill "$SUDO_HEARTBEAT_PID" >/dev/null 2>&1 || true | |
| # Wait for the process to ensure it is reaped and to reduce PID reuse hazards. | |
| wait "$SUDO_HEARTBEAT_PID" 2>/dev/null || true | |
| fi | |
| unset SUDO_HEARTBEAT_PID |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| if [ -n "${SUDO_HEARTBEAT_PID:-}" ]; then | ||
| kill "$SUDO_HEARTBEAT_PID" >/dev/null 2>&1 || true | ||
| fi | ||
| } |
There was a problem hiding this comment.
Stale PID causes double-kill on signal cleanup
Low Severity
cleanup_sudo_heartbeat never unsets SUDO_HEARTBEAT_PID after killing the process. On INT/TERM, the signal handler calls cleanup_sudo_heartbeat (first kill), resets its trap, then re-raises the signal, which causes the EXIT trap to call cleanup_sudo_heartbeat a second time with the same now-stale PID. This risks sending a signal to an unrelated process if the PID has been recycled.
Additional Locations (1)
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee5ec4aa09
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) & | ||
| SUDO_HEARTBEAT_PID=$! | ||
|
|
||
| trap cleanup_sudo_heartbeat EXIT |
There was a problem hiding this comment.
start_sudo_heartbeat() only tears down the keepalive on shell exit via trap cleanup_sudo_heartbeat EXIT, but run.sh later hands control to an interactive shell at run.sh:609-610. In the normal success path, the background loop from run.sh:34-39 therefore keeps calling sudo -n -v every 30 seconds for as long as the user stays in that zsh session, continuously extending the sudo ticket well after the installer is done. That turns a temporary install-time workaround into an effectively unbounded sudo grace period until the user exits the shell.
Useful? React with 👍 / 👎.
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
All requested follow-up work is now in. Final state before merge:
Verification completed:
Closes #4. |
Summary
run.shso long-running privileged installs keep their cached sudo session aliveREADME.mdVerification
bash -n run.shpixi run testsudo -vfollowed by repeatedsudo -n -vCloses #4
Note
Medium Risk
Touches installer execution flow by adding a background sudo keep-alive and new signal/exit traps; a bug here could leave stray processes or affect script termination, but scope is limited to setup scripting.
Overview
Prevents long
run.shinstalls from failing due to sudo credential timeouts by starting a background keep-alive loop after the first sudo prompt.Adds cleanup and signal handling (
EXIT/INT/TERM) to stop the heartbeat process when the script exits or is interrupted, and documents the new keep-alive behavior inREADME.md.Written by Cursor Bugbot for commit 0a88c94. This will update automatically on new commits. Configure here.