From b440e6ae3235464c4c9e667ff92a5ac609dd5b03 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sun, 26 Jul 2026 23:35:19 +0800 Subject: [PATCH] feat: bring up collaboration role-children (collab sessions P1b) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes P1 of docs/collaborative-session-design.md §11. P1a landed the contract; this makes it do something. Creating a collaborative session now brings up its role-children on their own backends, and destroying it tears them down — P1's stated exit criterion, end to end. Key structural decision: a role-child is a normal long-lived session, NOT a dispatch-spawned disposable worker. The dispatcher destroys a spawn-task worker the moment its turn ends (#finishWorkerTask), which is exactly wrong for a role that has to survive the implement↔review fix-loop. A `kind:"send"` dispatch, by contrast, delivers and completes without taking ownership of the target's lifetime — so children receive fleet_send and the collaboration owns teardown. That means per-goal lifetime needs NO change to the disposable-worker path, which stays as it is for fleet_spawn. Second decision: no brief is SENT at spawn. A child's role, contract, and goal ride in its compiled 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. - protocol: `write?: boolean` on CollaborationRole — absent = read-only. The default is the point: §3 gives review/search no repo write and §6 wants a reviewer that *provably* cannot write, so write authority is opt-in and enforced two ways, not requested in a prompt. Plus `collaborationRole` on SessionInfo (parent id, role, ordinal, write) — the mirror of `collaboration`, so a client can group a fleet without parsing names. - collaboration.ts: planChildren (flattens fan-out to ordinals, EXCLUDES the orchestrator since the session itself plays it), compileGoalPack (the ephemeral one-goal pack for the orchestrator — synthetic id, never installed, so pack vocabulary stays hidden per §9), childBrief, childSessionName. - MAX_COLLABORATION_CHILDREN = 12. The schema bounds multiply: 15 worker roles × 8 fan-out is 120 agent subprocesses from one session.create. The real governor is P3's live-worker cap; this is the blast-radius backstop that has to exist before anything spawns. Rejects rather than truncating — a collaboration quietly missing a reviewer is worse than one that refused. - session-manager: children planned BEFORE the parent is created, so an over-ceiling request builds nothing; all-or-nothing spawn with rollback of parent + siblings, since an orchestrator with a partial fleet would delegate to children that don't exist; cascade teardown on destroy, children first so OK is never returned over live orphans. Membership is DERIVED from the live session set rather than a side registry that could drift and orphan a subprocess. collaboration + pack now rejected as mutually exclusive — two topologies competing for one constitution. - Read-only enforcement is doubled up: shape "scout" gives a LEAF identity with no tools:write (so it cannot mint write authority even via a sub-agent), and the child's RoleDef makes roleDeniesTool hard-deny write tools at the canUseTool fence. `network: "read-only"` not false, or the search role loses the web tools §3 gives it. Tests: +13. Asserts children land on the backend their role named, ordinals, orchestrator exclusion, read-only default, cascade teardown, pre-create rejection, and that the envelope genuinely denies Write/Edit/MultiEdit/ NotebookEdit while still permitting Read/Grep/Bash/WebSearch — the security claim, checked rather than labelled. Mutation-checked: dropping the per-child providerId fails the backend test; dropping the cascade fails the teardown test. Suite 1915 pass / 0 fail, typecheck + biome + build clean. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Yash Datta --- packages/protocol/src/schemas.ts | 2 + packages/protocol/src/types.ts | 31 ++++ src/daemon/collaboration.ts | 157 ++++++++++++++++++ src/daemon/session-manager.ts | 207 +++++++++++++++++++++++- src/daemon/session.ts | 14 ++ src/daemon/transcript.ts | 8 + src/tests/collaboration.test.ts | 263 +++++++++++++++++++++++++++++++ 7 files changed, 679 insertions(+), 3 deletions(-) diff --git a/packages/protocol/src/schemas.ts b/packages/protocol/src/schemas.ts index 86c7ae6..cb8c6c3 100644 --- a/packages/protocol/src/schemas.ts +++ b/packages/protocol/src/schemas.ts @@ -44,6 +44,8 @@ export const collaborationRoleSchema = z.object({ model: z.string().max(LIMITS.MODEL_MAX).optional(), count: z.number().int().min(1).max(LIMITS.COLLABORATION_ROLE_COUNT_MAX).optional(), purpose: z.string().max(500).optional(), + /** Opt-in write authority. Absent = read-only (least privilege). */ + write: z.boolean().optional(), }); export const collaborationConfigSchema = z.object({ diff --git a/packages/protocol/src/types.ts b/packages/protocol/src/types.ts index e19e434..7d0c529 100644 --- a/packages/protocol/src/types.ts +++ b/packages/protocol/src/types.ts @@ -256,6 +256,24 @@ export interface SessionInfo { * survives a daemon restart the way `role`/`providerId` already do. */ collaboration?: CollaborationConfig; + /** + * Set on a role-CHILD of a collaborative session: which collaboration it + * belongs to and which role it plays. Absent = not a collaboration child. + * + * The mirror of `collaboration` (set on the parent), so a client can group + * a fleet without inferring it from names. `ordinal` distinguishes the + * members of a fanned-out role (`review` ×3 → ordinals 1..3). + */ + collaborationRole?: { + /** Session id of the orchestrating parent. */ + parentSessionId: string; + /** Role name from the parent's config (already lowercased). */ + roleName: string; + /** 1-based index within this role's fan-out. */ + ordinal: number; + /** Whether this child's identity carries write authority. */ + write: boolean; + }; } /** A git worktree backing a session's workdir (see SessionInfo.worktree). */ @@ -852,6 +870,19 @@ export interface CollaborationRole { count?: number; /** What this role is for; surfaced in the child's brief. */ purpose?: string; + /** + * Whether this role's children may modify the workspace. + * + * **Absent = false**, and that default is the point. §3 gives `review` and + * `search` no repo write, and §6 wants a reviewer that *provably* cannot + * write rather than one asked not to — so write authority is opt-in per + * role and is enforced by the child's leaf identity holding no + * `tools:write` scope at all, not by a line in a prompt. + * + * Maps onto the existing dispatch worker shapes: `true` → "ship", + * `false` → "scout". + */ + write?: boolean; } /** The role name that must be present exactly once in a collaboration, and diff --git a/src/daemon/collaboration.ts b/src/daemon/collaboration.ts index cb3c71c..f7638d9 100644 --- a/src/daemon/collaboration.ts +++ b/src/daemon/collaboration.ts @@ -125,6 +125,9 @@ export function validateCollaboration( ...(model !== undefined ? { model } : {}), count, ...(raw.purpose !== undefined ? { purpose: raw.purpose } : {}), + // Normalize to an explicit boolean so downstream code never has to + // re-decide what "absent" means for write authority. + write: raw.write === true, }); } @@ -179,6 +182,160 @@ export function orchestratorRole( return config.roles.find((r) => r.name.toLowerCase() === ORCHESTRATOR_ROLE); } +// ── Role → children (P1b) ─────────────────────────────────────────────────── + +/** + * Ceiling on the total children one collaboration may bring up. + * + * The per-role and per-collaboration schema bounds multiply: 15 worker roles + * × 8 fan-out is 120 live agent subprocesses from a single `session.create`. + * The real concurrency governor is the live-worker cap in P3; this is the + * blast-radius backstop that must exist BEFORE anything spawns, because + * without it one create request can exhaust the machine. + */ +export const MAX_COLLABORATION_CHILDREN = 12; + +/** One child to bring up: a role instance bound to a backend. */ +export interface PlannedChild { + roleName: string; + /** 1-based index within this role's fan-out (`review` ×3 → 1, 2, 3). */ + ordinal: number; + providerId: string; + model?: string; + /** Dispatch worker shape, derived from the role's write authority. */ + shape: "ship" | "scout"; + write: boolean; + purpose?: string; +} + +/** + * Flatten a validated config into the children to spawn. + * + * The orchestrator is deliberately EXCLUDED: the collaborative session itself + * is the orchestrator (that is why `#create` derives its provider from this + * role), so spawning a child for it would double it. + * + * Fails rather than truncating when the total exceeds the ceiling — silently + * dropping roles would give the caller a collaboration quietly missing a + * reviewer, which is worse than a clear rejection. + */ +export function planChildren( + config: CollaborationConfig, +): + | { ok: true; children: PlannedChild[] } + | { ok: false; error: string } { + const children: PlannedChild[] = []; + for (const role of config.roles) { + if (role.name.toLowerCase() === ORCHESTRATOR_ROLE) continue; + const count = role.count ?? 1; + for (let ordinal = 1; ordinal <= count; ordinal++) { + children.push({ + roleName: role.name, + ordinal, + providerId: role.providerId, + ...(role.model !== undefined ? { model: role.model } : {}), + // scout holds no tools:write — see WORKER_SCOPE_PROFILES. This is the + // enforcement behind §6's "a reviewer that provably cannot write". + shape: role.write === true ? "ship" : "scout", + write: role.write === true, + ...(role.purpose !== undefined ? { purpose: role.purpose } : {}), + }); + } + } + if (children.length > MAX_COLLABORATION_CHILDREN) { + return { + ok: false, + error: `Collaboration would spawn ${children.length} children — max ${MAX_COLLABORATION_CHILDREN}. Reduce role count or fan-out.`, + }; + } + return { ok: true, children }; +} + +/** Stable display name for a child session. */ +export function childSessionName(parentName: string, child: PlannedChild): string { + const suffix = child.ordinal > 1 ? `-${child.ordinal}` : ""; + return `${parentName}:${child.roleName}${suffix}`; +} + +/** + * The goal brief handed to a role-child on spawn. + * + * Deliberately narrow. A child is told its goal, its role, and its contract — + * never how the other roles are doing, and never the orchestrator's + * reasoning. §6's independence property depends on a reviewer not seeing the + * implementer's thinking, and the cheapest way to honor that is to not put it + * in the brief in the first place. Structured handoffs arrive through the + * blackboard in the next phase, scoped per role. + */ +export function childBrief( + config: CollaborationConfig, + child: PlannedChild, +): string { + 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."; + 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.", + "Wait for instructions from the orchestrator before acting; it will send you a specific task.", + "", + "", + `GOAL: ${config.goal}`, + ] + .filter((l) => l !== null) + .join("\n"); +} + +/** + * Compile a collaboration into the ephemeral one-goal pack activation the + * orchestrator session runs under (§9: the toggle "compiles to an ephemeral + * one-goal pack" and pack vocabulary stays hidden on this path). + * + * `id` is synthetic and never installed on disk — it exists so `SessionInfo. + * profile` reads sensibly and so the pipeline machinery, which already keys + * off an activation, needs no special case for collaborations. + */ +export function compileGoalPack( + config: CollaborationConfig, + children: readonly PlannedChild[], +): { id: string; constitution: string; subagents: [] } { + const roster = children + .map( + (c) => + `- ${c.roleName}${c.ordinal > 1 ? ` #${c.ordinal}` : ""} — ${c.providerId}${c.model ? `/${c.model}` : ""}, ${c.write ? "may write" : "read-only"}`, + ) + .join("\n"); + return { + id: "collaboration", + constitution: [ + "# Collaborative session", + "", + "You are the ORCHESTRATOR of a collaborative session working ONE goal:", + "", + config.goal, + "", + "## Your role", + "", + "You plan, delegate, and synthesize. You do NOT do the work yourself — that is what your role-children are for.", + "Direct them with the fleet tools; each dispatch needs the owner's approval, and the owner sees your exact tool input first.", + "", + "## Your fleet", + "", + roster || "(no role-children — this collaboration declared only an orchestrator)", + "", + "## Rules", + "", + "- A read-only child CANNOT edit files; its identity holds no write scope. Don't ask it to.", + "- Children cannot see each other's work or your reasoning. When a role needs another's output, you pass it deliberately.", + "- Reviewers must stay independent: give them the change and the goal, never the implementer's reasoning or another reviewer's findings.", + ].join("\n"), + subagents: [], + }; +} + /** * Parse one `--role` CLI spec into a `CollaborationRole`. * diff --git a/src/daemon/session-manager.ts b/src/daemon/session-manager.ts index e7a60ba..e449096 100644 --- a/src/daemon/session-manager.ts +++ b/src/daemon/session-manager.ts @@ -38,7 +38,15 @@ import { resolveAgainstList, resolveModelIdForProvider, } from "./models.js"; -import { orchestratorRole, validateCollaboration } from "./collaboration.js"; +import { + childBrief, + childSessionName, + compileGoalPack, + orchestratorRole, + planChildren, + validateCollaboration, + type PlannedChild, +} from "./collaboration.js"; import { packSession, unpackBundle, @@ -1370,10 +1378,10 @@ mcpHub: this.#mcpHub, return this.#config?.conductor?.name ?? "conductor"; } - #create( + async #create( msg: Extract, auth: AuthContext, - ): DaemonMessage { + ): Promise { if (!hasScope(auth.scopes as string[], SCOPES.SESSION_CREATE)) { return { type: "response.error", requestId: msg.id, error: "Missing scope: session:create", code: "forbidden" }; } @@ -1445,6 +1453,18 @@ mcpHub: this.#mcpHub, }; } collaboration = checked.config; + // Two different topologies competing for one constitution: the + // collaborative toggle compiles its OWN ephemeral one-goal pack (§9), + // so an installed pack would either be silently overridden or silently + // override it. Neither is acceptable — say so instead. + if (msg.pack) { + return { + type: "response.error", + requestId: msg.id, + error: "collaboration and pack are mutually exclusive — a collaborative session compiles its own one-goal pack. Use /pipeline for a pre-authored pack.", + code: "invalid_request", + }; + } const orchestrator = orchestratorRole(collaboration); if (orchestrator) { if (providerId && providerId !== orchestrator.providerId) { @@ -1474,6 +1494,32 @@ mcpHub: this.#mcpHub, return { type: "response.error", requestId: msg.id, error: "packRole requires pack", code: "invalid_request" }; } + // Plan the role-children BEFORE creating anything, so a fan-out over the + // ceiling rejects the request outright instead of leaving a half-built + // collaboration behind. + let planned: PlannedChild[] = []; + if (collaboration) { + const plan = planChildren(collaboration); + if (!plan.ok) { + return { + type: "response.error", + requestId: msg.id, + error: plan.error, + code: "invalid_request", + }; + } + planned = plan.children; + // The orchestrator runs under the compiled one-goal pack: the goal, its + // fleet roster, and the delegation rules become its constitution, so + // pack vocabulary never surfaces on this path. + const compiled = compileGoalPack(collaboration, planned); + pack = { + id: compiled.id, + constitution: compiled.constitution, + subagents: compiled.subagents, + }; + } + const session = new Session({ name: msg.name, workdir, @@ -1501,6 +1547,29 @@ mcpHub: this.#mcpHub, this.#sessions.set(session.id, session); this.#rateLimiter.recordCreation(auth.sub); + if (collaboration && planned.length > 0) { + const spawned = await this.#spawnCollaborationChildren(session, collaboration, planned, auth); + if (!spawned.ok) { + // All-or-nothing: a collaboration missing a role is not a working + // collaboration, and leaving the orchestrator up with a partial fleet + // would have it delegate to children that don't exist. Unwind. + await this.#teardownCollaborationChildren(session.id, "partial spawn rolled back"); + try { + await session.destroy(auth); + } catch { + // Best-effort — the error we report is the spawn failure. + } + this.#sessions.delete(session.id); + this.#rateLimiter.recordDestruction(auth.sub); + return { + type: "response.error", + requestId: msg.id, + error: spawned.error, + code: "internal", + }; + } + } + return { type: "response.ok", requestId: msg.id, @@ -1508,6 +1577,129 @@ mcpHub: this.#mcpHub, }; } + /** + * Bring up a collaborative session's role-children (P1b). + * + * Each child is a normal long-lived session — NOT a dispatch-spawned + * disposable worker. That distinction is deliberate: the dispatcher destroys + * a spawn-task worker the moment its turn ends (`#finishWorkerTask`), which + * is wrong for a role that has to survive the implement↔review fix-loop. + * These children instead receive `fleet_send` dispatches, which the + * dispatcher delivers without taking ownership of their lifetime, so the + * collaboration owns teardown. + * + * No brief is SENT here. The child's role, contract, and goal ride in its + * 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. + */ + async #spawnCollaborationChildren( + parent: Session, + collaboration: CollaborationConfig, + planned: readonly PlannedChild[], + auth: AuthContext, + ): Promise<{ ok: true } | { ok: false; error: string }> { + for (const child of planned) { + try { + const childSession = new Session({ + name: childSessionName(parent.name, child), + workdir: parent.workdir, + auth, + store: this.#store, + transcriptStore: this.#transcriptStore, + providers: this.#providers, + hooks: this.#hooks, + providerId: child.providerId, + defaultModel: child.model, + role: "worker", + // The enforcement behind §6: a read-only role becomes a "scout", + // whose LEAF identity profile carries no tools:write at all, so it + // cannot mint write authority even via a sub-agent. + workerShape: child.shape, + // ...and the same restriction at the canUseTool fence, where + // roleDeniesTool turns `write: false` into a hard tool deny + // (Claude-hard; advisory + logged on backends whose tools don't all + // route through the gate — see roleEnforcement). + pack: { + id: "collaboration", + constitution: childBrief(collaboration, child), + role: { + name: child.roleName, + write: child.write, + // Not `false`: §3 gives the search role web access, and + // roleDeniesTool only denies network tools on an explicit false. + // Per-role network gating is a later phase. + network: "read-only", + envelope: "all", + }, + roleName: child.roleName, + subagents: [], + }, + collaborationRole: { + parentSessionId: parent.id, + roleName: child.roleName, + ordinal: child.ordinal, + write: child.write, + }, + identityManager: this.#identityManager, + memory: this.#memory, + memoryMcp: this.#memoryMcp, + mcpRegistry: this.#mcpRegistry, + mcpHub: this.#mcpHub, + config: this.#config, + compressionRegistry: this.#compressionRegistry, + _testProvider: this.#testProviderFactory?.(), + onStatusChange: this.#statusObserver, + onModels: (providerId, m) => this._cacheModels(providerId, m), + }); + this.#sessions.set(childSession.id, childSession); + // 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. + this.#store.audit( + auth.sub, + "collaboration.child_spawned", + childSession.id, + `parent=${parent.id} role=${child.roleName}#${child.ordinal} provider=${child.providerId}${child.model ? `/${child.model}` : ""} shape=${child.shape}`, + ); + } catch (err) { + return { + ok: false, + error: `Failed to bring up collaboration role "${child.roleName}" on "${child.providerId}": ${err instanceof Error ? err.message : String(err)}`, + }; + } + } + return { ok: true }; + } + + /** + * Tear down every live child of a collaboration (goal end). + * + * Membership is DERIVED from the live session set rather than tracked in a + * side map, so it cannot drift out of sync with reality — the failure mode + * of a parallel registry here is an orphaned agent subprocess. + */ + async #teardownCollaborationChildren(parentSessionId: string, reason: string): Promise { + const children = [...this.#sessions.values()].filter( + (s) => s.collaborationRole?.parentSessionId === parentSessionId, + ); + for (const child of children) { + try { + await child.destroy(this.#dispatchSystemAuth(child.accountId, child.projectId)); + } catch (err) { + console.error( + `[codeoid/collaboration] child teardown failed (${reason}): ${err instanceof Error ? err.message : String(err)}`, + ); + } + this.#sessions.delete(child.id); + this.#store.audit( + "system:collaboration", + "collaboration.child_destroyed", + child.id, + `parent=${parentSessionId} role=${child.collaborationRole?.roleName ?? "?"} reason=${reason}`, + ); + } + } + /** * Fork a session (`session.fork`) — branch its conversation into a new, * independent session seeded with a COPY of the parent's canonical history @@ -3378,6 +3570,15 @@ mcpHub: this.#mcpHub, // races the still-running consumer task and the appendFile that // landed in P1 #10 — ENOENT or partially-written final lines on // the new session. + // Goal end for a collaborative session: its role-children have per-goal + // lifetime, so they go with it. Children FIRST — the orchestrator is what + // the owner asked to destroy, and returning OK while N child agent + // subprocesses are still live would orphan them with no handle left to + // reach them by. + if (session.collaboration) { + await this.#teardownCollaborationChildren(msg.sessionId, "collaboration goal ended"); + } + await session.destroy(auth); this.#sessions.delete(msg.sessionId); this.#rateLimiter.recordDestruction(auth.sub); diff --git a/src/daemon/session.ts b/src/daemon/session.ts index 04dd7ef..49723b9 100644 --- a/src/daemon/session.ts +++ b/src/daemon/session.ts @@ -241,6 +241,14 @@ export interface SessionCreateOptions { * Absent = a normal session. */ collaboration?: CollaborationConfig; + /** + * Set on a role-CHILD of a collaborative session. The mirror of + * `collaboration` (which is set on the orchestrating parent), so the + * manager can DERIVE a collaboration's membership from the live session set + * rather than keeping a side registry that could drift out of sync and + * orphan an agent subprocess. + */ + collaborationRole?: SessionInfo["collaborationRole"]; /** * Pre-built codeoid_fleet MCP server (conductor sessions only). Built by * the SessionManager because its tools close over the manager's tenant- @@ -321,6 +329,8 @@ export class Session { * lifetime); changing backends mid-goal would orphan live children. */ readonly collaboration?: CollaborationConfig; + /** Which collaboration + role this session serves, when it is a child. */ + readonly collaborationRole?: SessionInfo["collaborationRole"]; readonly createdBy: string; readonly createdAt: string; /** @@ -611,6 +621,7 @@ export class Session { this.forkedFrom = opts.forkedFrom; this.worktree = opts.worktree; this.collaboration = opts.collaboration; + this.collaborationRole = opts.collaborationRole; this.#onStatusChange = opts.onStatusChange; this.#workerShape = opts.workerShape; if (opts.initialMode) { @@ -755,6 +766,7 @@ export class Session { forkedFrom: this.forkedFrom, worktree: this.worktree, collaboration: this.collaboration, + collaborationRole: this.collaborationRole, // Fire-and-forget: saveMeta's write chain owns the failure log; an // unconsumed rejection here would be an unhandled-rejection crash. }).catch(() => {}); @@ -2221,6 +2233,7 @@ export class Session { forkedFrom: this.forkedFrom, worktree: this.worktree, collaboration: this.collaboration, + collaborationRole: this.collaborationRole, // Ambient pack driving this session (docs/pack-loading.md) — id, or // "id (role)" when a capability role is active. ...(this.#pack @@ -4067,6 +4080,7 @@ export class Session { // reads. Leaving it out cost the collaboration on the first status // transition — i.e. on every session that had taken a single turn. collaboration: this.collaboration, + collaborationRole: this.collaborationRole, }).catch(() => {}); } } diff --git a/src/daemon/transcript.ts b/src/daemon/transcript.ts index 1011594..d33294a 100644 --- a/src/daemon/transcript.ts +++ b/src/daemon/transcript.ts @@ -19,6 +19,7 @@ import { join } from "node:path"; import type { CollaborationConfig, DaemonMessage, + SessionInfo, SessionMessage, SessionStatus, SessionWorktree, @@ -109,6 +110,13 @@ export interface TranscriptMeta { * because meta is what the resume path actually reads. */ collaboration?: CollaborationConfig; + /** + * Which collaboration + role this session serves, when it is a child. + * Written in BOTH saveMeta calls (create and status-persist) — meta is a + * whole-file overwrite, so a field present in only one of them is erased by + * the first status transition. + */ + collaborationRole?: SessionInfo["collaborationRole"]; } /** Types we persist. Skip ephemeral events like heartbeats. */ diff --git a/src/tests/collaboration.test.ts b/src/tests/collaboration.test.ts index 5c3edfe..32acc8d 100644 --- a/src/tests/collaboration.test.ts +++ b/src/tests/collaboration.test.ts @@ -23,6 +23,7 @@ import type { CodeoidConfig } from "../config.js"; import { orchestratorRole, parseRoleSpec, + planChildren, validateCollaboration, type ProviderLookup, } from "../daemon/collaboration.js"; @@ -32,6 +33,7 @@ import type { ProviderEvent } from "../daemon/providers/interface.js"; import { SessionManager } from "../daemon/session-manager.js"; import { Store } from "../daemon/store.js"; import { TranscriptStore } from "../daemon/transcript.js"; +import { roleDeniesTool } from "../daemon/providers/tool-safety.js"; import { ALL_SCOPES } from "../protocol/scopes.js"; import { LIMITS } from "../protocol/types.js"; import type { @@ -332,6 +334,11 @@ afterEach(async () => { try { await manager.drain(3_000); } catch {} + // Let in-flight fire-and-forget meta writes land before the tmp dir goes + // away. Without this, tearing down a collaboration's children races their + // own meta writes and floods the output with ENOENT rename warnings that + // would mask a real failure. + await Bun.sleep(150); rmSync(tmp, { recursive: true, force: true }); }); @@ -477,6 +484,262 @@ describe("session.create --collaborate fails closed", () => { }); }); +// ── 4. Role children (P1b) ────────────────────────────────────────────────── + +/** Every live session for this tenant, parent + children. */ +async function allSessions(): Promise { + const resp = await run({ type: "session.list", id: `ls-${Math.random()}` }); + return (resp as { sessions: SessionInfo[] }).sessions; +} + +const childrenOf = (all: SessionInfo[], parentId: string) => + all + .filter((s) => s.collaborationRole?.parentSessionId === parentId) + .sort((a, b) => + `${a.collaborationRole?.roleName}${a.collaborationRole?.ordinal}`.localeCompare( + `${b.collaborationRole?.roleName}${b.collaborationRole?.ordinal}`, + ), + ); + +describe("planChildren", () => { + test("excludes the orchestrator — the session itself plays that role", () => { + const r = validateCollaboration( + { goal: "g", roles: [{ name: "orchestrator", providerId: "claude" }] }, + LOOKUP, + ); + expect(r.ok).toBe(true); + if (!r.ok) return; + const plan = planChildren(r.config); + expect(plan.ok).toBe(true); + if (plan.ok) expect(plan.children).toHaveLength(0); + }); + + test("expands fan-out into ordinals", () => { + const r = validateCollaboration( + { + goal: "g", + roles: [ + { name: "orchestrator", providerId: "claude" }, + { name: "review", providerId: "gemini", count: 3 }, + ], + }, + LOOKUP, + ); + if (!r.ok) throw new Error(r.error); + const plan = planChildren(r.config); + if (!plan.ok) throw new Error(plan.error); + expect(plan.children.map((c) => c.ordinal)).toEqual([1, 2, 3]); + expect(plan.children.every((c) => c.roleName === "review")).toBe(true); + }); + + // Least privilege: absent `write` means the child's identity carries no + // write scope at all, which is what makes §6's reviewer guarantee real. + test("defaults a role to the read-only scout shape", () => { + const r = validateCollaboration( + { + goal: "g", + roles: [ + { name: "orchestrator", providerId: "claude" }, + { name: "review", providerId: "gemini" }, + { name: "reasoning", providerId: "openai", write: true }, + ], + }, + LOOKUP, + ); + if (!r.ok) throw new Error(r.error); + const plan = planChildren(r.config); + if (!plan.ok) throw new Error(plan.error); + const byRole = Object.fromEntries(plan.children.map((c) => [c.roleName, c])); + expect(byRole.review!.shape).toBe("scout"); + expect(byRole.review!.write).toBe(false); + expect(byRole.reasoning!.shape).toBe("ship"); + expect(byRole.reasoning!.write).toBe(true); + }); + + // Rejects rather than truncating: a collaboration quietly missing a reviewer + // is worse than one that refused to start. + test("rejects a fan-out over the child ceiling instead of truncating", () => { + const r = validateCollaboration( + { + goal: "g", + roles: [ + { name: "orchestrator", providerId: "claude" }, + { name: "a", providerId: "gemini", count: 8 }, + { name: "b", providerId: "openai", count: 8 }, + ], + }, + LOOKUP, + ); + if (!r.ok) throw new Error(r.error); + const plan = planChildren(r.config); + expect(plan.ok).toBe(false); + if (!plan.ok) { + expect(plan.error).toMatch(/would spawn 16 children — max 12/); + } + }); +}); + +// The security claim in §6 is that a reviewer *provably* cannot write, not +// that it was asked not to. Two independent mechanisms back that, so assert +// the envelope actually denies rather than trusting the shape label. +describe("read-only roles are enforced, not requested", () => { + const envelopeFor = (write: boolean) => ({ + write, + network: "read-only" as const, + envelope: "all" as const, + }); + + test("the envelope built for a read-only role denies every write tool", () => { + const readOnly = envelopeFor(false); + for (const tool of ["Write", "Edit", "MultiEdit", "NotebookEdit"]) { + expect(roleDeniesTool(readOnly, tool)).toMatch(/read-only/); + } + }); + + test("it still permits reads and the web tools a search role needs", () => { + const readOnly = envelopeFor(false); + for (const tool of ["Read", "Grep", "Glob", "Bash", "WebSearch", "WebFetch"]) { + expect(roleDeniesTool(readOnly, tool)).toBeNull(); + } + }); + + test("a write role is permitted the write tools", () => { + const writer = envelopeFor(true); + for (const tool of ["Write", "Edit"]) { + expect(roleDeniesTool(writer, tool)).toBeNull(); + } + }); +}); + +describe("collaboration children come up and are torn down", () => { + const THREE: CollaborationConfig = { + goal: "Add rate limiting to the public API", + roles: [ + { name: "orchestrator", providerId: "claude" }, + { name: "reasoning", providerId: "claude", write: true }, + { name: "review", providerId: "gemini", count: 2 }, + ], + }; + + test("children spawn on their own bound backends", async () => { + const resp = await run({ + type: "session.create", + id: "c1", + name: "collab", + workdir, + collaboration: THREE, + }); + expect(resp.type).toBe("response.ok"); + if (resp.type !== "response.ok") return; + const parent = resp.data as SessionInfo; + + const kids = childrenOf(await allSessions(), parent.id); + expect(kids).toHaveLength(3); // reasoning ×1 + review ×2, orchestrator excluded + expect(kids.map((k) => k.collaborationRole!.roleName)).toEqual([ + "reasoning", + "review", + "review", + ]); + // The whole point: each child is on the backend its role named. + expect(kids.map((k) => k.providerId)).toEqual(["claude", "gemini", "gemini"]); + expect(kids.map((k) => k.collaborationRole!.ordinal)).toEqual([1, 1, 2]); + // Write authority is per role, and read-only is the default. + expect(kids.map((k) => k.collaborationRole!.write)).toEqual([true, false, false]); + // Children are workers, so they can never see or direct the fleet. + expect(kids.every((k) => k.role === "worker")).toBe(true); + }); + + test("the parent runs under the compiled one-goal pack", async () => { + const resp = await run({ + type: "session.create", + id: "c2", + name: "collab2", + workdir, + collaboration: THREE, + }); + expect(resp.type).toBe("response.ok"); + if (resp.type !== "response.ok") return; + // Compiled, not installed — pack vocabulary stays hidden on this path. + expect((resp.data as SessionInfo).profile).toBe("collaboration"); + }); + + test("destroying the parent tears down every child", async () => { + const resp = await run({ + type: "session.create", + id: "c3", + name: "collab3", + workdir, + collaboration: THREE, + }); + expect(resp.type).toBe("response.ok"); + if (resp.type !== "response.ok") return; + const parent = resp.data as SessionInfo; + expect(childrenOf(await allSessions(), parent.id)).toHaveLength(3); + + const destroyed = await run({ + type: "session.destroy", + id: "c3d", + sessionId: parent.id, + }); + expect(destroyed.type).toBe("response.ok"); + + const after = await allSessions(); + expect(childrenOf(after, parent.id)).toHaveLength(0); + expect(after.find((s) => s.id === parent.id)).toBeUndefined(); + }); + + test("a collaboration with only an orchestrator spawns nothing", async () => { + const resp = await run({ + type: "session.create", + id: "c4", + name: "collab4", + workdir, + collaboration: { goal: "g", roles: [{ name: "orchestrator", providerId: "claude" }] }, + }); + expect(resp.type).toBe("response.ok"); + if (resp.type !== "response.ok") return; + expect(childrenOf(await allSessions(), (resp.data as SessionInfo).id)).toHaveLength(0); + }); + + test("collaboration and pack are mutually exclusive", async () => { + const resp = await run({ + type: "session.create", + id: "c5", + name: "collab5", + workdir, + collaboration: THREE, + pack: "some-pack", + }); + expect(resp.type).toBe("response.error"); + if (resp.type === "response.error") { + expect(resp.code).toBe("invalid_request"); + expect(resp.error).toMatch(/mutually exclusive/); + } + }); + + test("an over-ceiling fan-out is rejected before anything is created", async () => { + const before = (await allSessions()).length; + const resp = await run({ + type: "session.create", + id: "c6", + name: "collab6", + workdir, + collaboration: { + goal: "g", + roles: [ + { name: "orchestrator", providerId: "claude" }, + { name: "a", providerId: "gemini", count: 8 }, + { name: "b", providerId: "gemini", count: 8 }, + ], + }, + }); + expect(resp.type).toBe("response.error"); + if (resp.type === "response.error") expect(resp.error).toMatch(/max 12/); + // Nothing half-built. + expect((await allSessions()).length).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", () => {