fix: make multi-writer artifacts readable, and stop destroying codex error causes

src/daemon/blackboard/mcp-http.ts

@@ -102,19 +102,48 @@ const TOOLS: ToolDef[] = [
   {
     name: "blackboard_read",
     description:
-      "Read the latest version of one artifact. Denied if your role's read scope doesn't include it.",
+      "Read the latest version of one artifact. Pass `slot` for a multi-writer kind — blackboard_index shows each entry's slot. Denied if your role's read scope doesn't include the kind.",
     jsonSchema: {
       type: "object",
-      properties: { kind: { type: "string", description: KIND_DESC } },
+      properties: {
+        kind: { type: "string", description: KIND_DESC },
+        // `read` takes a slot; `write` deliberately does NOT (§4 — a writer must
+        // not be able to name a peer's slot and overwrite it). Without this,
+        // every multi-writer artifact was unreachable: the index advertised
+        // `findings` in slot "review", and a slotless read looked in the
+        // singleton slot and reported it did not exist. Observed live — an
+        // orchestrator retried six times and concluded the board was broken.
+        slot: {
+          type: "string",
+          description:
+            "Writer slot, exactly as blackboard_index reports it (e.g. \"review\" or \"review#2\"). Omit for a single-writer kind.",
+        },
+      },
       required: ["kind"],
       additionalProperties: false,
     },
     run: (args, bb) => {
-      const r = bb.read(str(args.kind));
+      const kind = str(args.kind);
+      const slot = args.slot === undefined ? null : str(args.slot);
+      const r = bb.read(kind, slot);
       if (!r.ok) throw new Error(r.error);
-      if (!r.value) return `No "${str(args.kind)}" has been written on this goal yet.`;
+      if (!r.value) {
+        // Point at the slots that DO exist rather than a bare "not written":
+        // the caller usually asked for the right kind with the wrong slot, and
+        // the index already knows the answer.
+        const slots = bb
+          .index()
+          .filter((e) => e.kind === kind)
+          .map((e) => e.slot)
+          .filter((sl): sl is string => sl !== null);
+        if (slot === null && slots.length > 0) {
+          return `"${kind}" has no single-writer entry, but ${slots.length} writer slot(s) exist: ${slots.join(", ")}. Re-read with \`slot\`, or use blackboard_read_all to get every writer's entry at once.`;
+        }
+        return `No "${kind}"${slot ? ` in slot "${slot}"` : ""} has been written on this goal yet.`;
+      }
       const a = r.value;
-      return `${a.kind} v${a.version} (by ${a.authorRole ?? a.authorSub}):\n\n${a.content}`;
+      const where = a.slot ? ` slot ${a.slot}` : "";
+      return `${a.kind}${where} v${a.version} (by ${a.authorRole ?? a.authorSub}):\n\n${a.content}`;
     },
   },
   {

src/daemon/providers/codex/index.ts

@@ -199,6 +199,40 @@ function sandboxPolicyWire(mode: SandboxMode, workdir: string): Record<string, u
   }
 }
 
