From 07a1a465917bbc4e0f7c69dd710339d68a507a5f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 27 Aug 2026 01:06:41 +0000 Subject: [PATCH 01/10] fix: land #549 leftovers and native Anthropic output_config (#550) Empty GRAFF_MCP_CONFIG is a wholesale MCP off-switch (project and plugin halves stay dark). #517/#523 tests now mutate behavior instead of grepping source. On provider id anthropic, the tools-off formatting turn sends output_config.format json_schema; minimax/kimi and a learned sox rejection keep the structured_output tool (ADR 0001). --- TUI/run.zig | 11 +-- TUI/run_stall.zig | 13 +++ TUI/tty.zig | 29 +++++-- lean-proofs/Graff/StructuredOutput.lean | 88 +++++++++++++------- spec/kernels/structured_output.json | 6 +- spec/ref/structured_output.py | 40 ++++++---- src/agent.zig | 2 +- src/agent_request.zig | 5 +- src/agent_request_body.zig | 6 +- src/agent_request_body_responses.zig | 93 +++++++++++++++++----- src/mcp.zig | 7 +- src/mcp_boot.zig | 3 +- src/mcp_cli.zig | 4 +- src/mcp_config.zig | 86 +++++++++++++++++++- src/session_start.zig | 3 +- src/spec_structured_output_conformance.zig | 9 ++- 16 files changed, 302 insertions(+), 103 deletions(-) diff --git a/TUI/run.zig b/TUI/run.zig index f50c3977..d515ad84 100644 --- a/TUI/run.zig +++ b/TUI/run.zig @@ -298,13 +298,7 @@ pub fn run( continue; } esc_stall = 0; - if (pending_len == inbuf.len) { - // A stuck head has filled the whole buffer: that is a parser - // wedge, not a dead tty. Drop it rather than letting the - // zero-length read below masquerade as a hangup and kill the - // TUI mid-session (#517). - pending_len = 0; - } + pending_len = stall.clearFullWedge(pending_len, inbuf.len); var filled = pending_len; const got = tty.readStdin(inbuf[filled..]); pacing.reads += 1; @@ -505,9 +499,6 @@ test "run loop enables click+hover tracking and bracketed paste" { try std.testing.expect(std.mem.indexOf(u8, src, &kitty_on) != null); try std.testing.expect(std.mem.indexOf(u8, src, &wrap_off) != null); try std.testing.expect(std.mem.indexOf(u8, src, "a=d,d=A") != null); - // #517: a buffer-filling parser wedge must be cleared before the read, - // or the zero-length read reads as a hangup and kills the TUI. - try std.testing.expect(std.mem.indexOf(u8, src, "pending_len == inbuf.len") != null); // The idle paste sweep must DISCARD whatever was stuck mid-sequence before // the stall path below can see it. Leaving it there let a lone pending ESC // become the Escape KEY the instant `in_paste` cleared, cancelling a live diff --git a/TUI/run_stall.zig b/TUI/run_stall.zig index 1a89a02b..3839be68 100644 --- a/TUI/run_stall.zig +++ b/TUI/run_stall.zig @@ -86,6 +86,12 @@ pub fn armExpired(now_ms: u64, arm_ms: u64) bool { return now_ms -| arm_ms > arm_window_ms; } +/// A stuck CSI/OSC head that filled the read buffer is a parser wedge, not a +/// hangup. Drop it so the next `read` is not a zero-length "TTY gone" (#517). +pub fn clearFullWedge(pending_len: usize, buf_len: usize) usize { + return if (pending_len == buf_len) 0 else pending_len; +} + /// Is this read nothing but complete SGR mouse reports? /// /// ?1003h is on by default for image-chip hover, and a pointer merely RESTING @@ -202,3 +208,10 @@ test "a lone ESC inside a latched paste is the escape hatch, not a 2s wait" { // Outside a paste nothing moved: #94's 2-stall Escape still fires. try std.testing.expectEqual(StallVerdict.escape_key, stallVerdict("\x1b", 2, .{})); } + +test "a buffer-filling parser wedge is dropped, not treated as hangup (#517)" { + try std.testing.expectEqual(@as(usize, 0), clearFullWedge(4096, 4096)); + try std.testing.expectEqual(@as(usize, 12), clearFullWedge(12, 4096)); + try std.testing.expectEqual(@as(usize, 0), clearFullWedge(0, 4096)); + try std.testing.expectEqual(@as(usize, 1), clearFullWedge(1, 2)); +} diff --git a/TUI/tty.zig b/TUI/tty.zig index eed0b08f..87211763 100644 --- a/TUI/tty.zig +++ b/TUI/tty.zig @@ -46,6 +46,16 @@ pub const RawState = if (is_windows) struct { input_cp: u32 = 0, } else std.posix.termios; +/// Clear the line-discipline bits that steal keys the TUI owns (#523). +/// Extracted so tests can mutate a termios-shaped value without a real tty. +pub fn surrenderLineDiscipline(raw: anytype) void { + raw.lflag.ICANON = false; + raw.lflag.ECHO = false; + raw.lflag.ISIG = false; + raw.lflag.IEXTEN = false; // ^V (0x16) reaches us, not the tty's lnext + raw.iflag.IXON = false; // ^S/^Q are keys, not XOFF/XON flow control +} + pub fn enterRaw() ?RawState { if (is_windows) { const h = w.GetStdHandle(w.STD_OUTPUT_HANDLE); @@ -67,11 +77,7 @@ pub fn enterRaw() ?RawState { const fd = std.posix.STDIN_FILENO; const orig = std.posix.tcgetattr(fd) catch return null; var raw = orig; - raw.lflag.ICANON = false; - raw.lflag.ECHO = false; - raw.lflag.ISIG = false; - raw.lflag.IEXTEN = false; // ^V (0x16) reaches us, not the tty's lnext (#523) - raw.iflag.IXON = false; // ^S/^Q are keys, not XOFF/XON flow control (#523) + surrenderLineDiscipline(&raw); raw.cc[@intFromEnum(std.posix.V.MIN)] = 0; raw.cc[@intFromEnum(std.posix.V.TIME)] = 0; std.posix.tcsetattr(fd, .NOW, raw) catch return null; @@ -100,9 +106,16 @@ pub fn readStdin(buf: []u8) usize { } test "raw mode surrenders ^V and ^S to the app, not the line discipline (#523)" { - const src = @embedFile("tty.zig"); - try std.testing.expect(std.mem.indexOf(u8, src, "IEXTEN = false") != null); - try std.testing.expect(std.mem.indexOf(u8, src, "IXON = false") != null); + var raw = struct { + lflag: struct { ICANON: bool = true, ECHO: bool = true, ISIG: bool = true, IEXTEN: bool = true } = .{}, + iflag: struct { IXON: bool = true } = .{}, + }{}; + surrenderLineDiscipline(&raw); + try std.testing.expect(!raw.lflag.IEXTEN); + try std.testing.expect(!raw.iflag.IXON); + try std.testing.expect(!raw.lflag.ICANON); + try std.testing.expect(!raw.lflag.ECHO); + try std.testing.expect(!raw.lflag.ISIG); } test "Windows enterRaw switches the console to UTF-8 and restore puts the CPs back (#607)" { diff --git a/lean-proofs/Graff/StructuredOutput.lean b/lean-proofs/Graff/StructuredOutput.lean index 2e7b5274..70ace06d 100644 --- a/lean-proofs/Graff/StructuredOutput.lean +++ b/lean-proofs/Graff/StructuredOutput.lean @@ -1,23 +1,30 @@ /- - Structured-output carrier ladder (#543): which server-visible carrier a - set --output-schema rides, per wire format, learned degrade state (sox), - and whether real tools occupy the request. + Structured-output carrier ladder (#543 / #550): which server-visible + carrier a set --output-schema rides, per wire format, learned degrade + state (sox), and whether real tools occupy the request. openai chat: json_schema until the provider rejects it (sox learned via the request() quirk ladder); degraded, the tools-off formatting turn carries the schema as a structured_output TOOL (dsh's pattern, forced by instruction, never tool_choice — thinking modes reject forcing), while a - turn with real tools falls back to json_object. anthropic: no - response_format exists at all — the tool is the only server carrier and - the prompt always embeds the schema. responses: native text.format. + turn with real tools falls back to json_object. - The invariant of record is never_silent: before 9e2da0a the anthropic - wire dropped a set schema with no error — the exact bug class this - kernel now makes unrepresentable. + anthropic: provider id `anthropic` tries native output_config.format + json_schema on the tools-off formatting turn (#550, kimi-code prior art). + A learned sox flag (output_config rejected) falls back to the + structured_output tool. minimax / kimi-anthropic stay on the tool + (modelled here as sox). Real tools never carry a grammar (ADR 0001). + + responses: native text.format. + + The invariant of record is never_silent on the formatting turn: before + 9e2da0a the anthropic wire dropped a set schema with no error. An + anthropic tools turn may be silent — the two-phase split holds the + schema for the formatting call. Executable port: spec/ref/structured_output.py. Impl: src/agent_request_body.zig + agent_request_body_responses.zig - (schemaAwarePrompt / writeStructuredOutputTool / writeAnthropicStructuredTool). + (schemaAwarePrompt / writeAnthropicSchema / writeAnthropicOutputConfig). -/ namespace Graff.StructuredOutput @@ -27,11 +34,11 @@ inductive Wire deriving DecidableEq, Repr inductive Carrier - | none | jsonSchema | jsonObject | toolOpenai | toolAnthropic | textFormat + | none | jsonSchema | jsonObject | toolOpenai | toolAnthropic | textFormat | outputConfig deriving DecidableEq, Repr /-- The server-visible carrier of the schema for one request. - Args: wire, schema present, sox (json_schema was rejected), real tools present. -/ + Args: wire, schema present, sox (native schema was rejected), real tools present. -/ def carrier : Wire → Bool → Bool → Bool → Carrier | _, false, _, _ => .none | .responses, true, _, _ => .textFormat @@ -39,30 +46,46 @@ def carrier : Wire → Bool → Bool → Bool → Carrier | .openai, true, true, true => .jsonObject | .openai, true, true, false => .toolOpenai | .anthropic, true, _, true => .none - | .anthropic, true, _, false => .toolAnthropic + | .anthropic, true, false, false => .outputConfig + | .anthropic, true, true, false => .toolAnthropic -/-- Whether the system prompt embeds the schema text (schemaAwarePrompt's - embedded-schema branch). Tools presence never changes the prompt. -/ +/-- Whether the system prompt embeds the schema JSON (schemaAwarePrompt's + cannot-enforce / tool-mode branch). Native Anthropic uses the light + prompt; sox (fallback) embeds. Tools presence never changes the prompt. -/ def promptSchema : Wire → Bool → Bool → Bool | _, false, _ => false - | .anthropic, true, _ => true + | .anthropic, true, sox => sox | .openai, true, sox => sox | .responses, true, _ => false -/-- #543's invariant of record: a set schema is NEVER silent — some - server-visible carrier exists, or the prompt itself embeds the schema. -/ +/-- A set schema is never silent on the formatting turn. An anthropic + tools turn may be — ADR 0001 forbids a grammar there; the two-phase + split holds the schema for the next call. -/ theorem never_silent (w : Wire) (sox tools : Bool) : - carrier w true sox tools ≠ .none ∨ promptSchema w true sox = true := by + carrier w true sox tools ≠ .none ∨ promptSchema w true sox = true ∨ + (w = .anthropic ∧ tools = true) := by cases w <;> cases sox <;> cases tools <;> decide -/-- The learned degrade is a chat-wire quirk: sox changes nothing elsewhere. -/ -theorem sox_only_on_chat (w : Wire) (tools : Bool) (h : w ≠ .openai) : - carrier w true true tools = carrier w true false tools := by - cases w <;> cases tools <;> first | decide | exact absurd rfl h +/-- The learned degrade does not touch the Responses wire. -/ +theorem sox_leaves_responses_alone (tools : Bool) : + carrier .responses true true tools = carrier .responses true false tools := by + cases tools <;> decide -/-- Once a provider rejected json_schema, it is never sent again. -/ +/-- #550: native Anthropic formatting uses output_config; sox falls back to the tool. -/ +theorem sox_degrades_anthropic_native : + carrier .anthropic true false false = .outputConfig ∧ + carrier .anthropic true true false = .toolAnthropic := by + decide + +/-- ADR 0001: no schema grammar on an anthropic tools turn. -/ +theorem no_grammar_on_anthropic_tools (sox : Bool) : + carrier .anthropic true sox true = .none := by + cases sox <;> decide + +/-- Once a provider rejected json_schema / output_config, it is never sent again. -/ theorem no_json_schema_after_rejection (w : Wire) (tools : Bool) : - carrier w true true tools ≠ .jsonSchema := by + carrier w true true tools ≠ .jsonSchema ∧ + carrier w true true tools ≠ .outputConfig := by cases w <;> cases tools <;> decide /-- No schema, no artifacts: the axis is byte-silent when unused. -/ @@ -70,10 +93,15 @@ theorem absent_schema_is_silent (w : Wire) (sox tools : Bool) : carrier w false sox tools = .none ∧ promptSchema w false sox = false := by cases w <;> cases sox <;> cases tools <;> decide -/-- The anthropic wire embeds the schema in the prompt unconditionally — - even when real tools displace the structured_output tool, nothing is lost. -/ -theorem anthropic_always_teaches (sox : Bool) : - promptSchema .anthropic true sox = true := by - cases sox <;> decide +/-- Tool-mode fallback still embeds the schema so a rejected output_config + cannot go silent. -/ +theorem anthropic_fallback_teaches : + promptSchema .anthropic true true = true := by + decide + +/-- Native Anthropic uses the light prompt (server enforces on the formatting turn). -/ +theorem anthropic_native_prompt_is_light : + promptSchema .anthropic true false = false := by + decide end Graff.StructuredOutput diff --git a/spec/kernels/structured_output.json b/spec/kernels/structured_output.json index 36589510..82388756 100644 --- a/spec/kernels/structured_output.json +++ b/spec/kernels/structured_output.json @@ -38,8 +38,8 @@ "schema": true, "sox": false, "tools": false, - "carrier": "toolAnthropic", - "prompt_schema": true + "carrier": "outputConfig", + "prompt_schema": false }, { "wire": "anthropic", @@ -47,7 +47,7 @@ "sox": false, "tools": true, "carrier": "none", - "prompt_schema": true + "prompt_schema": false }, { "wire": "anthropic", diff --git a/spec/ref/structured_output.py b/spec/ref/structured_output.py index 351066fd..d1f11cba 100644 --- a/spec/ref/structured_output.py +++ b/spec/ref/structured_output.py @@ -1,4 +1,4 @@ -"""Executable port of lean-proofs/Graff/StructuredOutput.lean (#543).""" +"""Executable port of lean-proofs/Graff/StructuredOutput.lean (#543 / #550).""" from __future__ import annotations @@ -15,16 +15,18 @@ def carrier(wire: str, schema: bool, sox: bool, tools: bool) -> str: if not sox: return "jsonSchema" return "jsonObject" if tools else "toolOpenai" - # anthropic: no response_format exists on this wire at all - return "none" if tools else "toolAnthropic" + # anthropic: native output_config.format on the formatting turn (#550); + # sox (rejected) falls back to the structured_output tool. Tools turns + # never carry a grammar (ADR 0001). + if tools: + return "none" + return "toolAnthropic" if sox else "outputConfig" def prompt_schema(wire: str, schema: bool, sox: bool) -> bool: if not schema: return False - if wire == "anthropic": - return True - if wire == "openai": + if wire in ("anthropic", "openai"): return sox return False @@ -51,18 +53,28 @@ def check_properties() -> int: for row in cells(): n += 1 w, s, x, t = row["wire"], row["schema"], row["sox"], row["tools"] - # never_silent: a set schema always reaches the provider somewhere. + # never_silent: a set schema always reaches the provider, except an + # anthropic tools turn (ADR 0001; the two-phase split holds it). if s and row["carrier"] == "none" and not row["prompt_schema"]: - raise ValueError(f"never_silent violated: {row}") - # sox_only_on_chat: the learned degrade changes nothing off the chat wire. - if w != "openai" and carrier(w, s, True, t) != carrier(w, s, False, t): - raise ValueError(f"sox_only_on_chat violated: {row}") - # no_json_schema_after_rejection. - if x and row["carrier"] == "jsonSchema": + if not (w == "anthropic" and t): + raise ValueError(f"never_silent violated: {row}") + # sox_leaves_responses_alone + if w == "responses" and carrier(w, s, True, t) != carrier(w, s, False, t): + raise ValueError(f"sox_leaves_responses_alone violated: {row}") + # sox_degrades_anthropic_native + if w == "anthropic" and s and not t: + want = "toolAnthropic" if x else "outputConfig" + if row["carrier"] != want: + raise ValueError(f"sox_degrades_anthropic_native violated: {row}") + # no_json_schema_after_rejection / no leftover output_config + if x and row["carrier"] in ("jsonSchema", "outputConfig"): raise ValueError(f"no_json_schema_after_rejection violated: {row}") - # absent_schema_is_silent. + # absent_schema_is_silent if not s and (row["carrier"] != "none" or row["prompt_schema"]): raise ValueError(f"absent_schema_is_silent violated: {row}") + # no_grammar_on_anthropic_tools + if w == "anthropic" and s and t and row["carrier"] != "none": + raise ValueError(f"no_grammar_on_anthropic_tools violated: {row}") return n diff --git a/src/agent.zig b/src/agent.zig index 031b8eca..12e8771d 100644 --- a/src/agent.zig +++ b/src/agent.zig @@ -225,7 +225,7 @@ pub const Agent = struct { streamed_args: ArgTool = .none, // which meta tool's prose streamed live this request streamed_args_len: usize = 0, // raw bytes emitted for it (gates re-print suppression) cap_new: bool = false, // provider rejected max_tokens → use max_completion_tokens - sox_json_object: bool = false, // #543: provider rejected response_format json_schema → json_object + schema-in-prompt + sox_json_object: bool = false, // #543/#550: rejected json_schema / output_config → tool or json_object + schema-in-prompt effort_rejected: bool = false, // model rejected reasoning_effort → drop it (e.g. gpt-5.5 on chat/completions wants /v1/responses) output_schema: ?[]const u8 = null, // --output-schema: JSON schema the final answer must satisfy (response_format / text.format, #502) next_ask_id: u64 = 1, diff --git a/src/agent_request.zig b/src/agent_request.zig index 7cd03bf2..3cbf5ae0 100644 --- a/src/agent_request.zig +++ b/src/agent_request.zig @@ -537,7 +537,10 @@ pub fn request(self: *Agent, tools_in: ?[]const u8) !std.json.ObjectMap { // "This response_format type is unavailable now") must not lose the // --output-schema contract — retry in json_object mode with the // schema moved into the prompt, on the same ladder as cap_new. - if (self.output_schema != null and !self.sox_json_object and std.mem.indexOf(u8, msg, "response_format") != null) { + if (self.output_schema != null and !self.sox_json_object and + (std.mem.indexOf(u8, msg, "response_format") != null or + std.mem.indexOf(u8, msg, "output_config") != null)) + { self.sox_json_object = true; continue; } diff --git a/src/agent_request_body.zig b/src/agent_request_body.zig index f8755edd..04377d38 100644 --- a/src/agent_request_body.zig +++ b/src/agent_request_body.zig @@ -96,10 +96,8 @@ pub fn buildBody(self: *Agent, tools: ?[]const u8, force_tool: bool, stream: boo try s.objectField("tool_choice"); try s.print("{s}", .{"{\"type\":\"any\"}"}); } - } else if (self.output_schema != null) { - // #543: this wire has no response_format at all — the schema is - // ALWAYS delivered as the structured_output tool (dsh pattern). - try @import("agent_request_body_responses.zig").writeAnthropicStructuredTool(&s, self.output_schema.?); + } else if (self.output_schema) |schema_json| { + try @import("agent_request_body_responses.zig").writeAnthropicSchema(&s, self, schema_json); } try s.objectField("messages"); // Cache the conversation prefix too (not just system) on the real diff --git a/src/agent_request_body_responses.zig b/src/agent_request_body_responses.zig index b76b962e..3c91f448 100644 --- a/src/agent_request_body_responses.zig +++ b/src/agent_request_body_responses.zig @@ -134,13 +134,19 @@ pub fn schemaAwarePrompt(self: *Agent) ![]const u8 { const base = self.systemPrompt(); if (self.output_schema == null) return base; // #543 degrade: this provider cannot enforce json_schema server-side — - // learned on the chat wire (sox), structural on the anthropic wire (no - // response_format exists) — so the schema itself must reach the model: - // through the structured_output tool when offered (the tools-off - // formatting turn), else as the entire final message. + // learned on the chat wire (sox), or structural on minimax / kimi-anthropic + // (tool mode) — so the schema itself must reach the model through the + // structured_output tool when offered (the tools-off formatting turn), + // else as the entire final message. // Chat-wire only for sox: a stale learned flag must not perturb the // responses wire's bytes (native text.format needs no prompt embed). - if ((self.provider.kind == .openai and self.sox_json_object) or self.provider.kind == .anthropic) return std.mem.concat(self.scratchAlloc(), u8, &.{ + // Full-schema embed is the tool-mode / learned-fallback path. Native + // Anthropic `output_config.format` (provider id `anthropic`, no sox) + // enforces server-side on the formatting turn — same light prompt as + // Responses. minimax / kimi-anthropic stay on the tool. + const anthropic_tool_mode = self.provider.kind == .anthropic and + (self.sox_json_object or !std.mem.eql(u8, self.provider.id, "anthropic")); + if ((self.provider.kind == .openai and self.sox_json_object) or anthropic_tool_mode) return std.mem.concat(self.scratchAlloc(), u8, &.{ base, "\n\nA JSON output schema is enforced on your final answer. Use tools first to gather every fact you need — never guess values. This provider cannot enforce the schema server-side, so satisfy it yourself: call the structured_output tool when it is offered, otherwise reply with a single JSON object, matching exactly this schema: ", self.output_schema.?, @@ -206,11 +212,35 @@ pub fn writeStructuredOutputTool(s: *std.json.Stringify, schema_json: []const u8 try s.endArray(); } -/// #543, anthropic wire: same tool, Anthropic tool shape (input_schema). This -/// wire has no response_format at all, so the tool is not a degrade — it is -/// the only server-visible carrier the schema has (Anthropic's own canonical -/// structured-output pattern). Unforced, as everywhere: forced tool_choice -/// conflicts with thinking on this wire too. +/// #550: native Anthropic structured outputs on the tools-off formatting +/// turn. Provider id `anthropic` only — minimax / kimi-anthropic keep the +/// structured_output tool, and a learned sox flag falls back to it too. +pub fn writeAnthropicSchema(s: *std.json.Stringify, self: *const Agent, schema_json: []const u8) !void { + if (std.mem.eql(u8, self.provider.id, "anthropic") and !self.sox_json_object) { + try writeAnthropicOutputConfig(s, schema_json); + return; + } + try writeAnthropicStructuredTool(s, schema_json); +} + +/// kimi-code's anthropic adapter: `output_config.format = {type, schema}`. +/// Formatting turn only (ADR 0001): never attach this to a tools turn. +pub fn writeAnthropicOutputConfig(s: *std.json.Stringify, schema_json: []const u8) !void { + try s.objectField("output_config"); + try s.beginObject(); + try s.objectField("format"); + try s.beginObject(); + try s.objectField("type"); + try s.write("json_schema"); + try s.objectField("schema"); + try s.print("{s}", .{schema_json}); + try s.endObject(); + try s.endObject(); +} + +/// #543, anthropic wire: same tool, Anthropic tool shape (input_schema). +/// Fallback for minimax / kimi-anthropic and for a rejected output_config. +/// Unforced: forced tool_choice conflicts with thinking on this wire too. pub fn writeAnthropicStructuredTool(s: *std.json.Stringify, schema_json: []const u8) !void { try s.objectField("tools"); try s.beginArray(); @@ -424,35 +454,54 @@ test "#543: json_schema rejection degrades dsh-style — forced structured_outpu try std.testing.expect(std.mem.indexOf(u8, body2, "json_schema") == null); } -test "#543: the anthropic wire always carries the schema as a structured_output tool (no response_format exists there)" { +test "#550: anthropic id uses output_config.format on the formatting turn; tool-mode is the fallback" { var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena_state.deinit(); const schema = "{\"type\":\"object\",\"properties\":{\"answer\":{\"type\":\"string\"}},\"required\":[\"answer\"],\"additionalProperties\":false}"; var agent = try testAgentFor(arena_state.allocator(), "anthropic", .anthropic, "claude-sonnet-5"); - agent.output_schema = schema; // no sox learning needed — structural on this wire + agent.output_schema = schema; - // Tools-off formatting turn: anthropic tool shape, schema as input_schema, unforced. + // Tools-off formatting turn: native json_schema (ADR 0001 — not on tools turns). const body = try agent.buildBody(null, false, true, true); defer std.testing.allocator.free(body); - try std.testing.expect(std.mem.indexOf(u8, body, "\"tools\":[{\"name\":\"structured_output\"") != null); - try std.testing.expect(std.mem.indexOf(u8, body, "\"input_schema\":{\"type\":\"object\",\"properties\":{\"answer\"") != null); - try std.testing.expect(std.mem.indexOf(u8, body, "tool_choice") == null); + try std.testing.expect(std.mem.indexOf(u8, body, "\"output_config\":{\"format\":{\"type\":\"json_schema\",\"schema\":{\"type\":\"object\"") != null); + try std.testing.expect(std.mem.indexOf(u8, body, "structured_output") == null); try std.testing.expect(std.mem.indexOf(u8, body, "response_format") == null); - // The cached system block carries the schema-aware suffix (was self.systemPrompt() before — the schema never reached this wire at all). - try std.testing.expect(std.mem.indexOf(u8, body, "cannot enforce the schema server-side") != null); + try std.testing.expect(std.mem.indexOf(u8, body, "cannot enforce the schema server-side") == null); - // Agentic turn with real tools: no synthetic tool, but the suffix still teaches the contract. + // Agentic turn: no grammar (ADR 0001), light prompt, no synthetic tool. const tools = "[{\"name\":\"bash\",\"description\":\"\",\"input_schema\":{\"type\":\"object\"}}]"; const body2 = try agent.buildBody(tools, false, true, true); defer std.testing.allocator.free(body2); try std.testing.expect(std.mem.indexOf(u8, body2, "\"name\":\"structured_output\"") == null); - try std.testing.expect(std.mem.indexOf(u8, body2, "cannot enforce the schema server-side") != null); - - // No schema → byte-identical prompt path (base), no tools field at all when tools==null. + try std.testing.expect(std.mem.indexOf(u8, body2, "output_config") == null); + try std.testing.expect(std.mem.indexOf(u8, body2, "cannot enforce the schema server-side") == null); + try std.testing.expect(std.mem.indexOf(u8, body2, "only the final message must match the schema") != null); + + // Learned rejection: same quirk flag as sox_json_object → tool mode. + agent.sox_json_object = true; + const body3 = try agent.buildBody(null, false, true, true); + defer std.testing.allocator.free(body3); + try std.testing.expect(std.mem.indexOf(u8, body3, "\"tools\":[{\"name\":\"structured_output\"") != null); + try std.testing.expect(std.mem.indexOf(u8, body3, "\"input_schema\":{\"type\":\"object\",\"properties\":{\"answer\"") != null); + try std.testing.expect(std.mem.indexOf(u8, body3, "output_config") == null); + try std.testing.expect(std.mem.indexOf(u8, body3, "cannot enforce the schema server-side") != null); + + // minimax keeps the tool even without sox (no native output_config). + var mm = try testAgentFor(arena_state.allocator(), "minimax", .anthropic, "MiniMax-M2.5"); + mm.output_schema = schema; + const mm_body = try mm.buildBody(null, false, true, true); + defer std.testing.allocator.free(mm_body); + try std.testing.expect(std.mem.indexOf(u8, mm_body, "\"tools\":[{\"name\":\"structured_output\"") != null); + try std.testing.expect(std.mem.indexOf(u8, mm_body, "output_config") == null); + + // No schema → byte-identical prompt path (base), no tools / output_config. + agent.sox_json_object = false; agent.output_schema = null; const plain = try agent.buildBody(null, false, true, true); defer std.testing.allocator.free(plain); try std.testing.expect(std.mem.indexOf(u8, plain, "structured_output") == null); + try std.testing.expect(std.mem.indexOf(u8, plain, "output_config") == null); try std.testing.expect(std.mem.indexOf(u8, plain, "\"tools\"") == null); } diff --git a/src/mcp.zig b/src/mcp.zig index d4db26d6..a1442473 100644 --- a/src/mcp.zig +++ b/src/mcp.zig @@ -85,6 +85,9 @@ pub const Registry = struct { /// registry came from `empty*` and `init` never ran, so session_start sets /// this field either way. global_config_path: ?[]const u8 = null, + /// True when `global_config_path` came from `GRAFF_MCP_CONFIG`. `/mcp trust` + /// re-reads through the same wholesale-off rule `init` used (#549). + global_is_override: bool = false, /// Per-server arenas from mcp_boot's parallel connect fan-out (the /// registry arena is not thread-safe, so each task allocated its own). /// Freed in deinit AFTER the transports referencing them are torn down. @@ -183,7 +186,7 @@ pub const Registry = struct { /// (no tool calls in flight). pub fn trustWorkspace(reg: *Registry, config_path: []const u8) !usize { const a = reg.arena(); - const merged = mcp_config.load(reg.io, a, Io.Dir.cwd(), config_path, reg.global_config_path, reg.home); + const merged = mcp_config.load(reg.io, a, Io.Dir.cwd(), config_path, reg.global_config_path, reg.home, reg.global_is_override); var servers: std.ArrayList(*Server) = .empty; try servers.appendSlice(a, reg.servers); @@ -217,7 +220,7 @@ pub const Registry = struct { /// best-effort (any read/parse failure contributes nothing). pub fn pendingWorkspace(reg: *Registry, config_path: []const u8) usize { const a = reg.arena(); - const merged = mcp_config.load(reg.io, a, Io.Dir.cwd(), config_path, reg.global_config_path, reg.home); + const merged = mcp_config.load(reg.io, a, Io.Dir.cwd(), config_path, reg.global_config_path, reg.home, reg.global_is_override); var n: usize = 0; var it = merged.servers.iterator(); while (it.next()) |entry| { diff --git a/src/mcp_boot.zig b/src/mcp_boot.zig index a80160ca..ceafed78 100644 --- a/src/mcp_boot.zig +++ b/src/mcp_boot.zig @@ -83,13 +83,14 @@ pub fn init(gpa: Allocator, io: Io, config_path: []const u8, global_path: ?[]con .stdio_probe = if (environ_map.get("GRAFF_MCP_PROBE")) |v| !std.mem.eql(u8, v, "0") else true, .show_diagnostics = show_diagnostics, .global_config_path = global_path, + .global_is_override = mcp_config.isEnvOverride(environ_map), }; mcp_rpc.applyHandshakeTimeoutEnv(environ_map); // #275 GRAFF_MCP_HANDSHAKE_SECS + #327 GRAFF_MCP_PROBE_MS, on the same pass as the probe flag errdefer reg.deinit(); const a = reg.arena(); - const merged = mcp_config.load(io, a, Io.Dir.cwd(), config_path, global_path, home); + const merged = mcp_config.load(io, a, Io.Dir.cwd(), config_path, global_path, home, reg.global_is_override); if (!merged.found) { reg.arena_state.deinit(); // nothing was started; no transports to tear down return null; diff --git a/src/mcp_cli.zig b/src/mcp_cli.zig index b20b24a1..e86c52d4 100644 --- a/src/mcp_cli.zig +++ b/src/mcp_cli.zig @@ -173,7 +173,7 @@ pub fn mcpCommand(io: Io, gpa: Allocator, arena: Allocator, home: []const u8, en } if (args.len == 0 or std.mem.eql(u8, args[0], "list")) { - const merged = mcp_config.load(io, arena, Io.Dir.cwd(), mcp_config_path, global_path, home); + const merged = mcp_config.load(io, arena, Io.Dir.cwd(), mcp_config_path, global_path, home, mcp_config.isEnvOverride(environ_map)); try mcp_config.reportInvalid(merged, &out.interface, mcp_config_path, global_path, "", ""); if (merged.servers.count() == 0) { try out.interface.writeAll("no MCP servers configured. Add one with `graff mcp add -- [args...]`,\nor list servers for every project in ~/" ++ mcp_config.global_rel_path ++ ".\n"); @@ -217,7 +217,7 @@ pub fn mcpCommand(io: Io, gpa: Allocator, arena: Allocator, home: []const u8, en const name = args[1]; // Global servers are loginable too — the merged set is the same one // the session connects from. - const merged = mcp_config.load(io, arena, Io.Dir.cwd(), mcp_config_path, global_path, home); + const merged = mcp_config.load(io, arena, Io.Dir.cwd(), mcp_config_path, global_path, home, mcp_config.isEnvOverride(environ_map)); // Say which file is broken before claiming the server is missing: // "not configured" for a server that IS configured, in a file that // does not parse, sends the user looking in the wrong place. diff --git a/src/mcp_config.zig b/src/mcp_config.zig index 63ec590e..cfb7d655 100644 --- a/src/mcp_config.zig +++ b/src/mcp_config.zig @@ -67,6 +67,14 @@ pub fn globalPath(arena: Allocator, home: []const u8, environ_map: anytype) ?[]c return std.fmt.allocPrint(arena, "{s}/" ++ global_rel_path, .{home}) catch null; } +/// Whether `GRAFF_MCP_CONFIG` points at a real path. An existing, valid, empty +/// override is a wholesale MCP off-switch (#549); the default global file is +/// never that — relocating `~/.codegraff/mcp.json` still merges the project. +pub fn isEnvOverride(environ_map: anytype) bool { + if (environ_map.get(path_env)) |override| return override.len > 0; + return false; +} + /// Read `path` and return its `mcpServers` object. Best-effort throughout: /// only "no such file" reads as absent, everything else reads as invalid (and /// empty) so a caller can say which file it could not use. @@ -105,12 +113,24 @@ fn readServers(io: Io, arena: Allocator, dir: Io.Dir, path: []const u8, found: * /// handle it is opened through. Never fails: a missing or malformed file simply /// contributes nothing. Values stay arena-allocated, like the rest of the MCP /// config handling. -pub fn load(io: Io, arena: Allocator, dir: Io.Dir, project_path: []const u8, global_path: ?[]const u8, home: []const u8) Merged { +/// +/// `global_is_override` is true only when `GRAFF_MCP_CONFIG` selected the +/// path. A file that exists, parses, and lists no servers is then a wholesale +/// off-switch (pty/harness): the project `.mcp.json` and plugin trees do not +/// merge in. A *populated* override still merges, so relocating the global +/// file is unchanged. A missing or invalid override is not an off-switch. +pub fn load(io: Io, arena: Allocator, dir: Io.Dir, project_path: []const u8, global_path: ?[]const u8, home: []const u8, global_is_override: bool) Merged { var merged: Merged = .{}; const global = if (global_path) |p| readServers(io, arena, dir, p, &merged.found, &merged.invalid_global) else std.json.ObjectMap.empty; + // #549: existing + valid + empty override → MCP stays off. `found` is + // already true (the file existed), so Registry.init does not treat this + // as "no config" and then pick up a project file on a later read. + if (global_is_override and merged.found and !merged.invalid_global and global.count() == 0) + return merged; + merged.project = readServers(io, arena, dir, project_path, &merged.found, &merged.invalid_project); // Global first, project second: the later `put` for a name overwrites, so @@ -163,7 +183,11 @@ const TestEnv = struct { /// Both halves are read through the tmp dir handle, so no test touches `$HOME` /// or the real workspace. fn loadTmp(arena: Allocator, dir: Io.Dir) Merged { - return load(testing.io, arena, dir, ".mcp.json", "global.json", ""); + return load(testing.io, arena, dir, ".mcp.json", "global.json", "", false); +} + +fn loadTmpOverride(arena: Allocator, dir: Io.Dir) Merged { + return load(testing.io, arena, dir, ".mcp.json", "global.json", "", true); } test "global MCP path prefers the env override and otherwise lives under ~/.codegraff" { @@ -174,6 +198,9 @@ test "global MCP path prefers the env override and otherwise lives under ~/.code try testing.expectEqualStrings("/tmp/mcp.json", globalPath(arena, "/home/alice", TestEnv{ .override = "/tmp/mcp.json" }).?); // An empty override falls back rather than resolving to the file "". try testing.expectEqualStrings("/home/alice/.codegraff/mcp.json", globalPath(arena, "/home/alice", TestEnv{ .override = "" }).?); + try testing.expect(isEnvOverride(TestEnv{ .override = "/tmp/mcp.json" })); + try testing.expect(!isEnvOverride(TestEnv{ .override = "" })); + try testing.expect(!isEnvOverride(TestEnv{})); // No HOME and no override is "no global config", not a crash. try testing.expect(globalPath(arena, "", TestEnv{}) == null); } @@ -262,3 +289,58 @@ test "an unreadable MCP config is invalid, not absent" { try testing.expect(!merged.invalid_project); try testing.expectEqual(@as(usize, 0), merged.servers.count()); } + +test "a valid empty GRAFF_MCP_CONFIG override is a wholesale MCP off-switch (#549)" { + var arena_state = std.heap.ArenaAllocator.init(testing.allocator); + defer arena_state.deinit(); + var tmp = testing.tmpDir(.{}); + defer tmp.cleanup(); + try tmp.dir.writeFile(testing.io, .{ .sub_path = "global.json", .data = "{\"mcpServers\":{}}" }); + try tmp.dir.writeFile(testing.io, .{ .sub_path = ".mcp.json", .data = "{\"mcpServers\":{\"local\":{\"command\":\"./srv\"}}}" }); + + const off = loadTmpOverride(arena_state.allocator(), tmp.dir); + try testing.expect(off.found); + try testing.expect(!off.invalid_global and !off.invalid_project); + try testing.expectEqual(@as(usize, 0), off.servers.count()); + try testing.expectEqual(@as(usize, 0), off.project.count()); + + // The default global file is never that switch: an empty ~/.codegraff/mcp.json + // still merges the project (relocating the global file stays a merge). + const merged = loadTmp(arena_state.allocator(), tmp.dir); + try testing.expectEqual(@as(usize, 1), merged.servers.count()); + try testing.expect(merged.servers.get("local") != null); +} + +test "a populated GRAFF_MCP_CONFIG override still merges the project file (#549)" { + var arena_state = std.heap.ArenaAllocator.init(testing.allocator); + defer arena_state.deinit(); + var tmp = testing.tmpDir(.{}); + defer tmp.cleanup(); + try tmp.dir.writeFile(testing.io, .{ .sub_path = "global.json", .data = "{\"mcpServers\":{\"deepwiki\":{\"url\":\"https://mcp.deepwiki.com/mcp\"}}}" }); + try tmp.dir.writeFile(testing.io, .{ .sub_path = ".mcp.json", .data = "{\"mcpServers\":{\"local\":{\"command\":\"./srv\"}}}" }); + + const merged = loadTmpOverride(arena_state.allocator(), tmp.dir); + try testing.expectEqual(@as(usize, 2), merged.servers.count()); + try testing.expect(merged.isGlobalOnly("deepwiki")); + try testing.expect(!merged.isGlobalOnly("local")); +} + +test "a missing or invalid GRAFF_MCP_CONFIG override is not an off-switch (#549)" { + var arena_state = std.heap.ArenaAllocator.init(testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + var missing = testing.tmpDir(.{}); + defer missing.cleanup(); + try missing.dir.writeFile(testing.io, .{ .sub_path = ".mcp.json", .data = "{\"mcpServers\":{\"local\":{\"command\":\"./srv\"}}}" }); + const no_file = loadTmpOverride(arena, missing.dir); + try testing.expect(no_file.servers.get("local") != null); + + var broken = testing.tmpDir(.{}); + defer broken.cleanup(); + try broken.dir.writeFile(testing.io, .{ .sub_path = "global.json", .data = "{ not json" }); + try broken.dir.writeFile(testing.io, .{ .sub_path = ".mcp.json", .data = "{\"mcpServers\":{\"local\":{\"command\":\"./srv\"}}}" }); + const bad = loadTmpOverride(arena, broken.dir); + try testing.expect(bad.invalid_global); + try testing.expect(bad.servers.get("local") != null); +} diff --git a/src/session_start.zig b/src/session_start.zig index ed194136..233686a8 100644 --- a/src/session_start.zig +++ b/src/session_start.zig @@ -422,7 +422,7 @@ pub fn initRegistryConsent(io: Io, gpa: Allocator, arena: Allocator, out: *Io.Wr // can carry chatter. With a global config `mcp_count > 0` in every project, // so an unguarded line here would corrupt the head of every --json run. const quiet = json_mode or flags.oneshot_prompt != null or environ_map.get("GRAFF_REPL_DEBUG") == null; - const merged = mcp_config.load(io, arena, Io.Dir.cwd(), mcp_config_path, global_path, home); + const merged = mcp_config.load(io, arena, Io.Dir.cwd(), mcp_config_path, global_path, home, mcp_config.isEnvOverride(environ_map)); // Interactive only: json/one-shot stdout cannot carry chatter. A slow // Cursor/Claude tree used to look like a hung boot; this line is the clock. if (!json_mode and flags.oneshot_prompt == null) { @@ -472,6 +472,7 @@ pub fn initRegistryConsent(io: Io, gpa: Allocator, arena: Allocator, out: *Io.Wr // file precisely when consent was declined and no `init` ever ran. `arena` // is the session arena, so the path outlives the registry. registry.global_config_path = global_path; + registry.global_is_override = mcp_config.isEnvOverride(environ_map); registry.show_diagnostics = json_mode or flags.oneshot_prompt != null or environ_map.get("GRAFF_REPL_DEBUG") != null; return registry; } diff --git a/src/spec_structured_output_conformance.zig b/src/spec_structured_output_conformance.zig index 2f03637f..7f1f4d8d 100644 --- a/src/spec_structured_output_conformance.zig +++ b/src/spec_structured_output_conformance.zig @@ -53,6 +53,8 @@ fn toolsFor(wire: []const u8) []const u8 { fn classify(body: []const u8, wire: []const u8) []const u8 { if (std.mem.indexOf(u8, body, "\"text\":{\"format\":{\"type\":\"json_schema\"") != null) return "textFormat"; + if (std.mem.indexOf(u8, body, "\"format\":{\"type\":\"json_schema\"") != null and + std.mem.indexOf(u8, body, "\"output_config\"") != null) return "outputConfig"; if (std.mem.indexOf(u8, body, "\"response_format\":{\"type\":\"json_schema\"") != null) return "jsonSchema"; if (std.mem.indexOf(u8, body, "\"response_format\":{\"type\":\"json_object\"}") != null) return "jsonObject"; if (std.mem.indexOf(u8, body, "\"name\":\"structured_output\"") != null) @@ -90,7 +92,10 @@ test "spec/structured_output: buildBody's carrier matches the model on every cel ); return error.CatalogMismatch; } - // never_silent, on the live bytes: a set schema always reaches the wire. - if (schema and std.mem.eql(u8, got, "none") and !prompt_got) return error.SchemaSilentlyDropped; + // never_silent: a set schema reaches the wire, except an anthropic + // tools turn (ADR 0001 — the two-phase split holds it for formatting). + if (schema and std.mem.eql(u8, got, "none") and !prompt_got) { + if (!(std.mem.eql(u8, wire, "anthropic") and tools)) return error.SchemaSilentlyDropped; + } } } From e755c17cf9f6bb00022542eaf328009c100813f2 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 27 Aug 2026 01:23:44 +0000 Subject: [PATCH 02/10] feat(#629): force experiment fan-out, list the pool, deliver back Mint/reuse is proven against a real git repo. While a pool is armed the root system prompt says it MUST spawn and must not edit the caller tree. graff worktree list tags exp-* trees. Child finish reports path, branch, keep-reason, and diffstat and never deletes a pool seat. --- docs/adr/0037-experiment-pool-is-opt-in.md | 10 +- docs/adr/README.md | 2 +- src/cli.zig | 2 +- src/commands_experiment.zig | 6 + src/experiment_pool.zig | 177 +++++++++++++++++++-- src/startup.zig | 4 + src/subagent_run.zig | 31 +++- src/worktree_prune.zig | 8 +- 8 files changed, 210 insertions(+), 30 deletions(-) diff --git a/docs/adr/0037-experiment-pool-is-opt-in.md b/docs/adr/0037-experiment-pool-is-opt-in.md index ea28ca79..b1641137 100644 --- a/docs/adr/0037-experiment-pool-is-opt-in.md +++ b/docs/adr/0037-experiment-pool-is-opt-in.md @@ -7,15 +7,17 @@ Status: accepted 2026-08-26 #629: default isolation is `shared_cwd`, so "run 3 approaches" stays on one agent. Per-spawn `isolation: worktree` pays `git worktree add` mid-turn. -This record lives on `release/v0.0.279` after the v0.0.279 tag. It is **not** -on `main` until a later cut. +On `main` as of the 279 continuation. ## Decision `--experiment N` / `/experiment N` mints N trees under `.graff/worktrees/exp-{id}/` **before** the first child. Each spawn claims -the next seat and sets `agent_cwd`. The root stays on the caller tree. -Default sessions are unchanged. Pool trees are not auto-deleted. +the next seat and sets `agent_cwd`. The root stays on the caller tree and +is told it **must** spawn (system-prompt mandate), not edit. Default +sessions are unchanged. Pool trees are not auto-deleted: finish reports +path, branch, keep-reason, and diffstat. `graff worktree list` tags them +`(experiment pool)`. ## Consequences diff --git a/docs/adr/README.md b/docs/adr/README.md index fcc12358..e9ef9c46 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -47,7 +47,7 @@ record only when you need the evidence or the edge cases. | [0034](0034-remote-images-stay-native.md) | JSON/serve image inputs are a typed URL/base64 union, validated atomically and preserved as native provider vision blocks; never flatten pixels into prompt text. | | [0035](0035-first-turn-skips-deferred-mcp-join.md) | First model call after a deferred MCP boot does not wait for the handshake; native tools run now, MCP catalogs merge on the next request. | | [0036](0036-computer-use-keeps-the-signed-codex-bridge.md) | Codex Computer Use keeps its authenticated node_repl process chain: Graff launches it through the signed Codex sandbox wrapper, never embeds V8 or spoofs the service. | -| [0037](0037-experiment-pool-is-opt-in.md) | `--experiment N` / `/experiment N` pre-mints a child worktree pool; default isolation stays `shared_cwd`. 279 continuation — not on main until a later cut. | +| [0037](0037-experiment-pool-is-opt-in.md) | `--experiment N` / `/experiment N` pre-mints a child worktree pool; the root must spawn; pool trees are listed and delivered back, never auto-deleted. | | [0038](0038-in-process-acp-core.md) | Same-process embed is `libgraff` + `graff-core.wasm` + `createGraffAgent()` (ACP core, echo turn). Live coding stays `graff acp`. 279 continuation — not on main until a later cut. | ## When to write one diff --git a/src/cli.zig b/src/cli.zig index c65e2dcb..0a36e2ca 100644 --- a/src/cli.zig +++ b/src/cli.zig @@ -240,7 +240,7 @@ pub const usage_text = \\ graff mcp list configured MCP servers \\ graff plugins [load ] list Claude/Cursor/Grok/Codex plugin trees (in place) \\ graff learn [help] local mutate/evaluate/promote/rollback engine - \\ graff worktree list list the per-tab worktrees created by -w + \\ graff worktree list list -w tabs and experiment-pool trees (tagged) \\ graff worktree merge squash-land worktree- onto the current branch + clean up \\ graff worktree remove discard worktree- (drops its scratch work) + delete the branch \\ graff worktree prune drop git registrations for worktrees whose dirs were deleted diff --git a/src/commands_experiment.zig b/src/commands_experiment.zig index 27c77822..a15e7397 100644 --- a/src/commands_experiment.zig +++ b/src/commands_experiment.zig @@ -38,6 +38,12 @@ pub fn tryHandle(root: *Agent, arena: Allocator, line: []const u8, out: *Io.Writ try out.flush(); return true; }; + if (pool.directive()) |d| { + if (root.sys_base.len > 0 and std.mem.indexOf(u8, root.sys_base, pool.directive_marker) == null) { + const next = std.fmt.allocPrint(arena, "{s}\n\n{s}", .{ root.sys_base, d }) catch root.sys_base; + @import("prompts.zig").setSystemPrompts(root, next, arena) catch {}; + } + } try out.print(" {s}experiment {s}: {d} trees under .graff/worktrees/exp-{s}/ — spawn, do not edit here{s}\n", .{ style.dim, id, minted, id, style.reset, }); diff --git a/src/experiment_pool.zig b/src/experiment_pool.zig index 44621aa1..cf968fea 100644 --- a/src/experiment_pool.zig +++ b/src/experiment_pool.zig @@ -1,8 +1,8 @@ -//! #629 first slice: a pre-minted worktree pool for one experiment. -//! Lives on `release/v0.0.279` only (not merged to main). Fan-out seats -//! children in already-created trees so the first tool is not `git worktree add`. +//! #629 experiment worktree pool: mint N trees first, seat children in them, +//! force the root to spawn, list/deliver-back without deleting the pool. const std = @import("std"); +const builtin = @import("builtin"); const Io = std.Io; const Allocator = std.mem.Allocator; @@ -12,12 +12,16 @@ const ranOk = process_runner.ranOk; pub const cap: u8 = 16; -const Slot = struct { path: []const u8, branch: []const u8 }; +pub const Seat = struct { path: []const u8 = "", branch: []const u8 = "", base: []const u8 = "" }; + +/// Standing line injected into the root system prompt while a pool is armed. +pub const directive_marker = "Experiment pool `"; var g_n: u8 = 0; var g_next: u8 = 0; var g_id: []const u8 = ""; -var g_slots: [cap]Slot = @splat(.{ .path = "", .branch = "" }); +var g_directive: []const u8 = ""; +var g_slots: [cap]Seat = @splat(.{}); pub fn enabled() bool { return g_n > 0; @@ -36,7 +40,8 @@ pub fn reset() void { g_n = 0; g_next = 0; g_id = ""; - g_slots = @splat(.{ .path = "", .branch = "" }); + g_directive = ""; + g_slots = @splat(.{}); } pub fn sanitizeId(buf: []u8, id: []const u8) []const u8 { @@ -57,10 +62,30 @@ pub fn slotBranch(buf: []u8, id: []const u8, i: u8) []const u8 { /// Next unused seat, or null when the pool is empty/exhausted. pub fn claim() ?[]const u8 { + const seat = claimSeat() orelse return null; + return seat.path; +} + +/// Path + branch + creation HEAD so finish can report keep-reason without +/// deleting the tree (ADR 0037: pool trees are not auto-deleted). +pub fn claimSeat() ?Seat { if (g_next >= g_n) return null; const i = g_next; g_next += 1; - return g_slots[i].path; + return g_slots[i]; +} + +/// Root must spawn, not edit. Null when the pool is off. +pub fn directive() ?[]const u8 { + if (g_n == 0 or g_directive.len == 0) return null; + return g_directive; +} + +/// `graff worktree list` tag: minted pool trees, even after this process resets. +pub fn isExperimentTree(path: []const u8, branch: []const u8) bool { + if (std.mem.indexOf(u8, path, ".graff/worktrees/exp-") != null) return true; + const b = if (std.mem.startsWith(u8, branch, "refs/heads/")) branch["refs/heads/".len..] else branch; + return std.mem.startsWith(u8, b, "graff/exp/"); } pub fn statusLine(buf: []u8) []const u8 { @@ -78,19 +103,31 @@ fn absOrRel(io: Io, arena: Allocator, path: []const u8) ![]const u8 { return arena.dupe(u8, buf[0..n]); } -fn mintOne(gpa: Allocator, io: Io, arena: Allocator, id: []const u8, i: u8) !Slot { +fn readHead(gpa: Allocator, io: Io, arena: Allocator, path: []const u8) []const u8 { + const r = runCapped(gpa, io, &.{ "git", "-C", path, "rev-parse", "HEAD" }, 4096, 4096, 15_000) catch return ""; + defer { + gpa.free(r.stdout); + gpa.free(r.stderr); + } + if (!ranOk(r)) return ""; + return arena.dupe(u8, std.mem.trim(u8, r.stdout, " \t\r\n")) catch ""; +} + +fn mintOne(gpa: Allocator, io: Io, arena: Allocator, id: []const u8, i: u8) !Seat { var pbuf: [256]u8 = undefined; var bbuf: [256]u8 = undefined; const rel = slotPath(&pbuf, id, i); const branch = try arena.dupe(u8, slotBranch(&bbuf, id, i)); - if (dirExists(io, rel)) return .{ .path = try absOrRel(io, arena, rel), .branch = branch }; - const add = runCapped(gpa, io, &.{ "git", "worktree", "add", rel, "-b", branch }, 8192, 8192, 60_000) catch return error.CreateFailed; - defer { - gpa.free(add.stdout); - gpa.free(add.stderr); + if (!dirExists(io, rel)) { + const add = runCapped(gpa, io, &.{ "git", "worktree", "add", rel, "-b", branch }, 8192, 8192, 60_000) catch return error.CreateFailed; + defer { + gpa.free(add.stdout); + gpa.free(add.stderr); + } + if (!ranOk(add) and !dirExists(io, rel)) return error.CreateFailed; } - if (!ranOk(add) and !dirExists(io, rel)) return error.CreateFailed; - return .{ .path = try absOrRel(io, arena, rel), .branch = branch }; + const path = try absOrRel(io, arena, rel); + return .{ .path = path, .branch = branch, .base = readHead(gpa, io, arena, path) }; } /// Create or reuse N trees under `.graff/worktrees/exp-{id}/`. Idempotent @@ -113,9 +150,55 @@ pub fn arm(gpa: Allocator, io: Io, arena: Allocator, id: []const u8, n: u8) !u8 } g_n = n; g_next = 0; + g_directive = try std.fmt.allocPrint(arena, "{s}{s}` is armed with {d} pre-minted worktrees under `.graff/worktrees/exp-{s}/` (branches `graff/exp/{s}/0` …). You MUST call the subagent tool once per independent task or A/B arm so each child claims a seat. Do not edit files in this caller tree. After children return, synthesize — do not redo their work here.", .{ + directive_marker, g_id, n, g_id, g_id, + }); return n; } +/// Report path / branch / keep-reason / diffstat. Never removes the tree. +pub fn deliverNote(gpa: Allocator, io: Io, seat: Seat) []const u8 { + const keep = @import("agent_worktree.zig"); + const st = runCapped(gpa, io, &.{ "git", "-C", seat.path, "status", "--porcelain" }, 1 << 16, 8192, 30_000) catch + return std.fmt.allocPrint(gpa, "\n\n[experiment seat kept (could not verify) — path: {s}, branch: {s}]", .{ seat.path, seat.branch }) catch ""; + defer { + gpa.free(st.stdout); + gpa.free(st.stderr); + } + var head_buf: [64]u8 = undefined; + const head = blk: { + const r = runCapped(gpa, io, &.{ "git", "-C", seat.path, "rev-parse", "HEAD" }, 4096, 4096, 15_000) catch break :blk ""; + defer { + gpa.free(r.stdout); + gpa.free(r.stderr); + } + if (!ranOk(r)) break :blk ""; + const trimmed = std.mem.trim(u8, r.stdout, " \t\r\n"); + const n = @min(trimmed.len, head_buf.len); + @memcpy(head_buf[0..n], trimmed[0..n]); + break :blk head_buf[0..n]; + }; + const reason = keep.worktreeKeepReason(ranOk(st), st.stdout, seat.base, head); + const why: []const u8 = if (reason == .removed) "clean, left in the pool" else keep.keepReasonText(reason); + var stat: []const u8 = ""; + if (seat.base.len > 0) { + if (runCapped(gpa, io, &.{ "git", "-C", seat.path, "diff", "--stat", seat.base }, 4096, 2048, 15_000)) |r| { + defer { + gpa.free(r.stdout); + gpa.free(r.stderr); + } + if (ranOk(r)) { + const trimmed = std.mem.trim(u8, r.stdout, " \t\r\n"); + if (trimmed.len > 0) stat = std.fmt.allocPrint(gpa, "; {s}", .{trimmed}) catch ""; + } + } else |_| {} + } + defer if (stat.len > 0) gpa.free(stat); + return std.fmt.allocPrint(gpa, "\n\n[experiment seat kept ({s}) — path: {s}, branch: {s}{s}]", .{ + why, seat.path, seat.branch, stat, + }) catch ""; +} + test "slot names are stable and claim walks the pool" { var pbuf: [64]u8 = undefined; var bbuf: [64]u8 = undefined; @@ -153,3 +236,67 @@ test "arm rejects empty and oversized pools before git" { try std.testing.expectError(error.BadPoolSize, arm(std.testing.allocator, std.testing.io, a, "live", 0)); try std.testing.expectError(error.BadPoolSize, arm(std.testing.allocator, std.testing.io, a, "live", 17)); } + +test "isExperimentTree matches pool paths and graff/exp branches" { + try std.testing.expect(isExperimentTree("/tmp/repo/.graff/worktrees/exp-live/0", "refs/heads/graff/exp/live/0")); + try std.testing.expect(isExperimentTree("/other", "graff/exp/q/1")); + try std.testing.expect(!isExperimentTree("/tmp/repo/.graff/worktrees/agent-sa-1", "refs/heads/graff/agents/sa-1")); + try std.testing.expect(!isExperimentTree("/tmp/repo", "main")); +} + +test "directive is off until arm writes the spawn mandate" { + reset(); + defer reset(); + try std.testing.expect(directive() == null); +} + +fn fixtureGit(gpa: Allocator, io: Io, argv: []const []const u8) !void { + const r = runCapped(gpa, io, argv, 1 << 16, 1 << 16, 60_000) catch return error.SkipZigTest; + defer gpa.free(r.stdout); + defer gpa.free(r.stderr); + if (!ranOk(r)) return error.FixtureCommandFailed; +} + +test "arm mints then reuses real git worktrees; deliver-back never deletes (#629)" { + if (builtin.os.tag == .windows) return; + const gpa = std.testing.allocator; + const io = std.testing.io; + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var orig_dir = try Io.Dir.cwd().openDir(io, ".", .{}); + defer orig_dir.close(io); + defer _ = std.posix.system.fchdir(orig_dir.handle); + if (std.posix.system.fchdir(tmp.dir.handle) != 0) return error.ChdirFailed; + + try Io.Dir.cwd().writeFile(io, .{ .sub_path = "work.txt", .data = "one\n" }); + try fixtureGit(gpa, io, &.{ "git", "init", "-q" }); + try fixtureGit(gpa, io, &.{ "git", "add", "-A" }); + try fixtureGit(gpa, io, &.{ "git", "-c", "user.email=t@example.com", "-c", "user.name=t", "-c", "commit.gpgsign=false", "commit", "-q", "--no-verify", "-m", "fixture" }); + + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + reset(); + defer reset(); + + try std.testing.expectEqual(@as(u8, 2), try arm(gpa, io, arena, "live", 2)); + try std.testing.expect(dirExists(io, ".graff/worktrees/exp-live/0")); + try std.testing.expect(dirExists(io, ".graff/worktrees/exp-live/1")); + try std.testing.expect(std.mem.indexOf(u8, directive().?, "MUST") != null); + try std.testing.expect(std.mem.indexOf(u8, directive().?, "Do not edit files in this caller tree") != null); + + const first0 = g_slots[0].path; + try std.testing.expectEqual(@as(u8, 2), try arm(gpa, io, arena, "live", 2)); + try std.testing.expectEqualStrings(first0, g_slots[0].path); + try std.testing.expect(g_slots[0].base.len > 0); + + const seat = claimSeat().?; + try Io.Dir.cwd().writeFile(io, .{ .sub_path = ".graff/worktrees/exp-live/0/dirty.txt", .data = "edit\n" }); + + const note = deliverNote(gpa, io, seat); + defer if (note.len > 0) gpa.free(note); + try std.testing.expect(std.mem.indexOf(u8, note, "experiment seat kept") != null); + try std.testing.expect(std.mem.indexOf(u8, note, seat.branch) != null); + try std.testing.expect(std.mem.indexOf(u8, note, "has changes") != null); + try std.testing.expect(dirExists(io, ".graff/worktrees/exp-live/0")); +} diff --git a/src/startup.zig b/src/startup.zig index 1e425e06..f851ae2d 100644 --- a/src/startup.zig +++ b/src/startup.zig @@ -173,6 +173,10 @@ pub fn buildSystemPrompt( // sys_normal/sys_strict/sys_ultra/sys_ultra_strict from it — the single // funnel every later mutation (repl, set_agent, set_system_prompt) must // also go through, so none of the four ever go stale independently. + // #629: --experiment is armed before this runs; the spawn mandate rides + // sys_base so a later playbook refresh cannot drop it. + if (@import("experiment_pool.zig").directive()) |d| + sys_normal = try std.fmt.allocPrint(arena, "{s}\n\n{s}", .{ sys_normal, d }); return sys_normal; } diff --git a/src/subagent_run.zig b/src/subagent_run.zig index 1850df6b..60ba8976 100644 --- a/src/subagent_run.zig +++ b/src/subagent_run.zig @@ -319,8 +319,11 @@ pub fn runSub(ctx: ToolCtx, kind: []const u8, label: []const u8, prompt: []const var wt: ?jobs.AgentWorktree = null; var isolation_note: []const u8 = ""; - if (@import("experiment_pool.zig").claim()) |seat| { - agent.agent_cwd = seat; + var pool_seat = false; + if (@import("experiment_pool.zig").claimSeat()) |seat| { + agent.agent_cwd = seat.path; + wt = .{ .path = seat.path, .branch = seat.branch, .base = seat.base }; + pool_seat = true; } else if (isolation == .worktree) { if (jobs.agentWorktreeCreate(gpa, ctx.io, arena, sub_id)) |created| { wt = created; @@ -419,9 +422,16 @@ pub fn runSub(ctx: ToolCtx, kind: []const u8, label: []const u8, prompt: []const const text = report catch |err| { var out = subagentFailure(gpa, sub_id, err, agent.last_api_error, attempts); if (wt) |w| { - const combined = std.fmt.allocPrint(gpa, "{s}\n\n[worktree left in place after failure — path: {s}, branch: {s}]", .{ out.text, w.path, w.branch }) catch return .{ .output = out, .usage = usage }; - gpa.free(out.text); - out.text = combined; + const tail = if (pool_seat) + @import("experiment_pool.zig").deliverNote(gpa, ctx.io, .{ .path = w.path, .branch = w.branch, .base = w.base }) + else + (std.fmt.allocPrint(gpa, "\n\n[worktree left in place after failure — path: {s}, branch: {s}]", .{ w.path, w.branch }) catch ""); + if (tail.len > 0) { + const combined = std.fmt.allocPrint(gpa, "{s}{s}", .{ out.text, tail }) catch return .{ .output = out, .usage = usage }; + gpa.free(out.text); + gpa.free(tail); + out.text = combined; + } } return .{ .output = out, .usage = usage }; }; @@ -437,10 +447,15 @@ pub fn runSub(ctx: ToolCtx, kind: []const u8, label: []const u8, prompt: []const var extra: []const u8 = isolation_note; var extra_owned = false; if (wt) |w| { - const outcome = jobs.agentWorktreeFinish(gpa, ctx.io, w); - if (outcome.kept) { - extra = std.fmt.allocPrint(gpa, "\n\n[worktree kept ({s}) — path: {s}, branch: {s}]", .{ jobs.keepReasonText(outcome.reason), w.path, w.branch }) catch ""; + if (pool_seat) { + extra = @import("experiment_pool.zig").deliverNote(gpa, ctx.io, .{ .path = w.path, .branch = w.branch, .base = w.base }); extra_owned = extra.len > 0; + } else { + const outcome = jobs.agentWorktreeFinish(gpa, ctx.io, w); + if (outcome.kept) { + extra = std.fmt.allocPrint(gpa, "\n\n[worktree kept ({s}) — path: {s}, branch: {s}]", .{ jobs.keepReasonText(outcome.reason), w.path, w.branch }) catch ""; + extra_owned = extra.len > 0; + } } } defer if (extra_owned) gpa.free(extra); diff --git a/src/worktree_prune.zig b/src/worktree_prune.zig index 9757c5df..a74669c4 100644 --- a/src/worktree_prune.zig +++ b/src/worktree_prune.zig @@ -258,7 +258,13 @@ pub fn listWithAge(gpa: Allocator, io: Io, arena: Allocator, out: *Io.Writer) !v const age = formatAge(&abuf, worktreeAgeMs(io, now_ms, e.path)); const label = if (e.branch.len > 0) shortBranch(e.branch) else if (e.detached) "(detached)" else if (e.bare) "(bare)" else ""; const is_main = i == 0 or (main_path.len > 0 and std.mem.eql(u8, e.path, main_path)); - try out.print("{s: <5} {s} [{s}]{s}\n", .{ age, e.path, label, if (is_main) " (main checkout)" else "" }); + const tag: []const u8 = if (is_main) + " (main checkout)" + else if (@import("experiment_pool.zig").isExperimentTree(e.path, e.branch)) + " (experiment pool)" + else + ""; + try out.print("{s: <5} {s} [{s}]{s}\n", .{ age, e.path, label, tag }); } } From f2b47e92a4cd40bda36aa04a70ac16cb1905d62f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 27 Aug 2026 02:48:37 +0000 Subject: [PATCH 03/10] TUI /tell and /peek route through the existing peer mailbox #563 leftovers: stop telling the changelog that the model can target "all", catalog the same /tell and /peek the line REPL already has, and run them through an engine PeerFn so the pager posts to the real room instead of reimplementing the channel. --- TUI/catalog.zig | 4 ++ TUI/dispatch.zig | 3 ++ TUI/engine.zig | 7 +++ TUI/overlaypane.zig | 8 ++-- TUI/peer_cmd.zig | 30 +++++++++++++ TUI/peer_tests.zig | 101 ++++++++++++++++++++++++++++++++++++++++++++ TUI/root.zig | 3 ++ TUI/run.zig | 1 + src/cli.zig | 2 +- src/presence.zig | 18 ++++++++ src/repl_glue.zig | 3 ++ src/test_hooks.zig | 1 + src/tui_launch.zig | 4 ++ src/tui_peer.zig | 69 ++++++++++++++++++++++++++++++ 14 files changed, 250 insertions(+), 4 deletions(-) create mode 100644 TUI/peer_cmd.zig create mode 100644 TUI/peer_tests.zig create mode 100644 src/tui_peer.zig diff --git a/TUI/catalog.zig b/TUI/catalog.zig index 3ed02ab6..6a4988fa 100644 --- a/TUI/catalog.zig +++ b/TUI/catalog.zig @@ -41,6 +41,8 @@ pub const items = [_]Item{ .{ .name = "/jump", .desc = "Jump to a previous turn" }, .{ .name = "/copy", .desc = "Copy the last reply to the clipboard" }, .{ .name = "/btw", .desc = "Queue an aside without interrupting" }, + .{ .name = "/tell", .desc = "Message a running graff (/tell all broadcasts)" }, + .{ .name = "/peek", .desc = "See what a live session is doing" }, .{ .name = "/vim-mode", .desc = "Vim keys in the scrollback", .aliases = &.{"/vim"} }, .{ .name = "/help", .desc = "List commands" }, .{ .name = "/doctor", .desc = "Health check" }, @@ -99,6 +101,8 @@ test "filter: slash prefix and alias" { try std.testing.expect(lookup("/debug") != null); try std.testing.expect(lookup("/cache") != null); try std.testing.expect(lookup("/cost") != null); + try std.testing.expect(lookup("/tell") != null); + try std.testing.expect(lookup("/peek") != null); try std.testing.expect(lookup("/not-a-cmd") == null); } diff --git a/TUI/dispatch.zig b/TUI/dispatch.zig index 43754dd9..e7ddb2c2 100644 --- a/TUI/dispatch.zig +++ b/TUI/dispatch.zig @@ -7,6 +7,7 @@ const app = @import("app.zig"); const bgop = @import("bgop.zig"); const catalog = @import("catalog.zig"); const engine = @import("engine.zig"); +const peer_cmd = @import("peer_cmd.zig"); const meters = @import("meters.zig"); const theme_mod = @import("theme.zig"); const turn = @import("turn.zig"); @@ -188,6 +189,8 @@ pub fn runCommand(self: *Model, line: []const u8) Effect { self.pushFmt(.system, "vim scrollback: {s}", .{onOff(self.vim_mode)}) catch {}; } else if (std.mem.eql(u8, canon, "/copy")) { copyLastReply(self); + } else if (std.mem.eql(u8, canon, "/tell") or std.mem.eql(u8, canon, "/peek")) { + peer_cmd.run(self, canon, arg); } else if (std.mem.eql(u8, canon, "/btw")) { if (arg.len == 0) { self.push(.system, "usage: /btw