Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions skills/continuous-learning-v2/hooks/observe.sh
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ _SECRET_RE = re.compile(

import signal
def _ecc_bail(*_):
print("[observe] SIGALRM timeout: parse-error fallback observation dropped before write (#2300)", file=sys.stderr)
sys.exit(0)
try:
signal.signal(signal.SIGALRM, _ecc_bail)
Expand Down Expand Up @@ -317,6 +318,7 @@ import json, sys, os, re
import signal

def _ecc_bail(*_):
print("[observe] SIGALRM timeout: in-flight observation dropped before write (#2300)", file=sys.stderr)
sys.exit(0)
try:
signal.signal(signal.SIGALRM, _ecc_bail)
Expand Down
188 changes: 188 additions & 0 deletions tests/hooks/observe-signal-timeout.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
/**
* Tests for observe.sh SIGALRM timeout visibility (#2300).
*
* observe.sh arms a signal.SIGALRM alarm (8s) inside its inline-Python blocks so
* the observation writer self-terminates before the async hook's 10s timeout can
* orphan it (#2278). Before #2300 the handler `_ecc_bail` called sys.exit(0) with
* no logging, so a timeout silently dropped the in-flight observation: nothing was
* logged and the shell saw a clean exit. The fix adds a stderr visibility line to
* each handler while keeping exit 0 (changing to a non-zero exit would make the
* Claude hook report a block, per the repo's "always exit 0; log to stderr" rule).
*
* Two checks:
* 1. Static regression guard — every `_ecc_bail` handler in observe.sh writes to
* sys.stderr before sys.exit(0).
* 2. Behavioral check — the REAL handler text extracted from observe.sh, when its
* alarm fires, exits 0 and emits the `[observe]` visibility token on stderr
* (and never on stdout, which is the observations-file stream).
*/

if (process.platform === 'win32') {
console.log('Skipping bash/SIGALRM-dependent observe tests on Windows');
process.exit(0);
}

const assert = require('assert');
const fs = require('fs');
const path = require('path');
const { spawnSync } = require('child_process');

let passed = 0;
let failed = 0;

function test(name, fn) {
try {
fn();
console.log(`PASS: ${name}`);
passed += 1;
} catch (error) {
console.log(`FAIL: ${name}`);
console.error(` ${error.message}`);
failed += 1;
}
}

function findPython() {
const candidates = [
{ command: process.env.PYTHON, args: [] },
{ command: 'python3', args: [] },
{ command: 'python', args: [] },
{ command: 'py', args: ['-3'] },
].filter(candidate => candidate.command);

for (const candidate of candidates) {
const result = spawnSync(candidate.command, [...candidate.args, '--version'], {
encoding: 'utf8',
timeout: 5000,
});
if (result && result.status === 0) {
return candidate;
}
}
return null;
}

const repoRoot = path.resolve(__dirname, '..', '..');
const observeShPath = path.join(
repoRoot,
'skills',
'continuous-learning-v2',
'hooks',
'observe.sh'
);

const observeSrc = fs.readFileSync(observeShPath, 'utf8');

// Extract each `_ecc_bail` handler body: the `def` line plus the indented lines
// that follow it, up to (and including) the first dedented `sys.exit(0)` line at
// the same indentation as the def's body.
function extractHandlers(src) {
const lines = src.split('\n');
const handlers = [];
for (let i = 0; i < lines.length; i += 1) {
if (/^def _ecc_bail\(\*_\):\s*$/.test(lines[i])) {
const body = [lines[i]];
for (let j = i + 1; j < lines.length; j += 1) {
// Stop when we hit a line that is not indented (next top-level stmt).
if (lines[j].length > 0 && !/^\s/.test(lines[j])) {
break;
}
body.push(lines[j]);
if (/^\s+sys\.exit\(0\)\s*$/.test(lines[j])) {
break;
}
}
handlers.push(body.join('\n'));
}
}
return handlers;
}

const handlers = extractHandlers(observeSrc);

test('observe.sh defines at least two _ecc_bail handlers', () => {
assert.ok(
handlers.length >= 2,
`expected >= 2 _ecc_bail handlers, found ${handlers.length}`
);
});

test('every _ecc_bail handler logs to stderr before exiting (regression guard)', () => {
handlers.forEach((body, idx) => {
const stderrIdx = body.indexOf('file=sys.stderr');
const exitIdx = body.indexOf('sys.exit(0)');
assert.ok(
stderrIdx !== -1,
`handler #${idx + 1} does not write to sys.stderr (silent drop regression):\n${body}`
);
assert.ok(
exitIdx !== -1,
`handler #${idx + 1} is missing sys.exit(0):\n${body}`
);
assert.ok(
stderrIdx < exitIdx,
`handler #${idx + 1} must log to stderr BEFORE sys.exit(0):\n${body}`
);
assert.ok(
body.includes('[observe]'),
`handler #${idx + 1} stderr log should use the [observe] prefix:\n${body}`
);
});
});

test('_ecc_bail handlers keep exit code 0 (no exit 2 / block regression)', () => {
handlers.forEach((body, idx) => {
assert.ok(
/sys\.exit\(0\)/.test(body),
`handler #${idx + 1} must exit 0 to preserve the async-hook timeout contract (#2278):\n${body}`
);
assert.ok(
!/sys\.exit\([1-9]/.test(body),
`handler #${idx + 1} must not exit non-zero (would surface as a hook block):\n${body}`
);
});
});

test('real _ecc_bail handler: SIGALRM fire emits stderr token and exits 0', () => {
const python = findPython();
if (!python) {
console.log(' (skipped: no python interpreter available)');
return;
}

// Run the ACTUAL handler text extracted from observe.sh, forcing the alarm.
Comment thread
greptile-apps[bot] marked this conversation as resolved.
const handler = handlers[0];
const program = [
'import sys, signal, time',
handler,
'signal.signal(signal.SIGALRM, _ecc_bail)',
'signal.alarm(1)',
'time.sleep(3)',
'print("REACHED_END_SHOULD_NOT_HAPPEN")',
].join('\n');

const result = spawnSync(python.command, [...python.args, '-c', program], {
encoding: 'utf8',
timeout: 15000,
});

assert.strictEqual(result.signal, null, `python killed by signal ${result.signal}`);
assert.strictEqual(result.status, 0, `expected exit 0 on timeout, got ${result.status}`);
assert.ok(
/\[observe\] SIGALRM timeout/.test(result.stderr),
`expected the [observe] SIGALRM timeout warning on stderr, got: ${JSON.stringify(result.stderr)}`
);
assert.ok(
!/REACHED_END_SHOULD_NOT_HAPPEN/.test(result.stdout),
'handler should have terminated before the post-sleep stdout write'
);
assert.ok(
!/\[observe\] SIGALRM timeout/.test(result.stdout),
'the warning must go to stderr, never stdout (stdout is the observations stream)'
);
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

console.log(`\nPassed: ${passed}`);
console.log(`Failed: ${failed}`);

process.exit(failed > 0 ? 1 : 0);
Loading