Skip to content
Merged
Changes from all commits
Commits
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
93 changes: 77 additions & 16 deletions prod-run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ SYSTEMD_STDOUT_LOG="$LOG_DIR/runewager-1.log"
SERVICE_FILE="/etc/systemd/system/runewager.service"
LOGROTATE_FILE="/etc/logrotate.d/runewager"
CRON_LINE='0 3 * * * /usr/sbin/logrotate -f /etc/logrotate.d/runewager'
HEALTH_LOG_MAX_AGE_MINUTES=10

say() {
printf '[runewager] %s\n' "$*"
Expand Down Expand Up @@ -45,6 +46,55 @@ ensure_log_paths() {
fi
}


get_bot_pid() {
pgrep -f "node .*index\.js" | head -n 1 || true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The bot PID detection uses a very generic pgrep -f "node .*index\.js" pattern, which can match any node index.js process on the host, so on systems running multiple Node apps it may incorrectly treat an unrelated process as this bot, causing health summaries and launch logic to report the bot as running and skip starting it when it is actually down. [logic error]

Severity Level: Major ⚠️
- ❌ Bot restart skipped when another node index.js is running.
- ⚠️ Health summary misreports status using wrong PID and logs.
- ⚠️ Operators may trust false positives from prod-run.sh checks.
Suggested change
pgrep -f "node .*index\.js" | head -n 1 || true
# Restrict match to this project's index.js to avoid grabbing unrelated node processes
pgrep -f "node .*${PROJECT_DIR}/index\.js" | head -n 1 || true
Steps of Reproduction ✅
1. Deploy the app using the documented flow so that systemd or an operator runs
`/var/www/html/Runewager/prod-run.sh` as described in `README.md:102-171` and
`runewager.service:8-9`.

2. Start a different Node service on the same host with a generic entrypoint, e.g. `node
/opt/otherapp/index.js`, so that `pgrep -f "node .*index\.js"` matches this unrelated
process.

3. Stop the actual Runewager bot so there is no `node $PROJECT_DIR/index.js` process, then
invoke `./prod-run.sh` (or let systemd restart it), which calls `check_bot_status_twice()`
at `prod-run.sh:487-490`.

4. Observe that `get_bot_pid()` at `prod-run.sh:50-52` returns the PID of the other app,
causing `print_health_summary()` at `prod-run.sh:66-75` and `is_bot_running()` at
`prod-run.sh:483-485` to treat the bot as running, so `start_background_if_needed()` at
`prod-run.sh:493-513` skips launching the real bot even though it is down.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** prod-run.sh
**Line:** 51:51
**Comment:**
	*Logic Error: The bot PID detection uses a very generic `pgrep -f "node .*index\.js"` pattern, which can match any `node index.js` process on the host, so on systems running multiple Node apps it may incorrectly treat an unrelated process as this bot, causing health summaries and launch logic to report the bot as running and skip starting it when it is actually down.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

}

log_age_seconds() {
local file="$1"
if [ ! -f "$file" ]; then
echo -1
return
fi
local now mtime
now="$(date +%s)"
mtime="$(stat -c %Y "$file" 2>/dev/null || echo 0)"
echo $((now - mtime))
}

print_health_summary() {
local pid
pid="$(get_bot_pid)"

if [ -n "$pid" ]; then
say "✅ Bot is running (pid: $pid)"
else
err "❌ Bot is not running"
return
fi

local active_log="$BOT_LOG"
if [ ! -s "$active_log" ] && [ -s "$LEGACY_BOT_LOG" ]; then
active_log="$LEGACY_BOT_LOG"
fi

local age
age="$(log_age_seconds "$active_log")"
if [ -s "$active_log" ] && [ "$age" -ge 0 ] && [ "$age" -le $((HEALTH_LOG_MAX_AGE_MINUTES * 60)) ]; then
say "✅ Health check passed: recent logs are present (${age}s old) at $active_log"
return
fi

if [ -s "$active_log" ] && [ "$age" -gt $((HEALTH_LOG_MAX_AGE_MINUTES * 60)) ]; then
warn "Bot process is running but logs are stale (${age}s old) at $active_log"
else
warn "Bot process is running but no recent logs were found"
fi

warn "If /start is unresponsive, restart with: pkill -f 'node .*index\.js' && bash $PROJECT_DIR/prod-run.sh"
}

fix_working_directory() {
if [ "$(pwd)" != "$PROJECT_DIR" ]; then
say "Switching working directory to $PROJECT_DIR"
Expand Down Expand Up @@ -265,6 +315,13 @@ fix_require_and_import_hints() {
}

ensure_logrotate_config() {
local log_owner="runewager"
local log_group="runewager"
if ! id -u "$log_owner" >/dev/null 2>&1; then
log_owner="$(id -un 2>/dev/null || echo root)"
log_group="$(id -gn 2>/dev/null || echo root)"
fi

local conf
conf=$(cat <<CONF
$LOG_DIR/runewager*.log $LOG_DIR/crash.log {
Expand All @@ -276,7 +333,7 @@ $LOG_DIR/runewager*.log $LOG_DIR/crash.log {
compress
delaycompress
copytruncate
create 0640 runewager runewager
create 0640 $log_owner $log_group
}
CONF
)
Expand All @@ -296,8 +353,10 @@ CONF
fi

if [ -f "$LOGROTATE_FILE" ] && command -v logrotate >/dev/null 2>&1; then
if ! logrotate -d "$LOGROTATE_FILE" >/dev/null 2>&1; then
local logrotate_output=''
if ! logrotate_output="$(logrotate -d "$LOGROTATE_FILE" 2>&1)"; then
warn "logrotate dry-run failed for $LOGROTATE_FILE"
warn "$logrotate_output"
else
say "logrotate config validation passed"
fi
Expand All @@ -319,6 +378,14 @@ ensure_cron_fallback() {
}

ensure_systemd_service() {
local service_user="runewager"
local service_group="runewager"
if ! id -u "$service_user" >/dev/null 2>&1; then
service_user="$(id -un 2>/dev/null || echo root)"
service_group="$(id -gn 2>/dev/null || echo root)"
warn "runewager user missing; using $service_user:$service_group for systemd service"
fi

local conf
conf=$(cat <<CONF
[Unit]
Expand All @@ -330,8 +397,8 @@ Wants=network-online.target
Type=simple
WorkingDirectory=$PROJECT_DIR
ExecStart=$PROJECT_DIR/prod-run.sh
User=runewager
Group=runewager
User=$service_user
Group=$service_group
EnvironmentFile=$PROJECT_DIR/.env
Environment=NODE_ENV=production
Restart=always
Expand Down Expand Up @@ -360,6 +427,8 @@ CONF
local required=(
"WorkingDirectory=$PROJECT_DIR"
"ExecStart=$PROJECT_DIR/prod-run.sh"
"User=$service_user"
"Group=$service_group"
"EnvironmentFile=$PROJECT_DIR/.env"
"StandardOutput=append:$SYSTEMD_STDOUT_LOG"
'Environment=NODE_ENV=production'
Expand Down Expand Up @@ -412,21 +481,13 @@ print_human_errors() {
}

is_bot_running() {
pgrep -f "node .*index\.js" >/dev/null 2>&1
[ -n "$(get_bot_pid)" ]
}

check_bot_status_twice() {
if is_bot_running; then
say "Bot running"
else
say "Bot NOT running"
fi
print_health_summary
sleep 15
if is_bot_running; then
say "Bot running"
else
say "Bot NOT running"
fi
print_health_summary
}

start_background_if_needed() {
Expand Down Expand Up @@ -482,10 +543,10 @@ main() {
ensure_cron_fallback
ensure_systemd_service

print_human_errors
check_bot_status_twice
start_background_if_needed
check_bot_status_twice
print_human_errors
}

main "$@"