mason: tolerate omitted subc response text

Author: ualtinok

crates/aft/src/subc/wire.rs

@@ -424,6 +424,7 @@ struct ToolResponseEnvelope<'a> {
 // reply without `structuredContent.text`, so a module-first rollout breaks
 // every installed plugin). Collapsing it safely means emitting both shapes,
 // waiting for plugins to update, then dropping one behind a version floor.
+// The bridge now accepts replies that omit `structuredContent.text`, and the module may omit that field after the minimum supported plugin version includes this compatibility behavior.
 //
 // Measured on a live daemon: the largest real frames were ~200 KB, with zero
 // egress-write time, writer queue depth 1, never full, and no reserve

packages/aft-bridge/src/__tests__/subc-transport.test.ts

@@ -168,6 +168,33 @@ function envelope(flat: Record<string, unknown>): Record<string, unknown> {
   };
 }
 
+// This fixture preserves the field order from a real read response: the Rust wire
+// serializer emits `text` as the final structured field.
+const CAPTURED_FIRST_PARTY_READ_ENVELOPE: Record<string, unknown> = {
+  content: [
+    {
+      type: "text",
+      text: "export function readFixture(): string { return 'captured'; }\n",
+    },
+  ],
+  isError: false,
+  structuredContent: {
+    id: "read-fixture-1",
+    success: true,
+    status_bar: { errors: 0, warnings: 1 },
+    bg_completions: [{ task_id: "bash-fixture-1" }],
+    text: "export function readFixture(): string { return 'captured'; }\n",
+  },
+};
+
+function withoutStructuredText(envelopeResponse: Record<string, unknown>): Record<string, unknown> {
+  const structuredContent = {
+    ...(envelopeResponse.structuredContent as Record<string, unknown>),
+  };
+  delete structuredContent.text;
+  return { ...envelopeResponse, structuredContent };
+}
+
 describe("SubcTransport.toolCall", () => {
   test("sends {name, arguments} and re-lifts structuredContent to the flat result", async () => {
     const client = new FakeClient(async () =>
@@ -436,6 +463,68 @@ describe("SubcTransport Rd reconnect", () => {
 });
 
 describe("SubcTransport reply envelope (B-#7)", () => {
+  test("synthesizes missing structuredContent.text from a single outer text block", async () => {
+    const duplicatedClient = new FakeClient(async () => CAPTURED_FIRST_PARTY_READ_ENVELOPE);
+    const { pool: duplicatedPool } = poolWith(duplicatedClient);
+    const duplicated = await duplicatedPool
+      .getBridge("/work/proj")
+      .toolCall("sess-1", "read", { filePath: "fixture.ts" });
+
+    const synthesizedClient = new FakeClient(async () =>
+      withoutStructuredText(CAPTURED_FIRST_PARTY_READ_ENVELOPE),
+    );
+    const { pool: synthesizedPool } = poolWith(synthesizedClient);
+    const synthesized = await synthesizedPool
+      .getBridge("/work/proj")
+      .toolCall("sess-1", "read", { filePath: "fixture.ts" });
+
+    const capturedBytes = new TextEncoder().encode(
+      JSON.stringify(CAPTURED_FIRST_PARTY_READ_ENVELOPE.structuredContent),
+    );
+    expect(new TextEncoder().encode(JSON.stringify(duplicated))).toEqual(capturedBytes);
+    expect(new TextEncoder().encode(JSON.stringify(synthesized))).toEqual(capturedBytes);
+  });
+
+  test("present structuredContent.text wins without inspecting outer content", async () => {
+    const response = {
+      ...CAPTURED_FIRST_PARTY_READ_ENVELOPE,
+      content: [{ type: "image", data: "not inspected" }],
+    };
+    const client = new FakeClient(async () => response);
+    const { pool } = poolWith(client);
+
+    const result = await pool.getBridge("/work/proj").toolCall("sess-1", "read", {});
+
+    expect(new TextEncoder().encode(JSON.stringify(result))).toEqual(
+      new TextEncoder().encode(
+        JSON.stringify(CAPTURED_FIRST_PARTY_READ_ENVELOPE.structuredContent),
+      ),
+    );
+  });
+
+  test.each([
+    ["zero blocks", []],
+    [
+      "multiple blocks",
+      [
+        { type: "text", text: "first" },
+        { type: "text", text: "second" },
+      ],
+    ],
+    ["non-text block", [{ type: "image", data: "not text" }]],
+  ])("missing structuredContent.text with %s stays fail-closed", async (_name, content) => {
+    const client = new FakeClient(async () => ({
+      content,
+      isError: false,
+      structuredContent: { id: "read-1", success: true },
+    }));
+    const { pool } = poolWith(client);
+
+    await expect(pool.getBridge("/work/proj").toolCall("sess-1", "read", {})).rejects.toThrow(
+      "subc tool reply structuredContent lacks a boolean `success` / string `text` (protocol violation)",
+    );
+  });
+
   test("a reply missing the structuredContent envelope throws (protocol violation)", async () => {
     // No structuredContent → must NOT be coerced to a silent {success:false}.
     const client = new FakeClient(async () => ({ content: [], isError: false }));

packages/aft-bridge/src/subc-transport.ts

@@ -351,13 +351,16 @@ class RouteTornDownError extends Error {}
  * `structuredContent`, so re-lifting it makes everything downstream (status_bar,
  * bg_completions, preview_diff, code, …) byte-identical to NDJSON.
  *
- * Every AFT tool reply over subc carries this envelope with a boolean `success`
- * and string `text`. A reply missing the envelope, or whose lifted shape lacks a
- * boolean `success`, is a PROTOCOL VIOLATION — never a tool result — and is thrown
- * rather than coerced. Coercing it (the old `{success:false,text:""}` /
- * raw-record fallback) could let a malformed reply with `success === undefined`
- * read downstream as a successful tool result (audit B-#7). Surfacing it loudly is
- * the honest contract: a broken wire shape is a failure, not a silent empty pass.
+ * Every AFT tool reply over subc carries this envelope with a boolean `success`.
+ * During the wire transition, `structuredContent.text` may be omitted; the bridge
+ * synthesizes it only from exactly one outer text block, while a present field wins
+ * unchanged. A reply missing the envelope, lacking boolean `success`, or missing
+ * text without an unambiguous outer block is a PROTOCOL VIOLATION — never a tool
+ * result — and is thrown rather than coerced. Coercing it (the old
+ * `{success:false,text:""}` / raw-record fallback) could let a malformed reply with
+ * `success === undefined` read downstream as a successful tool result (audit B-#7).
+ * Surfacing it loudly is the honest contract: a broken wire shape is a failure, not
+ * a silent empty pass.
  */
 function reliftReply(reply: unknown): Record<string, unknown> {
   if (!isRecord(reply) || !isRecord(reply.structuredContent)) {
@@ -366,7 +369,19 @@ function reliftReply(reply: unknown): Record<string, unknown> {
     );
   }
   const flat = reply.structuredContent;
-  if (typeof flat.success !== "boolean" || typeof flat.text !== "string") {
+  if (typeof flat.success !== "boolean") {
+    throw new Error(
+      "subc tool reply structuredContent lacks a boolean `success` / string `text` (protocol violation)",
+    );
+  }
+  if (!Object.hasOwn(flat, "text")) {
+    const content = reply.content;
+    const block = Array.isArray(content) && content.length === 1 ? content[0] : undefined;
+    if (isRecord(block) && block.type === "text" && typeof block.text === "string") {
+      return { ...flat, text: block.text };
+    }
+  }
+  if (typeof flat.text !== "string") {
     throw new Error(
       "subc tool reply structuredContent lacks a boolean `success` / string `text` (protocol violation)",
     );