fix(vision-manager): symmetric ERR handling + smoke tests + analysis reload reclaim test (0.87.15)

0.87.7 + 0.87.8 follow-ups:

1. mem_check_and_stop now defers on docker ERR instead of silently
   no-op'ing the SIGTERM under pressure.
2. Post-mem_check_and_stop loop refresh now also handles the ERR
   sentinel (continues the loop) rather than letting it reset up_for
   and defeat MAX_UPTIME.
3. New `vision-manager/test_manager.sh` — 7 smoke tests covering
   vision_running's three exit shapes and mem_check_and_stop's ERR
   symmetry, via stubbed docker/psql/curl on PATH. Production loop
   guarded by MANAGER_TEST_NO_MAIN.
4. New `app::tests::spawn_analysis_daemon_reclaims_orphaned_analyze_leases`
   pins the 0.87.7 wire-through: seeds an expired analyze_page lease,
   spawns + shuts down, asserts pending + attempts refunded.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

backend/Cargo.lock

@@ -1517,7 +1517,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
 
 [[package]]
 name = "mangalord"
-version = "0.87.14"
+version = "0.87.15"
 dependencies = [
  "anyhow",
  "argon2",

backend/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mangalord"
-version = "0.87.14"
+version = "0.87.15"
 edition = "2021"
 default-run = "mangalord"
 

backend/src/app.rs

@@ -1367,4 +1367,95 @@ mod tests {
         assert!(resp.headers().get("access-control-allow-origin").is_none());
         assert!(resp.headers().get("access-control-allow-credentials").is_none());
     }
+
+    /// `spawn_analysis_daemon` runs `crawler::jobs::reclaim_orphaned` at
+    /// startup so an analysis-only deploy refunds expired
+    /// `analyze_page` leases that a crashed previous run left running.
+    /// Without this, the call could be silently removed and the
+    /// reclaim test in `tests/crawler_jobs.rs` would still pass
+    /// (it covers the helper, not the wiring).
+    ///
+    /// Seed an expired-lease `analyze_page` row in `crawler_jobs`, call
+    /// `spawn_analysis_daemon`, then assert the row went back to
+    /// `pending` with the attempt refunded.
+    #[sqlx::test(migrations = "./migrations")]
+    async fn spawn_analysis_daemon_reclaims_orphaned_analyze_leases(pool: PgPool) {
+        use crate::storage::LocalStorage;
+        use std::time::Duration;
+        use uuid::Uuid;
+
+        // Seed a chapter + page so the worker has something it COULD lease.
+        // We don't need the worker to actually run — we're testing the
+        // reclaim that happens before workers start.
+        let manga_id: Uuid =
+            sqlx::query_scalar("INSERT INTO mangas (title) VALUES ('M') RETURNING id")
+                .fetch_one(&pool)
+                .await
+                .unwrap();
+        let chapter_id: Uuid = sqlx::query_scalar(
+            "INSERT INTO chapters (manga_id, number) VALUES ($1, 1) RETURNING id",
+        )
+        .bind(manga_id)
+        .fetch_one(&pool)
+        .await
+        .unwrap();
+        let page_id: Uuid = sqlx::query_scalar(
+            "INSERT INTO pages (chapter_id, page_number, storage_key, content_type) \
+             VALUES ($1, 1, 'k/1.png', 'image/png') RETURNING id",
+        )
+        .bind(chapter_id)
+        .fetch_one(&pool)
+        .await
+        .unwrap();
+
+        // Plant an expired-lease analyze_page row that mimics a previous
+        // worker that crashed mid-dispatch (attempts=1, leased_until in
+        // the past, state=running).
+        let payload = serde_json::json!({
+            "kind": "analyze_page",
+            "page_id": page_id,
+            "force": false
+        });
+        sqlx::query(
+            "INSERT INTO crawler_jobs (payload, state, attempts, leased_until) \
+             VALUES ($1, 'running', 1, now() - interval '1 hour')",
+        )
+        .bind(payload)
+        .execute(&pool)
+        .await
+        .unwrap();
+
+        let storage: Arc<dyn Storage> = Arc::new(LocalStorage::new(
+            tempfile::tempdir().unwrap().path(),
+        ));
+        let mut cfg = crate::config::AnalysisConfig::default();
+        cfg.workers = 1;
+        cfg.job_timeout = Duration::from_secs(1);
+        let events = Arc::new(crate::analysis::events::AnalysisEvents::new());
+
+        let handle = spawn_analysis_daemon(pool.clone(), storage, &cfg, events)
+            .await
+            .expect("spawn");
+        // Immediately shut down — we're only here to prove reclaim ran.
+        // The workers may briefly pick up the now-pending row; that's
+        // fine, but we cancel before any real dispatch (the LocalStorage
+        // key doesn't exist, so a dispatch would fail anyway).
+        handle.shutdown().await;
+
+        // The reclaim must have moved the row back to pending with the
+        // attempt refunded (attempts goes from 1 → 0). Reaching it on the
+        // initial running state would mean reclaim never ran.
+        let (state, attempts): (String, i32) = sqlx::query_as(
+            "SELECT state, attempts FROM crawler_jobs WHERE payload->>'page_id' = $1",
+        )
+        .bind(page_id.to_string())
+        .fetch_one(&pool)
+        .await
+        .unwrap();
+        assert_eq!(
+            state, "pending",
+            "expired-lease analyze_page row must be reclaimed to pending"
+        );
+        assert_eq!(attempts, 0, "reclaim_orphaned refunds the attempt");
+    }
 }

