diff --git a/CHANGELOG.md b/CHANGELOG.md index aa00ab2..5d23a5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,25 @@ All notable changes to this project are documented here. Format loosely follows [Keep a Changelog](https://keepachangelog.com/); versions are [SemVer](https://semver.org/). +## [2.1.0] — 2026-07-03 — npm distribution + untracked-aware verifier + +### 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. + +### Fixed + +- **The verifier sees edits to files that stay untracked.** The completion check and LLM judge diffed with + `git status`/`git diff HEAD`, blind to an edit to a file already untracked when the task started — so real + work read as "no changes" and was aborted. Both now diff two untracked-aware `git write-tree` snapshots. + A failed tree diff falls through to the ref diff instead of reporting "no changes", a timed-out snapshot is + killed rather than leaked, and both degradations log to stderr. +- **The create_task race-warning fires for a live-but-idle drainer.** It keyed off in-progress tasks, so it + stayed silent in exactly the mid-curation race it guards. It now reads the worker heartbeat, matched to the + task's tags; the redundant `worker_likely_active` field is dropped from `board_status` (use `worker_draining`). + ## [2.0.2] — 2026-06-30 — Faster completion + Bob-skill correctness ### Fixed diff --git a/README.md b/README.md index 45682e9..0708281 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,7 @@ # Bob Control [![CI](https://github.com/PounceAI/bob-control/actions/workflows/ci.yml/badge.svg)](https://github.com/PounceAI/bob-control/actions/workflows/ci.yml) +[![MCP](https://lobehub.com/badge/mcp/pounceai-bob-control)](https://lobehub.com/mcp/pounceai-bob-control) Bob Control turns an AI coding agent from a one-prompt-at-a-time chat window into an unattended queue worker. Fill a board and each task gets dispatched, auto-approved inside risk guardrails, @@ -40,6 +41,23 @@ npm run build # tsc -> dist/, then bundles claude-plugin/server/server.mjs npm run smoke # optional self-test ``` +Or skip the clone and run the published package — the MCP server (`bob-control`) and CLI +(`bob-tasks`) come straight from npm. Point an MCP client at it instead of an absolute `dist/` path: + +```jsonc +{ + "mcpServers": { + "bob-tasks": { "type": "stdio", "command": "npx", "args": ["-y", "@pounceai/bob-control"] } + } +} +``` + +Under Claude Code the board resolves automatically (it sets `CLAUDE_PROJECT_DIR`); any other MCP +client should add `"env": { "BOB_TASKS_DB": "…" }` pointing at the board the worker drains — a bare +`npx` server with no board env writes to a throwaway path inside the npx cache that nothing can share. + +The unattended **worker** and the Claude Code **plugin** still want the repo (see below). + Needs Node 22.5+ (uses the built-in `node:sqlite`, so there's no native build step). Board path resolution: `BOB_TASKS_DB` (explicit) › `BOB_TASKS_PORTABLE=1` (a shared `~/.bob-tasks/tasks.db`) › `BOB_TASKS_WORKTREE_SHARED=1` (every linked git worktree resolves the diff --git a/claude-plugin/.claude-plugin/plugin.json b/claude-plugin/.claude-plugin/plugin.json index 628e4b0..049ba1c 100644 --- a/claude-plugin/.claude-plugin/plugin.json +++ b/claude-plugin/.claude-plugin/plugin.json @@ -2,7 +2,7 @@ "name": "bob-companion", "displayName": "Bob Companion", "description": "Use Claude Code from any repo as the foreman and worker for the IBM Bob task board: provision, route, triage, and drain tasks Bob shares. Ships a self-contained MCP server.", - "version": "2.0.2", + "version": "2.1.0", "author": { "name": "Joshua Gilbert" }, diff --git a/package.json b/package.json index 9fef3c5..7de8329 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { - "name": "bob-control", - "version": "2.0.2", + "name": "@pounceai/bob-control", + "version": "2.1.0", "description": "Bob Control: MCP server + CLI + worker that runs IBM Bob (and any MCP-capable agent) unattended against a SQLite task board.", "author": "Joshua Gilbert", "license": "Apache-2.0", @@ -18,9 +18,16 @@ ], "type": "module", "bin": { + "bob-control": "dist/server.js", "bob-tasks": "dist/cli.js" }, + "files": ["dist/**/*.js", "!dist/**/*.test.js", "!dist/ipc-test-harness.js", "!dist/smoke.js"], + "publishConfig": { + "access": "public" + }, "scripts": { + "prepublishOnly": "npm run build && npm run check:shebang", + "check:shebang": "node -e \"process.exit(['dist/server.js','dist/cli.js'].every(f => require('fs').readFileSync(f,'utf8').startsWith('#!/usr/bin/env node')) ? 0 : 1)\"", "build": "tsc && npm run build:plugin", "build:plugin": "esbuild src/server.ts --bundle --platform=node --format=esm --target=node22 --outfile=claude-plugin/server/server.mjs", "server": "node dist/server.js", diff --git a/src/bob-polls.test.ts b/src/bob-polls.test.ts index af6b7b4..88308aa 100644 --- a/src/bob-polls.test.ts +++ b/src/bob-polls.test.ts @@ -1,6 +1,17 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { createPollLoop, type PollDeps, type PollResult, type VerifyResult } from "./bob-polls.js"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + createPollLoop, + defaultCaptureSnapshot, + defaultCheckDidWork, + type PollDeps, + type PollResult, + type VerifyResult, +} from "./bob-polls.js"; // A recording harness: a poll loop wired to fakes that capture each continue // dispatch, log line, and note, plus a stub verifier the test controls. A continue @@ -517,3 +528,94 @@ test("plan-stop detection: combines with verify-and-continue correctly", async ( assert.match(h.dispatchArgs[0], /presented a plan/); assert.match(h.dispatchArgs[1], /did NOT pass verification/); }); + +// ── defaultCaptureSnapshot: untracked-aware (real git) ──────────────────────────────────────────── +// Plan-stop shares the verifier's blind spot: a `git status`/`git diff` snapshot can't see an edit to +// a still-untracked file, so it read as "no new work". The tree-sha snapshot changes on the edit. + +function gitRepo(): string { + const dir = mkdtempSync(join(tmpdir(), "bobpolls-snap-")); + const run = (...args: string[]) => spawnSync("git", args, { cwd: dir }); + run("init", "-q"); + run("config", "user.email", "t@t.t"); + run("config", "user.name", "t"); + run("config", "commit.gpgsign", "false"); + writeFileSync(join(dir, "README.md"), "# repo\n"); + run("add", "-A"); + run("commit", "-qm", "init"); + return dir; +} + +test("defaultCaptureSnapshot: an edit to a still-untracked file changes the snapshot", async () => { + const dir = gitRepo(); + try { + writeFileSync(join(dir, "scratch.py"), "x = 1\n"); // untracked before the task + const before = await defaultCaptureSnapshot(dir); + assert.notEqual(before, "GIT_ERROR"); + + writeFileSync(join(dir, "scratch.py"), "x = 1\ny = 2\n"); // task edits the still-untracked file + const after = await defaultCaptureSnapshot(dir); + assert.notEqual(after, "GIT_ERROR"); + assert.notEqual(after, before, "editing an untracked file must change the snapshot"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("defaultCaptureSnapshot: an unchanged tree yields a stable snapshot", async () => { + const dir = gitRepo(); + try { + writeFileSync(join(dir, "scratch.py"), "x = 1\n"); + const a = await defaultCaptureSnapshot(dir); + const b = await defaultCaptureSnapshot(dir); + assert.equal(a, b, "no change → identical snapshot (no false plan-stop)"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("defaultCaptureSnapshot: a non-git cwd yields the GIT_ERROR sentinel", async () => { + const snap = await defaultCaptureSnapshot(join(tmpdir(), "definitely-not-a-git-repo-xyz")); + assert.equal(snap, "GIT_ERROR"); +}); + +// ── defaultCheckDidWork: the plan-stop verdict (real git) ───────────────────────────────────────── +// Direct coverage for the verdict itself, not just the snapshot: an inverted current===baseline check +// or a mis-ordered GIT_ERROR guard would otherwise ship green (the loop tests inject their own stub). + +test("defaultCheckDidWork: an unchanged tree since baseline → didWork false (plan-stop)", async () => { + const dir = gitRepo(); + try { + writeFileSync(join(dir, "scratch.py"), "x = 1\n"); + const baseline = await defaultCaptureSnapshot(dir); + const res = await defaultCheckDidWork(dir, baseline); + assert.equal(res.didWork, false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("defaultCheckDidWork: an edit to a still-untracked file since baseline → didWork true", async () => { + const dir = gitRepo(); + try { + writeFileSync(join(dir, "scratch.py"), "x = 1\n"); + const baseline = await defaultCaptureSnapshot(dir); + writeFileSync(join(dir, "scratch.py"), "x = 1\ny = 2\n"); + const res = await defaultCheckDidWork(dir, baseline); + assert.equal(res.didWork, true); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("defaultCheckDidWork: a GIT_ERROR baseline is treated as work done (never a false plan-stop)", async () => { + const dir = gitRepo(); + try { + // Both sides can read GIT_ERROR; the sentinel guard must win over the equality check, else two + // GIT_ERRORs would compare equal and wrongly report "no work". + const res = await defaultCheckDidWork(dir, "GIT_ERROR"); + assert.equal(res.didWork, true); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); diff --git a/src/bob-polls.ts b/src/bob-polls.ts index aeffcbc..7b3bdb2 100644 --- a/src/bob-polls.ts +++ b/src/bob-polls.ts @@ -8,6 +8,7 @@ import { spawn } from "node:child_process"; import type { DispatchResult } from "./bob-ipc.js"; +import { snapshotWorktreeTreeBounded } from "./git.js"; /** * The poll loop reads and passes a dispatch result straight through, so it IS a DispatchResult @@ -106,56 +107,13 @@ export async function defaultVerify( } /** - * Capture a content-aware snapshot of the working tree state. - * Combines `git status --porcelain` (new/deleted/modified files) with `git diff HEAD` - * (actual content changes in tracked files). This allows detecting changes even when - * the tree is already dirty from prior tasks. - * - * Known limitation: editing an UNTRACKED file that a prior task created is NOT detected - * (porcelain shows ?? for both snapshots, diff omits untracked content). This is acceptable - * because untracked files are typically intermediate artifacts, not the primary deliverable. + * Working-tree snapshot as a single untracked-aware tree sha: two shas differ iff content changed — + * including edits to files that stay untracked, which a `git status`/`git diff` pair misses (status + * shows the same `??`, diff omits untracked content). null (non-git, git failure, timeout) → GIT_ERROR, + * which checkDidWork reads as "assume work done" over a false plan-stop. */ export async function defaultCaptureSnapshot(cwd: string, timeoutMs = 30_000): Promise { - return new Promise((resolve) => { - // Run both commands in parallel for efficiency - const statusProc = spawn("git", ["status", "--porcelain"], { cwd, stdio: "pipe" }); - const diffProc = spawn("git", ["diff", "HEAD"], { cwd, stdio: "pipe" }); - - let statusOut = ""; - let diffOut = ""; - let completed = 0; - let settled = false; - - // Single idempotent exit: clears the timer, kills both children, resolves once. A git that hangs - // (a credential prompt, an index.lock) is bounded by the timeout → GIT_ERROR instead of forever. - const finish = (s: string) => { - if (settled) return; - settled = true; - clearTimeout(timer); - for (const p of [statusProc, diffProc]) { - try { - p.kill(); - } catch { - /* already exited */ - } - } - resolve(s); - }; - const timer = setTimeout(() => finish("GIT_ERROR"), timeoutMs); - timer.unref?.(); - - const checkComplete = () => { - if (++completed === 2) finish(`STATUS:\n${statusOut}\nDIFF:\n${diffOut}`); - }; - - statusProc.stdout?.on("data", (chunk: Buffer) => (statusOut += chunk.toString())); - statusProc.on("close", (code: number | null) => (code !== 0 ? finish("GIT_ERROR") : checkComplete())); - statusProc.on("error", () => finish("GIT_ERROR")); - - diffProc.stdout?.on("data", (chunk: Buffer) => (diffOut += chunk.toString())); - diffProc.on("close", (code: number | null) => (code !== 0 ? finish("GIT_ERROR") : checkComplete())); - diffProc.on("error", () => finish("GIT_ERROR")); - }); + return (await snapshotWorktreeTreeBounded(cwd, timeoutMs)) ?? "GIT_ERROR"; } /** @@ -163,7 +121,7 @@ export async function defaultCaptureSnapshot(cwd: string, timeoutMs = 30_000): P * Returns didWork=false if the snapshot is unchanged (no new changes since baseline), * meaning Bob likely just presented a plan without implementing it. */ -async function defaultCheckDidWork(cwd: string, baseline: string): Promise { +export async function defaultCheckDidWork(cwd: string, baseline: string): Promise { const current = await defaultCaptureSnapshot(cwd); // Git command failed: conservatively assume work happened. @@ -171,25 +129,10 @@ async function defaultCheckDidWork(cwd: string, baseline: string): Promise 0) { - return { didWork: true, reason: `${delta} new file change${delta === 1 ? "" : "s"} detected` }; - } else if (delta < 0) { - return { didWork: true, reason: `${-delta} file change${delta === -1 ? "" : "s"} resolved` }; - } else { - return { didWork: true, reason: "file content changed" }; - } + return { didWork: true, reason: "working tree changed since baseline" }; } /** diff --git a/src/checkpoint.test.ts b/src/checkpoint.test.ts index e287d55..a425e28 100644 --- a/src/checkpoint.test.ts +++ b/src/checkpoint.test.ts @@ -12,7 +12,7 @@ import { preserveWipToBranch, releaseCheckpoint, } from "./checkpoint.js"; -import { snapshotWorktreeTree } from "./git.js"; +import { snapshotWorktreeTree, snapshotWorktreeTreeBounded } from "./git.js"; import { getDb, createTask, setCheckpoint, getCheckpoint, clearCheckpoint, getNotes, recordArtifact } from "./db.js"; function git(dir: string, ...args: string[]): string { @@ -202,6 +202,36 @@ describe("checkpoint capture + restore (real git)", () => { ); rm(dir); }); + + it("snapshotWorktreeTreeBounded returns the same tree as the unbounded call under a generous timeout", async () => { + const dir = makeRepo(); + commit(dir, "tracked.txt", "v1"); + writeFileSync(join(dir, "untracked.txt"), "new"); + const [bounded, unbounded] = await Promise.all([ + snapshotWorktreeTreeBounded(dir, 30_000), + snapshotWorktreeTree(dir), + ]); + assert.ok(bounded && /^[0-9a-f]{40}$/.test(bounded)); + assert.equal(bounded, unbounded, "bounded wrapper yields the identical snapshot when git is fast"); + rm(dir); + }); + + it("snapshotWorktreeTreeBounded resolves null on a non-git cwd (and never throws)", async () => { + const dir = mkdtempSync(join(tmpdir(), "bob-nogit-")); + assert.equal(await snapshotWorktreeTreeBounded(dir, 30_000), null); + rm(dir); + }); + + it("snapshotWorktreeTree: an aborted signal yields null and leaves no temp index behind", async () => { + const dir = makeRepo(); + commit(dir, "tracked.txt", "v1"); + const ac = new AbortController(); + ac.abort(); // stand in for a bounded-caller timeout: the git children must be killed, not orphaned + assert.equal(await snapshotWorktreeTree(dir, ac.signal), null); + const leftovers = readdirSync(join(dir, ".git")).filter((f) => f.startsWith("bob-tmp-index-")); + assert.deepEqual(leftovers, [], "the temp index must be cleaned up even when git is aborted"); + rm(dir); + }); }); describe("checkpoint orchestration + persistence (db)", () => { diff --git a/src/git.ts b/src/git.ts index 1f41880..fee39a6 100644 --- a/src/git.ts +++ b/src/git.ts @@ -20,11 +20,19 @@ export interface GitResult { * set the process is killed and output truncated once the limit is exceeded; in that case * `truncated` is true and `ok` only means "stopped deliberately", not "git succeeded". */ -export function runGit(args: string[], cwd: string, maxChars?: number, env?: NodeJS.ProcessEnv): Promise { +export function runGit( + args: string[], + cwd: string, + maxChars?: number, + env?: NodeJS.ProcessEnv, + signal?: AbortSignal, +): Promise { return new Promise((resolve) => { let proc; try { - proc = spawn("git", args, { cwd, stdio: "pipe", env: env ? { ...process.env, ...env } : undefined }); + // signal: when aborted, spawn kills the child (SIGTERM on POSIX, TerminateProcess on Windows) + // and emits 'error', so a bounded caller can unwedge a hung git and let cleanup run. + proc = spawn("git", args, { cwd, stdio: "pipe", env: env ? { ...process.env, ...env } : undefined, signal }); } catch { return resolve({ ok: false, truncated: false, stdout: "" }); } @@ -49,8 +57,14 @@ export function runGit(args: string[], cwd: string, maxChars?: number, env?: Nod } /** Convenience: stdout only (for callers that don't care whether git succeeded). */ -export async function gitOut(args: string[], cwd: string, maxChars?: number, env?: NodeJS.ProcessEnv): Promise { - return (await runGit(args, cwd, maxChars, env)).stdout; +export async function gitOut( + args: string[], + cwd: string, + maxChars?: number, + env?: NodeJS.ProcessEnv, + signal?: AbortSignal, +): Promise { + return (await runGit(args, cwd, maxChars, env, signal)).stdout; } export function splitLines(s: string): string[] { @@ -100,15 +114,13 @@ export async function listUntracked(cwd: string): Promise { let tmpIndexSeq = 0; /** - * Snapshot the current worktree — tracked changes AND untracked (non-ignored) files — into a git - * tree object, returning its sha WITHOUT touching the real index. Stages into a throwaway TEMP index - * (`add -A` → `write-tree`), so callers get an untracked-aware snapshot that `git stash create` - * can't produce (it silently drops untracked files). `write-tree` persists the tree in the object - * DB, so the returned sha stays valid after the temp index is removed. Returns null when cwd isn't a - * git work tree or the snapshot can't be built. The temp index (and any leftover lock) is always - * cleaned up; never throws. + * Snapshot the worktree — tracked changes AND untracked (non-ignored) files — as a git tree sha, + * WITHOUT touching the real index: stages into a throwaway TEMP index (`add -A` → `write-tree`), so + * unlike `git stash create` (which drops untracked files) the snapshot is untracked-aware. The sha + * outlives the temp index (write-tree persists it). null on non-git / failure; temp index + lock are + * always cleaned up; never throws. `signal` lets a bounded caller abort a wedged add/write-tree. */ -export async function snapshotWorktreeTree(cwd: string): Promise { +export async function snapshotWorktreeTree(cwd: string, signal?: AbortSignal): Promise { // --absolute-git-dir needs git ≥2.13; fall back to the always-present --git-dir (possibly // relative) so an older git still produces a snapshot instead of silently giving up. let gitDir = (await gitOut(["rev-parse", "--absolute-git-dir"], cwd)).trim(); @@ -120,10 +132,10 @@ export async function snapshotWorktreeTree(cwd: string): Promise const tmpIndex = resolve(gitDir, `bob-tmp-index-${process.pid}-${Date.now()}-${tmpIndexSeq++}`); const env = { GIT_INDEX_FILE: tmpIndex }; try { - // add -A stages adds + modifications + deletions relative to the empty temp index → a faithful - // snapshot of what's on disk now (still honoring .gitignore). - if (!(await runGit(["add", "-A"], cwd, undefined, env)).ok) return null; - return (await gitOut(["write-tree"], cwd, undefined, env)).trim() || null; + // add -A stages adds + mods + deletions into the empty temp index → a faithful on-disk snapshot + // (honoring .gitignore); signal kills a wedged child so a hang doesn't orphan a process + index. + if (!(await runGit(["add", "-A"], cwd, undefined, env, signal)).ok) return null; + return (await gitOut(["write-tree"], cwd, undefined, env, signal)).trim() || null; } finally { for (const f of [tmpIndex, `${tmpIndex}.lock`]) { try { @@ -134,3 +146,27 @@ export async function snapshotWorktreeTree(cwd: string): Promise } } } + +/** + * snapshotWorktreeTree under a timeout: on timeout it aborts the git children (a wedged clean/smudge + * filter or network FS — `index.lock` just fails fast), so the hang is killed, its temp index cleaned + * up, and null returned. Only a child that ignores the kill and stays wedged can still leak. + */ +export async function snapshotWorktreeTreeBounded(cwd: string, timeoutMs = 30_000): Promise { + const controller = new AbortController(); + let timer: ReturnType | undefined; + const timeout = new Promise((resolve) => { + timer = setTimeout(() => { + // stderr trail so a slow-git env (the judge silently seeing "no changes") is diagnosable. + console.error(`[bob-control] git worktree snapshot timed out after ${timeoutMs}ms in ${cwd}`); + controller.abort(); // kill the git children so snapshotWorktreeTree's finally drops the temp index + resolve(null); + }, timeoutMs); + timer.unref?.(); + }); + try { + return await Promise.race([snapshotWorktreeTree(cwd, controller.signal), timeout]); + } finally { + clearTimeout(timer); + } +} diff --git a/src/judge.test.ts b/src/judge.test.ts index 0e2ac49..151a523 100644 --- a/src/judge.test.ts +++ b/src/judge.test.ts @@ -1,6 +1,7 @@ import { test } from "node:test"; import { strict as assert } from "node:assert"; -import { mkdtempSync, rmSync } from "node:fs"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { @@ -264,6 +265,85 @@ test("captureGitDiff: skips untracked files listed in priorUntracked", async () assert.equal(diff, "(no changes detected)"); }); +// ── captureGitDiff: untracked-aware tree diff (real git) ────────────────────────────────────────── +// Regression: a plain `git diff` can't see an edit to a file that was already untracked when the task +// started, so the judge read "no changes" and aborted good work. The tree-vs-tree diff surfaces it. + +function gitRepo(): string { + const dir = mkdtempSync(join(tmpdir(), "judge-gitdiff-")); + const run = (...args: string[]) => spawnSync("git", args, { cwd: dir }); + run("init", "-q"); + run("config", "user.email", "t@t.t"); + run("config", "user.name", "t"); + run("config", "commit.gpgsign", "false"); + writeFileSync(join(dir, "README.md"), "# repo\n"); + run("add", "-A"); + run("commit", "-qm", "init"); + return dir; +} + +test("captureGitDiff: surfaces edits to a file that was already untracked at baseline", async () => { + const dir = gitRepo(); + try { + // A brand-new file created BEFORE the task — untracked, never `git add`ed (the repro). + writeFileSync(join(dir, "foo.py"), "original = 1\n"); + const baseline = await captureGitBaseline(dir); + assert.ok(baseline.tree, "baseline should capture an untracked-aware tree"); + assert.ok(baseline.untracked.includes("foo.py"), "foo.py predates the task → priorUntracked"); + + // The task edits the still-untracked file. + writeFileSync(join(dir, "foo.py"), "original = 1\nadded_by_task = 2\n"); + + const diff = await captureGitDiff(dir, 4000, baseline.ref, baseline.untracked, baseline.tree); + assert.match(diff, /foo\.py/, "diff names the edited file"); + assert.match(diff, /added_by_task/, "diff shows the task's new content"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("captureGitDiff: an untouched prior-untracked file is NOT reported as changed", async () => { + const dir = gitRepo(); + try { + writeFileSync(join(dir, "artifact.log"), "pre-existing scratch\n"); + const baseline = await captureGitBaseline(dir); + // The task touches nothing: the two tree snapshots are identical → no diff. + const diff = await captureGitDiff(dir, 4000, baseline.ref, baseline.untracked, baseline.tree); + assert.equal(diff, "(no changes detected)"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("captureGitDiff: surfaces a file the task newly creates", async () => { + const dir = gitRepo(); + try { + const baseline = await captureGitBaseline(dir); + writeFileSync(join(dir, "new.py"), "fresh = True\n"); + const diff = await captureGitDiff(dir, 4000, baseline.ref, baseline.untracked, baseline.tree); + assert.match(diff, /new\.py/); + assert.match(diff, /fresh/); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("captureGitDiff: a failed tree diff falls through to the ref diff, not a false 'no changes'", async () => { + const dir = gitRepo(); + try { + const baseline = await captureGitBaseline(dir); + writeFileSync(join(dir, "work.py"), "did = 'work'\n"); // real work after baseline + // A well-formed but unresolvable tree sha makes `git diff ` exit non-zero with + // empty stdout. The fix must gate on git's exit code and fall through to the ref-based diff + // rather than reporting the git error as "(no changes detected)". + const diff = await captureGitDiff(dir, 4000, baseline.ref, baseline.untracked, "0".repeat(40)); + assert.notEqual(diff, "(no changes detected)"); + assert.match(diff, /work\.py/); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + test("parseVerdict: reason containing a brace is extracted whole (balanced scan)", () => { // A `\{[^{}]*\}` regex would stop at the inner '{' and miss this object, falling // through to the token scan. The balanced scanner extracts it intact. diff --git a/src/judge.ts b/src/judge.ts index e993627..c82d1b9 100644 --- a/src/judge.ts +++ b/src/judge.ts @@ -3,7 +3,7 @@ // Designed to be testable: the LLM call is injected, and the verdict parsing is pure. import { callModel, type LlmDeps } from "./llm.js"; import { resolve as resolvePath } from "node:path"; -import { gitOut, splitLines, isInsideWorkTree, listUntracked } from "./git.js"; +import { gitOut, runGit, splitLines, isInsideWorkTree, listUntracked, snapshotWorktreeTreeBounded } from "./git.js"; import { extractJsonObjects } from "./json-extract.js"; import { defaultVerify, type VerifyResult } from "./bob-polls.js"; import { judgeAppliesToMode } from "./modes.js"; @@ -30,6 +30,9 @@ export interface GitBaseline { ref: string; /** Untracked files that already existed before the task (so new files can be told apart). */ untracked: string[]; + /** Untracked-aware tree sha of the pre-task worktree, so the diff can see content edits to files + * that stay untracked (plain `git diff` omits untracked content). Undefined when not a git tree. */ + tree?: string; } /** Backend + model + transport overrides (see llm.ts). */ @@ -58,7 +61,7 @@ function userContent(ctx: JudgeContext): string { "AGENT'S COMPLETION STATEMENT:", ctx.completionResult, "", - "ACTUAL CHANGES (git diff HEAD):", + "ACTUAL CHANGES (working-tree diff since the task started):", ctx.gitDiff || "(no changes detected)", ].join("\n"); } @@ -133,23 +136,40 @@ export function parseVerdict(text: string): JudgeVerdict { export async function captureGitBaseline(cwd: string): Promise { const ref = (await gitOut(["stash", "create"], cwd)).trim() || "HEAD"; const untracked = await listUntracked(cwd); - return { ref, untracked }; + // Untracked-aware tree so captureGitDiff diffs untracked CONTENT, not just presence; bounded + // against a wedged git. + const tree = (await snapshotWorktreeTreeBounded(cwd)) ?? undefined; + return { ref, untracked, tree }; } /** - * Capture a bounded diff of THIS task's changes for the judge's ground truth. - * Diffs the working tree against `baselineRef` (a real ref/SHA, default HEAD, so - * pre-existing tracked changes are excluded), and temporarily marks task-created - * untracked files as intent-to-add so new files appear in the diff. Files in - * `priorUntracked` are skipped (they predate the task), and the intent-to-add marks - * are reset in a finally block so the user's index is left exactly as it was found. + * Bounded diff of THIS task's changes — the judge's ground truth. + * + * With `baselineTree`: tree-vs-fresh-snapshot diff. Both stage untracked files, so edits that stay + * untracked surface and pre-existing dirt cancels — which plain `git diff` can't do. Blind spot: + * `add -A` honors `.gitignore`, so an ignored-path deliverable is invisible. + * Without one (non-git / git too old): diff against `baselineRef`, intent-to-adding task-created + * untracked files (not in `priorUntracked`) so they appear; marks reset in a finally. */ export async function captureGitDiff( cwd: string, maxChars = 4000, baselineRef = "HEAD", priorUntracked: string[] = [], + baselineTree?: string, ): Promise { + if (baselineTree) { + const currTree = await snapshotWorktreeTreeBounded(cwd); + if (currTree) { + // Gate on `ok`: a failed diff (unresolvable baselineTree) has empty stdout that gitOut can't + // tell from a clean tree. Truncation counts as ok (deliberate stop, valid partial diff). + const res = await runGit(["diff", baselineTree, currTree], cwd, maxChars); + if (res.ok) return res.stdout || "(no changes detected)"; + } + // Tree path failed (snapshot or diff) — degrade to the ref diff, and leave a trail: the two paths + // differ exactly on untracked-content edits, so a silent fallback hides why the judge saw less. + console.error(`[bob-control] captureGitDiff: tree path unavailable in ${cwd}, using ref-based diff`); + } const prior = new Set(priorUntracked); const newFiles = (await listUntracked(cwd)).filter((f) => !prior.has(f)); if (newFiles.length) await gitOut(["add", "--intent-to-add", "--", ...newFiles], cwd); @@ -241,12 +261,13 @@ export function buildJudgeVerifier( if (!judgeAppliesToMode(deps.mode)) return undefined; const ref = deps.evidenceBaseline?.ref ?? "HEAD"; const priorUntracked = deps.evidenceBaseline?.untracked ?? []; + const baselineTree = deps.evidenceBaseline?.tree; return async (result, command, cwd): Promise => { if (command) { const cmd = await defaultVerify(result, command, cwd); if (!cmd.passed) return cmd; // command failed → short-circuit, skip the judge } - const gitDiff = await captureGitDiff(cwd, 4000, ref, priorUntracked); + const gitDiff = await captureGitDiff(cwd, 4000, ref, priorUntracked, baselineTree); const verdict = await judgeCompletion( { taskPrompt: deps.taskPrompt, completionResult: result, gitDiff }, deps.judge, diff --git a/src/server.ts b/src/server.ts index bfcb68b..1d9d0f7 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1,3 +1,4 @@ +#!/usr/bin/env node import "./suppress-warnings.js"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; @@ -10,8 +11,6 @@ import { revertTaskToCheckpoint, deleteTaskAndCheckpoint } from "./checkpoint.js import { buildReport } from "./report.js"; import { awaitTaskOutcome } from "./await-task.js"; -const WORKER_ACTIVE_WINDOW_MS = 5 * 60 * 1000; - // Max live tasks board_status inlines as open_tasks before it flags open_tasks_truncated. const OPEN_TASKS_CAP = 50; @@ -22,14 +21,15 @@ const AWAIT_CHUNK_MAX_MS = 55_000; const AWAIT_POLL_INTERVAL_MS = 700; const sleep = (ms: number): Promise => new Promise((r) => setTimeout(r, ms)); -/** Heuristic: a drainer looks active if any task is in_progress with a recent touch. (A - * needs_input task means a worker is WAITING on a human, not actively draining — and the - * human answerer bumps updated_at too — so it is deliberately NOT counted as active.) */ -function workerLikelyActive(): boolean { - const now = Date.now(); - return repo - .listTasks({ status: "in_progress" }) - .some((t) => now - Date.parse(t.updated_at) < WORKER_ACTIVE_WINDOW_MS); +/** Would a live drainer pull a task carrying `taskTags` right now? A worker pulls a task only if its + * --tag pin is null (drains all tags) or matches one of the task's tags. Heartbeat-based, so — unlike + * the old in-progress heuristic — it fires for a live-but-idle drainer, which is exactly the + * mid-curation race the create_task warning guards against. */ +function liveDrainerWouldPull(taskTags: string[] = []): boolean { + const live = repo.getWorkerLiveness(); + if (!live.draining) return false; + const wanted = new Set(taskTags); + return live.tags.some((pin) => pin === null || wanted.has(pin)); } // Bob Control MCP server. Bob connects and gets tools to pull, claim, @@ -41,7 +41,7 @@ function workerLikelyActive(): boolean { const server = new McpServer({ name: "bob-control", - version: "1.1.0", + version: "2.1.0", }); type ToolResult = { @@ -107,12 +107,12 @@ server.registerTool( try { const task = repo.createTask({ title, description, priority, tags, mode, depends_on, staged }); // Warn when a pullable task drops onto a live board (bulk-create race). - if (!staged && repo.isBoardArmed() && workerLikelyActive()) { + if (!staged && repo.isBoardArmed() && liveDrainerWouldPull(task.tags)) { return json({ ...task, warning: - "Board is ARMED and a worker looks active — this task may be pulled before you finish curating. " + - "Disarm the board (disarm_board) or create staged:true while bulk-creating, then release_tasks.", + "Board is ARMED and a live worker drains this task's tags — it may be pulled before you finish " + + "curating. Disarm the board (disarm_board) or create staged:true while bulk-creating, then release_tasks.", }); } return json(task); @@ -616,7 +616,7 @@ server.registerTool( title: "Board Status", description: "Dispatch state, counts, and the live task list: whether the board is `armed`, task `counts` by " + - "status, whether a worker looks active, whether a drainer is currently servicing the board " + + "status, whether a drainer is currently servicing the board " + "(`worker_draining` — a live heartbeat from either a 1.x worker process or the 2.0 in-process " + "loop, with `.tags` = the --tag each live worker drains, null = an unfiltered worker that drains " + "all tags), `worker_leases` (which checkout each live worker owns), " + @@ -637,7 +637,6 @@ server.registerTool( const { open_tasks, truncated } = repo.selectOpenTasks(tasks, OPEN_TASKS_CAP); return json({ armed: repo.isBoardArmed(), - worker_likely_active: workerLikelyActive(), worker_draining: repo.getWorkerLiveness(), worker_leases: repo.getWorkerLeases(), // T7: which worktree each live worker owns counts: repo.countByStatus(tasks),