Bob 2.0 driver observability: review-findings capture + last_dispatch health signal

CHANGELOG.md

@@ -3,20 +3,41 @@
 All notable changes to this project are documented here. Format loosely follows
 [Keep a Changelog](https://keepachangelog.com/); versions are [SemVer](https://semver.org/).
 
+## [Unreleased]
+
+### Added
+
+- **`--webhook <url>` on the worker.** POSTs notable transitions (a task done / blocked / needs-input /
+  retrying, and the worker stopping / erroring) to a URL as `application/json`. One payload serves Slack
+  (`text`), Discord (`content`), and a generic receiver (`{ event, seq, data, worker, ts }`; `seq` is
+  monotonic within `worker.run`, a per-process id, so a restart reads as a new run, not data loss).
+  Best-effort: a POST never blocks or crashes the drain, an in-flight cap drops bursts to a slow endpoint
+  (re-warning on each new overload), and pending POSTs flush before every exit so the final event lands.
+  The URL is validated at startup (http/https only) and redacted in logs; `--webhook-secret <s>` HMAC-signs
+  the body (`X-Bob-Signature`) and fails loud when passed without `--webhook`.
+- **`board_status.worker_draining.last_dispatch`** — each live drainer's most recent dispatch outcome
+  (status, failure detail, freshness), stamped by both the 2.0 in-process loop and the standalone worker,
+  so a live-but-failing drainer (e.g. Bob logged out → every dispatch aborts) surfaces up front instead of
+  reading as a healthy heartbeat. Detail is stored only for failures — success text never reaches the board.
+- **Full review findings on the 2.0 in-process driver** — review-mode dispatches parse Bob's whole
+  transcript into the structured `bob-review` board note (parity with 1.x), not just the closing summary.
+
+### Fixed
+
+- **The review parser no longer invents findings from prose headings.** A `### ` section with no severity
+  and no `Location`/`Category` field (a reasoning turn's "Investigation plan") is skipped instead of
+  persisted as a phantom `info` finding.
+
 ## [2.1.0] — 2026-07-03 — npm distribution + untracked-aware verifier
 
+_Correction: `--webhook` was listed here but merged after the 2.1.0 npm publish; it ships in the next
+release (see Unreleased)._
+
 ### Added
 
 - **Published to npm as `@pounceai/bob-control`.** `npx -y @pounceai/bob-control` runs the MCP server
   standalone via a new `bob-control` bin. The `files` allowlist ships only runtime `dist` (no tests or
   fixtures), and a `check:shebang` publish gate guards both bin shebangs.
-- **`--webhook <url>` on the worker.** POSTs notable transitions (a task done / blocked / needs-input /
-  retrying, and the worker stopping / erroring) to a URL as `application/json`. One payload serves Slack
-  (`text`), Discord (`content`), and a generic receiver (`{ event, seq, data, worker, ts }`, `seq`
-  monotonic for reordering). Best-effort: a POST never blocks or crashes the drain, an in-flight cap drops
-  bursts to a slow endpoint, and pending POSTs flush before every exit so the final event lands. The URL
-  is validated at startup (http/https only) and redacted in logs; `--webhook-secret <s>` HMAC-signs the
-  body (`X-Bob-Signature`) for a generic receiver to verify.
 
 ### Fixed
 

README.md

@@ -338,7 +338,8 @@ system sound and terminal bell are off by default).
 retrying, and the worker itself stopping or erroring — to a URL as `application/json`. One payload
 serves three consumers: it carries `text` (a Slack incoming webhook renders it), `content` (a Discord
 webhook renders it), and the structured `{ event, seq, data, worker, ts }` for a generic receiver
-(`seq` monotonic, so concurrent POSTs can be reordered). Delivery is
+(`seq` monotonic within `worker.run`, a per-process id, so a restart reads as a new run, not dropped
+deliveries; concurrent POSTs can be reordered). Delivery is
 best-effort: a POST never blocks or crashes the drain, bursts past an in-flight cap to a slow endpoint
 are dropped, and the worker flushes pending POSTs before it exits so the final event still lands. The
 URL is validated at startup (http/https only) and only its host is logged, since a Slack/Discord webhook

src/bob2-driver.test.ts

@@ -135,6 +135,10 @@ test("mapOutcome: real error → aborted, settled → completed, unsettled → t
   assert.equal(mapOutcome(r({ last_error: "boom" }), true, { result: "x", tokensUsed: 7 }).result, ""); // not on abort
   assert.equal(mapOutcome(r(), false, { tokensUsed: 42 }).tokensUsed, 42);
   assert.equal(mapOutcome(r(), true, { maxIdleMs: 6500 }).maxIdleMs, 6500); // watchdog telemetry, any outcome
+  // reviewFindings pass through on completion (parity with 1.x), absent on a non-completed outcome
+  const finding = [{ severity: "HIGH", title: "a bug", description: "details", file: "x.ts", category: "correctness" }];
+  assert.deepEqual(mapOutcome(r(), true, { reviewFindings: finding }).reviewFindings, finding);
+  assert.equal(mapOutcome(r(), false, { reviewFindings: finding }).reviewFindings, undefined); // not on timeout
 });
 
 // ── connect / queryWorkspace ─────────────────────────────────────────────────────────────────────

