From 90f82d360b5a483926998a71ec10d87a3e5e59a0 Mon Sep 17 00:00:00 2001
From: Jun <39075334+mc856@users.noreply.github.com>
Date: Sat, 4 Jul 2026 11:38:37 +0800
Subject: [PATCH] fix(observer): replace hardcoded sleep 2 with PID file poll
 in start-observer.sh (#2356)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix(observer): replace hardcoded sleep 2 with PID file poll in start-observer.sh

Fixes #2295

The previous `sleep 2` after launching the observer loop has two
problems: on slow filesystems or loaded systems 2 seconds may not be
enough, producing a false-negative on the subsequent PID file check; on
healthy systems it adds unnecessary latency.

Replace with a poll loop that exits as soon as the PID file appears:

    for _i in $(seq 1 50); do [ -f "$PID_FILE" ] && break; sleep 0.2; done

50 × 0.2s = 10s max wait (vs the previous fixed 2s), but typical startup
returns within the first iteration. No behavior change in the success
path — only the wait strategy changes.

Tests: `node tests/run-all.js` 2891 passed / 0 failed; `npm run lint`,
`catalog:check`, `command-registry:check` all clean.

* test(observer): add regression guard for sleep-2 → PID-file poll (#2295)

Asserts start-observer.sh never reverts to the fixed `sleep 2` wait and
keeps the 50 × 0.2s `$PID_FILE` poll in place. Sits next to the existing
observer-loop invariant block in tests/hooks/hooks.test.js, matching the
repo's pattern of guarding shell-script invariants via source-content
assertions.

Without this, any future "cleanup" that reintroduces a fixed sleep would
silently regress the slow-filesystem fix from the previous commit.

* fix(observer): loosen poll-regression assertions and document failure-path latency (#2356 review)

Addresses CodeRabbit + Greptile feedback on PR #2356:

- tests/hooks/hooks.test.js: split the over-specific positive assertion
  (which pinned the exact `for _i in $(seq 1 50); … sleep 0.2; done`
  line) into three intent-based assertions — bounded iteration count,
  early-exit on $PID_FILE, sub-second interval. Valid refactors (rename
  loop var, switch to `while`, retune to 100 × 0.1s) no longer false-fail
  while the `sleep 2` regression remains guarded.
- start-observer.sh: extend the inline comment to record the trade-off
  Greptile flagged — a loop that crashes before writing $PID_FILE is now
  detected in ~10s instead of ~2s. Healthy startups still return in
  iteration 1.

Tests: node tests/run-all.js (Node v22.18.0) → 2892 passed / 0 failed.
---
 .../continuous-learning-v2/agents/start-observer.sh |  8 ++++++--
 tests/hooks/hooks.test.js                           | 13 +++++++++++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/skills/continuous-learning-v2/agents/start-observer.sh b/skills/continuous-learning-v2/agents/start-observer.sh
index c3ada314..096a5d7b 100755
--- a/skills/continuous-learning-v2/agents/start-observer.sh
+++ b/skills/continuous-learning-v2/agents/start-observer.sh
@@ -215,8 +215,12 @@ case "$ACTION" in
       CLV2_OBSERVER_PROMPT_PATTERN="$CLV2_OBSERVER_PROMPT_PATTERN" \
       "$OBSERVER_LOOP_SCRIPT" >> "$LOG_FILE" 2>&1 &
 
-    # Wait for PID file
-    sleep 2
+    # Wait for PID file (poll up to 10s, exits early when it appears).
+    # Trade-off vs the old `sleep 2`: healthy startups return in iteration 1
+    # (no fixed latency), but a loop that crashes before writing the PID file
+    # is now detected in ~10s instead of ~2s. The longer ceiling is needed to
+    # tolerate slow filesystems where 2s under-waited and false-negatived.
+    for _i in $(seq 1 50); do [ -f "$PID_FILE" ] && break; sleep 0.2; done
 
     # Check for confirmation-seeking output in the observer log
     if tail -n +"$((start_line + 1))" "$LOG_FILE" 2>/dev/null | grep -E -i -q "$CLV2_OBSERVER_PROMPT_PATTERN"; then
diff --git a/tests/hooks/hooks.test.js b/tests/hooks/hooks.test.js
index f02bf4d1..8ccde106 100644
--- a/tests/hooks/hooks.test.js
+++ b/tests/hooks/hooks.test.js
@@ -3066,6 +3066,19 @@ async function runTests() {
     passed++;
   else failed++;
 
+  if (
+    test('start-observer waits for PID file via poll instead of fixed sleep (#2295)', () => {
+      const startObserverSource = fs.readFileSync(path.join(__dirname, '..', '..', 'skills', 'continuous-learning-v2', 'agents', 'start-observer.sh'), 'utf8');
+
+      assert.ok(!/^\s*sleep 2\s*$/m.test(startObserverSource), 'start-observer.sh should not use the fixed `sleep 2` wait after spawning the observer loop');
+      assert.ok(/\bseq 1 \d+\b/.test(startObserverSource), 'start-observer.sh should bound PID-file polling to a finite iteration count');
+      assert.ok(/\[ -f "\$PID_FILE" \] && break/.test(startObserverSource), 'start-observer.sh should exit polling as soon as $PID_FILE appears');
+      assert.ok(/sleep 0\.\d+/.test(startObserverSource), 'start-observer.sh should poll at sub-second intervals so healthy startups do not pay multi-second latency');
+    })
+  )
+    passed++;
+  else failed++;
+
   if (SKIP_BASH) {
     console.log('  ⊘ detect-project exports the resolved Python command (skipped on Windows)');
     passed++;