diff --git a/src/agent_compact.zig b/src/agent_compact.zig index 22170b3d..772ad8ca 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 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; @@ -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 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- @@ -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 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 f63e40ee..64b17d86 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 handoff_note = @import("compact_handoff_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 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)); + 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_handoff_note.zig b/src/compact_handoff_note.zig new file mode 100644 index 00000000..436653b4 --- /dev/null +++ b/src/compact_handoff_note.zig @@ -0,0 +1,377 @@ +//! #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 +//! 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/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..1a42d134 100644 --- a/src/session_index.zig +++ b/src/session_index.zig @@ -18,6 +18,30 @@ 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 +/// 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..c469bf9f 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,46 @@ 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, + // 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"; + 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()); + 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..41a8aad9 --- /dev/null +++ b/src/session_transcript.zig @@ -0,0 +1,339 @@ +//! #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. +/// +/// 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 }); +} + +/// 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; + // '\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()) |raw| { + const line = std.mem.trimEnd(u8, raw, "\r"); + 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. +/// +/// `.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, .read = true }) catch return null; + defer f.close(io); + 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. +/// +/// 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. +/// +/// 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; + 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(); +} + +// 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/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/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; diff --git a/src/tool_spill.zig b/src/tool_spill.zig index f7252957..692b1c46 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 @@ -107,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 { @@ -118,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; } }; @@ -165,35 +173,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 +433,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 +}