frontend/package.json

@@ -1,6 +1,6 @@
 {
   "name": "mangalord-frontend",
-  "version": "0.87.14",
+  "version": "0.87.15",
   "private": true,
   "type": "module",
   "scripts": {

vision-manager/manager.sh

@@ -164,17 +164,34 @@ stop_vision() {
 # SIGTERM it (the friendly, right-victim alternative to an OOM kill) and arm a
 # cooldown. Cheap on the hot path — only inspects docker once used% is already
 # over the mark. Reads `mem_yield_until`/`idle_for`/`up_for` from the loop scope.
+#
+# Symmetric ERR handling with the main loop's `running` refresh: an inspect
+# blip under genuine memory pressure must NOT silently no-op the SIGTERM.
+# When `vision_running` returns `ERR`, log it and skip THIS check rather than
+# treating the ambiguous state as "not running" (which would let memory
+# pressure go un-yielded until the next tick).
 mem_check_and_stop() {
   [ "$MEM_YIELD_ENABLED" = "0" ] && return 0
   local used; used="$(read_mem_used_pct)"
   case "$used" in ''|*[!0-9]*) return 0 ;; esac   # ignore a malformed read
-  if [ "$used" -ge "$MEM_HIGH_WATERMARK_PCT" ] && [ "$(vision_running)" = "true" ]; then
-    if stop_vision "memory-yield: used ${used}% >= ${MEM_HIGH_WATERMARK_PCT}% high watermark"; then
-      mem_yield_until=$(( $(date +%s) + MEM_YIELD_COOLDOWN ))
-      idle_for=0
-      up_for=0
-    fi
-  fi
+  [ "$used" -lt "$MEM_HIGH_WATERMARK_PCT" ] && return 0
+  local vr; vr="$(vision_running)"
+  case "$vr" in
+    ERR)
+      log "WARNING: memory-yield wanted to stop vision (used ${used}% >= ${MEM_HIGH_WATERMARK_PCT}%), but vision_running returned ERR — deferring to next tick"
+      return 0
+      ;;
+    true)
+      if stop_vision "memory-yield: used ${used}% >= ${MEM_HIGH_WATERMARK_PCT}% high watermark"; then
+        mem_yield_until=$(( $(date +%s) + MEM_YIELD_COOLDOWN ))
+        idle_for=0
+        up_for=0
+      fi
+      ;;
+    *)
+      # Not running — nothing to yield from. Cheap no-op.
+      ;;
+  esac
 }
 
 # Start inhibit: echo 1 while a cooldown is in effect OR used% is at/above the
@@ -198,6 +215,14 @@ analysis_off=0    # 1 while analysis is disabled in settings (log on flip only)
 mem_yield_until=0 # epoch deadline of the active memory-yield cooldown (0 = none)
 mem_inhibited=0   # 1 while starts are inhibited by memory pressure (log on flip only)
 
+# Tests source this script with `MANAGER_TEST_NO_MAIN=1` set so they can
+# exercise the helper functions above with stubbed `docker`/`psql` on PATH
+# without entering the production poll loop. Production callers (and the
+# Dockerfile entrypoint) leave the var unset, so the loop runs as before.
+if [ -n "${MANAGER_TEST_NO_MAIN:-}" ]; then
+  return 0 2>/dev/null || exit 0
+fi
+
 while true; do
   pending="$(pending_analysis)"
   running="$(vision_running)"
