Skip to content

GH#2899: Fix quality-debt review feedback on memory-pressure-monitor.sh #2930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marcusquinn merged 1 commit into from
Mar 5, 2026
Merged

GH#2899: Fix quality-debt review feedback on memory-pressure-monitor.sh #2930

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 26 additions & 20 deletions .agents/scripts/memory-pressure-monitor.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ AGGREGATE_RSS_WARN_MB="${AGGREGATE_RSS_WARN_MB:-8192}" # 8 GB total
# Auto-kill: ShellCheck processes are safe to kill (language server respawns them)
readonly AUTO_KILL_SHELLCHECK="${AUTO_KILL_SHELLCHECK:-true}"

# Notification
readonly COOLDOWN_SECS="${MEMORY_COOLDOWN_SECS:-300}"
# Notification — COOLDOWN_SECS and DAEMON_INTERVAL validated below with _validate_int
COOLDOWN_SECS="${MEMORY_COOLDOWN_SECS:-300}"
readonly NOTIFY_ENABLED="${MEMORY_NOTIFY:-true}"
readonly DAEMON_INTERVAL="${MEMORY_DAEMON_INTERVAL:-60}"
DAEMON_INTERVAL="${MEMORY_DAEMON_INTERVAL:-60}"

# Paths
readonly LOG_DIR="${MEMORY_LOG_DIR:-${HOME}/.aidevops/logs}"
Expand Down Expand Up @@ -120,6 +120,9 @@ SHELLCHECK_RUNTIME_MAX=$(_validate_int SHELLCHECK_RUNTIME_MAX "$SHELLCHECK_RUNTI
TOOL_RUNTIME_MAX=$(_validate_int TOOL_RUNTIME_MAX "$TOOL_RUNTIME_MAX" 1800 120)
SESSION_COUNT_WARN=$(_validate_int SESSION_COUNT_WARN "$SESSION_COUNT_WARN" 5 2)
AGGREGATE_RSS_WARN_MB=$(_validate_int AGGREGATE_RSS_WARN_MB "$AGGREGATE_RSS_WARN_MB" 8192 1024)
COOLDOWN_SECS=$(_validate_int COOLDOWN_SECS "$COOLDOWN_SECS" 300 30)
DAEMON_INTERVAL=$(_validate_int DAEMON_INTERVAL "$DAEMON_INTERVAL" 60 10)
readonly COOLDOWN_SECS DAEMON_INTERVAL

# --- Helpers ------------------------------------------------------------------

Expand Down Expand Up @@ -164,14 +167,19 @@ notify() {
return 0
fi

# Fallback: osascript — sanitise inputs to prevent AppleScript injection
# Fallback: osascript — pass title/message as positional arguments via
# 'on run argv', piping the AppleScript via stdin. This prevents injection
# because the values are never interpolated into the AppleScript source code.
# Previous approach used -e with string interpolation, which allowed breakout
# via crafted process names (e.g., '"; do shell script "...').
if command -v osascript &>/dev/null; then
# Replace quotes and backslashes to prevent breaking out of the string
local safe_title="${title//\\/\\\\}"
safe_title="${safe_title//\"/\\\"}"
local safe_message="${message//\\/\\\\}"
safe_message="${safe_message//\"/\\\"}"
osascript -e "display notification \"${safe_message}\" with title \"${safe_title}\"" 2>/dev/null || true
osascript - "$title" "$message" <<-'APPLESCRIPT' 2>/dev/null || true
on run argv
set theTitle to item 1 of argv
set theMessage to item 2 of argv
display notification theMessage with title theTitle
end run
APPLESCRIPT
return 0
fi

Expand All @@ -186,7 +194,7 @@ check_cooldown() {
local cooldown_file="${STATE_DIR}/memory-pressure-${category}.cooldown"
if [[ -f "${cooldown_file}" ]]; then
local last_notify
last_notify="$(cat "${cooldown_file}" 2>/dev/null || echo 0)"
last_notify="$(cat "${cooldown_file}" || echo 0)"
if ! [[ "$last_notify" =~ ^[0-9]+$ ]]; then
last_notify=0
fi
Expand Down Expand Up @@ -310,10 +318,9 @@ _collect_monitored_processes() {

while IFS= read -r line; do
[[ -z "$line" ]] && continue
# Parse with read builtin — avoids spawning echo/awk/cut subshells per line
local pid rss_kb cmd
pid=$(echo "$line" | awk '{print $1}')
rss_kb=$(echo "$line" | awk '{print $2}')
cmd=$(echo "$line" | cut -d' ' -f3-)
read -r pid rss_kb cmd <<<"$line"

# Validate PID and RSS are numeric
[[ "$pid" =~ ^[0-9]+$ ]] || continue
Expand All @@ -323,9 +330,10 @@ _collect_monitored_processes() {
local runtime
runtime=$(_get_process_age "$pid")

# Extract short command name
# Extract short command name via parameter expansion (no subshell)
local cmd_path="${cmd%% *}"
local cmd_name
cmd_name=$(basename "$(echo "$cmd" | awk '{print $1}')" 2>/dev/null || echo "unknown")
cmd_name=$(basename "$cmd_path" 2>/dev/null || echo "unknown")

printf '%s|%s|%s|%s|%s\n' "$pid" "$rss_mb" "$runtime" "$cmd_name" "$cmd"
done <<<"$ps_output"
Expand All @@ -339,10 +347,8 @@ _count_interactive_sessions() {
local ps_output
ps_output=$(ps axo pid=,tty=,command= 2>/dev/null | grep -iE "(opencode|claude)" | grep -v "grep" | grep -v "run " || true)

while IFS= read -r line; do
[[ -z "$line" ]] && continue
local tty
tty=$(echo "$line" | awk '{print $2}')
while read -r _ tty _; do
# Parse with read builtin — avoids spawning echo/awk subshells per line
# Interactive sessions have a TTY (not "??" on macOS or "?" on Linux)
if [[ "$tty" != "??" && "$tty" != "?" ]]; then
count=$((count + 1))
Expand Down
Loading