From 08380532e29c3e75ef623b54a0291c1d229b9132 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Mon, 27 Jul 2026 09:12:32 +0800 Subject: [PATCH 1/5] =?UTF-8?q?feat:=20goal=20blackboard=20=E2=80=94=20typ?= =?UTF-8?q?ed=20artifact=20store=20+=20role=20scoping=20(blackboard=201/3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First slice of the blackboard phase (docs/collaborative-session-design.md §4). Storage and the access model; the MCP tool surface and mid-goal resume follow in slices 2 and 3. This is the piece that lets a searcher hand work to an architect without the orchestrator re-serializing it as prose — §4's whole argument for why codeoid can beat a message-relay harness. - blackboard/types.ts: six core kinds (spec/research/adr/task-list/diff/ findings) + a scoped `extra/` hatch. Fixed core because access scoping is meaningless over a free-form key space — you cannot grant "read the spec" if `spec` isn't a real name — and an escape hatch because a closed enum would make every new pack a code change. Unknown kinds REJECT rather than normalize: a typo'd kind quietly becoming a new extra/ slot would look like a successful handoff while the intended reader waits forever. - blackboard/store.ts: versioned, identity-stamped, append-only. Every query scoped on account_id AND project_id — a goal id is not a permission, and without the tenant a leaked or colliding session id would expose another account's artifacts. append() runs in an IMMEDIATE transaction because children on different backends genuinely do write concurrently, and a UNIQUE (goal, kind, slot, version) makes append-only a storage guarantee rather than a convention. - blackboard/service.ts: role-scoped handles, fail-closed both directions, with the §3 default profile. Enforcement lives HERE, not in the tool wrappers: scoping in the MCP layer would leave every other caller — a future frontend, the pipeline engine, a test helper — silently unscoped. With the service owning it, slice 2 cannot expose a bypass because there isn't one. - protocol: reads/writes on CollaborationRole + validation. "Declared empty" is kept distinct from "declared nothing" (the latter falls back to the default profile); collapsing them would silently strip every default. Two properties the tests exist for: Reviewer independence (§6) is a consequence of the read set, not a request. `review` reads exactly spec+diff — not `research` (implementer reasoning by proxy) and not `findings`, not even a peer's. A panel whose members can read each other is an echo, not a panel. Slots stop a panel collapsing. §4 has EACH reviewer write a findings entry; without per-writer slots reviewer #2's write becomes version 2 of the same artifact and a reader taking "latest" sees one opinion, with no error anywhere. The write API takes no slot parameter at all — a caller-chosen slot would let one reviewer overwrite another's findings. Tests: +33. Mutation-checked — dropping account_id from the lookup fails 2 (tenant isolation), making reviewers share a slot fails 2 (panel collapse). Stacked on fix/rate-limiter-record-destruction (#259) so this diff stays pure blackboard; main does not currently typecheck without it. Suite 2021 pass / 0 fail, typecheck + biome clean. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Yash Datta --- packages/protocol/src/schemas.ts | 7 + packages/protocol/src/types.ts | 24 +++ src/daemon/blackboard/service.ts | 220 +++++++++++++++++++++ src/daemon/blackboard/store.ts | 292 ++++++++++++++++++++++++++++ src/daemon/blackboard/types.ts | 99 ++++++++++ src/daemon/collaboration.ts | 27 +++ src/tests/blackboard.test.ts | 318 +++++++++++++++++++++++++++++++ 7 files changed, 987 insertions(+) create mode 100644 src/daemon/blackboard/service.ts create mode 100644 src/daemon/blackboard/store.ts create mode 100644 src/daemon/blackboard/types.ts create mode 100644 src/tests/blackboard.test.ts diff --git a/packages/protocol/src/schemas.ts b/packages/protocol/src/schemas.ts index cb8c6c3..84132a4 100644 --- a/packages/protocol/src/schemas.ts +++ b/packages/protocol/src/schemas.ts @@ -46,6 +46,13 @@ export const collaborationRoleSchema = z.object({ purpose: z.string().max(500).optional(), /** Opt-in write authority. Absent = read-only (least privilege). */ write: z.boolean().optional(), + /** + * Blackboard artifact scoping. Bounded string arrays here; whether each name + * is a real kind is checked by the daemon, so an unknown kind gets a specific + * error naming the valid set instead of an opaque schema rejection. + */ + reads: z.array(z.string().min(1).max(64)).max(16).optional(), + writes: z.array(z.string().min(1).max(64)).max(16).optional(), }); export const collaborationConfigSchema = z.object({ diff --git a/packages/protocol/src/types.ts b/packages/protocol/src/types.ts index cd97fb8..1169813 100644 --- a/packages/protocol/src/types.ts +++ b/packages/protocol/src/types.ts @@ -883,6 +883,30 @@ export interface CollaborationRole { * `false` → "scout". */ write?: boolean; + /** + * Blackboard artifact kinds this role may READ — `spec`, `research`, `adr`, + * `task-list`, `diff`, `findings`, or `extra/` + * (docs/collaborative-session-design.md §4). + * + * Absent = the default profile for this role name from §3. A role name with + * no profile and no declaration reads NOTHING — fail-closed in both + * directions, because the design's standing rule is that an unenforced field + * is false security. + * + * This is what makes reviewer independence structural rather than polite: + * `review` reads `diff`+`spec` and NOT `research` (the implementer's + * reasoning by proxy) or `findings` (its peers' opinions). + */ + reads?: string[]; + /** + * Blackboard artifact kinds this role may WRITE. Absent = the §3 default + * profile for this role name; an unprofiled role that declares nothing + * writes nothing. + * + * A role writing a multi-writer kind (`findings`) writes into its OWN slot, + * chosen by the daemon — so one reviewer can never overwrite another's. + */ + writes?: string[]; } /** The role name that must be present exactly once in a collaboration, and diff --git a/src/daemon/blackboard/service.ts b/src/daemon/blackboard/service.ts new file mode 100644 index 0000000..44a2c10 --- /dev/null +++ b/src/daemon/blackboard/service.ts @@ -0,0 +1,220 @@ +/** + * Goal blackboard — the role-scoped access layer + * (docs/collaborative-session-design.md §4, §6). + * + * Every artifact read and write in a collaboration goes through a handle + * obtained from `Blackboard.forRole()`. The handle is the gate: it knows which + * kinds its role may read and write, and it refuses anything else. + * + * Enforcement lives HERE rather than in the tool layer on purpose. The design's + * standing rule is that "an unenforced field is false security", and the + * reserved `reads`/`writes` comment in pipeline/interface.ts repeats it. If + * scoping lived only in the MCP tool wrappers, then any second caller — a + * future frontend, the pipeline engine, a test helper — would silently get + * unscoped access. With the service owning it, the tool surface cannot expose a + * path that bypasses scoping because there isn't one to expose. + * + * The property this protects (§6): a reviewer may read `diff`+`spec` and write + * its own `findings`. It may NOT read `research` (the implementer's reasoning + * by proxy) and may NOT read `findings` — not even another reviewer's. A panel + * whose members can read each other is not a panel; it's an echo. That is why + * `review`'s default read set is exactly two kinds. + */ + +import type { BlackboardStore, GoalScope } from "./store.js"; +import type { Artifact, ArtifactIndexEntry } from "./types.js"; +import { ARTIFACT_CONTENT_MAX, isValidArtifactKind } from "./types.js"; + +/** What a role may read and write. Both default to EMPTY — fail closed. */ +export interface RoleIo { + reads: readonly string[]; + writes: readonly string[]; +} + +/** + * The default role→artifact profile from §3's table. A *default*, not a + * closed list: §3 is explicit that the five named roles are a starting profile + * and that adding a role must stay a config change. A role absent from this + * table and declaring nothing gets nothing — the fail-closed direction. + * + * Note what `review` deliberately lacks: `research` (the implementer's + * reasoning by proxy) and `findings` (its peers' opinions). Independence is a + * consequence of the read set, not of asking nicely. + */ +export const DEFAULT_ROLE_IO: Readonly> = { + orchestrator: { reads: ["spec", "findings"], writes: ["spec", "task-list"] }, + search: { reads: ["spec"], writes: ["research"] }, + architecture: { reads: ["spec", "research"], writes: ["adr", "task-list"] }, + reasoning: { reads: ["spec", "adr", "task-list"], writes: ["diff"] }, + review: { reads: ["spec", "diff"], writes: ["findings"] }, +}; + +/** + * Kinds where each writer gets its own slot. + * + * §4 has *each* reviewer write a `findings` entry. Without per-writer slots, + * reviewer #2's write would become version 2 of the same artifact and a reader + * taking "latest" would see one opinion — a panel silently collapsed to a + * single voice, with no error anywhere. Everything else is a singleton with + * version history. + */ +export const MULTI_WRITER_KINDS: ReadonlySet = new Set(["findings"]); + +export type BlackboardDenial = { ok: false; error: string }; +export type BlackboardResult = { ok: true; value: T } | BlackboardDenial; + +/** Identity of the agent behind a handle — for attribution on every write. */ +export interface RoleIdentity { + /** Role name (already lowercased by validateCollaboration). */ + roleName: string; + /** 1-based fan-out index; 1 for a singleton role. */ + ordinal: number; + /** ZeroID subject of the agent. */ + authorSub: string; +} + +/** + * A role's view of one goal's blackboard. Obtained from `Blackboard.forRole`; + * cannot widen its own scope. + */ +export class RoleBlackboard { + #store: BlackboardStore; + #scope: GoalScope; + #identity: RoleIdentity; + #io: RoleIo; + + constructor(store: BlackboardStore, scope: GoalScope, identity: RoleIdentity, io: RoleIo) { + this.#store = store; + this.#scope = scope; + this.#identity = identity; + this.#io = io; + } + + get reads(): readonly string[] { + return this.#io.reads; + } + get writes(): readonly string[] { + return this.#io.writes; + } + + /** This role's own slot for a multi-writer kind — never another role's. */ + #ownSlot(kind: string): string | null { + if (!MULTI_WRITER_KINDS.has(kind)) return null; + return this.#identity.ordinal > 1 + ? `${this.#identity.roleName}#${this.#identity.ordinal}` + : this.#identity.roleName; + } + + #denyRead(kind: string): BlackboardDenial | null { + if (!isValidArtifactKind(kind)) { + return { ok: false, error: `Unknown artifact kind "${kind}"` }; + } + if (!this.#io.reads.includes(kind)) { + return { + ok: false, + error: `Role "${this.#identity.roleName}" may not read "${kind}" — it reads: ${this.#io.reads.join(", ") || "(nothing)"}`, + }; + } + return null; + } + + /** Latest version of a readable artifact. `null` value = not written yet. */ + read(kind: string, slot?: string | null): BlackboardResult { + const denied = this.#denyRead(kind); + if (denied) return denied; + return { ok: true, value: this.#store.latest(this.#scope, kind, slot ?? null) }; + } + + /** + * Every slot of a readable multi-writer kind — how the orchestrator collects + * all N reviewers' findings for synthesis. + */ + readAll(kind: string): BlackboardResult { + const denied = this.#denyRead(kind); + if (denied) return denied; + return { ok: true, value: this.#store.latestAllSlots(this.#scope, kind) }; + } + + /** + * Append a new version of a writable artifact. + * + * The slot is chosen by the SERVICE, never by the caller: a reviewer writes + * into its own slot and has no way to name someone else's. Letting a caller + * pass a slot would hand one reviewer the ability to overwrite another's + * findings, which is the whole thing slots exist to prevent. + */ + write(kind: string, content: string): BlackboardResult { + if (!isValidArtifactKind(kind)) { + return { ok: false, error: `Unknown artifact kind "${kind}"` }; + } + if (!this.#io.writes.includes(kind)) { + return { + ok: false, + error: `Role "${this.#identity.roleName}" may not write "${kind}" — it writes: ${this.#io.writes.join(", ") || "(nothing)"}`, + }; + } + if (content.length > ARTIFACT_CONTENT_MAX) { + return { + ok: false, + error: `Artifact "${kind}" is ${content.length} bytes — max ${ARTIFACT_CONTENT_MAX}. Put large output in the workspace and reference it.`, + }; + } + return { + ok: true, + value: this.#store.append({ + scope: this.#scope, + kind, + slot: this.#ownSlot(kind), + content, + authorSub: this.#identity.authorSub, + authorRole: this.#identity.roleName, + now: Date.now(), + }), + }; + } + + /** + * The index. Deliberately NOT scoped by `reads`: knowing that a `diff` exists + * at v3 is not the same as reading it, the orchestrator needs the whole + * picture to schedule (§4), and bodies never appear here. Reading still + * requires the read scope. + */ + index(): ArtifactIndexEntry[] { + return this.#store.index(this.#scope); + } +} + +/** The daemon-owned blackboard: one store, many goal-and-role-scoped views. */ +export class Blackboard { + #store: BlackboardStore; + + constructor(store: BlackboardStore) { + this.#store = store; + } + + /** + * A role's handle on one goal. + * + * `declared` comes from the role's own `reads`/`writes`. When it declares + * neither, the §3 default profile for that role name applies; when the name + * isn't in the profile either, the handle can do nothing — a new role must + * say what it touches before it touches anything. + */ + forRole( + scope: GoalScope, + identity: RoleIdentity, + declared?: { reads?: readonly string[]; writes?: readonly string[] }, + ): RoleBlackboard { + const fallback = DEFAULT_ROLE_IO[identity.roleName]; + const io: RoleIo = { + reads: declared?.reads ?? fallback?.reads ?? [], + writes: declared?.writes ?? fallback?.writes ?? [], + }; + return new RoleBlackboard(this.#store, scope, identity, io); + } + + /** Drop a goal's artifacts. Called on collaboration teardown. */ + deleteGoal(scope: GoalScope): number { + return this.#store.deleteGoal(scope); + } +} diff --git a/src/daemon/blackboard/store.ts b/src/daemon/blackboard/store.ts new file mode 100644 index 0000000..b5bb0c6 --- /dev/null +++ b/src/daemon/blackboard/store.ts @@ -0,0 +1,292 @@ +/** + * Durable storage for goal-blackboard artifacts + * (docs/collaborative-session-design.md §4). + * + * Shares the daemon's SQLite connection rather than opening a second handle — + * same contract as the pipeline store (`Store.database`). + * + * Two invariants this layer owns: + * + * 1. **Every query is tenant-scoped** on `account_id` AND `project_id`. A + * goal id alone is not a permission: reading by goal without the tenant + * would let one account's agent read another's artifacts if a session id + * ever leaked or collided. + * 2. **Writes never overwrite.** A write appends version N+1, so the history + * of a handoff is intact and a reviewer's findings can't be silently + * replaced by a later run. Readers ask for "latest" explicitly. + */ + +import type { Database } from "bun:sqlite"; +import { randomUUID } from "node:crypto"; +import type { Artifact, ArtifactIndexEntry } from "./types.js"; + +interface RawArtifactRow { + id: string; + goal_session_id: string; + kind: string; + slot: string | null; + version: number; + content: string; + author_sub: string; + author_role: string | null; + created_at: number; +} + +function toArtifact(r: RawArtifactRow): Artifact { + return { + id: r.id, + goalSessionId: r.goal_session_id, + kind: r.kind, + slot: r.slot, + version: r.version, + content: r.content, + authorSub: r.author_sub, + authorRole: r.author_role, + createdAt: r.created_at, + }; +} + +/** Tenant + goal scope carried on every call — never derived from a client. */ +export interface GoalScope { + accountId: string; + projectId: string; + goalSessionId: string; +} + +export class BlackboardStore { + #db: Database; + + constructor(db: Database) { + this.#db = db; + this.#migrate(); + } + + #migrate(): void { + this.#db.exec(` + CREATE TABLE IF NOT EXISTS collaboration_artifacts ( + id TEXT PRIMARY KEY, + account_id TEXT NOT NULL, + project_id TEXT NOT NULL, + -- Goal scope = the orchestrating parent session's id. Artifacts die + -- with the goal, so this cascades on session delete. + goal_session_id TEXT NOT NULL, + kind TEXT NOT NULL, -- core kind | 'extra/' + slot TEXT, -- multi-writer discriminator; NULL = singleton + version INTEGER NOT NULL, -- 1-based, monotonic per (goal, kind, slot) + content TEXT NOT NULL, + author_sub TEXT NOT NULL, -- producing ZeroID subject + author_role TEXT, -- collaboration role name + created_at INTEGER NOT NULL, + -- Makes the append-only contract a storage guarantee, not a convention: + -- a racing double-write on the same version fails loudly instead of one + -- silently winning. + UNIQUE (goal_session_id, kind, slot, version) + ); + -- The read path is always (tenant, goal) then kind/slot, newest first. + CREATE INDEX IF NOT EXISTS idx_artifacts_goal + ON collaboration_artifacts(account_id, project_id, goal_session_id, kind, slot, version DESC); + `); + } + + /** + * Append the next version of one artifact and return it. + * + * The read-max-then-insert pair runs inside an IMMEDIATE transaction so two + * concurrent writers can't compute the same next version — one of them would + * otherwise lose its write to the UNIQUE constraint. Children on different + * backends genuinely do write concurrently, so this is a real race, not a + * theoretical one. + */ + append(input: { + scope: GoalScope; + kind: string; + slot?: string | null; + content: string; + authorSub: string; + authorRole?: string | null; + now: number; + }): Artifact { + const slot = input.slot ?? null; + const run = this.#db.transaction((): Artifact => { + const row = this.#db + .prepare( + `SELECT COALESCE(MAX(version), 0) AS v + FROM collaboration_artifacts + WHERE account_id = ? AND project_id = ? AND goal_session_id = ? + AND kind = ? AND slot IS ?`, + ) + .get( + input.scope.accountId, + input.scope.projectId, + input.scope.goalSessionId, + input.kind, + slot, + ) as { v: number }; + const version = row.v + 1; + const id = randomUUID(); + this.#db + .prepare( + `INSERT INTO collaboration_artifacts + (id, account_id, project_id, goal_session_id, kind, slot, version, + content, author_sub, author_role, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + ) + .run( + id, + input.scope.accountId, + input.scope.projectId, + input.scope.goalSessionId, + input.kind, + slot, + version, + input.content, + input.authorSub, + input.authorRole ?? null, + input.now, + ); + return { + id, + goalSessionId: input.scope.goalSessionId, + kind: input.kind, + slot, + version, + content: input.content, + authorSub: input.authorSub, + authorRole: input.authorRole ?? null, + createdAt: input.now, + }; + }); + // IMMEDIATE: take the write lock up front rather than upgrading mid-txn, + // which under WAL is what produces SQLITE_BUSY between two writers. + return run.immediate(); + } + + /** Latest version of one artifact, or null. */ + latest(scope: GoalScope, kind: string, slot?: string | null): Artifact | null { + const row = this.#db + .prepare( + `SELECT * FROM collaboration_artifacts + WHERE account_id = ? AND project_id = ? AND goal_session_id = ? + AND kind = ? AND slot IS ? + ORDER BY version DESC LIMIT 1`, + ) + .get( + scope.accountId, + scope.projectId, + scope.goalSessionId, + kind, + slot ?? null, + ) as RawArtifactRow | undefined; + return row ? toArtifact(row) : null; + } + + /** One specific version — for auditing a handoff after the fact. */ + version( + scope: GoalScope, + kind: string, + version: number, + slot?: string | null, + ): Artifact | null { + const row = this.#db + .prepare( + `SELECT * FROM collaboration_artifacts + WHERE account_id = ? AND project_id = ? AND goal_session_id = ? + AND kind = ? AND slot IS ? AND version = ?`, + ) + .get( + scope.accountId, + scope.projectId, + scope.goalSessionId, + kind, + slot ?? null, + version, + ) as RawArtifactRow | undefined; + return row ? toArtifact(row) : null; + } + + /** + * Every slot of one kind at its latest version — how a synthesizing + * orchestrator collects all N reviewers' `findings` in one call. + */ + latestAllSlots(scope: GoalScope, kind: string): Artifact[] { + const rows = this.#db + .prepare( + `SELECT a.* FROM collaboration_artifacts a + JOIN ( + SELECT slot, MAX(version) AS v + FROM collaboration_artifacts + WHERE account_id = ? AND project_id = ? AND goal_session_id = ? AND kind = ? + GROUP BY slot + ) m ON m.v = a.version AND m.slot IS a.slot + WHERE a.account_id = ? AND a.project_id = ? AND a.goal_session_id = ? AND a.kind = ? + ORDER BY a.slot IS NULL DESC, a.slot ASC`, + ) + .all( + scope.accountId, + scope.projectId, + scope.goalSessionId, + kind, + scope.accountId, + scope.projectId, + scope.goalSessionId, + kind, + ) as RawArtifactRow[]; + return rows.map(toArtifact); + } + + /** + * The index: what exists at what version, no bodies. `bytes` is computed in + * SQL so a large artifact is never loaded just to report its size. + */ + index(scope: GoalScope): ArtifactIndexEntry[] { + const rows = this.#db + .prepare( + `SELECT a.kind, a.slot, a.version, a.author_sub, a.author_role, + a.created_at, LENGTH(a.content) AS bytes + FROM collaboration_artifacts a + JOIN ( + SELECT kind, slot, MAX(version) AS v + FROM collaboration_artifacts + WHERE account_id = ? AND project_id = ? AND goal_session_id = ? + GROUP BY kind, slot + ) m ON m.kind = a.kind AND m.slot IS a.slot AND m.v = a.version + WHERE a.account_id = ? AND a.project_id = ? AND a.goal_session_id = ? + ORDER BY a.kind ASC, a.slot IS NULL DESC, a.slot ASC`, + ) + .all( + scope.accountId, + scope.projectId, + scope.goalSessionId, + scope.accountId, + scope.projectId, + scope.goalSessionId, + ) as Array<{ + kind: string; + slot: string | null; + version: number; + author_sub: string; + author_role: string | null; + created_at: number; + bytes: number; + }>; + return rows.map((r) => ({ + kind: r.kind, + slot: r.slot, + version: r.version, + authorSub: r.author_sub, + authorRole: r.author_role, + updatedAt: r.created_at, + bytes: r.bytes, + })); + } + + /** Drop a goal's artifacts (goal end). Tenant-scoped like everything else. */ + deleteGoal(scope: GoalScope): number { + return this.#db + .prepare( + `DELETE FROM collaboration_artifacts + WHERE account_id = ? AND project_id = ? AND goal_session_id = ?`, + ) + .run(scope.accountId, scope.projectId, scope.goalSessionId).changes; + } +} diff --git a/src/daemon/blackboard/types.ts b/src/daemon/blackboard/types.ts new file mode 100644 index 0000000..1c22876 --- /dev/null +++ b/src/daemon/blackboard/types.ts @@ -0,0 +1,99 @@ +/** + * Goal blackboard — the typed-artifact vocabulary + * (docs/collaborative-session-design.md §4). + * + * The blackboard is how role-children hand work to each other WITHOUT the + * orchestrator re-serializing it as prose. A searcher writes `research`; an + * architect reads `research`+`spec` and writes `adr`; each reviewer writes its + * own `findings` entry. The orchestrator holds an index of what exists at what + * version — never the artifact bodies. + * + * Why a fixed core plus a scoped escape hatch, settled in the 2026-07-25 + * grill: a wholly free-form key space makes access scoping meaningless (you + * cannot grant "read the spec" if `spec` isn't a real name), while a closed + * enum makes a new pack a code change. So: six core kinds that scoping and + * tooling can rely on, and `extra/` for anything else. + */ + +/** + * The fixed core artifact kinds. Order is the natural SDLC flow, which is also + * how an index renders. + */ +export const CORE_ARTIFACT_KINDS = [ + "spec", + "research", + "adr", + "task-list", + "diff", + "findings", +] as const; + +export type CoreArtifactKind = (typeof CORE_ARTIFACT_KINDS)[number]; + +/** `extra/` — the scoped escape hatch. Lowercase, bounded, no nesting. */ +const EXTRA_PREFIX = "extra/"; +const EXTRA_KEY_RE = /^[a-z0-9][a-z0-9-]{0,39}$/; + +/** Max stored body per artifact version. A handoff is a document, not a blob; + * anything larger belongs in the workspace with the artifact pointing at it. */ +export const ARTIFACT_CONTENT_MAX = 256 * 1024; + +/** Max `extra/` length including the prefix, for column sizing sanity. */ +export const ARTIFACT_KIND_MAX = 64; + +export function isCoreArtifactKind(kind: string): kind is CoreArtifactKind { + return (CORE_ARTIFACT_KINDS as readonly string[]).includes(kind); +} + +/** + * Validate an artifact kind — a core name, or a well-formed `extra/`. + * + * Rejects rather than normalizing: a typo'd kind that silently became a new + * `extra/` slot would look like a successful handoff while the intended reader + * waits forever on an artifact nobody wrote. + */ +export function isValidArtifactKind(kind: string): boolean { + if (isCoreArtifactKind(kind)) return true; + if (!kind.startsWith(EXTRA_PREFIX)) return false; + if (kind.length > ARTIFACT_KIND_MAX) return false; + return EXTRA_KEY_RE.test(kind.slice(EXTRA_PREFIX.length)); +} + +/** One stored version of one artifact. */ +export interface Artifact { + id: string; + /** Goal scope: the orchestrating (parent) session's id. */ + goalSessionId: string; + /** A core kind or `extra/`. */ + kind: string; + /** + * Discriminator within a kind, for the genuinely multi-writer case: §4 has + * *each* reviewer write a `findings` entry, and without a slot reviewer #2 + * would overwrite reviewer #1 — silently collapsing a panel to one opinion. + * NULL/absent = the singleton slot. + */ + slot: string | null; + /** 1-based, monotonic per (goal, kind, slot). Writes never overwrite. */ + version: number; + content: string; + /** ZeroID subject of the producing agent — every contribution attributable. */ + authorSub: string; + /** Collaboration role that produced it, when written by a role-child. */ + authorRole: string | null; + createdAt: number; +} + +/** An index row: what exists, at what version, by whom — no bodies. This is + * all the orchestrator ever needs (§4: "holds an index, not the artifacts"). */ +export interface ArtifactIndexEntry { + kind: string; + slot: string | null; + /** Latest version present. */ + version: number; + authorSub: string; + authorRole: string | null; + updatedAt: number; + /** Body size of the latest version, so the orchestrator can reason about + * cost before asking a child to read it. */ + bytes: number; +} diff --git a/src/daemon/collaboration.ts b/src/daemon/collaboration.ts index f7638d9..672b0eb 100644 --- a/src/daemon/collaboration.ts +++ b/src/daemon/collaboration.ts @@ -18,6 +18,7 @@ import type { CollaborationConfig, CollaborationRole } from "../protocol/types.js"; import { LIMITS, ORCHESTRATOR_ROLE } from "../protocol/types.js"; import { CLAUDE_PROVIDER_ID, resolveModelIdForProvider } from "./models.js"; +import { CORE_ARTIFACT_KINDS, isValidArtifactKind } from "./blackboard/types.js"; /** The provider-registry surface this module needs — kept narrow so tests * can pass a stub instead of building a real registry. */ @@ -86,6 +87,25 @@ export function validateCollaboration( }; } + // Blackboard scoping: an unknown kind must reject here. Left to pass, a + // typo like `reads: ["diffs"]` would produce a role that appears scoped but + // can never read the artifact it needs, and the failure would surface much + // later as an agent inexplicably waiting on a handoff. + for (const [field, kinds] of [ + ["reads", raw.reads], + ["writes", raw.writes], + ] as const) { + if (!kinds) continue; + for (const kind of kinds) { + if (!isValidArtifactKind(kind)) { + return { + ok: false, + error: `Role "${name}" ${field} unknown artifact kind "${kind}" — valid: ${CORE_ARTIFACT_KINDS.join(", ")}, or extra/`, + }; + } + } + } + const count = raw.count ?? 1; if (!Number.isInteger(count) || count < 1) { return { ok: false, error: `Role "${name}" count must be a positive integer` }; @@ -128,6 +148,13 @@ export function validateCollaboration( // Normalize to an explicit boolean so downstream code never has to // re-decide what "absent" means for write authority. write: raw.write === true, + // Left ABSENT when undeclared, deliberately — the blackboard service + // distinguishes "declared nothing" (fall back to the §3 default profile + // for this role name) from "declared an empty list" (reads/writes + // nothing). Defaulting to [] here would erase that distinction and + // silently strip every default profile. + ...(raw.reads !== undefined ? { reads: [...raw.reads] } : {}), + ...(raw.writes !== undefined ? { writes: [...raw.writes] } : {}), }); } diff --git a/src/tests/blackboard.test.ts b/src/tests/blackboard.test.ts new file mode 100644 index 0000000..f95fcf2 --- /dev/null +++ b/src/tests/blackboard.test.ts @@ -0,0 +1,318 @@ +/** + * Goal blackboard — store + role-scoped access + * (docs/collaborative-session-design.md §4, §6). + * + * The properties under test, in priority order: + * 1. TENANT ISOLATION — a goal id is not a permission. Two tenants can hold + * the same goal session id and must not see each other's artifacts. + * 2. INDEPENDENCE — a reviewer reads `diff`+`spec` and cannot reach + * `research` or `findings`, not even a peer reviewer's. A panel whose + * members can read each other is an echo, not a panel. + * 3. NO SILENT COLLAPSE — each reviewer writes its own slot, so reviewer #2 + * cannot overwrite reviewer #1 and quietly reduce a panel to one voice. + * 4. APPEND-ONLY — writes version, never overwrite, so a handoff's history + * survives. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { Blackboard, DEFAULT_ROLE_IO, type RoleIdentity } from "../daemon/blackboard/service.js"; +import { BlackboardStore, type GoalScope } from "../daemon/blackboard/store.js"; +import { + ARTIFACT_CONTENT_MAX, + CORE_ARTIFACT_KINDS, + isValidArtifactKind, +} from "../daemon/blackboard/types.js"; +import { Store } from "../daemon/store.js"; + +let tmp: string; +let store: Store; +let bbStore: BlackboardStore; +let bb: Blackboard; + +const GOAL: GoalScope = { accountId: "acc", projectId: "proj", goalSessionId: "goal-1" }; +/** Same goal id, different tenant — the isolation probe. */ +const OTHER_TENANT: GoalScope = { ...GOAL, accountId: "acc-2" }; +/** Same tenant + account, different project — projects are a boundary too. */ +const OTHER_PROJECT: GoalScope = { ...GOAL, projectId: "proj-2" }; + +const ident = (roleName: string, ordinal = 1): RoleIdentity => ({ + roleName, + ordinal, + authorSub: `agent:${roleName}${ordinal}`, +}); + +beforeEach(() => { + tmp = mkdtempSync(join(tmpdir(), "codeoid-bb-")); + store = new Store(join(tmp, "codeoid.db")); + bbStore = new BlackboardStore(store.database); + bb = new Blackboard(bbStore); +}); + +afterEach(() => { + rmSync(tmp, { recursive: true, force: true }); +}); + +// ── Kind vocabulary ───────────────────────────────────────────────────────── + +describe("artifact kinds", () => { + test("accepts every core kind", () => { + for (const k of CORE_ARTIFACT_KINDS) expect(isValidArtifactKind(k)).toBe(true); + }); + + test("accepts a well-formed extra/", () => { + expect(isValidArtifactKind("extra/bench-results")).toBe(true); + expect(isValidArtifactKind("extra/x")).toBe(true); + }); + + // Rejecting matters: a typo'd kind that became a fresh extra/ slot would look + // like a successful handoff while the intended reader waits forever. + test.each([ + "diffs", + "Spec", + "extra/", + "extra/UPPER", + "extra/has space", + "extra/nested/key", + "extra/-leading", + "", + "findings ", + ])("rejects %p", (kind) => { + expect(isValidArtifactKind(kind)).toBe(false); + }); +}); + +// ── Store: versioning + tenant isolation ──────────────────────────────────── + +describe("BlackboardStore", () => { + test("appends versions instead of overwriting", () => { + const a = bbStore.append({ scope: GOAL, kind: "spec", content: "v1", authorSub: "a", now: 1 }); + const b = bbStore.append({ scope: GOAL, kind: "spec", content: "v2", authorSub: "a", now: 2 }); + expect(a.version).toBe(1); + expect(b.version).toBe(2); + expect(bbStore.latest(GOAL, "spec")?.content).toBe("v2"); + // History intact — the point of append-only. + expect(bbStore.version(GOAL, "spec", 1)?.content).toBe("v1"); + }); + + test("versions independently per slot", () => { + bbStore.append({ scope: GOAL, kind: "findings", slot: "review", content: "r1", authorSub: "a", now: 1 }); + bbStore.append({ scope: GOAL, kind: "findings", slot: "review#2", content: "r2", authorSub: "b", now: 2 }); + bbStore.append({ scope: GOAL, kind: "findings", slot: "review", content: "r1b", authorSub: "a", now: 3 }); + expect(bbStore.latest(GOAL, "findings", "review")?.content).toBe("r1b"); + expect(bbStore.latest(GOAL, "findings", "review#2")?.content).toBe("r2"); + const all = bbStore.latestAllSlots(GOAL, "findings"); + expect(all).toHaveLength(2); + expect(all.map((a) => a.content).sort()).toEqual(["r1b", "r2"]); + }); + + test("a null slot is distinct from a named one", () => { + bbStore.append({ scope: GOAL, kind: "spec", content: "singleton", authorSub: "a", now: 1 }); + bbStore.append({ scope: GOAL, kind: "spec", slot: "odd", content: "slotted", authorSub: "a", now: 2 }); + expect(bbStore.latest(GOAL, "spec")?.content).toBe("singleton"); + expect(bbStore.latest(GOAL, "spec", "odd")?.content).toBe("slotted"); + }); + + // A goal id is not a permission. If it were, one leaked/colliding session id + // would expose another account's artifacts. + test("isolates tenants that share a goal session id", () => { + bbStore.append({ scope: GOAL, kind: "spec", content: "ours", authorSub: "a", now: 1 }); + bbStore.append({ scope: OTHER_TENANT, kind: "spec", content: "theirs", authorSub: "z", now: 1 }); + + expect(bbStore.latest(GOAL, "spec")?.content).toBe("ours"); + expect(bbStore.latest(OTHER_TENANT, "spec")?.content).toBe("theirs"); + expect(bbStore.index(GOAL)).toHaveLength(1); + expect(bbStore.index(OTHER_TENANT)).toHaveLength(1); + // Both start at version 1 — neither tenant's write advanced the other's. + expect(bbStore.latest(GOAL, "spec")?.version).toBe(1); + expect(bbStore.latest(OTHER_TENANT, "spec")?.version).toBe(1); + }); + + test("isolates projects within one account", () => { + bbStore.append({ scope: GOAL, kind: "spec", content: "p1", authorSub: "a", now: 1 }); + expect(bbStore.latest(OTHER_PROJECT, "spec")).toBeNull(); + }); + + test("deleteGoal removes only that tenant's goal", () => { + bbStore.append({ scope: GOAL, kind: "spec", content: "ours", authorSub: "a", now: 1 }); + bbStore.append({ scope: OTHER_TENANT, kind: "spec", content: "theirs", authorSub: "z", now: 1 }); + expect(bbStore.deleteGoal(GOAL)).toBe(1); + expect(bbStore.latest(GOAL, "spec")).toBeNull(); + expect(bbStore.latest(OTHER_TENANT, "spec")?.content).toBe("theirs"); + }); + + test("the index reports version, author and size without bodies", () => { + bbStore.append({ scope: GOAL, kind: "spec", content: "hello", authorSub: "a", authorRole: "orchestrator", now: 5 }); + bbStore.append({ scope: GOAL, kind: "spec", content: "hello there", authorSub: "a", authorRole: "orchestrator", now: 6 }); + const [entry] = bbStore.index(GOAL); + expect(entry).toMatchObject({ + kind: "spec", + slot: null, + version: 2, + authorSub: "a", + authorRole: "orchestrator", + bytes: "hello there".length, + }); + // No `content` key at all — the orchestrator holds an index, not bodies. + expect(entry as unknown as Record).not.toHaveProperty("content"); + }); + + test("index lists one row per (kind, slot) at its latest version", () => { + bbStore.append({ scope: GOAL, kind: "findings", slot: "review", content: "a", authorSub: "a", now: 1 }); + bbStore.append({ scope: GOAL, kind: "findings", slot: "review#2", content: "b", authorSub: "b", now: 2 }); + bbStore.append({ scope: GOAL, kind: "diff", content: "d", authorSub: "c", now: 3 }); + const idx = bbStore.index(GOAL); + expect(idx).toHaveLength(3); + expect(idx.map((e) => `${e.kind}/${e.slot ?? "-"}`).sort()).toEqual([ + "diff/-", + "findings/review", + "findings/review#2", + ]); + }); +}); + +// ── Service: role scoping ─────────────────────────────────────────────────── + +describe("role scoping is fail-closed", () => { + test("an unprofiled role that declares nothing can do nothing", () => { + const h = bb.forRole(GOAL, ident("mystery-role")); + expect(h.reads).toEqual([]); + expect(h.writes).toEqual([]); + const r = h.read("spec"); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.error).toMatch(/may not read "spec"/); + const w = h.write("spec", "x"); + expect(w.ok).toBe(false); + if (!w.ok) expect(w.error).toMatch(/may not write "spec"/); + }); + + test("an explicit empty declaration also grants nothing", () => { + const h = bb.forRole(GOAL, ident("review"), { reads: [], writes: [] }); + // Declared-empty must NOT fall through to the default profile. + expect(h.reads).toEqual([]); + expect(h.read("diff").ok).toBe(false); + }); + + test("a declaration overrides the default profile", () => { + const h = bb.forRole(GOAL, ident("review"), { reads: ["research"], writes: ["extra/notes"] }); + expect(h.read("research").ok).toBe(true); + expect(h.read("diff").ok).toBe(false); // not declared, despite the profile + expect(h.write("extra/notes", "n").ok).toBe(true); + expect(h.write("findings", "f").ok).toBe(false); + }); + + test("an unknown kind is rejected even when scoping would allow it", () => { + const h = bb.forRole(GOAL, ident("review"), { reads: ["diffs"], writes: ["findings"] }); + const r = h.read("diffs"); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.error).toMatch(/Unknown artifact kind/); + }); + + test("oversized content is refused rather than truncated", () => { + const h = bb.forRole(GOAL, ident("search")); + const w = h.write("research", "x".repeat(ARTIFACT_CONTENT_MAX + 1)); + expect(w.ok).toBe(false); + if (!w.ok) expect(w.error).toMatch(/max \d+/); + }); +}); + +// This is the §6 guarantee, and the reason review's read set is exactly two +// kinds: a reviewer is unbiased BECAUSE it cannot see the author's reasoning or +// its peers' verdicts — not because a prompt asked it not to look. +describe("reviewer independence", () => { + test("the default review profile reads diff + spec and nothing else", () => { + expect(DEFAULT_ROLE_IO.review).toEqual({ reads: ["spec", "diff"], writes: ["findings"] }); + }); + + test("a reviewer cannot read research (implementer reasoning by proxy)", () => { + bb.forRole(GOAL, ident("search")).write("research", "how I approached it"); + const r = bb.forRole(GOAL, ident("review")).read("research"); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.error).toMatch(/may not read "research"/); + }); + + test("a reviewer cannot read findings — not even a peer's", () => { + const r1 = bb.forRole(GOAL, ident("review", 1)); + const r2 = bb.forRole(GOAL, ident("review", 2)); + expect(r1.write("findings", "looks fine").ok).toBe(true); + expect(r2.read("findings").ok).toBe(false); + expect(r2.readAll("findings").ok).toBe(false); + }); + + test("each reviewer writes its own slot, so a panel cannot collapse", () => { + expect(bb.forRole(GOAL, ident("review", 1)).write("findings", "from one").ok).toBe(true); + expect(bb.forRole(GOAL, ident("review", 2)).write("findings", "from two").ok).toBe(true); + expect(bb.forRole(GOAL, ident("review", 3)).write("findings", "from three").ok).toBe(true); + + // Three distinct opinions survive, each at version 1 of its own slot. + const all = bbStore.latestAllSlots(GOAL, "findings"); + expect(all).toHaveLength(3); + expect(all.every((a) => a.version === 1)).toBe(true); + expect(all.map((a) => a.content).sort()).toEqual(["from one", "from three", "from two"]); + expect(all.map((a) => a.slot).sort()).toEqual(["review", "review#2", "review#3"]); + }); + + // A caller-supplied slot would hand one reviewer the ability to overwrite + // another's findings, which is exactly what slots exist to prevent — so the + // write API takes no slot at all. + test("a reviewer has no way to name another reviewer's slot", () => { + const h = bb.forRole(GOAL, ident("review", 2)); + expect(h.write.length).toBe(2); // (kind, content) — no slot parameter + }); + + test("the orchestrator can read every reviewer's findings for synthesis", () => { + bb.forRole(GOAL, ident("review", 1)).write("findings", "one"); + bb.forRole(GOAL, ident("review", 2)).write("findings", "two"); + const all = bb.forRole(GOAL, ident("orchestrator")).readAll("findings"); + expect(all.ok).toBe(true); + if (all.ok) expect(all.value.map((a) => a.content).sort()).toEqual(["one", "two"]); + }); +}); + +describe("the default profile wires the §3 handoff chain", () => { + test("search → architecture → reasoning → review flows through artifacts", () => { + const search = bb.forRole(GOAL, ident("search")); + const arch = bb.forRole(GOAL, ident("architecture")); + const reason = bb.forRole(GOAL, ident("reasoning")); + const review = bb.forRole(GOAL, ident("review")); + const orch = bb.forRole(GOAL, ident("orchestrator")); + + expect(orch.write("spec", "SPEC").ok).toBe(true); + expect(search.read("spec").ok).toBe(true); + expect(search.write("research", "RESEARCH").ok).toBe(true); + + expect(arch.read("research").ok).toBe(true); + expect(arch.write("adr", "ADR").ok).toBe(true); + + expect(reason.read("adr").ok).toBe(true); + // The reasoner never reads raw research — it works from the decided ADR. + expect(reason.read("research").ok).toBe(false); + expect(reason.write("diff", "DIFF").ok).toBe(true); + + expect(review.read("diff").ok).toBe(true); + expect(review.write("findings", "FINDINGS").ok).toBe(true); + + // Nobody wrote outside their lane. + expect(search.write("diff", "x").ok).toBe(false); + expect(review.write("diff", "x").ok).toBe(false); + expect(arch.write("diff", "x").ok).toBe(false); + }); + + test("every artifact is stamped with its producing identity and role", () => { + bb.forRole(GOAL, ident("search")).write("research", "R"); + const a = bbStore.latest(GOAL, "research"); + expect(a?.authorSub).toBe("agent:search1"); + expect(a?.authorRole).toBe("search"); + }); + + // Knowing a diff EXISTS is not reading it; the orchestrator needs the whole + // picture to schedule, and the index carries no bodies. + test("the index is visible regardless of read scope", () => { + bb.forRole(GOAL, ident("search")).write("research", "R"); + const idx = bb.forRole(GOAL, ident("review")).index(); + expect(idx.map((e) => e.kind)).toContain("research"); + expect(bb.forRole(GOAL, ident("review")).read("research").ok).toBe(false); + }); +}); From 7e1230d92b97381c5bf88a9f674cf0a944b1988a Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Mon, 27 Jul 2026 09:44:36 +0800 Subject: [PATCH 2/5] feat: mountable blackboard MCP endpoint + per-child scoped mounts (blackboard 2/3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Folds slice 2 into #260. Slice 1 gave the blackboard a store and an access model; this gives it a surface any backend can mount. Deliberately an HTTP Streamable MCP server, not the in-process Claude-SDK object that backs the fleet tools. That in-process server is exactly why the orchestrator is claude-only in v1 — no other backend can mount it — and the blackboard must not inherit that limit, because the entire premise is a gemini reviewer and an openai reasoner handing work to each other (#245). - blackboard/mcp-http.ts: four tools — index / read / read_all / write. The token IS the scope: mint() binds it to one RoleBlackboard handle (one goal, one role's read/write set). Note what the schemas do NOT contain — no goal id on any tool, and no slot on write. A child cannot address another goal or a peer's findings because there is no parameter with which to try. Scope denials come back as MCP tool errors, not transport errors, so the agent sees WHY and adapts instead of retrying an opaque failure. - mcp/jsonrpc-http.ts: extracted the JSON-RPC + bearer helpers that memory's endpoint had inline. tokenFrom is the authorization boundary for every one of these mounts; two hand-maintained copies is the duplication that drifts, one gets a fix and the other quietly keeps the hole. - server.ts routes /mcp/blackboard and sets the loopback URL — loopback regardless of bind address, so the endpoint can't become reachable off-box just because the daemon binds wide. - session-manager: mints a role-scoped token per child before construction, revokes it BEFORE dropping the session at teardown (a wedged subprocess still holding the URL must not keep reading the goal), and deletes the goal's artifacts after the children are gone so a mid-write child can't recreate rows behind the delete. Attribution is keyed to role-within-goal rather than child session id: a child replaced after a restart is the same contributor. Tests: +15. Mutation testing earned its keep here — a first pass at the auth tests PASSED against an endpoint mutated to fall back to any live binding instead of requiring a matching token, because those tests ran with an empty binding map and so only proved "401 when nothing is minted". They now mint an unrelated valid token first, and all three fail against that mutation. Suite 2036 pass / 0 fail, typecheck + biome + build clean. NOT done, and the reason slice 2 is not yet end-to-end: no provider consumes the mount. It reaches ProviderInit alongside memoryMcp, but acp/codex must still surface it as an MCP server before an agent can call these tools. That is per-backend spawn-args + revoke work; splitting it out keeps this reviewable. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Yash Datta --- src/daemon/blackboard/mcp-http.ts | 276 ++++++++++++++++++++++++++++++ src/daemon/mcp/jsonrpc-http.ts | 52 ++++++ src/daemon/memory/mcp-http.ts | 45 ++--- src/daemon/providers/registry.ts | 7 + src/daemon/server.ts | 16 ++ src/daemon/session-manager.ts | 101 +++++++++++ src/daemon/session.ts | 12 ++ src/tests/blackboard-mcp.test.ts | 242 ++++++++++++++++++++++++++ 8 files changed, 717 insertions(+), 34 deletions(-) create mode 100644 src/daemon/blackboard/mcp-http.ts create mode 100644 src/daemon/mcp/jsonrpc-http.ts create mode 100644 src/tests/blackboard-mcp.test.ts diff --git a/src/daemon/blackboard/mcp-http.ts b/src/daemon/blackboard/mcp-http.ts new file mode 100644 index 0000000..cb88889 --- /dev/null +++ b/src/daemon/blackboard/mcp-http.ts @@ -0,0 +1,276 @@ +/** + * Goal blackboard — MOUNTABLE MCP endpoint + * (docs/collaborative-session-design.md §4; mountability is #245). + * + * Deliberately an HTTP Streamable MCP server rather than the in-process + * Claude-SDK object (`createSdkMcpServer`) that backs the fleet tools. The + * fleet's in-process server is exactly why the orchestrator is claude-only in + * v1: no other backend can mount it. The blackboard must not inherit that + * limitation, because the whole premise is that a *gemini* reviewer and an + * *openai* reasoner hand work to each other. Anything that can mount an MCP + * URL — claude, codex, gemini, openai, pi — gets the same surface. + * + * ## The token is the scope + * + * `mint()` binds a token to one `RoleBlackboard` handle: one goal, one role + * identity, one read/write set. Every call resolves through that handle, so a + * child physically cannot address another goal or widen its own scope — there + * is no tool parameter that would let it try. An unknown or revoked token + * fails closed with 401, same contract as the memory mount. + * + * This is why slice 1 put enforcement in the service: these tools are a + * transport over `RoleBlackboard`, not a second place where scoping decisions + * get made and could disagree. + */ + +import { randomUUID } from "node:crypto"; +import { + DEFAULT_MCP_PROTOCOL_VERSION, + ok, + rpcErr, + tokenFrom, + type JsonRpcMessage, + type JsonRpcResponse, +} from "../mcp/jsonrpc-http.js"; +import type { RoleBlackboard } from "./service.js"; +import { CORE_ARTIFACT_KINDS } from "./types.js"; + +/** Path the endpoint is mounted at on the daemon's HTTP server. */ +export const BLACKBOARD_MCP_PATH = "/mcp/blackboard"; + +/** + * The MCP server name backends mount this under. Tool calls arrive namespaced + * by it (e.g. `codeoid_blackboard__blackboard_read`), which is what lets the + * tool-safety layer recognize them. + */ +export const BLACKBOARD_MCP_SERVER_NAME = "codeoid_blackboard"; + +const SERVER_INFO = { name: BLACKBOARD_MCP_SERVER_NAME, version: "0.1.0" } as const; + +/** What a mount hands to a session so it can reach the blackboard. */ +export interface BlackboardMcpMount { + endpoint: BlackboardMcpHttp; + url: string; +} + +const KIND_DESC = `Artifact kind: ${CORE_ARTIFACT_KINDS.join(", ")}, or extra/`; + +interface ToolDef { + name: string; + description: string; + jsonSchema: Record; + run(args: Record, bb: RoleBlackboard): string; +} + +const str = (v: unknown): string => (typeof v === "string" ? v : ""); + +/** + * The four tools. Note what is absent: nothing takes a goal id, and `write` + * takes no slot. Both omissions are the security model — a child cannot name + * another goal, and a reviewer cannot name another reviewer's slot. + */ +const TOOLS: ToolDef[] = [ + { + name: "blackboard_index", + description: + "List which artifacts exist on this goal, at what version, written by whom, and how large — without their contents. Start here to see what is ready.", + jsonSchema: { type: "object", properties: {}, additionalProperties: false }, + run: (_args, bb) => { + const idx = bb.index(); + if (idx.length === 0) return "The blackboard is empty — no artifacts written yet."; + const lines = idx.map( + (e) => + `- ${e.kind}${e.slot ? ` [${e.slot}]` : ""} v${e.version} · ${e.bytes} bytes · by ${e.authorRole ?? e.authorSub}`, + ); + return [ + `${idx.length} artifact(s) on this goal:`, + ...lines, + "", + `You may read: ${bb.reads.join(", ") || "(nothing)"}`, + `You may write: ${bb.writes.join(", ") || "(nothing)"}`, + ].join("\n"); + }, + }, + { + name: "blackboard_read", + description: + "Read the latest version of one artifact. Denied if your role's read scope doesn't include it.", + jsonSchema: { + type: "object", + properties: { kind: { type: "string", description: KIND_DESC } }, + required: ["kind"], + additionalProperties: false, + }, + run: (args, bb) => { + const r = bb.read(str(args.kind)); + if (!r.ok) throw new Error(r.error); + if (!r.value) return `No "${str(args.kind)}" 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}`; + }, + }, + { + name: "blackboard_read_all", + description: + "Read every writer's latest entry for one artifact kind — e.g. all reviewers' findings. Denied if the kind is outside your read scope.", + jsonSchema: { + type: "object", + properties: { kind: { type: "string", description: KIND_DESC } }, + required: ["kind"], + additionalProperties: false, + }, + run: (args, bb) => { + const r = bb.readAll(str(args.kind)); + if (!r.ok) throw new Error(r.error); + if (r.value.length === 0) return `No "${str(args.kind)}" entries on this goal yet.`; + return r.value + .map( + (a) => + `── ${a.kind}${a.slot ? ` [${a.slot}]` : ""} v${a.version} by ${a.authorRole ?? a.authorSub} ──\n${a.content}`, + ) + .join("\n\n"); + }, + }, + { + name: "blackboard_write", + description: + "Publish your output as an artifact. Appends a new version — it never overwrites, and for multi-writer kinds you write your own entry, so you cannot clobber a peer. Denied if the kind is outside your write scope.", + jsonSchema: { + type: "object", + properties: { + kind: { type: "string", description: KIND_DESC }, + content: { type: "string", description: "The artifact body" }, + }, + required: ["kind", "content"], + additionalProperties: false, + }, + run: (args, bb) => { + const w = bb.write(str(args.kind), str(args.content)); + if (!w.ok) throw new Error(w.error); + const a = w.value; + return `Wrote ${a.kind}${a.slot ? ` [${a.slot}]` : ""} v${a.version} (${a.content.length} bytes).`; + }, + }, +]; + +export class BlackboardMcpHttp { + /** token → the role-scoped handle it authorizes. */ + readonly #bindings = new Map(); + + /** + * Mint a bearer token bound to one role's view of one goal. The token IS the + * scope — there is no wider handle reachable from it. + */ + mint(handle: RoleBlackboard): string { + const token = `bbt_${randomUUID().replace(/-/g, "")}`; + this.#bindings.set(token, handle); + return token; + } + + revoke(token: string): void { + this.#bindings.delete(token); + } + + /** Live token count — for teardown assertions + telemetry. */ + get activeTokens(): number { + return this.#bindings.size; + } + + /** Bun.serve fetch handler for {@link BLACKBOARD_MCP_PATH}. */ + async handle(req: Request): Promise { + if (req.method !== "POST") { + // No server-initiated SSE stream; some clients probe GET first. + return new Response("Method Not Allowed", { status: 405, headers: { Allow: "POST" } }); + } + + const token = tokenFrom(req); + const bb = token ? this.#bindings.get(token) : undefined; + if (!bb) { + // Fail closed — never run a tool without a resolved role scope. + return new Response(JSON.stringify({ error: "unauthorized" }), { + status: 401, + headers: { "Content-Type": "application/json", "WWW-Authenticate": "Bearer" }, + }); + } + + let body: unknown; + try { + body = await req.json(); + } catch { + return Response.json(rpcErr(null, -32700, "Parse error"), { status: 400 }); + } + + const batch = Array.isArray(body); + const messages = (batch ? body : [body]) as JsonRpcMessage[]; + const responses: JsonRpcResponse[] = []; + let sawInitialize = false; + for (const m of messages) { + if (m && m.method === "initialize") sawInitialize = true; + const res = this.#dispatch(m, bb); + if (res) responses.push(res); + } + + if (responses.length === 0) return new Response(null, { status: 202 }); + + const headers: Record = { "Content-Type": "application/json" }; + if (sawInitialize && token) headers["Mcp-Session-Id"] = token; + return new Response(JSON.stringify(batch ? responses : responses[0]), { + status: 200, + headers, + }); + } + + #dispatch(msg: JsonRpcMessage | null, bb: RoleBlackboard): JsonRpcResponse | null { + const id = msg?.id ?? null; + // JSON-RPC notifications carry no id — acknowledge with no response. + if (msg?.id === undefined) return null; + + switch (msg?.method) { + case "initialize": { + const requested = msg?.params?.protocolVersion; + return ok(id, { + protocolVersion: + typeof requested === "string" ? requested : DEFAULT_MCP_PROTOCOL_VERSION, + capabilities: { tools: {} }, + serverInfo: SERVER_INFO, + }); + } + case "ping": + return ok(id, {}); + case "tools/list": + return ok(id, { + tools: TOOLS.map((t) => ({ + name: t.name, + description: t.description, + inputSchema: t.jsonSchema, + })), + }); + case "tools/call": { + const name = msg?.params?.name; + const args = (msg?.params?.arguments ?? {}) as Record; + const def = TOOLS.find((t) => t.name === name); + if (!def) { + return ok(id, { + content: [{ type: "text", text: `Unknown tool: ${String(name)}` }], + isError: true, + }); + } + try { + return ok(id, { content: [{ type: "text", text: def.run(args, bb) }], isError: false }); + } catch (e) { + // A scope denial surfaces as an MCP tool error, not a transport + // error: the agent should see *why* it was refused and adapt, not + // get an opaque failure it might retry forever. + return ok(id, { + content: [ + { type: "text", text: `Error: ${e instanceof Error ? e.message : String(e)}` }, + ], + isError: true, + }); + } + } + default: + return rpcErr(id, -32601, `Method not found: ${String(msg?.method)}`); + } + } +} diff --git a/src/daemon/mcp/jsonrpc-http.ts b/src/daemon/mcp/jsonrpc-http.ts new file mode 100644 index 0000000..24a31b2 --- /dev/null +++ b/src/daemon/mcp/jsonrpc-http.ts @@ -0,0 +1,52 @@ +/** + * Shared JSON-RPC + bearer-auth plumbing for the daemon's in-process MCP + * endpoints (memory, goal blackboard). + * + * Extracted rather than copied: `tokenFrom` is the authorization boundary for + * every one of these mounts, and two hand-maintained copies of bearer parsing + * is precisely the kind of duplication that drifts — one gets a fix, the other + * quietly keeps the hole. + */ + +export type JsonRpcId = string | number | null; + +export interface JsonRpcMessage { + jsonrpc?: string; + id?: JsonRpcId; + method?: string; + params?: Record; +} + +export interface JsonRpcResponse { + jsonrpc: "2.0"; + id: JsonRpcId; + result?: unknown; + error?: { code: number; message: string; data?: unknown }; +} + +export function ok(id: JsonRpcId, result: unknown): JsonRpcResponse { + return { jsonrpc: "2.0", id, result }; +} + +export function rpcErr(id: JsonRpcId, code: number, message: string): JsonRpcResponse { + return { jsonrpc: "2.0", id, error: { code, message } }; +} + +/** + * Bearer token from the Authorization header, falling back to a `token` query + * param for clients that can't set headers on an MCP mount. + */ +export function tokenFrom(req: Request): string | null { + const auth = req.headers.get("authorization"); + if (auth && auth.length > 7 && auth.slice(0, 7).toLowerCase() === "bearer ") { + const t = auth.slice(7).trim(); + if (t) return t; + } + // Fallback base so a relative req.url (some test/client setups) can't throw; + // Bun.serve hands us absolute URLs, the base is only used to parse the query. + const q = new URL(req.url, "http://localhost").searchParams.get("token"); + return q && q.length > 0 ? q : null; +} + +/** Echoed only when the client doesn't propose its own protocolVersion. */ +export const DEFAULT_MCP_PROTOCOL_VERSION = "2025-06-18"; diff --git a/src/daemon/memory/mcp-http.ts b/src/daemon/memory/mcp-http.ts index b59d734..86e2b14 100644 --- a/src/daemon/memory/mcp-http.ts +++ b/src/daemon/memory/mcp-http.ts @@ -25,6 +25,14 @@ */ import { randomUUID } from "node:crypto"; +import { + DEFAULT_MCP_PROTOCOL_VERSION, + ok, + rpcErr, + tokenFrom, + type JsonRpcMessage, + type JsonRpcResponse, +} from "../mcp/jsonrpc-http.js"; import type { MemoryEngine } from "./engine.js"; import { memoryToolDefs, type MemoryToolContext, type MemoryToolDef } from "./tools.js"; @@ -58,42 +66,11 @@ export interface MemoryMcpMount { } const SERVER_INFO = { name: "codeoid-memory", version: "0.1.0" } as const; -/** Echoed only when the client doesn't propose its own protocolVersion. */ -const DEFAULT_PROTOCOL_VERSION = "2025-06-18"; - -type JsonRpcId = string | number | null; -interface JsonRpcMessage { - jsonrpc?: string; - id?: JsonRpcId; - method?: string; - params?: Record; -} -interface JsonRpcResponse { - jsonrpc: "2.0"; - id: JsonRpcId; - result?: unknown; - error?: { code: number; message: string; data?: unknown }; -} -function ok(id: JsonRpcId, result: unknown): JsonRpcResponse { - return { jsonrpc: "2.0", id, result }; -} -function rpcErr(id: JsonRpcId, code: number, message: string): JsonRpcResponse { - return { jsonrpc: "2.0", id, error: { code, message } }; -} + + /** Bearer token from the Authorization header, or a `?token=` query fallback. */ -function tokenFrom(req: Request): string | null { - const auth = req.headers.get("authorization"); - if (auth && auth.length > 7 && auth.slice(0, 7).toLowerCase() === "bearer ") { - const t = auth.slice(7).trim(); - if (t) return t; - } - // Fallback base so a relative req.url (some test/client setups) can't throw; - // Bun.serve hands us absolute URLs, the base is only used to parse the query. - const q = new URL(req.url, "http://localhost").searchParams.get("token"); - return q && q.length > 0 ? q : null; -} export class MemoryMcpHttp { readonly #engine: MemoryEngine; @@ -188,7 +165,7 @@ export class MemoryMcpHttp { case "initialize": { const requested = msg?.params?.protocolVersion; return ok(id, { - protocolVersion: typeof requested === "string" ? requested : DEFAULT_PROTOCOL_VERSION, + protocolVersion: typeof requested === "string" ? requested : DEFAULT_MCP_PROTOCOL_VERSION, capabilities: { tools: {} }, serverInfo: SERVER_INFO, }); diff --git a/src/daemon/providers/registry.ts b/src/daemon/providers/registry.ts index a6658ea..9d9772a 100644 --- a/src/daemon/providers/registry.ts +++ b/src/daemon/providers/registry.ts @@ -52,6 +52,13 @@ export interface ProviderSessionInit { /** Shared in-daemon memory MCP endpoint + URL — mounted by URL-based backends * (gemini-cli, later codex). Present only when memory is enabled. */ memoryMcp?: MemoryMcpMount; + /** + * Role-scoped goal-blackboard mount for a collaboration child (URL + a bearer + * token that IS the scope). URL-mounting backends surface it as an MCP + * server; that mountability is the whole reason it isn't an in-process + * Claude-SDK object (#245). + */ + blackboardMcp?: { url: string; token: string }; /** Cross-backend MCP registry — the servers to mount on this session's backend. */ mcpRegistry?: McpRegistry; /** Daemon-owned MCP client pool backing the registry (Model-B backends execute diff --git a/src/daemon/server.ts b/src/daemon/server.ts index 381c5a2..110b054 100644 --- a/src/daemon/server.ts +++ b/src/daemon/server.ts @@ -32,6 +32,7 @@ import { MEMORY_MCP_PATH, type MemoryEngine, } from "./memory/index.js"; +import { BLACKBOARD_MCP_PATH } from "./blackboard/mcp-http.js"; import { McpRegistry } from "./mcp/registry.js"; import { McpHub } from "./mcp/hub.js"; import { importClaudeMcpServers } from "./mcp/import-claude.js"; @@ -449,6 +450,12 @@ export class DaemonServer { daemonEnv: process.env, }); for (const w of this.#mcpRegistry.warnings) console.warn(`[codeoid] ${w}`); + // Goal blackboard: loopback URL regardless of bind address — the agent + // subprocess runs on this host, and the endpoint must not become + // reachable off-box just because the daemon binds wide. + this.#manager.setBlackboardUrl( + `http://127.0.0.1:${this.#config.port}${BLACKBOARD_MCP_PATH}`, + ); this.#manager.setMcp(this.#mcpRegistry, this.#mcpHub); const mcpCount = this.#mcpRegistry.list().filter((s) => !s.builtin).length; if (mcpCount > 0) console.log(`[codeoid] mcp: ${mcpCount} external server(s) registered`); @@ -503,6 +510,15 @@ export class DaemonServer { return ep.handle(req); } + // Goal-blackboard MCP endpoint. Same contract as memory: the bearer + // token minted per role-child IS the scope (one goal, one role's + // read/write set), and an unknown token fails closed. Always mounted — + // unlike memory it has no enable flag, and with no minted tokens every + // request 401s anyway. + if (url.pathname === BLACKBOARD_MCP_PATH) { + return self.#manager.blackboardMcp.handle(req); + } + if (url.pathname === "/config") { return Response.json({ zeroid_url: authConfig.baseUrl, diff --git a/src/daemon/session-manager.ts b/src/daemon/session-manager.ts index 1b178e9..0d9b8f6 100644 --- a/src/daemon/session-manager.ts +++ b/src/daemon/session-manager.ts @@ -38,6 +38,9 @@ import { resolveAgainstList, resolveModelIdForProvider, } from "./models.js"; +import { Blackboard } from "./blackboard/service.js"; +import { BlackboardStore } from "./blackboard/store.js"; +import { BlackboardMcpHttp } from "./blackboard/mcp-http.js"; import { childBrief, childSessionName, @@ -93,6 +96,7 @@ import type { AuthContext, ClientMessage, CollaborationConfig, + CollaborationRole, DaemonMessage, McpServerStatus, ModelInfo, @@ -204,6 +208,14 @@ export class SessionManager { #rateLimiter: RateLimiter; #memory?: MemoryEngine; #memoryMcp?: MemoryMcpMount; + /** Goal-blackboard MCP endpoint + the loopback URL children mount it from. + * Always constructed: with no minted tokens every request fails closed. */ + readonly #blackboardMcp = new BlackboardMcpHttp(); + #blackboardUrl?: string; + /** Per-goal artifact store, lazily built on the shared DB connection. */ + #blackboard?: Blackboard; + /** child session id → its blackboard bearer token, revoked on teardown. */ + readonly #blackboardTokens = new Map(); #mcpRegistry?: McpRegistry; #mcpHub?: McpHub; /** Live model catalogs by provider id (via each backend's supportedModels @@ -1321,6 +1333,21 @@ mcpHub: this.#mcpHub, this.#memoryMcp = mount; } + /** The goal-blackboard MCP endpoint, routed by the HTTP server. */ + get blackboardMcp(): BlackboardMcpHttp { + return this.#blackboardMcp; + } + + /** + * The URL a role-child mounts the blackboard from. Loopback regardless of the + * daemon's bind address — the agent subprocess runs on this host, and the + * endpoint must not become reachable off-box merely because the daemon binds + * wide. Same reasoning as the memory mount. + */ + setBlackboardUrl(url: string): void { + this.#blackboardUrl = url; + } + /** Inject the cross-backend MCP registry + daemon-owned client pool. Sessions * hand both to every provider so the registry's servers mount on all backends. */ setMcp(registry: McpRegistry, hub: McpHub): void { @@ -1617,6 +1644,47 @@ mcpHub: this.#mcpHub, * pack constitution instead, so bringing up a fleet of N costs zero tokens * and no child burns a turn just to learn it should wait. */ + /** Lazily build the blackboard over the daemon's existing DB connection. */ + #goalBlackboard(): Blackboard { + if (!this.#blackboard) { + this.#blackboard = new Blackboard(new BlackboardStore(this.#store.database)); + } + return this.#blackboard; + } + + /** + * Mount config for one role-child's blackboard access, or undefined when the + * URL isn't known yet (the HTTP server sets it at startup; unit tests that + * construct a bare SessionManager legitimately have none). + * + * The minted token carries the role's scope, so the child's mount is its + * permission — there is no wider handle reachable from it. + */ + #blackboardMountFor( + parent: Session, + child: PlannedChild, + role: CollaborationRole | undefined, + ): { url: string; token: string } | undefined { + if (!this.#blackboardUrl) return undefined; + const handle = this.#goalBlackboard().forRole( + { + accountId: parent.accountId, + projectId: parent.projectId, + goalSessionId: parent.id, + }, + { + roleName: child.roleName, + ordinal: child.ordinal, + // Attribution keyed to the ROLE within the goal, not the child's + // session id: a role-child replaced after a restart is still the same + // contributor, and its earlier artifacts should keep reading that way. + authorSub: `agent:${parent.id}:${child.roleName}#${child.ordinal}`, + }, + role ? { reads: role.reads, writes: role.writes } : undefined, + ); + return { url: this.#blackboardUrl, token: this.#blackboardMcp.mint(handle) }; + } + async #spawnCollaborationChildren( parent: Session, collaboration: CollaborationConfig, @@ -1625,6 +1693,13 @@ mcpHub: this.#mcpHub, ): Promise<{ ok: true } | { ok: false; error: string }> { for (const child of planned) { try { + // Minted before construction so the child's provider can mount it from + // the start — the token carries this role's read/write scope. + const blackboard = this.#blackboardMountFor( + parent, + child, + collaboration.roles.find((r) => r.name === child.roleName), + ); const childSession = new Session({ name: childSessionName(parent.name, child), workdir: parent.workdir, @@ -1665,6 +1740,10 @@ mcpHub: this.#mcpHub, ordinal: child.ordinal, write: child.write, }, + // Role-scoped goal blackboard, mountable by ANY backend (#245) — + // this is how a gemini reviewer and an openai reasoner hand work to + // each other without the orchestrator relaying it as prose. + blackboardMcp: blackboard, identityManager: this.#identityManager, memory: this.#memory, memoryMcp: this.#memoryMcp, @@ -1677,6 +1756,7 @@ mcpHub: this.#mcpHub, onModels: (providerId, m) => this._cacheModels(providerId, m), }); this.#sessions.set(childSession.id, childSession); + if (blackboard) this.#blackboardTokens.set(childSession.id, blackboard.token); // No rate-limiter charge: the human called session.create once, and // the child count is already bounded by MAX_COLLABORATION_CHILDREN. // Mirrors spawnWorker, which charges nothing for the same reason. @@ -1715,6 +1795,14 @@ mcpHub: this.#mcpHub, `[codeoid/collaboration] child teardown failed (${reason}): ${err instanceof Error ? err.message : String(err)}`, ); } + // Revoke the blackboard token BEFORE dropping the session, so a mount + // that outlives teardown (a wedged subprocess still holding the URL) + // cannot keep reading the goal's artifacts. + const token = this.#blackboardTokens.get(child.id); + if (token) { + this.#blackboardMcp.revoke(token); + this.#blackboardTokens.delete(child.id); + } this.#sessions.delete(child.id); this.#store.audit( "system:collaboration", @@ -3627,6 +3715,19 @@ mcpHub: this.#mcpHub, // reach them by. if (session.collaboration) { await this.#teardownCollaborationChildren(msg.sessionId, "collaboration goal ended"); + // Artifacts are goal-scoped, so they die with the goal. Dropped AFTER the + // children so a child mid-write can't recreate rows behind the delete. + try { + this.#goalBlackboard().deleteGoal({ + accountId: session.accountId, + projectId: session.projectId, + goalSessionId: session.id, + }); + } catch (err) { + console.error( + `[codeoid/collaboration] artifact cleanup failed for ${session.id}: ${err instanceof Error ? err.message : String(err)}`, + ); + } } await session.destroy(auth); diff --git a/src/daemon/session.ts b/src/daemon/session.ts index f26f63c..8003c95 100644 --- a/src/daemon/session.ts +++ b/src/daemon/session.ts @@ -249,6 +249,14 @@ export interface SessionCreateOptions { * orphan an agent subprocess. */ collaborationRole?: SessionInfo["collaborationRole"]; + /** + * Role-scoped goal-blackboard mount for a collaboration child: the endpoint + * URL plus a bearer token that IS the scope (one goal, this role's read/write + * set). Handed to the provider like `memoryMcp`, so any backend able to mount + * an MCP URL gets it — which is the point of making the blackboard mountable + * rather than an in-process Claude-SDK server (#245). + */ + blackboardMcp?: { url: string; token: string }; /** * Pre-built codeoid_fleet MCP server (conductor sessions only). Built by * the SessionManager because its tools close over the manager's tenant- @@ -331,6 +339,8 @@ export class Session { readonly collaboration?: CollaborationConfig; /** Which collaboration + role this session serves, when it is a child. */ readonly collaborationRole?: SessionInfo["collaborationRole"]; + /** Role-scoped blackboard mount (URL + scope-bearing token), when a child. */ + readonly #blackboardMcp?: { url: string; token: string }; readonly createdBy: string; readonly createdAt: string; /** @@ -631,6 +641,7 @@ export class Session { this.worktree = opts.worktree; this.collaboration = opts.collaboration; this.collaborationRole = opts.collaborationRole; + this.#blackboardMcp = opts.blackboardMcp; this.#onStatusChange = opts.onStatusChange; this.#workerShape = opts.workerShape; if (opts.initialMode) { @@ -824,6 +835,7 @@ export class Session { identityManager: this.#identityManager, memory: this.#memory, memoryMcp: this.#memoryMcp, + blackboardMcp: this.#blackboardMcp, mcpRegistry: this.#mcpRegistry, mcpHub: this.#mcpHub, fleet: this.#fleet, diff --git a/src/tests/blackboard-mcp.test.ts b/src/tests/blackboard-mcp.test.ts new file mode 100644 index 0000000..867a3ed --- /dev/null +++ b/src/tests/blackboard-mcp.test.ts @@ -0,0 +1,242 @@ +/** + * Goal-blackboard MCP endpoint — the mountable surface (#245). + * + * The property under test is that THE TOKEN IS THE SCOPE. A mount cannot + * address another goal or widen its own read/write set, because no tool takes + * a goal id and `blackboard_write` takes no slot. If that ever stops being + * true, a gemini reviewer could read the implementer's reasoning or overwrite + * a peer's findings, and the independence guarantee in §6 evaporates. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { BlackboardMcpHttp, BLACKBOARD_MCP_PATH } from "../daemon/blackboard/mcp-http.js"; +import { Blackboard, type RoleIdentity } from "../daemon/blackboard/service.js"; +import { BlackboardStore, type GoalScope } from "../daemon/blackboard/store.js"; +import { Store } from "../daemon/store.js"; + +let tmp: string; +let store: Store; +let bb: Blackboard; +let mcp: BlackboardMcpHttp; + +const GOAL: GoalScope = { accountId: "acc", projectId: "proj", goalSessionId: "goal-1" }; +const ident = (roleName: string, ordinal = 1): RoleIdentity => ({ + roleName, + ordinal, + authorSub: `agent:${roleName}#${ordinal}`, +}); + +const URL_ = `http://127.0.0.1:7400${BLACKBOARD_MCP_PATH}`; + +/** POST one JSON-RPC message with a bearer token. */ +async function rpc( + token: string | null, + method: string, + params?: Record, +): Promise<{ status: number; body: any }> { + const headers: Record = { "Content-Type": "application/json" }; + if (token) headers.Authorization = `Bearer ${token}`; + const res = await mcp.handle( + new Request(URL_, { + method: "POST", + headers, + body: JSON.stringify({ jsonrpc: "2.0", id: 1, method, params }), + }), + ); + const text = await res.text(); + return { status: res.status, body: text ? JSON.parse(text) : null }; +} + +const call = (token: string, name: string, args: Record = {}) => + rpc(token, "tools/call", { name, arguments: args }); + +/** The text payload of a tools/call result. */ +const textOf = (body: any): string => body?.result?.content?.[0]?.text ?? ""; +const isErr = (body: any): boolean => body?.result?.isError === true; + +beforeEach(() => { + tmp = mkdtempSync(join(tmpdir(), "codeoid-bbmcp-")); + store = new Store(join(tmp, "codeoid.db")); + bb = new Blackboard(new BlackboardStore(store.database)); + mcp = new BlackboardMcpHttp(); +}); + +afterEach(() => { + rmSync(tmp, { recursive: true, force: true }); +}); + +describe("transport + auth", () => { + // Each auth test mints an unrelated VALID token first. Without that, the map + // is empty and a "reject unless the token resolves" test passes even against + // an endpoint that falls back to whatever binding happens to exist — proving + // only "401 when nothing is minted". Mutation testing caught exactly that. + const mintOther = () => mcp.mint(bb.forRole({ ...GOAL, goalSessionId: "other" }, ident("review"))); + + test("fails closed with no token, even when other mounts are live", async () => { + mintOther(); + const r = await rpc(null, "tools/list"); + expect(r.status).toBe(401); + }); + + test("fails closed on an unknown token, even when other mounts are live", async () => { + mintOther(); + const r = await rpc("bbt_nope", "tools/list"); + expect(r.status).toBe(401); + }); + + test("a revoked token stops working while its siblings keep working", async () => { + const sibling = mintOther(); + const token = mcp.mint(bb.forRole(GOAL, ident("review"))); + expect((await rpc(token, "tools/list")).status).toBe(200); + + mcp.revoke(token); + // Revoked one is dead... + expect((await rpc(token, "tools/list")).status).toBe(401); + // ...and the endpoint did NOT silently fall through to the live sibling. + expect((await rpc(sibling, "tools/list")).status).toBe(200); + expect(mcp.activeTokens).toBe(1); + }); + + test("GET is rejected — POST-only Streamable HTTP", async () => { + const res = await mcp.handle(new Request(URL_, { method: "GET" })); + expect(res.status).toBe(405); + }); + + test("initialize + tools/list advertise the four tools", async () => { + const token = mcp.mint(bb.forRole(GOAL, ident("review"))); + const init = await rpc(token, "initialize", { protocolVersion: "2025-06-18" }); + expect(init.body.result.serverInfo.name).toBe("codeoid_blackboard"); + const list = await rpc(token, "tools/list"); + expect(list.body.result.tools.map((t: { name: string }) => t.name).sort()).toEqual([ + "blackboard_index", + "blackboard_read", + "blackboard_read_all", + "blackboard_write", + ]); + }); + + test("a notification (no id) gets 202 and no body", async () => { + const token = mcp.mint(bb.forRole(GOAL, ident("review"))); + const res = await mcp.handle( + new Request(URL_, { + method: "POST", + headers: { "Content-Type": "application/json", Authorization: `Bearer ${token}` }, + body: JSON.stringify({ jsonrpc: "2.0", method: "notifications/initialized" }), + }), + ); + expect(res.status).toBe(202); + }); +}); + +describe("the token carries the role's scope", () => { + test("a reviewer can read diff and write findings", async () => { + bb.forRole(GOAL, ident("reasoning")).write("diff", "the change"); + const token = mcp.mint(bb.forRole(GOAL, ident("review"))); + + const read = await call(token, "blackboard_read", { kind: "diff" }); + expect(isErr(read.body)).toBe(false); + expect(textOf(read.body)).toContain("the change"); + + const write = await call(token, "blackboard_write", { + kind: "findings", + content: "looks good", + }); + expect(isErr(write.body)).toBe(false); + expect(textOf(write.body)).toMatch(/Wrote findings \[review\] v1/); + }); + + // The §6 guarantee, exercised through the transport an actual agent uses. + test("a reviewer is refused research, with a reason it can act on", async () => { + bb.forRole(GOAL, ident("search")).write("research", "how I got here"); + const token = mcp.mint(bb.forRole(GOAL, ident("review"))); + const r = await call(token, "blackboard_read", { kind: "research" }); + // An MCP tool error, not a transport error: the agent should see WHY and + // adapt, not get an opaque failure it retries forever. + expect(r.status).toBe(200); + expect(isErr(r.body)).toBe(true); + expect(textOf(r.body)).toMatch(/may not read "research"/); + }); + + test("a reviewer is refused writing outside its lane", async () => { + const token = mcp.mint(bb.forRole(GOAL, ident("review"))); + const r = await call(token, "blackboard_write", { kind: "diff", content: "sneaky" }); + expect(isErr(r.body)).toBe(true); + expect(textOf(r.body)).toMatch(/may not write "diff"/); + }); + + // 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 () => { + 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"]) { + expect(params).not.toContain(forbidden); + } + }); + + // Two goals, two tokens: neither can see the other's artifacts. + test("a token cannot reach another goal's artifacts", async () => { + const other: GoalScope = { ...GOAL, goalSessionId: "goal-2" }; + bb.forRole(GOAL, ident("reasoning")).write("diff", "goal one diff"); + bb.forRole(other, ident("reasoning")).write("diff", "goal two diff"); + + const t1 = mcp.mint(bb.forRole(GOAL, ident("review"))); + const t2 = mcp.mint(bb.forRole(other, ident("review"))); + + expect(textOf((await call(t1, "blackboard_read", { kind: "diff" })).body)).toContain( + "goal one diff", + ); + expect(textOf((await call(t2, "blackboard_read", { kind: "diff" })).body)).toContain( + "goal two diff", + ); + }); + + test("blackboard_write takes no slot, so a peer's entry is unreachable", async () => { + const t1 = mcp.mint(bb.forRole(GOAL, ident("review", 1))); + const t2 = mcp.mint(bb.forRole(GOAL, ident("review", 2))); + await call(t1, "blackboard_write", { kind: "findings", content: "from one" }); + // Even passing a slot explicitly cannot redirect the write — the schema + // rejects unknown properties and the service picks the slot regardless. + await call(t2, "blackboard_write", { kind: "findings", content: "from two", slot: "review" }); + + const all = new BlackboardStore(store.database).latestAllSlots(GOAL, "findings"); + expect(all).toHaveLength(2); + expect(all.map((a) => a.content).sort()).toEqual(["from one", "from two"]); + }); + + test("the index is readable and carries no bodies", async () => { + bb.forRole(GOAL, ident("search")).write("research", "SECRET-RESEARCH-BODY"); + const token = mcp.mint(bb.forRole(GOAL, ident("review"))); + const r = await call(token, "blackboard_index"); + const text = textOf(r.body); + expect(text).toContain("research"); + // Knowing it exists is not reading it. + expect(text).not.toContain("SECRET-RESEARCH-BODY"); + // And the mount tells the agent its own scope, so it can plan. + expect(text).toMatch(/You may read: spec, diff/); + expect(text).toMatch(/You may write: findings/); + }); + + test("an unknown tool is an error result, not a crash", async () => { + const token = mcp.mint(bb.forRole(GOAL, ident("review"))); + const r = await call(token, "blackboard_nope"); + expect(isErr(r.body)).toBe(true); + expect(textOf(r.body)).toMatch(/Unknown tool/); + }); + + test("an unknown method is a JSON-RPC error", async () => { + const token = mcp.mint(bb.forRole(GOAL, ident("review"))); + const r = await rpc(token, "resources/list"); + expect(r.body.error.code).toBe(-32601); + }); +}); From a0646914c2371a9e68aaca1e84242bc330d36d5e Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Mon, 27 Jul 2026 11:26:14 +0800 Subject: [PATCH 3/5] =?UTF-8?q?feat:=20mount=20the=20blackboard=20on=20eve?= =?UTF-8?q?ry=20backend=20=E2=80=94=20collaboration=20is=20now=20end-to-en?= =?UTF-8?q?d?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the blackboard's usable path. Until now the endpoint existed and minted scoped tokens, but no provider surfaced it, so no agent could call the tools. A role-child can now actually read its inputs and publish its output. - claude: mounted as `{type:"http"}` in the SDK's mcpServers. Deliberately the HTTP mount and not an in-process createSdkMcpServer object — claude gets no privileged path here, because the same surface has to work on gemini and codex (#245). That in-process shortcut is exactly why the ORCHESTRATOR is claude-only today, and the blackboard must not repeat it. - acp (gemini-cli): pushed onto session/new's MCP server list. - codex: `-c mcp_servers.codeoid_blackboard.{url,bearer_token_env_var}` with the token passed via env, not argv — process argv is world-readable on Linux and these tokens ARE the scope. - registry forwards the mount to all three factories. None of the providers mint or revoke, unlike the memory mount they sit next to. The SessionManager owns this token because the scope it encodes belongs to the collaboration, not to a backing session that resetToNewSession may recreate — re-minting per backing session would leave a second live token that teardown cannot revoke. tool-safety: blackboard READS auto-approve (both `mcp__server__tool` and `server__tool` conventions); `blackboard_write` deliberately does NOT. The role's write scope decides whether a write is PERMITTED; auto-approval decides whether it happens without anyone looking, and a write publishes into shared state peers act on. Look-alike server names and unknown tools fall through to prompting, matching the memory rule. childBrief now tells the child which tools exist and what it may read/write — an agent that isn't told about a tool never calls it, so the mount alone would have demoed as "nothing happens". The brief resolves scope through the SAME resolveRoleIo the fence uses (extracted for this): computing them separately would eventually tell an agent it can read something the fence then refuses, which is the worst failure mode to hand a model. Tests: +4 on tool-safety classification. Suite 2040 pass / 0 fail, typecheck + biome + build clean. Note on CI: retargeting this PR to main did NOT trigger the workflow — it fires on opened/synchronize/reopened, and a base change is `edited`. This push is a synchronize, so daemon/web should finally run against the right base. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Yash Datta --- src/daemon/blackboard/mcp-http.ts | 8 ++++++ src/daemon/blackboard/service.ts | 33 ++++++++++++++++++---- src/daemon/collaboration.ts | 19 +++++++++++++ src/daemon/providers/acp/index.ts | 26 ++++++++++++++++++ src/daemon/providers/claude/index.ts | 20 ++++++++++++++ src/daemon/providers/codex/index.ts | 41 ++++++++++++++++++++++++++-- src/daemon/providers/registry.ts | 3 ++ src/daemon/providers/tool-safety.ts | 27 ++++++++++++++++++ src/tests/blackboard-mcp.test.ts | 29 ++++++++++++++++++++ 9 files changed, 198 insertions(+), 8 deletions(-) diff --git a/src/daemon/blackboard/mcp-http.ts b/src/daemon/blackboard/mcp-http.ts index cb88889..3d8d0d4 100644 --- a/src/daemon/blackboard/mcp-http.ts +++ b/src/daemon/blackboard/mcp-http.ts @@ -45,6 +45,14 @@ export const BLACKBOARD_MCP_PATH = "/mcp/blackboard"; */ export const BLACKBOARD_MCP_SERVER_NAME = "codeoid_blackboard"; +/** + * Env var carrying the bearer token for backends that mount by config rather + * than by header (codex reads `bearer_token_env_var`). Keeping the token out of + * the `-c` args matters: process argv is world-readable on Linux, and these + * tokens are the scope. + */ +export const BLACKBOARD_MCP_TOKEN_ENV = "CODEOID_BLACKBOARD_TOKEN"; + const SERVER_INFO = { name: BLACKBOARD_MCP_SERVER_NAME, version: "0.1.0" } as const; /** What a mount hands to a session so it can reach the blackboard. */ diff --git a/src/daemon/blackboard/service.ts b/src/daemon/blackboard/service.ts index 44a2c10..7ed8a05 100644 --- a/src/daemon/blackboard/service.ts +++ b/src/daemon/blackboard/service.ts @@ -60,6 +60,27 @@ export const DEFAULT_ROLE_IO: Readonly> = { */ export const MULTI_WRITER_KINDS: ReadonlySet = new Set(["findings"]); +/** + * Resolve a role's effective artifact scope: what it declared, else the §3 + * default profile for its name, else nothing. + * + * Exported and shared with `childBrief` on purpose. The brief TELLS a child + * what it may touch and this function DECIDES it; if those were computed + * separately they would eventually disagree, and the agent would be told it + * can read something the fence then refuses — the most confusing possible + * failure for a model to recover from. + */ +export function resolveRoleIo( + roleName: string, + declared?: { reads?: readonly string[]; writes?: readonly string[] }, +): RoleIo { + const fallback = DEFAULT_ROLE_IO[roleName]; + return { + reads: declared?.reads ?? fallback?.reads ?? [], + writes: declared?.writes ?? fallback?.writes ?? [], + }; +} + export type BlackboardDenial = { ok: false; error: string }; export type BlackboardResult = { ok: true; value: T } | BlackboardDenial; @@ -205,12 +226,12 @@ export class Blackboard { identity: RoleIdentity, declared?: { reads?: readonly string[]; writes?: readonly string[] }, ): RoleBlackboard { - const fallback = DEFAULT_ROLE_IO[identity.roleName]; - const io: RoleIo = { - reads: declared?.reads ?? fallback?.reads ?? [], - writes: declared?.writes ?? fallback?.writes ?? [], - }; - return new RoleBlackboard(this.#store, scope, identity, io); + return new RoleBlackboard( + this.#store, + scope, + identity, + resolveRoleIo(identity.roleName, declared), + ); } /** Drop a goal's artifacts. Called on collaboration teardown. */ diff --git a/src/daemon/collaboration.ts b/src/daemon/collaboration.ts index 672b0eb..d2e925d 100644 --- a/src/daemon/collaboration.ts +++ b/src/daemon/collaboration.ts @@ -19,6 +19,7 @@ import type { CollaborationConfig, CollaborationRole } from "../protocol/types.j import { LIMITS, ORCHESTRATOR_ROLE } from "../protocol/types.js"; import { CLAUDE_PROVIDER_ID, resolveModelIdForProvider } from "./models.js"; import { CORE_ARTIFACT_KINDS, isValidArtifactKind } from "./blackboard/types.js"; +import { resolveRoleIo } from "./blackboard/service.js"; /** The provider-registry surface this module needs — kept narrow so tests * can pass a stub instead of building a real registry. */ @@ -233,6 +234,9 @@ export interface PlannedChild { shape: "ship" | "scout"; write: boolean; purpose?: string; + /** Declared blackboard scope, if the role set one; absent = §3 default. */ + reads?: readonly string[]; + writes?: readonly string[]; } /** @@ -266,6 +270,8 @@ export function planChildren( shape: role.write === true ? "ship" : "scout", write: role.write === true, ...(role.purpose !== undefined ? { purpose: role.purpose } : {}), + ...(role.reads !== undefined ? { reads: role.reads } : {}), + ...(role.writes !== undefined ? { writes: role.writes } : {}), }); } } @@ -301,12 +307,25 @@ export function childBrief( const contract = child.write ? "You MAY modify files in your workdir. Keep the diff minimal and verify your work." : "You are READ-ONLY: your identity holds no write scope, so file edits will be denied. Investigate and report — your written findings are the deliverable."; + const io = resolveRoleIo(child.roleName, { reads: child.reads, writes: child.writes }); return [ ` 1 ? ` member="${child.ordinal}"` : ""}>`, `You are the "${child.roleName}" role in a collaborative session working one shared goal.`, child.purpose ? `Your purpose: ${child.purpose}` : null, contract, "You are one of several agents on this goal, possibly on different model backends. You cannot see the others' work or the orchestrator's reasoning — that is deliberate, so your contribution stays independent.", + "", + "## Handing work off", + "", + "Shared state lives on the goal BLACKBOARD, not in chat. Use the blackboard tools:", + "- `blackboard_index` — what exists, at what version, written by whom (no contents).", + "- `blackboard_read` / `blackboard_read_all` — read an artifact you are scoped for.", + "- `blackboard_write` — publish YOUR output. It appends a version; it never overwrites, and for multi-writer kinds you write your own entry.", + "", + `You can READ: ${io.reads.length > 0 ? io.reads.join(", ") : "(nothing — you work only from the task you are sent)"}`, + `You can WRITE: ${io.writes.length > 0 ? io.writes.join(", ") : "(nothing — report back in your reply instead)"}`, + "Anything outside that is refused by the daemon, not by your own judgement — don't work around it, and don't ask another agent to fetch it for you.", + "", "Wait for instructions from the orchestrator before acting; it will send you a specific task.", "", "", diff --git a/src/daemon/providers/acp/index.ts b/src/daemon/providers/acp/index.ts index 4ad1245..9cc5104 100644 --- a/src/daemon/providers/acp/index.ts +++ b/src/daemon/providers/acp/index.ts @@ -39,6 +39,7 @@ import { renderHistorySeed, type CanonicalTurn, type HistorySeedResult } from ". import { buildGeminiCliEnv } from "../env.js"; import { StdioJsonRpcProcess } from "../jsonrpc-stdio.js"; import { MEMORY_MCP_SERVER_NAME, type MemoryMcpMount } from "../../memory/mcp-http.js"; +import { BLACKBOARD_MCP_SERVER_NAME } from "../../blackboard/mcp-http.js"; import type { McpRegistry } from "../../mcp/registry.js"; import { resolveEnvMap } from "../../mcp/types.js"; @@ -57,6 +58,14 @@ export interface GeminiAcpProviderInit { * store on demand — the precondition for the Verbatim Working Set strategy. */ memoryMcp?: MemoryMcpMount; + /** + * Role-scoped goal-blackboard mount for a collaboration child. Unlike the + * memory mount, the token is minted ONCE by the SessionManager (it encodes + * this role's read/write scope) and revoked at collaboration teardown — so + * this provider carries it, it does not mint or revoke. + */ + blackboardMcp?: { url: string; token: string }; + /** Cross-backend MCP registry — external servers mount on session/new * (gemini-cli owns its client); approval flows through canUseTool. */ mcpRegistry?: McpRegistry; @@ -77,6 +86,8 @@ export class GeminiAcpProvider implements SessionProvider { #argsPrefix: string[]; #workspaceId: string; #memoryMcp: MemoryMcpMount | null; + /** Role-scoped blackboard mount; token minted+revoked by the SessionManager. */ + readonly #blackboardMcp: { url: string; token: string } | null; #mcpRegistry: McpRegistry | null; /** Live scoped token for the mounted memory endpoint; revoked on teardown. */ #memoryToken: string | null = null; @@ -110,6 +121,7 @@ export class GeminiAcpProvider implements SessionProvider { this.#argsPrefix = init.argsPrefix ?? []; this.#workspaceId = init.workspaceId ?? init.sessionId; this.#memoryMcp = init.memoryMcp ?? null; + this.#blackboardMcp = init.blackboardMcp ?? null; this.#mcpRegistry = init.mcpRegistry ?? null; } @@ -348,6 +360,20 @@ export class GeminiAcpProvider implements SessionProvider { headers: [{ name: "Authorization", value: `Bearer ${this.#memoryToken}` }], }); } + // Role-scoped goal blackboard (collaboration children). Deliberately NO + // mint/revoke here, unlike memory above: the SessionManager owns this + // token's lifetime, because the scope it encodes belongs to the + // collaboration rather than to a backing session that resetToNewSession + // may recreate. Re-minting per backing session would hand this child a + // second live token the manager can't revoke at teardown. + if (this.#blackboardMcp) { + servers.push({ + type: "http", + name: BLACKBOARD_MCP_SERVER_NAME, + url: this.#blackboardMcp.url, + headers: [{ name: "Authorization", value: `Bearer ${this.#blackboardMcp.token}` }], + }); + } // Registry servers — native mount (gemini-cli owns its MCP client). ACP's // McpServer shape: http → {type,url,headers[]}; stdio → {command,args,env[]} // (env/headers are {name,value} pairs, mirroring the memory http mount). diff --git a/src/daemon/providers/claude/index.ts b/src/daemon/providers/claude/index.ts index a750f4f..5a9a773 100644 --- a/src/daemon/providers/claude/index.ts +++ b/src/daemon/providers/claude/index.ts @@ -33,6 +33,7 @@ import { MEMORY_TOOL_NAMES, type MemoryEngine, } from "../../memory/index.js"; +import { BLACKBOARD_MCP_SERVER_NAME } from "../../blackboard/mcp-http.js"; import type { McpRegistry } from "../../mcp/registry.js"; import { resolveEnvMap } from "../../mcp/types.js"; import type { CompressionRegistry } from "../../compress/index.js"; @@ -79,6 +80,13 @@ export interface ClaudeProviderInit { memory?: MemoryEngine; /** codeoid_fleet MCP server — conductor sessions only (read-only fleet view). */ fleet?: McpSdkServerConfigWithInstance; + /** + * Role-scoped goal-blackboard mount for a collaboration child. Unlike the + * memory mount, the token is minted ONCE by the SessionManager (it encodes + * this role's read/write scope) and revoked at collaboration teardown — so + * this provider carries it, it does not mint or revoke. + */ + blackboardMcp?: { url: string; token: string }; /** Cross-backend MCP registry — external servers are mounted natively on the * SDK (claude owns its own MCP client); approval flows through canUseTool. */ mcpRegistry?: McpRegistry; @@ -388,6 +396,18 @@ export class ClaudeProvider implements SessionProvider { // Conductor sessions only — the read-only fleet view (P3). In-process, // so the external-server timeout doesn't apply. ...(init.fleet ? { codeoid_fleet: init.fleet } : {}), + // Collaboration children only — the role-scoped goal blackboard. Mounted + // over HTTP rather than in-process precisely so the SAME surface works on + // gemini/codex; claude deliberately gets no privileged path here (#245). + ...(init.blackboardMcp + ? { + [BLACKBOARD_MCP_SERVER_NAME]: { + type: "http", + url: init.blackboardMcp.url, + headers: { Authorization: `Bearer ${init.blackboardMcp.token}` }, + } as unknown as McpServerConfig, + } + : {}), // Registry servers — mounted natively on the SDK (claude owns its client); // tool calls surface as `mcp____` and gate via canUseTool. ...withMcpToolTimeout(registryServersForClaude(init.mcpRegistry), mcpToolTimeoutMs), diff --git a/src/daemon/providers/codex/index.ts b/src/daemon/providers/codex/index.ts index ec3f1e6..ee5b0a9 100644 --- a/src/daemon/providers/codex/index.ts +++ b/src/daemon/providers/codex/index.ts @@ -61,6 +61,10 @@ import { buildCodexEnv } from "../env.js"; import { CodexRpcProcess } from "./rpc.js"; import type { SessionMode } from "../../../protocol/types.js"; import { MEMORY_MCP_SERVER_NAME, MEMORY_MCP_TOKEN_ENV, type MemoryMcpMount } from "../../memory/mcp-http.js"; +import { + BLACKBOARD_MCP_SERVER_NAME, + BLACKBOARD_MCP_TOKEN_ENV, +} from "../../blackboard/mcp-http.js"; import type { McpRegistry } from "../../mcp/registry.js"; import { resolveEnvMap } from "../../mcp/types.js"; @@ -89,6 +93,14 @@ export interface CodexProviderInit { * codex can page the verbatim store on demand — the precondition for VWS. */ memoryMcp?: MemoryMcpMount; + /** + * Role-scoped goal-blackboard mount for a collaboration child. Unlike the + * memory mount, the token is minted ONCE by the SessionManager (it encodes + * this role's read/write scope) and revoked at collaboration teardown — so + * this provider carries it, it does not mint or revoke. + */ + blackboardMcp?: { url: string; token: string }; + /** Cross-backend MCP registry — external servers mount natively via `-c * mcp_servers.*` (codex owns its client); approval flows through canUseTool. */ mcpRegistry?: McpRegistry; @@ -198,6 +210,8 @@ export class CodexProvider implements SessionProvider { #onModels?: CodexProviderInit["onModels"]; #workspaceId: string; #memoryMcp: MemoryMcpMount | null; + /** Role-scoped blackboard mount; token minted+revoked by the SessionManager. */ + readonly #blackboardMcp: { url: string; token: string } | null; #mcpRegistry: McpRegistry | null; /** Live scoped token for the mounted memory endpoint; revoked on teardown. */ #memoryToken: string | null = null; @@ -241,6 +255,7 @@ export class CodexProvider implements SessionProvider { this.#onModels = init.onModels; this.#workspaceId = init.workspaceId ?? init.sessionId; this.#memoryMcp = init.memoryMcp ?? null; + this.#blackboardMcp = init.blackboardMcp ?? null; this.#mcpRegistry = init.mcpRegistry ?? null; } @@ -315,6 +330,27 @@ export class CodexProvider implements SessionProvider { }; } + /** + * `-c mcp_servers.codeoid_blackboard.*` — the role-scoped goal blackboard for + * a collaboration child, same TOML shape as the memory mount above. + * + * No mint/revoke here: the SessionManager owns this token, because the scope + * it carries belongs to the collaboration rather than to a codex backing + * session that may be recreated mid-goal. + */ + #blackboardMcpSpawn(): { args: string[]; env: Record } { + const mount = this.#blackboardMcp; + if (!mount) return { args: [], env: {} }; + const key = `mcp_servers.${BLACKBOARD_MCP_SERVER_NAME}`; + return { + args: [ + "-c", `${key}.url=${JSON.stringify(mount.url)}`, + "-c", `${key}.bearer_token_env_var=${JSON.stringify(BLACKBOARD_MCP_TOKEN_ENV)}`, + ], + env: { [BLACKBOARD_MCP_TOKEN_ENV]: mount.token }, + }; + } + /** * `-c mcp_servers.*` overrides mounting the registry's external servers on the * codex app-server — a native mount, since codex owns its own MCP client. @@ -452,13 +488,14 @@ export class CodexProvider implements SessionProvider { // demand. No CODEX_HOME/auth.json juggling — the default ~/.codex keeps // the user's auth + config; these just add the one server. const mcp = this.#memoryMcpSpawn(); + const bb = this.#blackboardMcpSpawn(); const reg = this.#registryMcpArgs(); this.#proc = new CodexRpcProcess({ command: this.#command, argsPrefix: this.#argsPrefix, - args: [...mcp.args, ...reg.args], + args: [...mcp.args, ...bb.args, ...reg.args], cwd: opts.workdir, - env: { ...buildCodexEnv(), ...mcp.env, ...reg.env }, + env: { ...buildCodexEnv(), ...mcp.env, ...bb.env, ...reg.env }, onNotification: (method, params) => this.#onNotification(method, params), onServerRequest: (method, params) => this.#onServerRequest(method, params), onExit: ({ code, signal, stderrTail }) => { diff --git a/src/daemon/providers/registry.ts b/src/daemon/providers/registry.ts index 9d9772a..2ddd571 100644 --- a/src/daemon/providers/registry.ts +++ b/src/daemon/providers/registry.ts @@ -180,6 +180,7 @@ export function createDefaultProviderRegistry(config?: CodeoidConfig): ProviderR identityManager: init.identityManager, memory: init.memory, fleet: init.fleet, + blackboardMcp: init.blackboardMcp, mcpRegistry: init.mcpRegistry, config: init.config, compressionRegistry: init.compressionRegistry, @@ -289,6 +290,7 @@ export function createDefaultProviderRegistry(config?: CodeoidConfig): ProviderR store: init.store, workspaceId: init.workspaceId, memoryMcp: init.memoryMcp, + blackboardMcp: init.blackboardMcp, mcpRegistry: init.mcpRegistry, onModels: init.onModels, }), @@ -318,6 +320,7 @@ export function createDefaultProviderRegistry(config?: CodeoidConfig): ProviderR store: init.store, workspaceId: init.workspaceId, memoryMcp: init.memoryMcp, + blackboardMcp: init.blackboardMcp, mcpRegistry: init.mcpRegistry, onModels: init.onModels, }), diff --git a/src/daemon/providers/tool-safety.ts b/src/daemon/providers/tool-safety.ts index af1709c..cbe57b5 100644 --- a/src/daemon/providers/tool-safety.ts +++ b/src/daemon/providers/tool-safety.ts @@ -5,6 +5,7 @@ * widen: an over-broad match here is a prompt-bypass, so it's security-relevant. */ +import { BLACKBOARD_MCP_SERVER_NAME } from "../blackboard/mcp-http.js"; import { MEMORY_MCP_SERVER_NAME } from "../memory/mcp-http.js"; import { MEMORY_TOOL_NAMES } from "../memory/tools.js"; @@ -17,6 +18,27 @@ const MEMORY_TOOL_PREFIXES = [ `${MEMORY_MCP_SERVER_NAME}__`, // gemini-cli / codex URL mount ] as const; +/** Same two namespacing conventions, for the goal-blackboard mount. */ +const BLACKBOARD_TOOL_PREFIXES = [ + `mcp__${BLACKBOARD_MCP_SERVER_NAME}__`, + `${BLACKBOARD_MCP_SERVER_NAME}__`, +] as const; + +/** + * Blackboard tools that may run unprompted. + * + * READS ONLY. `blackboard_write` is deliberately absent even though the service + * already scope-checks it: a write publishes into shared state other agents act + * on, so it stays on the same footing as any other write tool. The role's write + * scope decides whether it is *permitted*; this decides whether it happens + * *without anyone looking*, and those are different questions. + */ +const BLACKBOARD_SAFE_TOOLS = [ + "blackboard_index", + "blackboard_read", + "blackboard_read_all", +] as const; + /** * True for read-only tools safe to run unprompted. The memory recall tools are * read-only, but a backend namespaces them (`mcp__codeoid_memory__recall`, @@ -33,6 +55,11 @@ export function isSafeTool(name: string): boolean { return (MEMORY_TOOL_NAMES as readonly string[]).includes(name.slice(prefix.length)); } } + for (const prefix of BLACKBOARD_TOOL_PREFIXES) { + if (name.startsWith(prefix)) { + return (BLACKBOARD_SAFE_TOOLS as readonly string[]).includes(name.slice(prefix.length)); + } + } return false; } diff --git a/src/tests/blackboard-mcp.test.ts b/src/tests/blackboard-mcp.test.ts index 867a3ed..bc267d6 100644 --- a/src/tests/blackboard-mcp.test.ts +++ b/src/tests/blackboard-mcp.test.ts @@ -15,6 +15,7 @@ import { join } from "node:path"; import { BlackboardMcpHttp, BLACKBOARD_MCP_PATH } from "../daemon/blackboard/mcp-http.js"; import { Blackboard, type RoleIdentity } from "../daemon/blackboard/service.js"; import { BlackboardStore, type GoalScope } from "../daemon/blackboard/store.js"; +import { isSafeTool } from "../daemon/providers/tool-safety.js"; import { Store } from "../daemon/store.js"; let tmp: string; @@ -240,3 +241,31 @@ describe("the token carries the role's scope", () => { expect(r.body.error.code).toBe(-32601); }); }); + +// ── Tool-safety classification ────────────────────────────────────────────── + +describe("blackboard tool safety", () => { + test("reads auto-approve under both namespacing conventions", () => { + for (const p of ["mcp__codeoid_blackboard__", "codeoid_blackboard__"]) { + for (const t of ["blackboard_index", "blackboard_read", "blackboard_read_all"]) { + expect(isSafeTool(`${p}${t}`)).toBe(true); + } + } + }); + + // Scope decides whether a write is PERMITTED; this decides whether it happens + // without anyone looking. A write publishes into shared state peers act on. + test("blackboard_write never auto-approves", () => { + expect(isSafeTool("mcp__codeoid_blackboard__blackboard_write")).toBe(false); + expect(isSafeTool("codeoid_blackboard__blackboard_write")).toBe(false); + }); + + test("a look-alike server name does not auto-approve", () => { + expect(isSafeTool("x_codeoid_blackboard__blackboard_read")).toBe(false); + expect(isSafeTool("mcp__codeoid_blackboard_evil__blackboard_read")).toBe(false); + }); + + test("an unknown blackboard tool does not auto-approve", () => { + expect(isSafeTool("mcp__codeoid_blackboard__blackboard_wipe")).toBe(false); + }); +}); From bb33930a66d63f2748a2bff5e968b6ee78b3c810 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Mon, 27 Jul 2026 11:52:27 +0800 Subject: [PATCH 4/5] fix: three defects found by live testing + audit (blackboard) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Everything below passed CI and 2040 unit tests while the feature could not complete a single handoff. Found by actually running it. 1. DEADLOCK — children spawned in interactive mode. blackboard_write needs approval, and NOBODY ATTACHES TO A CHILD, so the first handoff parked at waiting_approval with zero clients and stayed there. Observed live: child status `waiting approval`, clients: 0, zero artifacts written. Children now spawn autonomous with a bounded budget — the posture dispatch already gives its workers, for the same reason. The owner's approval happens once at the R3 dispatch gate, not per tool call. sendToSession also re-arms the budget on every dispatch: a child is long-lived across the goal (unlike a disposable spawn worker), so one initial budget is spent down across successive dispatches and the child would wedge partway through with nobody to approve. 2. THE ORCHESTRATOR HAD NO BLACKBOARD AT ALL. Only children got a mount. §4 has the orchestrator holding the index of artifact states and §7 has it reading every reviewer's findings to synthesize — both impossible. Children published artifacts the orchestrator could never see, so the coordination loop never closed. DEFAULT_ROLE_IO.orchestrator was defined and unreachable. It now gets its own orchestrator-scoped mount. This required making the mount lazily resolved (a getter, not a captured value): the orchestrator's goal id IS its session id, so its mount cannot exist until after the Session is constructed. 3. TOKEN LEAK — destroying a child directly skipped revocation. The token lives in the endpoint's binding map, not on the Session, so dropping the session left a credential that still authorized reads and writes against the goal. Revocation is now centralized in #revokeBlackboardToken and called from every path that removes a child, including the dispatch destroyWorker path (a child is role:"worker", so it can reach it). Live verification, end to end, against a real claude backend in an isolated XDG_CONFIG_HOME sandbox: - child called blackboard_index then blackboard_write; artifact landed (research v1, author_role=search, author_sub=agent::search#1) and the child returned to idle instead of wedging; - the fence refused out-of-scope reads in the transcript ("may not read …") with no artifacts created; - the orchestrator called blackboard_index and correctly reported "research v1, 365 bytes, author search" — the loop closing. Tests: +4, all mutation-checked (reverting each fix fails its test). One honest gap: no test distinguishes "mount minted and registered" from "mount attached to the Session" — asserting that needs a public session accessor I wasn't willing to add for a test alone. The teardown test covers registration. Suite 2044 pass / 0 fail, typecheck + biome + build clean. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Yash Datta --- src/daemon/providers/acp/index.ts | 21 ++++--- src/daemon/providers/claude/index.ts | 39 ++++++++----- src/daemon/providers/codex/index.ts | 16 +++--- src/daemon/providers/registry.ts | 10 +--- src/daemon/session-manager.ts | 82 +++++++++++++++++++++++++--- src/daemon/session.ts | 21 ++++++- src/tests/collaboration.test.ts | 73 +++++++++++++++++++++++++ 7 files changed, 214 insertions(+), 48 deletions(-) diff --git a/src/daemon/providers/acp/index.ts b/src/daemon/providers/acp/index.ts index 9cc5104..81d3375 100644 --- a/src/daemon/providers/acp/index.ts +++ b/src/daemon/providers/acp/index.ts @@ -59,12 +59,14 @@ export interface GeminiAcpProviderInit { */ memoryMcp?: MemoryMcpMount; /** - * Role-scoped goal-blackboard mount for a collaboration child. Unlike the - * memory mount, the token is minted ONCE by the SessionManager (it encodes - * this role's read/write scope) and revoked at collaboration teardown — so - * this provider carries it, it does not mint or revoke. + * Role-scoped goal-blackboard mount, resolved LAZILY. + * + * A getter rather than a value because the orchestrator's own mount is + * scoped to a goal id that IS its session id — so it cannot exist until + * after the Session is constructed. Providers read it when they build their + * server list (per turn for claude), by which time it is set. */ - blackboardMcp?: { url: string; token: string }; + blackboardMcp?: () => { url: string; token: string } | undefined; /** Cross-backend MCP registry — external servers mount on session/new * (gemini-cli owns its client); approval flows through canUseTool. */ @@ -87,7 +89,7 @@ export class GeminiAcpProvider implements SessionProvider { #workspaceId: string; #memoryMcp: MemoryMcpMount | null; /** Role-scoped blackboard mount; token minted+revoked by the SessionManager. */ - readonly #blackboardMcp: { url: string; token: string } | null; + readonly #blackboardMcp: (() => { url: string; token: string } | undefined) | null; #mcpRegistry: McpRegistry | null; /** Live scoped token for the mounted memory endpoint; revoked on teardown. */ #memoryToken: string | null = null; @@ -366,12 +368,13 @@ export class GeminiAcpProvider implements SessionProvider { // collaboration rather than to a backing session that resetToNewSession // may recreate. Re-minting per backing session would hand this child a // second live token the manager can't revoke at teardown. - if (this.#blackboardMcp) { + const bb = this.#blackboardMcp?.(); + if (bb) { servers.push({ type: "http", name: BLACKBOARD_MCP_SERVER_NAME, - url: this.#blackboardMcp.url, - headers: [{ name: "Authorization", value: `Bearer ${this.#blackboardMcp.token}` }], + url: bb.url, + headers: [{ name: "Authorization", value: `Bearer ${bb.token}` }], }); } // Registry servers — native mount (gemini-cli owns its MCP client). ACP's diff --git a/src/daemon/providers/claude/index.ts b/src/daemon/providers/claude/index.ts index 5a9a773..8395918 100644 --- a/src/daemon/providers/claude/index.ts +++ b/src/daemon/providers/claude/index.ts @@ -49,6 +49,21 @@ import type { PackSubagent } from "../../pipeline/subagents.js"; /** Map ambient-pack subagents to the Claude Agent SDK's programmatic `agents` * option (keyed by name). Empty → no `agents` key at all. */ +/** `{ codeoid_blackboard: … }` when a mount is present, else `{}`. Split out so + * the spread never introduces an `undefined`-valued key. */ +function blackboardServerEntry( + mount: { url: string; token: string } | undefined, +): Record { + if (!mount) return {}; + return { + [BLACKBOARD_MCP_SERVER_NAME]: { + type: "http", + url: mount.url, + headers: { Authorization: `Bearer ${mount.token}` }, + } as unknown as McpServerConfig, + }; +} + function packAgentsOption( subs?: readonly PackSubagent[], ): { agents?: Record } { @@ -81,12 +96,14 @@ export interface ClaudeProviderInit { /** codeoid_fleet MCP server — conductor sessions only (read-only fleet view). */ fleet?: McpSdkServerConfigWithInstance; /** - * Role-scoped goal-blackboard mount for a collaboration child. Unlike the - * memory mount, the token is minted ONCE by the SessionManager (it encodes - * this role's read/write scope) and revoked at collaboration teardown — so - * this provider carries it, it does not mint or revoke. + * Role-scoped goal-blackboard mount, resolved LAZILY. + * + * A getter rather than a value because the orchestrator's own mount is + * scoped to a goal id that IS its session id — so it cannot exist until + * after the Session is constructed. Providers read it when they build their + * server list (per turn for claude), by which time it is set. */ - blackboardMcp?: { url: string; token: string }; + blackboardMcp?: () => { url: string; token: string } | undefined; /** Cross-backend MCP registry — external servers are mounted natively on the * SDK (claude owns its own MCP client); approval flows through canUseTool. */ mcpRegistry?: McpRegistry; @@ -399,15 +416,9 @@ export class ClaudeProvider implements SessionProvider { // Collaboration children only — the role-scoped goal blackboard. Mounted // over HTTP rather than in-process precisely so the SAME surface works on // gemini/codex; claude deliberately gets no privileged path here (#245). - ...(init.blackboardMcp - ? { - [BLACKBOARD_MCP_SERVER_NAME]: { - type: "http", - url: init.blackboardMcp.url, - headers: { Authorization: `Bearer ${init.blackboardMcp.token}` }, - } as unknown as McpServerConfig, - } - : {}), + // Resolved per query build, not captured at construction — the + // orchestrator's mount is attached after its Session exists. + ...blackboardServerEntry(init.blackboardMcp?.()), // Registry servers — mounted natively on the SDK (claude owns its client); // tool calls surface as `mcp____` and gate via canUseTool. ...withMcpToolTimeout(registryServersForClaude(init.mcpRegistry), mcpToolTimeoutMs), diff --git a/src/daemon/providers/codex/index.ts b/src/daemon/providers/codex/index.ts index ee5b0a9..5d20718 100644 --- a/src/daemon/providers/codex/index.ts +++ b/src/daemon/providers/codex/index.ts @@ -94,12 +94,14 @@ export interface CodexProviderInit { */ memoryMcp?: MemoryMcpMount; /** - * Role-scoped goal-blackboard mount for a collaboration child. Unlike the - * memory mount, the token is minted ONCE by the SessionManager (it encodes - * this role's read/write scope) and revoked at collaboration teardown — so - * this provider carries it, it does not mint or revoke. + * Role-scoped goal-blackboard mount, resolved LAZILY. + * + * A getter rather than a value because the orchestrator's own mount is + * scoped to a goal id that IS its session id — so it cannot exist until + * after the Session is constructed. Providers read it when they build their + * server list (per turn for claude), by which time it is set. */ - blackboardMcp?: { url: string; token: string }; + blackboardMcp?: () => { url: string; token: string } | undefined; /** Cross-backend MCP registry — external servers mount natively via `-c * mcp_servers.*` (codex owns its client); approval flows through canUseTool. */ @@ -211,7 +213,7 @@ export class CodexProvider implements SessionProvider { #workspaceId: string; #memoryMcp: MemoryMcpMount | null; /** Role-scoped blackboard mount; token minted+revoked by the SessionManager. */ - readonly #blackboardMcp: { url: string; token: string } | null; + readonly #blackboardMcp: (() => { url: string; token: string } | undefined) | null; #mcpRegistry: McpRegistry | null; /** Live scoped token for the mounted memory endpoint; revoked on teardown. */ #memoryToken: string | null = null; @@ -339,7 +341,7 @@ export class CodexProvider implements SessionProvider { * session that may be recreated mid-goal. */ #blackboardMcpSpawn(): { args: string[]; env: Record } { - const mount = this.#blackboardMcp; + const mount = this.#blackboardMcp?.(); if (!mount) return { args: [], env: {} }; const key = `mcp_servers.${BLACKBOARD_MCP_SERVER_NAME}`; return { diff --git a/src/daemon/providers/registry.ts b/src/daemon/providers/registry.ts index 2ddd571..6c8760e 100644 --- a/src/daemon/providers/registry.ts +++ b/src/daemon/providers/registry.ts @@ -52,13 +52,9 @@ export interface ProviderSessionInit { /** Shared in-daemon memory MCP endpoint + URL — mounted by URL-based backends * (gemini-cli, later codex). Present only when memory is enabled. */ memoryMcp?: MemoryMcpMount; - /** - * Role-scoped goal-blackboard mount for a collaboration child (URL + a bearer - * token that IS the scope). URL-mounting backends surface it as an MCP - * server; that mountability is the whole reason it isn't an in-process - * Claude-SDK object (#245). - */ - blackboardMcp?: { url: string; token: string }; + /** Role-scoped goal-blackboard mount, resolved lazily — see the provider + * inits for why it is a getter and not a value. */ + blackboardMcp?: () => { url: string; token: string } | undefined; /** Cross-backend MCP registry — the servers to mount on this session's backend. */ mcpRegistry?: McpRegistry; /** Daemon-owned MCP client pool backing the registry (Model-B backends execute diff --git a/src/daemon/session-manager.ts b/src/daemon/session-manager.ts index 0d9b8f6..49af9d8 100644 --- a/src/daemon/session-manager.ts +++ b/src/daemon/session-manager.ts @@ -1594,6 +1594,31 @@ mcpHub: this.#mcpHub, this.#sessions.set(session.id, session); this.#rateLimiter.recordCreation(auth.sub); + // The orchestrator needs the blackboard too, and needs it MOST: §4 has it + // holding the index of artifact states, and §7 has it reading every + // reviewer's findings to synthesize. Without a mount it cannot see a + // single thing its children publish, and the coordination loop never + // closes. Attached here rather than passed to the constructor because the + // goal id it is scoped to IS this session's id. + if (collaboration) { + const orchestrator = orchestratorRole(collaboration); + const mount = this.#blackboardMountFor( + session, + { + roleName: ORCHESTRATOR_ROLE, + ordinal: 1, + providerId: session.providerId, + shape: "scout", + write: false, + }, + orchestrator, + ); + if (mount) { + session.attachBlackboard(mount); + this.#blackboardTokens.set(session.id, mount.token); + } + } + if (collaboration && planned.length > 0) { const spawned = await this.#spawnCollaborationChildren(session, collaboration, planned, auth); if (!spawned.ok) { @@ -1644,6 +1669,24 @@ mcpHub: this.#mcpHub, * pack constitution instead, so bringing up a fleet of N costs zero tokens * and no child burns a turn just to learn it should wait. */ + /** + * Revoke a collaboration child's blackboard token. Idempotent, and safe to + * call for any session id. + * + * Must be called from EVERY path that removes a child from `#sessions`, not + * just goal teardown. The token lives in the endpoint's binding map, not on + * the Session, so dropping the session without revoking leaves a credential + * that still authorizes reads and writes against the goal — anything still + * holding the URL (a wedged subprocess, a leaked env var) keeps working after + * the child is gone. Destroying a child directly used to do exactly that. + */ + #revokeBlackboardToken(sessionId: string): void { + const token = this.#blackboardTokens.get(sessionId); + if (!token) return; + this.#blackboardMcp.revoke(token); + this.#blackboardTokens.delete(sessionId); + } + /** Lazily build the blackboard over the daemon's existing DB connection. */ #goalBlackboard(): Blackboard { if (!this.#blackboard) { @@ -1740,6 +1783,17 @@ mcpHub: this.#mcpHub, ordinal: child.ordinal, write: child.write, }, + // Autonomous with a bounded budget — the same posture dispatch gives + // its workers, and for the same reason: NOBODY ATTACHES TO A CHILD. + // The owner's approval happens once at dispatch time (the R3 gate on + // fleet_send/fleet_spawn), not per tool call. Left interactive, a + // child's first non-safe tool call parks it at waiting_approval with + // zero clients and the collaboration deadlocks on its very first + // handoff — observed live before this was set. + initialMode: { + mode: "autonomous", + maxTurns: this.#dispatcher.config.workerToolBudget, + }, // Role-scoped goal blackboard, mountable by ANY backend (#245) — // this is how a gemini reviewer and an openai reasoner hand work to // each other without the orchestrator relaying it as prose. @@ -1795,14 +1849,7 @@ mcpHub: this.#mcpHub, `[codeoid/collaboration] child teardown failed (${reason}): ${err instanceof Error ? err.message : String(err)}`, ); } - // Revoke the blackboard token BEFORE dropping the session, so a mount - // that outlives teardown (a wedged subprocess still holding the URL) - // cannot keep reading the goal's artifacts. - const token = this.#blackboardTokens.get(child.id); - if (token) { - this.#blackboardMcp.revoke(token); - this.#blackboardTokens.delete(child.id); - } + this.#revokeBlackboardToken(child.id); this.#sessions.delete(child.id); this.#store.audit( "system:collaboration", @@ -2705,6 +2752,16 @@ mcpHub: this.#mcpHub, `target session ${task.targetSession ?? "?"} no longer exists`, ); } + // Re-arm a collaboration child's autonomous budget on every dispatch. + // A child is long-lived across the whole goal (unlike a disposable + // spawn worker), so one initial budget is spent down across successive + // dispatches and the child would silently wedge at waiting_approval + // partway through — with nobody attached to approve. Same reasoning as + // continueWorker re-arming after a restart; the approval that + // authorizes this work already happened at the R3 dispatch gate. + if (target.collaborationRole) { + target.setMode("autonomous", this.#dispatcher.config.workerToolBudget); + } await target.send( `[conductor dispatch ${task.id.slice(0, 8)} — owner-approved]\n\n${task.prompt}`, this.#dispatchSenderAuth(task), @@ -2850,6 +2907,9 @@ mcpHub: this.#mcpHub, `[codeoid/dispatch] worker teardown failed (${reason}): ${err instanceof Error ? err.message : String(err)}`, ); } + // A collaboration child is role:"worker", so it can reach this path. + // Revoke before dropping, same reason as the destroy handler. + this.#revokeBlackboardToken(sessionId); this.#sessions.delete(sessionId); }, @@ -3715,6 +3775,8 @@ mcpHub: this.#mcpHub, // reach them by. if (session.collaboration) { await this.#teardownCollaborationChildren(msg.sessionId, "collaboration goal ended"); + // ...and the orchestrator's own mount. + this.#revokeBlackboardToken(msg.sessionId); // Artifacts are goal-scoped, so they die with the goal. Dropped AFTER the // children so a child mid-write can't recreate rows behind the delete. try { @@ -3731,6 +3793,10 @@ mcpHub: this.#mcpHub, } await session.destroy(auth); + // Covers destroying a CHILD directly (not via goal teardown) — without + // this its token stays live in the endpoint's binding map and keeps + // authorizing reads/writes on the goal after the session is gone. + this.#revokeBlackboardToken(msg.sessionId); this.#sessions.delete(msg.sessionId); return { type: "response.ok", requestId: msg.id }; } diff --git a/src/daemon/session.ts b/src/daemon/session.ts index 8003c95..cb80f1f 100644 --- a/src/daemon/session.ts +++ b/src/daemon/session.ts @@ -339,8 +339,10 @@ export class Session { readonly collaboration?: CollaborationConfig; /** Which collaboration + role this session serves, when it is a child. */ readonly collaborationRole?: SessionInfo["collaborationRole"]; - /** Role-scoped blackboard mount (URL + scope-bearing token), when a child. */ - readonly #blackboardMcp?: { url: string; token: string }; + /** Role-scoped blackboard mount. NOT readonly: the orchestrator's own mount + * is scoped to a goal id that IS this session's id, so it can only be + * attached after construction (see attachBlackboard). */ + #blackboardMcp?: { url: string; token: string }; readonly createdBy: string; readonly createdAt: string; /** @@ -835,7 +837,9 @@ export class Session { identityManager: this.#identityManager, memory: this.#memory, memoryMcp: this.#memoryMcp, - blackboardMcp: this.#blackboardMcp, + // A getter, so a mount attached after construction still reaches the + // provider when it next builds its server list. + blackboardMcp: () => this.#blackboardMcp, mcpRegistry: this.#mcpRegistry, mcpHub: this.#mcpHub, fleet: this.#fleet, @@ -2244,6 +2248,17 @@ export class Session { } } + /** + * Attach this session's role-scoped blackboard mount. + * + * Used for the ORCHESTRATOR, whose goal id is its own session id and so + * cannot be known before construction. Providers resolve the mount lazily, + * so one attached before the first turn is picked up normally. + */ + attachBlackboard(mount: { url: string; token: string }): void { + this.#blackboardMcp = mount; + } + toInfo(): SessionInfo { return { id: this.id, diff --git a/src/tests/collaboration.test.ts b/src/tests/collaboration.test.ts index 32acc8d..6185788 100644 --- a/src/tests/collaboration.test.ts +++ b/src/tests/collaboration.test.ts @@ -740,6 +740,79 @@ describe("collaboration children come up and are torn down", () => { }); }); +// ── Regressions found by the live smoke test + audit ──────────────────────── +// +// Every one of these passed CI and the unit suite while the feature was +// actually broken end to end. They exist because "the tests are green" was not +// the same as "an agent can complete a handoff". +describe("live-verified wiring", () => { + const CONFIG: CollaborationConfig = { + goal: "Ship it", + roles: [ + { name: "orchestrator", providerId: "claude" }, + { name: "search", providerId: "claude" }, + ], + }; + + const createCollab = async (id: string, name: string) => { + manager.setBlackboardUrl("http://127.0.0.1:7400/mcp/blackboard"); + const resp = await run({ + type: "session.create", + id, + name, + workdir, + collaboration: CONFIG, + }); + if (resp.type !== "response.ok") throw new Error("create failed"); + return resp.data as SessionInfo; + }; + + // Observed live: children spawned interactive, `blackboard_write` needs + // approval, and NOBODY attaches to a child — so the first handoff parked at + // waiting_approval with zero clients and the collaboration deadlocked. + test("children spawn autonomous, because no one is there to approve", async () => { + const parent = await createCollab("lv1", "lv1"); + const kids = childrenOf(await allSessions(), parent.id); + expect(kids).toHaveLength(1); + expect(kids[0]!.mode).toBe("autonomous"); + expect(kids[0]!.turnsRemaining).toBeGreaterThan(0); + }); + + // Observed by audit: only children got a mount, so the orchestrator could + // not call blackboard_index or read findings — §4's index and §7's synthesis + // were both impossible and the coordination loop never closed. + // The orchestrator is the one that MOST needs the blackboard (§4 index, §7 + // synthesis) and originally got no mount at all. Paired with the teardown + // test below, which fails if the mount is minted but never registered. + test("a token is minted for the orchestrator as well as each child", async () => { + const before = manager.blackboardMcp.activeTokens; + const parent = await createCollab("lv2", "lv2"); + const kids = childrenOf(await allSessions(), parent.id); + expect(manager.blackboardMcp.activeTokens).toBe(before + kids.length + 1); + }); + + // Observed by audit: destroying a child directly skipped revocation, leaving + // a credential that still authorized reads/writes on the goal. + test("destroying a child directly revokes its token", async () => { + const parent = await createCollab("lv3", "lv3"); + const kid = childrenOf(await allSessions(), parent.id)[0]!; + const withChild = manager.blackboardMcp.activeTokens; + + const destroyed = await run({ type: "session.destroy", id: "lv3d", sessionId: kid.id }); + expect(destroyed.type).toBe("response.ok"); + expect(manager.blackboardMcp.activeTokens).toBe(withChild - 1); + }); + + test("goal teardown revokes the orchestrator's token too", async () => { + const before = manager.blackboardMcp.activeTokens; + const parent = await createCollab("lv4", "lv4"); + expect(manager.blackboardMcp.activeTokens).toBeGreaterThan(before); + + await run({ type: "session.destroy", id: "lv4d", sessionId: parent.id }); + expect(manager.blackboardMcp.activeTokens).toBe(before); + }); +}); + // A collaborative session IS its orchestrator, so the claude-only rule has to // bind THIS session's backend — not just a config row that nothing runs on. describe("the session is its orchestrator", () => { From 8663dd908900d5ec2e0ec82362e7e189d9b1e8d4 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Mon, 27 Jul 2026 12:13:09 +0800 Subject: [PATCH 5/5] feat: collaborative-session create in the web UI (blackboard front door) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Until now a collaboration could only be created from the CLI — the web create dialog sent name/workdir/providerId/pack/packRole and had no notion of `collaboration` at all, so the browser could not start one. This is §9's front door. Extends the EXISTING dialog rather than adding a panel, because a collaboration is a session plus a goal and role bindings, not a separate object. The modal already served two flows (session, pipeline); this makes it three, with a Single agent ↔ Collaborative toggle. Pipeline stays externally-entered from /pipeline, so it isn't offered in the toggle. Role rows: name, backend, model, ×count, "can write". Design choices that mirror what the daemon enforces, so the UI can't offer something that will be rejected: - The orchestrator row is present, non-removable and claude-pinned. The daemon requires exactly one orchestrator, the session being created IS it, and claude is the only backend that mounts the fleet MCP server in v1 (#245). A removable or re-pointable row would just produce a server-side error. - "can write" defaults OFF and isn't offered for the orchestrator. Read-only is the §3 default and the daemon backs it with an identity holding no write scope — the checkbox's tooltip says exactly that, since "can write" alone reads like a soft preference. - No top-level providerId is ever sent. A collaborative session IS its orchestrator, so the daemon derives the backend from that role and REJECTS a conflicting explicit value — sending one would fail every create. - Optional fields are omitted rather than sent empty: count:1 and write:false are daemon defaults, and an empty-string model would fail provider-aware validation. Local pre-flight (goal present, names non-empty, no duplicates, an orchestrator exists) only spares a round-trip; the daemon re-validates everything and its error is surfaced verbatim, because it names the precise rule broken. The unmet condition is shown under the form — a disabled Create with no explanation is the worst version of this. Tests: +8 web tests, mutation-checked — sending a top-level providerId fails one, and sending count at its default fails another. Web 319 pass, daemon 2044 pass / 0 fail, typecheck + biome + both builds clean. Still not rendered: children appear as separate sessions with no grouping, no read-only badge, and no artifact view, though `collaborationRole` is already on the wire. That is the other half of §9 and wants its own change. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Yash Datta --- web/src/components/NewSessionModal.test.tsx | 132 +++++++- web/src/components/NewSessionModal.tsx | 314 +++++++++++++++++++- 2 files changed, 432 insertions(+), 14 deletions(-) diff --git a/web/src/components/NewSessionModal.test.tsx b/web/src/components/NewSessionModal.test.tsx index d47f5de..590e4a2 100644 --- a/web/src/components/NewSessionModal.test.tsx +++ b/web/src/components/NewSessionModal.test.tsx @@ -40,7 +40,11 @@ const runPipelineMock = vi.hoisted(() => vi.fn(() => Promise.resolve())); vi.mock("../state/pipelines", () => ({ runPipeline: runPipelineMock })); import type { PackWire } from "../protocol/types"; -import NewSessionModal, { openNewSessionModal, openPipelineModal } from "./NewSessionModal"; +import NewSessionModal, { + openCollaborateModal, + openNewSessionModal, + openPipelineModal, +} from "./NewSessionModal"; import { _resetSessionsForTest } from "../state/sessions"; /** Minimal installed PackWire for the modal's pack/role selectors. */ @@ -274,3 +278,129 @@ describe("NewSessionModal pipeline mode", () => { expect((getByText("start run") as HTMLButtonElement).disabled).toBe(true); }); }); + + +describe("NewSessionModal collaborative mode", () => { + /** Open in collaborative mode with a name filled in. */ + function openCollab(providers = ["claude", "gemini"]) { + authMock.mockReturnValue(authOk(providers)); + requestMock.mockResolvedValue({ id: "s-collab", name: "demo", workdir: "/w" }); + const r = render(() => ); + openCollaborateModal(); + fireEvent.input(r.getByPlaceholderText("e.g. shield-refactor"), { + target: { value: "demo" }, + }); + return r; + } + + const goalBox = (r: ReturnType) => + r.getByPlaceholderText( + "The one goal every role works on — e.g. Add rate limiting to the public API", + ); + + it("sends a collaboration with the default orchestrator + worker profile", async () => { + const r = openCollab(); + fireEvent.input(goalBox(r), { target: { value: "Ship rate limiting" } }); + fireEvent.click(r.getByText("create collaboration")); + + await waitFor(() => expect(requestMock).toHaveBeenCalled()); + const sent = requestMock.mock.calls[0]![0] as Record; + expect(sent.type).toBe("session.create"); + const collab = sent.collaboration as { goal: string; roles: Array> }; + expect(collab.goal).toBe("Ship rate limiting"); + expect(collab.roles.map((x) => x.name)).toEqual(["orchestrator", "search"]); + // The orchestrator is claude-pinned in v1 (#245). + expect(collab.roles[0]!.providerId).toBe("claude"); + }); + + // A collaborative session IS its orchestrator, so the daemon derives the + // backend from that role and REJECTS a conflicting explicit providerId. + // Sending one would turn every create into an error. + it("never sends a top-level providerId", async () => { + const r = openCollab(); + fireEvent.input(goalBox(r), { target: { value: "g" } }); + fireEvent.click(r.getByText("create collaboration")); + await waitFor(() => expect(requestMock).toHaveBeenCalled()); + const sent = requestMock.mock.calls[0]![0] as Record; + expect("providerId" in sent).toBe(false); + }); + + it("omits optional fields rather than sending empty values", async () => { + const r = openCollab(); + fireEvent.input(goalBox(r), { target: { value: "g" } }); + fireEvent.click(r.getByText("create collaboration")); + await waitFor(() => expect(requestMock).toHaveBeenCalled()); + const sent = requestMock.mock.calls[0]![0] as Record; + const roles = (sent.collaboration as { roles: Array> }).roles; + // count:1 and write:false are the daemon's defaults — sending them adds + // noise, and an empty-string model would fail provider-aware validation. + for (const role of roles) { + expect("model" in role).toBe(false); + expect("count" in role).toBe(false); + expect("write" in role).toBe(false); + } + }); + + it("blocks submit until a goal is given, and says why", async () => { + const r = openCollab(); + expect(r.getByText("a goal is required")).toBeTruthy(); + expect((r.getByText("create collaboration") as HTMLButtonElement).disabled).toBe(true); + fireEvent.input(goalBox(r), { target: { value: "g" } }); + await waitFor(() => + expect((r.getByText("create collaboration") as HTMLButtonElement).disabled).toBe(false), + ); + }); + + it("rejects duplicate role names before hitting the daemon", async () => { + const r = openCollab(); + fireEvent.input(goalBox(r), { target: { value: "g" } }); + fireEvent.click(r.getByText("+ add role")); + const nameInputs = r.getAllByLabelText("Role name") as HTMLInputElement[]; + // Row 0 is the orchestrator (locked); rename the two workers to collide. + fireEvent.input(nameInputs[1]!, { target: { value: "review" } }); + fireEvent.input(nameInputs[2]!, { target: { value: "Review" } }); + await waitFor(() => expect(r.getByText('duplicate role "Review"')).toBeTruthy()); + expect(requestMock).not.toHaveBeenCalled(); + }); + + it("keeps the orchestrator row non-removable and claude-pinned", () => { + const r = openCollab(); + const removes = r.getAllByLabelText("Remove role") as HTMLButtonElement[]; + expect(removes[0]!.disabled).toBe(true); + const backends = r.getAllByLabelText("Backend") as HTMLSelectElement[]; + expect(backends[0]!.disabled).toBe(true); + expect(backends[0]!.value).toBe("claude"); + }); + + it("carries model, count and write when set", async () => { + const r = openCollab(); + fireEvent.input(goalBox(r), { target: { value: "g" } }); + const models = r.getAllByLabelText("Model") as HTMLInputElement[]; + fireEvent.input(models[1]!, { target: { value: "gemini-2.5-pro" } }); + const counts = r.getAllByLabelText("Count") as HTMLInputElement[]; + fireEvent.input(counts[0]!, { target: { value: "3" } }); + const writes = r.container.querySelectorAll('input[type="checkbox"]'); + fireEvent.click(writes[0]!); + + fireEvent.click(r.getByText("create collaboration")); + await waitFor(() => expect(requestMock).toHaveBeenCalled()); + const sent = requestMock.mock.calls[0]![0] as Record; + const worker = (sent.collaboration as { roles: Array> }).roles[1]!; + expect(worker.model).toBe("gemini-2.5-pro"); + expect(worker.count).toBe(3); + expect(worker.write).toBe(true); + }); + + it("a plain session still sends no collaboration", async () => { + authMock.mockReturnValue(authOk(["claude"])); + requestMock.mockResolvedValue({ id: "s", name: "n", workdir: "/w" }); + const r = render(() => ); + openNewSessionModal(); + fireEvent.input(r.getByPlaceholderText("e.g. shield-refactor"), { + target: { value: "plain" }, + }); + fireEvent.click(r.getByText("create")); + await waitFor(() => expect(requestMock).toHaveBeenCalled()); + expect("collaboration" in (requestMock.mock.calls[0]![0] as object)).toBe(false); + }); +}); diff --git a/web/src/components/NewSessionModal.tsx b/web/src/components/NewSessionModal.tsx index 8b5bcd8..75e5cf2 100644 --- a/web/src/components/NewSessionModal.tsx +++ b/web/src/components/NewSessionModal.tsx @@ -28,9 +28,36 @@ import { focusSession, mergeSession, sessionList } from "../state/sessions"; import type { PackWire, SessionInfo } from "../protocol/types"; import DirectoryPicker from "./files/DirectoryPicker"; -/** The modal serves two flows from one dialog (docs/pipeline-run.md): a plain - * session, or a governed pipeline run (adds a goal box + requires a pack). */ -type Mode = "session" | "pipeline"; +/** The modal serves three flows from one dialog: a plain session, a governed + * pipeline run (docs/pipeline-run.md — adds a goal box + requires a pack), or + * a COLLABORATIVE session (docs/collaborative-session-design.md §9 — a goal + * plus role→backend bindings, which the daemon compiles to an ephemeral + * one-goal pack; pack vocabulary deliberately stays hidden on that path). */ +type Mode = "session" | "pipeline" | "collaborate"; + +/** One editable role row. Kept as strings so partially-typed input renders; + * it is normalized into the wire shape at submit. */ +interface CollabRoleRow { + name: string; + /** "" = the daemon default backend. */ + providerId: string; + /** "" = that backend's own default model. */ + model: string; + count: number; + /** Opt-in write authority. Default OFF — §3 gives review/search no repo + * write, and a read-only child's identity carries no write scope at all. */ + write: boolean; +} + +/** The §3 starting profile: an orchestrator plus one worker. The orchestrator + * is claude-pinned in v1 (#245) — it is the only backend that mounts the + * fleet MCP server, and the daemon rejects anything else. */ +function defaultRoles(): CollabRoleRow[] { + return [ + { name: "orchestrator", providerId: "claude", model: "", count: 1, write: false }, + { name: "search", providerId: "", model: "", count: 1, write: false }, + ]; +} const [openSignal, setOpenSignal] = createSignal(false); const [mode, setMode] = createSignal("session"); @@ -45,6 +72,14 @@ export function openNewSessionModal(): void { /** Open the SAME dialog in pipeline mode: a goal / feature box + a required pack. * Submitting starts a governed run and focuses its bound session (the run shows * up as a normal chat). Wired to `/pipeline` and the Pack Browser's Run action. */ +/** Open the dialog in COLLABORATIVE mode: one goal worked by several + * role-children on their own backends. */ +export function openCollaborateModal(goal?: string): void { + setMode("collaborate"); + setGoalPrefill(goal ?? ""); + setOpenSignal(true); +} + export function openPipelineModal(goal?: string): void { setMode("pipeline"); setGoalPrefill(goal ?? ""); @@ -66,6 +101,38 @@ const NewSessionModal: Component = () => { // Capability role declared by the chosen pack; "" = no role restriction. const [packRole, setPackRole] = createSignal(""); + // ── Collaborative mode ──────────────────────────────────────────────────── + // One row per role. The orchestrator is always present and is NOT removable: + // the daemon requires exactly one, and the session being created IS it. + const [roles, setRoles] = createSignal(defaultRoles()); + + const updateRole = (i: number, patch: Partial): void => { + setRoles((rs) => rs.map((r, j) => (j === i ? { ...r, ...patch } : r))); + }; + const addRole = (): void => { + setRoles((rs) => [...rs, { name: "", providerId: "", model: "", count: 1, write: false }]); + }; + const removeRole = (i: number): void => { + setRoles((rs) => rs.filter((_, j) => j !== i)); + }; + + /** Client-side pre-flight. The daemon re-validates everything — this only + * spares a round-trip on the mistakes that are obvious locally. */ + const collabProblem = createMemo(() => { + if (mode() !== "collaborate") return null; + if (!goal().trim()) return "a goal is required"; + const named = roles().filter((r) => r.name.trim()); + if (named.length !== roles().length) return "every role needs a name"; + const seen = new Set(); + for (const r of named) { + const key = r.name.trim().toLowerCase(); + if (seen.has(key)) return `duplicate role "${r.name.trim()}"`; + seen.add(key); + } + if (!seen.has("orchestrator")) return 'one role must be named "orchestrator"'; + return null; + }); + // Backends this daemon registered (auth.ok `providers`, default first). // Older daemons don't advertise — hide the picker, sessions stay claude. const providers = createMemo(() => authIdentity()?.providers ?? []); @@ -141,7 +208,8 @@ const NewSessionModal: Component = () => { if (v) { setBusy(false); setError(null); - if (mode() === "pipeline") setGoal(goalPrefill()); + if (mode() === "pipeline" || mode() === "collaborate") setGoal(goalPrefill()); + if (mode() === "collaborate") setRoles(defaultRoles()); // Refresh the pack list every open. fetchPacks swallows its own // errors (it sets pack-state.error rather than rejecting), but guard // anyway so a rejected read can never break opening the modal. @@ -195,6 +263,63 @@ const NewSessionModal: Component = () => { return; } + // ── Collaborative session ───────────────────────────────────────────────── + if (mode() === "collaborate") { + const problem = collabProblem(); + if (problem) { + setError(problem); + return; + } + if (!n) { + setError("name required"); + return; + } + setBusy(true); + setError(null); + try { + const data = (await request({ + type: "session.create", + id: newRequestId(), + name: n, + workdir: workdir().trim() || ".", + collaboration: { + goal: goal().trim(), + roles: roles().map((r) => ({ + name: r.name.trim(), + // A blank picker means "daemon default"; the wire field is + // required, so resolve it to the advertised default here. + providerId: r.providerId || providers()[0] || "claude", + ...(r.model.trim() ? { model: r.model.trim() } : {}), + ...(r.count > 1 ? { count: r.count } : {}), + ...(r.write ? { write: true } : {}), + })), + }, + // providerId is deliberately NOT sent: a collaborative session IS its + // orchestrator, so the daemon derives the backend from that role and + // rejects a conflicting explicit value. + })) as SessionInfo | undefined; + if (data && typeof data === "object" && "id" in data) { + mergeSession(data); + focusSession(data.id); + } else { + await refreshSessions().catch(() => []); + } + setBusy(false); + setOpenSignal(false); + setName(""); + setWorkdir(""); + setGoal(""); + setRoles(defaultRoles()); + } catch (err) { + // The daemon's message is the useful one here — it names the exact + // rule broken (unknown provider, non-claude orchestrator, over the + // child ceiling), so surface it verbatim rather than paraphrasing. + setError(err instanceof Error ? err.message : String(err)); + setBusy(false); + } + return; + } + // ── Plain session ───────────────────────────────────────────────────────── if (!n) { setError("name required"); @@ -251,14 +376,46 @@ const NewSessionModal: Component = () => { onSubmit={submit} class="mt-[16vh] w-full max-w-md space-y-4 rounded-lg border border-border bg-bg-elev p-5 shadow-2xl" > -
+

- {mode() === "pipeline" ? "Start a pipeline run" : "New session"} + {mode() === "pipeline" + ? "Start a pipeline run" + : mode() === "collaborate" + ? "New collaborative session" + : "New session"}

+ {/* Plain ↔ collaborative is a toggle on the SAME dialog (§9): a + collaboration is a session plus a goal and role bindings, not a + separate object. Pipeline mode is entered from /pipeline, so it + isn't offered here. */} + +
+ + {(m) => ( + + )} + +
+

{mode() === "pipeline" ? "Run an installed pack against a goal. It creates a session, auto-advances through the pack's phases, and halts at each boundary for you to Approve / Revise / Reject." - : "A session is one Claude conversation rooted at a workdir. The daemon registers a per-session ZeroID agent identity automatically."} + : mode() === "collaborate" + ? "One goal, several agents in named roles — each on its own backend. This session is the orchestrator; the others come up as its children and hand work to each other through a shared goal blackboard." + : "A session is one Claude conversation rooted at a workdir. The daemon registers a per-session ZeroID agent identity automatically."}

@@ -335,14 +492,18 @@ const NewSessionModal: Component = () => {

- +