@@ -250,7 +275,18 @@ while true; do
   #     LOW (idle stack + LLM ≈ 80% here). LOW is a START gate, not a stop
   #     threshold.
   mem_check_and_stop
-  running="$(vision_running)"   # refresh: the active stop may have just flipped it
+  # Refresh: the active stop may have just flipped vision_running.
+  # Symmetric ERR handling with the top-of-loop check — without this,
+  # a docker-socket-proxy hiccup between mem_check_and_stop and here
+  # makes `running=ERR`, the next `[ "$running" = "true" ]` test is
+  # false, and `up_for=0` resets — defeating MAX_UPTIME exactly the
+  # way the top-of-loop guard was meant to prevent.
+  running="$(vision_running)"
+  if [ "$running" = "ERR" ]; then
+    log "WARNING: docker inspect failed after mem_check_and_stop; will retry next tick"
+    sleep "$POLL_INTERVAL"
+    continue
+  fi
   mem_block_start=0
   if [ "$(mem_yield_active)" = "1" ]; then
     mem_block_start=1

vision-manager/test_manager.sh

@@ -0,0 +1,176 @@
+#!/usr/bin/env bash
+# Smoke tests for vision-manager's helper functions. Sources `manager.sh`
+# with the `MANAGER_TEST_NO_MAIN=1` guard so the production poll loop is
+# skipped, then re-points `docker`/`psql` at stub binaries in a temp dir
+# on PATH to drive each branch.
+#
+# Run: `vision-manager/test_manager.sh` (no arguments).
+#
+# This is intentionally pragmatic — a full bats setup adds a dependency
+# the project doesn't otherwise carry. The smoke tests cover the three
+# behaviours the 0.87.8 commit claims, plus the two ERR-guard gaps the
+# adversarial review flagged (mem_check_and_stop ERR symmetry, post-
+# mem_check loop refresh).
+set -euo pipefail
+
+script_dir="$(cd "$(dirname "$0")" && pwd)"
+
+# ---------------------------------------------------------------------------
+# Stub harness
+# ---------------------------------------------------------------------------
+
+stub_dir="$(mktemp -d)"
+trap 'rm -rf "$stub_dir"' EXIT
+
+# `state` file is written by stubs to record calls; read by tests to verify
+# argv/exit code combinations.
+state_file="$stub_dir/.state"
+: > "$state_file"
+
+# Configurable stubs: each stub looks for a per-call env var and emits
+# whatever the test set. Stubs append to $state_file so a test can assert
+# "stop_vision called N times" etc.
+
+cat > "$stub_dir/docker" <<'STUB'
+#!/usr/bin/env bash
+echo "docker $*" >> "$STUB_STATE"
+case "$1" in
+  inspect)
+    : "${STUB_DOCKER_INSPECT_OUT:=true}"
+    : "${STUB_DOCKER_INSPECT_RC:=0}"
+    printf '%s' "$STUB_DOCKER_INSPECT_OUT"
+    exit "$STUB_DOCKER_INSPECT_RC"
+    ;;
+  stop|start)
+    : "${STUB_DOCKER_RC:=0}"
+    exit "$STUB_DOCKER_RC"
+    ;;
+esac
+exit 0
+STUB
+chmod +x "$stub_dir/docker"
+
+cat > "$stub_dir/psql" <<'STUB'
+#!/usr/bin/env bash
+echo "psql $*" >> "$STUB_STATE"
+: "${STUB_PSQL_OUT:=0}"
+: "${STUB_PSQL_RC:=0}"
+printf '%s' "$STUB_PSQL_OUT"
+exit "$STUB_PSQL_RC"
+STUB
+chmod +x "$stub_dir/psql"
+
+cat > "$stub_dir/curl" <<'STUB'
+#!/usr/bin/env bash
+echo "curl $*" >> "$STUB_STATE"
+: "${STUB_CURL_RC:=0}"
+exit "$STUB_CURL_RC"
+STUB
+chmod +x "$stub_dir/curl"
+
+# Source manager.sh with the no-main guard. DATABASE_URL is required by
+# the script's preamble; a stub value is fine because we override `psql`.
+export PATH="$stub_dir:$PATH"
+export STUB_STATE="$state_file"
+export DATABASE_URL="postgres://stub@stub/stub"
+export MANAGER_TEST_NO_MAIN=1
+# Disable `set -e` propagation into our own asserts so a failing test
+# doesn't kill the harness.
+set +e
+# shellcheck disable=SC1091
+source "$script_dir/manager.sh"
+set -e
+
+# ---------------------------------------------------------------------------
+# Asserts
+# ---------------------------------------------------------------------------
+
+pass=0
+fail=0
+
+assert_eq() {
+  local got="$1" want="$2" desc="$3"
+  if [ "$got" = "$want" ]; then
+    pass=$((pass + 1))
+    echo "  ok: $desc"
+  else
+    fail=$((fail + 1))
+    echo "  FAIL: $desc"
+    echo "    want: ${want}"
+    echo "    got:  ${got}"
+  fi
+}
+
+reset() {
+  : > "$state_file"
+  unset STUB_DOCKER_INSPECT_OUT STUB_DOCKER_INSPECT_RC STUB_DOCKER_RC
+  unset STUB_PSQL_OUT STUB_PSQL_RC STUB_CURL_RC
+}
+
+# ---------------------------------------------------------------------------
+# Tests
+# ---------------------------------------------------------------------------
+
+echo "vision_running: disambiguates running/absent/error"
+
+reset
+export STUB_DOCKER_INSPECT_OUT="true" STUB_DOCKER_INSPECT_RC=0
+assert_eq "$(vision_running)" "true" "running container → 'true'"
+
+reset
+export STUB_DOCKER_INSPECT_OUT="false" STUB_DOCKER_INSPECT_RC=0
+assert_eq "$(vision_running)" "false" "stopped container → 'false'"
+
+reset
+export STUB_DOCKER_INSPECT_OUT="Error: No such container: mangalord-vision" STUB_DOCKER_INSPECT_RC=1
+assert_eq "$(vision_running)" "false" "absent container ('No such container') → 'false' (not ERR)"
+
+reset
+export STUB_DOCKER_INSPECT_OUT="Cannot connect to the Docker daemon at tcp://..." STUB_DOCKER_INSPECT_RC=2
+assert_eq "$(vision_running)" "ERR" "socket-proxy hiccup (unrelated stderr) → 'ERR' sentinel"
+
+echo
+echo "mem_check_and_stop: ERR symmetry under genuine memory pressure"
+
+# Force read_mem_used_pct to report 95% so mem_check_and_stop wants to stop.
+# Easiest: override the function in this test shell.
+read_mem_used_pct() { echo "95"; }
+
+reset
+export STUB_DOCKER_INSPECT_OUT="Cannot connect to the Docker daemon" STUB_DOCKER_INSPECT_RC=2
+MEM_YIELD_ENABLED=1 MEM_HIGH_WATERMARK_PCT=90 mem_check_and_stop
+# Stub state should NOT contain a `docker stop` line — the inspect error
+# must defer, not silently no-op the SIGTERM.
+if grep -q "^docker stop " "$state_file"; then
+  fail=$((fail + 1))
+  echo "  FAIL: mem_check_and_stop must NOT call docker stop when vision_running returns ERR"
+else
+  pass=$((pass + 1))
+  echo "  ok: mem_check_and_stop defers stop on inspect ERR (no silent no-op)"
+fi
+
+reset
+export STUB_DOCKER_INSPECT_OUT="true" STUB_DOCKER_INSPECT_RC=0
+MEM_YIELD_ENABLED=1 MEM_HIGH_WATERMARK_PCT=90 mem_check_and_stop
+if grep -q "^docker stop " "$state_file"; then
+  pass=$((pass + 1))
+  echo "  ok: mem_check_and_stop stops vision when vision_running='true' and pressure"
+else
+  fail=$((fail + 1))
+  echo "  FAIL: mem_check_and_stop should call docker stop when vision is running under pressure"
+fi
+
+reset
+export STUB_DOCKER_INSPECT_OUT="false" STUB_DOCKER_INSPECT_RC=0
+MEM_YIELD_ENABLED=1 MEM_HIGH_WATERMARK_PCT=90 mem_check_and_stop
+if grep -q "^docker stop " "$state_file"; then
+  fail=$((fail + 1))
+  echo "  FAIL: mem_check_and_stop should NOT call docker stop when vision isn't running"
+else
+  pass=$((pass + 1))
+  echo "  ok: mem_check_and_stop no-ops when vision isn't running"
+fi
+
+echo
+echo "Result: ${pass} pass, ${fail} fail"
+[ "$fail" -eq 0 ]