-
Notifications
You must be signed in to change notification settings - Fork 0
Claude/update prod runner script lq2g s #67
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.
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
Changes from all commits
a74ae80
9d23185
b7ec945
dfab1f4
c4ded2c
ecbc0e7
9461c2c
a8f7b44
790e8a9
5210111
7fd2c29
9b66dc0
4233e91
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -26,9 +26,28 @@ JOURNAL_VACUUM_MB=300 | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| # ── Logging ──────────────────────────────────────────────────────────────────── | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| log() { echo "[$(date -u +"%Y-%m-%dT%H:%M:%SZ")] [disk-protect] $*"; } | ||||||||||||||||||||||
| log() { echo "[$(date -u +"%Y-%m-%dT%H:%M:%SZ")] [disk-protect] $*"; } | ||||||||||||||||||||||
| warn() { echo "[$(date -u +"%Y-%m-%dT%H:%M:%SZ")] [disk-protect] WARN: $*" >&2; } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # ── Helper: delete files found by find and log each one reliably ─────────────── | ||||||||||||||||||||||
| # Uses process substitution + null-delimited names to handle filenames with | ||||||||||||||||||||||
| # spaces or special characters, and separates print/delete so logging is never | ||||||||||||||||||||||
| # suppressed by find implementation differences. | ||||||||||||||||||||||
| delete_found() { | ||||||||||||||||||||||
| # Usage: delete_found <find_args...> | ||||||||||||||||||||||
| # Example: delete_found "$LOG_DIR" -type f -mtime +7 | ||||||||||||||||||||||
| local _count=0 | ||||||||||||||||||||||
| while IFS= read -r -d '' f; do | ||||||||||||||||||||||
| if rm -f "$f"; then | ||||||||||||||||||||||
| log " Deleted: $f" | ||||||||||||||||||||||
| _count=$((_count + 1)) | ||||||||||||||||||||||
| else | ||||||||||||||||||||||
| warn " Failed to delete: $f" | ||||||||||||||||||||||
| fi | ||||||||||||||||||||||
| done < <(find "$@" -print0 2>/dev/null) | ||||||||||||||||||||||
| log " Total deleted: $_count file(s)" | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # ── 1. Log rotation (via logrotate if available) ─────────────────────────────── | ||||||||||||||||||||||
| log "Step 1: Log rotation" | ||||||||||||||||||||||
| if command -v logrotate >/dev/null 2>&1; then | ||||||||||||||||||||||
|
|
@@ -45,25 +64,27 @@ fi | |||||||||||||||||||||
| # ── 2. Compress logs older than 1 day ───────────────────────────────────────── | ||||||||||||||||||||||
| log "Step 2: Compress logs older than 1 day" | ||||||||||||||||||||||
| if [[ -d "$LOG_DIR" ]]; then | ||||||||||||||||||||||
| find "$LOG_DIR" -type f \( -name "*.log" -o -name "*.out" \) -mtime +1 ! -name "*.gz" | while read -r f; do | ||||||||||||||||||||||
| gzip -f "$f" && log " Compressed: $f" | ||||||||||||||||||||||
| done | ||||||||||||||||||||||
| while IFS= read -r -d '' f; do | ||||||||||||||||||||||
| if gzip -f "$f" 2>/dev/null; then | ||||||||||||||||||||||
| log " Compressed: $f" | ||||||||||||||||||||||
| else | ||||||||||||||||||||||
| warn " Failed to compress: $f" | ||||||||||||||||||||||
| fi | ||||||||||||||||||||||
| done < <(find "$LOG_DIR" -type f \( -name "*.log" -o -name "*.out" \) -mtime +1 ! -name "*.gz" -print0 2>/dev/null) | ||||||||||||||||||||||
| else | ||||||||||||||||||||||
| log " Log directory not found: $LOG_DIR — skipping" | ||||||||||||||||||||||
| fi | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # ── 3. Delete logs older than LOG_MAX_DAYS days ──────────────────────────────── | ||||||||||||||||||||||
| log "Step 3: Delete logs older than ${LOG_MAX_DAYS} days" | ||||||||||||||||||||||
| if [[ -d "$LOG_DIR" ]]; then | ||||||||||||||||||||||
| find "$LOG_DIR" -type f -mtime "+${LOG_MAX_DAYS}" -delete -print | while read -r f; do | ||||||||||||||||||||||
| log " Deleted old log: $f" | ||||||||||||||||||||||
| done | ||||||||||||||||||||||
| delete_found "$LOG_DIR" -type f -mtime "+${LOG_MAX_DAYS}" | ||||||||||||||||||||||
| fi | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # ── 4. journalctl vacuum ─────────────────────────────────────────────────────── | ||||||||||||||||||||||
| log "Step 4: journalctl vacuum (keep up to ${JOURNAL_VACUUM_MB}MB)" | ||||||||||||||||||||||
| if command -v journalctl >/dev/null 2>&1; then | ||||||||||||||||||||||
| journalctl --vacuum-size="${JOURNAL_VACUUM_MB}M" 2>&1 | grep -v "^$" | while read -r line; do | ||||||||||||||||||||||
| journalctl --vacuum-size="${JOURNAL_VACUUM_MB}M" 2>&1 | grep -v "^$" | while IFS= read -r line; do | ||||||||||||||||||||||
| log " [journal] $line" | ||||||||||||||||||||||
| done | ||||||||||||||||||||||
| else | ||||||||||||||||||||||
|
|
@@ -73,55 +94,55 @@ fi | |||||||||||||||||||||
| # ── 5. Clear npm cache ───────────────────────────────────────────────────────── | ||||||||||||||||||||||
| log "Step 5: Clear npm cache" | ||||||||||||||||||||||
| if command -v npm >/dev/null 2>&1; then | ||||||||||||||||||||||
| npm cache clean --force 2>&1 | tail -1 | while read -r line; do log " [npm] $line"; done | ||||||||||||||||||||||
| npm cache clean --force 2>&1 | tail -1 | while IFS= read -r line; do log " [npm] $line"; done | ||||||||||||||||||||||
| log " npm cache cleared" | ||||||||||||||||||||||
|
Comment on lines
96
to
98
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: With Severity Level: Major
|
||||||||||||||||||||||
| if command -v npm >/dev/null 2>&1; then | |
| npm cache clean --force 2>&1 | tail -1 | while read -r line; do log " [npm] $line"; done | |
| npm cache clean --force 2>&1 | tail -1 | while IFS= read -r line; do log " [npm] $line"; done | |
| log " npm cache cleared" | |
| if command -v npm >/devnull 2>&1; then | |
| if npm cache clean --force 2>&1 | tail -1 | while IFS= read -r line; do log " [npm] $line"; done; then | |
| log " npm cache cleared" | |
| else | |
| warn " npm cache clear failed (non-fatal)" | |
| fi |
Steps of Reproduction ✅
1. Ensure `/workspace/Runewager/scripts/disk-protect.sh` is executed with `set -euo
pipefail` enabled (line 12) either via the cron job installed in
`/workspace/Runewager/prod-run.sh` (disk-protect cron block around lines 22–32 in the
shown snippet) or by invoking the script manually.
2. Run the script on a system where `npm` is present (`command -v npm >/dev/null 2>&1` at
line 96 succeeds) but where `npm cache clean --force` will exit with a non-zero status
(for example, due to permission problems or a corrupted npm cache), causing the pipeline
at line 97 to fail.
3. When Step 5 executes, the pipeline `npm cache clean --force 2>&1 | tail -1 | while ...`
(line 97) returns a non-zero exit status; under `set -e` with `pipefail`, this unguarded
failing pipeline causes the entire `disk-protect.sh` script to terminate at Step 5.
4. Observe in `logs/disk-protect.log` that subsequent steps (Step 6 temp file cleanup at
lines 104–111, Step 7 backup pruning at lines 114–119, Step 8 safety check at lines
124–136, and Step 9 disk usage summary at lines 139–145) are missing, confirming that an
npm cache error aborted the disk-protect run instead of being treated as best-effort.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/disk-protect.sh
**Line:** 96:98
**Comment:**
*Logic Error: With `set -euo pipefail` enabled, a non-zero exit from `npm cache clean --force` will cause the whole script to exit at Step 5 even though clearing the npm cache is non-critical, and it will still log "npm cache cleared" on failure; wrapping this pipeline in an `if` and handling the failure explicitly avoids terminating the script and ensures success is logged only when the command actually succeeds.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Because the script uses
set -euo pipefail, a non-zero exit status fromjournalctl --vacuum-size(for example, due to insufficient permissions) will cause the entire disk-protect script to abort at Step 4, skipping later cleanup and safety checks even though the journal vacuum is intended as a best-effort, non-critical operation; adding a guard to ignore failures here prevents unintended termination. [logic error]Severity Level: Major⚠️
Steps of Reproduction ✅
Prompt for AI Agent 🤖