fix: require observer analysis completion sentinel

skills/continuous-learning-v2/agents/observer-loop.sh

@@ -9,6 +9,12 @@ set +e
 unset CLAUDECODE
 
 SLEEP_PID=""
+CLAUDE_PID=""
+CLAUDE_PROCESS_GROUP=0
+WATCHDOG_PID=""
+ACTIVE_ANALYSIS_FILE=""
+ACTIVE_RESULT_FILE=""
+RESULT_FDS_OPEN=0
 USR1_FIRED=0
 PENDING_ANALYSIS=0
 ANALYZING=0
@@ -25,7 +31,81 @@ ACTIVITY_FILE="${PROJECT_DIR}/.observer-last-activity"
 # ${BASH_SOURCE[0]}, which always points at this file (#2370).
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 
+claude_process_alive() {
+  local process_pid="$1"
+
+  if [ -z "$process_pid" ]; then
+    return 1
+  fi
+
+  if [ "$CLAUDE_PROCESS_GROUP" -eq 1 ]; then
+    kill -0 -- "-$process_pid" 2>/dev/null
+  else
+    kill -0 "$process_pid" 2>/dev/null
+  fi
+}
+
+signal_claude_process() {
+  local process_pid="$1"
+  local signal_name="$2"
+
+  if [ "$CLAUDE_PROCESS_GROUP" -eq 1 ]; then
+    kill -"$signal_name" -- "-$process_pid" 2>/dev/null || true
+  else
+    kill -"$signal_name" "$process_pid" 2>/dev/null || true
+  fi
+}
+
+stop_claude_process() {
+  local process_pid="$1"
+  local attempts=0
+
+  if [ -z "$process_pid" ]; then
+    return
+  fi
+
+  if claude_process_alive "$process_pid"; then
+    signal_claude_process "$process_pid" TERM
+    while claude_process_alive "$process_pid" && [ "$attempts" -lt 20 ]; do
+      sleep 0.1
+      attempts=$((attempts + 1))
+    done
+    if claude_process_alive "$process_pid"; then
+      signal_claude_process "$process_pid" KILL
+    fi
+  fi
+  wait "$process_pid" 2>/dev/null || true
+  CLAUDE_PROCESS_GROUP=0
+}
+
+cleanup_analysis_resources() {
+  if [ -n "$WATCHDOG_PID" ]; then
+    kill "$WATCHDOG_PID" 2>/dev/null || true
+    wait "$WATCHDOG_PID" 2>/dev/null || true
+    WATCHDOG_PID=""
+  fi
+  if [ -n "$CLAUDE_PID" ]; then
+    stop_claude_process "$CLAUDE_PID"
+    CLAUDE_PID=""
+  fi
+
+  if [ "$RESULT_FDS_OPEN" -eq 1 ]; then
+    { exec 8>&-; } 2>/dev/null || true
+    if [ -n "${LOG_FILE:-}" ]; then
+      cat <&9 >> "$LOG_FILE" 2>/dev/null || true
+    fi
+    { exec 7<&-; } 2>/dev/null || true
+    { exec 9<&-; } 2>/dev/null || true
+    RESULT_FDS_OPEN=0
+  fi
+  [ -n "$ACTIVE_ANALYSIS_FILE" ] && rm -f "$ACTIVE_ANALYSIS_FILE"
+  [ -n "$ACTIVE_RESULT_FILE" ] && rm -f "$ACTIVE_RESULT_FILE"
+  ACTIVE_ANALYSIS_FILE=""
+  ACTIVE_RESULT_FILE=""
+}
+
 cleanup() {
+  cleanup_analysis_resources
   [ -n "$SLEEP_PID" ] && kill "$SLEEP_PID" 2>/dev/null
   if [ -f "$PID_FILE" ] && [ "$(cat "$PID_FILE" 2>/dev/null)" = "$$" ]; then
     rm -f "$PID_FILE"
@@ -149,7 +229,17 @@ analyze_observations() {
   # substitutes a trailing X run, so a suffix after it (e.g. `.jsonl`) produces a
   # literal, non-random name that wedges every later cycle with "File exists" (#2417).
   analysis_file="$(mktemp "${observer_tmp_dir}/ecc-observer-analysis.jsonl.XXXXXX")"
-  tail -n "$MAX_ANALYSIS_LINES" "$OBSERVATIONS_FILE" > "$analysis_file"
+  if [ -z "$analysis_file" ] || [ ! -f "$analysis_file" ]; then
+    echo "[$(date)] Failed to create observer analysis file; retaining observations for retry" >> "$LOG_FILE"
+    return
+  fi
+  ACTIVE_ANALYSIS_FILE="$analysis_file"
+
+  if ! tail -n "$MAX_ANALYSIS_LINES" "$OBSERVATIONS_FILE" > "$analysis_file"; then
+    echo "[$(date)] Failed to snapshot observations; retaining them for retry" >> "$LOG_FILE"
+    cleanup_analysis_resources
+    return
+  fi
   analysis_count=$(wc -l < "$analysis_file" 2>/dev/null || echo 0)
   echo "[$(date)] Using last $analysis_count of $obs_count observations for analysis" >> "$LOG_FILE"
 
@@ -206,6 +296,12 @@ Rules:
 - If a pattern seems universal (not project-specific), set scope to global instead of project
 - Examples of global patterns: always validate user input, prefer explicit error handling
 - Examples of project patterns: use React functional components, follow Django REST framework conventions
+
+Completion contract:
+- After successfully reading and analyzing the sampled observations, and after completing any required instinct writes, output this exact JSON record as the final non-empty line:
+{"status":"analysis_complete"}
+- Do not output that record if reading, analysis, or a required write is blocked or fails
+- A completed analysis with no qualifying pattern must still output the record
 PROMPT
 
   # Read the prompt into memory before the Claude subprocess is spawned.
@@ -216,7 +312,7 @@ PROMPT
   rm -f "$prompt_file"
   if [ -z "$prompt_content" ]; then
     echo "[$(date)] Failed to load observer prompt content, skipping analysis" >> "$LOG_FILE"
-    rm -f "$analysis_file"
+    cleanup_analysis_resources
     return
   fi
 
@@ -249,7 +345,30 @@ PROMPT
 
   # Ensure CWD is PROJECT_DIR so the relative analysis_relpath resolves correctly
   # on all platforms, not just when the observer happens to be launched from the project root.
-  cd "$PROJECT_DIR" || { echo "[$(date)] Failed to cd to PROJECT_DIR ($PROJECT_DIR), skipping analysis" >> "$LOG_FILE"; rm -f "$analysis_file"; return; }
+  cd "$PROJECT_DIR" || { echo "[$(date)] Failed to cd to PROJECT_DIR ($PROJECT_DIR), skipping analysis" >> "$LOG_FILE"; cleanup_analysis_resources; return; }
+
+  analysis_result_file="$(mktemp "${observer_tmp_dir}/ecc-observer-result.XXXXXX")"
+  if [ -z "$analysis_result_file" ] || [ ! -f "$analysis_result_file" ]; then
+    echo "[$(date)] Failed to create observer result file, skipping analysis" >> "$LOG_FILE"
+    cleanup_analysis_resources
+    return
+  fi
+  ACTIVE_RESULT_FILE="$analysis_result_file"
+
+  # Keep validation bound to the inode created by mktemp. Removing the path
+  # after opening both descriptors prevents a workspace process from replacing
+  # it with a forged completion record while Claude is running.
+  RESULT_FDS_OPEN=1
+  if ! { exec 7<"$analysis_result_file" && exec 9<"$analysis_result_file" && exec 8>"$analysis_result_file"; }; then
+    echo "[$(date)] Failed to open observer result descriptors, skipping analysis" >> "$LOG_FILE"
+    cleanup_analysis_resources
+    return
+  fi
+  if ! rm -f "$analysis_result_file" || [ -e "$analysis_result_file" ] || [ -L "$analysis_result_file" ]; then
+    echo "[$(date)] Failed to unlink observer result file, skipping analysis" >> "$LOG_FILE"
+    cleanup_analysis_resources
+    return
+  fi
 
   # Prevent observe.sh from recording this automated observer session as observations.
   # Pass prompt via -p flag instead of stdin redirect for Windows compatibility (#842).
@@ -262,34 +381,77 @@ PROMPT
   # e.g. ECC_OBSERVER_MODEL=opus for higher-quality instinct extraction. Heavier models are
   # slower — consider raising ECC_OBSERVER_TIMEOUT_SECONDS (default 120s) so the watchdog
   # doesn't kill the analysis mid-run.
+  # Job control gives the background Claude command its own process group on
+  # Bash, including macOS's Bash 3.2 and Git Bash. That lets timeout/signal
+  # cleanup terminate tool subprocesses as well as the direct CLI process.
+  set -m
   ECC_SKIP_OBSERVE=1 ECC_HOOK_PROFILE=minimal claude --model "${ECC_OBSERVER_MODEL:-haiku}" --max-turns "$max_turns" --print \
     --allowedTools "Read,Write" \
-    -p "$prompt_content" < /dev/null >> "$LOG_FILE" 2>&1 &
-  claude_pid=$!
+    -p "$prompt_content" < /dev/null >&8 2>> "$LOG_FILE" &
+  CLAUDE_PID=$!
+  CLAUDE_PROCESS_GROUP=1
+  set +m
 
   (
     sleep "$timeout_seconds"
-    if kill -0 "$claude_pid" 2>/dev/null; then
+    if claude_process_alive "$CLAUDE_PID"; then
       echo "[$(date)] Claude analysis timed out after ${timeout_seconds}s; terminating process" >> "$LOG_FILE"
-      kill "$claude_pid" 2>/dev/null || true
+      signal_claude_process "$CLAUDE_PID" TERM
+      grace_attempts=0
+      while claude_process_alive "$CLAUDE_PID" && [ "$grace_attempts" -lt 20 ]; do
+        sleep 0.1
+        grace_attempts=$((grace_attempts + 1))
+      done
+      if claude_process_alive "$CLAUDE_PID"; then
+        echo "[$(date)] Claude analysis ignored TERM; killing process" >> "$LOG_FILE"
+        signal_claude_process "$CLAUDE_PID" KILL
+      fi
     fi
-  ) &
-  watchdog_pid=$!
+  ) </dev/null >/dev/null 2>&1 &
+  WATCHDOG_PID=$!
 
-  wait_for_claude_analysis "$claude_pid"
+  wait_for_claude_analysis "$CLAUDE_PID"
   exit_code=$?
-  kill "$watchdog_pid" 2>/dev/null || true
+  completed_claude_pid="$CLAUDE_PID"
+  CLAUDE_PID=""
+  kill "$WATCHDOG_PID" 2>/dev/null || true
+  wait "$WATCHDOG_PID" 2>/dev/null || true
+  WATCHDOG_PID=""
+  # A successful CLI can still leave tool subprocesses behind. Terminate any
+  # remaining members before closing the inherited result descriptors.
+  if claude_process_alive "$completed_claude_pid"; then
+    stop_claude_process "$completed_claude_pid"
+  else
+    CLAUDE_PROCESS_GROUP=0
+  fi
+  { exec 8>&-; } 2>/dev/null || true
+
+  analysis_complete=0
+  if awk '{ sub(/\r$/, "", $0); if ($0 == "{\"status\":\"analysis_complete\"}") count++; if (NF) last = $0 } END { exit !(count == 1 && last == "{\"status\":\"analysis_complete\"}") }' <&7; then
+    analysis_complete=1
+  fi
+  cat <&9 >> "$LOG_FILE" 2>/dev/null || true
+  { exec 7<&-; } 2>/dev/null || true
+  { exec 9<&-; } 2>/dev/null || true
+  RESULT_FDS_OPEN=0
+  rm -f "$analysis_result_file"
   rm -f "$analysis_file"
+  ACTIVE_RESULT_FILE=""
+  ACTIVE_ANALYSIS_FILE=""
 
   if [ "$exit_code" -ne 0 ]; then
     echo "[$(date)] Claude analysis failed (exit $exit_code); retaining observations for retry" >> "$LOG_FILE"
     return
   fi
 
-  # Archive observations only after a successful analysis. A transient
-  # failure (timeout, non-zero exit, rate limit) must not discard the batch
-  # before it has been turned into instincts, since the analyzer only ever
-  # reads the live observations file (#2370).
+  if [ "$analysis_complete" -ne 1 ]; then
+    echo "[$(date)] Claude analysis incomplete (completion record missing); retaining observations for retry" >> "$LOG_FILE"
+    return
+  fi
+
+  # Archive observations only after process success and the current analysis
+  # result's exact completion record. A semantic failure can still exit zero,
+  # so exit status alone must not discard the only live copy (#2370, #2673).
   if [ -f "$OBSERVATIONS_FILE" ]; then
     archive_dir="${PROJECT_DIR}/observations.archive"
     mkdir -p "$archive_dir"

tests/hooks/observer-loop-archive.test.js

@@ -1,19 +1,10 @@
 /**
- * Tests for observer-loop archive-on-failure fix (#2370)
+ * Tests for observer-loop archive-on-failure fixes (#2370, #2673).
  *
- * Bug: analyze_observations() in observer-loop.sh moved the live
- * observations.jsonl into observations.archive/ unconditionally, even when
- * the Claude analysis step failed (timeout, non-zero exit, rate limit).
- * Because the analyzer only ever reads the live file, a failed batch could
- * never be re-analyzed and its instincts were silently lost.
- *
- * Fix: archive only after a successful analysis; on failure log and return,
- * retaining observations for the next cycle to retry.
- *
- * Strategy: source observer-loop.sh (a BASH_SOURCE guard stops the main
- * loop from running when sourced) and drive analyze_observations directly
- * with a stub `claude` (exit code controlled per case) and a stub sibling
- * session-guardian.sh. Assert symmetric outcomes for failure vs success.
+ * A batch may be archived only when the Claude process exits successfully and
+ * its current stdout contains one exact completion record as the final
+ * non-empty line. Process failures, semantic failures, stderr/log markers,
+ * duplicate markers, and tampering with the result path must fail closed.
  *
  * Run with: node tests/hooks/observer-loop-archive.test.js
  */
@@ -26,6 +17,8 @@ const { spawnSync } = require('child_process');
 
 let passed = 0;
 let failed = 0;
+let skipped = 0;
+const SKIP = Symbol('skip');
 
 function test(name, fn) {
   try {
@@ -33,12 +26,23 @@ function test(name, fn) {
     console.log(`  ✓ ${name}`);
     passed++;
   } catch (err) {
+    if (err === SKIP) {
+      console.log(`  - ${name} (skipped: requires bash fixture)`);
+      skipped++;
+      return;
+    }
     console.log(`  ✗ ${name}`);
     console.log(`    Error: ${err.message}`);
     failed++;
   }
 }
 
+function skipOnWindows() {
+  if (process.platform === 'win32') {
+    throw SKIP;
+  }
+}
+
 function createTempDir() {
   return fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-observer-archive-'));
 }
@@ -47,7 +51,17 @@ function cleanupDir(dir) {
   try {
     fs.rmSync(dir, { recursive: true, force: true });
   } catch {
-    // ignore cleanup errors
+    // Ignore cleanup errors in an already-isolated test directory.
+  }
+}
+
+function processExists(pid) {
+  if (!Number.isInteger(pid) || pid <= 0) return false;
+  try {
+    process.kill(pid, 0);
+    return true;
+  } catch {
+    return false;
   }
 }
 
@@ -55,148 +69,310 @@ const repoRoot = path.resolve(__dirname, '..', '..');
 const observerLoopPath = path.join(
   repoRoot, 'skills', 'continuous-learning-v2', 'agents', 'observer-loop.sh'
 );
+const ANALYSIS_COMPLETE_RECORD = '{"status":"analysis_complete"}';
+const ORIGINAL_OBSERVATIONS = '{"a":1}\n{"a":2}\n{"a":3}\n';
 
 /**
- * Run analyze_observations once with the given stub claude exit code.
- * Returns { liveExists, archivedCount, log } describing the resulting state.
+ * Source observer-loop.sh in a sandbox and invoke analyze_observations once.
  */
-function runAnalyzeOnce(claudeExitCode) {
+function runAnalyzeOnce(options = {}) {
+  const {
+    claudeExitCode = 0,
+    claudeOutput = '',
+    claudeStderr = '',
+    claudeDelaySeconds = 0,
+    claudeIgnoreTerm = false,
+    claudeSpawnChild = false,
+    existingLog = '',
+    observerTimeoutSeconds = 10,
+    probeResultPath = false,
+  } = options;
   const sandbox = createTempDir();
+
   try {
     const binDir = path.join(sandbox, 'bin');
     const projectDir = path.join(sandbox, 'project');
+    const observerFixtureDir = path.join(sandbox, 'observer');
     fs.mkdirSync(binDir, { recursive: true });
     fs.mkdirSync(projectDir, { recursive: true });
+    fs.mkdirSync(observerFixtureDir, { recursive: true });
 
-    // Stub claude: exit with the requested code, ignoring all args.
     const claudeStub = path.join(binDir, 'claude');
-    fs.writeFileSync(claudeStub, '#!/usr/bin/env bash\nexit ${CLAUDE_STUB_EXIT:-0}\n');
+    fs.writeFileSync(claudeStub, [
+      '#!/usr/bin/env bash',
+      "printf '%s\n' \"$$\" > \"${CLAUDE_STUB_PID_FILE}\"",
+      "if [ \"${CLAUDE_STUB_IGNORE_TERM:-false}\" = \"true\" ]; then trap '' TERM; fi",
+      'if [ "${CLAUDE_STUB_SPAWN_CHILD:-false}" = "true" ]; then',
+      '  sleep "${CLAUDE_STUB_DELAY_SECONDS:-10}" &',
+      "  printf '%s\n' \"$!\" > \"${CLAUDE_STUB_CHILD_PID_FILE}\"",
+      '  wait',
+      'fi',
+      'if [ "${CLAUDE_STUB_PROBE_RESULT_PATH:-false}" = "true" ]; then',
+      '  for candidate in "${PROJECT_DIR}"/.observer-tmp/ecc-observer-result.*; do',
+      '    [ -e "$candidate" ] || [ -L "$candidate" ] || continue',
+      "    printf 'found\n' > \"${CLAUDE_STUB_ATTACK_FILE}\"",
+      '    rm -f "$candidate"',
+      "    printf '%s\n' '${ANALYSIS_COMPLETE_RECORD}' > \"$candidate\"",
+      '  done',
+      'fi',
+      'if [ "${CLAUDE_STUB_DELAY_SECONDS:-0}" != "0" ]; then',
+      '  sleep "${CLAUDE_STUB_DELAY_SECONDS}"',
+      'fi',
+      "printf '%s' \"${CLAUDE_STUB_OUTPUT:-}\"",
+      "printf '%s' \"${CLAUDE_STUB_STDERR:-}\" >&2",
+      'exit "${CLAUDE_STUB_EXIT:-0}"',
+      '',
+    ].join('\n'));
     fs.chmodSync(claudeStub, 0o755);
 
-    // analyze_observations resolves the real session-guardian.sh via its own
-    // ${BASH_SOURCE[0]}-derived SCRIPT_DIR, so we drive the real guardian with
-    // all of its gates disabled/isolated (see env below) rather than stubbing it.
+    // Source a sandbox copy so the sibling guardian is deterministic.
+    const observerFixture = path.join(observerFixtureDir, 'observer-loop.sh');
+    const guardianStub = path.join(observerFixtureDir, 'session-guardian.sh');
+    fs.copyFileSync(observerLoopPath, observerFixture);
+    fs.writeFileSync(guardianStub, '#!/usr/bin/env bash\nexit 0\n');
+    fs.chmodSync(guardianStub, 0o755);
 
-    // Driver sources observer-loop.sh (guard stops the main loop) then runs
-    // the single function under test.
     const driver = path.join(sandbox, 'driver.sh');
     fs.writeFileSync(
       driver,
-      `#!/usr/bin/env bash\nsource ${JSON.stringify(observerLoopPath)}\nanalyze_observations\n`
+      `#!/usr/bin/env bash\nsource ${JSON.stringify(observerFixture)}\nanalyze_observations\n`
     );
     fs.chmodSync(driver, 0o755);
 
     const observationsFile = path.join(projectDir, 'observations.jsonl');
-    fs.writeFileSync(observationsFile, '{"a":1}\n{"a":2}\n{"a":3}\n');
+    const logFile = path.join(projectDir, 'observer.log');
+    const claudePidFile = path.join(projectDir, 'claude-stub.pid');
+    const claudeChildPidFile = path.join(projectDir, 'claude-stub-child.pid');
+    const attackFile = path.join(projectDir, 'result-path-attack-found');
+    fs.writeFileSync(observationsFile, ORIGINAL_OBSERVATIONS);
+    if (existingLog) fs.writeFileSync(logFile, existingLog);
 
-    // Defensive: never leak CLAUDE_PLUGIN_ROOT into the ECC test shell (it
-    // contaminates this project's hook-root resolution).
-    const childEnv = Object.assign({}, process.env);
-    delete childEnv.CLAUDE_PLUGIN_ROOT;
-    childEnv.PATH = binDir + path.delimiter + process.env.PATH;
-    childEnv.CLAUDE_STUB_EXIT = String(claudeExitCode);
-    childEnv.OBSERVATIONS_FILE = observationsFile;
-    childEnv.MIN_OBSERVATIONS = '1';
-    childEnv.PROJECT_DIR = projectDir;
-    childEnv.LOG_FILE = path.join(projectDir, 'observer.log');
-    childEnv.PROJECT_NAME = 'test-project';
-    childEnv.PROJECT_ID = 'test-project';
-    childEnv.INSTINCTS_DIR = path.join(projectDir, 'instincts');
-    childEnv.CONFIG_DIR = projectDir;
-    childEnv.CLV2_IS_WINDOWS = 'false';
-    childEnv.ECC_OBSERVER_TIMEOUT_SECONDS = '2';
-    // Make the real session-guardian.sh deterministically proceed (exit 0):
-    // disable the active-hours and idle gates, isolate the cooldown log, and
-    // zero the cooldown interval so a fresh project always passes.
-    childEnv.OBSERVER_ACTIVE_HOURS_START = '0';
-    childEnv.OBSERVER_ACTIVE_HOURS_END = '0';
-    childEnv.OBSERVER_MAX_IDLE_SECONDS = '0';
-    childEnv.OBSERVER_INTERVAL_SECONDS = '0';
-    childEnv.OBSERVER_LAST_RUN_LOG = path.join(projectDir, 'observer-last-run.log');
+    const inheritedEnv = Object.fromEntries(
+      Object.entries(process.env).filter(([key]) => key !== 'CLAUDE_PLUGIN_ROOT')
+    );
+    const childEnv = {
+      ...inheritedEnv,
+      PATH: binDir + path.delimiter + process.env.PATH,
+      CLAUDE_STUB_EXIT: String(claudeExitCode),
+      CLAUDE_STUB_OUTPUT: claudeOutput,
+      CLAUDE_STUB_STDERR: claudeStderr,
+      CLAUDE_STUB_DELAY_SECONDS: String(claudeDelaySeconds),
+      CLAUDE_STUB_IGNORE_TERM: String(claudeIgnoreTerm),
+      CLAUDE_STUB_SPAWN_CHILD: String(claudeSpawnChild),
+      CLAUDE_STUB_PROBE_RESULT_PATH: String(probeResultPath),
+      CLAUDE_STUB_PID_FILE: claudePidFile,
+      CLAUDE_STUB_CHILD_PID_FILE: claudeChildPidFile,
+      CLAUDE_STUB_ATTACK_FILE: attackFile,
+      OBSERVATIONS_FILE: observationsFile,
+      MIN_OBSERVATIONS: '1',
+      PROJECT_DIR: projectDir,
+      LOG_FILE: logFile,
+      PROJECT_NAME: 'test-project',
+      PROJECT_ID: 'test-project',
+      INSTINCTS_DIR: path.join(projectDir, 'instincts'),
+      CONFIG_DIR: projectDir,
+      CLV2_IS_WINDOWS: 'false',
+      ECC_OBSERVER_TIMEOUT_SECONDS: String(observerTimeoutSeconds),
+    };
 
+    const startedAt = Date.now();
     const result = spawnSync('bash', [driver], {
       encoding: 'utf8',
       timeout: 15000,
-      env: childEnv
+      env: childEnv,
     });
+    const durationMs = Date.now() - startedAt;
 
+    assert.ifError(result.error);
     assert.strictEqual(
-      result.status, 0,
+      result.status,
+      0,
       `driver should exit 0, got ${result.status}; stderr: ${result.stderr}`
     );
 
     const archiveDir = path.join(projectDir, 'observations.archive');
-    let archivedCount = 0;
-    if (fs.existsSync(archiveDir)) {
-      archivedCount = fs.readdirSync(archiveDir)
-        .filter(f => /^processed-.*\.jsonl$/.test(f)).length;
-    }
-    let log = '';
-    try { log = fs.readFileSync(childEnv.LOG_FILE, 'utf8'); } catch { /* none */ }
+    const archivedContents = fs.existsSync(archiveDir)
+      ? fs.readdirSync(archiveDir)
+        .filter(file => /^processed-.*\.jsonl$/.test(file))
+        .sort()
+        .map(file => fs.readFileSync(path.join(archiveDir, file), 'utf8'))
+      : [];
+    const liveContent = fs.existsSync(observationsFile)
+      ? fs.readFileSync(observationsFile, 'utf8')
+      : null;
+    const log = fs.existsSync(logFile) ? fs.readFileSync(logFile, 'utf8') : '';
+    const observerTempDir = path.join(projectDir, '.observer-tmp');
+    const tempEntries = fs.existsSync(observerTempDir)
+      ? fs.readdirSync(observerTempDir)
+      : [];
+    const claudePid = fs.existsSync(claudePidFile)
+      ? Number(fs.readFileSync(claudePidFile, 'utf8').trim())
+      : null;
+    const claudeChildPid = fs.existsSync(claudeChildPidFile)
+      ? Number(fs.readFileSync(claudeChildPidFile, 'utf8').trim())
+      : null;
 
-    return { liveExists: fs.existsSync(observationsFile), archivedCount, log };
+    return {
+      archivedContents,
+      attackFound: fs.existsSync(attackFile),
+      claudeStillRunning: processExists(claudePid),
+      claudeChildStillRunning: processExists(claudeChildPid),
+      durationMs,
+      liveContent,
+      log,
+      tempEntries,
+    };
   } finally {
     cleanupDir(sandbox);
   }
 }
 
-console.log('\n=== Observer-loop Archive-on-Failure Tests (#2370) ===\n');
+function assertOriginalBatchIsRetryable(state) {
+  assert.strictEqual(
+    state.liveContent,
+    ORIGINAL_OBSERVATIONS,
+    'the live batch must remain byte-for-byte intact for retry'
+  );
+  assert.deepStrictEqual(state.archivedContents, []);
+}
 
+console.log('\n=== Observer-loop Archive-on-Failure Tests (#2370, #2673) ===\n');
 console.log('--- behavioral ---');
 
 test('failed analysis retains observations and archives nothing', () => {
-  // Shell-driven behavioral check; skip on Windows where the bash driver's
-  // $0 path handling differs (matches observer-memory.test.js convention).
-  if (process.platform === 'win32') {
-    return;
-  }
-  const { liveExists, archivedCount, log } = runAnalyzeOnce(1);
-  assert.ok(liveExists, 'live observations.jsonl must be retained when analysis fails');
-  assert.strictEqual(archivedCount, 0, 'nothing should be archived when analysis fails');
-  assert.ok(
-    /retaining observations for retry/.test(log),
-    `failure log should note retention; got: ${log}`
-  );
+  skipOnWindows();
+  const state = runAnalyzeOnce({
+    claudeExitCode: 1,
+    claudeOutput: `${ANALYSIS_COMPLETE_RECORD}\n`,
+  });
+  assertOriginalBatchIsRetryable(state);
+  assert.match(state.log, /retaining observations for retry/);
+  assert.deepStrictEqual(state.tempEntries, []);
 });
 
-test('successful analysis archives the batch (happy path preserved)', () => {
-  // Shell-driven behavioral check; skip on Windows (see note above).
-  if (process.platform === 'win32') {
-    return;
-  }
-  const { liveExists, archivedCount } = runAnalyzeOnce(0);
-  assert.ok(!liveExists, 'live observations.jsonl should be moved after a successful analysis');
-  assert.strictEqual(archivedCount, 1, 'exactly one processed-*.jsonl should be archived on success');
+test('zero-exit analysis without a completion record retains observations', () => {
+  skipOnWindows();
+  const state = runAnalyzeOnce({
+    claudeOutput: 'Analysis blocked because the sampled file was not found.\n',
+  });
+  assertOriginalBatchIsRetryable(state);
+  assert.match(state.log, /completion record missing.*retaining observations for retry/i);
+  assert.deepStrictEqual(state.tempEntries, []);
+});
+
+test('mentioning the completion record in prose does not authorize archival', () => {
+  skipOnWindows();
+  const state = runAnalyzeOnce({
+    claudeOutput: `I would emit ${ANALYSIS_COMPLETE_RECORD} after analysis, but the read failed.\n`,
+  });
+  assertOriginalBatchIsRetryable(state);
+});
+
+test('completion record followed by failure text does not authorize archival', () => {
+  skipOnWindows();
+  const state = runAnalyzeOnce({
+    claudeOutput: `${ANALYSIS_COMPLETE_RECORD}\nLater failure: instinct write did not complete.\n`,
+  });
+  assertOriginalBatchIsRetryable(state);
+});
+
+test('a completion record from an older log entry cannot authorize this run', () => {
+  skipOnWindows();
+  const state = runAnalyzeOnce({
+    existingLog: `prior run\n${ANALYSIS_COMPLETE_RECORD}\n`,
+    claudeOutput: 'Current run could not read its analysis file.\n',
+  });
+  assertOriginalBatchIsRetryable(state);
+});
+
+test('a completion record written only to stderr does not authorize archival', () => {
+  skipOnWindows();
+  const state = runAnalyzeOnce({
+    claudeOutput: 'Analysis did not complete.\n',
+    claudeStderr: `${ANALYSIS_COMPLETE_RECORD}\n`,
+  });
+  assertOriginalBatchIsRetryable(state);
+  assert.ok(state.log.includes(ANALYSIS_COMPLETE_RECORD));
+});
+
+test('duplicate exact completion records do not authorize archival', () => {
+  skipOnWindows();
+  const state = runAnalyzeOnce({
+    claudeOutput: `${ANALYSIS_COMPLETE_RECORD}\n${ANALYSIS_COMPLETE_RECORD}\n`,
+  });
+  assertOriginalBatchIsRetryable(state);
+});
+
+test('replacing the result pathname cannot forge completion', () => {
+  skipOnWindows();
+  const state = runAnalyzeOnce({
+    claudeOutput: 'Analysis did not complete.\n',
+    probeResultPath: true,
+  });
+  assertOriginalBatchIsRetryable(state);
+  assert.strictEqual(state.attackFound, false, 'the open result inode must not remain path-addressable');
+});
+
+test('successful analysis archives the original batch byte-for-byte', () => {
+  skipOnWindows();
+  const state = runAnalyzeOnce({
+    claudeOutput: `Analysis finished.\n\n${ANALYSIS_COMPLETE_RECORD}\n\n`,
+  });
+  assert.strictEqual(state.liveContent, null);
+  assert.deepStrictEqual(state.archivedContents, [ORIGINAL_OBSERVATIONS]);
+  assert.deepStrictEqual(state.tempEntries, []);
+});
+
+test('completion record accepts a CRLF line ending', () => {
+  skipOnWindows();
+  const state = runAnalyzeOnce({
+    claudeOutput: `Analysis complete.\r\n${ANALYSIS_COMPLETE_RECORD}\r\n\r\n`,
+  });
+  assert.strictEqual(state.liveContent, null);
+  assert.deepStrictEqual(state.archivedContents, [ORIGINAL_OBSERVATIONS]);
+});
+
+test('watchdog force-stops a process that ignores TERM and retains observations', () => {
+  skipOnWindows();
+  const state = runAnalyzeOnce({
+    claudeDelaySeconds: 10,
+    claudeIgnoreTerm: true,
+    claudeSpawnChild: true,
+    observerTimeoutSeconds: 1,
+  });
+  assertOriginalBatchIsRetryable(state);
+  assert.ok(state.durationMs < 8000, `watchdog should return promptly; took ${state.durationMs}ms`);
+  assert.strictEqual(state.claudeStillRunning, false, 'timed-out Claude process must be reaped');
+  assert.strictEqual(state.claudeChildStillRunning, false, 'timed-out Claude descendants must stop');
+  assert.match(state.log, /timed out after 1s/);
+  assert.deepStrictEqual(state.tempEntries, []);
 });
 
 console.log('--- static guards ---');
 
-test('analyze_observations returns on failure before the archive mv', () => {
+test('process and semantic failure guards run before archival', () => {
   const content = fs.readFileSync(observerLoopPath, 'utf8');
-  // Operate on full file content with explicit anchors rather than a lazy
-  // function-body extraction (which could truncate on a future inner "\n}"
-  // and pass vacuously). These tokens each occur once, inside the function.
-  const failIdx = content.search(/exit_code"?\s+-ne\s+0/);
-  const returnIdx = content.indexOf('return', failIdx);
+  const processGuardIdx = content.search(/exit_code"?\s+-ne\s+0/);
+  const semanticGuardIdx = content.indexOf('if [ "$analysis_complete" -ne 1 ]');
   const archiveIdx = content.indexOf('observations.archive');
-  assert.ok(failIdx !== -1, 'should find the non-zero exit_code check');
-  assert.ok(archiveIdx !== -1, 'should find the archive block');
-  assert.ok(returnIdx !== -1, 'failure branch should contain a return');
-  assert.ok(returnIdx < archiveIdx,
-    'failure branch must return before reaching the archive block');
+  assert.ok(processGuardIdx !== -1);
+  assert.ok(semanticGuardIdx !== -1);
+  assert.ok(archiveIdx !== -1);
+  assert.ok(processGuardIdx < archiveIdx);
+  assert.ok(semanticGuardIdx < archiveIdx);
 });
 
-test('observer-loop.sh has a source-guard so it can be sourced in tests', () => {
+test('observer-loop.sh has a source guard', () => {
   const content = fs.readFileSync(observerLoopPath, 'utf8');
   assert.ok(
-    content.includes('BASH_SOURCE[0]') && content.includes('return 0 2>/dev/null'),
-    'observer-loop.sh should short-circuit when sourced rather than executed'
+    content.includes('BASH_SOURCE[0]') && content.includes('return 0 2>/dev/null')
   );
 });
 
 console.log('\n=== Test Results ===');
 console.log(`Passed: ${passed}`);
 console.log(`Failed: ${failed}`);
-console.log(`Total:  ${passed + failed}\n`);
+console.log(`Skipped: ${skipped}`);
+console.log(`Total:  ${passed + failed + skipped}\n`);
 
 process.exit(failed > 0 ? 1 : 0);