From ea998d373836efb78d8c61f42118013b6149ba72 Mon Sep 17 00:00:00 2001 From: Rach Pradhan Date: Thu, 6 Aug 2026 13:45:34 +0800 Subject: [PATCH 1/6] feat(context): oversized tool outputs spill to a session artifact (#409) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-output cap (#193/#196) destroyed the elided bytes: the model's only recovery was to re-run the tool and guess a better slice. Now, when the agent has a durable session, the FULL output is written to `.graff/sessions//artifacts/tool-.txt` before the cap shrinks it, and the marker cites the absolute path and the byte count, so the next turn can read or grep exactly what it needs. Same call site and same safety class as #196 (no WS-close bracketing): the cap still only shrinks strings in place. A subagent has no persisted history, so it has no durable session and keeps the plain truncation, as does any process that never wired a sink (every unit test). Growth is bounded twice: `session_cap_bytes` (64 MiB, all-or-nothing per artifact so a marker can never lie about its byte count), and reclamation of the artifact dirs whose `.session.json` is gone — the session file is the ground truth for "this session was deleted", so an rm, the AI-title rename and /new all reclaim what they left behind. The sweep runs once, at the first spill, so a run that never spills does no extra I/O. The cap's truncation primitives move to the new module with it: the spill has to happen inside truncateStrField, the one place still holding the pre-truncation string, and agent_compact.zig sits at the 600-line cap. Co-Authored-By: Codegraff --- src/agent_compact.zig | 70 ++----- src/agent_compact_test.zig | 4 +- src/session_start.zig | 7 + src/tool_spill.zig | 363 +++++++++++++++++++++++++++++++++++++ 4 files changed, 390 insertions(+), 54 deletions(-) create mode 100644 src/tool_spill.zig diff --git a/src/agent_compact.zig b/src/agent_compact.zig index 3125f4d5..22170b3d 100644 --- a/src/agent_compact.zig +++ b/src/agent_compact.zig @@ -17,6 +17,12 @@ const goal_flow = @import("goal_flow.zig"); const messages_mod = @import("messages.zig"); const textMessage = messages_mod.textMessage; +// #409: the per-output cap's truncation primitives, plus the artifact spill that +// now runs inside them. Moved out of this file, which sits at the 600-line cap. +const tool_spill = @import("tool_spill.zig"); +const isToolOutputMsg = tool_spill.isToolOutputMsg; +const truncateToolOutput = tool_spill.truncateToolOutput; + const title_mod = @import("title.zig"); const assistantText = title_mod.assistantText; @@ -366,56 +372,6 @@ pub fn emergencyCutIndex(items: []const Value) ?usize { return null; } -/// True if `m` is a tool-output message whose payload can be truncated to -/// reclaim context: responses `function_call_output`, openai `role:"tool"`, or an -/// anthropic user message carrying `tool_result` blocks (#163). -fn isToolOutputMsg(m: Value) bool { - if (m != .object) return false; - if (m.object.get("type")) |t| if (t == .string and std.mem.eql(u8, t.string, "function_call_output")) return true; - if (m.object.get("role")) |r| if (r == .string) { - if (std.mem.eql(u8, r.string, "tool")) return true; - if (std.mem.eql(u8, r.string, "user")) if (m.object.get("content")) |c| if (c == .array) - for (c.array.items) |blk| { - if (blk == .object) if (blk.object.get("type")) |bt| - if (bt == .string and std.mem.eql(u8, bt.string, "tool_result")) return true; - }; - }; - return false; -} - -fn truncateStrField(arena: Allocator, o: *std.json.ObjectMap, key: []const u8, cap: usize, note: []const u8) usize { - const v = o.get(key) orelse return 0; - if (v != .string or v.string.len <= cap) return 0; - const orig = v.string.len; - // Keep the prefix short enough that prefix + '\n' + note <= cap, so the marker - // never grows an output that was only barely over the cap. - const stub = std.fmt.allocPrint(arena, "{s}\n{s}", .{ utf8Prefix(v.string, cap -| (note.len + 1)), note }) catch return 0; - o.put(arena, key, .{ .string = stub }) catch return 0; - return orig -| stub.len; -} - -/// Truncate an over-large tool-output payload in `m` in place to ~`cap` bytes, -/// preserving the message and its call/output pairing. Returns bytes reclaimed. -fn truncateToolOutput(arena: Allocator, m: *Value, cap: usize, note: []const u8) usize { - if (m.* != .object) return 0; - if (m.object.get("type")) |t| if (t == .string and std.mem.eql(u8, t.string, "function_call_output")) - return truncateStrField(arena, &m.object, "output", cap, note); - if (m.object.get("role")) |r| if (r == .string) { - if (std.mem.eql(u8, r.string, "tool")) return truncateStrField(arena, &m.object, "content", cap, note); - if (std.mem.eql(u8, r.string, "user")) if (m.object.get("content")) |c| if (c == .array) { - var saved: usize = 0; - for (m.object.get("content").?.array.items) |*blk| { - if (blk.* != .object) continue; - const bt = blk.object.get("type") orelse continue; - if (bt == .string and std.mem.eql(u8, bt.string, "tool_result")) - saved += truncateStrField(arena, &blk.object, "content", cap, note); - } - return saved; - }; - }; - return 0; -} - /// Re-pair the meter after removing locally measurable context. The server-only /// delta remains intact, while the current local component reflects the trim. fn accountForReclaimedTokens(self: *Agent, reclaimed_tokens: u64) void { @@ -447,7 +403,7 @@ pub fn trimOldestToolOutputsAlloc(self: *Agent, arena: Allocator) usize { if (!isToolOutputMsg(m.*)) continue; seen += 1; if (seen > total - keep_recent) break; // keep the most recent verbatim - reclaimed += truncateToolOutput(arena, m, stub_cap, "[old tool output truncated to recover context (#163)]"); + reclaimed += truncateToolOutput(arena, m, stub_cap, .{ .fallback = "[old tool output truncated to recover context (#163)]" }); } accountForReclaimedContext(self, reclaimed); return reclaimed; @@ -467,12 +423,22 @@ pub fn trimOldestToolOutputs(self: *Agent) usize { /// the most-recent outputs verbatim. Cap is window-proportional (Provider.perOutputCap) /// so large-context models keep full results untouched. Preserves every call/output /// pairing (shrinks strings, never drops a message). Returns bytes reclaimed. +/// +/// #409: the elided bytes are no longer destroyed. When this agent has a durable +/// session, each oversized output is written to that session's artifact dir +/// first and the marker cites the absolute path and the full byte count, so the +/// model can read or grep the slice it needs instead of re-running the tool. A +/// subagent (no persisted history) keeps the plain truncation below. pub fn capOversizedToolOutputs(self: *Agent, cap: usize) usize { if (cap == 0) return 0; + const note: tool_spill.Note = .{ + .fallback = "[tool output truncated: over this model's per-result cap — read/fetch a smaller range (#193)]", + .session = tool_spill.sessionFor(self.sub, self.session_name), + }; var reclaimed: usize = 0; for (self.messages.items) |*m| { if (isToolOutputMsg(m.*)) - reclaimed += truncateToolOutput(self.messageMutationAlloc(), m, cap, "[tool output truncated: over this model's per-result cap — read/fetch a smaller range (#193)]"); + reclaimed += truncateToolOutput(self.messageMutationAlloc(), m, cap, note); } // These outputs are appended after the prior response's usage was recorded, // so reclaimed bytes were never part of that authoritative server reading. diff --git a/src/agent_compact_test.zig b/src/agent_compact_test.zig index d0b9036e..600c96aa 100644 --- a/src/agent_compact_test.zig +++ b/src/agent_compact_test.zig @@ -333,6 +333,7 @@ test "capOversizedToolOutputs (#193): bounds an oversized output in every wire f agent.last_context_tokens = 200_000; agent.provider = .{ .id = "codex", .kind = .responses, .auth = .bearer, .url = "", .api_key = "", .model = "gpt-5", .context = 270_000 }; agent.sub = false; + agent.session_name = ""; // #409: no durable session here, so the cap truncates without spilling agent.strict = false; agent.sys_normal = ""; agent.sys_strict = ""; @@ -358,8 +359,7 @@ test "capOversizedToolOutputs (#193): bounds an oversized output in every wire f // within-cap output and the non-tool message are untouched try std.testing.expectEqualStrings("ok", agent.messages.items[3].object.get("output").?.string); try std.testing.expectEqualStrings("hello", agent.messages.items[4].object.get("content").?.string); - // cap == 0 disables the cap entirely (unknown window) - try std.testing.expectEqual(@as(usize, 0), capOversizedToolOutputs(&agent, 0)); + try std.testing.expectEqual(@as(usize, 0), capOversizedToolOutputs(&agent, 0)); // cap == 0 disables the cap entirely (unknown window) } test "cleanUserTurn: plain user text yes; assistant/tool_result no" { diff --git a/src/session_start.zig b/src/session_start.zig index dea2fffd..4e207b4c 100644 --- a/src/session_start.zig +++ b/src/session_start.zig @@ -38,6 +38,7 @@ const mcp = @import("mcp.zig"); const mcp_cli = @import("mcp_cli.zig"); const mcp_config = @import("mcp_config.zig"); const jobs = @import("jobs.zig"); +const tool_spill = @import("tool_spill.zig"); // #409: where an over-cap tool output's full bytes go const trace = @import("trace.zig"); const scoring = @import("scoring.zig"); const telemetry = @import("telemetry.zig"); @@ -128,6 +129,12 @@ pub fn setupWorktreeAndBanner( try arena.dupe(u8, cwd_buf[0..n]) else |_| try arena.dupe(u8, environ_map.get("PWD") orelse "."); + // #409: the oversized-tool-output cap spills the full bytes into this + // workspace before eliding them. Wired here because this is where the cwd + // (post `-w` chdir) is first known absolutely, and the marker hands the + // model an ABSOLUTE path. Left unwired in tests, where the cap stays the + // pre-#409 plain truncation. + tool_spill.enable(.{ .io = io, .dir = .cwd(), .base_abs = main_mod.g_cwd_display }); if (!main_mod.json_mode and flags.oneshot_prompt == null) { try out.print("{s}codegraff{s} · folder: {s}{s}{s} · / for commands · @ picks a file · esc interrupts · ↑/↓ history · tab completes · ctrl-d quits · trace → {s}\n", .{ style.bold, style.reset, style.accent, main_mod.g_cwd_display, style.reset, trace_path }); diff --git a/src/tool_spill.zig b/src/tool_spill.zig new file mode 100644 index 00000000..19ff1e3b --- /dev/null +++ b/src/tool_spill.zig @@ -0,0 +1,363 @@ +//! #409: spill, don't truncate. The per-output cap (#193/#196) bounds any single +//! tool output before send; this is where the ORIGINAL bytes go first, so the +//! elision is addressable instead of destructive. The marker that replaces them +//! carries the ABSOLUTE path and the full byte count, so the next turn can read +//! or grep exactly the slice it needs instead of re-running the tool. +//! +//! 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. +//! +//! 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 +//! attach an artifact to and the cap stays a plain truncation. +//! +//! Also the home of the cap's truncation primitives (moved here from +//! agent_compact.zig, which sits at the 600-line cap): the spill has to happen +//! inside `truncateStrField`, the one place that still holds the pre-truncation +//! string. + +const std = @import("std"); +const Io = std.Io; +const Value = std.json.Value; +const Allocator = std.mem.Allocator; + +const util = @import("util.zig"); +const utf8Prefix = util.utf8Prefix; +const session_index = @import("session_index.zig"); + +/// Ceiling on what one session may leave on disk. Every artifact is a tool +/// output that ALREADY exceeded the per-output cap, so a runaway tool loop is +/// exactly the case that would otherwise fill a disk one oversized result at a +/// time. `pub var` so a test can shrink it without writing 64 MiB. +pub var session_cap_bytes: usize = 64 * 1024 * 1024; + +/// How stale an orphaned artifact dir must be before it is reclaimed. A second +/// graff can spill during its first turn, before its own session file has been +/// autosaved; the grace window means that window never costs it its artifacts. +pub var orphan_grace_ms: i64 = std.time.ms_per_hour; + +/// Where the artifacts go. Null until `enable` wires it, so unit tests and any +/// run without a workspace get the pre-#409 plain truncation. +pub const Sink = struct { + io: Io, + /// The directory `.graff` lives under: the cwd in production, a tmp dir in tests. + dir: Io.Dir, + /// Absolute path of `dir`. The marker names an absolute path so the model can + /// open it from anywhere (a subagent may be running inside a worktree). + base_abs: []const u8, +}; + +var g_sink: ?Sink = null; +var g_used: std.atomic.Value(usize) = .init(0); +var g_seq: std.atomic.Value(u64) = .init(0); +var g_swept: std.atomic.Value(bool) = .init(false); + +pub fn enable(sink: Sink) void { + g_sink = sink; +} + +pub fn resetForTest() void { + g_sink = null; + g_used.store(0, .monotonic); + g_seq.store(0, .monotonic); + g_swept.store(false, .monotonic); +} + +/// The session an agent's spills belong to, or "" for plain truncation. Only a +/// root agent has one: a subagent's history is never written to disk, which is +/// the issue's "only spill when a durable session exists". +pub fn sessionFor(sub: bool, session_name: []const u8) []const u8 { + return if (sub) "" else session_name; +} + +/// The byte-budget half of the spill decision, pure. An artifact is written +/// whole or not at all — a partial one would make the marker lie about its byte +/// count — so an output that would overrun the remaining budget is truncated the +/// old way instead. +pub fn withinBudget(used: usize, len: usize, cap: usize) bool { + return len > 0 and used +| len <= cap; +} + +/// An artifact dir is reclaimable once its session file is gone AND it is older +/// than the grace window. An unreadable mtime is never "age 0" (worktree_prune's +/// rule): it keeps the directory. +pub fn reclaimable(session_file_exists: bool, age_ms: i64, grace_ms: i64) bool { + if (session_file_exists or age_ms < 0) return false; + return age_ms >= grace_ms; +} + +/// A session name that can only ever name a directory INSIDE `.graff/sessions`. +/// Session names reach us from /save and from AI titles; a '/' or a ".." in one +/// must never become a write outside the sessions dir. +pub fn safeName(session: []const u8) bool { + if (session.len == 0 or session.len > 128) return false; + if (std.mem.indexOfAny(u8, session, "/\\") != null) return false; + return !std.mem.eql(u8, session, ".") and !std.mem.eql(u8, session, ".."); +} + +/// What replaces the elided bytes. `session` empty (a subagent, an unwired +/// process) means plain truncation with `fallback`. +pub const Note = struct { + fallback: []const u8, + session: []const u8 = "", + + /// The marker for an output of `full`, bounded by the same `cap` as the stub + /// it goes into: a marker that cannot fit falls back to the short one rather + /// 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; + return if (marker.len + 1 > cap) self.fallback else marker; + } +}; + +/// Write `full` to this session's artifact dir; the ABSOLUTE path on success. +/// Null — i.e. plain truncation — when no durable session is wired, when the +/// per-session budget is spent, or when any part of the write fails. +fn spill(arena: Allocator, session: []const u8, full: []const u8) ?[]const u8 { + const sink = g_sink orelse return null; + if (!safeName(session)) return null; + if (!reserve(full.len)) return null; + const dir = std.fmt.allocPrint(arena, "{s}/{s}/artifacts", .{ session_index.sessions_dir, session }) catch return refund(full.len); + sweepOnce(sink, arena, session); + sink.dir.createDirPath(sink.io, dir) catch return refund(full.len); + const seq = g_seq.fetchAdd(1, .monotonic); + const rel = std.fmt.allocPrint(arena, "{s}/tool-{d}.txt", .{ dir, seq }) catch return refund(full.len); + sink.dir.writeFile(sink.io, .{ .sub_path = rel, .data = full, .flags = .{ .exclusive = true } }) catch return refund(full.len); + if (sink.base_abs.len == 0) return rel; + return std.fmt.allocPrint(arena, "{s}/{s}", .{ sink.base_abs, rel }) catch rel; +} + +fn reserve(len: usize) bool { + const prev = g_used.fetchAdd(len, .monotonic); + if (withinBudget(prev, len, session_cap_bytes)) return true; + _ = g_used.fetchSub(len, .monotonic); + return false; +} + +fn refund(len: usize) ?[]const u8 { + _ = g_used.fetchSub(len, .monotonic); + 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. +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 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 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; + 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 {}; + } +} + +fn ageMs(sink: Sink, path: []const u8, now_ms: i64) i64 { + const st = sink.dir.statFile(sink.io, path, .{}) catch return -1; + const mtime_ms: i64 = @intCast(@divTrunc(st.mtime.nanoseconds, std.time.ns_per_ms)); + const age = now_ms - mtime_ms; + return if (age < 0) 0 else age; // clock skew, not a stale artifact dir +} + +/// True if `m` is a tool-output message whose payload can be truncated to +/// reclaim context: responses `function_call_output`, openai `role:"tool"`, or an +/// anthropic user message carrying `tool_result` blocks (#163). +pub fn isToolOutputMsg(m: Value) bool { + if (m != .object) return false; + if (m.object.get("type")) |t| if (t == .string and std.mem.eql(u8, t.string, "function_call_output")) return true; + if (m.object.get("role")) |r| if (r == .string) { + if (std.mem.eql(u8, r.string, "tool")) return true; + if (std.mem.eql(u8, r.string, "user")) if (m.object.get("content")) |c| if (c == .array) + for (c.array.items) |blk| { + if (blk == .object) if (blk.object.get("type")) |bt| + if (bt == .string and std.mem.eql(u8, bt.string, "tool_result")) return true; + }; + }; + return false; +} + +fn truncateStrField(arena: Allocator, o: *std.json.ObjectMap, key: []const u8, cap: usize, note: Note) usize { + const v = o.get(key) orelse return 0; + if (v != .string or v.string.len <= cap) return 0; + const orig = v.string.len; + // The full bytes go to an artifact first (when the session is durable), so + // the marker below can point at them instead of only announcing the loss. + const marker = note.text(arena, v.string, cap); + // Keep the prefix short enough that prefix + '\n' + marker <= cap, so the + // marker never grows an output that was only barely over the cap. + const stub = std.fmt.allocPrint(arena, "{s}\n{s}", .{ utf8Prefix(v.string, cap -| (marker.len + 1)), marker }) catch return 0; + o.put(arena, key, .{ .string = stub }) catch return 0; + return orig -| stub.len; +} + +/// Truncate an over-large tool-output payload in `m` in place to ~`cap` bytes, +/// preserving the message and its call/output pairing. Returns bytes reclaimed. +pub fn truncateToolOutput(arena: Allocator, m: *Value, cap: usize, note: Note) usize { + if (m.* != .object) return 0; + if (m.object.get("type")) |t| if (t == .string and std.mem.eql(u8, t.string, "function_call_output")) + return truncateStrField(arena, &m.object, "output", cap, note); + if (m.object.get("role")) |r| if (r == .string) { + if (std.mem.eql(u8, r.string, "tool")) return truncateStrField(arena, &m.object, "content", cap, note); + if (std.mem.eql(u8, r.string, "user")) if (m.object.get("content")) |c| if (c == .array) { + var saved: usize = 0; + for (m.object.get("content").?.array.items) |*blk| { + if (blk.* != .object) continue; + const bt = blk.object.get("type") orelse continue; + if (bt == .string and std.mem.eql(u8, bt.string, "tool_result")) + saved += truncateStrField(arena, &blk.object, "content", cap, note); + } + return saved; + }; + }; + return 0; +} + +test "spill decision (#409): durable session, non-empty payload, room in the budget" { + // A subagent has no persisted history, so it has no artifact to attach to. + try std.testing.expectEqualStrings("", sessionFor(true, "session-17")); + try std.testing.expectEqualStrings("session-17", sessionFor(false, "session-17")); + // All-or-nothing at the budget edge. + try std.testing.expect(withinBudget(0, 100, 100)); + try std.testing.expect(withinBudget(90, 10, 100)); + try std.testing.expect(!withinBudget(91, 10, 100)); + try std.testing.expect(!withinBudget(0, 0, 100)); // nothing to spill + try std.testing.expect(!withinBudget(std.math.maxInt(usize), 8, 100)); // no wraparound + // A name that could escape .graff/sessions never becomes a directory. + try std.testing.expect(safeName("session-1770000000000")); + try std.testing.expect(!safeName("")); + try std.testing.expect(!safeName("../../etc")); + try std.testing.expect(!safeName("..")); + try std.testing.expect(!safeName("a/b")); +} + +test "reclaimable (#409): the session file is the ground truth, and an unknown age keeps" { + try std.testing.expect(!reclaimable(true, 999_999, 0)); // session still exists + try std.testing.expect(reclaimable(false, 3_600_000, 3_600_000)); // gone and past the grace + try std.testing.expect(!reclaimable(false, 60_000, 3_600_000)); // gone but still young + try std.testing.expect(!reclaimable(false, -1, 0)); // unreadable mtime keeps +} + +test "no durable session (#409): the cap stays a plain truncation" { + resetForTest(); + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const a = arena_state.allocator(); + const note: Note = .{ .fallback = "[truncated]", .session = "" }; + // Unwired process AND empty session: both fall back, and neither writes. + try std.testing.expectEqualStrings("[truncated]", note.text(a, &util.repeatBytes("x", 4096), 1024)); +} + +test "spill writes the full output and the marker points at it (#409)" { + 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(); + defer resetForTest(); + enable(.{ .io = io, .dir = tmp.dir, .base_abs = "/work" }); + + const cap: usize = 1024; + const big = try a.alloc(u8, 8192); + @memset(big, 'x'); + big[8000] = 'N'; // a needle past the cap: only the artifact can still hold it + + var fco: std.json.ObjectMap = .empty; + try fco.put(a, "type", .{ .string = "function_call_output" }); + try fco.put(a, "output", .{ .string = big }); + var m: Value = .{ .object = fco }; + const reclaimed = truncateToolOutput(a, &m, cap, .{ .fallback = "[truncated]", .session = "s1" }); + try std.testing.expect(reclaimed > 0); + + // (a) the artifact holds the FULL original bytes + const rel = ".graff/sessions/s1/artifacts/tool-0.txt"; + const spilled = try tmp.dir.readFileAlloc(io, rel, std.testing.allocator, .limited(1 << 20)); + defer std.testing.allocator.free(spilled); + try std.testing.expectEqualStrings(big, spilled); + // (b) the capped message stays within the cap and cites path + byte count + const stub = m.object.get("output").?.string; + try std.testing.expect(stub.len <= cap); + try std.testing.expect(std.mem.indexOf(u8, stub, "/work/" ++ rel) != null); + try std.testing.expect(std.mem.indexOf(u8, stub, "8192 bytes") != null); + try std.testing.expect(std.mem.indexOf(u8, stub, "truncated") != null); + // the needle is gone from the transcript and recoverable only from the file + try std.testing.expect(std.mem.indexOf(u8, stub, "N") == null); + try std.testing.expect(std.mem.indexOfScalar(u8, spilled, 'N') != null); +} + +test "the per-session byte cap bounds the spill, and over it the cap truncates as before (#409)" { + 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_cap = session_cap_bytes; + defer { + session_cap_bytes = saved_cap; + resetForTest(); + } + session_cap_bytes = 9000; + enable(.{ .io = io, .dir = tmp.dir, .base_abs = "" }); + + const note: Note = .{ .fallback = "[truncated]", .session = "s2" }; + const big = try a.alloc(u8, 8192); + @memset(big, 'x'); + // First spill fits the budget; the second would overrun it whole, so it is + // truncated the pre-#409 way rather than half-written. + try std.testing.expect(std.mem.indexOf(u8, note.text(a, big, 1024), "tool-0.txt") != null); + try std.testing.expectEqualStrings("[truncated]", note.text(a, big, 1024)); + try std.testing.expect(tmp.dir.statFile(io, ".graff/sessions/s2/artifacts/tool-1.txt", .{}) == error.FileNotFound); +} + +test "artifacts are reclaimed with their session, and a live session keeps its own (#409)" { + 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; // every orphan is old enough + enable(.{ .io = io, .dir = tmp.dir, .base_abs = "" }); + + // A session that was deleted (no .session.json), one that is still saved, + // and the one doing the spilling. + try tmp.dir.createDirPath(io, ".graff/sessions/dead/artifacts"); + try tmp.dir.writeFile(io, .{ .sub_path = ".graff/sessions/dead/artifacts/tool-9.txt", .data = "stale" }); + try tmp.dir.createDirPath(io, ".graff/sessions/alive/artifacts"); + try tmp.dir.writeFile(io, .{ .sub_path = ".graff/sessions/alive/artifacts/tool-9.txt", .data = "keep" }); + try tmp.dir.writeFile(io, .{ .sub_path = ".graff/sessions/alive.session.json", .data = "{}" }); + + const note: Note = .{ .fallback = "[truncated]", .session = "current" }; + try std.testing.expect(std.mem.indexOf(u8, note.text(a, &util.repeatBytes("y", 4096), 1024), "tool-0.txt") != null); + + try std.testing.expect(tmp.dir.statFile(io, ".graff/sessions/dead", .{}) == error.FileNotFound); + _ = try tmp.dir.statFile(io, ".graff/sessions/alive/artifacts/tool-9.txt", .{}); + _ = try tmp.dir.statFile(io, ".graff/sessions/current/artifacts/tool-0.txt", .{}); +} From 6e9f2f918372682b5ed9b0b2b0a9251a3d38d580 Mon Sep 17 00:00:00 2001 From: Rach Pradhan Date: Thu, 6 Aug 2026 13:51:59 +0800 Subject: [PATCH 2/6] test(context): end-to-end proof that the spill loop closes (#409) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scripts/test-spill-artifact.py drives a real graff against the repo's scripted model (scripts/eval/mock_model.py on the lmstudio port) with a seeded session whose last tool output is over the per-output cap, and asserts all three halves of the claim: the request the harness sent carries the marker (absolute path + byte count) and NOT the elided needle, the artifact on disk holds the original bytes byte for byte, and a follow-up `bash` call against THE PATH THE MARKER CITED brings the needle back. Negative control: with the sink unwired the test fails with "no #409 marker" and no read-back. The marker's path is now resolved with realPathFile through the same dir handle the artifact was written with. The declared base (g_cwd_display) falls back to $PWD, which a caller that changed directory without exporting it gets wrong — and that produced a path that pointed at nothing, exactly what the e2e caught. The user-facing cap note now distinguishes the two outcomes: bytes elided but kept as an artifact, versus the pre-#409 destructive truncation (#202). Co-Authored-By: Codegraff --- .github/workflows/ci.yml | 3 + scripts/test-spill-artifact.py | 200 +++++++++++++++++++++++++++++++++ src/agent_request.zig | 20 +++- src/tool_spill.zig | 24 +++- 4 files changed, 242 insertions(+), 5 deletions(-) create mode 100755 scripts/test-spill-artifact.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b175e684..b21ee987 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -97,6 +97,9 @@ jobs: - name: Embedder mode resumable serve streams (#330) run: python3 scripts/test-serve-resume.py zig-out/bin/graff + - name: Over-cap tool output spills to a session artifact (#409) + run: python3 scripts/test-spill-artifact.py zig-out/bin/graff + - name: Live JSON stream contains no raw stdout lines run: python3 scripts/test-json-live.py zig-out/bin/graff diff --git a/scripts/test-spill-artifact.py b/scripts/test-spill-artifact.py new file mode 100755 index 00000000..f9d52bcc --- /dev/null +++ b/scripts/test-spill-artifact.py @@ -0,0 +1,200 @@ +#!/usr/bin/env python3 +"""End-to-end proof that an over-cap tool output is spilled, not destroyed (#409). + +The per-output cap (#193/#196) used to delete the elided bytes; the model's only +recovery was to re-run the tool. #409 writes the FULL output to +`.graff/sessions//artifacts/tool-.txt` first and makes the marker +cite that path, so the next turn can read or grep exactly the slice it needs. + +The loop is closed here with the repo's scripted-model recipe +(scripts/eval/mock_model.py on the fixed lmstudio port), against a real graff: + + 1. a session file is seeded with an oversized tool output carrying a needle + PAST the cap, and graff is started with `--resume` on it; + 2. turn 1's request is the harness's own wire history — it must carry the + marker (absolute path + byte count) and NOT the needle; + 3. the artifact on disk must hold the original bytes, byte for byte; + 4. the mock answers with a `bash` call against THE PATH THE MARKER CITED, and + turn 2's request must carry the needle back — the model recovered the + elided content without re-running the tool. + +No network beyond loopback, no provider credentials, no model. + + python3 scripts/test-spill-artifact.py [zig-out/bin/graff] +""" + +from __future__ import annotations + +import json +import os +import pathlib +import re +import shlex +import subprocess +import sys +import tempfile +import time + +REPO = pathlib.Path(__file__).resolve().parents[1] +sys.path.insert(0, str(REPO / "scripts" / "eval")) +from mock_model import ScriptedModel # noqa: E402 + +SESSION = "spill-e2e" +NEEDLE = "GRAFF-SPILL-NEEDLE-409" +# The cap is window-proportional (Provider.perOutputCap = context/2 bytes), and +# GRAFF_CONTEXT declares the window for an unknown/local model. 40k tokens -> +# a 20_000-byte cap, with auto-compaction (80% = 32k tokens) far out of reach of +# the ~7.5k tokens this history weighs. +CONTEXT_TOKENS = 40_000 +OUTPUT_BYTES = 30_000 +# "…the FULL bytes are at ; read or grep…" — src/tool_spill.zig +MARKER_RE = re.compile(r"the FULL (\d+) bytes are at (\S+?); read or grep") + + +class SpillModel(ScriptedModel): + """Turn 1: read back the artifact the marker cited. Turn 2: stop.""" + + def __init__(self) -> None: + super().__init__([]) + self.cited: tuple[int, str] | None = None + + def next_reply(self, body: dict) -> dict: + super().next_reply(body) # records the request; the empty script never answers + if len(self.requests) == 1: + found = MARKER_RE.search(json.dumps(body)) + if not found: + return {"text": "no marker in the history"} + self.cited = (int(found.group(1)), found.group(2)) + return {"tool": "bash", "arguments": { + "command": f"tail -c 120 {shlex.quote(found.group(2))}", + }} + return {"text": "recovered the tail from the artifact"} + + +def seed_session(workspace: pathlib.Path, output: str) -> None: + """A saved conversation whose last tool output is over the per-output cap.""" + sessions = workspace / ".graff" / "sessions" + sessions.mkdir(parents=True, exist_ok=True) + messages = [ + {"role": "user", "content": "dump the build log"}, + {"role": "assistant", "content": "", "tool_calls": [{ + "id": "call_seed", "type": "function", + "function": {"name": "bash", "arguments": json.dumps({"command": "cat build.log"})}, + }]}, + {"role": "tool", "tool_call_id": "call_seed", "content": output}, + ] + (sessions / f"{SESSION}.session.json").write_text(json.dumps({ + "provider": "lmstudio", "model": "spill-mock-model", "strict": False, + "ultracode_mode": False, "goal": None, "todos": [], + "title": "spill artifact e2e", "updated_ms": 0, "messages": messages, + }), encoding="utf-8") + harness = workspace / ".harness" + harness.mkdir(parents=True, exist_ok=True) + # The AI titler would otherwise fire an extra quiet turn on the first prompt. + (harness / "settings.json").write_text('{"ai_title": false}', encoding="utf-8") + + +def run(graff: str, workspace: pathlib.Path, model: SpillModel) -> tuple[str, str, int | None]: + env = {k: v for k, v in os.environ.items() if not k.endswith("_API_KEY")} + env.update({ + "HOME": str(workspace), + "LMSTUDIO_API_KEY": "local", + "GRAFF_CONTEXT": str(CONTEXT_TOKENS), + "GRAFF_NO_TELEMETRY": "1", + "GRAFF_FLEET": "off", + "GRAFF_NO_SMOLIFY": "1", + "GRAFF_LEARN_AUTO": "0", + "GRAFF_BEHAVIOR_UPLOAD": "0", + "GRAFF_NO_BROWSER": "1", + "NO_COLOR": "1", + }) + argv = [graff, "--json", "--yolo", "--no-telemetry", + "--model", "lmstudio", "--resume", SESSION] + try: + done = subprocess.run( + argv, cwd=workspace, env=env, text=True, capture_output=True, + input=json.dumps({"type": "user", "text": "what did the build log end with?"}) + "\n", + timeout=90, + ) + return done.stdout, done.stderr, done.returncode + except subprocess.TimeoutExpired as exc: + out = exc.stdout if isinstance(exc.stdout, str) else (exc.stdout or b"").decode("utf-8", "ignore") + err = exc.stderr if isinstance(exc.stderr, str) else (exc.stderr or b"").decode("utf-8", "ignore") + return out, err, None + + +def main() -> None: + graff = os.path.abspath(sys.argv[1] if len(sys.argv) > 1 else str(REPO / "zig-out" / "bin" / "graff")) + if not os.access(graff, os.X_OK): + sys.exit(f"test-spill-artifact: not an executable: {graff}") + + # A padded output whose needle sits well past the cap, so nothing but the + # artifact can still produce it. + output = ("build step ok\n" * 3000)[:OUTPUT_BYTES - len(NEEDLE) - 1] + NEEDLE + "\n" + assert len(output) == OUTPUT_BYTES, len(output) + + failures: list[str] = [] + spilled: str | None = None + model = SpillModel() + model.start(1234) + try: + with tempfile.TemporaryDirectory(prefix="graff-spill-") as tmp: + workspace = pathlib.Path(tmp) + seed_session(workspace, output) + stdout, stderr, code = run(graff, workspace, model) + # Read the artifact while the workspace still exists. + if model.cited is not None: + try: + spilled = pathlib.Path(model.cited[1]).read_text(encoding="utf-8") + except OSError as exc: + failures.append(f"the cited artifact is not readable: {exc}") + finally: + model.stop() + time.sleep(0.05) # the port is fixed; let the socket clear + + if code != 0: + failures.append(f"graff exited {code}\n{stderr[-2000:]}") + if not model.requests: + failures.append("the harness never called the model") + report(failures, stdout, stderr) + + first = json.dumps(model.requests[0]) + # (b) the capped message carries an actionable marker, and only the marker. + if model.cited is None: + failures.append("request[0] carried no #409 marker (path + byte count)") + else: + cited_bytes, cited_path = model.cited + if cited_bytes != OUTPUT_BYTES: + failures.append(f"the marker claimed {cited_bytes} bytes, wanted {OUTPUT_BYTES}") + if not cited_path.startswith("/"): + failures.append(f"the marker cited a relative path: {cited_path}") + expected_tail = f"/.graff/sessions/{SESSION}/artifacts/tool-0.txt" + if not cited_path.endswith(expected_tail): + failures.append(f"the artifact is not under this session: {cited_path}") + # (a) the artifact holds the ORIGINAL bytes. + if spilled is not None and spilled != output: + failures.append(f"the artifact holds {len(spilled)} bytes, wanted the original {OUTPUT_BYTES}") + if NEEDLE in first: + failures.append("request[0] still carried the elided bytes; the cap did not apply") + if len(model.requests) < 2: + failures.append("the harness never sent a second request, so the read-back never happened") + # (c) the follow-up read of the cited path brought the elided content back. + elif NEEDLE not in json.dumps(model.requests[1]): + failures.append("request[1] did not carry the artifact tail back; the loop does not close") + + report(failures, stdout, stderr) + + +def report(failures: list[str], stdout: str, stderr: str) -> None: + if failures: + print("test-spill-artifact: FAIL") + for failure in failures: + print(f" - {failure}") + print(f"--- graff stdout (tail) ---\n{stdout[-2000:]}") + print(f"--- graff stderr (tail) ---\n{stderr[-2000:]}") + sys.exit(1) + print("test-spill-artifact: ok — over-cap output spilled, cited, and read back (#409)") + + +if __name__ == "__main__": + main() diff --git a/src/agent_request.zig b/src/agent_request.zig index 20f624ed..c51d50de 100644 --- a/src/agent_request.zig +++ b/src/agent_request.zig @@ -24,6 +24,7 @@ const tools_mod = @import("tools.zig"); const apiErrorMessage = tools_mod.apiErrorMessage; const mentionsReasoningEffort = tools_mod.mentionsReasoningEffort; const telemetry = @import("telemetry.zig"); +const tool_spill = @import("tool_spill.zig"); // #409: did the cap preserve the bytes, or destroy them? const run_budget_mod = @import("run_budget.zig"); const wire_messages = @import("messages.zig"); @@ -141,13 +142,24 @@ pub fn request(self: *Agent, tools_in: ?[]const u8) !std.json.ObjectMap { // window past what the in-turn recovery below can reclaim (it keeps the most // recent outputs verbatim). Window-proportional, so large-context models keep // full tool results untouched. + const spills_before = tool_spill.spillCount(); const capped = self.capOversizedToolOutputs(self.provider.perOutputCap()); if (capped > 0) { // #202: don't truncate silently. The model already sees an inline marker; - // surface it to the trace and (interactively) to the user too. - if (self.tracer) |tr| tr.note("context", "capped an oversized tool output before send"); - if (!main_mod.json_mode and !self.sub) - self.say("[tool output over this model's per-result cap — truncated {d} bytes before send (#193)]\n", .{capped}) catch {}; + // surface it to the trace and (interactively) to the user too. #409: say + // which of the two happened — the elided bytes are only GONE when there + // was no durable session to spill them to. + const spilled = tool_spill.spillCount() > spills_before; + if (self.tracer) |tr| tr.note("context", if (spilled) + "capped an oversized tool output before send (full bytes kept as a session artifact)" + else + "capped an oversized tool output before send"); + if (!main_mod.json_mode and !self.sub) { + if (spilled) + self.say("[tool output over this model's per-result cap — {d} bytes elided before send; the full output is in this session's artifacts and the model has the path (#409)]\n", .{capped}) catch {} + else + self.say("[tool output over this model's per-result cap — truncated {d} bytes before send (#193)]\n", .{capped}) catch {}; + } } var context_retried = false; // #193: at most one in-turn overflow recovery per request // #56: bounded stream-stall / drop reconnect budget (codex's stream_max_retries diff --git a/src/tool_spill.zig b/src/tool_spill.zig index 19ff1e3b..e26a4674 100644 --- a/src/tool_spill.zig +++ b/src/tool_spill.zig @@ -54,8 +54,15 @@ pub const Sink = struct { var g_sink: ?Sink = null; var g_used: std.atomic.Value(usize) = .init(0); var g_seq: std.atomic.Value(u64) = .init(0); +var g_spills: std.atomic.Value(u64) = .init(0); var g_swept: std.atomic.Value(bool) = .init(false); +/// Artifacts written this process. The caller's user-facing note reads it so a +/// cap that PRESERVED the bytes cannot read like one that destroyed them (#202). +pub fn spillCount() u64 { + return g_spills.load(.monotonic); +} + pub fn enable(sink: Sink) void { g_sink = sink; } @@ -64,6 +71,7 @@ pub fn resetForTest() void { g_sink = null; g_used.store(0, .monotonic); g_seq.store(0, .monotonic); + g_spills.store(0, .monotonic); g_swept.store(false, .monotonic); } @@ -128,6 +136,19 @@ fn spill(arena: Allocator, session: []const u8, full: []const u8) ?[]const u8 { const seq = g_seq.fetchAdd(1, .monotonic); const rel = std.fmt.allocPrint(arena, "{s}/tool-{d}.txt", .{ dir, seq }) catch return refund(full.len); sink.dir.writeFile(sink.io, .{ .sub_path = rel, .data = full, .flags = .{ .exclusive = true } }) catch return refund(full.len); + _ = g_spills.fetchAdd(1, .monotonic); + return absolute(sink, arena, rel); +} + +/// The path the marker hands the model. Resolved through the same dir handle the +/// artifact was written with, so it names the file that actually exists — the +/// declared `base_abs` is only a fallback, and it is derived from $PWD when the +/// cwd cannot be resolved, which a caller that inherited a stale PWD gets wrong. +fn absolute(sink: Sink, arena: Allocator, rel: []const u8) []const u8 { + var buf: [4096]u8 = undefined; + if (sink.dir.realPathFile(sink.io, rel, &buf)) |n| { + return arena.dupe(u8, buf[0..n]) catch rel; + } else |_| {} if (sink.base_abs.len == 0) return rel; return std.fmt.allocPrint(arena, "{s}/{s}", .{ sink.base_abs, rel }) catch rel; } @@ -296,7 +317,8 @@ test "spill writes the full output and the marker points at it (#409)" { // (b) the capped message stays within the cap and cites path + byte count const stub = m.object.get("output").?.string; try std.testing.expect(stub.len <= cap); - try std.testing.expect(std.mem.indexOf(u8, stub, "/work/" ++ rel) != null); + try std.testing.expect(std.mem.indexOf(u8, stub, "are at /") != null); // absolute, resolved through the dir handle + try std.testing.expect(std.mem.indexOf(u8, stub, rel) != null); try std.testing.expect(std.mem.indexOf(u8, stub, "8192 bytes") != null); try std.testing.expect(std.mem.indexOf(u8, stub, "truncated") != null); // the needle is gone from the transcript and recoverable only from the file From 31824d9e067508e7cd91a48e70be21aa0982a74f Mon Sep 17 00:00:00 2001 From: Rach Pradhan Date: Thu, 6 Aug 2026 13:54:37 +0800 Subject: [PATCH 3/6] docs: the cap keeps the bytes now, and says where (#409) CHANGELOG entry for v0.0.240 and the README's context-management paragraph, which described the tool-result preview pointer but stopped at the send-time cap, where the bytes used to simply go. Co-Authored-By: Codegraff --- CHANGELOG.md | 11 +++++++++++ README.md | 8 +++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2f8415b..057feb2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,17 @@ current is part of cutting a release. ## v0.0.240 (unreleased) +- An oversized tool output is now spilled, not destroyed (#409). The + per-result cap (#193/#201) used to delete the elided bytes, leaving the + model to re-run the tool and guess a better slice; when the session is + durable the full output is now written to + `.graff/sessions//artifacts/tool-.txt` first, and the marker in + the transcript cites the absolute path and the byte count, so the next turn + can read or grep exactly what it needs. Bounded by a 64 MiB per-session + budget (whole artifacts only, so a marker can never overstate what is on + disk), and reclaimed with the session: an artifact dir whose + `.session.json` is gone is swept at the next spill. Subagents, + whose history is never persisted, keep the plain truncation. - The REPL/engine separation began (#422): agent output now flows through a typed event vocabulary and a strict sink boundary (`engine_events.zig` / `engine_sink.zig`), with streamed model output, the codex WS transport diff --git a/README.md b/README.md index f972deaf..6c3d83a9 100644 --- a/README.md +++ b/README.md @@ -1067,7 +1067,13 @@ the data plane for task-aware recipe comparison; they do not silently switch models or effort levels. Long tool results are stored exactly under `.graff/tool-results/`; model history -receives a short preview and an inspectable file pointer instead. Responses +receives a short preview and an inspectable file pointer instead. A result that +is still over the per-model result cap at send time is spilled the same way +rather than truncated away: the full bytes go to +`.graff/sessions//artifacts/`, and the note left in the transcript +carries that absolute path and the byte count, so the next turn can read or grep +the slice it needs. Artifacts are bounded per session and are reclaimed once the +session file they belong to is gone. Responses requests are explicitly capped at 16k output tokens (4k for compaction and 64 for titles), while compaction carries the latest clean ~8k-token user-turn suffix forward verbatim. A shared atomic run budget allows at most four model From 3d8d5a28e2bcaed9e57afeca044d7e045c199938 Mon Sep 17 00:00:00 2001 From: Rach Pradhan Date: Thu, 6 Aug 2026 13:56:44 +0800 Subject: [PATCH 4/6] docs(context): say why the artifact sweep reclaims late, not at the rename (#409) Moving a renamed session's artifacts (or dropping them at the rename) would strand every path already handed to the model in that transcript. Co-Authored-By: Codegraff --- CHANGELOG.md | 5 ++++- src/tool_spill.zig | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 057feb2f..468dd84f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ The release workflow uses a tag's section here as its release notes (a hand-written `docs/releases/.md` wins if present), so keeping this file current is part of cutting a release. -## v0.0.240 (unreleased) +## v0.0.241 (unreleased) - An oversized tool output is now spilled, not destroyed (#409). The per-result cap (#193/#201) used to delete the elided bytes, leaving the @@ -23,6 +23,9 @@ current is part of cutting a release. disk), and reclaimed with the session: an artifact dir whose `.session.json` is gone is swept at the next spill. Subagents, whose history is never persisted, keep the plain truncation. + +## v0.0.240 (2026-08-06) + - The REPL/engine separation began (#422): agent output now flows through a typed event vocabulary and a strict sink boundary (`engine_events.zig` / `engine_sink.zig`), with streamed model output, the codex WS transport diff --git a/src/tool_spill.zig b/src/tool_spill.zig index e26a4674..c1339e95 100644 --- a/src/tool_spill.zig +++ b/src/tool_spill.zig @@ -169,6 +169,12 @@ fn refund(len: usize) ?[]const u8 { /// 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. +/// +/// 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. 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 From ec86f1389543fc95f8425c232b03258df15367e5 Mon Sep 17 00:00:00 2001 From: Rach Pradhan <54503978+justrach@users.noreply.github.com> Date: Thu, 6 Aug 2026 14:42:10 +0800 Subject: [PATCH 5/6] fix(test): #409 spill needle is a string, not a bare 'N' the tmpdir can forge The marker embeds the artifact's REAL absolute path, and std.testing.tmpDir names its directory with 16 random base64-url characters. 'N' is in that alphabet, so "the needle is gone from the transcript" failed whenever the random name happened to contain one: 4 failures in 15 runs, measured identically on this integration AND on feat/prime-409-spill alone. Test-only; the assertion's intent is unchanged. Co-Authored-By: Codegraff --- src/tool_spill.zig | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/tool_spill.zig b/src/tool_spill.zig index c1339e95..42da6a66 100644 --- a/src/tool_spill.zig +++ b/src/tool_spill.zig @@ -304,9 +304,16 @@ test "spill writes the full output and the marker points at it (#409)" { enable(.{ .io = io, .dir = tmp.dir, .base_abs = "/work" }); const cap: usize = 1024; + // A needle past the cap: only the artifact can still hold it. It has to be + // a STRING, not a bare 'N' — the marker embeds the artifact's real absolute + // path, and std.testing.tmpDir names its directory with 16 random + // base64-url characters. One of those is 'N' about 22% of the time, so a + // single-character needle made this test fail on ~1 run in 5 for reasons + // that had nothing to do with spilling. + const needle = "NEEDLE409"; const big = try a.alloc(u8, 8192); @memset(big, 'x'); - big[8000] = 'N'; // a needle past the cap: only the artifact can still hold it + @memcpy(big[8000..][0..needle.len], needle); var fco: std.json.ObjectMap = .empty; try fco.put(a, "type", .{ .string = "function_call_output" }); @@ -328,8 +335,8 @@ test "spill writes the full output and the marker points at it (#409)" { try std.testing.expect(std.mem.indexOf(u8, stub, "8192 bytes") != null); try std.testing.expect(std.mem.indexOf(u8, stub, "truncated") != null); // the needle is gone from the transcript and recoverable only from the file - try std.testing.expect(std.mem.indexOf(u8, stub, "N") == null); - try std.testing.expect(std.mem.indexOfScalar(u8, spilled, 'N') != null); + try std.testing.expect(std.mem.indexOf(u8, stub, needle) == null); + try std.testing.expect(std.mem.indexOf(u8, spilled, needle) != null); } test "the per-session byte cap bounds the spill, and over it the cap truncates as before (#409)" { From ee28d8cea1a7bc42e01310dfe8ec18b35cbc42b1 Mon Sep 17 00:00:00 2001 From: Rach Pradhan <54503978+justrach@users.noreply.github.com> Date: Thu, 6 Aug 2026 16:09:38 +0800 Subject: [PATCH 6/6] =?UTF-8?q?fix(test):=20the=20spill=20marker's=20path?= =?UTF-8?q?=20is=20asserted=20portably=20=E2=80=94=20windows=20paths=20are?= =?UTF-8?q?=20not=20/-rooted=20(#409)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test matched 'are at /' and posix separators in the marker; on windows the resolved path is drive-rooted with backslashes and the windows CI job failed on exactly this test. The path is now extracted from the marker and asserted absolute via std.fs.path.isAbsolute, with the artifact name matched separator-free. Co-Authored-By: Codegraff --- src/tool_spill.zig | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/tool_spill.zig b/src/tool_spill.zig index 42da6a66..f7252957 100644 --- a/src/tool_spill.zig +++ b/src/tool_spill.zig @@ -330,8 +330,14 @@ test "spill writes the full output and the marker points at it (#409)" { // (b) the capped message stays within the cap and cites path + byte count const stub = m.object.get("output").?.string; try std.testing.expect(stub.len <= cap); - try std.testing.expect(std.mem.indexOf(u8, stub, "are at /") != null); // absolute, resolved through the dir handle - try std.testing.expect(std.mem.indexOf(u8, stub, rel) != null); + // The cited path is absolute on every OS ("/…" on posix, "C:\…" or + // "\\?\…" on Windows), and the separators are the platform's — so + // extract the path from the marker and assert absoluteness, rather than + // matching a leading slash or posix separators (broke on windows CI). + const at = (std.mem.indexOf(u8, stub, "are at ") orelse return error.TestUnexpectedResult) + "are at ".len; + const semi = std.mem.indexOfScalarPos(u8, stub, at, ';') orelse return error.TestUnexpectedResult; + try std.testing.expect(std.fs.path.isAbsolute(stub[at..semi])); + try std.testing.expect(std.mem.indexOf(u8, stub, "tool-0.txt") != null); try std.testing.expect(std.mem.indexOf(u8, stub, "8192 bytes") != null); try std.testing.expect(std.mem.indexOf(u8, stub, "truncated") != null); // the needle is gone from the transcript and recoverable only from the file