+/**
+ * Best-effort human-readable text for a codex `error` notification.
+ *
+ * Deliberately tolerant of shape rather than asserting one: the payload has
+ * carried a bare `message` string in some versions and a JSON-RPC-style
+ * `{code, message, data}` object in others, and a wrong assumption here costs
+ * the entire diagnostic. Falls back to bounded JSON so an unrecognised shape
+ * still tells the operator something.
+ */
+export function describeCodexError(params: Record<string, unknown>): string {
+  const direct = params.message;
+  if (typeof direct === "string" && direct.length > 0) return direct;
+
+  const err = params.error;
+  if (typeof err === "string" && err.length > 0) return err;
+  if (err && typeof err === "object") {
+    const o = err as { message?: unknown; code?: unknown; data?: unknown };
+    const msg = typeof o.message === "string" ? o.message : undefined;
+    const code = o.code !== undefined ? ` (code ${String(o.code)})` : "";
+    if (msg) return `${msg}${code}`;
+    try {
+      return JSON.stringify(err).slice(0, 500);
+    } catch {
+      return "unserializable error payload";
+    }
+  }
+  try {
+    const dump = JSON.stringify(params);
+    return dump && dump !== "{}" ? dump.slice(0, 500) : "unspecified codex error";
+  } catch {
+    return "unspecified codex error";
+  }
+}
+
 export class CodexProvider implements SessionProvider {
   readonly id = "codex";
   readonly displayName = "Codex (OpenAI)";
@@ -652,8 +686,12 @@ export class CodexProvider implements SessionProvider {
         break;
       }
       case "error": {
-        const message = (params.message ?? params.error ?? "codex error") as string;
-        this.#push({ type: "error", message: String(message) });
+        // `params.error` is an OBJECT on this wire ({code, message, data}), so
+        // the previous `as string` + String() turned every codex failure into
+        // the literal "[object Object]" — the cause destroyed at the one place
+        // it was supposed to be reported. Observed live: a role-child failed
+        // twice with `Error: [object Object]` and nothing anywhere said why.
+        this.#push({ type: "error", message: `codex: ${describeCodexError(params)}` });
         break;
       }
       default:

src/tests/blackboard-mcp.test.ts

@@ -170,19 +170,69 @@ describe("the token carries the role's scope", () => {
 
   // No tool takes a goal id, so a child cannot address another goal even by
   // guessing one — the mount is the boundary.
-  test("no tool accepts a goal id, a session id, or a slot", async () => {
+  //
+  // This test used to forbid `slot` on EVERY tool, which was stricter than the
+  // design and made the feature wrong: §4 forbids a slot on `write` (so a writer
+  // cannot name a peer's slot and overwrite it), but a READER must be able to
+  // address the slot the index just showed it. With reads slotless, every
+  // multi-writer artifact was unreachable — observed live, an orchestrator read
+  // `findings` six times, was told it did not exist, and gave up on the board.
+  // The invariant is now stated per-tool instead of as one blanket rule.
+  test("no tool accepts a goal id or a session id", async () => {
     const token = mcp.mint(bb.forRole(GOAL, ident("review")));
     const list = await rpc(token, "tools/list");
     // Assert on PARAMETER names, not the serialized blob — the descriptions
     // legitimately mention "this goal" in prose.
-    const params = (list.body.result.tools as Array<{ inputSchema: { properties: object } }>)
-      .flatMap((t) => Object.keys(t.inputSchema.properties ?? {}))
-      .sort();
-    // The full parameter vocabulary of the surface is exactly two names.
-    expect([...new Set(params)]).toEqual(["content", "kind"]);
-    for (const forbidden of ["goal", "goalSessionId", "sessionId", "slot", "accountId"]) {
+    const tools = list.body.result.tools as Array<{
+      name: string;
+      inputSchema: { properties?: object };
+    }>;
+    const params = tools.flatMap((t) => Object.keys(t.inputSchema.properties ?? {})).sort();
+    // Scope is carried by the TOKEN, never named by the caller.
+    for (const forbidden of ["goal", "goalSessionId", "sessionId", "accountId", "projectId"]) {
       expect(params).not.toContain(forbidden);
     }
+    // The whole parameter vocabulary, so a new knob cannot appear unnoticed.
+    expect([...new Set(params)].sort()).toEqual(["content", "kind", "slot"]);
+  });
+
+  test("`slot` is readable but never writable — a writer cannot name a peer's slot", async () => {
+    const token = mcp.mint(bb.forRole(GOAL, ident("review")));
+    const list = await rpc(token, "tools/list");
+    const byName = new Map(
+      (list.body.result.tools as Array<{ name: string; inputSchema: { properties?: object } }>).map(
+        (t) => [t.name, Object.keys(t.inputSchema.properties ?? {})],
+      ),
+    );
+    // The asymmetry IS the security model (§4): the service picks a writer's
+    // slot from its own identity, so exposing one on write would hand a
+    // reviewer the ability to overwrite another reviewer's findings.
+    expect(byName.get("blackboard_write")).not.toContain("slot");
+    expect(byName.get("blackboard_read")).toContain("slot");
+  });
+
+  test("a reader can fetch a multi-writer artifact by the slot the index reports", async () => {
+    // The end-to-end shape of the live failure: two reviewers write `findings`,
+    // the index reports their slots, and a read must be able to use them.
+    bb.forRole(GOAL, ident("review", 1)).write("findings", "FIRST OPINION");
+    bb.forRole(GOAL, ident("review", 2)).write("findings", "SECOND OPINION");
+    const token = mcp.mint(bb.forRole(GOAL, ident("orchestrator")));
+
+    const idx = await call(token, "blackboard_index", {});
+    expect(textOf(idx.body)).toContain("review#2");
+
+    const first = await call(token, "blackboard_read", { kind: "findings", slot: "review" });
+    expect(textOf(first.body)).toContain("FIRST OPINION");
+    const second = await call(token, "blackboard_read", { kind: "findings", slot: "review#2" });
+    expect(textOf(second.body)).toContain("SECOND OPINION");
+
+    // And a slotless read now POINTS AT the slots instead of claiming the
+    // artifact does not exist — the message that misled a live orchestrator.
+    const slotless = await call(token, "blackboard_read", { kind: "findings" });
+    const text = textOf(slotless.body);
+    expect(text).toMatch(/writer slot\(s\) exist/);
+    expect(text).toContain("review#2");
+    expect(text).not.toMatch(/has been written on this goal yet/);
   });
 
   // Two goals, two tokens: neither can see the other's artifacts.

src/tests/provider-codex.test.ts

@@ -27,7 +27,7 @@ import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:f
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { CodexRpcProcess } from "../daemon/providers/codex/rpc.js";
-import { CodexProvider } from "../daemon/providers/codex/index.js";
+import { CodexProvider, describeCodexError } from "../daemon/providers/codex/index.js";
 import { MemoryMcpHttp, MEMORY_MCP_SERVER_NAME } from "../daemon/memory/mcp-http.js";
 import { McpRegistry } from "../daemon/mcp/registry.js";
 import type { RawMcpServerConfig } from "../config.js";
@@ -764,3 +764,52 @@ describe("codex resolution + registry", () => {
     expect(compareNodeVersionsDesc("v18.1.0", "v18.1.0")).toBe(0);
   });
 });
+
+// ── error reporting ─────────────────────────────────────────────────────────
+
+// This exists because a live role-child failed twice with the literal
+// `Error: [object Object]` and nothing anywhere said why. The `error`
+// notification carries an OBJECT on this wire, and the handler cast it
+// `as string` then String()'d it — destroying the cause at the one place whose
+// entire job was to report it. A backend whose failures are unreadable is a
+// backend nobody can debug.
+describe("describeCodexError", () => {
+  it("extracts a JSON-RPC-style error object rather than stringifying it", () => {
+    const out = describeCodexError({
+      error: { code: -32603, message: "model gpt-5.6-sol is not available to this account" },
+    });
+    expect(out).toContain("not available to this account");
+    expect(out).toContain("-32603");
+    expect(out).not.toContain("[object Object]");
+  });
+
+  it("prefers a direct message when present", () => {
+    expect(describeCodexError({ message: "stream closed" })).toBe("stream closed");
+  });
+
+  it("accepts a bare string error", () => {
+    expect(describeCodexError({ error: "rate limited" })).toBe("rate limited");
+  });
+
+  it("falls back to bounded JSON for an unrecognised shape", () => {
+    // Tolerating shape beats asserting it: a wrong assumption here costs the
+    // whole diagnostic, which is exactly what happened.
+    const out = describeCodexError({ error: { unexpected: { nested: true } } });
+    expect(out).toContain("unexpected");
+    expect(out).not.toContain("[object Object]");
+  });
+
+  it("never returns [object Object], whatever it is handed", () => {
+    for (const params of [
+      {},
+      { error: {} },
+      { error: null },
+      { message: 42 },
+      { error: { message: null, code: 7 } },
+    ] as Array<Record<string, unknown>>) {
+      const out = describeCodexError(params);
+      expect(out).not.toContain("[object Object]");
+      expect(out.length).toBeGreaterThan(0);
+    }
+  });
+});