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

* 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.
This commit is contained in:
Jun 2026-07-04 11:38:37 +08:00 committed by GitHub
parent c8c83ef428
commit 90f82d360b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 19 additions and 2 deletions

View file

@ -215,8 +215,12 @@ case "$ACTION" in
CLV2_OBSERVER_PROMPT_PATTERN="$CLV2_OBSERVER_PROMPT_PATTERN" \ CLV2_OBSERVER_PROMPT_PATTERN="$CLV2_OBSERVER_PROMPT_PATTERN" \
"$OBSERVER_LOOP_SCRIPT" >> "$LOG_FILE" 2>&1 & "$OBSERVER_LOOP_SCRIPT" >> "$LOG_FILE" 2>&1 &
# Wait for PID file # Wait for PID file (poll up to 10s, exits early when it appears).
sleep 2 # 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 # 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 if tail -n +"$((start_line + 1))" "$LOG_FILE" 2>/dev/null | grep -E -i -q "$CLV2_OBSERVER_PROMPT_PATTERN"; then

View file

@ -3066,6 +3066,19 @@ async function runTests() {
passed++; passed++;
else failed++; 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) { if (SKIP_BASH) {
console.log(' ⊘ detect-project exports the resolved Python command (skipped on Windows)'); console.log(' ⊘ detect-project exports the resolved Python command (skipped on Windows)');
passed++; passed++;