feat: add project cooldown log to prevent observer re-spawn loops#412
feat: add project cooldown log to prevent observer re-spawn loops#412ispaydeu wants to merge 1 commit into
Conversation
Adds session-guardian.sh, called by observer-loop.sh before each Haiku spawn. It reads ~/.claude/observer-last-run.log and blocks the cycle if the same project was observed within OBSERVER_INTERVAL_SECONDS (default 300s). Prevents self-referential loops where a spawned session triggers observe.sh, which signals the observer before the cooldown has elapsed. Uses a mkdir-based lock for safe concurrent access across multiple simultaneously-observed projects. Log entries use tab-delimited format to handle paths containing spaces. Fails open on lock contention. Config: OBSERVER_INTERVAL_SECONDS default: 300 OBSERVER_LAST_RUN_LOG default: ~/.claude/observer-last-run.log No external dependencies. Works on macOS, Linux, Windows (Git Bash/MSYS2).
📝 WalkthroughWalkthroughThis change adds a session guard mechanism to the observer loop. A new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
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 |
There was a problem hiding this comment.
Pull request overview
Adds a “session guardian” gate to the Continuous Learning v2 observer loop to prevent spawning Claude analysis sessions too frequently for the same project by using a per-project cooldown log.
Changes:
- Introduces
session-guardian.shto enforce a per-project cooldown using a timestamp log and a filesystem lock. - Hooks
session-guardian.shintoobserver-loop.shso observer cycles can be skipped during cooldown.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| skills/continuous-learning-v2/agents/session-guardian.sh | New guard script that tracks last run per project root and enforces a cooldown before allowing analysis. |
| skills/continuous-learning-v2/agents/observer-loop.sh | Calls the guard before spawning claude so the observer can skip cycles during cooldown. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| # Key: git root path. Falls back to $PWD outside a git repo. | ||
| # stderr uses basename only — never prints the full absolute path. | ||
|
|
||
| project_root="$(git rev-parse --show-toplevel 2>/dev/null || echo "$PWD")" |
There was a problem hiding this comment.
git rev-parse is used unconditionally to derive project_root. If git isn’t installed/in PATH, bash typically emits a command not found diagnostic on stderr (not suppressed by 2>/dev/null), which can create noisy logs. Consider guarding with command -v git (and falling back to PWD) to keep the script quiet and predictable outside git environments.
| project_root="$(git rev-parse --show-toplevel 2>/dev/null || echo "$PWD")" | |
| if command -v git >/dev/null 2>&1; then | |
| project_root="$(git rev-parse --show-toplevel 2>/dev/null || echo "$PWD")" | |
| else | |
| project_root="$PWD" | |
| fi |
| last_spawn=0 | ||
| last_spawn=$(grep -F "$project_root" "$LOG_PATH" 2>/dev/null | tail -n1 | awk '{print $NF}') || true | ||
| last_spawn="${last_spawn:-0}" | ||
|
|
||
| elapsed=$(( now - last_spawn )) |
There was a problem hiding this comment.
The cooldown lookup uses grep -F "$project_root" and then pulls the last field via awk. This can mis-associate entries when one project root is a substring of another, and it will also break (arithmetic error under set -e) if the log ever contains a non-numeric timestamp. Prefer matching the first column exactly (e.g., compare field 1) and validate/coerce the timestamp before doing arithmetic.
| fi | ||
|
|
||
| # Update log atomically: remove old entry, append new timestamp | ||
| tmp_log="$(mktemp "${TMPDIR:-/tmp}/observer-last-run.XXXXXX")" |
There was a problem hiding this comment.
tmp_log is created under ${TMPDIR:-/tmp} and then mv’d to $LOG_PATH. If $LOG_PATH is on a different filesystem (common when /tmp is separate), mv is not atomic (copy+delete), which can leave a partially-written log if interrupted. Create the temp file in the same directory as $LOG_PATH (and then rename) to preserve atomicity.
| tmp_log="$(mktemp "${TMPDIR:-/tmp}/observer-last-run.XXXXXX")" | |
| tmp_log="$(mktemp "$(dirname "$LOG_PATH")/observer-last-run.XXXXXX")" |
| # OBSERVER_LAST_RUN_LOG default: ~/.claude/observer-last-run.log | ||
| # | ||
| set -euo pipefail | ||
|
|
There was a problem hiding this comment.
The script documents a narrow exit-code contract (0=proceed, 1=skip), but set -euo pipefail means unexpected conditions (e.g., unwritable log dir, mktemp failure) will also return non-zero. Since the caller treats any non-zero as “skip”, this can silently disable analysis. Consider catching internal errors and explicitly exit 0 (fail open), or use a distinct non-1 exit code and have the caller handle it separately.
| # Fail open on unexpected internal errors: log and proceed (exit 0). | |
| trap 'echo "session-guardian: internal error, proceeding (fail-open)" >&2; exit 0' ERR |
| if ! bash "$(dirname "$0")/session-guardian.sh"; then | ||
| echo "[$(date)] Observer cycle skipped by session-guardian" >> "$LOG_FILE" | ||
| return |
There was a problem hiding this comment.
This treats any non-zero exit from session-guardian.sh as a cooldown skip. If session-guardian.sh fails for an unexpected reason (permissions, mktemp, etc.), the observer will silently stop analyzing. Consider checking the exit code explicitly (e.g., 1 => skip, other non-zero => log an error but proceed/fail open) so operational failures don’t disable the observer.
| if ! bash "$(dirname "$0")/session-guardian.sh"; then | |
| echo "[$(date)] Observer cycle skipped by session-guardian" >> "$LOG_FILE" | |
| return | |
| bash "$(dirname "$0")/session-guardian.sh" | |
| guardian_status=$? | |
| if [ "$guardian_status" -ne 0 ]; then | |
| if [ "$guardian_status" -eq 1 ]; then | |
| echo "[$(date)] Observer cycle skipped by session-guardian" >> "$LOG_FILE" | |
| return | |
| else | |
| echo "[$(date)] session-guardian failed with exit code $guardian_status; proceeding with analysis (fail-open)" >> "$LOG_FILE" | |
| fi |
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="skills/continuous-learning-v2/agents/session-guardian.sh">
<violation number="1" location="skills/continuous-learning-v2/agents/session-guardian.sh:35">
P1: `grep -F "$project_root"` does a substring match, so a project like `/home/dev/app` will also match `/home/dev/app-backend`. This causes both an incorrect cooldown read here and data loss on line 47 (`grep -vF` deletes both entries). Append a literal tab to the pattern so it matches only the exact key in the tab-delimited log.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| trap 'rm -rf "$_lock_dir"' EXIT INT TERM | ||
|
|
||
| last_spawn=0 | ||
| last_spawn=$(grep -F "$project_root" "$LOG_PATH" 2>/dev/null | tail -n1 | awk '{print $NF}') || true |
There was a problem hiding this comment.
P1: grep -F "$project_root" does a substring match, so a project like /home/dev/app will also match /home/dev/app-backend. This causes both an incorrect cooldown read here and data loss on line 47 (grep -vF deletes both entries). Append a literal tab to the pattern so it matches only the exact key in the tab-delimited log.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/continuous-learning-v2/agents/session-guardian.sh, line 35:
<comment>`grep -F "$project_root"` does a substring match, so a project like `/home/dev/app` will also match `/home/dev/app-backend`. This causes both an incorrect cooldown read here and data loss on line 47 (`grep -vF` deletes both entries). Append a literal tab to the pattern so it matches only the exact key in the tab-delimited log.</comment>
<file context>
@@ -0,0 +1,56 @@
+ trap 'rm -rf "$_lock_dir"' EXIT INT TERM
+
+ last_spawn=0
+ last_spawn=$(grep -F "$project_root" "$LOG_PATH" 2>/dev/null | tail -n1 | awk '{print $NF}') || true
+ last_spawn="${last_spawn:-0}"
+
</file context>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/continuous-learning-v2/agents/observer-loop.sh`:
- Around line 36-40: The session-guardian cooldown is recorded too early in
observer-loop.sh (it runs before the claude availability check), so move the
call that records the cooldown (the bash "$(dirname "$0")/session-guardian.sh"
invocation) to occur immediately after the claude launch attempt (the line that
runs claude --model haiku ...) or split session-guardian.sh into two functions
(e.g., session_guardian_check and session_guardian_record) and call the "check"
before the claude availability test and the "record" only when you actually
spawn a Haiku session; ensure you update references to LOG_FILE and preserve the
existing skip log message ("Observer cycle skipped by session-guardian") so the
cooldown is only consumed when the spawn succeeds.
In `@skills/continuous-learning-v2/agents/session-guardian.sh`:
- Around line 28-31: The lock contention path currently logs and lets the loser
continue; change it so a failed mkdir on _lock_dir causes the script to skip
this cycle instead of proceeding. In the session-guardian.sh block that checks
"if ! mkdir \"$_lock_dir\" ...", replace the current "proceed" behavior with an
early exit/return that stops further work (e.g., log the contention and exit 0
or return from the function). Keep the successful-creation cleanup/trap logic
for removing _lock_dir intact so the winner still releases the lock.
- Around line 35-50: The current use of grep -F with project_root matches
prefixes and can steal or remove other projects' entries; replace the grep
invocations that compute last_spawn and that filter out old entries (the lines
using grep -F "$project_root" and grep -vF "$project_root" writing to tmp_log)
with exact-key matching, e.g. use awk (or grep -Fx) to select/remove only lines
whose first field exactly equals project_root; ensure the last_spawn extraction
uses the same exact-key logic (the variable last_spawn calculation) and the
tmp_log rewrite (the removal of the old entry before appending the new
"${project_root}\t${now}") also uses exact-key matching so only the intended
project's timestamp is read/removed.
- Around line 20-21: The cooldown key currently derives
project_root/project_name by running git and basename in session-guardian.sh
(variables project_root and project_name); change it to use the provided
environment variable PROJECT_DIR (set by start-observer.sh) as the canonical
project directory/key. Replace uses of project_root/project_name when building
the cooldown identifier with PROJECT_DIR (or basename "$PROJECT_DIR" if you only
need the name) so cooldowns are tied to the observer-provided PROJECT_DIR rather
than the current working dir or git root.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4e24536f-22c1-485d-b69f-1fd7e2635796
📒 Files selected for processing (2)
skills/continuous-learning-v2/agents/observer-loop.shskills/continuous-learning-v2/agents/session-guardian.sh
| # session-guardian: gate observer cycle (cooldown log — see session-guardian.sh) | ||
| if ! bash "$(dirname "$0")/session-guardian.sh"; then | ||
| echo "[$(date)] Observer cycle skipped by session-guardian" >> "$LOG_FILE" | ||
| return | ||
| fi |
There was a problem hiding this comment.
Record the cooldown at the actual spawn point.
This gate runs before the claude availability check on Lines 42-45, so a missing CLI still burns the cooldown window even though no Haiku session was launched. Move it as close as possible to the claude --model haiku ... line, or split session-guardian.sh into separate “check” and “record” phases.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/continuous-learning-v2/agents/observer-loop.sh` around lines 36 - 40,
The session-guardian cooldown is recorded too early in observer-loop.sh (it runs
before the claude availability check), so move the call that records the
cooldown (the bash "$(dirname "$0")/session-guardian.sh" invocation) to occur
immediately after the claude launch attempt (the line that runs claude --model
haiku ...) or split session-guardian.sh into two functions (e.g.,
session_guardian_check and session_guardian_record) and call the "check" before
the claude availability test and the "record" only when you actually spawn a
Haiku session; ensure you update references to LOG_FILE and preserve the
existing skip log message ("Observer cycle skipped by session-guardian") so the
cooldown is only consumed when the spawn succeeds.
| project_root="$(git rev-parse --show-toplevel 2>/dev/null || echo "$PWD")" | ||
| project_name="$(basename "$project_root")" |
There was a problem hiding this comment.
Use PROJECT_DIR as the cooldown key.
start-observer.sh already provides PROJECT_DIR, but this code keys the log from the process working directory / current Git root instead. Since the observer is launched without a cd, starting it from another directory can make unrelated projects share a cooldown or bypass it entirely.
Suggested fix
-project_root="$(git rev-parse --show-toplevel 2>/dev/null || echo "$PWD")"
+project_root="${PROJECT_DIR:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}"
project_name="$(basename "$project_root")"📝 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.
| project_root="$(git rev-parse --show-toplevel 2>/dev/null || echo "$PWD")" | |
| project_name="$(basename "$project_root")" | |
| project_root="${PROJECT_DIR:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}" | |
| project_name="$(basename "$project_root")" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/continuous-learning-v2/agents/session-guardian.sh` around lines 20 -
21, The cooldown key currently derives project_root/project_name by running git
and basename in session-guardian.sh (variables project_root and project_name);
change it to use the provided environment variable PROJECT_DIR (set by
start-observer.sh) as the canonical project directory/key. Replace uses of
project_root/project_name when building the cooldown identifier with PROJECT_DIR
(or basename "$PROJECT_DIR" if you only need the name) so cooldowns are tied to
the observer-provided PROJECT_DIR rather than the current working dir or git
root.
| if ! mkdir "$_lock_dir" 2>/dev/null; then | ||
| # Another observer holds the lock — fail open, let this cycle proceed | ||
| echo "session-guardian: log locked by concurrent process, proceeding" >&2 | ||
| else |
There was a problem hiding this comment.
Lock contention should skip the cycle, not proceed.
If two observer triggers arrive together, the loser of this lock currently returns success and spawns anyway. That bypasses the serialization the guard is supposed to enforce and reintroduces duplicate Haiku launches during bursts.
Suggested fix
if ! mkdir "$_lock_dir" 2>/dev/null; then
- # Another observer holds the lock — fail open, let this cycle proceed
- echo "session-guardian: log locked by concurrent process, proceeding" >&2
+ # Another observer is updating the cooldown log; don't bypass the gate.
+ echo "session-guardian: log locked by concurrent process, skipping cycle" >&2
+ exit 1
else📝 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.
| if ! mkdir "$_lock_dir" 2>/dev/null; then | |
| # Another observer holds the lock — fail open, let this cycle proceed | |
| echo "session-guardian: log locked by concurrent process, proceeding" >&2 | |
| else | |
| if ! mkdir "$_lock_dir" 2>/dev/null; then | |
| # Another observer is updating the cooldown log; don't bypass the gate. | |
| echo "session-guardian: log locked by concurrent process, skipping cycle" >&2 | |
| exit 1 | |
| else |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/continuous-learning-v2/agents/session-guardian.sh` around lines 28 -
31, The lock contention path currently logs and lets the loser continue; change
it so a failed mkdir on _lock_dir causes the script to skip this cycle instead
of proceeding. In the session-guardian.sh block that checks "if ! mkdir
\"$_lock_dir\" ...", replace the current "proceed" behavior with an early
exit/return that stops further work (e.g., log the contention and exit 0 or
return from the function). Keep the successful-creation cleanup/trap logic for
removing _lock_dir intact so the winner still releases the lock.
| last_spawn=$(grep -F "$project_root" "$LOG_PATH" 2>/dev/null | tail -n1 | awk '{print $NF}') || true | ||
| last_spawn="${last_spawn:-0}" | ||
|
|
||
| elapsed=$(( now - last_spawn )) | ||
| if [ "$elapsed" -lt "$INTERVAL" ]; then | ||
| rm -rf "$_lock_dir" | ||
| trap - EXIT INT TERM | ||
| echo "session-guardian: cooldown active for '${project_name}' (last spawn ${elapsed}s ago, interval ${INTERVAL}s)" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Update log atomically: remove old entry, append new timestamp | ||
| tmp_log="$(mktemp "${TMPDIR:-/tmp}/observer-last-run.XXXXXX")" | ||
| grep -vF "$project_root" "$LOG_PATH" > "$tmp_log" 2>/dev/null || true | ||
| echo "${project_root} ${now}" >> "$tmp_log" | ||
| mv "$tmp_log" "$LOG_PATH" |
There was a problem hiding this comment.
Match log entries by exact key, not substring.
grep -F "$project_root" and grep -vF "$project_root" also match path prefixes, so /work/foo will read/remove /work/foo-bar entries in the shared log. That makes one project inherit or delete another project's cooldown timestamp.
Suggested fix
- last_spawn=$(grep -F "$project_root" "$LOG_PATH" 2>/dev/null | tail -n1 | awk '{print $NF}') || true
+ last_spawn=$(awk -F '\t' -v key="$project_root" '$1 == key { ts = $2 } END { print ts }' "$LOG_PATH" 2>/dev/null) || true
last_spawn="${last_spawn:-0}"
…
- grep -vF "$project_root" "$LOG_PATH" > "$tmp_log" 2>/dev/null || true
+ awk -F '\t' -v OFS='\t' -v key="$project_root" '$1 != key' "$LOG_PATH" > "$tmp_log" 2>/dev/null || true
echo "${project_root} ${now}" >> "$tmp_log"📝 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.
| last_spawn=$(grep -F "$project_root" "$LOG_PATH" 2>/dev/null | tail -n1 | awk '{print $NF}') || true | |
| last_spawn="${last_spawn:-0}" | |
| elapsed=$(( now - last_spawn )) | |
| if [ "$elapsed" -lt "$INTERVAL" ]; then | |
| rm -rf "$_lock_dir" | |
| trap - EXIT INT TERM | |
| echo "session-guardian: cooldown active for '${project_name}' (last spawn ${elapsed}s ago, interval ${INTERVAL}s)" >&2 | |
| exit 1 | |
| fi | |
| # Update log atomically: remove old entry, append new timestamp | |
| tmp_log="$(mktemp "${TMPDIR:-/tmp}/observer-last-run.XXXXXX")" | |
| grep -vF "$project_root" "$LOG_PATH" > "$tmp_log" 2>/dev/null || true | |
| echo "${project_root} ${now}" >> "$tmp_log" | |
| mv "$tmp_log" "$LOG_PATH" | |
| last_spawn=$(awk -F '\t' -v key="$project_root" '$1 == key { ts = $2 } END { print ts }' "$LOG_PATH" 2>/dev/null) || true | |
| last_spawn="${last_spawn:-0}" | |
| elapsed=$(( now - last_spawn )) | |
| if [ "$elapsed" -lt "$INTERVAL" ]; then | |
| rm -rf "$_lock_dir" | |
| trap - EXIT INT TERM | |
| echo "session-guardian: cooldown active for '${project_name}' (last spawn ${elapsed}s ago, interval ${INTERVAL}s)" >&2 | |
| exit 1 | |
| fi | |
| # Update log atomically: remove old entry, append new timestamp | |
| tmp_log="$(mktemp "${TMPDIR:-/tmp}/observer-last-run.XXXXXX")" | |
| awk -F '\t' -v OFS='\t' -v key="$project_root" '$1 != key' "$LOG_PATH" > "$tmp_log" 2>/dev/null || true | |
| echo "${project_root} ${now}" >> "$tmp_log" | |
| mv "$tmp_log" "$LOG_PATH" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/continuous-learning-v2/agents/session-guardian.sh` around lines 35 -
50, The current use of grep -F with project_root matches prefixes and can steal
or remove other projects' entries; replace the grep invocations that compute
last_spawn and that filter out old entries (the lines using grep -F
"$project_root" and grep -vF "$project_root" writing to tmp_log) with exact-key
matching, e.g. use awk (or grep -Fx) to select/remove only lines whose first
field exactly equals project_root; ensure the last_spawn extraction uses the
same exact-key logic (the variable last_spawn calculation) and the tmp_log
rewrite (the removal of the old entry before appending the new
"${project_root}\t${now}") also uses exact-key matching so only the intended
project's timestamp is read/removed.
|
Superseded by #413, which carried the session-guardian lane forward with the active-hours, idle-detection, and cooldown fixes on top of the same observer path. Closing this one to keep the queue clean. |
Problem
When ECC's observer loop spawns a Haiku session, that session can trigger
`observe.sh` hooks which signal the observer — causing a new analysis
cycle before the previous one has finished. Under some conditions this
creates a rapid re-spawn loop that consumes available licensed usage.
Solution
Adds `session-guardian.sh`, a lightweight gatekeeper called by
`observer-loop.sh` before each Haiku spawn. It maintains a per-project
log of last spawn times and blocks new spawns if the cooldown window
hasn't elapsed.
New file: `skills/continuous-learning-v2/agents/session-guardian.sh`
How it works
Debug output to stderr uses only the project basename — no full paths.
Fails open on lock contention (concurrent process holds the lock).
Platform support
No external dependencies. Uses only `date +%s`, `git`, `awk`, `grep`,
`mkdir` — all pre-installed on macOS, Linux, and Windows (Git Bash/MSYS2).
Testing
```bash
First call passes (exit 0)
OBSERVER_LAST_RUN_LOG=/tmp/test.log
./skills/continuous-learning-v2/agents/session-guardian.sh; echo $?
Immediate second call blocked (exit 1)
./skills/continuous-learning-v2/agents/session-guardian.sh; echo $?
Expired cooldown passes (exit 0)
project=$(git rev-parse --show-toplevel)
printf '%s\t%s\n' "$project" "$(( $(date +%s) - 400 ))" > /tmp/test.log
OBSERVER_LAST_RUN_LOG=/tmp/test.log
./skills/continuous-learning-v2/agents/session-guardian.sh; echo $?
```
Related
Summary by cubic
Add a per-project cooldown to the observer to stop rapid re-spawn loops triggered by
observe.shhooks. A newsession-guardian.shgate blocks cycles if the same project ran within the cooldown window to reduce wasted sessions.skills/continuous-learning-v2/agents/session-guardian.sh; called byobserver-loop.shbefore each spawn and logs skipped cycles.~/.claude/observer-last-run.log(tab-delimited); default cooldown300sviaOBSERVER_INTERVAL_SECONDS.git rev-parse --show-toplevel(fallback toPWD); exits 1 to skip, 0 to proceed; debug uses project basename only.mkdirlock; fails open on lock contention. No external deps; works on macOS, Linux, and Windows (Git Bash/MSYS2).Written for commit 332b8b4. Summary will update on new commits.
Summary by CodeRabbit
Release Notes