fix: make automatic resume pane-safe - #4
Merged
Conversation
증상: 5am 등 리밋 리셋 시각이 지나도, 화면에 확인 다이얼로그(Do you want to…? / ❯ 1.Yes)나 "How is Claude doing this session?" 피드백 프롬프트가 떠 있으면 가디언이 "⏸️ 확인 다이얼로그 대기 중"으로 영구 보류하고 resume를 보내지 않아 작업이 안 이어짐. 수정: - 리셋 도래 시 영구 보류 금지. 오버레이 감지 시 Escape×2(취소=안전 기본값 + 입력창 비우기) 후 무조건 resume 주입. - GUARDIAN_FEEDBACK_RE 신설(피드백 오버레이 매칭), DIALOG_RE와 함께 판정. - send-keys 텍스트→sleep1→C-m 분리 전송으로 Enter 레이스 방지. - selftest에 피드백 회귀 케이스 4종 추가 → ALL GREEN. 검증: selftest exit0, 합성 세션 end-to-end(Escape후 resume 전송), 실제 Claude Code TUI에서 동일 키 시퀀스 제출·응답(PONG) 확인. 함께 커밋: 이전 세션들의 미커밋 nightguardian 작업(keepalive.sh, parse_reset.py, CLI/Makefile, launchd plist) — 현재 동작 중인 데몬 상태를 정본화. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Why: session-wide input and duplicate retry ownership could send commands to the wrong terminal, so NightGuardian now pins and verifies one Claude pane with fail-closed QA before public use. Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughNightGuardian이 세션 단위 처리에서 pane 단위 감시·재개로 전환되었습니다. 리셋 시간 파서, Claude 프로세스 검증, pane별 상태·락·쿨다운, CLI 운영 명령, 통합 테스트와 fail-closed QA 게이트가 추가되었습니다. ChangesNightGuardian 안전 재개
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
.github/workflows/verify.yml (1)
16-16: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win보안 강화를 위해
actions/checkout에persist-credentials: false를 설정하세요.현재 설정에서는 GitHub Actions 토큰이
.git디렉터리에 남아 이후 단계나 써드파티 도구에 의해 노출될 수 있습니다. 코드를 체크아웃할 때 인증 정보가 로컬에 지속되지 않도록 명시적으로 비활성화하는 것을 권장합니다.🔒 제안하는 수정안
- - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/verify.yml at line 16, Update the actions/checkout step to explicitly disable persisted credentials by configuring persist-credentials to false, ensuring the GitHub Actions token is not retained in the local .git directory.Source: Linters/SAST tools
src/keepalive.sh (1)
24-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
nightguardian stop명령을 사용하여 tmux 세션 이름을 캡슐화하고 로그 기록을 통합하세요.현재 스크립트에
forge-night라는 tmux 세션 이름이 하드코딩되어 있습니다. 이로 인해 CLI 내부에 설정된$TMUX_SESSION환경변수와 세션 이름이 다를 경우 비정상 세션 정리가 제대로 이루어지지 않을 수 있습니다. 하드코딩된tmux kill-session대신nightguardian stop을 호출하여 세션 이름을 캡슐화하는 것을 권장합니다.또한, 여러 번 반복되는
>> "$LOG"리다이렉션을 하나의 중괄호{ ... } >> "$LOG"블록으로 묶으면 가독성이 향상되고 I/O 처리가 더 효율적이 됩니다.🛠 제안하는 수정안
-ts="$(date '+%Y-%m-%d %H:%M:%S')" -tmux kill-session -t forge-night 2>/dev/null || true -echo "[$ts] guardian 프로세스 부재 감지 → 재시작" >> "$LOG" -nightguardian start >> "$LOG" 2>&1 -echo "[$ts] 재시작 결과 pid: $(pgrep -f guardian-watch.sh 2>/dev/null || echo NONE)" >> "$LOG" +{ + ts="$(date '+%Y-%m-%d %H:%M:%S')" + nightguardian stop 2>/dev/null || true + echo "[$ts] guardian 프로세스 부재 감지 → 재시작" + nightguardian start 2>&1 + echo "[$ts] 재시작 결과 pid: $(pgrep -f guardian-watch.sh 2>/dev/null || echo NONE)" +} >> "$LOG"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/keepalive.sh` around lines 24 - 28, Replace the hardcoded tmux kill-session call in the keepalive restart flow with nightguardian stop, preserving the existing tolerant behavior when no session exists. Group the related timestamp, restart, and result log commands in a single { ... } >> "$LOG" block, while keeping nightguardian start output redirected appropriately and retaining the existing restart PID message.Source: Linters/SAST tools
src/guardian-watch.sh (1)
302-319: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value미사용 지역 변수
row.Shellcheck(SC2034)가 지적하듯 302-303행에서 선언된
row는 이후 어디서도 사용되지 않습니다. 나머지 스킵 로직(session/window_index/pane_index/pane_id 파싱,GUARDIAN_SKIP_SESSIONS글롭 매칭)은 정상입니다.🧹 제안
scan_all_panes() { - local row session window_index pane_index pane_id + local session window_index pane_index pane_id🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/guardian-watch.sh` around lines 302 - 319, Remove the unused local variable row from scan_all_panes, leaving session, window_index, pane_index, and pane_id unchanged so the existing pane parsing and skip logic remain intact.Source: Linters/SAST tools
tests/integration_tmux.sh (1)
45-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
force_due에 실행되지 않는 데드 코드가 남아있음.
reset.join(line.split("=", 1)[:1]) + "=" + reset if False else (...)구문에서if False앞의 표현식은 절대 평가되지 않습니다. 실질적으로는 else 분기만 항상 실행되므로, 디버깅 흔적으로 보이는 앞부분을 제거해 가독성을 높이는 것을 권장합니다.🧹 정리 제안
lines = path.read_text().splitlines() -path.write_text("\n".join(reset.join(line.split("=", 1)[:1]) + "=" + reset if False else (f"reset_epoch={reset}" if line.startswith("reset_epoch=") else line) for line in lines) + "\n") +path.write_text( + "\n".join( + f"reset_epoch={reset}" if line.startswith("reset_epoch=") else line + for line in lines + ) + + "\n" +)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration_tmux.sh` around lines 45 - 53, Remove the unreachable expression before “if False” in the Python transformation inside force_due, leaving only the logic that replaces lines starting with “reset_epoch=” while preserving all other lines unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@FAILURE_LOG.md`:
- Around line 7-10: FAILURE_LOG.md의 NG-001부터 NG-004까지 기록된 날짜를 실제 발생일인
2026-07-21로 수정하세요. 아직 발생하지 않은 검증 시나리오라면 해당 항목을 실패 기록에서 제거하고 별도 계획 문서로 분리하세요.
In `@README.md`:
- Around line 46-68: Reorder the README installation steps so the PATH export
for ~/.local/bin appears before the nightguardian start command, or invoke the
installed CLI via its absolute path. Ensure users do not attempt to run
nightguardian until the CLI directory is available in PATH.
In `@src/guardian-watch.sh`:
- Around line 268-285: Throttle the “HOLD: reset time missing” log in the
rate-limit handling flow around parse_reset_time and GUARDIAN_FALLBACK_MODE,
using the existing 300-second throttling pattern from the nearby “Waiting” log.
Keep the hold behavior unchanged while preventing the message from being emitted
on every scan.
- Around line 152-160: Update acquire_pane_lock to detect stale lock directories
by checking the existing lock_dir mtime and remove locks older than the
configured threshold (for example, 120 seconds) before retrying mkdir. Preserve
the current atomic mkdir acquisition behavior and ensure only stale locks are
reclaimed, allowing resume_pane to proceed after crashes.
- Around line 192-211: After the two Escape operations in the overlay-closing
flow, recapture the target pane content and recheck it against
GUARDIAN_DIALOG_RE and GUARDIAN_FEEDBACK_RE before sending the resume prompt. If
either pattern remains, abort safely using the existing cleanup and no-prompt
path; retain pane_runs_claude as an additional process-state check.
- Around line 350-358: Update cleanup_legacy_state so pane_*.state files are
removed based on their stored reset_epoch rather than the file mtime. Parse each
state file’s reset_epoch and delete it only when that reset time has expired,
preserving valid pending states such as weekly restrictions across daemon
restarts; retain the existing cleanup for legacy session files.
---
Nitpick comments:
In @.github/workflows/verify.yml:
- Line 16: Update the actions/checkout step to explicitly disable persisted
credentials by configuring persist-credentials to false, ensuring the GitHub
Actions token is not retained in the local .git directory.
In `@src/guardian-watch.sh`:
- Around line 302-319: Remove the unused local variable row from scan_all_panes,
leaving session, window_index, pane_index, and pane_id unchanged so the existing
pane parsing and skip logic remain intact.
In `@src/keepalive.sh`:
- Around line 24-28: Replace the hardcoded tmux kill-session call in the
keepalive restart flow with nightguardian stop, preserving the existing tolerant
behavior when no session exists. Group the related timestamp, restart, and
result log commands in a single { ... } >> "$LOG" block, while keeping
nightguardian start output redirected appropriately and retaining the existing
restart PID message.
In `@tests/integration_tmux.sh`:
- Around line 45-53: Remove the unreachable expression before “if False” in the
Python transformation inside force_due, leaving only the logic that replaces
lines starting with “reset_epoch=” while preserving all other lines unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 42fb74d0-cd89-47b8-9655-938831cbf684
📒 Files selected for processing (23)
.github/workflows/codeql.yml.github/workflows/verify.yml.gitignoreCHANGELOG.mdFAILURE_LOG.mdMakefileREADME.mdREQUIREMENTS.mdconfig/com.voidlight.nightguardian.plistconfig/sessions.json.templategates/docs_gate.shgates/lib/gate-lib.shgates/no_vertical_stripe_gate.shgates/quality_gate.shgates/safety_gate.shgates/secrets_gate.shgates/tests_gate.shgates/verify_nightguardian.shsrc/guardian-watch.shsrc/keepalive.shsrc/nightguardiansrc/parse_reset.pytests/integration_tmux.sh
💤 Files with no reviewable changes (1)
- .github/workflows/codeql.yml
Comment on lines
+7
to
+10
| | 2026-07-22 | NG-001 | Concurrent watchers sent duplicate resume input | No atomic claim around due pane state | Added pane-scoped `mkdir` lock and atomic state writes | `tests_gate.sh`, `safety_gate.sh` | | ||
| | 2026-07-22 | NG-002 | Installed `nightguardian verify` resolved the wrong root | CLI derived paths from the symlink location | Resolve the executable realpath before deriving project root | `tests_gate.sh` | | ||
| | 2026-07-22 | NG-003 | Claude-looking process arguments could pass validation | Full command lines were matched as unstructured text | Match executable paths/names from `ps comm` only | `tests_gate.sh`, `safety_gate.sh` | | ||
| | 2026-07-22 | NG-004 | An unrelated clock time could be parsed as reset time | Absolute-time parser searched the whole pane | Scope absolute times to reset/retry context | `tests_gate.sh` | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
실패 기록의 날짜를 수정하세요.
현재 날짜는 2026년 7월 21일인데 모든 항목이 2026년 7월 22일로 기록되어 있습니다. 실제 발생일을 사용하거나, 아직 발생하지 않은 검증 시나리오라면 별도 계획 문서로 분리해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@FAILURE_LOG.md` around lines 7 - 10, FAILURE_LOG.md의 NG-001부터 NG-004까지 기록된
날짜를 실제 발생일인 2026-07-21로 수정하세요. 아직 발생하지 않은 검증 시나리오라면 해당 항목을 실패 기록에서 제거하고 별도 계획
문서로 분리하세요.
Comment on lines
46
to
+68
| ```bash | ||
| # 1. 클론/다운로드 후 | ||
| git clone https://github.com/VoidLight00/nightguardian.git | ||
| cd nightguardian | ||
| make test | ||
| make install | ||
| nightguardian start | ||
| ``` | ||
|
|
||
| `make install` creates: | ||
|
|
||
| ```text | ||
| ~/.forgechain-nightguardian/ | ||
| ├── bin -> <checkout>/src | ||
| ├── config -> <checkout>/config | ||
| ├── manifest/ | ||
| └── logs/ | ||
| ``` | ||
|
|
||
| # 또는 수동 | ||
| mkdir -p ~/.forgechain-nightguardian | ||
| ln -s "$(pwd)/src" ~/.forgechain-nightguardian/bin | ||
| ln -s "$(pwd)/config" ~/.forgechain-nightguardian/config | ||
| mkdir -p ~/.forgechain-nightguardian/{manifest,logs} | ||
| Add the CLI directory to your shell if it is not already present: | ||
|
|
||
| # 2. PATH 추가 | ||
| export PATH="${HOME}/.local/bin:${PATH}" | ||
| ```bash | ||
| export PATH="$HOME/.local/bin:$PATH" | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
PATH 설정 전에 CLI를 실행하지 마세요.
새 셸에서는 ~/.local/bin이 PATH에 없을 수 있는데, 현재 문서는 nightguardian start를 먼저 실행한 뒤 PATH 설정을 안내합니다. PATH 블록을 nightguardian start보다 앞에 배치하거나 설치된 CLI의 절대 경로를 사용해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 46 - 68, Reorder the README installation steps so the
PATH export for ~/.local/bin appears before the nightguardian start command, or
invoke the installed CLI via its absolute path. Ensure users do not attempt to
run nightguardian until the CLI directory is available in PATH.
Comment on lines
+152
to
+160
| acquire_pane_lock() { | ||
| local pane_id="$1" lock_dir | ||
| lock_dir="${MANIFEST_DIR}/pane_$(safe_id "$pane_id").lock" | ||
| if mkdir "$lock_dir" 2>/dev/null; then | ||
| printf '%s\n' "$lock_dir" | ||
| return 0 | ||
| fi | ||
| return 1 | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
pane 락에 만료(staleness) 회수 로직이 없어, 크래시 시 해당 pane이 영구적으로 막힘.
mkdir 기반 락은 정상 종료 경로(173-233의 각 return 지점)에서만 rmdir로 해제됩니다. 데몬이 resume_pane 실행 중 SIGKILL 등으로 중단되면 락 디렉터리가 영구히 남고, 이후 모든 스캔에서 acquire_pane_lock이 항상 실패해 resume_pane은 176행에서 조용히 return 0합니다. 결과적으로 해당 pane은 로그 한 줄도 남기지 않고 영원히 자동 재개 대상에서 제외됩니다. "fail-closed"가 아니라 "silent永久 hold"가 되는 셈이라, 락 디렉터리의 mtime을 확인해 일정 시간(예: 120초) 이상 지난 락은 회수하는 처리를 권장합니다.
🔒 오래된 락 회수 제안
acquire_pane_lock() {
- local pane_id="$1" lock_dir
+ local pane_id="$1" lock_dir age lock_mtime
lock_dir="${MANIFEST_DIR}/pane_$(safe_id "$pane_id").lock"
if mkdir "$lock_dir" 2>/dev/null; then
printf '%s\n' "$lock_dir"
return 0
fi
+ lock_mtime=$(stat -c %Y "$lock_dir" 2>/dev/null || stat -f %m "$lock_dir" 2>/dev/null || printf '%s' "$(date +%s)")
+ age=$(( $(date +%s) - lock_mtime ))
+ if [ "$age" -gt 120 ] && rmdir "$lock_dir" 2>/dev/null && mkdir "$lock_dir" 2>/dev/null; then
+ printf '%s\n' "$lock_dir"
+ return 0
+ fi
return 1
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| acquire_pane_lock() { | |
| local pane_id="$1" lock_dir | |
| lock_dir="${MANIFEST_DIR}/pane_$(safe_id "$pane_id").lock" | |
| if mkdir "$lock_dir" 2>/dev/null; then | |
| printf '%s\n' "$lock_dir" | |
| return 0 | |
| fi | |
| return 1 | |
| } | |
| acquire_pane_lock() { | |
| local pane_id="$1" lock_dir age lock_mtime | |
| lock_dir="${MANIFEST_DIR}/pane_$(safe_id "$pane_id").lock" | |
| if mkdir "$lock_dir" 2>/dev/null; then | |
| printf '%s\n' "$lock_dir" | |
| return 0 | |
| fi | |
| lock_mtime=$(stat -c %Y "$lock_dir" 2>/dev/null || stat -f %m "$lock_dir" 2>/dev/null || printf '%s' "$(date +%s)") | |
| age=$(( $(date +%s) - lock_mtime )) | |
| if [ "$age" -gt 120 ] && rmdir "$lock_dir" 2>/dev/null && mkdir "$lock_dir" 2>/dev/null; then | |
| printf '%s\n' "$lock_dir" | |
| return 0 | |
| fi | |
| return 1 | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/guardian-watch.sh` around lines 152 - 160, Update acquire_pane_lock to
detect stale lock directories by checking the existing lock_dir mtime and remove
locks older than the configured threshold (for example, 120 seconds) before
retrying mkdir. Preserve the current atomic mkdir acquisition behavior and
ensure only stale locks are reclaimed, allowing resume_pane to proceed after
crashes.
Comment on lines
+192
to
211
| resume_prompt=$(get_resume_prompt "$session") | ||
| if printf '%s' "$pane_content" | grep -qiE "$GUARDIAN_DIALOG_RE|$GUARDIAN_FEEDBACK_RE"; then | ||
| log "[$session $pane_id] Closing Claude overlay before resume." | ||
| tmux_run send-keys -t "$pane_id" Escape 2>/dev/null || { | ||
| rmdir "$lock_dir" 2>/dev/null || true | ||
| return 1 | ||
| } | ||
| sleep 1 | ||
| tmux_run send-keys -t "$pane_id" Escape 2>/dev/null || { | ||
| rmdir "$lock_dir" 2>/dev/null || true | ||
| return 1 | ||
| } | ||
| sleep 1 | ||
| pane_runs_claude "$pane_id" || { | ||
| log "[$session $pane_id] BLOCKED: target changed after overlay close; no prompt sent." | ||
| rm -f "$state_file" | ||
| else | ||
| local remaining=$((reset_epoch + 60 - now_epoch)) | ||
| local min=$((remaining / 60)) | ||
| if [ "$((now_epoch % 300))" -lt "$CHECK_INTERVAL" ]; then | ||
| log "[$session] ⏳ Still waiting... ${min}m until reset" | ||
| fi | ||
| fi | ||
| rmdir "$lock_dir" 2>/dev/null || true | ||
| return 0 | ||
| } | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Escape 전송 후 오버레이가 실제로 닫혔는지 재검증하지 않음.
두 번의 Escape 전송 후에는 pane_runs_claude(프로세스 생존)만 재확인합니다. GUARDIAN_DIALOG_RE/GUARDIAN_FEEDBACK_RE에 대해 pane 콘텐츠를 다시 캡처해 오버레이가 실제로 사라졌는지는 검증하지 않습니다. Escape가 무시되거나 다른 형태의 오버레이(예: 다중 단계 확인창)라면, 다이얼로그가 여전히 떠 있는 상태로 resume 프롬프트 텍스트가 그대로 입력될 위험이 있습니다. 이는 이 PR의 핵심 목표("pane-safe" 자동 재개)와 직접 충돌하는 지점이라 recapture 후 재검사를 권장합니다.
🛡 재검증 제안
sleep 1
- pane_runs_claude "$pane_id" || {
+ pane_content=$(capture_pane "$pane_id")
+ if printf '%s' "$pane_content" | grep -qiE "$GUARDIAN_DIALOG_RE|$GUARDIAN_FEEDBACK_RE"; then
+ log "[$session $pane_id] BLOCKED: overlay still visible after Escape; no prompt sent."
+ rmdir "$lock_dir" 2>/dev/null || true
+ return 1
+ fi
+ pane_runs_claude "$pane_id" || {
log "[$session $pane_id] BLOCKED: target changed after overlay close; no prompt sent."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| resume_prompt=$(get_resume_prompt "$session") | |
| if printf '%s' "$pane_content" | grep -qiE "$GUARDIAN_DIALOG_RE|$GUARDIAN_FEEDBACK_RE"; then | |
| log "[$session $pane_id] Closing Claude overlay before resume." | |
| tmux_run send-keys -t "$pane_id" Escape 2>/dev/null || { | |
| rmdir "$lock_dir" 2>/dev/null || true | |
| return 1 | |
| } | |
| sleep 1 | |
| tmux_run send-keys -t "$pane_id" Escape 2>/dev/null || { | |
| rmdir "$lock_dir" 2>/dev/null || true | |
| return 1 | |
| } | |
| sleep 1 | |
| pane_runs_claude "$pane_id" || { | |
| log "[$session $pane_id] BLOCKED: target changed after overlay close; no prompt sent." | |
| rm -f "$state_file" | |
| else | |
| local remaining=$((reset_epoch + 60 - now_epoch)) | |
| local min=$((remaining / 60)) | |
| if [ "$((now_epoch % 300))" -lt "$CHECK_INTERVAL" ]; then | |
| log "[$session] ⏳ Still waiting... ${min}m until reset" | |
| fi | |
| fi | |
| rmdir "$lock_dir" 2>/dev/null || true | |
| return 0 | |
| } | |
| fi | |
| resume_prompt=$(get_resume_prompt "$session") | |
| if printf '%s' "$pane_content" | grep -qiE "$GUARDIAN_DIALOG_RE|$GUARDIAN_FEEDBACK_RE"; then | |
| log "[$session $pane_id] Closing Claude overlay before resume." | |
| tmux_run send-keys -t "$pane_id" Escape 2>/dev/null || { | |
| rmdir "$lock_dir" 2>/dev/null || true | |
| return 1 | |
| } | |
| sleep 1 | |
| tmux_run send-keys -t "$pane_id" Escape 2>/dev/null || { | |
| rmdir "$lock_dir" 2>/dev/null || true | |
| return 1 | |
| } | |
| sleep 1 | |
| pane_content=$(capture_pane "$pane_id") | |
| if printf '%s' "$pane_content" | grep -qiE "$GUARDIAN_DIALOG_RE|$GUARDIAN_FEEDBACK_RE"; then | |
| log "[$session $pane_id] BLOCKED: overlay still visible after Escape; no prompt sent." | |
| rmdir "$lock_dir" 2>/dev/null || true | |
| return 1 | |
| fi | |
| pane_runs_claude "$pane_id" || { | |
| log "[$session $pane_id] BLOCKED: target changed after overlay close; no prompt sent." | |
| rm -f "$state_file" | |
| rmdir "$lock_dir" 2>/dev/null || true | |
| return 0 | |
| } | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/guardian-watch.sh` around lines 192 - 211, After the two Escape
operations in the overlay-closing flow, recapture the target pane content and
recheck it against GUARDIAN_DIALOG_RE and GUARDIAN_FEEDBACK_RE before sending
the resume prompt. If either pattern remains, abort safely using the existing
cleanup and no-prompt path; retain pane_runs_claude as an additional
process-state check.
Comment on lines
268
to
+285
|
|
||
| while true; do | ||
| local session_list | ||
| session_list=$(tmux ls 2>/dev/null | cut -d: -f1 || true) | ||
| if ! pane_runs_claude "$pane_id"; then | ||
| log "[$session $pane_id] IGNORED: limit text found outside a verified Claude pane." | ||
| return 0 | ||
| fi | ||
|
|
||
| if [ -z "$session_list" ]; then | ||
| if [ "$(($(date +%s) % 60))" -lt "$CHECK_INTERVAL" ]; then | ||
| log "No active tmux sessions found." | ||
| fi | ||
| reset_epoch=$(parse_reset_time "$pane_content") | ||
| if [ -n "$reset_epoch" ] && [ "$reset_epoch" -gt 0 ] 2>/dev/null; then | ||
| atomic_write_state "$state_file" rate_limited "$reset_epoch" "$now_epoch" "$pane_id" "$session" "$window_index" "$pane_index" | ||
| log "[$session $pane_id] Rate limit detected; exact pane pinned until epoch $reset_epoch." | ||
| elif [ "$GUARDIAN_FALLBACK_MODE" = "resume" ]; then | ||
| reset_epoch=$((now_epoch + GUARDIAN_FALLBACK_WAIT)) | ||
| atomic_write_state "$state_file" rate_limited_fallback "$reset_epoch" "$now_epoch" "$pane_id" "$session" "$window_index" "$pane_index" | ||
| log "[$session $pane_id] Reset time missing; opt-in fallback scheduled in ${GUARDIAN_FALLBACK_WAIT}s." | ||
| else | ||
| for session in $session_list; do | ||
| # Skip the guardian's own session and control sessions. | ||
| # 추가 제외 패턴은 GUARDIAN_SKIP_SESSIONS(공백 구분 glob)로 지정. | ||
| case "$session" in | ||
| forge-night|claude-retry-*|main) continue ;; | ||
| esac | ||
| for _skip in ${GUARDIAN_SKIP_SESSIONS:-}; do | ||
| case "$session" in $_skip) continue 2 ;; esac | ||
| done | ||
| log "[$session $pane_id] HOLD: reset time missing; automatic fallback is disabled." | ||
| fi | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
HOLD 분기 로그가 스캔 주기마다 무제한 반복 기록됨.
reset 시각 파싱 실패 + GUARDIAN_FALLBACK_MODE=hold(기본값)인 경우 state/cooldown 파일이 전혀 기록되지 않습니다. limit 텍스트가 pane에 남아있는 동안 매 스캔(기본 CHECK_INTERVAL=30초)마다 283행의 "HOLD..." 로그가 스로틀 없이 계속 기록됩니다. 바로 위 260행의 "Waiting" 로그는 동일한 300초 스로틀 패턴을 이미 사용하므로, 같은 방식을 적용해 로그 파일 무한 증가를 막는 것을 권장합니다.
♻ 스로틀 적용 제안
else
- log "[$session $pane_id] HOLD: reset time missing; automatic fallback is disabled."
+ if [ "$((now_epoch % 300))" -lt "$CHECK_INTERVAL" ]; then
+ log "[$session $pane_id] HOLD: reset time missing; automatic fallback is disabled."
+ fi
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| while true; do | |
| local session_list | |
| session_list=$(tmux ls 2>/dev/null | cut -d: -f1 || true) | |
| if ! pane_runs_claude "$pane_id"; then | |
| log "[$session $pane_id] IGNORED: limit text found outside a verified Claude pane." | |
| return 0 | |
| fi | |
| if [ -z "$session_list" ]; then | |
| if [ "$(($(date +%s) % 60))" -lt "$CHECK_INTERVAL" ]; then | |
| log "No active tmux sessions found." | |
| fi | |
| reset_epoch=$(parse_reset_time "$pane_content") | |
| if [ -n "$reset_epoch" ] && [ "$reset_epoch" -gt 0 ] 2>/dev/null; then | |
| atomic_write_state "$state_file" rate_limited "$reset_epoch" "$now_epoch" "$pane_id" "$session" "$window_index" "$pane_index" | |
| log "[$session $pane_id] Rate limit detected; exact pane pinned until epoch $reset_epoch." | |
| elif [ "$GUARDIAN_FALLBACK_MODE" = "resume" ]; then | |
| reset_epoch=$((now_epoch + GUARDIAN_FALLBACK_WAIT)) | |
| atomic_write_state "$state_file" rate_limited_fallback "$reset_epoch" "$now_epoch" "$pane_id" "$session" "$window_index" "$pane_index" | |
| log "[$session $pane_id] Reset time missing; opt-in fallback scheduled in ${GUARDIAN_FALLBACK_WAIT}s." | |
| else | |
| for session in $session_list; do | |
| # Skip the guardian's own session and control sessions. | |
| # 추가 제외 패턴은 GUARDIAN_SKIP_SESSIONS(공백 구분 glob)로 지정. | |
| case "$session" in | |
| forge-night|claude-retry-*|main) continue ;; | |
| esac | |
| for _skip in ${GUARDIAN_SKIP_SESSIONS:-}; do | |
| case "$session" in $_skip) continue 2 ;; esac | |
| done | |
| log "[$session $pane_id] HOLD: reset time missing; automatic fallback is disabled." | |
| fi | |
| } | |
| if ! pane_runs_claude "$pane_id"; then | |
| log "[$session $pane_id] IGNORED: limit text found outside a verified Claude pane." | |
| return 0 | |
| fi | |
| reset_epoch=$(parse_reset_time "$pane_content") | |
| if [ -n "$reset_epoch" ] && [ "$reset_epoch" -gt 0 ] 2>/dev/null; then | |
| atomic_write_state "$state_file" rate_limited "$reset_epoch" "$now_epoch" "$pane_id" "$session" "$window_index" "$pane_index" | |
| log "[$session $pane_id] Rate limit detected; exact pane pinned until epoch $reset_epoch." | |
| elif [ "$GUARDIAN_FALLBACK_MODE" = "resume" ]; then | |
| reset_epoch=$((now_epoch + GUARDIAN_FALLBACK_WAIT)) | |
| atomic_write_state "$state_file" rate_limited_fallback "$reset_epoch" "$now_epoch" "$pane_id" "$session" "$window_index" "$pane_index" | |
| log "[$session $pane_id] Reset time missing; opt-in fallback scheduled in ${GUARDIAN_FALLBACK_WAIT}s." | |
| else | |
| if [ "$((now_epoch % 300))" -lt "$CHECK_INTERVAL" ]; then | |
| log "[$session $pane_id] HOLD: reset time missing; automatic fallback is disabled." | |
| fi | |
| fi | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/guardian-watch.sh` around lines 268 - 285, Throttle the “HOLD: reset time
missing” log in the rate-limit handling flow around parse_reset_time and
GUARDIAN_FALLBACK_MODE, using the existing 300-second throttling pattern from
the nearby “Waiting” log. Keep the hold behavior unchanged while preventing the
message from being emitted on every scan.
Comment on lines
+350
to
+358
| cleanup_legacy_state() { | ||
| local legacy | ||
| for legacy in "$MANIFEST_DIR"/session_*.state "$MANIFEST_DIR"/session_*.cooldown; do | ||
| [ -e "$legacy" ] || continue | ||
| log "Removing legacy session-scoped state: $(basename "$legacy")" | ||
| rm -f "$legacy" | ||
| done | ||
| find "$MANIFEST_DIR" -name 'pane_*.state' -mmin +720 -delete 2>/dev/null || true | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
cleanup_legacy_state가 mtime만으로 유효한 pane 상태를 삭제할 수 있음.
find "$MANIFEST_DIR" -name 'pane_*.state' -mmin +720 -delete는 파일 생성(=detected_epoch) 시점 기준 12시간이 지난 상태 파일을 무조건 삭제합니다. 그런데 GUARDIAN_DETECT_RE(25행)는 weekly 제한도 감지 대상이며, weekly 리셋은 detected_epoch로부터 12시간을 훌쩍 넘길 수 있습니다. 이 함수는 데몬 시작 시 1회 호출(371행)되므로, 라이브 데몬 재시작(테스트 플랜에 명시된 시나리오) 시 아직 유효한 weekly pending 상태가 삭제되고, pane 콘텐츠가 tail -80 밖으로 스크롤된 경우 재감지도 불가능해 자동 재개가 영구히 실패할 수 있습니다. mtime 대신 저장된 reset_epoch 값을 기준으로 만료 여부를 판정하는 것을 권장합니다.
🗑 reset_epoch 기반 정리 제안
cleanup_legacy_state() {
local legacy
for legacy in "$MANIFEST_DIR"/session_*.state "$MANIFEST_DIR"/session_*.cooldown; do
[ -e "$legacy" ] || continue
log "Removing legacy session-scoped state: $(basename "$legacy")"
rm -f "$legacy"
done
- find "$MANIFEST_DIR" -name 'pane_*.state' -mmin +720 -delete 2>/dev/null || true
+ local pane_state now_epoch reset_epoch
+ now_epoch=$(date +%s)
+ for pane_state in "$MANIFEST_DIR"/pane_*.state; do
+ [ -e "$pane_state" ] || continue
+ reset_epoch=$(state_value "$pane_state" reset_epoch)
+ if [ -n "$reset_epoch" ] && [ "$now_epoch" -lt "$((reset_epoch + 86400))" ] 2>/dev/null; then
+ continue
+ fi
+ log "Removing stale pane state: $(basename "$pane_state")"
+ rm -f "$pane_state"
+ done
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| cleanup_legacy_state() { | |
| local legacy | |
| for legacy in "$MANIFEST_DIR"/session_*.state "$MANIFEST_DIR"/session_*.cooldown; do | |
| [ -e "$legacy" ] || continue | |
| log "Removing legacy session-scoped state: $(basename "$legacy")" | |
| rm -f "$legacy" | |
| done | |
| find "$MANIFEST_DIR" -name 'pane_*.state' -mmin +720 -delete 2>/dev/null || true | |
| } | |
| cleanup_legacy_state() { | |
| local legacy | |
| for legacy in "$MANIFEST_DIR"/session_*.state "$MANIFEST_DIR"/session_*.cooldown; do | |
| [ -e "$legacy" ] || continue | |
| log "Removing legacy session-scoped state: $(basename "$legacy")" | |
| rm -f "$legacy" | |
| done | |
| local pane_state now_epoch reset_epoch | |
| now_epoch=$(date +%s) | |
| for pane_state in "$MANIFEST_DIR"/pane_*.state; do | |
| [ -e "$pane_state" ] || continue | |
| reset_epoch=$(state_value "$pane_state" reset_epoch) | |
| if [ -n "$reset_epoch" ] && [ "$now_epoch" -lt "$((reset_epoch + 86400))" ] 2>/dev/null; then | |
| continue | |
| fi | |
| log "Removing stale pane state: $(basename "$pane_state")" | |
| rm -f "$pane_state" | |
| done | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/guardian-watch.sh` around lines 350 - 358, Update cleanup_legacy_state so
pane_*.state files are removed based on their stored reset_epoch rather than the
file mtime. Parse each state file’s reset_epoch and delete it only when that
reset time has expired, preserving valid pending states such as weekly
restrictions across daemon restarts; retain the existing cleanup for legacy
session files.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
make testmake verifynightguardian verifyLocal operator note
The duplicate
claude-auto-retryshell wrapper was disabled locally while retaining the npm package for rollback. That machine-specific change is intentionally not part of this public repository.🤖 Generated with Claude Code
Summary by CodeRabbit
새로운 기능
verify검증 명령과 macOS 자동 시작 설정을 지원합니다.개선 사항
품질 및 문서