From 6bb92cec7481f9c75c2342c091d48f361c4751e8 Mon Sep 17 00:00:00 2001 From: Rach Pradhan <54503978+justrach@users.noreply.github.com> Date: Thu, 6 Aug 2026 13:46:37 +0800 Subject: [PATCH 1/2] feat(goal): skip a re-verify the workspace cannot have changed (#412) A /goal (or /loop) run is plan-act-VERIFY and re-verifies on every continuation, so the obvious move for a model that has just been told "completion is blocked, run eval" is to run eval again. When it has edited nothing since the last RED, that re-run costs the whole --eval command, a --judge model call, and 1500 bytes of output tail to re-derive a verdict that could not have changed. verify_fingerprint.zig fingerprints the verification before running it, and a fingerprint identical to the one the last verification FAILED on means the verifier is not run at all: - the fold covers every input to "could this produce a different result": the eval command text, `git status --porcelain -z -uall`, `git diff --binary HEAD`, and the CONTENTS of every untracked file the status listed. The last one is load-bearing - editing an untracked file leaves the porcelain line and the diff byte-identical, so without it the guard would skip a verify over real work. Parts are length-prefixed, so a byte moved across a file boundary is a change, not a collision. - the attempt still counts (eval_iter advances). A model that keeps calling eval without editing has to converge on the iteration cap, not spin for free. - the steer names the actual blocker instead of manufacturing a verdict: the workspace has not changed since the last failed verification, edit source files or tests first. Completion stays blocked either way, since a skipped run verifies nothing. FAIL-OPEN everywhere: no repo, no git, a timed-out probe, a truncated stream, an unreadable file, an absurd untracked count - each yields an unknown fingerprint, which never matches, so the verifier runs. The guard can cost a skipped re-run only when it is certain nothing moved; it can never invent a pass. A command that could not RUN disarms it too - "edit source files" is not the fix for that, and a stale fingerprint must not suppress the retry. The fold and the decision are pure and unit-tested without a repo (identical / tracked change / untracked-only change / boundary collision / fail-open matrix). The end-to-end test drives the real runEval against a real single-commit fixture repo and counts VERIFIER INVOCATIONS: six attempts, four spawns, the two no-progress ones free, and a tracked edit, a new untracked file and an edit to that untracked file each re-arm it. Co-Authored-By: Codegraff --- CHANGELOG.md | 18 ++- src/agent.zig | 1 + src/agent_eval.zig | 25 ++++ src/agent_eval_tests.zig | 111 +++++++++++++++ src/test_hooks.zig | 5 + src/verify_fingerprint.zig | 282 +++++++++++++++++++++++++++++++++++++ 6 files changed, 441 insertions(+), 1 deletion(-) create mode 100644 src/verify_fingerprint.zig diff --git a/CHANGELOG.md b/CHANGELOG.md index b2f8415b..2280c316 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,23 @@ The release workflow uses a tag's section here as its release notes (a hand-written `docs/releases/.md` wins if present), so keeping this file current is part of cutting a release. -## v0.0.240 (unreleased) +## v0.0.241 (unreleased) + +- A no-progress `eval` is no longer paid for (#412). A goal/loop run + re-verifies on every continuation, so a model that has edited nothing since + the last RED re-ran the whole `--eval` command — plus a `--judge` model call, + plus 1500 bytes of output tail — to re-derive a verdict that could not have + changed. graff now fingerprints the verification before running it (the eval + command text, `git status --porcelain -z -uall`, `git diff --binary HEAD`, + and the contents of every untracked file, since the first two are blind to an + untracked edit); identical to the tree the last verification failed on means + the verifier is not run at all. The attempt still counts, so a stuck loop + converges on its iteration cap instead of spinning for free, and the model is + steered at the real blocker: the workspace has not changed, edit something + first. Fail-open throughout — no repo, no git, a timed-out or truncated + probe, an unreadable file all read as "changed" and the verifier runs. + +## v0.0.240 (2026-08-06) - The REPL/engine separation began (#422): agent output now flows through a typed event vocabulary and a strict sink boundary (`engine_events.zig` / diff --git a/src/agent.zig b/src/agent.zig index 2693a4c6..36516e80 100644 --- a/src/agent.zig +++ b/src/agent.zig @@ -165,6 +165,7 @@ pub const Agent = struct { eval_verified: bool = false, // latest workspace state has a target-meeting verifier result eval_repair_pending: bool = false, // a contradiction blocks completion until a fresh green eval eval_repair_grants: u8 = 0, // RED continuations consumed (agent_steps.grantRepairTurn); reset by any green eval, capped by eval_control.max_repair_grants + eval_fp: ?[32]u8 = null, // #412: worktree fingerprint at the last RED eval (verify_fingerprint.Digest); an identical tree skips the re-verify instead of paying for it strict: bool = false, completed: ?[]const u8 = null, last_context_tokens: u64 = 0, diff --git a/src/agent_eval.zig b/src/agent_eval.zig index 16c963de..e7bb03ad 100644 --- a/src/agent_eval.zig +++ b/src/agent_eval.zig @@ -28,6 +28,7 @@ const playbook_reflect = @import("playbook_reflect.zig"); // #383 Reflector: one const orch_rows = @import("orchestration_rows.zig"); // the orchestration outcome rides the same score funnel const shapes = @import("shapes.zig"); const failure_evidence = @import("failure_evidence.zig"); // a RED verdict is parked as escalation evidence +const verify_fingerprint = @import("verify_fingerprint.zig"); // #412 no-progress guard: an unmoved worktree is not re-verified test { _ = @import("agent_eval_tests.zig"); @@ -43,6 +44,21 @@ pub fn runEval(self: *Agent, note: []const u8) !ExecResult { .is_error = true, }; + // #412 no-progress guard. The tree the verifier is about to run against is + // fingerprinted BEFORE the command runs (the command itself may write), and + // when the last verdict was RED over a byte-identical tree the verifier is + // not run at all: nothing it could report can have changed, so the whole + // scoring command, its judge model call and its output tail are saved and + // the model is steered at the real blocker instead. The attempt still + // counts - a model that keeps calling eval without editing has to converge + // on the iteration cap, not spin for free. Unknown (no repo, git error, + // truncated probe) never matches, so the guard fails open. + const tree_fp = verify_fingerprint.capture(self.gpa, self.io, cmd); + if (verify_fingerprint.skipReverify(self.eval_repair_pending, self.eval_fp, tree_fp)) { + self.eval_iter += 1; + return .{ .text = try verify_fingerprint.noProgressText(self.arena, self.eval_iter), .is_error = true }; + } + // Behavioral commitment (issue #256): the eval-driven loop is the first // production caller of turn_committed/model_mispredicted. The commitment // asserts the loop's own belief - the command will meet the target - @@ -64,6 +80,11 @@ pub fn runEval(self: *Agent, note: []const u8) !ExecResult { if (behavior) |bt| bt.recordMisprediction(eval_turn, commitment_id, .{ .pass = true }, .{ .pass = false, .exit = @as(i32, -1) }, "eval command could not run"); self.eval_verified = false; self.eval_repair_pending = true; + // #412: a command that never ran is not a verdict about the workspace, + // and "edit source files" is not the fix for it - disarm the guard so + // the next call really does try the command again (and so a fingerprint + // left by an earlier RED cannot suppress it). + self.eval_fp = null; eval_memory.record(self, note, null, -1, false); return .{ .text = try std.fmt.allocPrint(self.arena, "eval command could not run: {t}", .{e}), .is_error = true }; }; @@ -104,6 +125,10 @@ pub fn runEval(self: *Agent, note: []const u8) !ExecResult { const met = if (combined) |s| s >= target_f else false; self.eval_repair_pending = exit_code != 0 or !met; self.eval_verified = !self.eval_repair_pending; + // #412: a RED remembers the tree it failed on, so the next eval over that + // same tree is skipped; a green forgets it, because the next RED must be + // measured against its own tree and never against a stale one. + self.eval_fp = if (self.eval_repair_pending) tree_fp else null; // A RED verdict is harness ground truth about a FAILED attempt: park a // capped excerpt so the next escalation decision carries the evidence // (the R0d revision advisory, or an R3 fleet's briefs). diff --git a/src/agent_eval_tests.zig b/src/agent_eval_tests.zig index fbe175aa..b5e9bec7 100644 --- a/src/agent_eval_tests.zig +++ b/src/agent_eval_tests.zig @@ -14,6 +14,8 @@ const trace = @import("trace.zig"); const Tracer = trace.Tracer; const btrace = @import("behavior_trace.zig"); const BehaviorTrace = btrace.BehaviorTrace; +const process_runner = @import("process_runner.zig"); +const verify_fingerprint = @import("verify_fingerprint.zig"); test "runEval: commits before the command runs, mispredicts on a missed target, and leaves a met target commitment-only (#256)" { // The chdir below is POSIX-only, matching session_start.zig's own @@ -130,3 +132,112 @@ test "runEval: commits before the command runs, mispredicts on a missed target, // success signal (docs/behavioral-trajectories.md). try std.testing.expect(lines.next() == null); } + +/// Run a command in the process cwd, failing the test if it did not succeed. +/// The #412 test needs a REAL git repo: reading git is the whole feature, and +/// a mocked probe would prove nothing about the porcelain/diff/untracked split. +fn fixtureCmd(gpa: std.mem.Allocator, io: Io, argv: []const []const u8) !void { + const r = process_runner.runCapped(gpa, io, argv, 1 << 16, 1 << 16, 60_000) catch + return error.SkipZigTest; // no git on this machine: skip rather than fail on the environment + defer gpa.free(r.stdout); + defer gpa.free(r.stderr); + if (!process_runner.ranOk(r)) return error.FixtureCommandFailed; +} + +/// How many times the --eval command was actually SPAWNED. The counter lives +/// under .graff/, which the fixture repo gitignores, so counting can never +/// itself move the fingerprint the guard is reading. +fn verifierCalls(gpa: std.mem.Allocator, io: Io) usize { + const body = Io.Dir.cwd().readFileAlloc(io, ".graff/verifier-calls", gpa, .limited(4096)) catch return 0; + defer gpa.free(body); + return body.len; +} + +test "#412: an unchanged worktree is not re-verified, and any change re-arms the verifier" { + // POSIX-only for the same reason as the test above: the fchdir isolation. + if (builtin.os.tag == .windows) return; + const gpa = std.testing.allocator; + const io = std.testing.io; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var orig_dir = try Io.Dir.cwd().openDir(io, ".", .{}); + defer orig_dir.close(io); + defer _ = std.posix.system.fchdir(orig_dir.handle); + if (std.posix.system.fchdir(tmp.dir.handle) != 0) return error.ChdirFailed; + + // A real single-commit repo. `.graff/` is ignored so the harness's own eval + // log, its notes and the call counter can never read as work the model did + // - without that the tree would move on every eval and never skip. + try Io.Dir.cwd().writeFile(io, .{ .sub_path = ".gitignore", .data = ".graff/\n" }); + try Io.Dir.cwd().writeFile(io, .{ .sub_path = "work.txt", .data = "one\n" }); + try fixtureCmd(gpa, io, &.{ "git", "init", "-q" }); + try fixtureCmd(gpa, io, &.{ "git", "add", "-A" }); + try fixtureCmd(gpa, io, &.{ "git", "-c", "user.email=t@example.com", "-c", "user.name=t", "-c", "commit.gpgsign=false", "commit", "-q", "--no-verify", "-m", "fixture" }); + + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + var agent: Agent = .{ + .gpa = gpa, + .arena = arena_state.allocator(), + .io = io, + .client = undefined, + .provider = undefined, + .messages = undefined, + .sub = false, + .label = "test", + .out = null, + .tracer = null, + // Always RED, and it leaves one byte behind per real invocation. + .eval_cmd = "mkdir -p .graff && printf x >> .graff/verifier-calls; exit 1", + .eval_target = 90, + }; + + // Attempt 1: nothing to compare against, so the verifier runs. Its RED + // verdict arms the guard over the tree it failed on. + const first = try agent.runEval(""); + try std.testing.expect(!first.is_error); // a RED is a verdict, not a tool error + try std.testing.expectEqual(@as(usize, 1), verifierCalls(gpa, io)); + try std.testing.expect(agent.eval_repair_pending); + try std.testing.expect(agent.eval_fp != null); + + // Attempt 2, workspace untouched. This is the whole feature: before it, a + // /goal continuation re-ran the entire scoring command (plus a --judge + // model call) to re-derive a verdict that could not have changed. + const second = try agent.runEval(""); + try std.testing.expectEqual(@as(usize, 1), verifierCalls(gpa, io)); // NOT spawned + try std.testing.expectEqual(@as(u32, 2), agent.eval_iter); // but the attempt still counts + try std.testing.expect(second.is_error); + try std.testing.expect(std.mem.indexOf(u8, second.text, verify_fingerprint.no_progress_steer) != null); + try std.testing.expect(std.mem.indexOf(u8, second.text, "eval output (tail)") == null); // no verdict was manufactured + // A skipped run verifies nothing, so completion stays blocked. + try std.testing.expect(agent.eval_repair_pending and !agent.eval_verified); + + // A TRACKED edit is progress: the verifier runs again... + try Io.Dir.cwd().writeFile(io, .{ .sub_path = "work.txt", .data = "two\n" }); + _ = try agent.runEval(""); + try std.testing.expectEqual(@as(usize, 2), verifierCalls(gpa, io)); + // ...and its RED arms the skip over the NEW tree, not the old one. + _ = try agent.runEval(""); + try std.testing.expectEqual(@as(usize, 2), verifierCalls(gpa, io)); + + // An UNTRACKED file is progress too, and its CONTENTS are what prove it: + // `git status --porcelain` says `?? scratch.txt` either way and + // `git diff HEAD` stays empty, so a fingerprint built from those two alone + // would skip both of these attempts over real work. + try Io.Dir.cwd().writeFile(io, .{ .sub_path = "scratch.txt", .data = "a" }); + _ = try agent.runEval(""); + try std.testing.expectEqual(@as(usize, 3), verifierCalls(gpa, io)); + try Io.Dir.cwd().writeFile(io, .{ .sub_path = "scratch.txt", .data = "b" }); + _ = try agent.runEval(""); + try std.testing.expectEqual(@as(usize, 4), verifierCalls(gpa, io)); + + // Six attempts, four verifier runs: the two no-progress ones were free. + try std.testing.expectEqual(@as(u32, 6), agent.eval_iter); + + // A green eval disarms the guard entirely - the next RED must be measured + // against its own tree, never against one this run already left behind. + agent.eval_cmd = "printf 'score: 100\\n'"; + _ = try agent.runEval(""); + try std.testing.expect(agent.eval_verified and agent.eval_fp == null); +} diff --git a/src/test_hooks.zig b/src/test_hooks.zig index cf8aeac5..5df7ef66 100644 --- a/src/test_hooks.zig +++ b/src/test_hooks.zig @@ -130,6 +130,10 @@ const credential_store = @import("credential_store.zig"); const engine_events = @import("engine_events.zig"); const engine_sink = @import("engine_sink.zig"); +// #412: the worktree fingerprint behind the no-progress verify guard. +// agent_eval.zig reaches it through a CALL only, which analyses nothing. +const verify_fingerprint = @import("verify_fingerprint.zig"); + test { _ = learn_holdout; _ = learn_receipt; @@ -169,6 +173,7 @@ test { _ = credential_store; _ = engine_events; _ = engine_sink; + _ = verify_fingerprint; _ = escalation; _ = escalation_tests; _ = edit_contract; diff --git a/src/verify_fingerprint.zig b/src/verify_fingerprint.zig new file mode 100644 index 00000000..f393d830 --- /dev/null +++ b/src/verify_fingerprint.zig @@ -0,0 +1,282 @@ +//! #412: the no-progress guard in front of a repeated verification. +//! +//! A /goal (or /loop) run is plan-act-VERIFY, and the verifier is the `eval` +//! tool: it runs the --eval command, optionally spawns an LLM judge subagent, +//! and a RED verdict blocks attempt_completion until a fresh green one +//! (agent_tools.handleMeta). So a continuation turn's obvious move is to call +//! eval again - and a model that has not edited anything since the last RED +//! does exactly that, repeatedly. Every one of those re-runs costs the wall +//! clock of the whole scoring command, a judge model call when --judge is set, +//! and 1500 bytes of output tail in the context window, to re-derive a verdict +//! that cannot have changed. +//! +//! So before a re-verify, fingerprint the worktree. Identical to the tree the +//! last verification failed on means nothing can be different, so the verifier +//! is not run at all: the attempt still counts (a no-progress loop must +//! converge on the iteration cap, not spin for free) and the model is steered +//! at the actual blocker - it has to change something first. Straight from the +//! prime agent's `core/autonomous.ts` gate. +//! +//! The fingerprint answers one question - could this verification produce a +//! different result than the last one? - so it folds every input to that +//! answer: the verifier itself, plus the three streams that together describe +//! every uncommitted byte. +//! * the verifier's own identity (the --eval command text). A session that +//! re-points --eval is running a DIFFERENT check, and it must run. +//! * `git status --porcelain -z -uall` - which paths are dirty, and how; +//! * `git diff --binary HEAD` - the exact content of every TRACKED change; +//! * the contents of every untracked file the status listed - which the +//! other two streams do NOT cover. Editing an untracked file leaves both +//! of them byte-identical (`?? notes.md` says nothing about its bytes), so +//! without this third stream the guard would skip a verify over real work. +//! +//! FAIL-OPEN is the whole safety story: no repo, no git, a timed-out probe, a +//! truncated stream, an unreadable file, an absurd number of untracked files - +//! every one of them yields "unknown", and an unknown fingerprint never +//! matches, so the verifier runs. The guard can only ever cost a skipped +//! re-run when it is certain nothing moved; it can never invent a pass. +//! +//! The fold and the decision are PURE (digestOf / untrackedPaths / +//! skipReverify) and unit-tested without a repo; only capture() touches git. +//! Reached through the test-root hook in test_hooks.zig - without that line +//! these tests silently compile to nothing and the suite still reports green. + +const std = @import("std"); +const Io = std.Io; +const Allocator = std.mem.Allocator; +const process_runner = @import("process_runner.zig"); + +/// A worktree fingerprint. `null` anywhere in this module means UNKNOWN, which +/// is always treated as "changed". +pub const Digest = [32]u8; + +/// The sentence the model is steered with when a re-verify is skipped. Kept as +/// its own decl so the tests assert on the contract, not on a format string. +pub const no_progress_steer = + "the workspace has not changed since the last failed verification — edit source files or tests before attempting to finish again"; + +/// One labelled byte stream folded into a fingerprint. +pub const Part = struct { label: []const u8, bytes: []const u8 }; + +/// Per-git-stream output ceiling. Above it the probe reports unknown rather +/// than fingerprinting a truncated diff - a change past the cut would be +/// invisible, which is the one way this guard could wrongly skip a verify. +pub const max_stream_bytes: usize = 8 << 20; +/// Untracked-file ceilings. A workspace carrying a whole vendored tree as +/// untracked files is not worth hashing on every eval; report unknown and let +/// the verifier run. +pub const max_untracked_files: usize = 512; +pub const max_untracked_bytes: usize = 8 << 20; + +/// PURE fold. Every field is length-prefixed, so no two different part lists +/// can produce the same byte stream: ("ab","") and ("a","b") must not collide, +/// or moving a byte across a file boundary would read as no change at all. +pub fn digestOf(parts: []const Part) Digest { + var h = std.crypto.hash.sha2.Sha256.init(.{}); + var len: [8]u8 = undefined; + for (parts) |p| { + std.mem.writeInt(u64, &len, p.label.len, .little); + h.update(&len); + h.update(p.label); + std.mem.writeInt(u64, &len, p.bytes.len, .little); + h.update(&len); + h.update(p.bytes); + } + var out: Digest = undefined; + h.final(&out); + return out; +} + +/// PURE parse of `git status --porcelain -z -uall` into the untracked paths +/// whose CONTENTS still have to be folded in. NUL-separated records, so paths +/// are never quoted or escaped - which is exactly why -z is used instead of +/// the human porcelain. A rename/copy record carries its origin path as a +/// second NUL field; that field is consumed here so it can never be mistaken +/// for a record of its own. +pub fn untrackedPaths(arena: Allocator, status_z: []const u8) ![]const []const u8 { + var out: std.ArrayList([]const u8) = .empty; + var it = std.mem.splitScalar(u8, status_z, 0); + while (it.next()) |rec| { + if (rec.len < 4 or rec[2] != ' ') continue; + const x = rec[0]; + const y = rec[1]; + if (x == 'R' or x == 'C' or y == 'R' or y == 'C') _ = it.next(); + if (x == '?' and y == '?') try out.append(arena, rec[3..]); + } + return out.items; +} + +/// One git stream, or null on ANY failure: spawn error, nonzero exit, timeout, +/// or a stdout cap hit. Allocated on `arena`; nothing here outlives the turn. +fn gitStream(arena: Allocator, gpa: Allocator, io: Io, argv: []const []const u8) ?[]const u8 { + const r = process_runner.runCapped(gpa, io, argv, max_stream_bytes, 8192, 30_000) catch return null; + defer gpa.free(r.stdout); + defer gpa.free(r.stderr); + if (!process_runner.ranOk(r) or r.stdout_truncated or r.timed_out) return null; + return arena.dupe(u8, r.stdout) catch null; +} + +/// Fingerprint the verification about to run: `verifier` (the --eval command +/// text) over the working tree at the PROCESS cwd, which is where runEval +/// spawns it. Returns null (unknown, i.e. "changed") for anything that is not +/// a clean, complete read. +/// +/// Everything read here is scratch and only the 32-byte digest escapes, so it +/// is folded in a PRIVATE arena rather than the caller's: an eval loop calls +/// this once per iteration, and an 8 MiB diff parked on the session arena +/// every time would outlive the whole run. +pub fn capture(gpa: Allocator, io: Io, verifier: []const u8) ?Digest { + var scratch = std.heap.ArenaAllocator.init(gpa); + defer scratch.deinit(); + const arena = scratch.allocator(); + const status = gitStream(arena, gpa, io, &.{ "git", "status", "--porcelain", "-z", "-uall" }) orelse return null; + const diff = gitStream(arena, gpa, io, &.{ "git", "diff", "--binary", "HEAD" }) orelse return null; + const paths = untrackedPaths(arena, status) catch return null; + if (paths.len > max_untracked_files) return null; + var parts: std.ArrayList(Part) = .empty; + parts.append(arena, .{ .label = "verifier", .bytes = verifier }) catch return null; + parts.append(arena, .{ .label = "status", .bytes = status }) catch return null; + parts.append(arena, .{ .label = "diff", .bytes = diff }) catch return null; + var budget: usize = max_untracked_bytes; + for (paths) |p| { + // A file that will not read - deleted between the two probes, a fifo, + // a symlink to nowhere, or simply bigger than what is left of the + // budget - makes the whole fingerprint unknown rather than partial. + const body = Io.Dir.cwd().readFileAlloc(io, p, arena, .limited(budget)) catch return null; + budget -= body.len; + parts.append(arena, .{ .label = p, .bytes = body }) catch return null; + } + return digestOf(parts.items); +} + +/// PURE decision: skip the verifier only when the LAST attempt failed and both +/// fingerprints are known and equal. Unknown on either side is "changed". +pub fn skipReverify(last_failed: bool, last: ?Digest, current: ?Digest) bool { + if (!last_failed) return false; + const a = last orelse return false; + const b = current orelse return false; + return std.mem.eql(u8, &a, &b); +} + +/// The tool result a skipped re-verify hands back. It names the iteration (the +/// attempt is counted either way) and says plainly that nothing ran, so the +/// model cannot read it as a fresh verdict. +pub fn noProgressText(arena: Allocator, iter: u32) ![]const u8 { + return std.fmt.allocPrint( + arena, + "eval #{d} was not run: {s}. The previous RED verdict stands and completion is still blocked; a verifier re-run over an identical tree can only repeat it.", + .{ iter, no_progress_steer }, + ); +} + +test "digestOf: identical evidence folds identically, and every stream moves it" { + const base = [_]Part{ + .{ .label = "verifier", .bytes = "zig build test" }, + .{ .label = "status", .bytes = "?? notes.md\x00" }, + .{ .label = "diff", .bytes = "diff --git a/x b/x\n+one\n" }, + .{ .label = "notes.md", .bytes = "first draft" }, + }; + try std.testing.expectEqual(digestOf(&base), digestOf(&base)); + + // A different VERIFIER moves it: a re-pointed --eval is a different check + // over the same tree, and it has to run. + var reverifier = base; + reverifier[0].bytes = "pytest -q"; + try std.testing.expect(!std.mem.eql(u8, &digestOf(&base), &digestOf(&reverifier))); + + // A TRACKED edit moves it (the diff stream changed). + var tracked = base; + tracked[2].bytes = "diff --git a/x b/x\n+two\n"; + try std.testing.expect(!std.mem.eql(u8, &digestOf(&base), &digestOf(&tracked))); + + // An UNTRACKED-ONLY edit moves it too, and this is the case the other two + // streams cannot see: `?? notes.md` and `git diff HEAD` are byte-identical + // whatever that file contains. Without folding the contents in, editing an + // untracked file would read as "nothing changed" and skip a real verify. + var untracked = base; + untracked[3].bytes = "second draft"; + try std.testing.expect(!std.mem.eql(u8, &digestOf(&untracked), &digestOf(&base))); + try std.testing.expectEqualStrings(base[1].bytes, untracked[1].bytes); + try std.testing.expectEqualStrings(base[2].bytes, untracked[2].bytes); + + // A new untracked file is a change even when it is empty. + const grown = base ++ [_]Part{.{ .label = "scratch", .bytes = "" }}; + try std.testing.expect(!std.mem.eql(u8, &digestOf(&grown), &digestOf(&base))); +} + +test "digestOf: length-prefixed, so a byte moved across a boundary is not a collision" { + // Plain concatenation would make these three indistinguishable - i.e. a + // rename, or one file's tail becoming the next file's head, would fold to + // "unchanged" and skip a verification over real work. + const a = [_]Part{ .{ .label = "f", .bytes = "ab" }, .{ .label = "g", .bytes = "" } }; + const b = [_]Part{ .{ .label = "f", .bytes = "a" }, .{ .label = "g", .bytes = "b" } }; + const c = [_]Part{ .{ .label = "f", .bytes = "" }, .{ .label = "g", .bytes = "ab" } }; + try std.testing.expect(!std.mem.eql(u8, &digestOf(&a), &digestOf(&b))); + try std.testing.expect(!std.mem.eql(u8, &digestOf(&b), &digestOf(&c))); + // And the LABEL is part of the identity: the same bytes under a different + // path is a different worktree. + const renamed = [_]Part{ .{ .label = "f2", .bytes = "ab" }, .{ .label = "g", .bytes = "" } }; + try std.testing.expect(!std.mem.eql(u8, &digestOf(&renamed), &digestOf(&a))); + try std.testing.expectEqual(digestOf(&[_]Part{}), digestOf(&[_]Part{})); +} + +test "untrackedPaths: only ?? records, and a rename's origin field is consumed" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const ar = arena_state.allocator(); + + // A realistic -z porcelain: modified, staged-add, a rename (two fields), + // two untracked files, one of them with a space in its name (unquoted, + // which is the reason -z is used at all). + const status = " M src/main.zig\x00A src/new.zig\x00R dst.zig\x00src.zig\x00?? notes.md\x00?? a b.txt\x00"; + const paths = try untrackedPaths(ar, status); + try std.testing.expectEqual(@as(usize, 2), paths.len); + try std.testing.expectEqualStrings("notes.md", paths[0]); + try std.testing.expectEqualStrings("a b.txt", paths[1]); + + // The rename's ORIGIN field must not be read as a record of its own: a + // path beginning "?? " would otherwise be picked up out of it. + const trap = "R dst\x00?? not-a-record\x00?? real.txt\x00"; + const trapped = try untrackedPaths(ar, trap); + try std.testing.expectEqual(@as(usize, 1), trapped.len); + try std.testing.expectEqualStrings("real.txt", trapped[0]); + + // Empty, all-tracked, and malformed input all yield nothing rather than + // erroring: a clean tree has no untracked contents to fold. + try std.testing.expectEqual(@as(usize, 0), (try untrackedPaths(ar, "")).len); + try std.testing.expectEqual(@as(usize, 0), (try untrackedPaths(ar, " M a\x00")).len); + try std.testing.expectEqual(@as(usize, 0), (try untrackedPaths(ar, "??\x00?x\x00")).len); +} + +test "skipReverify fails OPEN: only a known, equal fingerprint after a failure skips" { + const a = digestOf(&[_]Part{.{ .label = "s", .bytes = "one" }}); + const b = digestOf(&[_]Part{.{ .label = "s", .bytes = "two" }}); + + // The one case that skips. + try std.testing.expect(skipReverify(true, a, a)); + // A changed tree re-arms it. + try std.testing.expect(!skipReverify(true, a, b)); + // The last attempt did not fail: there is nothing to re-verify, so the + // guard is inert (a green eval must never be turned into a skip). + try std.testing.expect(!skipReverify(false, a, a)); + // Fail-open: a git error, a truncated stream or an unreadable untracked + // file lands here as null on either side, and never matches. A repo the + // probe cannot read must never be able to suppress a verification. + try std.testing.expect(!skipReverify(true, null, a)); + try std.testing.expect(!skipReverify(true, a, null)); + try std.testing.expect(!skipReverify(true, null, null)); +} + +test "noProgressText names the iteration, carries the steer, and never reads as a verdict" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const ar = arena_state.allocator(); + const text = try noProgressText(ar, 4); + try std.testing.expect(std.mem.startsWith(u8, text, "eval #4 was not run:")); + try std.testing.expect(std.mem.indexOf(u8, text, no_progress_steer) != null); + try std.testing.expect(std.mem.indexOf(u8, text, "completion is still blocked") != null); + // No score, no "TARGET MET": the model must not be able to mistake a + // skipped run for a fresh green one. + try std.testing.expect(std.mem.indexOf(u8, text, "score") == null); + try std.testing.expect(std.mem.indexOf(u8, text, "TARGET MET") == null); +} From 2428753ab57200463c2152929d151859639ca864 Mon Sep 17 00:00:00 2001 From: Rach Pradhan <54503978+justrach@users.noreply.github.com> Date: Thu, 6 Aug 2026 14:59:26 +0800 Subject: [PATCH 2/2] fix(goal): .graff/ bookkeeping never counts as workspace progress (#412) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fingerprint folds untracked file contents, and runEval appends to .graff/eval-log.tsv on every call — in a repo that does not gitignore .graff/, the tree provably moved each iteration and the guard failed open (measured: strictly worse than no guard, +prompt growth, zero savings). graff's own state dir is now excluded from the untracked scan. Found by the integration batch comparison. Co-Authored-By: Codegraff --- src/verify_fingerprint.zig | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/verify_fingerprint.zig b/src/verify_fingerprint.zig index f393d830..1d66241d 100644 --- a/src/verify_fingerprint.zig +++ b/src/verify_fingerprint.zig @@ -101,7 +101,11 @@ pub fn untrackedPaths(arena: Allocator, status_z: []const u8) ![]const []const u const x = rec[0]; const y = rec[1]; if (x == 'R' or x == 'C' or y == 'R' or y == 'C') _ = it.next(); - if (x == '?' and y == '?') try out.append(arena, rec[3..]); + // graff's own bookkeeping (.graff/ eval-log, traces, sessions) moves on + // every iteration by construction; counting it as workspace progress + // would fail the guard open in any repo that does not gitignore it. + if (x == '?' and y == '?' and !std.mem.startsWith(u8, rec[3..], ".graff/")) + try out.append(arena, rec[3..]); } return out.items; } @@ -234,6 +238,15 @@ test "untrackedPaths: only ?? records, and a rename's origin field is consumed" try std.testing.expectEqualStrings("notes.md", paths[0]); try std.testing.expectEqualStrings("a b.txt", paths[1]); + // graff's own state dir never counts as progress: in a repo that does not + // gitignore .graff/, the eval-log append would otherwise move the tree on + // every iteration and fail the guard open (found by the batch comparison). + const own = "?? .graff/eval-log.tsv\x00?? .graffx\x00?? src/real.zig\x00"; + const own_paths = try untrackedPaths(ar, own); + try std.testing.expectEqual(@as(usize, 2), own_paths.len); + try std.testing.expectEqualStrings(".graffx", own_paths[0]); + try std.testing.expectEqualStrings("src/real.zig", own_paths[1]); + // The rename's ORIGIN field must not be read as a record of its own: a // path beginning "?? " would otherwise be picked up out of it. const trap = "R dst\x00?? not-a-record\x00?? real.txt\x00";