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 help to get the list of available commands and usage tips.}

Greptile Summary

This PR addresses the observer memory explosion from issue #521 by introducing three complementary throttling mechanisms: SIGUSR1 signal throttling in observe.sh (every 20 tool calls), a re-entrancy guard (ANALYZING flag) and cooldown timer in observer-loop.sh's on_usr1 handler, and tail-based sampling (last 500 lines) to cap the LLM payload size. The changes integrate cleanly with the existing session-guardian.sh and lock-file patterns already established in the skill.

Key issues found:

• Re-entrancy guard has a gap: The ANALYZING=1 flag is only set within on_usr1(). The main while loop calls analyze_observations directly (line 183) without setting the flag. If a SIGUSR1 arrives during a scheduled (non-signal-triggered) analysis — specifically during its wait "$claude_pid" — on_usr1 will see ANALYZING=0, pass the guard, and spawn a second concurrent Claude process. LAST_ANALYSIS_EPOCH is also not updated from this path, so the cooldown does not provide a backstop here either.

• Signal counter file is unprotected against concurrent writes: The read-increment-write on .observer-signal-counter in observe.sh is not atomic. Concurrent hook invocations can both read the same counter value, both evaluate >= SIGNAL_EVERY_N, and both send kill -USR1, sending two simultaneous signals — which was the root cause of the original explosion.

• Cooldown timer starts even on skipped analyses: LAST_ANALYSIS_EPOCH is updated after every analyze_observations call regardless of whether it did any work (e.g., not enough observations, claude not found), which can unnecessarily delay legitimate analyses.

• Tests are structural, not behavioral: All 17 tests validate the presence of code patterns via string matching rather than exercising the runtime signal-handling and concurrency semantics, so the residual race conditions above are not caught.

Confidence Score: 2/5

• The PR improves on the original explosion scenario but leaves a residual re-entrancy window in the scheduled-analysis path and an unguarded race condition on the counter file.
• The three mechanisms (throttle, guard, tail-sampling) are sound in concept and address the most common trigger path. However, the ANALYZING flag is never set in the main loop's direct call to analyze_observations, meaning a SIGUSR1 arriving during scheduled analysis can still spawn parallel Claude processes — the exact scenario that caused Memory Explosion in Continuous Learning v2 Observer #521. The unprotected counter file write adds a second concurrent path to the same outcome. Both are logic-level bugs rather than edge-case style concerns.
• Pay close attention to observer-loop.sh (main while-loop else branch, lines 180-184) and observe.sh (counter read-write block, lines 374-385).

Important Files Changed

Sequence Diagram

sequenceDiagram
    participant Hook as observe.sh (hook)
    participant Counter as .observer-signal-counter
    participant Observer as observer-loop.sh (bg)
    participant Claude as claude (analysis)

    Note over Hook,Counter: Every tool call
    Hook->>Counter: read counter
    Counter-->>Hook: N
    Hook->>Counter: write N+1
    alt N+1 >= SIGNAL_EVERY_N (20)
        Hook->>Counter: write 0 (reset)
        Hook->>Observer: kill -USR1
        Note over Observer: on_usr1() fires
        alt ANALYZING == 1
            Observer-->>Observer: skip (re-entrancy guard)
        else elapsed < ANALYSIS_COOLDOWN (60s)
            Observer-->>Observer: skip (cooldown)
        else
            Observer->>Observer: ANALYZING=1
            Observer->>Observer: tail last 500 lines → analysis_file
            Observer->>Claude: spawn claude --print < prompt_file
            Note over Claude: runs up to 120s / 10 turns
            Claude-->>Observer: exit
            Observer->>Observer: LAST_ANALYSIS_EPOCH = now
            Observer->>Observer: ANALYZING=0
            Observer->>Observer: archive observations.jsonl
        end
    end

    Note over Observer: Periodic (every 300s, main loop)
    Observer->>Observer: analyze_observations()
    Note over Observer: ⚠️ ANALYZING flag NOT set here
    Observer->>Claude: spawn claude
    alt SIGUSR1 fires during wait
        Hook->>Observer: kill -USR1
        Note over Observer: ANALYZING==0 → guard bypassed!
        Observer->>Claude: spawn 2nd concurrent claude
    end

_{Last reviewed commit: 3a26747}