From a012a237957c4d0c36581bf6dde7f592b3810f8b Mon Sep 17 00:00:00 2001 From: Rach Pradhan <54503978+justrach@users.noreply.github.com> Date: Thu, 6 Aug 2026 18:42:44 +0800 Subject: [PATCH 1/4] feat(session): append-only transcript per durable session (#441) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Corrects a false premise. #438 (issue #410) shipped a prompt line telling the model that the session file preserves what compaction discards. It does not: `.graff/sessions/.session.json` is a SINGLE JSON object whose `messages` array is rewritten in place, so the next autosave after a compaction drops the pre-compaction history permanently. "Grep your own conversation log" therefore had no graff equivalent — the trace and trajectory JSONLs record events for telemetry, not the conversation for recall. `.graff/sessions/.transcript.jsonl` is that equivalent: one line per message as it FIRST enters history, appended by a single positional write at the end of file and never rewritten. A line is the message's provider-native JSON verbatim (JSON escapes every control character, so a message is always exactly one line), which makes the file greppable with no tooling and makes a resume's re-seed exact — hashing a line reproduces the digest that wrote it. Hooked at session.queueSave, the point where the autosave already observes a new message, so none of the history's ~25 mutation sites needs a hook of its own. Identity is a MULTISET of the digests on disk, not a position: compact() builds `[handoff summary] ++ recent_messages`, putting the summary at the front of the history while it is the last line in the file, and any position-based match would re-append the retained tail behind it. Counting instead records the summary alone. An ordinary turn takes a fast path (two serializations plus one per new message) and only a rewrite, a resume, or a session's first append pays for a full walk. Bounds, as the issue asks: - Subagents excluded. Their history is never persisted, so there is no durable session to attach a transcript to — #409's rule, unchanged. - One lifecycle, not two. #409's sweep now reclaims transcripts and artifact dirs together (tool_spill.sweepSessionsOnce), by the same rule — the session file is the ground truth for "this session is gone" — and the same grace window. It now also runs at the first transcript append, so a run that never spills still collects what deleted sessions left behind. - The size cap ROTATES rather than head-truncates. Head-truncation means reading the file, dropping a prefix and writing what is left back over it: an in-place rewrite of the history, which is the exact failure mode this change exists to fix, and one a crash halfway through turns into total loss. A rename is one atomic syscall, the old bytes survive intact under a sibling name the model greps identically, and disk use is bounded at two generations (16 MiB each) rather than merely slowed. Tests 1043 -> 1050. The rewrite case asserts the pre-compaction file is an exact byte PREFIX of the post-compaction one and that the discarded detail is still greppable; repeated autosaves over an unchanged history add nothing; a resume re-seeds from disk instead of re-appending its restored history; a subagent writes nothing; rotation leaves the previous generation byte-identical. Verified end to end against a mock provider: two processes, four turns, four lines, no duplicates. session_transcript.activePath(root, arena) is the accessor #411's post-compaction note should use — null for a subagent or a session with no transcript, so the note can never cite a file that does not exist, with lineCount() for its "N messages". Co-Authored-By: Codegraff --- src/session.zig | 4 + src/session_index.zig | 7 + src/session_tests.zig | 51 ++++ src/session_transcript.zig | 537 +++++++++++++++++++++++++++++++++++++ src/tool_spill.zig | 97 +++++-- 5 files changed, 679 insertions(+), 17 deletions(-) create mode 100644 src/session_transcript.zig diff --git a/src/session.zig b/src/session.zig index 6b3e03bf..0d99b2a8 100644 --- a/src/session.zig +++ b/src/session.zig @@ -26,6 +26,7 @@ const goal_state = @import("goal_state.zig"); const session_writer = @import("session_writer.zig"); // #273: the fingerprint + the background write const shutdown_trace = @import("shutdown_trace.zig"); // #364: teardown phase stamps const protocol_seq = @import("protocol_seq.zig"); // #330: the --json event sequence survives a resume +const session_transcript = @import("session_transcript.zig"); // #441: the append-only history this file's rewrites discard const Agent = agent_mod.Agent; const Keys = provider_mod.Keys; const unixMs = util.unixMs; @@ -239,6 +240,9 @@ fn queueSave(root: *Agent, arena: Allocator, dir: Io.Dir, name: []const u8) !u64 const fp = fingerprint(root, name); const rel = try sessionPath(arena, name); if (session_writer.alreadySaved(root.io, dir, rel, fp)) return 0; + // #441: the same observation point, one line per new message, appended to a + // file this function's in-place rewrite of `messages` can never reach. + session_transcript.record(root, dir, name); var aw: Io.Writer.Allocating = .init(root.gpa); defer aw.deinit(); var s: std.json.Stringify = .{ .writer = &aw.writer }; diff --git a/src/session_index.zig b/src/session_index.zig index a8f48ad2..a6d1722d 100644 --- a/src/session_index.zig +++ b/src/session_index.zig @@ -18,6 +18,13 @@ pub const session_ext = ".session.json"; /// Title-named session files live here (resume reads this). pub const sessions_dir = ".graff/sessions"; +/// #441: the append-only transcript beside each session file, and the one +/// rotated generation behind it. The suffixes live here, with the session +/// suffix, because BOTH the writer (session_transcript.zig) and the sweep that +/// reclaims them with their session (tool_spill.zig) have to agree on them. +pub const transcript_ext = ".transcript.jsonl"; +pub const transcript_rotated_ext = ".transcript.1.jsonl"; + /// Path to a session file: .graff/sessions/.session.json. pub fn sessionPath(arena: Allocator, name: []const u8) ![]const u8 { return std.fmt.allocPrint(arena, "{s}/{s}{s}", .{ sessions_dir, name, session_ext }); diff --git a/src/session_tests.zig b/src/session_tests.zig index 0e2574a2..c2a837c9 100644 --- a/src/session_tests.zig +++ b/src/session_tests.zig @@ -15,6 +15,10 @@ const Allocator = std.mem.Allocator; const agent_mod = @import("agent.zig"); const session = @import("session.zig"); const session_writer = @import("session_writer.zig"); +// #441: the save path now also appends to the session transcript, and that +// state is per-PROCESS. Every test that reaches saveSessionTo resets it, or the +// digests it holds outlive the testing allocator that owns them. +const session_transcript = @import("session_transcript.zig"); const Agent = agent_mod.Agent; test "todos round-trip: appendTodosFromValue parses content/status/epoch, skips junk (#318)" { @@ -164,6 +168,8 @@ test "an unchanged conversation skips the save entirely (#273)" { const arena = arena_state.allocator(); session_writer.resetForTest(); defer session_writer.resetForTest(); + session_transcript.resetForTest(); + defer session_transcript.resetForTest(); var tmp = std.testing.tmpDir(.{}); defer tmp.cleanup(); @@ -194,6 +200,8 @@ test "the skip never fires over a session file something else rewrote (#273/#289 const arena = arena_state.allocator(); session_writer.resetForTest(); defer session_writer.resetForTest(); + session_transcript.resetForTest(); + defer session_transcript.resetForTest(); var tmp = std.testing.tmpDir(.{}); defer tmp.cleanup(); @@ -225,6 +233,8 @@ test "any change to the conversation produces a save (#273)" { const arena = arena_state.allocator(); session_writer.resetForTest(); defer session_writer.resetForTest(); + session_transcript.resetForTest(); + defer session_transcript.resetForTest(); var tmp = std.testing.tmpDir(.{}); defer tmp.cleanup(); @@ -268,6 +278,8 @@ test "a turn's queued save is not lost when the session ends (#273)" { const arena = arena_state.allocator(); session_writer.resetForTest(); defer session_writer.resetForTest(); + session_transcript.resetForTest(); + defer session_transcript.resetForTest(); var tmp = std.testing.tmpDir(.{}); defer tmp.cleanup(); @@ -288,3 +300,42 @@ test "a turn's queued save is not lost when the session ends (#273)" { defer gpa.free(saved); try std.testing.expect(std.mem.indexOf(u8, saved, "the last thing the model said") != null); } + +test "the autosave is where the transcript sees a new message (#441)" { + const io = std.testing.io; + const gpa = std.testing.allocator; + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + session_writer.resetForTest(); + defer session_writer.resetForTest(); + session_transcript.resetForTest(); + defer session_transcript.resetForTest(); + var tmp = std.testing.tmpDir(.{ .iterate = true }); + defer tmp.cleanup(); + + const f = try fixture(gpa, arena, io); + defer gpa.destroy(f); + try session.saveSessionTo(&f.root, arena, tmp.dir, "wf"); + try appendTurn(arena, &f.root, "the reply"); + try session.saveSessionTo(&f.root, arena, tmp.dir, "wf"); + session.flushSaves(); + + // The real save path wrote the transcript beside the session file, one line + // per message, with no hook of its own at any of the history's mutation + // sites. #411's note gets the path from the accessor, not by rebuilding it. + f.root.session_name = "wf"; + try std.testing.expectEqualStrings(".graff/sessions/wf.transcript.jsonl", session_transcript.activePath(&f.root, arena).?); + const lines = try tmp.dir.readFileAlloc(io, ".graff/sessions/wf.transcript.jsonl", gpa, .limited(64 * 1024)); + defer gpa.free(lines); + try std.testing.expectEqual(@as(usize, 2), std.mem.count(u8, lines, "\n")); + try std.testing.expectEqual(@as(usize, 2), session_transcript.lineCount()); + try std.testing.expect(std.mem.indexOf(u8, lines, "first prompt") != null); + try std.testing.expect(std.mem.indexOf(u8, lines, "the reply") != null); + + // An unchanged conversation skips the save whole (#273), and therefore the + // transcript too: a repeated autosave never re-appends a message. + try session.saveSessionTo(&f.root, arena, tmp.dir, "wf"); + session.flushSaves(); + try std.testing.expectEqual(@as(usize, 2), session_transcript.lineCount()); +} diff --git a/src/session_transcript.zig b/src/session_transcript.zig new file mode 100644 index 00000000..cc15461b --- /dev/null +++ b/src/session_transcript.zig @@ -0,0 +1,537 @@ +//! #441: the append-only per-session transcript — the history compaction +//! discards, kept and kept greppable. +//! +//! THE FALSE PREMISE THIS CORRECTS. #438 (issue #410) added a prompt line +//! telling the model that the session file preserves what compaction throws +//! away. It does not. `.graff/sessions/.session.json` is a SINGLE JSON +//! object whose `messages` array is rewritten in place: when compact() replaces +//! a hundred turns with one summary, the very next autosave overwrites the file +//! and the pre-compaction history is gone for good. Prime-agent's "grep your +//! own conversation log" property — recovering the exact wording of an error a +//! summary paraphrased — therefore had no graff equivalent. The trace and +//! trajectory JSONLs do not provide it either: they record events for +//! telemetry, not the conversation for recall. +//! +//! WHAT THIS IS. `.graff/sessions/.transcript.jsonl`, one line per +//! message as it FIRST enters history, appended and never rewritten. A line is +//! that message's provider-native JSON verbatim — JSON escapes every control +//! character, so a message is always exactly one line — which makes the file +//! greppable with no tooling and re-readable with no parser state. Compaction +//! cannot touch it: it rewrites `root.messages` in memory, while every line +//! already on disk stays exactly where it was. +//! +//! WHERE IT HOOKS. session.queueSave, the point at which the autosave already +//! observes a new message. The history has ~25 mutation sites (session_writer's +//! own comment counts them); hooking each one is the hand-maintained dirty flag +//! that file rejects for the same reason. Observing the array instead costs one +//! serialization per turn — see `collect`'s fast path. +//! +//! IDENTITY. A message is already transcribed when its serialized bytes digest +//! to a line already written — counted, not positioned. `counts` is a MULTISET +//! of the digests on disk, because a rewrite reorders as well as removes: +//! compact() builds `[handoff summary] ++ recent_messages` with the tail +//! verbatim, so the summary sits at the FRONT of the history while it is the +//! LAST line in the file, and any position-based match re-appends the tail +//! behind it. Counting instead: the tail's digests are already accounted for, +//! only the summary is new. emergencyTrim drops a prefix and the survivors are +//! likewise accounted for; capOversizedToolOutputs edits a message in place, +//! and the transcript simply keeps the PRE-truncation bytes, which is the whole +//! point of it. A genuine repeat is still recorded twice: the second identical +//! "ok" is the history's SECOND, and the file holds only one. +//! +//! The one thing counting gives up, deliberately: a message identical to one +//! whose every copy compaction has already discarded is not written again. Its +//! bytes are in the file verbatim either way, so recall — what this exists for +//! — is unaffected; only exact multiplicity is, and only after a compaction. +//! +//! BOUNDS. Subagents are excluded — their history is never persisted, so there +//! is no durable session to attach a transcript to (#409's rule, unchanged). +//! The size cap ROTATES rather than head-truncates; `rotate` says why. The +//! lifecycle is #409's rather than a second one: `tool_spill.sweepSessionsOnce` +//! now reclaims transcripts and artifact dirs together, by the same rule (the +//! session file is the ground truth for "this session is gone") and the same +//! grace window. + +const std = @import("std"); +const Io = std.Io; +const Value = std.json.Value; +const Allocator = std.mem.Allocator; + +const agent_mod = @import("agent.zig"); +const Agent = agent_mod.Agent; +const session_index = @import("session_index.zig"); +const tool_spill = @import("tool_spill.zig"); // #409: safeName + the shared session sweep + +pub const transcript_ext = session_index.transcript_ext; +pub const transcript_rotated_ext = session_index.transcript_rotated_ext; + +/// Ceiling on ONE generation, so a session keeps at most twice this on disk +/// (the live file plus the one rotated generation). `pub var` so a test can +/// shrink it without writing 16 MiB. +pub var cap_bytes: usize = 16 * 1024 * 1024; + +/// `.graff/sessions/.transcript.jsonl`. Pure; #411's post-compaction note +/// and anything else that needs the path calls this instead of re-deriving the +/// filename. +pub fn transcriptPath(arena: Allocator, name: []const u8) ![]const u8 { + return std.fmt.allocPrint(arena, "{s}/{s}{s}", .{ session_index.sessions_dir, name, transcript_ext }); +} + +/// The previous generation (see `rotate`), which is just as greppable. +pub fn rotatedPath(arena: Allocator, name: []const u8) ![]const u8 { + return std.fmt.allocPrint(arena, "{s}/{s}{s}", .{ session_index.sessions_dir, name, transcript_rotated_ext }); +} + +/// The transcript path for THIS agent, or null when it has none: a subagent, a +/// name that cannot be a file, or a session no save has reached yet. #411's +/// note calls this rather than building the path itself, so it can never cite a +/// file that does not exist. +pub fn activePath(root: *Agent, arena: Allocator) ?[]const u8 { + if (root.sub or g.total == 0) return null; + if (g.name.len == 0 or !std.mem.eql(u8, g.name, root.session_name)) return null; + return transcriptPath(arena, g.name) catch null; +} + +/// How many messages this session's transcript is known to hold — the "N +/// messages" half of a note that cites the path. Known: a resume re-reads the +/// live generation, not the rotated one behind it. +pub fn lineCount() usize { + return g.total; +} + +/// The session currently attached and what is already on disk for it. One root +/// agent owns the durable session, so this is process state rather than agent +/// state; `attach` re-seeds it whenever the name changes (startup, /resume, +/// /new, /save , the AI-title rename). +const State = struct { + name: []const u8 = "", // a slice of name_buf; "" = nothing attached + /// How many lines carry each digest — see IDENTITY above. + counts: std.AutoHashMapUnmanaged(u64, u32) = .empty, + total: usize = 0, // lines accounted for + /// The cursor the fast path rides: this many messages of the CURRENT + /// history are accounted for, and `anchor` is the digest the last of them + /// had when that became true. If it still does, nothing before it moved. + seen_len: usize = 0, + anchor: u64 = 0, + bytes: usize = 0, // size of the live generation on disk +}; + +var g: State = .{}; +// Owns `g.name`'s bytes, and `g.counts` is allocated with the page allocator +// rather than the session's `gpa`, for session_writer.zig's #365 reason: this +// is PROCESS state that outlives every session in it, so charging it to the +// conversation's allocator makes the last session's state a leak at exit — +// and, in the test suite, a free across a test boundary that has already +// reclaimed the arena underneath it. +var name_buf: [256]u8 = undefined; +const state_gpa = std.heap.page_allocator; +// std.Io owns the synchronization primitives (session_writer.zig's note): the +// save path is the caller's thread, and `serve` can hold more than one session +// in a process. +var mutex: Io.Mutex = .init; + +fn digest(line: []const u8) u64 { + return std.hash.Wyhash.hash(0x441, line); +} + +/// One message as it goes to disk. Serializing to the arena is the same walk +/// std.json.Stringify performs for the save itself. +fn serialize(arena: Allocator, m: Value) ?[]const u8 { + var aw: Io.Writer.Allocating = .init(arena); + var s: std.json.Stringify = .{ .writer = &aw.writer }; + s.write(m) catch return null; + return aw.toOwnedSlice() catch null; +} + +const Line = struct { text: []const u8, hash: u64 }; + +fn push(arena: Allocator, out: *std.ArrayList(Line), m: Value) void { + const text = serialize(arena, m) orelse return; + out.append(arena, .{ .text = text, .hash = digest(text) }) catch {}; +} + +/// The messages in `items` that are not yet in the transcript, in order. +/// +/// The fast path is the one every ordinary turn takes: the accounted-for prefix +/// is still in place — proven by re-digesting its LAST message — so everything +/// after it is new by construction. Two serializations plus one per new +/// message, instead of one per message in the whole history, every save. +/// +/// The slow path runs on the first append of a session, and after a rewrite: +/// tally the history's digests in order and write a message only once its +/// running tally passes the number of lines the file already has for it. +fn collect(arena: Allocator, items: []const Value, out: *std.ArrayList(Line)) void { + if (g.seen_len > 0 and g.seen_len <= items.len and lastMatches(arena, items[g.seen_len - 1], g.anchor)) { + for (items[g.seen_len..]) |m| push(arena, out, m); + return; + } + var seen: std.AutoHashMapUnmanaged(u64, u32) = .empty; // arena-owned, dies with the save + for (items) |m| { + const text = serialize(arena, m) orelse continue; + const h = digest(text); + const e = seen.getOrPut(arena, h) catch return; + e.value_ptr.* = (if (e.found_existing) e.value_ptr.* else 0) + 1; + if (e.value_ptr.* <= (g.counts.get(h) orelse 0)) continue; // this copy is already a line + out.append(arena, .{ .text = text, .hash = h }) catch return; + } +} + +fn lastMatches(arena: Allocator, last: Value, anchor: u64) bool { + const text = serialize(arena, last) orelse return false; + return digest(text) == anchor; +} + +/// Append every message that is not already in this session's transcript. +/// Called from session.queueSave, after the blank-draft and unchanged-session +/// gates, so a transcript exists exactly when a durable session does. +/// +/// Never fails a save: every error path is a skip. A skipped append costs a +/// duplicate line on the next one, never a lost message. +/// +/// Serializes into a scratch arena of its own rather than the caller's: the +/// turn path's arena lives as long as the process (mainloop.Ctx.arena), and a +/// copy of every new message per turn parked there for the whole session would +/// double what the conversation already costs. +pub fn record(root: *Agent, dir: Io.Dir, name: []const u8) void { + if (root.sub) return; // a subagent's history is never persisted + if (!tool_spill.safeName(name)) return; // never write outside .graff/sessions + if (root.messages.items.len == 0) return; + const io = root.io; + var scratch = std.heap.ArenaAllocator.init(root.gpa); + defer scratch.deinit(); + const arena = scratch.allocator(); + mutex.lockUncancelable(io); + defer mutex.unlock(io); + attach(io, dir, arena, name); + if (!std.mem.eql(u8, g.name, name)) return; // attach failed; try again next save + const items = root.messages.items; + var pending: std.ArrayList(Line) = .empty; + collect(arena, items, &pending); + // A batch that never reached the disk leaves the cursor alone, so the next + // save writes it instead of stepping over it. + if (pending.items.len > 0 and !flush(io, dir, arena, name, pending.items)) return; + g.seen_len = items.len; + g.anchor = if (serialize(arena, items[items.len - 1])) |text| digest(text) else 0; +} + +/// Point the state at `name`, rebuilding it from disk when the session changed. +/// A resumed conversation is the case that matters: its history is restored +/// whole, and every message in it is already a line in its transcript. +fn attach(io: Io, dir: Io.Dir, arena: Allocator, name: []const u8) void { + if (g.name.len > 0 and std.mem.eql(u8, g.name, name)) return; + detach(); + if (name.len > name_buf.len) return; // safeName caps well below this + @memcpy(name_buf[0..name.len], name); + g.name = name_buf[0..name.len]; + // #409's sweep, now shared: at the first append as well as the first spill, + // so a run that never spills still reclaims what deleted sessions left. + tool_spill.sweepSessionsOnce(io, dir, arena, name); + seed(io, dir, arena, name); +} + +fn detach() void { + g.counts.deinit(state_gpa); + g = .{}; +} + +/// Read back the digests of the lines already in the live generation. Exact, +/// because a line IS the serialized message: hashing the line bytes reproduces +/// the digest the writer recorded. An unreadable file seeds nothing, which +/// costs duplicated lines and never a lost one. +fn seed(io: Io, dir: Io.Dir, arena: Allocator, name: []const u8) void { + const path = transcriptPath(arena, name) catch return; + const data = dir.readFileAlloc(io, path, arena, .limited(cap_bytes)) catch return; + g.bytes = data.len; + var it = std.mem.splitScalar(u8, data, '\n'); + while (it.next()) |line| { + if (line.len == 0) continue; // the trailing newline, or a torn write + bump(digest(line)); + } +} + +fn bump(h: u64) void { + const e = g.counts.getOrPut(state_gpa, h) catch return; + e.value_ptr.* = (if (e.found_existing) e.value_ptr.* else 0) + 1; + g.total += 1; +} + +fn flush(io: Io, dir: Io.Dir, arena: Allocator, name: []const u8, pending: []const Line) bool { + var buf: std.ArrayList(u8) = .empty; + for (pending) |line| { + buf.appendSlice(arena, line.text) catch return false; + buf.append(arena, '\n') catch return false; + } + if (g.bytes +| buf.items.len > cap_bytes) rotate(io, dir, arena, name); + const path = transcriptPath(arena, name) catch return false; + const size = appendWhole(io, dir, path, buf.items) orelse return false; + g.bytes = size; + for (pending) |line| bump(line.hash); // only now: these bytes are on disk + return true; +} + +/// ONE positional write at the current end of file (playbook.appendLine's and +/// serve_events.EventLog's shape). Nothing already in the file is read, moved +/// or rewritten — that is the whole append-only guarantee, and it is why a +/// compaction running against the same session cannot cost the transcript a +/// byte. Returns the new size. +fn appendWhole(io: Io, dir: Io.Dir, path: []const u8, data: []const u8) ?usize { + dir.createDirPath(io, session_index.sessions_dir) catch {}; + const f = dir.createFile(io, path, .{ .truncate = false }) catch return null; + defer f.close(io); + const st = f.stat(io) catch return null; + f.writePositionalAll(io, data, st.size) catch return null; + return @as(usize, @intCast(st.size)) + data.len; +} + +/// At the cap the live generation is RENAMED aside, never trimmed in place. +/// +/// Head-truncation would mean reading the file, dropping a prefix and writing +/// what is left back over it — an in-place rewrite of the history, which is +/// precisely the failure mode this file exists to fix, and one a crash halfway +/// through turns into total loss. A rename is a single atomic syscall: the old +/// bytes survive intact under a sibling name the model greps exactly like the +/// live one, every line ever written stays a line somewhere, and the bound is +/// simply two generations. The previous generation is what the next rotation +/// replaces, so disk use is capped rather than merely slowed. +fn rotate(io: Io, dir: Io.Dir, arena: Allocator, name: []const u8) void { + const live = transcriptPath(arena, name) catch return; + const prev = rotatedPath(arena, name) catch return; + dir.rename(live, dir, prev, io) catch return; + g.bytes = 0; + // The digests stay: they identify messages, not file offsets, so a message + // already in the rotated generation is still not re-appended to the live one. +} + +pub fn resetForTest() void { + detach(); +} + +// ── tests ──────────────────────────────────────────────────────────────── + +const testing = std.testing; + +fn msg(arena: Allocator, role: []const u8, text: []const u8) !Value { + var o: std.json.ObjectMap = .empty; + try o.put(arena, "role", .{ .string = role }); + try o.put(arena, "content", .{ .string = text }); + return .{ .object = o }; +} + +/// The fields `record` reads, and nothing else. +fn agentFor(gpa: Allocator, arena: Allocator, io: Io, name: []const u8) Agent { + var root: Agent = undefined; + root.gpa = gpa; + root.io = io; + root.sub = false; + root.session_name = name; + root.messages = std.json.Array.init(arena); + return root; +} + +fn readTranscript(dir: Io.Dir, gpa: Allocator, rel: []const u8) ![]u8 { + return dir.readFileAlloc(testing.io, rel, gpa, .limited(1 << 20)); +} + +fn countLines(data: []const u8) usize { + return std.mem.count(u8, data, "\n"); +} + +test "one line per message, however many times the autosave runs (#441)" { + const io = testing.io; + const gpa = testing.allocator; + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const a = arena_state.allocator(); + resetForTest(); + tool_spill.resetForTest(); + defer resetForTest(); + var tmp = testing.tmpDir(.{ .iterate = true }); + defer tmp.cleanup(); + + var root = agentFor(gpa, a, io, "s"); + try root.messages.append(try msg(a, "user", "why did the build fail?")); + record(&root, tmp.dir, "s"); + // The autosave runs again over an unchanged history: no second line. + record(&root, tmp.dir, "s"); + record(&root, tmp.dir, "s"); + const one = try readTranscript(tmp.dir, gpa, ".graff/sessions/s.transcript.jsonl"); + defer gpa.free(one); + try testing.expectEqual(@as(usize, 1), countLines(one)); + try testing.expect(std.mem.indexOf(u8, one, "why did the build fail?") != null); + + // A new turn adds exactly its own lines. + try root.messages.append(try msg(a, "assistant", "error: undefined symbol GRAFF_441")); + record(&root, tmp.dir, "s"); + record(&root, tmp.dir, "s"); + const two = try readTranscript(tmp.dir, gpa, ".graff/sessions/s.transcript.jsonl"); + defer gpa.free(two); + try testing.expectEqual(@as(usize, 2), countLines(two)); + // Append-only at the byte level: the first line is untouched where it was. + try testing.expect(std.mem.startsWith(u8, two, one)); + + // A genuine repeat is a real message, not a duplicate: forward-only + // matching must not swallow it. + try root.messages.append(try msg(a, "user", "why did the build fail?")); + record(&root, tmp.dir, "s"); + const three = try readTranscript(tmp.dir, gpa, ".graff/sessions/s.transcript.jsonl"); + defer gpa.free(three); + try testing.expectEqual(@as(usize, 3), countLines(three)); +} + +test "compaction discards the history; the transcript still has it (#441)" { + const io = testing.io; + const gpa = testing.allocator; + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const a = arena_state.allocator(); + resetForTest(); + tool_spill.resetForTest(); + defer resetForTest(); + var tmp = testing.tmpDir(.{ .iterate = true }); + defer tmp.cleanup(); + + var root = agentFor(gpa, a, io, "c"); + // The exact wording that a summary would paraphrase away. + const detail = "ld: symbol(s) not found for architecture arm64: _graff_441_probe"; + try root.messages.append(try msg(a, "user", "build it")); + try root.messages.append(try msg(a, "tool", detail)); + try root.messages.append(try msg(a, "assistant", "fixing the link order")); + try root.messages.append(try msg(a, "user", "and now?")); + record(&root, tmp.dir, "c"); + const before = try readTranscript(tmp.dir, gpa, ".graff/sessions/c.transcript.jsonl"); + defer gpa.free(before); + try testing.expectEqual(@as(usize, 4), countLines(before)); + + // compact()'s shape: a fresh array of [handoff summary] ++ the recent + // suffix verbatim. The detail is now unreachable from `messages` — the very + // next autosave rewrites the session file without it. + const tail = root.messages.items[3]; + var fresh = std.json.Array.init(a); + try fresh.append(try msg(a, "user", "[summary] we were fixing a link error")); + try fresh.append(tail); + root.messages = fresh; + record(&root, tmp.dir, "c"); + + const after = try readTranscript(tmp.dir, gpa, ".graff/sessions/c.transcript.jsonl"); + defer gpa.free(after); + // (a) every earlier line survived, byte for byte, in place + try testing.expect(std.mem.startsWith(u8, after, before)); + // (b) the discarded detail is still greppable + try testing.expect(std.mem.indexOf(u8, after, detail) != null); + // (c) only the summary was added — the verbatim tail is not duplicated + try testing.expectEqual(@as(usize, 5), countLines(after)); + try testing.expectEqual(@as(usize, 1), std.mem.count(u8, after, "and now?")); + try testing.expect(std.mem.indexOf(u8, after, "[summary] we were fixing") != null); + + // And the turn after the compaction keeps appending normally. + try root.messages.append(try msg(a, "assistant", "linked clean")); + record(&root, tmp.dir, "c"); + const later = try readTranscript(tmp.dir, gpa, ".graff/sessions/c.transcript.jsonl"); + defer gpa.free(later); + try testing.expectEqual(@as(usize, 6), countLines(later)); + try testing.expect(std.mem.startsWith(u8, later, after)); +} + +test "a resumed session does not re-append its restored history (#441)" { + const io = testing.io; + const gpa = testing.allocator; + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const a = arena_state.allocator(); + resetForTest(); + tool_spill.resetForTest(); + defer resetForTest(); + var tmp = testing.tmpDir(.{ .iterate = true }); + defer tmp.cleanup(); + + var root = agentFor(gpa, a, io, "r"); + try root.messages.append(try msg(a, "user", "first")); + try root.messages.append(try msg(a, "assistant", "second")); + record(&root, tmp.dir, "r"); + + // A new process resumes "r": same history, no in-memory state at all. The + // digests are re-seeded from the file, so nothing is written twice. + resetForTest(); + record(&root, tmp.dir, "r"); + const data = try readTranscript(tmp.dir, gpa, ".graff/sessions/r.transcript.jsonl"); + defer gpa.free(data); + try testing.expectEqual(@as(usize, 2), countLines(data)); + try testing.expectEqual(@as(usize, 2), lineCount()); + // The accessor #411 uses answers for the attached session only. + try testing.expectEqualStrings(".graff/sessions/r.transcript.jsonl", activePath(&root, a).?); + root.session_name = "somewhere-else"; + try testing.expect(activePath(&root, a) == null); +} + +test "a subagent writes no transcript (#441)" { + const io = testing.io; + const gpa = testing.allocator; + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const a = arena_state.allocator(); + resetForTest(); + tool_spill.resetForTest(); + defer resetForTest(); + var tmp = testing.tmpDir(.{ .iterate = true }); + defer tmp.cleanup(); + + var root = agentFor(gpa, a, io, "sub-session"); + root.sub = true; + try root.messages.append(try msg(a, "user", "delegated mandate")); + record(&root, tmp.dir, "sub-session"); + try testing.expect(tmp.dir.statFile(io, ".graff/sessions/sub-session.transcript.jsonl", .{}) == error.FileNotFound); + try testing.expect(activePath(&root, a) == null); + + // A name that could escape the sessions dir writes nothing either. + root.sub = false; + record(&root, tmp.dir, "../escape"); + try testing.expectEqual(@as(usize, 0), lineCount()); +} + +test "the size cap rotates the generation instead of rewriting it (#441)" { + const io = testing.io; + const gpa = testing.allocator; + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const a = arena_state.allocator(); + resetForTest(); + tool_spill.resetForTest(); + const saved_cap = cap_bytes; + defer { + cap_bytes = saved_cap; + resetForTest(); + } + cap_bytes = 400; + + var tmp = testing.tmpDir(.{ .iterate = true }); + defer tmp.cleanup(); + var root = agentFor(gpa, a, io, "rot"); + try root.messages.append(try msg(a, "user", "the oldest thing said, worth keeping")); + record(&root, tmp.dir, "rot"); + const first = try readTranscript(tmp.dir, gpa, ".graff/sessions/rot.transcript.jsonl"); + defer gpa.free(first); + + // Enough turns to pass the cap. + var i: usize = 0; + while (i < 8) : (i += 1) { + try root.messages.append(try msg(a, "assistant", "0123456789012345678901234567890123456789")); + record(&root, tmp.dir, "rot"); + } + + // The previous generation holds the old bytes UNCHANGED — nothing was read, + // trimmed and written back — and the oldest line is still greppable there. + const prev = try readTranscript(tmp.dir, gpa, ".graff/sessions/rot.transcript.1.jsonl"); + defer gpa.free(prev); + try testing.expect(std.mem.startsWith(u8, prev, first)); + try testing.expect(std.mem.indexOf(u8, prev, "the oldest thing said") != null); + // Both generations respect the cap, so the session is bounded at 2x it. + try testing.expect(prev.len <= cap_bytes); + const live = try readTranscript(tmp.dir, gpa, ".graff/sessions/rot.transcript.jsonl"); + defer gpa.free(live); + try testing.expect(live.len > 0 and live.len <= cap_bytes); + // Rotation is a file operation, not an identity reset: the messages already + // written are still not re-appended to the fresh generation. + const lines_before = lineCount(); + record(&root, tmp.dir, "rot"); + try testing.expectEqual(lines_before, lineCount()); +} diff --git a/src/tool_spill.zig b/src/tool_spill.zig index f7252957..c98aa308 100644 --- a/src/tool_spill.zig +++ b/src/tool_spill.zig @@ -6,10 +6,11 @@ //! //! Layout: `.graff/sessions//artifacts/tool-.txt`, a sibling of that //! session's `.session.json`. Two bounds keep it from growing without -//! limit: `session_cap_bytes` per session, and reclamation of the artifact dirs -//! whose session file is gone (`sweepOnce`) — the session FILE is the ground -//! truth for "this session was deleted", so an `rm`, the AI-title rename, or a -//! `/new` all reclaim what they left behind. +//! limit: `session_cap_bytes` per session, and reclamation of the leftovers +//! whose session file is gone (`sweepSessionsOnce`) — the session FILE is the +//! ground truth for "this session was deleted", so an `rm`, the AI-title +//! rename, or a `/new` all reclaim what they left behind. That sweep is shared +//! with #441's transcripts: same rule, same grace window, one lifecycle. //! //! Spilling is off until `enable` wires a target, and never happens for a //! subagent: its history is not persisted, so there is no durable session to @@ -165,35 +166,58 @@ fn refund(len: usize) ?[]const u8 { return null; } -/// Reclaim the artifact dirs of sessions that no longer exist. Once per process, -/// at the FIRST spill: a run that never spills does no extra I/O at all, and by -/// then this session's own dir is either current (skipped by name) or still -/// young enough for the grace window. +/// Everything a session leaves BESIDE its `.session.json`: this file's +/// artifacts directory, and #441's transcript generations. Returns the session +/// base name that owns `entry`, or null when the entry is not leftovers (the +/// session files themselves, anything unrecognized). One rule, one sweep — a +/// transcript must not outlive its session any more than an artifact does. +pub fn leftoverOwner(entry: []const u8, is_dir: bool) ?[]const u8 { + if (is_dir) return if (entry.len == 0) null else entry; + inline for (.{ session_index.transcript_ext, session_index.transcript_rotated_ext }) |ext| { + if (std.mem.endsWith(u8, entry, ext) and entry.len > ext.len) return entry[0 .. entry.len - ext.len]; + } + return null; +} + +const Leftover = struct { name: []const u8, owner: []const u8, is_dir: bool }; + +/// The session-lifecycle sweep, shared with session_transcript.zig (#441). +/// Runs once per process, at whichever comes first: the first spill or the +/// first transcript append. A run that does neither does no extra I/O at all, +/// and by then this session's own leftovers are either current (skipped by +/// name) or still young enough for the grace window. /// /// Reclaiming late is deliberate. The AI-title rename deletes the old session /// file mid-conversation; MOVING that session's artifacts with it (or deleting /// them there and then) would strand every path already handed to the model in -/// this transcript. Leaving them put keeps those paths valid for the rest of the -/// session, and the next run collects what the rename left behind. +/// this conversation. Leaving them put keeps those paths valid for the rest of +/// the session, and the next run collects what the rename left behind. +pub fn sweepSessionsOnce(io: Io, dir: Io.Dir, arena: Allocator, current: []const u8) void { + sweepOnce(.{ .io = io, .dir = dir, .base_abs = "" }, arena, current); +} + fn sweepOnce(sink: Sink, arena: Allocator, current: []const u8) void { if (g_swept.swap(true, .monotonic)) return; - var names: std.ArrayList([]const u8) = .empty; // collect first: deleting mid-iteration is not portable + var found: std.ArrayList(Leftover) = .empty; // collect first: deleting mid-iteration is not portable { var dir = sink.dir.openDir(sink.io, session_index.sessions_dir, .{ .iterate = true }) catch return; defer dir.close(sink.io); var it = dir.iterate(); while (it.next(sink.io) catch null) |entry| { - if (entry.kind != .directory or std.mem.eql(u8, entry.name, current)) continue; - names.append(arena, arena.dupe(u8, entry.name) catch continue) catch continue; + const is_dir = entry.kind == .directory; + const name = arena.dupe(u8, entry.name) catch continue; + const owner = leftoverOwner(name, is_dir) orelse continue; + if (std.mem.eql(u8, owner, current)) continue; // this session's own + found.append(arena, .{ .name = name, .owner = owner, .is_dir = is_dir }) catch continue; } } const now = util.unixMs(sink.io); - for (names.items) |name| { - const path = std.fmt.allocPrint(arena, "{s}/{s}", .{ session_index.sessions_dir, name }) catch continue; - const file = std.fmt.allocPrint(arena, "{s}{s}", .{ path, session_index.session_ext }) catch continue; + for (found.items) |left| { + const path = std.fmt.allocPrint(arena, "{s}/{s}", .{ session_index.sessions_dir, left.name }) catch continue; + const file = std.fmt.allocPrint(arena, "{s}/{s}{s}", .{ session_index.sessions_dir, left.owner, session_index.session_ext }) catch continue; const live = if (sink.dir.statFile(sink.io, file, .{})) |_| true else |_| false; if (!reclaimable(live, ageMs(sink, path, now), orphan_grace_ms)) continue; - sink.dir.deleteTree(sink.io, path) catch {}; + if (left.is_dir) sink.dir.deleteTree(sink.io, path) catch {} else sink.dir.deleteFile(sink.io, path) catch {}; } } @@ -402,3 +426,42 @@ test "artifacts are reclaimed with their session, and a live session keeps its o _ = try tmp.dir.statFile(io, ".graff/sessions/alive/artifacts/tool-9.txt", .{}); _ = try tmp.dir.statFile(io, ".graff/sessions/current/artifacts/tool-0.txt", .{}); } + +test "the same sweep reclaims #441's transcripts: one lifecycle, not two" { + // The suffix rule, first: only leftovers have an owner, and the session + // files themselves must never be mistaken for one. + try std.testing.expectEqualStrings("s1", leftoverOwner("s1", true).?); + try std.testing.expectEqualStrings("s1", leftoverOwner("s1.transcript.jsonl", false).?); + try std.testing.expectEqualStrings("s1", leftoverOwner("s1.transcript.1.jsonl", false).?); + try std.testing.expect(leftoverOwner("s1.session.json", false) == null); + try std.testing.expect(leftoverOwner(".transcript.jsonl", false) == null); // no session owns it + + const io = std.testing.io; + var tmp = std.testing.tmpDir(.{ .iterate = true }); + defer tmp.cleanup(); + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const a = arena_state.allocator(); + resetForTest(); + const saved_grace = orphan_grace_ms; + defer { + orphan_grace_ms = saved_grace; + resetForTest(); + } + orphan_grace_ms = 0; + + try tmp.dir.createDirPath(io, ".graff/sessions"); + try tmp.dir.writeFile(io, .{ .sub_path = ".graff/sessions/gone.transcript.jsonl", .data = "{}\n" }); + try tmp.dir.writeFile(io, .{ .sub_path = ".graff/sessions/gone.transcript.1.jsonl", .data = "{}\n" }); + try tmp.dir.writeFile(io, .{ .sub_path = ".graff/sessions/kept.transcript.jsonl", .data = "{}\n" }); + try tmp.dir.writeFile(io, .{ .sub_path = ".graff/sessions/kept.session.json", .data = "{}" }); + try tmp.dir.writeFile(io, .{ .sub_path = ".graff/sessions/now.transcript.jsonl", .data = "{}\n" }); + + sweepSessionsOnce(io, tmp.dir, a, "now"); // "now" is the live session + + try std.testing.expect(tmp.dir.statFile(io, ".graff/sessions/gone.transcript.jsonl", .{}) == error.FileNotFound); + try std.testing.expect(tmp.dir.statFile(io, ".graff/sessions/gone.transcript.1.jsonl", .{}) == error.FileNotFound); + _ = try tmp.dir.statFile(io, ".graff/sessions/kept.transcript.jsonl", .{}); // its session is still saved + _ = try tmp.dir.statFile(io, ".graff/sessions/now.transcript.jsonl", .{}); // and the live one is never touched + _ = try tmp.dir.statFile(io, ".graff/sessions/kept.session.json", .{}); // the sweep deletes leftovers, not sessions +} From 8e7f8112baf2d8d59ed575b3b3fc4166bf4f9260 Mon Sep 17 00:00:00 2001 From: Rach Pradhan <54503978+justrach@users.noreply.github.com> Date: Thu, 6 Aug 2026 19:20:05 +0800 Subject: [PATCH 2/4] fix(session): the transcript never wrote a byte on windows (#441) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One genuine portability defect, not five test bugs. Every Windows failure — four wrong line counts and one null deref — is the same root cause, and the CI log names it: each count was "found 0", and activePath was null because nothing had been recorded. `appendWhole` opened the file write-only and then called `File.stat` on that handle to find the append offset. On Windows, Io.Dir.createFile maps `read` straight onto the NT access mask (`GENERIC = .{ .WRITE = true, .READ = flags.read }`), so a write-only handle carries FILE_GENERIC_WRITE, which does NOT include FILE_READ_ATTRIBUTES. `File.stat` is NtQueryInformationFile(.All), which requires exactly that right, and std handles the resulting ACCESS_DENIED explicitly. So on Windows the stat failed, `appendWhole` returned null having ALREADY created the file, and every transcript was a 0-byte file: the feature was inert on the platform, and #411's note would have cited an empty file. `.read = true` on the create is the fix; a path-based `statFile` fallback keeps a future std change from turning this back into a silent no-write rather than an append. Nothing else about the write changed — it is still one positional write at the end of file, still append-only. This class is invisible to the POSIX suite: with the fix reverted, macOS still runs 1050/1050 green. CI was the only possible signal, which is the argument for the two hardenings below rather than for trusting a local pass. The other two suspects, checked and reported rather than assumed: - Line endings: NOT a defect. '\n' is written explicitly and JSON escapes every control character, so a message is always exactly one line and no '\r' can appear. Hardened anyway, since the digests are over line bytes and a stray '\r' would silently re-append the entire history: `seed` trims a trailing '\r', and the tests now fail hard if a '\r' ever reaches the file. - Rotation's rename: NOT a defect. Windows rename does not overwrite, but Io.Dir.rename is the REPLACING variant on every OS (dirRenameWindows passes replace_if_exists=true; renamePreserve is the one that refuses a taken name). The rotation test only ever rotated onto a free name and could not have told the difference, so it now rotates twice and asserts the previous generation really was replaced. INVARIANT, now stated where the paths are built (session_index.zig) because two downstream branches depend on the answer: every `.graff/` path this harness builds is forward-slashed on every platform, Windows included. Deliberately — Windows accepts '/' in the paths reaching Io.Dir, and these strings are shown to the model (#410's prompt line, #409's cap marker, #441's path inside #411's note), so a shape that changes per platform buys nothing and costs goldens. Separators were never the Windows failure here; the null deref was a downstream symptom of the empty file. The corollary is for tests, and ee28d8c is the precedent: assert on a basename or on a path built through the helpers, never by matching a separator by hand. Both `activePath` assertions now go through the accessor and check the cited file exists; the readers build their paths with `transcriptPath`/`rotatedPath` so a test can no longer disagree with the code. The tests moved to session_transcript_tests.zig: the hardening pushed the module to 614 lines, over the ceiling. Reachability needed the `_ = ...` line in test_hooks.zig's test block, not just the import — the count caught it at 1045 before it was added, and is 1050 again after. Co-Authored-By: Codegraff --- src/session_index.zig | 17 ++ src/session_tests.zig | 10 +- src/session_transcript.zig | 270 ++++----------------------- src/session_transcript_tests.zig | 304 +++++++++++++++++++++++++++++++ src/test_hooks.zig | 6 + 5 files changed, 370 insertions(+), 237 deletions(-) create mode 100644 src/session_transcript_tests.zig diff --git a/src/session_index.zig b/src/session_index.zig index a6d1722d..1a42d134 100644 --- a/src/session_index.zig +++ b/src/session_index.zig @@ -18,6 +18,23 @@ pub const session_ext = ".session.json"; /// Title-named session files live here (resume reads this). pub const sessions_dir = ".graff/sessions"; +/// INVARIANT: every `.graff/` path this harness builds is FORWARD-SLASHED on +/// every platform, including Windows. Not an accident of `allocPrint` — a +/// choice, and callers may rely on it: +/// +/// - Windows accepts '/' in the paths reaching Io.Dir (sliceToPrefixedFileW +/// normalizes them), so nothing is lost by not using the platform join. +/// - These strings are SHOWN TO THE MODEL. The #410 system-prompt line names +/// the session file, #409's cap marker cites an artifact, and #441's +/// transcript path travels into #411's post-compaction note. A path whose +/// shape changes per platform makes those prompts, and the goldens that +/// pin them, harder to reason about for no gain. +/// +/// The corollary is for TESTS: assert on the basename, or on a path built +/// through these helpers — never by matching a separator by hand against +/// something the OS produced. A path that came back OUT of the OS +/// (realPathFile and friends, as in #409's spill marker) is the platform's +/// shape, not ours, and ee28d8c is the commit that learned it. /// #441: the append-only transcript beside each session file, and the one /// rotated generation behind it. The suffixes live here, with the session /// suffix, because BOTH the writer (session_transcript.zig) and the sweep that diff --git a/src/session_tests.zig b/src/session_tests.zig index c2a837c9..c469bf9f 100644 --- a/src/session_tests.zig +++ b/src/session_tests.zig @@ -323,10 +323,14 @@ test "the autosave is where the transcript sees a new message (#441)" { // The real save path wrote the transcript beside the session file, one line // per message, with no hook of its own at any of the history's mutation - // sites. #411's note gets the path from the accessor, not by rebuilding it. + // sites. #411's note gets the path from the accessor, not by rebuilding it, + // and the assertion goes through that accessor rather than a hand-written + // separator: the path is forward-slashed on every platform by design + // (session_index.zig), but what matters here is that it names a real file. f.root.session_name = "wf"; - try std.testing.expectEqualStrings(".graff/sessions/wf.transcript.jsonl", session_transcript.activePath(&f.root, arena).?); - const lines = try tmp.dir.readFileAlloc(io, ".graff/sessions/wf.transcript.jsonl", gpa, .limited(64 * 1024)); + const path = session_transcript.activePath(&f.root, arena) orelse return error.TestUnexpectedResult; + try std.testing.expectEqualStrings("wf.transcript.jsonl", std.fs.path.basename(path)); + const lines = try tmp.dir.readFileAlloc(io, path, gpa, .limited(64 * 1024)); defer gpa.free(lines); try std.testing.expectEqual(@as(usize, 2), std.mem.count(u8, lines, "\n")); try std.testing.expectEqual(@as(usize, 2), session_transcript.lineCount()); diff --git a/src/session_transcript.zig b/src/session_transcript.zig index cc15461b..41a8aad9 100644 --- a/src/session_transcript.zig +++ b/src/session_transcript.zig @@ -73,6 +73,9 @@ pub var cap_bytes: usize = 16 * 1024 * 1024; /// `.graff/sessions/.transcript.jsonl`. Pure; #411's post-compaction note /// and anything else that needs the path calls this instead of re-deriving the /// filename. +/// +/// Forward-slashed on every platform, Windows included — deliberately, and +/// downstream may rely on it. session_index.zig states the invariant and why. pub fn transcriptPath(arena: Allocator, name: []const u8) ![]const u8 { return std.fmt.allocPrint(arena, "{s}/{s}{s}", .{ session_index.sessions_dir, name, transcript_ext }); } @@ -242,8 +245,13 @@ fn seed(io: Io, dir: Io.Dir, arena: Allocator, name: []const u8) void { const path = transcriptPath(arena, name) catch return; const data = dir.readFileAlloc(io, path, arena, .limited(cap_bytes)) catch return; g.bytes = data.len; + // '\n' explicitly, both here and in `flush` — never a platform default. + // The digest is over the line's bytes, so a stray '\r' from an editor that + // rewrote the file with CRLF would silently stop every line matching and + // re-append the whole history; strip it rather than trust the writer. var it = std.mem.splitScalar(u8, data, '\n'); - while (it.next()) |line| { + while (it.next()) |raw| { + const line = std.mem.trimEnd(u8, raw, "\r"); if (line.len == 0) continue; // the trailing newline, or a torn write bump(digest(line)); } @@ -274,13 +282,27 @@ fn flush(io: Io, dir: Io.Dir, arena: Allocator, name: []const u8, pending: []con /// or rewritten — that is the whole append-only guarantee, and it is why a /// compaction running against the same session cannot cost the transcript a /// byte. Returns the new size. +/// +/// `.read = true` is load-bearing on WINDOWS and cost-free everywhere else. +/// Io.Dir.createFile maps `read` straight onto the NT access mask +/// (`GENERIC = .{ .WRITE = true, .READ = flags.read }`), and a write-only +/// handle carries FILE_GENERIC_WRITE, which does NOT include +/// FILE_READ_ATTRIBUTES. `File.stat` is NtQueryInformationFile(.All), which +/// needs exactly that right, so on a write-only handle it fails with +/// ACCESS_DENIED — and this function then returned null having already CREATED +/// the file, so every Windows transcript was an empty file and no message was +/// ever recorded. The path-based `statFile` fallback keeps a future std change +/// from re-introducing a silent no-write instead of an append. fn appendWhole(io: Io, dir: Io.Dir, path: []const u8, data: []const u8) ?usize { dir.createDirPath(io, session_index.sessions_dir) catch {}; - const f = dir.createFile(io, path, .{ .truncate = false }) catch return null; + const f = dir.createFile(io, path, .{ .truncate = false, .read = true }) catch return null; defer f.close(io); - const st = f.stat(io) catch return null; - f.writePositionalAll(io, data, st.size) catch return null; - return @as(usize, @intCast(st.size)) + data.len; + const end: u64 = if (f.stat(io)) |st| st.size else |_| blk: { + const st = dir.statFile(io, path, .{}) catch return null; + break :blk st.size; + }; + f.writePositionalAll(io, data, end) catch return null; + return @as(usize, @intCast(end)) + data.len; } /// At the cap the live generation is RENAMED aside, never trimmed in place. @@ -293,6 +315,13 @@ fn appendWhole(io: Io, dir: Io.Dir, path: []const u8, data: []const u8) ?usize { /// live one, every line ever written stays a line somewhere, and the bound is /// simply two generations. The previous generation is what the next rotation /// replaces, so disk use is capped rather than merely slowed. +/// +/// Replacing an EXISTING previous generation is the part Windows would +/// normally refuse (its rename does not overwrite). Io.Dir.rename is the +/// replacing one on every OS — dirRenameWindows passes replace_if_exists=true, +/// and Io.Dir.renamePreserve is the variant that fails on a taken name — so +/// the second and every later rotation works there too. The test below rotates +/// twice for exactly that reason. fn rotate(io: Io, dir: Io.Dir, arena: Allocator, name: []const u8) void { const live = transcriptPath(arena, name) catch return; const prev = rotatedPath(arena, name) catch return; @@ -306,232 +335,5 @@ pub fn resetForTest() void { detach(); } -// ── tests ──────────────────────────────────────────────────────────────── - -const testing = std.testing; - -fn msg(arena: Allocator, role: []const u8, text: []const u8) !Value { - var o: std.json.ObjectMap = .empty; - try o.put(arena, "role", .{ .string = role }); - try o.put(arena, "content", .{ .string = text }); - return .{ .object = o }; -} - -/// The fields `record` reads, and nothing else. -fn agentFor(gpa: Allocator, arena: Allocator, io: Io, name: []const u8) Agent { - var root: Agent = undefined; - root.gpa = gpa; - root.io = io; - root.sub = false; - root.session_name = name; - root.messages = std.json.Array.init(arena); - return root; -} - -fn readTranscript(dir: Io.Dir, gpa: Allocator, rel: []const u8) ![]u8 { - return dir.readFileAlloc(testing.io, rel, gpa, .limited(1 << 20)); -} - -fn countLines(data: []const u8) usize { - return std.mem.count(u8, data, "\n"); -} - -test "one line per message, however many times the autosave runs (#441)" { - const io = testing.io; - const gpa = testing.allocator; - var arena_state = std.heap.ArenaAllocator.init(gpa); - defer arena_state.deinit(); - const a = arena_state.allocator(); - resetForTest(); - tool_spill.resetForTest(); - defer resetForTest(); - var tmp = testing.tmpDir(.{ .iterate = true }); - defer tmp.cleanup(); - - var root = agentFor(gpa, a, io, "s"); - try root.messages.append(try msg(a, "user", "why did the build fail?")); - record(&root, tmp.dir, "s"); - // The autosave runs again over an unchanged history: no second line. - record(&root, tmp.dir, "s"); - record(&root, tmp.dir, "s"); - const one = try readTranscript(tmp.dir, gpa, ".graff/sessions/s.transcript.jsonl"); - defer gpa.free(one); - try testing.expectEqual(@as(usize, 1), countLines(one)); - try testing.expect(std.mem.indexOf(u8, one, "why did the build fail?") != null); - - // A new turn adds exactly its own lines. - try root.messages.append(try msg(a, "assistant", "error: undefined symbol GRAFF_441")); - record(&root, tmp.dir, "s"); - record(&root, tmp.dir, "s"); - const two = try readTranscript(tmp.dir, gpa, ".graff/sessions/s.transcript.jsonl"); - defer gpa.free(two); - try testing.expectEqual(@as(usize, 2), countLines(two)); - // Append-only at the byte level: the first line is untouched where it was. - try testing.expect(std.mem.startsWith(u8, two, one)); - - // A genuine repeat is a real message, not a duplicate: forward-only - // matching must not swallow it. - try root.messages.append(try msg(a, "user", "why did the build fail?")); - record(&root, tmp.dir, "s"); - const three = try readTranscript(tmp.dir, gpa, ".graff/sessions/s.transcript.jsonl"); - defer gpa.free(three); - try testing.expectEqual(@as(usize, 3), countLines(three)); -} - -test "compaction discards the history; the transcript still has it (#441)" { - const io = testing.io; - const gpa = testing.allocator; - var arena_state = std.heap.ArenaAllocator.init(gpa); - defer arena_state.deinit(); - const a = arena_state.allocator(); - resetForTest(); - tool_spill.resetForTest(); - defer resetForTest(); - var tmp = testing.tmpDir(.{ .iterate = true }); - defer tmp.cleanup(); - - var root = agentFor(gpa, a, io, "c"); - // The exact wording that a summary would paraphrase away. - const detail = "ld: symbol(s) not found for architecture arm64: _graff_441_probe"; - try root.messages.append(try msg(a, "user", "build it")); - try root.messages.append(try msg(a, "tool", detail)); - try root.messages.append(try msg(a, "assistant", "fixing the link order")); - try root.messages.append(try msg(a, "user", "and now?")); - record(&root, tmp.dir, "c"); - const before = try readTranscript(tmp.dir, gpa, ".graff/sessions/c.transcript.jsonl"); - defer gpa.free(before); - try testing.expectEqual(@as(usize, 4), countLines(before)); - - // compact()'s shape: a fresh array of [handoff summary] ++ the recent - // suffix verbatim. The detail is now unreachable from `messages` — the very - // next autosave rewrites the session file without it. - const tail = root.messages.items[3]; - var fresh = std.json.Array.init(a); - try fresh.append(try msg(a, "user", "[summary] we were fixing a link error")); - try fresh.append(tail); - root.messages = fresh; - record(&root, tmp.dir, "c"); - - const after = try readTranscript(tmp.dir, gpa, ".graff/sessions/c.transcript.jsonl"); - defer gpa.free(after); - // (a) every earlier line survived, byte for byte, in place - try testing.expect(std.mem.startsWith(u8, after, before)); - // (b) the discarded detail is still greppable - try testing.expect(std.mem.indexOf(u8, after, detail) != null); - // (c) only the summary was added — the verbatim tail is not duplicated - try testing.expectEqual(@as(usize, 5), countLines(after)); - try testing.expectEqual(@as(usize, 1), std.mem.count(u8, after, "and now?")); - try testing.expect(std.mem.indexOf(u8, after, "[summary] we were fixing") != null); - - // And the turn after the compaction keeps appending normally. - try root.messages.append(try msg(a, "assistant", "linked clean")); - record(&root, tmp.dir, "c"); - const later = try readTranscript(tmp.dir, gpa, ".graff/sessions/c.transcript.jsonl"); - defer gpa.free(later); - try testing.expectEqual(@as(usize, 6), countLines(later)); - try testing.expect(std.mem.startsWith(u8, later, after)); -} - -test "a resumed session does not re-append its restored history (#441)" { - const io = testing.io; - const gpa = testing.allocator; - var arena_state = std.heap.ArenaAllocator.init(gpa); - defer arena_state.deinit(); - const a = arena_state.allocator(); - resetForTest(); - tool_spill.resetForTest(); - defer resetForTest(); - var tmp = testing.tmpDir(.{ .iterate = true }); - defer tmp.cleanup(); - - var root = agentFor(gpa, a, io, "r"); - try root.messages.append(try msg(a, "user", "first")); - try root.messages.append(try msg(a, "assistant", "second")); - record(&root, tmp.dir, "r"); - - // A new process resumes "r": same history, no in-memory state at all. The - // digests are re-seeded from the file, so nothing is written twice. - resetForTest(); - record(&root, tmp.dir, "r"); - const data = try readTranscript(tmp.dir, gpa, ".graff/sessions/r.transcript.jsonl"); - defer gpa.free(data); - try testing.expectEqual(@as(usize, 2), countLines(data)); - try testing.expectEqual(@as(usize, 2), lineCount()); - // The accessor #411 uses answers for the attached session only. - try testing.expectEqualStrings(".graff/sessions/r.transcript.jsonl", activePath(&root, a).?); - root.session_name = "somewhere-else"; - try testing.expect(activePath(&root, a) == null); -} - -test "a subagent writes no transcript (#441)" { - const io = testing.io; - const gpa = testing.allocator; - var arena_state = std.heap.ArenaAllocator.init(gpa); - defer arena_state.deinit(); - const a = arena_state.allocator(); - resetForTest(); - tool_spill.resetForTest(); - defer resetForTest(); - var tmp = testing.tmpDir(.{ .iterate = true }); - defer tmp.cleanup(); - - var root = agentFor(gpa, a, io, "sub-session"); - root.sub = true; - try root.messages.append(try msg(a, "user", "delegated mandate")); - record(&root, tmp.dir, "sub-session"); - try testing.expect(tmp.dir.statFile(io, ".graff/sessions/sub-session.transcript.jsonl", .{}) == error.FileNotFound); - try testing.expect(activePath(&root, a) == null); - - // A name that could escape the sessions dir writes nothing either. - root.sub = false; - record(&root, tmp.dir, "../escape"); - try testing.expectEqual(@as(usize, 0), lineCount()); -} - -test "the size cap rotates the generation instead of rewriting it (#441)" { - const io = testing.io; - const gpa = testing.allocator; - var arena_state = std.heap.ArenaAllocator.init(gpa); - defer arena_state.deinit(); - const a = arena_state.allocator(); - resetForTest(); - tool_spill.resetForTest(); - const saved_cap = cap_bytes; - defer { - cap_bytes = saved_cap; - resetForTest(); - } - cap_bytes = 400; - - var tmp = testing.tmpDir(.{ .iterate = true }); - defer tmp.cleanup(); - var root = agentFor(gpa, a, io, "rot"); - try root.messages.append(try msg(a, "user", "the oldest thing said, worth keeping")); - record(&root, tmp.dir, "rot"); - const first = try readTranscript(tmp.dir, gpa, ".graff/sessions/rot.transcript.jsonl"); - defer gpa.free(first); - - // Enough turns to pass the cap. - var i: usize = 0; - while (i < 8) : (i += 1) { - try root.messages.append(try msg(a, "assistant", "0123456789012345678901234567890123456789")); - record(&root, tmp.dir, "rot"); - } - - // The previous generation holds the old bytes UNCHANGED — nothing was read, - // trimmed and written back — and the oldest line is still greppable there. - const prev = try readTranscript(tmp.dir, gpa, ".graff/sessions/rot.transcript.1.jsonl"); - defer gpa.free(prev); - try testing.expect(std.mem.startsWith(u8, prev, first)); - try testing.expect(std.mem.indexOf(u8, prev, "the oldest thing said") != null); - // Both generations respect the cap, so the session is bounded at 2x it. - try testing.expect(prev.len <= cap_bytes); - const live = try readTranscript(tmp.dir, gpa, ".graff/sessions/rot.transcript.jsonl"); - defer gpa.free(live); - try testing.expect(live.len > 0 and live.len <= cap_bytes); - // Rotation is a file operation, not an identity reset: the messages already - // written are still not re-appended to the fresh generation. - const lines_before = lineCount(); - record(&root, tmp.dir, "rot"); - try testing.expectEqual(lines_before, lineCount()); -} +// The tests live in session_transcript_tests.zig — the 600-line cap — and are +// reached through test_hooks.zig. diff --git a/src/session_transcript_tests.zig b/src/session_transcript_tests.zig new file mode 100644 index 00000000..bc2c1663 --- /dev/null +++ b/src/session_transcript_tests.zig @@ -0,0 +1,304 @@ +//! session_transcript.zig's tests (#441). They live here for the 600-line cap, +//! mirroring session_tests.zig and agent_overflow_tests.zig; the module they +//! cover is reached in production from session.queueSave, but nothing +//! references THIS file outside test_hooks.zig, which is what makes its tests +//! run at all (AGENTS.md). +//! +//! The cases that earn their keep: a rewrite of the history leaves every line +//! already on disk byte-identical and in place, a repeated autosave adds +//! nothing, a resume re-seeds from the file instead of re-appending, a subagent +//! writes nothing, and the cap rotates TWICE so a rename that refused an +//! existing target could not pass. + +const std = @import("std"); +const Io = std.Io; +const Value = std.json.Value; +const Allocator = std.mem.Allocator; + +const agent_mod = @import("agent.zig"); +const Agent = agent_mod.Agent; +const tool_spill = @import("tool_spill.zig"); +const transcript = @import("session_transcript.zig"); +const activePath = transcript.activePath; +const lineCount = transcript.lineCount; +const record = transcript.record; +const resetForTest = transcript.resetForTest; +const rotatedPath = transcript.rotatedPath; +const transcriptPath = transcript.transcriptPath; +const transcript_ext = transcript.transcript_ext; + +const testing = std.testing; + +fn msg(arena: Allocator, role: []const u8, text: []const u8) !Value { + var o: std.json.ObjectMap = .empty; + try o.put(arena, "role", .{ .string = role }); + try o.put(arena, "content", .{ .string = text }); + return .{ .object = o }; +} + +/// The fields `record` reads, and nothing else. +fn agentFor(gpa: Allocator, arena: Allocator, io: Io, name: []const u8) Agent { + var root: Agent = undefined; + root.gpa = gpa; + root.io = io; + root.sub = false; + root.session_name = name; + root.messages = std.json.Array.init(arena); + return root; +} + +/// Read a generation through the SAME builder the writer uses, so the test can +/// never disagree with the code about where the file is — and so no separator +/// is ever written by hand here (session_index.zig's invariant). +fn readGen(dir: Io.Dir, gpa: Allocator, arena: Allocator, name: []const u8, rotated: bool) ![]u8 { + const rel = if (rotated) try rotatedPath(arena, name) else try transcriptPath(arena, name); + return dir.readFileAlloc(testing.io, rel, gpa, .limited(1 << 20)); +} + +/// Lines are '\n'-terminated by construction. The '\r' check is the guard: a +/// writer that ever picked up a platform line ending would break the digests +/// silently (every line would stop matching and the history would be +/// re-appended), so make it a hard failure here instead. +fn countLines(data: []const u8) !usize { + try testing.expect(std.mem.indexOfScalar(u8, data, '\r') == null); + return std.mem.count(u8, data, "\n"); +} + +/// `activePath` asserted portably: it is the path of a file that really exists, +/// and its basename is the one we expect. Matching a '/'-joined literal would +/// pass today (the builder is forward-slashed on every platform, deliberately) +/// but it asserts the separator rather than the behaviour — ee28d8c's lesson. +fn expectActive(root: *Agent, arena: Allocator, dir: Io.Dir, name: []const u8) !void { + const p = activePath(root, arena) orelse return error.TestUnexpectedResult; + const want = try std.fmt.allocPrint(arena, "{s}{s}", .{ name, transcript_ext }); + try testing.expectEqualStrings(want, std.fs.path.basename(p)); + _ = try dir.statFile(testing.io, p, .{}); // the cited file is really there +} + +test "one line per message, however many times the autosave runs (#441)" { + const io = testing.io; + const gpa = testing.allocator; + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const a = arena_state.allocator(); + resetForTest(); + tool_spill.resetForTest(); + defer resetForTest(); + var tmp = testing.tmpDir(.{ .iterate = true }); + defer tmp.cleanup(); + + var root = agentFor(gpa, a, io, "s"); + try root.messages.append(try msg(a, "user", "why did the build fail?")); + record(&root, tmp.dir, "s"); + // The autosave runs again over an unchanged history: no second line. + record(&root, tmp.dir, "s"); + record(&root, tmp.dir, "s"); + const one = try readGen(tmp.dir, gpa, a, "s", false); + defer gpa.free(one); + try testing.expectEqual(@as(usize, 1), try countLines(one)); + try testing.expect(std.mem.indexOf(u8, one, "why did the build fail?") != null); + + // A new turn adds exactly its own lines. + try root.messages.append(try msg(a, "assistant", "error: undefined symbol GRAFF_441")); + record(&root, tmp.dir, "s"); + record(&root, tmp.dir, "s"); + const two = try readGen(tmp.dir, gpa, a, "s", false); + defer gpa.free(two); + try testing.expectEqual(@as(usize, 2), try countLines(two)); + // Append-only at the byte level: the first line is untouched where it was. + try testing.expect(std.mem.startsWith(u8, two, one)); + + // A genuine repeat is a real message, not a duplicate: forward-only + // matching must not swallow it. + try root.messages.append(try msg(a, "user", "why did the build fail?")); + record(&root, tmp.dir, "s"); + const three = try readGen(tmp.dir, gpa, a, "s", false); + defer gpa.free(three); + try testing.expectEqual(@as(usize, 3), try countLines(three)); +} + +test "compaction discards the history; the transcript still has it (#441)" { + const io = testing.io; + const gpa = testing.allocator; + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const a = arena_state.allocator(); + resetForTest(); + tool_spill.resetForTest(); + defer resetForTest(); + var tmp = testing.tmpDir(.{ .iterate = true }); + defer tmp.cleanup(); + + var root = agentFor(gpa, a, io, "c"); + // The exact wording that a summary would paraphrase away. + const detail = "ld: symbol(s) not found for architecture arm64: _graff_441_probe"; + try root.messages.append(try msg(a, "user", "build it")); + try root.messages.append(try msg(a, "tool", detail)); + try root.messages.append(try msg(a, "assistant", "fixing the link order")); + try root.messages.append(try msg(a, "user", "and now?")); + record(&root, tmp.dir, "c"); + const before = try readGen(tmp.dir, gpa, a, "c", false); + defer gpa.free(before); + try testing.expectEqual(@as(usize, 4), try countLines(before)); + + // compact()'s shape: a fresh array of [handoff summary] ++ the recent + // suffix verbatim. The detail is now unreachable from `messages` — the very + // next autosave rewrites the session file without it. + const tail = root.messages.items[3]; + var fresh = std.json.Array.init(a); + try fresh.append(try msg(a, "user", "[summary] we were fixing a link error")); + try fresh.append(tail); + root.messages = fresh; + record(&root, tmp.dir, "c"); + + const after = try readGen(tmp.dir, gpa, a, "c", false); + defer gpa.free(after); + // (a) every earlier line survived, byte for byte, in place + try testing.expect(std.mem.startsWith(u8, after, before)); + // (b) the discarded detail is still greppable + try testing.expect(std.mem.indexOf(u8, after, detail) != null); + // (c) only the summary was added — the verbatim tail is not duplicated + try testing.expectEqual(@as(usize, 5), try countLines(after)); + try testing.expectEqual(@as(usize, 1), std.mem.count(u8, after, "and now?")); + try testing.expect(std.mem.indexOf(u8, after, "[summary] we were fixing") != null); + + // And the turn after the compaction keeps appending normally. + try root.messages.append(try msg(a, "assistant", "linked clean")); + record(&root, tmp.dir, "c"); + const later = try readGen(tmp.dir, gpa, a, "c", false); + defer gpa.free(later); + try testing.expectEqual(@as(usize, 6), try countLines(later)); + try testing.expect(std.mem.startsWith(u8, later, after)); +} + +test "a resumed session does not re-append its restored history (#441)" { + const io = testing.io; + const gpa = testing.allocator; + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const a = arena_state.allocator(); + resetForTest(); + tool_spill.resetForTest(); + defer resetForTest(); + var tmp = testing.tmpDir(.{ .iterate = true }); + defer tmp.cleanup(); + + var root = agentFor(gpa, a, io, "r"); + try root.messages.append(try msg(a, "user", "first")); + try root.messages.append(try msg(a, "assistant", "second")); + record(&root, tmp.dir, "r"); + + // A new process resumes "r": same history, no in-memory state at all. The + // digests are re-seeded from the file, so nothing is written twice. + resetForTest(); + record(&root, tmp.dir, "r"); + const data = try readGen(tmp.dir, gpa, a, "r", false); + defer gpa.free(data); + try testing.expectEqual(@as(usize, 2), try countLines(data)); + try testing.expectEqual(@as(usize, 2), lineCount()); + // The accessor #411 uses answers for the attached session only. + try expectActive(&root, a, tmp.dir, "r"); + root.session_name = "somewhere-else"; + try testing.expect(activePath(&root, a) == null); +} + +test "a subagent writes no transcript (#441)" { + const io = testing.io; + const gpa = testing.allocator; + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const a = arena_state.allocator(); + resetForTest(); + tool_spill.resetForTest(); + defer resetForTest(); + var tmp = testing.tmpDir(.{ .iterate = true }); + defer tmp.cleanup(); + + var root = agentFor(gpa, a, io, "sub-session"); + root.sub = true; + try root.messages.append(try msg(a, "user", "delegated mandate")); + record(&root, tmp.dir, "sub-session"); + try testing.expectError(error.FileNotFound, readGen(tmp.dir, gpa, a, "sub-session", false)); + try testing.expect(activePath(&root, a) == null); + + // A name that could escape the sessions dir writes nothing either. + root.sub = false; + record(&root, tmp.dir, "../escape"); + try testing.expectEqual(@as(usize, 0), lineCount()); +} + +test "the size cap rotates the generation instead of rewriting it (#441)" { + const io = testing.io; + const gpa = testing.allocator; + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const a = arena_state.allocator(); + resetForTest(); + tool_spill.resetForTest(); + const saved_cap = transcript.cap_bytes; + defer { + transcript.cap_bytes = saved_cap; + resetForTest(); + } + transcript.cap_bytes = 400; + + var tmp = testing.tmpDir(.{ .iterate = true }); + defer tmp.cleanup(); + var root = agentFor(gpa, a, io, "rot"); + try root.messages.append(try msg(a, "user", "the oldest thing said, worth keeping")); + record(&root, tmp.dir, "rot"); + const first = try readGen(tmp.dir, gpa, a, "rot", false); + defer gpa.free(first); + + // Enough turns to pass the cap. + var i: usize = 0; + while (i < 8) : (i += 1) { + try root.messages.append(try msg(a, "assistant", "0123456789012345678901234567890123456789")); + record(&root, tmp.dir, "rot"); + } + + // The previous generation holds the old bytes UNCHANGED — nothing was read, + // trimmed and written back — and the oldest line is still greppable there. + const prev = try readGen(tmp.dir, gpa, a, "rot", true); + defer gpa.free(prev); + try testing.expect(std.mem.startsWith(u8, prev, first)); + try testing.expect(std.mem.indexOf(u8, prev, "the oldest thing said") != null); + // Both generations respect the cap, so the session is bounded at 2x it. + try testing.expect(prev.len <= transcript.cap_bytes); + const live = try readGen(tmp.dir, gpa, a, "rot", false); + defer gpa.free(live); + try testing.expect(live.len > 0 and live.len <= transcript.cap_bytes); + // Rotation is a file operation, not an identity reset: the messages already + // written are still not re-appended to the fresh generation. + const lines_before = lineCount(); + record(&root, tmp.dir, "rot"); + try testing.expectEqual(lines_before, lineCount()); + + // A SECOND rotation, over a previous generation that already exists. This + // is the case a rename that refuses a taken target would fail — POSIX + // replaces, Windows natively does not, and Io.Dir.rename is the replacing + // variant on both. Without this the suite only ever rotated onto a free + // name and could not tell the difference. + while (i < 40) : (i += 1) { + try root.messages.append(try msg(a, "assistant", "0123456789012345678901234567890123456789")); + record(&root, tmp.dir, "rot"); + } + const prev2 = try readGen(tmp.dir, gpa, a, "rot", true); + defer gpa.free(prev2); + const live2 = try readGen(tmp.dir, gpa, a, "rot", false); + defer gpa.free(live2); + // The previous generation really was REPLACED: it used to be the one + // holding the oldest line, and is not any more. A rename that refused a + // taken target would have left the first generation sitting there and + // stalled the live file past its cap instead. + try testing.expect(std.mem.indexOf(u8, prev, "the oldest thing said") != null); + try testing.expect(std.mem.indexOf(u8, prev2, "the oldest thing said") == null); + // Both generations still hold whole lines, and the cap still binds. + try testing.expect(prev2.len > 0 and prev2.len <= transcript.cap_bytes); + try testing.expect(live2.len > 0 and live2.len <= transcript.cap_bytes); + try testing.expect(std.mem.endsWith(u8, prev2, "\n")); + try testing.expect(std.mem.endsWith(u8, live2, "\n")); + // 1 + 8 + 32 messages ever entered the history, and every one was recorded + // exactly once — no rotation re-appended anything. + try testing.expectEqual(@as(usize, 41), lineCount()); +} diff --git a/src/test_hooks.zig b/src/test_hooks.zig index 033f8746..161458f8 100644 --- a/src/test_hooks.zig +++ b/src/test_hooks.zig @@ -70,6 +70,11 @@ const scoring_slot_test = @import("scoring_slot_test.zig"); // #273: session.zig's own tests, moved off it for the same reason. const session_tests = @import("session_tests.zig"); +// #441: and session_transcript.zig's, moved off it for the same reason again. +// The module itself is reached from session.queueSave, but this FILE is not, so +// without the hook its whole suite compiles to nothing and reports green. +const session_transcript_tests = @import("session_transcript_tests.zig"); + // #375: `graff acp` (Zed's Agent Client Protocol over stdio). args.zig calls // one predicate from it, which analyses the file but does not run its tests. const acp = @import("acp.zig"); @@ -170,6 +175,7 @@ test { _ = serve_create; _ = scoring_slot_test; _ = session_tests; + _ = session_transcript_tests; _ = mcp_config; _ = acp; _ = agent_eval_control_tests; From ea1e62146c0be78b84667f52d854fe01de195bd2 Mon Sep 17 00:00:00 2001 From: Rach Pradhan <54503978+justrach@users.noreply.github.com> Date: Thu, 6 Aug 2026 19:03:38 +0800 Subject: [PATCH 3/4] feat(compact): tell the model what survives a compaction (#411) Compaction never said what the summary did NOT have to carry, so the model treated it as total loss: it hoarded file contents and quoted tool output into the summary, and the state the harness itself keeps came back only as its own recollection of it, drifting a little more at every compaction. Both halves of prime-agent's compaction design, in a new module (compact_note.zig) so agent_compact.zig keeps its line count: BEFORE. The summary REQUEST now carries a note saying what persists - every file on disk, any goal/checklist (restated in full straight after the summary), and #441's append-only transcript, cited by its real path and message count when one is live. It asks for NAMES rather than contents and points the summary at what disk cannot give back: decisions, dead ends, constraints, unfinished work. The instruction itself still LEADS the request, so #379 classifies an empty or truncated reply exactly as before. AFTER. The new history head carries the harness's own ground truth, re-derived at every compaction rather than copied forward - so a later summary that paraphrases it away costs nothing, the next compaction regenerates it exactly. Three fields, each omitted unless real: the files this session modified (from /rewind's snapshot ledger, which is exact for write_file/edit_file/imagegen and blind to bash - the note says so rather than passing a partial list off as the whole diff); the #409 artifact paths the discarded messages' own spill markers cite, so every handle named was written by a spill that succeeded; and the transcript path via session_transcript.activePath, which returns null rather than let the note invent a file. A subagent and a /review turn get nothing, and a session with nothing durable to report keeps a byte-identical handoff. Co-Authored-By: Codegraff --- src/agent_compact.zig | 14 +- src/agent_compact_summary_test.zig | 42 ++++ src/agent_compact_test.zig | 21 +- src/agent_compact_test_support.zig | 5 + src/compact_note.zig | 372 +++++++++++++++++++++++++++++ src/snapshots.zig | 24 ++ src/tool_spill.zig | 9 +- 7 files changed, 467 insertions(+), 20 deletions(-) create mode 100644 src/compact_note.zig diff --git a/src/agent_compact.zig b/src/agent_compact.zig index 22170b3d..170ca32d 100644 --- a/src/agent_compact.zig +++ b/src/agent_compact.zig @@ -10,9 +10,8 @@ const context_tokens = @import("context_tokens.zig"); const main_mod = @import("main.zig"); const agent_mod = @import("agent.zig"); const Agent = agent_mod.Agent; -const prompts = @import("prompts.zig"); -const compact_instruction = prompts.compact_instruction; const goal_flow = @import("goal_flow.zig"); +const compact_note = @import("compact_note.zig"); // #411: both halves of "what survives a compaction" const messages_mod = @import("messages.zig"); const textMessage = messages_mod.textMessage; @@ -161,7 +160,7 @@ pub fn compact(self: *Agent) anyerror!usize { } }; - try self.messages.append(try textMessage(compact_arena, "user", compact_instruction)); + try self.messages.append(try textMessage(compact_arena, "user", try compact_note.summaryRequest(compact_arena, self))); // #174: establish the synthetic summary turn before pruning Responses // reasoning. An active tool loop's reasoning is newer than the real user // turn and must remain while that loop is in flight, but it becomes prior- @@ -195,7 +194,7 @@ pub fn compact(self: *Agent) anyerror!usize { } var fresh = std.json.Array.init(self.arena); - try fresh.append(try textMessage(self.arena, "user", try handoffMessage(self, summary))); + try fresh.append(try textMessage(self.arena, "user", try handoffMessage(self, summary, live_messages.items[0..recent_start]))); // Preserve a valid recent suffix verbatim (up to ~8k estimated tokens), // including its user boundary and paired tool calls/results. for (recent_messages) |message| try fresh.append(message); @@ -225,13 +224,14 @@ pub fn compact(self: *Agent) anyerror!usize { /// Without a pin, compaction would summarize the mandate away with nothing /// left to restate it - childHandoff below restates it verbatim instead of /// re-deriving it, so it can never drift or compound across compactions. -pub fn handoffMessage(self: *Agent, summary: []const u8) ![]const u8 { +/// `discarded` is the history this summary replaces, read only for the #409 artifact paths #411 re-states out of it. +pub fn handoffMessage(self: *Agent, summary: []const u8, discarded: []const Value) ![]const u8 { const base = if (self.sub) (if (self.task_prompt) |tp| try childHandoff(self, tp, summary) else try rootHandoff(self, summary)) else try rootHandoff(self, summary); - const standing = (try goal_flow.compactionSnapshot(self.arena, self)) orelse return base; - return std.fmt.allocPrint(self.arena, "{s}\n\n{s}", .{ base, standing }); + const standing = try goal_flow.compactionSnapshot(self.arena, self); + return compact_note.handoff(self.arena, self, base, standing, discarded); } fn rootHandoff(self: *Agent, summary: []const u8) ![]const u8 { diff --git a/src/agent_compact_summary_test.zig b/src/agent_compact_summary_test.zig index f63e40ee..f7fa73fd 100644 --- a/src/agent_compact_summary_test.zig +++ b/src/agent_compact_summary_test.zig @@ -5,6 +5,9 @@ const std = @import("std"); const Agent = @import("agent.zig").Agent; const compact = @import("agent_compact.zig"); const repeatedEmptySummaryFailure = compact.repeatedEmptySummaryFailure; +const compact_note = @import("compact_note.zig"); +const session_transcript = @import("session_transcript.zig"); +const compact_instruction = @import("prompts.zig").compact_instruction; test "repeated empty summaries unlock bounded recovery (#379)" { var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); @@ -49,3 +52,42 @@ test "repeated empty summaries unlock bounded recovery (#379)" { agent.compact_summary_failures = 1; try std.testing.expect(!repeatedEmptySummaryFailure(&agent, error.EmptySummary)); } + +// #411 appended a "what persists" note to the summary REQUEST. #379 classifies +// the RESPONSE to that request, so the two must not interact: the request still +// LEADS with the byte-identical instruction, carries none of the after-the-fact +// ground truth, and two consecutive unusable replies still escalate exactly as +// they did. A note that changed the request's head is the shape of regression +// this guards - it would be invisible in #411's own tests. +test "#411's request note leaves #379's empty-summary escalation exactly as it was" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const a = arena_state.allocator(); + session_transcript.resetForTest(); + defer session_transcript.resetForTest(); + + var agent: Agent = undefined; + agent.provider = .{ .id = "codex", .kind = .responses, .auth = .bearer, .url = "", .api_key = "", .model = "gpt-5", .context = 100_000 }; + agent.messages = std.json.Array.init(a); + agent.sub = false; + agent.review_mode = false; + agent.snapshots = null; + agent.session_name = ""; + agent.strict = false; + agent.sys_normal = ""; + agent.sys_strict = ""; + agent.tools_responses = ""; + agent.last_context_tokens = 85_000; + agent.context_local_tokens = agent.fullRequestEstimateTokens(); + agent.compact_summary_failures = 0; + + const request = try compact_note.summaryRequest(a, &agent); + try std.testing.expect(std.mem.startsWith(u8, request, compact_instruction)); + try std.testing.expect(std.mem.indexOf(u8, request, "durable state, re-derived") == null); + try std.testing.expect(!repeatedEmptySummaryFailure(&agent, error.EmptySummary)); + try std.testing.expect(repeatedEmptySummaryFailure(&agent, error.EmptySummary)); + // And a usable summary still ends the streak, which is what stops a healthy + // session accumulating its way into an emergency trim. + agent.compact_summary_failures = 0; + try std.testing.expect(!repeatedEmptySummaryFailure(&agent, error.ApiError)); +} diff --git a/src/agent_compact_test.zig b/src/agent_compact_test.zig index 600c96aa..1952f5fa 100644 --- a/src/agent_compact_test.zig +++ b/src/agent_compact_test.zig @@ -440,19 +440,16 @@ test "a compaction handoff carries the live checklist across the summary (#318)" var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena_state.deinit(); const a = arena_state.allocator(); - var agent: Agent = undefined; - agent.arena = a; - agent.task_prompt = null; // this test flips agent.sub to true below (#B3) - agent.sub = false; - agent.review_mode = false; - agent.todos = .empty; + // Root agent, every field handoffMessage reads initialized (incl. #411's + // ledger + session name); this test flips agent.sub to true below (#B3). + var agent = th.subAgent(a, false); agent.goal = .{ .objective = "ship the epoch fix", .epoch = 2 }; try agent.todos.append(a, .{ .content = "write the helper", .status = "completed", .epoch = 2 }); try agent.todos.append(a, .{ .content = "wire it into compact", .status = "completed", .epoch = 2 }); try agent.todos.append(a, .{ .content = "test it", .status = "pending", .epoch = 2 }); try agent.todos.append(a, .{ .content = "parked by an older goal", .status = "pending", .epoch = 1 }); - const handoff = try handoffMessage(&agent, "the model summarized the earlier work"); + const handoff = try handoffMessage(&agent, "the model summarized the earlier work", &.{}); // The summary and its framing are unchanged... try std.testing.expect(std.mem.indexOf(u8, handoff, "the model summarized the earlier work") != null); try std.testing.expect(std.mem.indexOf(u8, handoff, "Continue assisting the user based on this summary.") != null); @@ -476,12 +473,12 @@ test "a compaction handoff carries the live checklist across the summary (#318)" // A subagent shares the Agent struct but not the goal, so its handoff is // byte-identical to the pre-#318 text - as is a session with no goal. agent.sub = true; - const plain = try handoffMessage(&agent, "the model summarized the earlier work"); + const plain = try handoffMessage(&agent, "the model summarized the earlier work", &.{}); try std.testing.expect(std.mem.indexOf(u8, plain, "standing state") == null); try std.testing.expect(std.mem.endsWith(u8, plain, "Continue assisting the user based on this summary.")); agent.sub = false; agent.goal = null; - try std.testing.expectEqualStrings(plain, try handoffMessage(&agent, "the model summarized the earlier work")); + try std.testing.expectEqualStrings(plain, try handoffMessage(&agent, "the model summarized the earlier work", &.{})); } test "an emergency trim re-queues the standing state, never over a user note (#318)" { @@ -553,7 +550,7 @@ test "a compacting subagent's handoff restates its task prompt verbatim" { const a = arena_state.allocator(); var agent = th.subAgent(a, true); agent.task_prompt = "AUDIT src/foo.zig and report every unguarded json deref"; - const handoff = try handoffMessage(&agent, "the model summarized the earlier work"); + const handoff = try handoffMessage(&agent, "the model summarized the earlier work", &.{}); try std.testing.expect(std.mem.indexOf(u8, handoff, agent.task_prompt.?) != null and std.mem.indexOf(u8, handoff, "the model summarized the earlier work") != null and std.mem.indexOf(u8, handoff, "still your mandate") != null); } test "the root handoff is byte-identical to before the child pin" { @@ -561,7 +558,7 @@ test "the root handoff is byte-identical to before the child pin" { defer arena_state.deinit(); const a = arena_state.allocator(); var agent = th.subAgent(a, false); - const handoff = try handoffMessage(&agent, "the model summarized the earlier work"); + const handoff = try handoffMessage(&agent, "the model summarized the earlier work", &.{}); try std.testing.expectEqualStrings("Context: the earlier conversation was compacted to save space.\nSummary of the earlier work:\n\nthe model summarized the earlier work\n\nContinue assisting the user based on this summary.", handoff); } test "pinChildTask captures the mandate once, never re-pins, and ignores a root agent or an unrecognised head" { @@ -588,7 +585,7 @@ test "an oversized task prompt is head-capped in the child handoff" { var agent = th.subAgent(a, true); const big = util.repeatBytes("T", 20000); agent.task_prompt = &big; - const handoff = try handoffMessage(&agent, "summary text"); + const handoff = try handoffMessage(&agent, "summary text", &.{}); try std.testing.expect(handoff.len < 12_000 and std.mem.indexOf(u8, handoff, "task prompt truncated for the handoff") != null and std.mem.indexOf(u8, handoff, big[0..100]) != null and std.mem.indexOf(u8, handoff, &big) == null); } test "emergencyCutIndex finds no cut in a subagent-shaped history, so the pinned mandate survives an emergency trim" { diff --git a/src/agent_compact_test_support.zig b/src/agent_compact_test_support.zig index 45f45ac4..8bfd0814 100644 --- a/src/agent_compact_test_support.zig +++ b/src/agent_compact_test_support.zig @@ -24,6 +24,11 @@ pub fn subAgent(a: std.mem.Allocator, sub: bool) Agent { agent.todos = .empty; agent.goal = null; agent.task_prompt = null; + // #411: the durable-state note reads the /rewind ledger and the session + // name on every handoff, including the null paths, so both have to be real + // here or an `undefined` pointer gets dereferenced instead of skipped. + agent.snapshots = null; + agent.session_name = ""; // compactPrelude reports the token figure compact() prints, so the estimate // path has to be reachable: system prompt, tool json, provider context and // the two meter anchors it reads. diff --git a/src/compact_note.zig b/src/compact_note.zig new file mode 100644 index 00000000..a45e4a47 --- /dev/null +++ b/src/compact_note.zig @@ -0,0 +1,372 @@ +//! #411: what SURVIVES a compaction, told to the model twice. +//! +//! THE PROBLEM. compact() replaces the history with a summary and never says +//! what the summary does NOT have to carry. The model therefore treats the +//! compaction as total loss and spends the summary hoarding contents - pasted +//! file bodies, quoted tool output - which is both the most expensive thing it +//! can put in a summary and the least necessary, because every one of those +//! bytes is still on disk. Worse, the state the harness itself keeps (the goal, +//! the checklist, the transcript, the spilled artifacts) came back only as the +//! model's recollection of it, which drifts a little more at every compaction. +//! +//! THE TWO HALVES, from prime-agent's compaction design. +//! +//! BEFORE - `summaryRequest`. The summarization request carries a note saying +//! what persists: the files, the goal/checklist, and the append-only transcript +//! (#441). It asks for NAMES rather than contents, and points the summary at +//! what disk genuinely cannot give back - decisions, dead ends, constraints, +//! the state of unfinished work. +//! +//! AFTER - `durableState`. The new history head carries the harness's own +//! ground truth: the files this session modified, the tool outputs spilled to +//! disk by the history that was just discarded, and the transcript path. This +//! is STATE, not conversation. It is re-derived from live harness state at +//! every compaction rather than copied forward, so a later summary that +//! paraphrases it away costs nothing: the next compaction regenerates it +//! exactly, from the ledger and from disk, and it cannot drift. +//! +//! HONESTY RULES. Every field is omitted unless it is real. The transcript line +//! comes from `session_transcript.activePath`, which returns null when no +//! transcript is live, so the note can never cite a file that does not exist. +//! The artifact paths are read back out of the #409 markers in the discarded +//! messages themselves, so each one was written by a spill that succeeded. The +//! file list is `/rewind`'s snapshot ledger, which is exact for the tools that +//! take snapshots and blind to bash - and says so, rather than presenting a +//! partial list as the session's whole diff. + +const std = @import("std"); +const Allocator = std.mem.Allocator; +const Value = std.json.Value; + +const agent_mod = @import("agent.zig"); +const Agent = agent_mod.Agent; +const compact_instruction = @import("prompts.zig").compact_instruction; +const session_transcript = @import("session_transcript.zig"); +const tool_spill = @import("tool_spill.zig"); + +/// Most modified paths named before the list is elided. A compaction runs at +/// the point context is scarce, so this block has to stay a note rather than +/// becoming a manifest. +const max_files: usize = 24; + +/// Most spilled artifacts named. Same reason, and the marker for each one is a +/// full absolute path. +const max_handles: usize = 12; + +const persists_note = + \\What SURVIVES this compaction, so you do not spend the summary preserving it: + \\every file on disk is exactly as you left it, and any goal or todo checklist + \\is harness state that is restated to you in full immediately after this + \\summary. Record NAMES, not contents: file paths, directories, artifact paths, + \\the command lines that worked, identifiers. Spend the words instead on what + \\disk cannot give back - the decisions and why they were made, what was tried + \\and failed, the constraints the user set, and the exact state of the + \\unfinished work. +; + +/// The user message compact() sends to ask for the handoff summary: the +/// instruction, unchanged, plus the note above. Unchanged matters - #379 +/// classifies the RESPONSE to this request, and an empty or truncated reply is +/// still exactly as unusable as it was before the note existed. +pub fn summaryRequest(arena: Allocator, root: *Agent) ![]const u8 { + const path = session_transcript.activePath(root, arena) orelse + return std.fmt.allocPrint(arena, "{s}\n\n{s}", .{ compact_instruction, persists_note }); + return std.fmt.allocPrint(arena, + \\{s} + \\ + \\{s} + \\The complete conversation, including every message this summary replaces, + \\also stays on disk at {s} ({d} messages, one JSON object per line). It is + \\greppable, so an exact wording you leave out is recoverable rather than + \\lost - which is another reason to summarize rather than transcribe. + , .{ compact_instruction, persists_note, path, session_transcript.lineCount() }); +} + +/// The harness's own answer to "what is still here", assembled from live state +/// at the moment of the compaction. `discarded` is the slice of history the +/// summary replaces, read (not kept) for the artifact paths its markers cite. +/// Null when there is nothing true to report - which keeps a plain session's +/// handoff byte-identical to what it has always been. +pub fn durableState(arena: Allocator, root: *Agent, discarded: []const Value) !?[]const u8 { + // A subagent has no durable anything: its history is never persisted, it + // gets no transcript, it never spills, and the /rewind ledger is the root's. + // A /review turn shares the Agent struct but not the session's work. + if (root.sub or root.review_mode) return null; + var lines: std.ArrayList([]const u8) = .empty; + if (try fileLine(arena, root)) |line| try lines.append(arena, line); + if (try handleLine(arena, discarded)) |line| try lines.append(arena, line); + if (try transcriptLine(arena, root)) |line| try lines.append(arena, line); + if (lines.items.len == 0) return null; + return try std.fmt.allocPrint(arena, + \\[durable state, re-derived by the harness at this compaction rather than + \\recalled, so it cannot drift: + \\{s} + \\Everything named above exists on disk right now: read it back instead of + \\re-deriving it, and do not re-run a tool whose output is already there.] + , .{try std.mem.join(arena, "\n", lines.items)}); +} + +/// compact()'s new history head: the model's handoff text, then the standing +/// goal/checklist state (#318), then the durable-state note. Joined with blank +/// lines, and byte-identical to `base` when neither block has anything to say. +pub fn handoff(arena: Allocator, root: *Agent, base: []const u8, standing: ?[]const u8, discarded: []const Value) ![]const u8 { + var out = base; + if (standing) |s| out = try std.fmt.allocPrint(arena, "{s}\n\n{s}", .{ out, s }); + if (try durableState(arena, root, discarded)) |d| out = try std.fmt.allocPrint(arena, "{s}\n\n{s}", .{ out, d }); + return out; +} + +fn fileLine(arena: Allocator, root: *Agent) !?[]const u8 { + const snaps = root.snapshots orelse return null; + const paths = snaps.modifiedPaths(arena); + if (paths.len == 0) return null; + const shown = @min(paths.len, max_files); + const joined = try std.mem.join(arena, ", ", paths[0..shown]); + const more = if (paths.len > shown) + try std.fmt.allocPrint(arena, ", and {d} more", .{paths.len - shown}) + else + ""; + // The parenthetical is the honest bound, not boilerplate: presenting this + // ledger as the session's whole diff would be a lie whenever a `sed`, a + // `git apply` or a companion CLI did the writing. + return try std.fmt.allocPrint(arena, "- files this session modified ({d}; write_file/edit_file/imagegen only - edits made through bash are not tracked): {s}{s}", .{ paths.len, joined, more }); +} + +/// Only the handles THIS compaction is discarding, deliberately. Re-harvesting +/// the previous note's list would fold each compaction's handles into the next +/// without bound - #B3's task-pin rule, one level down. The artifacts stay on +/// disk regardless, and the transcript line above says where every old marker +/// can still be grepped in full. +fn handleLine(arena: Allocator, discarded: []const Value) !?[]const u8 { + var paths: std.ArrayList([]const u8) = .empty; + for (discarded) |m| try collectHandles(arena, m, &paths); + if (paths.items.len == 0) return null; + const shown = @min(paths.items.len, max_handles); + const joined = try std.mem.join(arena, ", ", paths.items[0..shown]); + const more = if (paths.items.len > shown) + try std.fmt.allocPrint(arena, ", and {d} more", .{paths.items.len - shown}) + else + ""; + return try std.fmt.allocPrint(arena, "- full tool outputs the discarded history spilled to disk (#409), still readable: {s}{s}", .{ joined, more }); +} + +fn transcriptLine(arena: Allocator, root: *Agent) !?[]const u8 { + const path = session_transcript.activePath(root, arena) orelse return null; + return try std.fmt.allocPrint(arena, "- this conversation's full transcript, including everything the summary above replaced: {s} ({d} messages, one JSON object per line) - grep it for an exact wording the summary paraphrased", .{ path, session_transcript.lineCount() }); +} + +/// Walk a message's JSON for #409 spill markers. Recursive because the marker +/// sits in a different field in each wire format (`output`, `content`, or a +/// `tool_result` block inside a content array), and a shape-blind walk cannot +/// be broken by a format this file has not heard of. +fn collectHandles(arena: Allocator, value: Value, out: *std.ArrayList([]const u8)) !void { + switch (value) { + .string => |s| if (handlePath(s)) |p| { + for (out.items) |seen| if (std.mem.eql(u8, seen, p)) return; + try out.append(arena, p); + }, + .array => |a| for (a.items) |item| try collectHandles(arena, item, out), + .object => |o| { + var it = o.iterator(); + while (it.next()) |entry| try collectHandles(arena, entry.value_ptr.*, out); + }, + else => {}, + } +} + +/// The artifact path inside a spill marker, or null for any other text. All +/// three fences must be present in order: a tool output that merely happens to +/// contain "bytes are at" is not a marker. +fn handlePath(s: []const u8) ?[]const u8 { + const head = std.mem.indexOf(u8, s, tool_spill.marker_head) orelse return null; + const rest = s[head..]; + const open = std.mem.indexOf(u8, rest, tool_spill.marker_path_open) orelse return null; + const tail = rest[open + tool_spill.marker_path_open.len ..]; + const close = std.mem.indexOf(u8, tail, tool_spill.marker_path_close) orelse return null; + return if (close == 0) null else tail[0..close]; +} + +// ── tests ──────────────────────────────────────────────────────────────── + +const testing = std.testing; +const textMessage = @import("messages.zig").textMessage; +const snapshots_mod = @import("snapshots.zig"); + +/// The fields the two halves read, and nothing else - every one of them is +/// consulted on the null path too, so an `undefined` here is a crash rather +/// than a silent pass. +fn noteAgent(sub: bool) Agent { + var root: Agent = undefined; + root.sub = sub; + root.review_mode = false; + root.snapshots = null; + root.session_name = ""; + root.gpa = testing.allocator; + root.io = testing.io; + return root; +} + +fn ledger() snapshots_mod.Snapshots { + return .{ .gpa = testing.allocator, .io = testing.io }; +} + +test "the summary request says what persists and asks for names, not contents (#411)" { + var arena_state = std.heap.ArenaAllocator.init(testing.allocator); + defer arena_state.deinit(); + const a = arena_state.allocator(); + session_transcript.resetForTest(); + defer session_transcript.resetForTest(); + + var root = noteAgent(false); + const req = try summaryRequest(a, &root); + // The instruction the model has always been given still LEADS the request: + // #379 classifies the response to it, and that classification must not + // start depending on a note that was appended after the fact. + try testing.expect(std.mem.startsWith(u8, req, compact_instruction)); + try testing.expect(std.mem.indexOf(u8, req, "Record NAMES, not contents") != null); + try testing.expect(std.mem.indexOf(u8, req, "restated to you in full") != null); + // The ground truth itself belongs on the FAR side of the summary, where the + // model cannot summarize it away. It must not be in the request. + try testing.expect(std.mem.indexOf(u8, req, "durable state, re-derived") == null); + // Nothing is live here, so the request cites no file at all. + try testing.expect(std.mem.indexOf(u8, req, ".transcript.jsonl") == null); +} + +test "a live transcript is named in both halves, by real path and message count (#411)" { + const gpa = testing.allocator; + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const a = arena_state.allocator(); + session_transcript.resetForTest(); + tool_spill.resetForTest(); + defer session_transcript.resetForTest(); + defer tool_spill.resetForTest(); + var tmp = testing.tmpDir(.{ .iterate = true }); + defer tmp.cleanup(); + + var root = noteAgent(false); + root.session_name = "n411"; + root.messages = std.json.Array.init(a); + try root.messages.append(try textMessage(a, "user", "why did the build fail?")); + try root.messages.append(try textMessage(a, "assistant", "the link order")); + session_transcript.record(&root, tmp.dir, "n411"); + + const req = try summaryRequest(a, &root); + try testing.expect(std.mem.indexOf(u8, req, ".graff/sessions/n411.transcript.jsonl") != null); + try testing.expect(std.mem.indexOf(u8, req, "(2 messages") != null); + + const note = (try durableState(a, &root, &.{})).?; + try testing.expect(std.mem.indexOf(u8, note, ".graff/sessions/n411.transcript.jsonl") != null); + try testing.expect(std.mem.indexOf(u8, note, "(2 messages") != null); + try testing.expect(std.mem.indexOf(u8, note, "grep it") != null); + + // A session whose transcript is not the live one omits the line entirely, + // rather than citing a path derived from its own name that nothing wrote. + // With nothing else durable, that leaves no note at all. + root.session_name = "someone-else"; + try testing.expect((try durableState(a, &root, &.{})) == null); + try testing.expect(std.mem.indexOf(u8, try summaryRequest(a, &root), ".transcript.jsonl") == null); +} + +test "the note names the files modified and the artifacts the discarded history spilled (#411)" { + const gpa = testing.allocator; + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const a = arena_state.allocator(); + session_transcript.resetForTest(); + tool_spill.resetForTest(); + defer session_transcript.resetForTest(); + defer tool_spill.resetForTest(); + var tmp = testing.tmpDir(.{ .iterate = true }); + defer tmp.cleanup(); + tool_spill.enable(.{ .io = testing.io, .dir = tmp.dir, .base_abs = "" }); + + var snaps = ledger(); + defer snaps.deinit(); + snaps.record("src/a.zig", .absent); + snaps.record("src/b.zig", .{ .content = "old bytes" }); + snaps.record("src/a.zig", .{ .content = "newer bytes" }); // edited twice, named once + var root = noteAgent(false); + root.snapshots = &snaps; + + // A marker written by the REAL spill writer, so the reader is proved against + // the format that actually ships rather than against a copy of it here. + const big = try a.alloc(u8, 4096); + @memset(big, 'x'); + const spill_note: tool_spill.Note = .{ .fallback = "[truncated]", .session = "s411" }; + const marker = spill_note.text(a, big, 1024); + try testing.expect(std.mem.indexOf(u8, marker, "tool-0.txt") != null); // the spill really happened + var fco: std.json.ObjectMap = .empty; + try fco.put(a, "type", .{ .string = "function_call_output" }); + try fco.put(a, "output", .{ .string = marker }); + var discarded = std.json.Array.init(a); + try discarded.append(try textMessage(a, "user", "run the big one")); + try discarded.append(.{ .object = fco }); + + const note = (try durableState(a, &root, discarded.items)).?; + try testing.expect(std.mem.indexOf(u8, note, "src/b.zig") != null); + try testing.expectEqual(@as(usize, 1), std.mem.count(u8, note, "src/a.zig")); + try testing.expect(std.mem.indexOf(u8, note, "(2; write_file") != null); + // The bound is stated, not implied: this ledger cannot see a bash edit. + try testing.expect(std.mem.indexOf(u8, note, "bash are not tracked") != null); + // The handle is the artifact PATH, extracted from the marker - not the + // marker prose, which would be re-pasting the very bytes compaction dropped. + try testing.expect(std.mem.indexOf(u8, note, "tool-0.txt") != null); + try testing.expect(std.mem.indexOf(u8, note, "read or grep") == null); + // A history with no spill in it gets no handle line. + var plain = std.json.Array.init(a); + try plain.append(try textMessage(a, "assistant", "the FULL story bytes are at the office")); + const no_handles = (try durableState(a, &root, plain.items)).?; + try testing.expect(std.mem.indexOf(u8, no_handles, "spilled to disk") == null); +} + +test "a subagent gets no durable-state note, whatever is hung on it (#411)" { + const gpa = testing.allocator; + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const a = arena_state.allocator(); + session_transcript.resetForTest(); + defer session_transcript.resetForTest(); + + var snaps = ledger(); + defer snaps.deinit(); + snaps.record("src/child.zig", .absent); + var sub = noteAgent(true); + sub.snapshots = &snaps; + sub.session_name = "n411"; + var discarded = std.json.Array.init(a); + try discarded.append(try textMessage(a, "user", tool_spill.marker_head ++ " — the FULL 9" ++ tool_spill.marker_path_open ++ "/tmp/x.txt" ++ tool_spill.marker_path_close ++ " it]")); + // A child's history is never persisted and its outputs are never spilled, + // so every field here would be a claim about state it does not own. + try testing.expect((try durableState(a, &sub, discarded.items)) == null); + try testing.expectEqualStrings("BASE", try handoff(a, &sub, "BASE", null, discarded.items)); + // A /review turn shares the Agent struct but not the session's work. + sub.sub = false; + sub.review_mode = true; + try testing.expect((try durableState(a, &sub, discarded.items)) == null); +} + +test "the durable-state note rides the new history head, last, and an empty one changes nothing (#411)" { + const gpa = testing.allocator; + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const a = arena_state.allocator(); + session_transcript.resetForTest(); + defer session_transcript.resetForTest(); + + var root = noteAgent(false); + // Nothing durable to report: the handoff is byte-identical to the text a + // plain session has always had. + try testing.expectEqualStrings("BASE", try handoff(a, &root, "BASE", null, &.{})); + + var snaps = ledger(); + defer snaps.deinit(); + snaps.record("src/z.zig", .absent); + root.snapshots = &snaps; + const out = try handoff(a, &root, "BASE", "[standing state: goal]", &.{}); + try testing.expect(std.mem.startsWith(u8, out, "BASE\n\n[standing state: goal]")); + try testing.expect(std.mem.indexOf(u8, out, "src/z.zig") != null); + // Ground truth is the LAST thing read before the model continues. + const standing_at = std.mem.indexOf(u8, out, "[standing state: goal]").?; + try testing.expect(std.mem.indexOf(u8, out, "durable state, re-derived").? > standing_at); +} diff --git a/src/snapshots.zig b/src/snapshots.zig index 1b5071a1..c9714213 100644 --- a/src/snapshots.zig +++ b/src/snapshots.zig @@ -113,6 +113,30 @@ pub const Snapshots = struct { return out; } + /// #411: the distinct paths this session has actually modified, in + /// first-modified order, duped into `arena`. This ledger is the only + /// EXACT answer the harness has to "what did I change this session" — + /// which is also its bound: it records write_file/edit_file/imagegen, so + /// an edit made through bash is not in it, and the note that prints these + /// says so rather than implying the list is the whole diff. A `/rewind` + /// has already dropped the snapshots it undid, so a rewound file + /// correctly stops being listed. + pub fn modifiedPaths(self: *Snapshots, arena: Allocator) []const []const u8 { + self.mutex.lockUncancelable(self.io); + defer self.mutex.unlock(self.io); + var out: std.ArrayList([]const u8) = .empty; + for (self.list.items) |snap| { + var seen = false; + for (out.items) |p| if (std.mem.eql(u8, p, snap.path)) { + seen = true; + }; + if (seen) continue; + const owned = arena.dupe(u8, snap.path) catch return out.items; + out.append(arena, owned) catch return out.items; + } + return out.items; + } + pub fn deinit(self: *Snapshots) void { for (self.list.items) |s| { self.gpa.free(s.path); diff --git a/src/tool_spill.zig b/src/tool_spill.zig index c98aa308..692b1c46 100644 --- a/src/tool_spill.zig +++ b/src/tool_spill.zig @@ -108,6 +108,13 @@ pub fn safeName(session: []const u8) bool { return !std.mem.eql(u8, session, ".") and !std.mem.eql(u8, session, ".."); } +/// The three fences #411's post-compaction note reads a spilled artifact's path +/// back out of a marker with. `Note.text` below is BUILT from them, so the +/// reader cannot drift from the writer: change the wording and both move. +pub const marker_head = "[tool output truncated at this model's per-result cap"; +pub const marker_path_open = " bytes are at "; +pub const marker_path_close = "; read or grep"; + /// What replaces the elided bytes. `session` empty (a subagent, an unwired /// process) means plain truncation with `fallback`. pub const Note = struct { @@ -119,7 +126,7 @@ pub const Note = struct { /// than growing an output the cap just shrank. pub fn text(self: Note, arena: Allocator, full: []const u8, cap: usize) []const u8 { const path = spill(arena, self.session, full) orelse return self.fallback; - const marker = std.fmt.allocPrint(arena, "[tool output truncated at this model's per-result cap — the FULL {d} bytes are at {s}; read or grep that file for the slice you need instead of re-running the tool (#409)]", .{ full.len, path }) catch return self.fallback; + const marker = std.fmt.allocPrint(arena, marker_head ++ " — the FULL {d}" ++ marker_path_open ++ "{s}" ++ marker_path_close ++ " that file for the slice you need instead of re-running the tool (#409)]", .{ full.len, path }) catch return self.fallback; return if (marker.len + 1 > cap) self.fallback else marker; } }; From 585d5503b9b31b82f6ce8c52f7e8018b4ec4ffd4 Mon Sep 17 00:00:00 2001 From: Rach Pradhan <54503978+justrach@users.noreply.github.com> Date: Thu, 6 Aug 2026 19:09:32 +0800 Subject: [PATCH 4/4] refactor(compact): compact_note -> compact_handoff_note, freeing the name for #391 #391 independently created src/compact_note.zig for the OTHER side of the same boundary: the buffer-reserved turn where the model writes notes to itself before rollover. This module is the harness's own ground truth injected around the handoff - which is what handoffMessage is and what the note actually rides - so the name it now has says what the old one only implied, and a future reader cannot confuse the two halves. #391 owns four files in that cluster, this owns one, so the rename is the cheap side. Pure rename plus the local alias (compact_note -> handoff_note) at its two call sites and one test import. agent_compact.zig stays at 564, still net zero against its branch point. Suite unchanged at 1056; the module keeps its reachability through agent_compact.zig's production import, so no test_hooks wiring changes. Co-Authored-By: Codegraff --- src/agent_compact.zig | 6 +++--- src/agent_compact_summary_test.zig | 4 ++-- src/{compact_note.zig => compact_handoff_note.zig} | 5 +++++ 3 files changed, 10 insertions(+), 5 deletions(-) rename src/{compact_note.zig => compact_handoff_note.zig} (98%) diff --git a/src/agent_compact.zig b/src/agent_compact.zig index 170ca32d..772ad8ca 100644 --- a/src/agent_compact.zig +++ b/src/agent_compact.zig @@ -11,7 +11,7 @@ const main_mod = @import("main.zig"); const agent_mod = @import("agent.zig"); const Agent = agent_mod.Agent; const goal_flow = @import("goal_flow.zig"); -const compact_note = @import("compact_note.zig"); // #411: both halves of "what survives a compaction" +const handoff_note = @import("compact_handoff_note.zig"); // #411: both halves of "what survives a compaction" const messages_mod = @import("messages.zig"); const textMessage = messages_mod.textMessage; @@ -160,7 +160,7 @@ pub fn compact(self: *Agent) anyerror!usize { } }; - try self.messages.append(try textMessage(compact_arena, "user", try compact_note.summaryRequest(compact_arena, self))); + try self.messages.append(try textMessage(compact_arena, "user", try handoff_note.summaryRequest(compact_arena, self))); // #174: establish the synthetic summary turn before pruning Responses // reasoning. An active tool loop's reasoning is newer than the real user // turn and must remain while that loop is in flight, but it becomes prior- @@ -231,7 +231,7 @@ pub fn handoffMessage(self: *Agent, summary: []const u8, discarded: []const Valu else try rootHandoff(self, summary); const standing = try goal_flow.compactionSnapshot(self.arena, self); - return compact_note.handoff(self.arena, self, base, standing, discarded); + return handoff_note.handoff(self.arena, self, base, standing, discarded); } fn rootHandoff(self: *Agent, summary: []const u8) ![]const u8 { diff --git a/src/agent_compact_summary_test.zig b/src/agent_compact_summary_test.zig index f7fa73fd..64b17d86 100644 --- a/src/agent_compact_summary_test.zig +++ b/src/agent_compact_summary_test.zig @@ -5,7 +5,7 @@ const std = @import("std"); const Agent = @import("agent.zig").Agent; const compact = @import("agent_compact.zig"); const repeatedEmptySummaryFailure = compact.repeatedEmptySummaryFailure; -const compact_note = @import("compact_note.zig"); +const handoff_note = @import("compact_handoff_note.zig"); const session_transcript = @import("session_transcript.zig"); const compact_instruction = @import("prompts.zig").compact_instruction; @@ -81,7 +81,7 @@ test "#411's request note leaves #379's empty-summary escalation exactly as it w agent.context_local_tokens = agent.fullRequestEstimateTokens(); agent.compact_summary_failures = 0; - const request = try compact_note.summaryRequest(a, &agent); + const request = try handoff_note.summaryRequest(a, &agent); try std.testing.expect(std.mem.startsWith(u8, request, compact_instruction)); try std.testing.expect(std.mem.indexOf(u8, request, "durable state, re-derived") == null); try std.testing.expect(!repeatedEmptySummaryFailure(&agent, error.EmptySummary)); diff --git a/src/compact_note.zig b/src/compact_handoff_note.zig similarity index 98% rename from src/compact_note.zig rename to src/compact_handoff_note.zig index a45e4a47..436653b4 100644 --- a/src/compact_note.zig +++ b/src/compact_handoff_note.zig @@ -1,5 +1,10 @@ //! #411: what SURVIVES a compaction, told to the model twice. //! +//! NOT `compact_note.zig` (#391), which is the other side of the same boundary: +//! that one is the buffer-reserved turn where the MODEL writes notes to itself +//! before rollover. This one is the HARNESS's own ground truth, injected around +//! the handoff - hence the name. Both halves of the boundary, two modules. +//! //! THE PROBLEM. compact() replaces the history with a summary and never says //! what the summary does NOT have to carry. The model therefore treats the //! compaction as total loss and spends the summary hoarding contents - pasted