fix(actions): stop shuts guests down gracefully instead of pulling the plug

CHANGELOG.md

@@ -8,6 +8,18 @@ Versions follow [Semantic Versioning](https://semver.org/).
 
 ## [Unreleased]
 
+### Fixed
+
+- Stop action (menu key 2) now runs `qm shutdown` / `pct shutdown` with
+  `--timeout STOP_TIMEOUT --forceStop 1`: the guest is shut down cleanly and
+  only hard-stopped by Proxmox once the timeout expires. Previously it called
+  `qm stop` / `pct stop`, which kills the guest immediately (like pulling the
+  power plug); the `--overrule-shutdown 1` retry did not force anything either,
+  it only aborts running shutdown tasks.
+- Tests: `qm`/`pct` mocks log their calls (`PMAN_MOCK_GUEST_LOG`) and can fail
+  on demand (`PMAN_MOCK_GUEST_FAIL`); new tests cover stop, its failure path,
+  a declined confirmation and restart for VM and CT.
+
 ## [2.13.0] - 2026-09-19
 
 ### Added

README.md

@@ -73,7 +73,7 @@ Check out how others are using `proxmox-manager`:
 | 🩺 | **Health Monitoring** | CPU / RAM / disk per guest, uptime and OK / WARN / CRIT — `pman --health`, key `h` in the TUI, `Health:` line in the status action |
 | 🔔 | **Alerts** | `pman --check` for cron: thresholds, stopped `onboot` guests, unexpected stops and failed tasks; ntfy / e-mail only when something changes (opt-in) |
 | 📋 | **List & Status** | All VMs and containers with live status — `[+]` running · `[-]` stopped · `[~]` paused · `[?]` unknown |
-| ⚡ | **Start / Stop / Restart** | Confirmation prompt for destructive actions. Proxmox error details on failure. Configurable timeout with force-stop fallback |
+| ⚡ | **Start / Stop / Restart** | Confirmation prompt for destructive actions. Proxmox error details on failure. Stop is a graceful shutdown (ACPI / guest agent); Proxmox force-stops the guest after the configurable timeout |
 | 🖥️ | **Console Access** | LXC shell via `pct enter` or QEMU terminal via `qm terminal`. Verifies running state before entering |
 | 🌐 | **IP Address Lookup** | Shows current IPv4 addresses for running VMs and CTs — VM via QEMU Guest Agent, CT via `pct exec` |
 | 📦 | **Snapshot Management** | List, create, rollback, delete — with name validation and snapshot preview before destructive actions |
@@ -214,7 +214,7 @@ the checkout cannot change the privileged executable.
 | `pman --filter running` | Filter output by status: `running` \| `stopped` \| `paused` |
 | `pman --name web` | Filter by VM/CT name (ERE substring-match; combinable with `--filter`) |
 | `pman --force` | Skip all confirmation prompts (for unattended scripts) |
-| `pman --timeout 30` | Custom stop timeout in seconds (default: 60) |
+| `pman --timeout 30` | Graceful shutdown timeout before force stop, in seconds (default: 60) |
 | `pman --no-clear` | Don't clear screen in interactive mode |
 | `pman --once` | Run a single interactive refresh cycle (useful for TTY recording) |
 | `pman --health` | Health overview: CPU / MEM / DISK %, uptime and level per guest (`--list`, `--json` combinable) |
@@ -451,7 +451,7 @@ CLI flags are applied last and always win over config file values.
 
 ```bash
 # ~/.pmanrc — example
-STOP_TIMEOUT=120                          # stop timeout in seconds (default: 60)
+STOP_TIMEOUT=120                          # shutdown timeout before force stop (default: 60)
 LOG_FILE="/var/log/proxmox-manager.log"   # structured log file (empty = disabled)
 PROXMOX_MANAGER_SPICE_ADDR="spice.example.invalid" # SPICE bind address
 HEALTH_MEM_WARN=80                        # health thresholds, see "Health monitoring & alerts"
@@ -547,7 +547,7 @@ shellcheck proxmox-manager.sh
 tests/run.sh
 ```
 
-131 tests covering `validate_vmid`, `validate_snapshot_name`, `ip_info`,
+141 tests covering `validate_vmid`, `validate_snapshot_name`, `ip_info`,
 `--filter`, CLI flags, config parsing, the health view, the `--check` engine and
 notifications (mocked `pvesh`, `curl`, `sendmail`; no network, no root).
 

completions/pman.zsh

@@ -18,7 +18,7 @@ _pman() {
     '--filter[Filter output by status]:status:(running stopped paused)' \
     '--no-clear[Do not clear the screen in interactive mode]' \
     '--once[Run a single interactive refresh cycle]' \
-    '--timeout[Timeout in seconds for stop operations]:seconds:(10 30 60 120 300)' \
+    '--timeout[Shutdown timeout in seconds before force stop]:seconds:(10 30 60 120 300)' \
     '--name[Filter by VM/CT name (ERE substring-match)]:pattern:' \
     '--force[Skip all confirmation prompts]' \
     '--health[Show CPU/memory/disk health of local VMs/CTs]' \

proxmox-manager.sh

@@ -22,7 +22,7 @@ LIST_FLAG=0
 JSON_FLAG=0
 LOG_FILE="${LOG_FILE:-}"           # Set LOG_FILE=/path/to/file to enable file logging
 FILTER_STATUS=""                   # Filter output by status: running|stopped|paused (empty = no filter)
-STOP_TIMEOUT="${STOP_TIMEOUT:-60}" # Timeout in seconds for stop operations; env-overridable
+STOP_TIMEOUT="${STOP_TIMEOUT:-60}" # Graceful shutdown timeout before force stop; env-overridable
 PROXMOX_MANAGER_SPICE_ADDR="${PROXMOX_MANAGER_SPICE_ADDR:-}"
 FORCE_MODE=0              # Set to 1 via --force to skip all confirm() prompts
 FILTER_NAME=""            # ERE substring-match against VM/CT name (empty = no filter)
@@ -535,7 +535,7 @@ Options:
   --name PATTERN    Filter by VM/CT name (ERE substring-match; combinable with --filter)
   --no-clear        Do not clear the screen in interactive mode
   --once            Run a single interactive refresh (useful for TTY recording)
-  --timeout SECS    Timeout for stop operations in seconds (default: 60)
+  --timeout SECS    Shutdown timeout before force stop in seconds (default: 60)
   --force           Skip all confirmation prompts (use with care)
   --health          Show CPU/memory/disk health of local VMs/CTs
                     (combine with --list for plain text or --json; --filter/--name apply)
@@ -2781,41 +2781,24 @@ do_action() {
       note "Aborted."
       return
     }
-    note "Stopping $ty $id ($name) (timeout: ${STOP_TIMEOUT}s)..."
-    local _timeout_out='' _timeout_exit=0 _force_out='' _force_exit=0
+    # Graceful shutdown (ACPI / guest agent); Proxmox itself hard-stops the guest
+    # once STOP_TIMEOUT expires (--forceStop 1). A plain "qm/pct stop" would
+    # kill the guest immediately, like pulling the power plug.
+    note "Shutting down $ty $id ($name) (force stop after ${STOP_TIMEOUT}s)..."
+    local _pve_out
     if [[ "$ty" == "CT" ]]; then
-      _timeout_out=$(timeout "${STOP_TIMEOUT}" pct stop "$id" 2>&1) || _timeout_exit=$?
-      if ((_timeout_exit == 0)); then
+      if _pve_out=$(pct shutdown "$id" --timeout "$STOP_TIMEOUT" --forceStop 1 2>&1); then
         ok "$ty $id stopped."
-      elif ((_timeout_exit == 124)); then
-        note "Timeout after ${STOP_TIMEOUT}s. Forcing stop with --overrule-shutdown..."
-        _force_out=$(pct stop "$id" --overrule-shutdown 1 2>&1) || _force_exit=$?
-        if ((_force_exit == 0)); then
-          ok "$ty $id force-stopped."
-        else
-          err "Force stop failed for CT $id."
-          _log_pve_err "$_force_out"
-        fi
       else
         err "Failed to stop CT $id."
-        _log_pve_err "$_timeout_out"
+        _log_pve_err "$_pve_out"
       fi
     else
-      _timeout_out=$(timeout "${STOP_TIMEOUT}" qm stop "$id" 2>&1) || _timeout_exit=$?
-      if ((_timeout_exit == 0)); then
+      if _pve_out=$(qm shutdown "$id" --timeout "$STOP_TIMEOUT" --forceStop 1 2>&1); then
         ok "$ty $id stopped."
-      elif ((_timeout_exit == 124)); then
-        note "Timeout after ${STOP_TIMEOUT}s. Forcing stop with --overrule-shutdown..."
-        _force_out=$(qm stop "$id" --overrule-shutdown 1 2>&1) || _force_exit=$?
-        if ((_force_exit == 0)); then
-          ok "$ty $id force-stopped."
-        else
-          err "Force stop failed for VM $id."
-          _log_pve_err "$_force_out"
-        fi
       else
         err "Failed to stop VM $id."
-        _log_pve_err "$_timeout_out"
+        _log_pve_err "$_pve_out"
       fi
     fi
     ;;

tests/mock-bin/pct

@@ -1,6 +1,15 @@
 #!/usr/bin/env bash
 set -euo pipefail
 
+# PMAN_MOCK_GUEST_LOG: append argv; PMAN_MOCK_GUEST_FAIL: exit 1 for that subcommand.
+if [[ -n "${PMAN_MOCK_GUEST_LOG:-}" ]]; then
+  printf '%s %s\n' "$(basename "$0")" "$*" >>"$PMAN_MOCK_GUEST_LOG"
+fi
+if [[ -n "${PMAN_MOCK_GUEST_FAIL:-}" && "${1:-}" == "$PMAN_MOCK_GUEST_FAIL" ]]; then
+  echo "mock: $1 failed for ${2:-}" >&2
+  exit 1
+fi
+
 cmd="${1:-}"
 shift || true
 

tests/mock-bin/qm

@@ -1,6 +1,15 @@
 #!/usr/bin/env bash
 set -euo pipefail
 
+# PMAN_MOCK_GUEST_LOG: append argv; PMAN_MOCK_GUEST_FAIL: exit 1 for that subcommand.
+if [[ -n "${PMAN_MOCK_GUEST_LOG:-}" ]]; then
+  printf '%s %s\n' "$(basename "$0")" "$*" >>"$PMAN_MOCK_GUEST_LOG"
+fi
+if [[ -n "${PMAN_MOCK_GUEST_FAIL:-}" && "${1:-}" == "$PMAN_MOCK_GUEST_FAIL" ]]; then
+  echo "mock: $1 failed for ${2:-}" >&2
+  exit 1
+fi
+
 cmd="${1:-}"
 shift || true
 

tests/run.sh

@@ -486,6 +486,62 @@ else
   _fail "status action: health line missing"
 fi
 
+# ---------------------------------------------------------------------------
+# Lifecycle actions: stop is a graceful shutdown with force-stop fallback
+# ---------------------------------------------------------------------------
+guest_log="$TEST_TMP/guest.log"
+# _act INPUT ID TYPE ACTION [VAR=VAL...] — run do_action in a subshell with a fresh guest log; sets act_out.
+_act() {
+  local input="$1" id="$2" ty="$3" act="$4"
+  shift 4
+  : >"$guest_log"
+  act_out="$(
+    export PMAN_MOCK_GUEST_LOG="$guest_log"
+    for kv in "$@"; do export "${kv?}"; done
+    STOP_TIMEOUT="${STOP_TIMEOUT:-60}"
+    printf '%s\n' "$input" | do_action "$id" "$ty" "$act" "name-$id" 2>&1
+  )" || true
+}
+
+for g in VM:qm:200 CT:pct:100; do
+  IFS=: read -r gty gcmd gid <<<"$g"
+  _act y "$gid" "$gty" stop
+  if grep -qx "$gcmd shutdown $gid --timeout 60 --forceStop 1" "$guest_log" &&
+    ! grep -q "^$gcmd stop" "$guest_log" && grep -q "$gty $gid stopped." <<<"$act_out"; then
+    _pass "stop action ($gty): graceful shutdown with --forceStop, never '$gcmd stop'"
+  else
+    _fail "stop action ($gty): unexpected calls: $(tr '\n' ';' <"$guest_log")"
+  fi
+
+  _act y "$gid" "$gty" stop STOP_TIMEOUT=17
+  if grep -qx "$gcmd shutdown $gid --timeout 17 --forceStop 1" "$guest_log"; then
+    _pass "stop action ($gty): STOP_TIMEOUT is passed to $gcmd shutdown"
+  else
+    _fail "stop action ($gty): STOP_TIMEOUT not passed: $(tr '\n' ';' <"$guest_log")"
+  fi
+
+  _act y "$gid" "$gty" stop PMAN_MOCK_GUEST_FAIL=shutdown
+  if grep -q "Failed to stop $gty $gid." <<<"$act_out" && grep -q "mock: shutdown failed for $gid" <<<"$act_out"; then
+    _pass "stop action ($gty): shutdown failure shows the Proxmox error"
+  else
+    _fail "stop action ($gty): shutdown failure not reported"
+  fi
+
+  _act n "$gid" "$gty" stop
+  if ! grep -qE "^$gcmd (shutdown|stop) " "$guest_log" && grep -q "Aborted." <<<"$act_out"; then
+    _pass "stop action ($gty): declined confirmation sends nothing"
+  else
+    _fail "stop action ($gty): declined confirmation still acted"
+  fi
+
+  _act y "$gid" "$gty" restart
+  if grep -qx "$gcmd reboot $gid" "$guest_log" && ! grep -qE "^$gcmd (stop|shutdown) " "$guest_log"; then
+    _pass "restart action ($gty): uses $gcmd reboot"
+  else
+    _fail "restart action ($gty): unexpected calls: $(tr '\n' ';' <"$guest_log")"
+  fi
+done
+
 # ---------------------------------------------------------------------------
 # Health check engine: --check / --dry-run (notifications are not sent here)
 # ---------------------------------------------------------------------------