From 07a1a465917bbc4e0f7c69dd710339d68a507a5f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 27 Aug 2026 01:06:41 +0000 Subject: [PATCH] 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; + } } }