src/bob2-driver.ts

@@ -12,6 +12,8 @@ import {
   type Bob2TaskRow,
 } from "./bob2-taskstore.js";
 import { writeAutoApprove } from "./bob2-config.js";
+import { producesReviewFindings } from "./modes.js";
+import { parseReviewFindings, type ReviewIssue } from "./review-findings.js";
 
 // The Bob 2.0 in-process driver. Bob 2.0 removed the node-ipc pipe, so the only way to start a task
 // is the extension's exported activate() API (`getExtension('IBM.bob-code').exports.startTask`), callable
@@ -82,7 +84,7 @@ export interface InProcessDriverOptions {
 export function mapOutcome(
   row: Bob2TaskRow | null,
   settled: boolean,
-  extras: { result?: string; tokensUsed?: number; maxIdleMs?: number } = {},
+  extras: { result?: string; reviewFindings?: ReviewIssue[]; tokensUsed?: number; maxIdleMs?: number } = {},
 ): DispatchResult {
   const err = row ? taskError(row) : null;
   const detail = row ? `bob2 status=${row.status}${err ? ` error=${err}` : ""}` : "";
@@ -95,7 +97,8 @@ export function mapOutcome(
     maxIdleMs: extras.maxIdleMs ?? 0,
   };
   if (err) return { ...base, status: "aborted" };
-  if (settled) return { ...base, status: "completed", result: extras.result ?? "" };
+  if (settled)
+    return { ...base, status: "completed", result: extras.result ?? "", reviewFindings: extras.reviewFindings };
   return { ...base, status: "timeout" };
 }
 
@@ -282,8 +285,15 @@ export class InProcessDriver implements BobDriver {
       // clean completion; a timeout/error row's last message would be partial/misleading). maxGapMs is
       // stall-watchdog telemetry (see DispatchResult.maxIdleMs).
       const tokensUsed = parseCosts(row?.costs ?? null)?.output ?? 0;
-      const result = settled && row && !taskError(row) ? (store.readResultText(id) ?? "") : "";
-      return mapOutcome(row, settled, { result, tokensUsed, maxIdleMs: maxGapMs });
+      const done = settled && !!row && !taskError(row);
+      const result = done ? (store.readResultText(id) ?? "") : "";
+      // Review mode: findings span the task's assistant messages and readResultText returns only the last
+      // (a summary), so parse the full transcript into structured findings for the board's bob-review note.
+      const reviewFindings =
+        done && producesReviewFindings(opts.mode ?? "")
+          ? parseReviewFindings(store.readReviewText(id) ?? "")
+          : undefined;
+      return mapOutcome(row, settled, { result, reviewFindings, tokensUsed, maxIdleMs: maxGapMs });
     } finally {
       store?.close();
     }

src/bob2-taskstore.test.ts

@@ -403,6 +403,32 @@ test("readResultText flattens array content and ignores unparseable rows", () =>
   assert.equal(store.readResultText("b"), null); // bad JSON → null, never throws
 });
 
