uninstall_confirm swallows Ctrl-D: the abort is silent, while db.sh one file away gets it right #68

New issue

Closed

opened 2026-07-19 20:41:05 +00:00 by dan-claude-bot · 0 comments

dan-claude-bot commented

2026-07-19 20:41:05 +00:00

(Migrated from github.com)

Found while fixing the identical defect in box (heavy-duty/box#111 / PR heavy-duty/box#112), reading rig to match its house style. Filing it here rather than leaving it in a box PR body.

The defect

uninstall_confirm() at bin/rig:240 reads the operator's answer unguarded:

uninstall_confirm() {   # $1 = question
  [ "$force" -eq 1 ] && return 0
  [ -n "${RIG_YES:-}" ] && return 0
  if [ ! -t 0 ]; then
    printf 'rig: refusing to %s without --force (no terminal to confirm on; RIG_YES=1 also means yes)\n' "$1" >&2
    exit 2
  fi
  local reply
  printf 'rig: %s? [y/N] ' "$1"
  read -r reply
  case "$reply" in y|Y|yes|YES|Yes) return 0 ;; *) die "aborted." ;; esac
}

Ctrl-D at the prompt makes read return non-zero. Under set -euo pipefail the function dies there — the case is never evaluated, so die "aborted." never fires. The operator sees the question, presses Ctrl-D, and gets nothing: no confirmation the uninstall was aborted, no message, just exit 1.

rig already has the cure, one file away

This is the part that makes it worth fixing rather than tolerating. commands/db.sh:152 handles the same shape correctly:

read -r reply || reply=""
case "$reply" in
  y|Y|yes|YES|Yes) ;;
  *) die "aborted — no changes made" ;;
esac

Same y/N confirm shape, same set -euo pipefail context, and it falls through to the die on EOF exactly as intended. So rig is internally inconsistent: the destructive database restore says "aborted" on Ctrl-D, and the destructive uninstall says nothing.

#43 cured this class for rig's four hidden -rsp token prompts using || { echo; die … }. Between that and db.sh, rig has now solved this twice — uninstall_confirm is simply the site that neither pass reached.

Severity: low, and honest about it

It fails closed. Nothing is uninstalled; the abort is real. The damage is purely that the tool goes silent at the exact moment it asked a question, which is the worst possible moment to say nothing — an operator who Ctrl-Ds out of rig uninstall cannot tell from the output whether anything was removed.

It is also not a regression; this has been the shape since uninstall_confirm was written.

The fix

One token, matching whichever sibling you prefer:

bin/rig:240 — read -r reply || reply="" (matches db.sh:152, the closer sibling — same confirm shape), or read -r reply || die "aborted." (matches what box#112 used)

Observable behavior is identical either way: exit 1 with the abort message on the prompt line. Worth picking one spelling and using it in both repos so this stops being re-litigated per site.

Testing note

The interactive accept/abort paths are currently untested here, structurally — they need a pty, which is exactly why this survived. box#112 built one: six checks in test/cli.sh driving y, n and Ctrl-D through util-linux script against the fake shim, guarded by a command -v script skip. Two details from that work worth copying if this gets a test:

Assert the message, not the exit code. The unfixed code also exits 1 on Ctrl-D, just silently — an exit-code assertion passes against the bug and proves nothing.
Mutation-verify it. box#112 confirmed unfixed bin/box gives 473 passed, 1 failed on output missing 'aborted.' and fixed gives 474 passed, 0 failed. A test that has not been shown to fail against the bug is not evidence.

A test is not required for a one-token fix, but if one is written, it should be that one.

Refs

heavy-duty/box#111 (the same defect in box) · heavy-duty/box#112 (the fix, with the pty harness) · #43 (the same class, cured for rig's token prompts) · commands/db.sh:152 (rig's own correct handling)

Found while fixing the identical defect in box (`heavy-duty/box#111` / PR `heavy-duty/box#112`), reading rig to match its house style. Filing it here rather than leaving it in a box PR body. ## The defect `uninstall_confirm()` at `bin/rig:240` reads the operator's answer unguarded: ```bash uninstall_confirm() { # $1 = question [ "$force" -eq 1 ] && return 0 [ -n "${RIG_YES:-}" ] && return 0 if [ ! -t 0 ]; then printf 'rig: refusing to %s without --force (no terminal to confirm on; RIG_YES=1 also means yes)\n' "$1" >&2 exit 2 fi local reply printf 'rig: %s? [y/N] ' "$1" read -r reply case "$reply" in y|Y|yes|YES|Yes) return 0 ;; *) die "aborted." ;; esac } ``` Ctrl-D at the prompt makes `read` return non-zero. Under `set -euo pipefail` the function dies there — the `case` is never evaluated, so `die "aborted."` never fires. The operator sees the question, presses Ctrl-D, and gets **nothing**: no confirmation the uninstall was aborted, no message, just exit 1. ## rig already has the cure, one file away This is the part that makes it worth fixing rather than tolerating. `commands/db.sh:152` handles the same shape correctly: ```bash read -r reply || reply="" case "$reply" in y|Y|yes|YES|Yes) ;; *) die "aborted — no changes made" ;; esac ``` Same y/N confirm shape, same `set -euo pipefail` context, and it falls through to the `die` on EOF exactly as intended. So rig is internally inconsistent: the destructive database restore says "aborted" on Ctrl-D, and the destructive uninstall says nothing. `#43` cured this class for rig's four hidden `-rsp` token prompts using `|| { echo; die … }`. Between that and `db.sh`, rig has now solved this twice — `uninstall_confirm` is simply the site that neither pass reached. ## Severity: low, and honest about it It **fails closed**. Nothing is uninstalled; the abort is real. The damage is purely that the tool goes silent at the exact moment it asked a question, which is the worst possible moment to say nothing — an operator who Ctrl-Ds out of `rig uninstall` cannot tell from the output whether anything was removed. It is also **not a regression**; this has been the shape since `uninstall_confirm` was written. ## The fix One token, matching whichever sibling you prefer: - `bin/rig:240` — `read -r reply || reply=""` (matches `db.sh:152`, the closer sibling — same confirm shape), or `read -r reply || die "aborted."` (matches what box#112 used) Observable behavior is identical either way: exit 1 with the abort message on the prompt line. Worth picking one spelling and using it in both repos so this stops being re-litigated per site. ## Testing note The interactive accept/abort paths are currently untested here, structurally — they need a pty, which is exactly why this survived. box#112 built one: six checks in `test/cli.sh` driving `y`, `n` and Ctrl-D through util-linux `script` against the fake shim, guarded by a `command -v script` skip. Two details from that work worth copying if this gets a test: - **Assert the message, not the exit code.** The unfixed code also exits 1 on Ctrl-D, just silently — an exit-code assertion passes against the bug and proves nothing. - **Mutation-verify it.** box#112 confirmed unfixed `bin/box` gives `473 passed, 1 failed` on `output missing 'aborted.'` and fixed gives `474 passed, 0 failed`. A test that has not been shown to fail against the bug is not evidence. A test is not required for a one-token fix, but if one is written, it should be that one. ## Refs `heavy-duty/box#111` (the same defect in box) · `heavy-duty/box#112` (the fix, with the pty harness) · #43 (the same class, cured for rig's token prompts) · `commands/db.sh:152` (rig's own correct handling)

claude-lead-andresmgsl referenced this issue

2026-07-29 15:23:22 +00:00

feat: install channel is forge-agnostic (RIG_HOST / BOX_HOST) #114

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/rig#68

No description provided.