+test("readReviewText joins all assistant messages oldest→newest (findings precede the summary)", () => {
+  const { db, store } = makeStore();
+  insertMessage(db, "r1", "user", { content: "review it" }, 100);
+  insertMessage(db, "r1", "assistant", { content: "Let me look." }, 200); // reasoning
+  insertMessage(db, "r1", "assistant", { content: "### HIGH: a real bug\nLocation: x.ts" }, 300); // findings
+  insertMessage(db, "r1", "tool", { content: "read file" }, 350); // not assistant → excluded
+  insertMessage(db, "r1", "assistant", { content: "Review complete. 1 finding." }, 400); // summary (last)
+  const text = store.readReviewText("r1") ?? "";
+  assert.ok(text.includes("### HIGH: a real bug"), "the findings message is included, not just the summary");
+  assert.ok(text.includes("Review complete"), "the closing summary is included too");
+  // Contrast with readResultText, which sees only the last (summary) message — the bug this fixes.
+  assert.equal(store.readResultText("r1"), "Review complete. 1 finding.");
+  assert.equal(store.readReviewText("none"), null);
+});
+
+test("readReviewText skips a malformed row but keeps the rest", () => {
+  const { db, store } = makeStore();
+  insertMessage(db, "r2", "assistant", { content: "### LOW: keep me" }, 10);
+  db.prepare(
+    "INSERT INTO messages (id, task_id, role, data, created_at) VALUES ('bad2','r2','assistant','{not json',20)",
+  ).run();
+  insertMessage(db, "r2", "assistant", { content: "and me" }, 30);
+  const text = store.readReviewText("r2") ?? "";
+  assert.ok(text.includes("### LOW: keep me") && text.includes("and me"));
+});
+
 test("open() throws a clear error when the db file is absent", () => {
   const missing = join(mkdtempSync(join(tmpdir(), "bob2db-")), "nope.db");
   assert.throws(() => Bob2TaskStore.open(missing), /not found/);

src/bob2-taskstore.ts

@@ -284,6 +284,30 @@ export class Bob2TaskStore {
     }
   }
 
+  /** The task's assistant messages joined oldest→newest — for review read-back. A review's findings sit in
+   *  an EARLIER message than readResultText's last one (which is Bob's closing summary), so the structured
+   *  parser must see the whole transcript, not just the tail. Per-row parse guard: one bad row can't drop
+   *  the rest. Best-effort — null when there are no assistant messages (never throws). */
+  readReviewText(id: string): string | null {
+    try {
+      const rows = this.q(
+        "SELECT data FROM messages WHERE task_id = ? AND role = 'assistant' ORDER BY created_at ASC",
+      ).all(id) as { data: string }[];
+      const parts: string[] = [];
+      for (const r of rows) {
+        try {
+          const t = extractText((JSON.parse(r.data) as { content?: unknown }).content);
+          if (t) parts.push(t);
+        } catch {
+          /* skip a malformed row, keep the rest */
+        }
+      }
+      return parts.length ? parts.join("\n\n") : null;
+    } catch {
+      return null;
+    }
+  }
+
   close(): void {
     this.db.close();
   }

src/db.ts

@@ -208,6 +208,9 @@ function migrate(d: DatabaseSync): void {
   addColumnIfMissing(d, "tasks", "estimated_tokens", "INTEGER");
   addColumnIfMissing(d, "worker_heartbeats", "worktree", "TEXT"); // T7 worktree lease
   addColumnIfMissing(d, "worker_heartbeats", "tag", "TEXT"); // the worker's --tag pin, surfaced in worker_draining
+  addColumnIfMissing(d, "worker_heartbeats", "last_dispatch_status", "TEXT"); // most recent dispatch outcome (health signal)
+  addColumnIfMissing(d, "worker_heartbeats", "last_dispatch_detail", "TEXT"); // short reason, e.g. a provider/auth error
+  addColumnIfMissing(d, "worker_heartbeats", "last_dispatch_at", "TEXT"); // when that dispatch settled (freshness)
 }
 
 /**
@@ -633,6 +636,10 @@ export interface WorkerLiveness {
   /** Distinct --tag pins of the live workers (null = unfiltered, drains every tag) — so a foreman can see
    *  why a live worker isn't pulling a task: if no entry is null or matches the task's tags, it's filtered out. */
   tags: (string | null)[];
+  /** The most recent dispatch outcome among the live workers — a health signal beyond "is anything beating".
+   *  A logged-out Bob keeps beating (the 2.0 loop runs) but its dispatches abort, so `status:"aborted"` here
+   *  flags "drainer alive but not actually working" up front. null when no live worker has dispatched yet. */
+  last_dispatch: { status: string; detail: string | null; seconds_ago: number } | null;
 }
 
 /** Identifying metadata for a beat. `worktree` is the checkout the worker is bound to (its normalized cwd)
@@ -657,6 +664,21 @@ export function recordWorkerHeartbeat(workerId: string, meta: HeartbeatMeta = {}
     .run(workerId, meta.assignee ?? null, meta.pid ?? null, meta.worktree ?? null, meta.tag ?? null, now, now);
 }
 
+/** Stamp the worker's most recent dispatch outcome onto its heartbeat — a board_status health signal, so a
+ *  live-but-failing drainer (e.g. Bob logged out → every dispatch aborts) shows as last_dispatch.status
+ *  "aborted" instead of a healthy-looking "draining". Refreshes last_beat too; no-op if the row is gone
+ *  (worker stopped). `detail` (Bob's error line) is whitespace-collapsed, truncated, and stored only for a
+ *  non-completed status — success text is task content and must not leak into board_status. */
+export function recordDispatchOutcome(workerId: string, status: string, detail?: string | null): void {
+  const now = nowIso();
+  const d = status !== "completed" && detail ? detail.replace(/\s+/g, " ").trim().slice(0, 160) || null : null;
+  getDb()
+    .prepare(
+      "UPDATE worker_heartbeats SET last_dispatch_status = ?, last_dispatch_detail = ?, last_dispatch_at = ?, last_beat = ? WHERE worker_id = ?",
+    )
+    .run(status, d, now, now, workerId);
+}
+
 export interface WorktreeLeaseHolder {
   worker_id: string;
   pid: number | null;
@@ -800,19 +822,33 @@ export function getWorkerLiveness(windowMs = WORKER_HEARTBEAT_WINDOW_MS, nowMs =
   getDb()
     .prepare("DELETE FROM worker_heartbeats WHERE last_beat < ?")
     .run(new Date(nowMs - windowMs * 30).toISOString()); // prune workers dead far past the window
-  const rows = getDb().prepare("SELECT last_beat, tag FROM worker_heartbeats").all() as {
+  const rows = getDb()
+    .prepare(
+      "SELECT last_beat, tag, last_dispatch_status, last_dispatch_detail, last_dispatch_at FROM worker_heartbeats",
+    )
+    .all() as {
     last_beat: string;
     tag: string | null;
+    last_dispatch_status: string | null;
+    last_dispatch_detail: string | null;
+    last_dispatch_at: string | null;
   }[];
   const liveCutoff = nowMs - windowMs;
   let workers = 0;
   let mostRecent: number | null = null;
   const tags = new Set<string | null>();
+  let lastDispatch: { status: string; detail: string | null; at: number } | null = null;
   for (const r of rows) {
     const t = Date.parse(r.last_beat);
     if (t >= liveCutoff) {
       workers++;
       tags.add(r.tag); // null = an unfiltered worker (drains all tags)
+      // The freshest dispatch outcome across live workers (a dead worker's stale outcome must not surface).
+      if (r.last_dispatch_status && r.last_dispatch_at) {
+        const dt = Date.parse(r.last_dispatch_at);
+        if (!lastDispatch || dt > lastDispatch.at)
+          lastDispatch = { status: r.last_dispatch_status, detail: r.last_dispatch_detail, at: dt };
+      }
     }
     if (mostRecent === null || t > mostRecent) mostRecent = t;
   }
@@ -821,6 +857,13 @@ export function getWorkerLiveness(windowMs = WORKER_HEARTBEAT_WINDOW_MS, nowMs =
     workers,
     last_beat_seconds_ago: mostRecent === null ? null : Math.max(0, Math.round((nowMs - mostRecent) / 1000)),
     tags: [...tags],
+    last_dispatch: lastDispatch
+      ? {
+          status: lastDispatch.status,
+          detail: lastDispatch.detail,
+          seconds_ago: Math.max(0, Math.round((nowMs - lastDispatch.at) / 1000)),
+        }
+      : null,
   };
 }
 

src/driver-loop.test.ts

@@ -201,6 +201,23 @@ test("runDriverLoop beats a liveness heartbeat while draining, and clears it on
   assert.equal(getWorkerLiveness().draining, false, "heartbeat should be cleared once the loop stops");
 });
 
+test("runDriverLoop stamps each dispatch outcome onto its own heartbeat (board_status last_dispatch)", async () => {
+  // Exercises the workerId threading end-to-end (runDriverLoop → driveCfg → finalize), not just the
+  // recordDispatchOutcome unit. The row clears on stop, so read it at the taskFail emit (fires post-stamp).
+  createTask({ title: "will abort", mode: "code" });
+  const driver = fakeDriver({ status: "aborted", lastText: "bob2 status=error error=ProviderAuth" });
+  const seen: { last: ReturnType<typeof getWorkerLiveness>["last_dispatch"] } = { last: null };
+  await runDriverLoop({
+    ...cfg(driver, ["--once"]),
+    emit: (t) => {
+      if (t === "taskFail") seen.last = getWorkerLiveness().last_dispatch;
+    },
+  });
+  assert.equal(seen.last?.status, "aborted", "the loop's own heartbeat carries the dispatch outcome");
+  assert.match(seen.last?.detail ?? "", /ProviderAuth/, "the failure detail surfaces for the on-call");
+  assert.equal(getWorkerLiveness().last_dispatch, null, "cleared with the heartbeat on stop");
+});
+
 test("runDriverLoop clears the heartbeat (and closes the driver) even if the loop throws", async () => {
   // Teardown is in finally, so a throw escaping the loop body must not leak the beat — an orphaned interval
   // would pin worker_draining true forever in the never-exiting extension host.