diff --git a/AGENTS.md b/AGENTS.md index 7347d1556..59c8fca4e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -138,9 +138,13 @@ collaborative state and external implementation runs are durable. summary and research workers stay isolated. There is no flag for this; the harness is the choice. - **`HARNESS=remote` moves the model, not the authority.** Turns run in a client - attached at `/planner-link` with a GitHub bearer and pull access; Chopin's + attached at `/planner-link` with a GitHub bearer and push access; Chopin's host tools, Decisions, and room writes still run in the server. The attached - client sees what a turn sees and steers its document writes. See + client sees what a turn sees and steers its document writes. Attaching makes + the account's live browser login the document's Planner owner, one client per + document; nothing else claims ownership. `@chopin` and `invoke_planner` on a + document without one fail with `planner-not-attached`. Signing out the + owner's login, its expiry, or a Planner reset closes the link. See [Hosted agent](docs/hosted-agent.md#remote-planner). - **Repository node IDs are authoritative.** Owner and repository names resolve GitHub requests but never replace the stored node identity. diff --git a/apps/server/src/chat/remote-planner.test.ts b/apps/server/src/chat/remote-planner.test.ts new file mode 100644 index 000000000..c265ca51b --- /dev/null +++ b/apps/server/src/chat/remote-planner.test.ts @@ -0,0 +1,342 @@ +import { afterEach, expect, test } from "bun:test"; + +import { ActiveOwnerBindings } from "../agent/active-owner"; +import { createPlannerAgent } from "../harness/agents"; +import { createRemoteAdapter } from "../harness/remote/adapter"; +import { notAttachedMessage } from "../harness/remote/attachments"; +import { openPlannerSession } from "../harness/session"; +import * as Plan from "../plan/service"; +import { configured } from "../testing/config"; +import { + attach, + finishTurn, + linkAuth, + linkServer, + RIVAL_TOKEN, + scriptedPlanner, +} from "../testing/planner-link"; +import * as Chat from "./service"; + +import type { Server } from "bun"; +import type { PlannerLinkClient } from "@chopin/planner-link/client"; +import type { HostedAuth } from "../auth/routes"; +import type { Socket, SocketData } from "../wire"; + +let cleanups: Array<() => unknown> = []; +afterEach(async () => { + for (let cleanup of cleanups.splice(0).toReversed()) await cleanup(); +}); + +const REMOTE = { HARNESS: "remote", BACKGROUND_JOBS: "off" }; +const RIVAL = { id: "U_rival", login: "rival" }; + +function card(status: "running" | "paused") { + return { + id: "run-1", + name: "plan-review", + status, + started: 1, + updated: 1, + stages: [], + waiting: 0, + }; +} + +/** + * A document's Chat under `HARNESS=remote`, served as `main.ts` serves it, + * with a real Planner link server. The rival sends from a browser with a + * login of their own; only the attached member may own the Planner. + */ +async function remoteRoom() { + let { auth, storage, channel, logins, revocations, signOut, clock } = await linkAuth(); + let server = linkServer(auth); + cleanups.push(server.stop); + revocations.add(id => server.attachments.sessionRevoked(id)); + let lease = (await storage.leases.acquire("writer", "test", 60_000))!; + let events: Array<{ kind: string; [key: string]: unknown }> = []; + let publisher = { + publish(_topic: string, message: string) { + events.push(JSON.parse(message)); + }, + } as unknown as Server; + let plan = await Plan.open(channel.id, { + storage, + lease: () => lease, + fatal: error => { + throw error; + }, + }, publisher); + let owners = new ActiveOwnerBindings(auth as unknown as HostedAuth); + let url = `https://chopin.test/documents/octo-org/score/${channel.slug}`; + let agent = createPlannerAgent(createRemoteAdapter()); + let context: Chat.Room = { + chat: plan.chat, + plan, + room: channel.id, + server: publisher, + auth: auth as unknown as HostedAuth, + repository: { id: "R_score", owner: "octo-org", name: "score", defaultBranch: "main" }, + config: configured(REMOTE), + claimantSessionId: logins.get("rival"), + plannerNotAttached: async () => + await server.attachments.confirmed(channel.id) + ? undefined + : notAttachedMessage(url, "octo-org/score"), + activeOwner: () => owners.resolve(channel.id), + persist: () => Plan.persist(plan), + openPlannerSession: (binding, options) => + openPlannerSession(binding, options, { + githubTools: async () => ({ ok: true, value: {} }), + createSandbox: async () => + ({ + defaultWorkingDirectory: "/tmp", + run: async () => ({ exitCode: 0, stdout: "", stderr: "" }), + destroy: async () => {}, + }) as never, + registerCredential: () => () => {}, + agent, + }), + }; + cleanups.push(async () => { + owners.revokeAll(); + await Chat.close(plan.chat); + await Plan.close(plan); + }); + let replies: Array<{ kind: string; rid?: string; [key: string]: unknown }> = []; + let ws = { + data: { handle: RIVAL.login, principalId: RIVAL.id, sessionId: logins.get("rival") }, + send(text: string) { + replies.push(JSON.parse(text)); + }, + } as unknown as Socket; + let send = async (text: string, to: "planner" | "room" = "planner") => { + let rid = crypto.randomUUID(); + await Chat.send(context, ws, { + kind: "chat:send", + rid, + requestId: crypto.randomUUID(), + to, + text, + ts: 0, + } as never); + return replies.find(reply => reply.rid === rid); + }; + let owner = async () => + (await storage.channels.readAgent(channel.id, new Date()))?.agent?.ownerSessionId; + return { + context, + server, + storage, + channel, + logins, + url, + ws, + send, + owner, + events, + signOut, + clock, + }; +} + +/** + * The room with the member attached as its Planner and every browser sign-in + * expired, before the link's periodic recheck or any cleanup has noticed, + * counting what Chat saves from then on. + */ +async function expiredOwnerRoom() { + let room = await remoteRoom(); + let member = await attach(room.server.url, room.channel.id, scriptedPlanner().handlers); + cleanups.push(() => member.detach()); + expect(await room.owner()).toBe(room.logins.get("member")); + room.clock.skewMs = 31 * 24 * 60 * 60 * 1_000; + let saves = 0; + let persist = room.context.persist; + room.context.persist = async chat => { + saves++; + await persist(chat); + }; + return { ...room, member, saves: () => saves }; +} + +test("with no attached Planner, @chopin and invoke_planner are refused with how to attach one, and nothing is posted or queued", async () => { + let room = await remoteRoom(); + let { context } = room; + let saves = 0; + let persist = context.persist; + context.persist = async chat => { + saves++; + await persist(chat); + }; + let message = "No Planner is attached to this document. To attach one, run pdd-plan with " + + `plan_document set to ${room.url} from a session in a checkout of octo-org/score; ` + + "that session becomes this document's Planner."; + + expect(await room.send("@chopin draft the plan")).toMatchObject({ + kind: "session:error", + code: "planner-not-attached", + message, + }); + context.chat.busy = true; + expect(await room.send("@chopin and then this")).toMatchObject({ code: "planner-not-attached" }); + context.chat.busy = false; + expect(await Chat.invoke(context, RIVAL, "Draft the plan")).toBe("planner-not-attached"); + expect(context.chat.entries).toEqual([]); + expect(context.chat.waiting).toEqual([]); + expect(saves).toBe(0); + expect(await room.owner()).toBeUndefined(); + + expect(await room.send("Just talking", "room")).toMatchObject({ kind: "chat:send" }); + expect(context.chat.entries.map(entry => entry.text)).toEqual(["Just talking"]); +}); + +test("other harnesses keep claiming ownership for the sender as before", async () => { + let room = await remoteRoom(); + let { context } = room; + context.config = configured({}); + context.plannerNotAttached = undefined; + let started: string[] = []; + context.openPlannerSession = async () => ({ + ok: true, + value: { + stream: async (prompt: string) => { + started.push(prompt); + return { fullStream: (async function*() {})() }; + }, + destroy: async () => {}, + } as never, + }); + expect(await room.send("@chopin draft the plan")).toMatchObject({ kind: "chat:send" }); + await context.chat.running; + expect(started).toHaveLength(1); + expect(await room.owner()).toBe(room.logins.get("rival")); +}); + +test("an attached Planner receives @chopin and invoke_planner turns as their owner, and Stop and Resume reach it", async () => { + let room = await remoteRoom(); + let { context } = room; + let script = scriptedPlanner(); + let controls: Array<{ type: string; run?: string }> = []; + let client: PlannerLinkClient; + client = await attach(room.server.url, room.channel.id, { + ...script.handlers, + async turn(turn) { + script.turns.push(turn.message); + if (turn.message.prompt.includes("Run the review")) { + client.reportRuns(turn.message.session, { + active: ["run-1"], + paused: [], + cards: [card("running")], + }); + } + finishTurn(turn); + }, + control(message) { + controls.push({ type: message.type, run: message.run }); + client.reportRuns( + message.session, + message.type === "stop" + ? { active: [], paused: ["run-1"], cards: [card("paused")] } + : { active: ["run-1"], paused: [], cards: [card("running")] }, + ); + }, + }); + cleanups.push(() => client.detach()); + expect(await room.owner()).toBe(room.logins.get("member")); + + expect(await room.send("@chopin draft the plan")).toMatchObject({ kind: "chat:send" }); + await context.chat.running; + expect(await Chat.invoke(context, RIVAL, "Run the review")).toBeUndefined(); + await context.chat.running; + expect(script.turns.map(turn => turn.prompt)).toEqual([ + expect.stringContaining("draft the plan"), + expect.stringContaining("Run the review"), + ]); + expect(await room.owner()).toBe(room.logins.get("member")); + expect(context.chat.retained).toBeDefined(); + + await Chat.abort(context, room.ws); + await Bun.sleep(20); + await Chat.resume(context, room.ws); + expect(controls).toEqual([{ type: "stop", run: undefined }, { type: "resume", run: undefined }]); + expect(context.chat.entries.map(entry => entry.text)).toContain( + "@rival stopped the Planner and paused its workflows.", + ); + expect(context.chat.entries.map(entry => entry.text)).toContain( + "@rival resumed the Planner's workflows.", + ); +}); + +test("once the attached Planner's owner sign-in expires, @chopin is refused with how to attach one before its periodic recheck, and nothing is posted or queued", async () => { + let room = await expiredOwnerRoom(); + let { context } = room; + expect(await room.send("@chopin draft the plan")).toMatchObject({ + kind: "session:error", + code: "planner-not-attached", + message: "No Planner is attached to this document. To attach one, run pdd-plan with " + + `plan_document set to ${room.url} from a session in a checkout of octo-org/score; ` + + "that session becomes this document's Planner.", + }); + expect(context.chat.entries).toEqual([]); + expect(context.chat.waiting).toEqual([]); + expect(room.saves()).toBe(0); + expect(room.events).toEqual([]); + expect(await room.member.closed).toEqual({ code: 4401, reason: "sign-in-required" }); + expect(room.server.attachments.attached(room.channel.id)).toBe(false); + expect(await room.owner()).toBeUndefined(); +}); + +test("once the attached Planner's owner sign-in expires, invoke_planner is refused as planner-not-attached before its periodic recheck", async () => { + let room = await expiredOwnerRoom(); + let { context } = room; + expect(await Chat.invoke(context, RIVAL, "Draft the plan")).toBe("planner-not-attached"); + expect(context.chat.entries).toEqual([]); + expect(context.chat.waiting).toEqual([]); + expect(room.saves()).toBe(0); + expect(await room.member.closed).toEqual({ code: 4401, reason: "sign-in-required" }); + expect(await room.owner()).toBeUndefined(); +}); + +test("once the attached Planner's owner signs out or is reset, @chopin and invoke_planner are refused again, and another member can attach", async () => { + let room = await remoteRoom(); + let { context } = room; + let refusals = async () => { + expect(await room.send("@chopin draft the plan")).toMatchObject({ + kind: "session:error", + code: "planner-not-attached", + }); + expect(await Chat.invoke(context, RIVAL, "Draft the plan")).toBe("planner-not-attached"); + expect(context.chat.entries).toEqual([]); + expect(context.chat.waiting).toEqual([]); + }; + + let member = await attach(room.server.url, room.channel.id, scriptedPlanner().handlers); + expect(await room.owner()).toBe(room.logins.get("member")); + await room.signOut("member"); + expect(await member.closed).toEqual({ code: 4401, reason: "sign-in-required" }); + expect(await room.owner()).toBeUndefined(); + await refusals(); + + let script = scriptedPlanner(); + let rival = await attach(room.server.url, room.channel.id, script.handlers, RIVAL_TOKEN); + expect(await room.owner()).toBe(room.logins.get("rival")); + let held = (await room.storage.channels.readAgent(room.channel.id, new Date()))!.agent!; + await room.storage.channels.clearAgentOwner( + room.channel.id, + held.ownerSessionId!, + held.generation, + new Date(), + ); + room.server.attachments.ownerReset(room.channel.id); + expect(await rival.closed).toEqual({ code: 4403, reason: "access-revoked" }); + await refusals(); + expect(script.turns).toEqual([]); + + let again = await attach(room.server.url, room.channel.id, script.handlers, RIVAL_TOKEN); + cleanups.push(() => again.detach()); + expect(await room.send("@chopin draft the plan")).toMatchObject({ kind: "chat:send" }); + await context.chat.running; + expect(script.turns.map(turn => turn.prompt)).toEqual([ + expect.stringContaining("draft the plan"), + ]); +}); diff --git a/apps/server/src/chat/service.ts b/apps/server/src/chat/service.ts index f35714372..eb4e0f79d 100644 --- a/apps/server/src/chat/service.ts +++ b/apps/server/src/chat/service.ts @@ -533,6 +533,12 @@ export type Room = { */ claimantSessionId: string | undefined; repository: HostedRepository; + /** + * Under `HARNESS=remote`, how to attach a Planner when none is attached, or + * undefined while one is. Planner requests are refused before anything is + * posted or queued, and only an attachment takes Planner ownership. + */ + plannerNotAttached?: () => Promise; activeOwner?: () => Promise; persist: (chat?: () => Pick) => Promise; commitRoomMessage?: (entry: Wire.Entry) => Promise; @@ -596,6 +602,7 @@ export async function invoke( checkout?: string, ): Promise< | "planner-unavailable" + | "planner-not-attached" | "planner-owner-unavailable" | "planner-queue-full" | "checkout-unverified" @@ -606,13 +613,14 @@ export async function invoke( chat.pendingSends++; let post = async () => { if (!context.config.agent || chat.closed) return "planner-unavailable" as const; + if (await context.plannerNotAttached?.()) return "planner-not-attached" as const; let verified: string | undefined; if (checkout !== undefined && context.config.harness === "atomic") { verified = await verifiedCheckout(context.repository, checkout); if (!verified) return "checkout-unverified" as const; } try { - await resolveOwner(context.auth, context.repository, room, context.claimantSessionId); + await resolveOwner(context.auth, context.repository, room, claimant(context)); } catch { return "planner-owner-unavailable" as const; } @@ -774,6 +782,10 @@ async function processSend(context: Room, ws: Socket, msg: Request): return; } + let unattached = await context.plannerNotAttached?.(); + if (unattached) return fail(ws, msg.rid, unattached, "planner-not-attached"); + if (chat.closed) return fail(ws, msg.rid, "chat is closed"); + if (chat.busy) { if (chat.waiting.length >= MAX_QUEUE) { say(chat, server, room, { @@ -1621,7 +1633,7 @@ async function repositorySession( context.auth, context.repository, context.room, - claimantSessionId, + context.plannerNotAttached ? undefined : claimantSessionId, ); if (context.ownerAvailable) void context.ownerAvailable().catch(() => {}); let { chat, auth } = context; @@ -1706,6 +1718,11 @@ async function repositorySession( } } +/** The login that may claim Planner ownership for the room; none where only an attachment may. */ +function claimant(context: Room): string | undefined { + return context.plannerNotAttached ? undefined : context.claimantSessionId; +} + /** * The channel's Planner owner, claimed for the claimant when it has none. * Without a claimant only an owner the channel already has will do. diff --git a/apps/server/src/config.test.ts b/apps/server/src/config.test.ts index 92a9ec59f..25762ca70 100644 --- a/apps/server/src/config.test.ts +++ b/apps/server/src/config.test.ts @@ -114,6 +114,18 @@ describe("configuration", () => { .toThrow("HARNESS=remote requires BACKGROUND_JOBS=off"); }); + it("keeps a detached remote Planner's ownership two minutes unless PLANNER_ATTACH_GRACE_MS says otherwise", () => { + let remote = { HARNESS: "remote", BACKGROUND_JOBS: "off" }; + expect(configured(remote).plannerAttachGraceMs).toBe(120_000); + expect(configured({ ...remote, PLANNER_ATTACH_GRACE_MS: "0" }).plannerAttachGraceMs).toBe(0); + expect(configured({ ...remote, PLANNER_ATTACH_GRACE_MS: "3600000" }).plannerAttachGraceMs) + .toBe(3_600_000); + for (let value of ["", "-1", "1.5", "3600001", "soon"]) { + expect(() => configured({ ...remote, PLANNER_ATTACH_GRACE_MS: value })) + .toThrow("PLANNER_ATTACH_GRACE_MS must be an integer between 0 and 3600000 milliseconds"); + } + }); + it("chooses a full Atomic Planner by harness alone, in hosted and local configuration", () => { let atomic = { HARNESS: "atomic", HARNESS_AUTH: "ai-gateway", MODEL: "stub/model" }; for (let mode of [{}, LOCAL]) { diff --git a/apps/server/src/config.ts b/apps/server/src/config.ts index d510778b2..0424b7637 100644 --- a/apps/server/src/config.ts +++ b/apps/server/src/config.ts @@ -25,6 +25,11 @@ export type Config = { * passed to the CLI's `--extension`. Split on the platform path delimiter. */ harnessExtensions: string[]; + /** + * Under `HARNESS=remote`, how long a detached or dropped Planner link keeps + * the document's Planner ownership for the same account to reattach. + */ + plannerAttachGraceMs: number; /** * Whether to run the agent at all. * @@ -59,6 +64,7 @@ export type Config = { }; const DEFAULT_PORT = 8787; +const DEFAULT_ATTACH_GRACE_MS = 120_000; const DEFAULT_MODEL = "gpt-6-luna"; /** The default is a Copilot model ID. Pi and Atomic resolve IDs against their @@ -139,6 +145,16 @@ export function load(): Config { ) { throw new Error("JEV_MODEL must be a valid model alias"); } + let graceRaw = process.env.PLANNER_ATTACH_GRACE_MS; + let plannerAttachGraceMs = graceRaw === undefined ? DEFAULT_ATTACH_GRACE_MS : Number(graceRaw); + if ( + (graceRaw !== undefined && !/^\d+$/.test(graceRaw)) + || !Number.isSafeInteger(plannerAttachGraceMs) || plannerAttachGraceMs > 3_600_000 + ) { + throw new Error( + "PLANNER_ATTACH_GRACE_MS must be an integer between 0 and 3600000 milliseconds", + ); + } let selection = harnessSelection(); if (selection.harness === "remote" && backgroundJobs) { throw new Error( @@ -150,6 +166,7 @@ export function load(): Config { ...selection, port: serverPort, model: model(selection.harness), + plannerAttachGraceMs, agent, backgroundJobs, webResearch: agent && backgroundJobs && process.env.WEB_RESEARCH !== "off", diff --git a/apps/server/src/conversation-plan/accepted-research.ts b/apps/server/src/conversation-plan/accepted-research.ts index 136f63f50..0bf0861ea 100644 --- a/apps/server/src/conversation-plan/accepted-research.ts +++ b/apps/server/src/conversation-plan/accepted-research.ts @@ -13,6 +13,8 @@ type AcceptedResearchDeps = { refreshAccess: (ws: Socket, force: boolean) => Promise; unavailable: (id: string) => boolean; ownerAvailable: (id: string) => Promise; + /** False under `HARNESS=remote`, where only an attached Planner takes ownership. */ + claimsOwnership?: boolean; placeReference: (channelId: string, workspaceId: string) => Promise<"placed" | "deferred">; scheduleRecovery: (deferred: number) => void; }; @@ -44,7 +46,12 @@ export async function startAcceptedResearch( || deps.unavailable(room.id) ) return false; try { - await Chat.resolveOwner(deps.auth, repository, room.id, ws.data.sessionId); + await Chat.resolveOwner( + deps.auth, + repository, + room.id, + deps.claimsOwnership === false ? undefined : ws.data.sessionId, + ); return true; } catch { return false; diff --git a/apps/server/src/harness/remote/attachments.ts b/apps/server/src/harness/remote/attachments.ts new file mode 100644 index 000000000..106d5719c --- /dev/null +++ b/apps/server/src/harness/remote/attachments.ts @@ -0,0 +1,397 @@ +/** + * Which client is each document's remote Planner. + * + * Attaching makes the client's account the document's Planner owner through + * the same generation-guarded storage claim a browser owner takes elsewhere. + * The owner is the account's live browser login, because Chopin's repository + * tools act with that login's GitHub App token, never with the link's bearer. + * A dropped or detached link keeps its ownership for a grace period, so the + * same account can reattach without losing it; then the ownership is cleared. + * Ending that login, resetting the document's Planner, or any other release of + * the stored ownership closes the link and frees the document at once. + */ + +import { GitHubError } from "../../github/client"; +import { attachLink } from "./registry"; + +import type { Admission } from "../../auth/admission"; +import type { AuthConfig } from "../../auth/config"; +import type { AuthenticatedSession, Sessions } from "../../auth/session"; +import type { GitHub } from "../../github/client"; +import type { StorageAdapter } from "../../storage/port"; +import type { PlannerLink } from "./link"; + +export type AttachmentAuth = { + config: Pick; + storage: Pick; + github: Pick; + admission: Pick; + sessions: Pick; + clock: () => Date; +}; + +export type AttachRequest = { + channelId: string; + repository: { id: string; owner: string; name: string }; + user: { id: string; login: string }; +}; + +export type AttachRefusal = { + ok: false; + code: "planner-attached" | "sign-in-required" | "repository-forbidden" | "access-revoked"; + message: string; +}; + +export type AttachGrant = { ok: true; detach: () => void }; + +type Hold = { + channelId: string; + userId: string; + login: string; + ownerSessionId: string; + generation: number; + link?: PlannerLink; + forget?: () => void; + timer?: ReturnType; + releaseAt?: Date; +}; + +type PendingAttach = { + login: string; + held?: Hold; + sessionIds: Set; + invalidated?: AttachRefusal; +}; + +/** What `@chopin` and `invoke_planner` say when no Planner is attached to the document. */ +export function notAttachedMessage(url: string, repository: string): string { + return `No Planner is attached to this document. To attach one, run pdd-plan with ` + + `plan_document set to ${url} from a session in a checkout of ${repository}; ` + + `that session becomes this document's Planner.`; +} + +export function forbiddenMessage(repository: string): string { + return `Attaching as this document's Planner needs push or administration access to ${repository}.`; +} + +export class PlannerAttachments { + #auth: AttachmentAuth; + #graceMs: number; + #holds = new Map(); + #pending = new Map(); + #queues = new Map>(); + + constructor(auth: AttachmentAuth, graceMs: number) { + this.#auth = auth; + this.#graceMs = graceMs; + } + + /** Whether a client is attached to the document now, not merely holding it through a grace period. */ + attached(channelId: string): boolean { + return this.#holds.get(channelId)?.link !== undefined; + } + + /** + * Whether a client is attached and its ownership still stands, checked as + * the link's periodic recheck does. A stale attachment is closed and + * released here, so a Planner request never proceeds on it. + */ + async confirmed(channelId: string): Promise { + let link = this.#holds.get(channelId)?.link; + return link !== undefined && await this.verify(channelId, link); + } + + /** The GitHub login holding the document's Planner, attached or within its grace period. */ + holder(channelId: string): string | undefined { + return this.#holds.get(channelId)?.login; + } + + /** Makes `link` the document's Planner and its account the owner, or says why not. */ + attach(request: AttachRequest, link: PlannerLink): Promise { + return this.#serialized(request.channelId, () => this.#attach(request, link)); + } + + /** The browser login ended: every document it owns through an attachment is released. */ + async sessionRevoked(sessionId: string): Promise { + let origin = this.#auth.config.origin; + for (let pending of this.#pending.values()) { + if (!pending.sessionIds.has(sessionId)) continue; + pending.invalidated ??= pending.held?.ownerSessionId === sessionId + ? signInEndedRefusal(origin, pending.login) + : signInRefusal(origin, pending.login); + } + let held = [...this.#holds.values()].filter(hold => hold.ownerSessionId === sessionId); + for (let hold of held) this.#invalidate(hold, signInEndedRefusal(origin, hold.login)); + await Promise.all(held.map(hold => this.#release(hold))); + } + + /** A repository writer reset the document's Planner, which already cleared its stored owner. */ + ownerReset(channelId: string): void { + let pending = this.#pending.get(channelId); + if (pending) pending.invalidated ??= resetRefusal(pending.login); + let hold = this.#holds.get(channelId); + if (hold) this.#invalidate(hold, resetRefusal(hold.login)); + } + + /** + * Whether `link` still holds the document: its login is still live and the + * stored owner is still its attachment's login and generation. + * Otherwise the link closes and the document is free to attach. + */ + async verify(channelId: string, link: PlannerLink): Promise { + let hold = this.#holds.get(channelId); + if (hold?.link !== link) return false; + let stored = await this.#auth.storage.channels.readAgent(channelId, this.#auth.clock()); + let live = await this.#auth.sessions.resolve(hold.ownerSessionId); + if (this.#holds.get(channelId) !== hold || hold.link !== link) return false; + if (!live) { + this.#invalidate(hold, signInEndedRefusal(this.#auth.config.origin, hold.login)); + await this.#release(hold); + return false; + } + if ( + stored?.agent?.ownerSessionId === hold.ownerSessionId + && stored.agent.generation === hold.generation + ) return true; + this.#invalidate(hold, { + ok: false, + code: "access-revoked", + message: `This document's Planner ownership was released, so ${hold.login} is no longer ` + + `attached. Attach again to resume.`, + }); + return false; + } + + /** Stops every grace timer. Ownership stays in storage, which startup clears. */ + close(): void { + for (let hold of this.#holds.values()) clearTimeout(hold.timer); + this.#holds.clear(); + } + + async #attach(request: AttachRequest, link: PlannerLink): Promise { + let { channelId, user } = request; + let held = this.#holds.get(channelId); + if (held && (held.link || held.userId !== user.id)) return attachedRefusal(held); + let pending: PendingAttach = { + login: user.login, + held, + sessionIds: new Set(held ? [held.ownerSessionId] : []), + }; + this.#pending.set(channelId, pending); + try { + return await this.#attachPending(request, link, pending); + } finally { + if (this.#pending.get(channelId) === pending) this.#pending.delete(channelId); + } + } + + /** + * Takes or keeps the document for `request` while `pending` collects every + * reset or ended login that lands during its awaits, so a hold binds only + * if nothing released its ownership in the meantime. + */ + async #attachPending( + request: AttachRequest, + link: PlannerLink, + pending: PendingAttach, + ): Promise { + let { channelId, user } = request; + let login = await this.#eligibleLogin(request, pending); + if ("ok" in login) return login; + if (pending.invalidated) return pending.invalidated; + let now = this.#auth.clock(); + let held = pending.held; + if (held && this.#holds.get(channelId) === held) { + let stored = await this.#auth.storage.channels.readAgent(channelId, now); + let live = await this.#auth.sessions.resolve(held.ownerSessionId); + if (pending.invalidated) return pending.invalidated; + if ( + live && this.#holds.get(channelId) === held + && stored?.agent?.ownerSessionId === held.ownerSessionId + && stored.agent.generation === held.generation + ) return this.#bind(held, link); + this.#drop(held); + if (!live) await this.#release(held); + } + if (pending.invalidated) return pending.invalidated; + let claimed; + try { + claimed = await this.#claim(channelId, login.session.id, now); + } catch (error) { + // Storage refuses a claim for a sign-in that ended after its last check. + if (pending.invalidated) return pending.invalidated; + if (!await this.#auth.sessions.resolve(login.session.id)) { + return signInRefusal(this.#auth.config.origin, user.login); + } + throw error; + } + if (!claimed) throw new Error("The document's Planner owner changed while attaching."); + let hold: Hold = { + channelId, + userId: user.id, + login: user.login, + ownerSessionId: login.session.id, + generation: claimed.generation, + }; + let live = await this.#auth.sessions.resolve(hold.ownerSessionId); + let refusal = pending.invalidated + ?? (live ? undefined : signInRefusal(this.#auth.config.origin, user.login)); + if (refusal) { + await this.#release(hold); + return refusal; + } + this.#holds.set(channelId, hold); + return this.#bind(hold, link); + } + + /** + * The caller's browser login, registered with `pending` before its access + * checks so that its sign-out during them refuses the attach. + */ + async #eligibleLogin( + request: AttachRequest, + pending: PendingAttach, + ): Promise { + let { repository, user } = request; + let fullName = `${repository.owner}/${repository.name}`; + let login = await this.#auth.sessions.forUser(user.id); + if (!login) return signInRefusal(this.#auth.config.origin, user.login); + pending.sessionIds.add(login.session.id); + let checked; + try { + checked = await this.#auth.sessions.use( + login, + token => this.#auth.github.repositoryAccess(token, repository.owner, repository.name), + ); + } catch (error) { + if (error instanceof GitHubError && error.status === 401) { + return signInRefusal(this.#auth.config.origin, user.login); + } + throw error; + } + let access = checked.value; + if ( + !access || access.id !== repository.id + || (!access.permissions.push && !access.permissions.admin) + || !await this.#auth.admission.allowed(checked.authenticated.access.token, user.id) + ) return { ok: false, code: "repository-forbidden", message: forbiddenMessage(fullName) }; + return checked.authenticated; + } + + /** + * Claims the document for `sessionId`. Under the remote Planner only an + * attachment takes ownership, so an owner no attachment holds is stale and + * is cleared, guarded by its generation, before claiming again. + */ + async #claim(channelId: string, sessionId: string, now: Date) { + let channels = this.#auth.storage.channels; + let state = await channels.claimAgentOwner(channelId, sessionId, now); + if (state.ownerSessionId === sessionId) return state; + if (state.ownerSessionId) { + await channels.clearAgentOwner(channelId, state.ownerSessionId, state.generation, now); + } + state = await channels.claimAgentOwner(channelId, sessionId, now); + return state.ownerSessionId === sessionId ? state : undefined; + } + + #bind(hold: Hold, link: PlannerLink): AttachGrant { + clearTimeout(hold.timer); + hold.timer = undefined; + hold.releaseAt = undefined; + hold.link = link; + hold.forget = attachLink(hold.channelId, link); + return { ok: true, detach: () => this.#detached(hold, link) }; + } + + #detached(hold: Hold, link: PlannerLink): void { + if (hold.link !== link) return; + hold.link = undefined; + hold.forget?.(); + hold.forget = undefined; + if (this.#holds.get(hold.channelId) !== hold) return; + hold.releaseAt = new Date(this.#auth.clock().getTime() + this.#graceMs); + hold.timer = setTimeout(() => void this.#expire(hold), this.#graceMs); + hold.timer.unref?.(); + } + + #expire(hold: Hold): Promise { + return this.#serialized(hold.channelId, async () => { + if (this.#holds.get(hold.channelId) !== hold || hold.link) return; + this.#drop(hold); + await this.#release(hold); + }); + } + + /** Forgets the hold before its link closes, so closing starts no grace period. */ + #invalidate(hold: Hold, refusal: AttachRefusal): void { + if (this.#holds.get(hold.channelId) !== hold) return; + this.#drop(hold); + hold.link?.revoke(refusal.code, refusal.message); + } + + async #release(hold: Hold): Promise { + try { + await this.#auth.storage.channels.clearAgentOwner( + hold.channelId, + hold.ownerSessionId, + hold.generation, + this.#auth.clock(), + ); + } catch (error) { + console.error("[planner-link] releasing the Planner owner failed:", error); + } + } + + #drop(hold: Hold): void { + clearTimeout(hold.timer); + hold.timer = undefined; + if (this.#holds.get(hold.channelId) === hold) this.#holds.delete(hold.channelId); + } + + #serialized(channelId: string, action: () => Promise): Promise { + let previous = this.#queues.get(channelId) ?? Promise.resolve(); + let next = previous.then(action); + let settled = next.then(() => {}, () => {}); + this.#queues.set(channelId, settled); + void settled.then(() => { + if (this.#queues.get(channelId) === settled) this.#queues.delete(channelId); + }); + return next; + } +} + +function resetRefusal(login: string): AttachRefusal { + return { + ok: false, + code: "access-revoked", + message: `A repository writer reset this document's Planner, so ${login} is no longer ` + + `attached. Attach again to resume.`, + }; +} + +function attachedRefusal(hold: Hold): AttachRefusal { + let message = hold.link || !hold.releaseAt + ? `${hold.login} is attached as this document's Planner. A document has one Planner at a time.` + : `${hold.login} detached from this document's Planner and holds it until ` + + `${hold.releaseAt.toISOString()} unless they reattach. A document has one Planner at a time.`; + return { ok: false, code: "planner-attached", message }; +} + +function signInRefusal(origin: string, login: string): AttachRefusal { + return { + ok: false, + code: "sign-in-required", + message: `Sign in to Chopin at ${origin} in a browser as ${login}, then attach again. ` + + `The Planner's repository tools act with that sign-in.`, + }; +} + +function signInEndedRefusal(origin: string, login: string): AttachRefusal { + return { + ok: false, + code: "sign-in-required", + message: `The Chopin browser sign-in that made ${login} this document's Planner ended, so the ` + + `Planner was released. Sign in to Chopin at ${origin} in a browser as ${login}, then ` + + `attach again.`, + }; +} diff --git a/apps/server/src/harness/remote/endpoint.ts b/apps/server/src/harness/remote/endpoint.ts index 90cccf450..59531b47e 100644 --- a/apps/server/src/harness/remote/endpoint.ts +++ b/apps/server/src/harness/remote/endpoint.ts @@ -2,24 +2,30 @@ * `/planner-link`: where a client attaches as a document's Planner. * * The bearer is a GitHub token checked as hosted MCP checks it, before the - * socket upgrades. The `attach` that follows names a document its account must - * be able to read. Nothing is posted to the document by attaching or refusing. + * socket upgrades. The `attach` that follows names a document whose repository + * its account can push to or administer, and makes that account the document's + * Planner owner. Nothing is posted to the document by attaching or refusing. */ import { AdmissionDenied } from "../../auth/admission"; import { documentUrl } from "../../channels/document-url"; import { GitHubError } from "../../github/client"; import { locateDocument } from "../../mcp/hosted"; +import { forbiddenMessage } from "./attachments"; import { PlannerLink } from "./link"; -import { attachLink } from "./registry"; import type { ServerWebSocket } from "bun"; import type { HostedAuth } from "../../auth/routes"; import type { HostedCaller } from "../../mcp/hosted"; +import type { PlannerAttachments } from "./attachments"; import type { AttachOutcome, LinkOptions, LinkSocket } from "./link"; export type PlannerLinkAuth = Pick; +function writable(repository: { permissions: { push: boolean; admin: boolean } }): boolean { + return repository.permissions.push || repository.permissions.admin; +} + export type PlannerLinkSocketData = { plannerLink: true; sidebar?: undefined; @@ -28,8 +34,11 @@ export type PlannerLinkSocketData = { }; export type PlannerLinkHooks = { + /** One Planner per document, and its ownership. */ + attachments: PlannerAttachments; /** A document being deleted cannot be attached. */ unavailable?: (channelId: string) => boolean; + /** Observes attached links. */ register?: LinkOptions["register"]; timeoutMs?: number; revalidateEveryMs?: number; @@ -84,14 +93,15 @@ export async function admitPlannerLink( } } -/** A link for one admitted caller, attaching only documents the caller can read. */ +/** A link for one admitted caller, attaching only documents whose repository it can write. */ export function plannerLink( caller: HostedCaller, auth: PlannerLinkAuth, socket: LinkSocket, - hooks: PlannerLinkHooks = {}, + hooks: PlannerLinkHooks, ): PlannerLink { let channelId: string | undefined; + let link: PlannerLink; async function attach(message: { document: string }): Promise { let located = await locateDocument(auth, caller, message.document); if (located === "forbidden") { @@ -112,15 +122,22 @@ export function plannerLink( if (channel.archivedAt) { return { ok: false, code: "document-archived", message: "This document is archived." }; } + let fullName = `${repository.owner}/${repository.name}`; + if (!writable(repository)) { + return { ok: false, code: "repository-forbidden", message: forbiddenMessage(fullName) }; + } + let url = new URL(await documentUrl(channel, auth.storage.channels), auth.config.origin).href; + let granted = await hooks.attachments.attach({ + channelId: channel.id, + repository: { id: repository.id, owner: repository.owner, name: repository.name }, + user: { id: caller.user.id, login: caller.user.login }, + }, link); + if (!granted.ok) return granted; channelId = channel.id; return { ok: true, - document: { - id: channel.id, - title: channel.title, - repository: `${repository.owner}/${repository.name}`, - url: new URL(await documentUrl(channel, auth.storage.channels), auth.config.origin).href, - }, + document: { id: channel.id, title: channel.title, repository: fullName, url }, + detach: granted.detach, }; } async function revalidate(): Promise { @@ -129,19 +146,22 @@ export function plannerLink( await auth.admission.user(caller.oauthToken); let located = await locateDocument(auth, caller, channelId); return typeof located === "object" && located.channel.id === channelId - && !located.channel.archivedAt && !hooks.unavailable?.(channelId); + && writable(located.repository) + && !located.channel.archivedAt && !hooks.unavailable?.(channelId) + && await hooks.attachments.verify(channelId, link); } catch (err) { if (revoked(err)) return false; throw err; } } - return new PlannerLink(socket, { + link = new PlannerLink(socket, { attach, revalidate, - register: hooks.register ?? attachLink, + register: hooks.register, timeoutMs: hooks.timeoutMs, revalidateEveryMs: hooks.revalidateEveryMs, }); + return link; } export function isPlannerLinkSocket( @@ -153,7 +173,7 @@ export function isPlannerLinkSocket( export function openPlannerLinkSocket( ws: ServerWebSocket, auth: PlannerLinkAuth, - hooks: PlannerLinkHooks = {}, + hooks: PlannerLinkHooks, ): void { ws.data.link = plannerLink(ws.data.caller, auth, { send: text => void ws.send(text), diff --git a/apps/server/src/harness/remote/link.test.ts b/apps/server/src/harness/remote/link.test.ts index 2120217cb..70cb7a0c6 100644 --- a/apps/server/src/harness/remote/link.test.ts +++ b/apps/server/src/harness/remote/link.test.ts @@ -16,12 +16,15 @@ import { attach, DENIED_TOKEN, finishTurn, + GUEST_TOKEN, linkAuth, linkServer, + MEMBER_TOKEN, OUTSIDER_TOKEN, rawLink, - READER_TOKEN, + RIVAL_TOKEN, scriptedPlanner, + VIEWER_TOKEN, } from "../../testing/planner-link"; import { createRemoteAdapter } from "./adapter"; import { admitPlannerLink } from "./endpoint"; @@ -145,7 +148,7 @@ describe("Planner link admission", () => { .toBe(403); expect( (await upgradeRequest(server.url, { - authorization: `Bearer ${READER_TOKEN}`, + authorization: `Bearer ${MEMBER_TOKEN}`, origin: "https://elsewhere.test", })).status, ).toBe(403); @@ -157,7 +160,7 @@ describe("Planner link admission", () => { it("answers 404 where the server does not run the remote Planner", async () => { let { auth } = await linkAuth(); let request = new Request("https://chopin.test/planner-link", { - headers: { authorization: `Bearer ${READER_TOKEN}`, upgrade: "websocket" }, + headers: { authorization: `Bearer ${MEMBER_TOKEN}`, upgrade: "websocket" }, }); let refused = await admitPlannerLink(request, auth, false); expect(refused).toBeInstanceOf(Response); @@ -174,18 +177,18 @@ describe("Planner link admission", () => { (error: PlannerLinkRefused) => error.code, ); expect(await refusal(OUTSIDER_TOKEN, channel.id)).toBe("repository-forbidden"); - expect(await refusal(READER_TOKEN, crypto.randomUUID())).toBe("document-unavailable"); + expect(await refusal(MEMBER_TOKEN, crypto.randomUUID())).toBe("document-unavailable"); let raw = await rawLink(server.url); raw.send({ type: "attach", version: PLANNER_LINK_VERSION + 1, document: channel.id }); expect(await raw.next("refused")).toMatchObject({ code: "unsupported-version" }); expect((await raw.closed).code).toBe(LINK_CLOSE.unsupportedVersion); expect(await storage.channels.get(channel.id)).toEqual(before); await storage.channels.archive({ id: channel.id, now: new Date() }); - expect(await refusal(READER_TOKEN, channel.id)).toBe("document-archived"); + expect(await refusal(MEMBER_TOKEN, channel.id)).toBe("document-archived"); expect(server.links).toEqual([]); }); - it("attaches a reader to the document by id or URL", async () => { + it("attaches a member to the document by id or URL", async () => { let { server, channel } = await setup(); let client = await attach(server.url, channel.id, scriptedPlanner().handlers); expect(client.document).toEqual({ @@ -194,15 +197,386 @@ describe("Planner link admission", () => { repository: "octo-org/score", url: `https://chopin.test/documents/octo-org/score/${channel.slug}`, }); + await client.detach(); let byUrl = await attach(server.url, client.document.url!, scriptedPlanner().handlers); expect(byUrl.document.id).toBe(channel.id); - expect(server.links.map(link => link.channelId)).toEqual([channel.id, channel.id]); + expect(server.links.map(link => link.channelId)).toEqual([channel.id]); await byUrl.detach(); - await client.detach(); expect(server.links).toEqual([]); }); }); +describe("Planner link ownership", () => { + /** The code, message, and close code an attach is refused with. */ + async function refused(url: string, document: string, token: string) { + let raw = await rawLink(url, token); + raw.send({ type: "attach", version: PLANNER_LINK_VERSION, document }); + let refusal = await raw.next("refused"); + return { code: refusal.code, message: refusal.message, close: (await raw.closed).code }; + } + + async function owner( + storage: Awaited>["storage"], + channelId: string, + ) { + return (await storage.channels.readAgent(channelId, new Date()))?.agent; + } + + it("makes the attaching member the owner through their browser login, replacing an unattached owner", async () => { + let { auth, storage, channel, logins } = await linkAuth(); + let server = linkServer(auth); + cleanups.push(server.stop); + let stale = await storage.channels.claimAgentOwner( + channel.id, + logins.get("rival")!, + new Date(), + ); + expect(stale).toMatchObject({ ownerSessionId: logins.get("rival"), generation: 1 }); + let client = await attach(server.url, channel.id, scriptedPlanner().handlers); + cleanups.push(() => client.detach()); + expect(await owner(storage, channel.id)) + .toMatchObject({ ownerSessionId: logins.get("member"), generation: 2, status: "ready" }); + expect(server.attachments.attached(channel.id)).toBe(true); + expect(server.attachments.holder(channel.id)).toBe("member"); + }); + + it("refuses a second attach while a Planner is attached, naming the holder", async () => { + let { auth, storage, channel, logins } = await linkAuth(); + let server = linkServer(auth); + cleanups.push(server.stop); + let client = await attach(server.url, channel.id, scriptedPlanner().handlers); + cleanups.push(() => client.detach()); + let held = await owner(storage, channel.id); + expect(held).toMatchObject({ ownerSessionId: logins.get("member"), generation: 1 }); + for (let token of [RIVAL_TOKEN, MEMBER_TOKEN]) { + expect(await refused(server.url, channel.id, token)).toEqual({ + code: "planner-attached", + message: "member is attached as this document's Planner. " + + "A document has one Planner at a time.", + close: LINK_CLOSE.attached, + }); + } + expect(await owner(storage, channel.id)).toEqual(held); + expect(server.links).toHaveLength(1); + }); + + it("refuses an account that cannot push or has no browser login, and takes no ownership", async () => { + let { auth, storage, channel } = await linkAuth(); + let server = linkServer(auth); + cleanups.push(server.stop); + expect(await refused(server.url, channel.id, VIEWER_TOKEN)).toEqual({ + code: "repository-forbidden", + message: "Attaching as this document's Planner needs push or administration access to " + + "octo-org/score.", + close: LINK_CLOSE.forbidden, + }); + expect(await refused(server.url, channel.id, GUEST_TOKEN)).toEqual({ + code: "sign-in-required", + message: "Sign in to Chopin at https://chopin.test in a browser as guest, then attach " + + "again. The Planner's repository tools act with that sign-in.", + close: LINK_CLOSE.unauthorized, + }); + expect((await owner(storage, channel.id))?.ownerSessionId).toBeUndefined(); + expect(server.links).toEqual([]); + }); + + it("keeps ownership through the grace period for the same member, then releases it", async () => { + let { auth, storage, channel, logins } = await linkAuth(); + let server = linkServer(auth, 2_000, undefined, 400); + cleanups.push(server.stop); + let first = await attach(server.url, channel.id, scriptedPlanner().handlers); + await first.detach(); + expect(await owner(storage, channel.id)) + .toMatchObject({ ownerSessionId: logins.get("member"), generation: 1 }); + let refusal = await refused(server.url, channel.id, RIVAL_TOKEN); + expect(refusal).toMatchObject({ code: "planner-attached", close: LINK_CLOSE.attached }); + expect(refusal.message).toStartWith( + "member detached from this document's Planner and holds it until ", + ); + + let dropped = await rawLink(server.url); + dropped.send({ type: "attach", version: PLANNER_LINK_VERSION, document: channel.id }); + await dropped.next("attached"); + expect(await owner(storage, channel.id)) + .toMatchObject({ ownerSessionId: logins.get("member"), generation: 1 }); + dropped.socket.close(); + await dropped.closed; + await Bun.sleep(100); + expect((await owner(storage, channel.id))?.ownerSessionId).toBe(logins.get("member")); + for (let deadline = Date.now() + 3_000; Date.now() < deadline;) { + if (!(await owner(storage, channel.id))?.ownerSessionId) break; + await Bun.sleep(25); + } + expect(await owner(storage, channel.id)) + .toMatchObject({ ownerSessionId: undefined, generation: 1, status: "unavailable" }); + + let rival = await attach(server.url, channel.id, scriptedPlanner().handlers, RIVAL_TOKEN); + cleanups.push(() => rival.detach()); + expect(await owner(storage, channel.id)) + .toMatchObject({ ownerSessionId: logins.get("rival"), generation: 2 }); + }); + + it("detaches a link whose account loses push access while it stays attached", async () => { + let { auth, channel } = await linkAuth(); + let server = linkServer(auth, 300, 20); + cleanups.push(server.stop); + let client = await attach(server.url, channel.id, scriptedPlanner().handlers); + let repository = auth.github.repository; + auth.github.repository = async (token, owner, name) => ({ + ...await repository(token, owner, name), + permissions: { pull: true, push: false, admin: false }, + }); + expect(await client.closed).toEqual({ code: LINK_CLOSE.forbidden, reason: "access-revoked" }); + expect(server.links).toEqual([]); + }); + + /** A member's raw link attached to the document, with ended logins reaching `attachments`. */ + async function attachedMember(revalidateEveryMs?: number) { + let linked = await linkAuth(); + let server = linkServer(linked.auth, 2_000, revalidateEveryMs, 2_000); + cleanups.push(server.stop); + linked.revocations.add(id => server.attachments.sessionRevoked(id)); + let raw = await rawLink(server.url); + raw.send({ type: "attach", version: PLANNER_LINK_VERSION, document: linked.channel.id }); + await raw.next("attached"); + expect(server.attachments.attached(linked.channel.id)).toBe(true); + return { ...linked, server, raw }; + } + + async function rivalAttaches( + url: string, + channelId: string, + storage: Awaited>["storage"], + rival: string, + ) { + let client = await attach(url, channelId, scriptedPlanner().handlers, RIVAL_TOKEN); + cleanups.push(() => client.detach()); + expect(await owner(storage, channelId)).toMatchObject({ ownerSessionId: rival }); + } + + it("releases the Planner at once when its owner signs out of the browser", async () => { + let { server, raw, channel, storage, logins, signOut } = await attachedMember(); + expect(await signOut("member")).toBe(logins.get("member")); + expect(await raw.next("refused")).toEqual({ + type: "refused", + code: "sign-in-required", + message: "The Chopin browser sign-in that made member this document's Planner ended, so " + + "the Planner was released. Sign in to Chopin at https://chopin.test in a browser as " + + "member, then attach again.", + }); + expect((await raw.closed).code).toBe(LINK_CLOSE.unauthorized); + expect(server.attachments.attached(channel.id)).toBe(false); + expect(server.attachments.holder(channel.id)).toBeUndefined(); + expect((await owner(storage, channel.id))?.ownerSessionId).toBeUndefined(); + await rivalAttaches(server.url, channel.id, storage, logins.get("rival")!); + }); + + it("releases the Planner when its owner's browser sign-in expires", async () => { + let { server, raw, channel, storage, sessions, clock } = await attachedMember(); + clock.skewMs = 31 * 24 * 60 * 60 * 1_000; + await sessions.cleanupExpired(); + expect(await raw.next("refused")).toMatchObject({ code: "sign-in-required" }); + expect((await raw.closed).code).toBe(LINK_CLOSE.unauthorized); + expect(server.attachments.attached(channel.id)).toBe(false); + expect((await owner(storage, channel.id))?.ownerSessionId).toBeUndefined(); + }); + + it("notices an expired owner sign-in on its periodic recheck when nothing revoked it", async () => { + let { server, raw, channel, storage, clock } = await attachedMember(20); + clock.skewMs = 31 * 24 * 60 * 60 * 1_000; + expect(await raw.next("refused")).toMatchObject({ code: "sign-in-required" }); + expect((await raw.closed).code).toBe(LINK_CLOSE.unauthorized); + expect(server.attachments.attached(channel.id)).toBe(false); + expect((await owner(storage, channel.id))?.ownerSessionId).toBeUndefined(); + }); + + it("releases the Planner as sign-in-required when storage already sees its owner's sign-in expired", async () => { + let { auth, server, raw, channel, storage, clock } = await attachedMember(20); + let held = (await owner(storage, channel.id))!; + auth.clock = () => new Date(Date.now() + clock.skewMs); + clock.skewMs = 31 * 24 * 60 * 60 * 1_000; + expect(await raw.next("refused")).toMatchObject({ + code: "sign-in-required", + message: expect.stringContaining("The Chopin browser sign-in that made member"), + }); + expect((await raw.closed).code).toBe(LINK_CLOSE.unauthorized); + expect(server.attachments.attached(channel.id)).toBe(false); + clock.skewMs = 0; + expect(await owner(storage, channel.id)).toMatchObject({ + ownerSessionId: undefined, + generation: held.generation, + status: "unavailable", + }); + }); + + /** Holds the next attach's ownership read until `resume`, once `reading` says it started. */ + function pauseOwnerRead(storage: Awaited>["storage"]) { + let readAgent = storage.channels.readAgent; + let reading = Promise.withResolvers(); + let resume = Promise.withResolvers(); + storage.channels.readAgent = async (channelId, now) => { + storage.channels.readAgent = readAgent; + let snapshot = await readAgent(channelId, now); + reading.resolve(); + await resume.promise; + return snapshot; + }; + return { reading: reading.promise, resume: () => resume.resolve() }; + } + + /** A member detached within the grace period, reattaching on a raw link paused at its ownership read. */ + async function reattachingMember() { + let linked = await linkAuth(); + let server = linkServer(linked.auth, 2_000, undefined, 2_000); + cleanups.push(server.stop); + linked.revocations.add(id => server.attachments.sessionRevoked(id)); + let first = await attach(server.url, linked.channel.id, scriptedPlanner().handlers); + await first.detach(); + let held = (await owner(linked.storage, linked.channel.id))!; + let paused = pauseOwnerRead(linked.storage); + let raw = await rawLink(server.url); + raw.send({ type: "attach", version: PLANNER_LINK_VERSION, document: linked.channel.id }); + await paused.reading; + return { ...linked, server, raw, held, resume: paused.resume }; + } + + it("refuses a grace reattach whose ownership a reset released while it was attaching", async () => { + let { server, raw, channel, storage, logins, held, resume } = await reattachingMember(); + await storage.channels.clearAgentOwner( + channel.id, + held.ownerSessionId!, + held.generation, + new Date(), + ); + server.attachments.ownerReset(channel.id); + resume(); + expect(await raw.next("refused")).toEqual({ + type: "refused", + code: "access-revoked", + message: "A repository writer reset this document's Planner, so member is no longer " + + "attached. Attach again to resume.", + }); + expect((await raw.closed).code).toBe(LINK_CLOSE.forbidden); + expect(server.attachments.attached(channel.id)).toBe(false); + expect(server.attachments.holder(channel.id)).toBeUndefined(); + expect(server.links).toEqual([]); + expect((await owner(storage, channel.id))?.ownerSessionId).toBeUndefined(); + await rivalAttaches(server.url, channel.id, storage, logins.get("rival")!); + }); + + it("refuses a grace reattach whose owner signed out while it was attaching", async () => { + let { server, raw, channel, storage, logins, signOut, resume } = await reattachingMember(); + expect(await signOut("member")).toBe(logins.get("member")); + resume(); + expect(await raw.next("refused")).toMatchObject({ + code: "sign-in-required", + message: expect.stringContaining("The Chopin browser sign-in that made member"), + }); + expect((await raw.closed).code).toBe(LINK_CLOSE.unauthorized); + expect(server.attachments.attached(channel.id)).toBe(false); + expect(server.links).toEqual([]); + expect((await owner(storage, channel.id))?.ownerSessionId).toBeUndefined(); + await rivalAttaches(server.url, channel.id, storage, logins.get("rival")!); + }); + + /** A member's fresh attach on a raw link, paused while GitHub checks its repository access. */ + async function checkingMember() { + let linked = await linkAuth(); + let server = linkServer(linked.auth); + cleanups.push(server.stop); + linked.revocations.add(id => server.attachments.sessionRevoked(id)); + let { github } = linked.auth; + let repositoryAccess = github.repositoryAccess; + let checking = Promise.withResolvers(); + let resume = Promise.withResolvers(); + github.repositoryAccess = async (token, repositoryOwner, name) => { + github.repositoryAccess = repositoryAccess; + checking.resolve(); + await resume.promise; + return repositoryAccess(token, repositoryOwner, name); + }; + let raw = await rawLink(server.url); + raw.send({ type: "attach", version: PLANNER_LINK_VERSION, document: linked.channel.id }); + await checking.promise; + return { ...linked, server, raw, resume: () => resume.resolve() }; + } + + async function refusedSignIn( + attempt: Awaited>, + ) { + let { server, raw, channel, storage } = attempt; + expect(await raw.next("refused")).toEqual({ + type: "refused", + code: "sign-in-required", + message: "Sign in to Chopin at https://chopin.test in a browser as member, then attach " + + "again. The Planner's repository tools act with that sign-in.", + }); + expect(await raw.closed).toEqual({ code: LINK_CLOSE.unauthorized, reason: "sign-in-required" }); + expect(server.attachments.attached(channel.id)).toBe(false); + expect(server.attachments.holder(channel.id)).toBeUndefined(); + expect(server.links).toEqual([]); + expect((await owner(storage, channel.id))?.ownerSessionId).toBeUndefined(); + } + + it("refuses a fresh attach whose owner signed out while its repository access was checked", async () => { + let attempt = await checkingMember(); + expect(await attempt.signOut("member")).toBe(attempt.logins.get("member")); + attempt.resume(); + await refusedSignIn(attempt); + let { server, channel, storage, logins } = attempt; + await rivalAttaches(server.url, channel.id, storage, logins.get("rival")!); + }); + + it("refuses a fresh attach whose sign-in storage saw expire before it claimed ownership", async () => { + let attempt = await checkingMember(); + attempt.auth.clock = () => new Date(Date.now() + attempt.clock.skewMs); + attempt.clock.skewMs = 31 * 24 * 60 * 60 * 1_000; + attempt.resume(); + await refusedSignIn(attempt); + }); + + it("releases the attachment when a repository writer resets the document's Planner", async () => { + let { server, raw, channel, storage, logins } = await attachedMember(); + let held = (await owner(storage, channel.id))!; + await storage.channels.clearAgentOwner( + channel.id, + held.ownerSessionId!, + held.generation, + new Date(), + ); + server.attachments.ownerReset(channel.id); + expect(await raw.next("refused")).toEqual({ + type: "refused", + code: "access-revoked", + message: "A repository writer reset this document's Planner, so member is no longer " + + "attached. Attach again to resume.", + }); + expect((await raw.closed).code).toBe(LINK_CLOSE.forbidden); + expect(server.attachments.attached(channel.id)).toBe(false); + await rivalAttaches(server.url, channel.id, storage, logins.get("rival")!); + }); + + it("closes a link whose stored ownership was released elsewhere on its next recheck", async () => { + let { server, raw, channel, storage, logins } = await attachedMember(20); + let held = (await owner(storage, channel.id))!; + await storage.channels.clearAgentOwner( + channel.id, + held.ownerSessionId!, + held.generation, + new Date(), + ); + expect(await raw.next("refused")).toEqual({ + type: "refused", + code: "access-revoked", + message: "This document's Planner ownership was released, so member is no longer " + + "attached. Attach again to resume.", + }); + expect((await raw.closed).code).toBe(LINK_CLOSE.forbidden); + expect(server.attachments.attached(channel.id)).toBe(false); + await rivalAttaches(server.url, channel.id, storage, logins.get("rival")!); + }); +}); + describe("Planner link protocol", () => { it("closes the link on malformed or out-of-order messages", async () => { let { server, channel } = await setup(); @@ -416,7 +790,7 @@ describe("Planner link protocol", () => { throw new GitHubError("Not Found", 404); }; await expect(harness.doStart({ sessionId: "second", sandboxSession, sessionWorkDir: "/tmp" })) - .rejects.toThrow("can no longer read this document"); + .rejects.toThrow("can no longer act for this document"); expect(server.links).toEqual([]); }); @@ -430,7 +804,7 @@ describe("Planner link protocol", () => { throw new GitHubError("Not Found", 404); }; let revoked = await streamOn(agent, session, "second"); - expect(revoked.errors.join("\n")).toContain("can no longer read this document"); + expect(revoked.errors.join("\n")).toContain("can no longer act for this document"); expect(server.links).toEqual([]); }); @@ -457,7 +831,7 @@ describe("Planner link protocol", () => { throw new GitHubError("Bad credentials", 401); }; let revoked = await streamOn(agent, session, "second"); - expect(revoked.errors.join("\n")).toContain("can no longer read this document"); + expect(revoked.errors.join("\n")).toContain("can no longer act for this document"); expect(await client.closed).toEqual({ code: LINK_CLOSE.forbidden, reason: "access-revoked" }); expect(server.links).toEqual([]); }); @@ -527,6 +901,8 @@ describe("Planner link protocol", () => { })(); let outcome = await streamTurn(harness, "reuse"); await driving; + raw.socket.close(); + await raw.closed; return outcome.errors.join("\n"); } readPlanRuns = 0; diff --git a/apps/server/src/harness/remote/link.ts b/apps/server/src/harness/remote/link.ts index 80679f2a2..f84f370f3 100644 --- a/apps/server/src/harness/remote/link.ts +++ b/apps/server/src/harness/remote/link.ts @@ -44,11 +44,16 @@ export type LinkSocket = { }; export type AttachOutcome = - | { ok: true; document: ServerMessageOf<"attached">["document"] } + | { + ok: true; + document: ServerMessageOf<"attached">["document"]; + /** Lets go of what attaching took, such as the document's Planner ownership. */ + detach?: () => void; + } | { ok: false; code: RefusalCode; message: string }; export type LinkOptions = { - /** Resolves the document the client asked for, refusing what its caller cannot read. */ + /** Resolves the document the client asked for and takes its Planner, or refuses. */ attach(message: ClientMessageOf<"attach">): Promise; /** Rechecks the caller's access; false closes the link. */ revalidate?(): Promise; @@ -65,6 +70,8 @@ const REFUSAL_CLOSE: Record = { "access-revoked": LINK_CLOSE.forbidden, "document-unavailable": LINK_CLOSE.unavailable, "document-archived": LINK_CLOSE.unavailable, + "planner-attached": LINK_CLOSE.attached, + "sign-in-required": LINK_CLOSE.unauthorized, unavailable: LINK_CLOSE.failed, }; @@ -601,11 +608,14 @@ export class PlannerLink { } } - /** Closes the link with `access-revoked` and throws when the caller can no longer read the document. */ + /** Closes the link with `access-revoked` and throws when the caller may no longer be the Planner. */ async #recheck(): Promise { if (this.#options.revalidate && !await this.#options.revalidate()) { - this.#refuse("access-revoked", "The attached account can no longer read this document."); - throw new Error("The attached Planner can no longer read this document."); + this.#refuse( + "access-revoked", + "The attached account can no longer act as this document's Planner.", + ); + throw new Error("The attached Planner can no longer act for this document."); } if (this.#state !== "attached") throw new Error("The Planner link closed."); } @@ -633,6 +643,12 @@ export class PlannerLink { this.#dispatch(message); } + /** Tells the client why it is no longer the Planner, then closes the link. */ + revoke(code: RefusalCode, message: string): void { + if (this.#state === "closed") return; + this.#refuse(code, message); + } + /** Ends the link from the server side. */ close(code: number, reason: string): void { if (this.#state === "closed") return; @@ -693,12 +709,20 @@ export class PlannerLink { console.error("[planner-link] attach failed:", error); outcome = { ok: false, code: "unavailable", message: "The document could not be checked." }; } - if (this.#state !== "attaching") return; + if (this.#state !== "attaching") { + if (outcome.ok) outcome.detach?.(); + return; + } if (!outcome.ok) return this.#refuse(outcome.code, outcome.message); this.#channelId = outcome.document.id; this.#state = "attached"; this.#send({ type: "attached", version: PLANNER_LINK_VERSION, document: outcome.document }); - this.#forget = this.#options.register?.(outcome.document.id, this); + let unregister = this.#options.register?.(outcome.document.id, this); + let detach = outcome.detach; + this.#forget = () => { + unregister?.(); + detach?.(); + }; this.#watch = setInterval( () => void this.#recheckAttached(), this.#options.revalidateEveryMs ?? REVALIDATE_EVERY_MS, diff --git a/apps/server/src/harness/remote/registry.ts b/apps/server/src/harness/remote/registry.ts index a50ed83eb..1b4075413 100644 --- a/apps/server/src/harness/remote/registry.ts +++ b/apps/server/src/harness/remote/registry.ts @@ -26,7 +26,7 @@ export type RemotePlanner = }; let planners = new Map(); -let links = new Map(); +let links = new Map(); export function registerRemotePlanner(sessionId: string, planner: RemotePlanner): () => void { if (planners.has(sessionId)) throw new Error("Remote Planner session is already registered"); @@ -40,19 +40,14 @@ export function remotePlanner(sessionId: string): RemotePlanner | undefined { return planners.get(sessionId); } -/** - * Records a link attached to a document. Until one Planner per document is - * enforced, the most recent attachment serves new sessions. - */ +/** Records the link attached as a document's Planner; `PlannerAttachments` admits one per document. */ export function attachLink(channelId: string, link: PlannerLink): () => void { - links.set(channelId, [...links.get(channelId) ?? [], link]); + links.set(channelId, link); return () => { - let remaining = (links.get(channelId) ?? []).filter(candidate => candidate !== link); - if (remaining.length) links.set(channelId, remaining); - else links.delete(channelId); + if (links.get(channelId) === link) links.delete(channelId); }; } export function attachedLink(channelId: string): PlannerLink | undefined { - return links.get(channelId)?.at(-1); + return links.get(channelId); } diff --git a/apps/server/src/main.ts b/apps/server/src/main.ts index aebbbd425..e9de009c9 100644 --- a/apps/server/src/main.ts +++ b/apps/server/src/main.ts @@ -18,6 +18,8 @@ import { isPlannerLinkSocket, openPlannerLinkSocket, } from "./harness/remote/endpoint"; +import { notAttachedMessage, PlannerAttachments } from "./harness/remote/attachments"; +import { documentUrl } from "./channels/document-url"; import { ActiveOwnerBindings } from "./agent/active-owner"; import { registerAuthRoutes } from "./auth/routes"; import { registerExperimentRoutes } from "./experiments/routes"; @@ -79,7 +81,7 @@ import { StorageError } from "./storage/errors"; import { createStorage } from "./storage/registry"; import { broadcast, fail, relay, reply, repositoryTopic, tell, topic } from "./wire"; -import { PLANNER_LINK_PATH } from "@chopin/planner-link"; +import { LINK_CLOSE, PLANNER_LINK_PATH } from "@chopin/planner-link"; import type { Server, ServerWebSocket } from "bun"; import type { PlannerLinkSocketData } from "./harness/remote/endpoint"; @@ -128,6 +130,7 @@ let renewingLease: Promise | undefined; let sessionCleanup: ReturnType | undefined; let cleaningSessions: Promise | undefined; let ownerBindings: ActiveOwnerBindings | undefined; +let plannerAttachments: PlannerAttachments | undefined; let jobRunner: JobRunner | undefined; let researchService: ResearchWorkspaceService | undefined; let researchRecoveryTimer: ReturnType | undefined; @@ -307,6 +310,19 @@ function conversation( auth: hostedAuth, claimantSessionId, repository, + plannerNotAttached: plannerAttachments + ? async () => { + if (await plannerAttachments!.confirmed(room.id)) return undefined; + let channel = await storage.channels.get(room.id); + let path = channel + ? await documentUrl(channel, storage.channels) + : `/channels/${room.id}`; + return notAttachedMessage( + new URL(path, hostedAuth.config.origin).href, + `${repository.owner}/${repository.name}`, + ); + } + : undefined, persist: chat => Service.persist(opened, chat), activeOwner: () => ownerBindings!.resolve(room.id), ownerAvailable: () => jobRunner?.ownerAvailable(room.id) ?? Promise.resolve(), @@ -674,6 +690,7 @@ async function receive(ws: Socket, raw: string): Promise { refreshAccess, unavailable: id => archivingChannels.has(id) || deletingChannels.has(id), ownerAvailable: id => jobRunner?.ownerAvailable(id) ?? Promise.resolve(), + claimsOwnership: !plannerAttachments, placeReference: placeResearchReference, scheduleRecovery: scheduleResearchRecovery, }), @@ -1148,8 +1165,7 @@ function listen(): Server { return upgraded(req, self, outcome.data); } if (url.pathname === PLANNER_LINK_PATH) { - let enabled = config.agent && config.harness === "remote"; - let outcome = await admitPlannerLink(req, hostedAuth, enabled); + let outcome = await admitPlannerLink(req, hostedAuth, !!plannerAttachments); if (outcome instanceof Response) return outcome; return upgraded(req, self, outcome); } @@ -1169,7 +1185,9 @@ function listen(): Server { websocket: { open(ws: AnySocket) { if (isPlannerLinkSocket(ws)) { + if (!plannerAttachments) return ws.close(LINK_CLOSE.failed, "unavailable"); openPlannerLinkSocket(ws, hostedAuth, { + attachments: plannerAttachments, unavailable: channelId => deletingChannels.has(channelId), }); } else if (isSidebarSocket(ws)) scheduleSidebarRecheck(ws); @@ -1234,6 +1252,7 @@ function drain(): Promise { summaryCoordinator?.close(); let stoppingJobs = jobRunner?.shutdown(); ownerBindings?.revokeAll(); + plannerAttachments?.close(); for (let result of await Promise.allSettled([stoppingJobs])) { if (result.status === "rejected") record(result.reason); } @@ -1601,7 +1620,11 @@ async function sessionRevoked(sessionId: string): Promise { experiments?.connections.revokeSession(sessionId); let jobs = jobRunner?.ownerRevoked(sessionId); ownerBindings?.revokeSession(sessionId); - await Promise.all([resetOpenAgents(() => true, sessionId), jobs]); + await Promise.all([ + resetOpenAgents(() => true, sessionId), + jobs, + plannerAttachments?.sessionRevoked(sessionId), + ]); } async function credentialsWillRotate(sessionId: string, revision: number): Promise { @@ -1621,6 +1644,7 @@ async function credentialsWillRotate(sessionId: string, revision: number): Promi async function channelOwnerReset(channelId: string): Promise { let jobs = jobRunner?.channelOwnerReset(channelId); ownerBindings?.revokeChannel(channelId); + plannerAttachments?.ownerReset(channelId); await Promise.all([resetOpenAgents(room => room.id === channelId), jobs]); } @@ -1672,6 +1696,9 @@ experiments = registerExperimentRoutes(router, hostedAuth, { }, }); ownerBindings = new ActiveOwnerBindings(hostedAuth); +if (config.agent && config.harness === "remote") { + plannerAttachments = new PlannerAttachments(hostedAuth, config.plannerAttachGraceMs); +} let definitions: JobDefinition[] = []; if (config.backgroundJobs) { definitions.push(researchBriefDefinition({ config })); @@ -1888,7 +1915,7 @@ registerResearchWorkspaceRoutes(router, hostedAuth, { defaultBranch: repository.defaultBranch, }, channel.id, - session.session.id, + plannerAttachments ? undefined : session.session.id, ); await jobRunner?.ownerAvailable(channel.id); }, diff --git a/apps/server/src/mcp.ts b/apps/server/src/mcp.ts index 00ed96b66..93c01a309 100644 --- a/apps/server/src/mcp.ts +++ b/apps/server/src/mcp.ts @@ -155,6 +155,7 @@ export type InvokePlannerError = | "document-unavailable" | "repository-forbidden" | "document-archived" + | "planner-not-attached" | "planner-owner-unavailable" | "checkout-unverified" | "planner-unavailable" @@ -163,7 +164,7 @@ export type InvokePlannerError = export type InvokePlanner = { invoke(caller: Caller, input: InvokePlannerInput): Promise< | { kind: "invoked"; document: DocumentSummary & { url: string } } - | { kind: "refused"; code: InvokePlannerError } + | { kind: "refused"; code: InvokePlannerError; message?: string } >; }; @@ -518,7 +519,10 @@ export const TOOLS: Tool[] = [ description: "Post an instruction to a document's Planner, attributed to the caller, and " + "return its URL without waiting for the turn. The turn runs under the document's " + "Planner owner; without one, the caller's live Chopin browser login becomes the " - + "owner, and otherwise the call is refused. Questions appear in Decisions.", + + "owner, and otherwise the call is refused. Where the Planner runs in an attached " + + "client, a document with no attached Planner refuses the call with " + + "planner-not-attached and a message saying how to attach one. Questions appear in " + + "Decisions.", inputSchema: { type: "object", properties: { @@ -554,15 +558,27 @@ export const TOOLS: Tool[] = [ required: ["id", "title", "url"], additionalProperties: false, }, - outcome([ - "document-unavailable", - "repository-forbidden", - "document-archived", - "planner-owner-unavailable", - "checkout-unverified", - "planner-unavailable", - "planner-queue-full", - ]), + { + type: "object", + properties: { + code: { + type: "string", + enum: [ + "document-unavailable", + "repository-forbidden", + "document-archived", + "planner-not-attached", + "planner-owner-unavailable", + "checkout-unverified", + "planner-unavailable", + "planner-queue-full", + ], + }, + message: { type: "string" }, + }, + required: ["code"], + additionalProperties: false, + }, ], }, }, @@ -1098,7 +1114,10 @@ export function handler( let result = await options.invoke.invoke(caller, args as InvokePlannerInput); return result.kind === "invoked" ? respond(text(result.document)) - : respond(text({ code: result.code }, true)); + : respond(text({ + code: result.code, + ...(result.message ? { message: result.message } : {}), + }, true)); } if (tool.name === "upload_image" && options.upload) { let prepared = prepareImage(tool.arguments); diff --git a/apps/server/src/mcp/hosted.test.ts b/apps/server/src/mcp/hosted.test.ts index fe33a4164..9675a868d 100644 --- a/apps/server/src/mcp/hosted.test.ts +++ b/apps/server/src/mcp/hosted.test.ts @@ -276,6 +276,30 @@ describe("the hosted MCP adapter", () => { } }); + it("tells an invocation with no attached Planner how to attach one, naming the document URL", async () => { + let context = setup(); + context.github.repositoryValue.permissions.push = true; + let opened = await plan(context); + let adapter = hosted(context.auth, undefined, { + invokePlanner: async () => "planner-not-attached", + }); + let caller = (await adapter.caller(request("Bearer allowed")))!; + try { + expect( + await adapter.invoke!.invoke(caller, { id: opened.channel.id, instruction: "Plan it." }), + ).toEqual({ + kind: "refused", + code: "planner-not-attached", + message: "No Planner is attached to this document. To attach one, run pdd-plan with " + + "plan_document set to " + + "https://chopin.test/documents/octo-org/score/release-readiness from a session in " + + "a checkout of octo-org/score; that session becomes this document's Planner.", + }); + } finally { + await Service.close(opened.plan); + } + }); + it("passes a URL invocation's checkout through unverified for the harness to judge", async () => { let context = setup(); context.github.repositoryValue.permissions.admin = true; diff --git a/apps/server/src/mcp/hosted.ts b/apps/server/src/mcp/hosted.ts index 69a585c0f..6c574489c 100644 --- a/apps/server/src/mcp/hosted.ts +++ b/apps/server/src/mcp/hosted.ts @@ -7,6 +7,7 @@ import { } from "@chopin/protocol/document-url"; import { documentUrl } from "../channels/document-url"; +import { notAttachedMessage } from "../harness/remote/attachments"; import { deterministicChannelId, isChannelId } from "../channels/id"; import { documentSlug } from "../channels/slug"; import { GitHubError } from "../github/client"; @@ -336,15 +337,17 @@ export function hosted( instruction: input.instruction, ...(input.checkout === undefined ? {} : { checkout: input.checkout }), }); + let url = + new URL(await documentUrl(channel, auth.storage.channels), auth.config.origin).href; + if (code === "planner-not-attached") { + return { + kind: "refused", + code, + message: notAttachedMessage(url, `${repository.owner}/${repository.name}`), + }; + } if (code) return { kind: "refused", code }; - return { - kind: "invoked", - document: { - ...summary(channel), - url: - new URL(await documentUrl(channel, auth.storage.channels), auth.config.origin).href, - }, - }; + return { kind: "invoked", document: { ...summary(channel), url } }; }, } : undefined, diff --git a/apps/server/src/mcp/invoke.test.ts b/apps/server/src/mcp/invoke.test.ts index e774e92eb..b17f3b7c2 100644 --- a/apps/server/src/mcp/invoke.test.ts +++ b/apps/server/src/mcp/invoke.test.ts @@ -93,6 +93,7 @@ test("invoke_planner exposes each refusal as an object code", async () => { "document-unavailable", "repository-forbidden", "document-archived", + "planner-not-attached", "planner-owner-unavailable", "checkout-unverified", "planner-unavailable", @@ -107,3 +108,22 @@ test("invoke_planner exposes each refusal as an object code", async () => { expect(result.result).toMatchObject({ isError: true, structuredContent: { code } }); } }); + +test("invoke_planner carries a refusal's message beside its code", async () => { + let message = "No Planner is attached to this document. Run pdd-plan to attach one."; + let mcp = endpoint({ + invoke: async () => ({ kind: "refused", code: "planner-not-attached", message }), + }); + let result = await call(mcp, "tools/call", { + name: "invoke_planner", + arguments: { id: "document-id", instruction: "go" }, + }); + expect(result.result).toMatchObject({ + isError: true, + structuredContent: { code: "planner-not-attached", message }, + }); + expect(JSON.parse(result.result.content[0].text)).toEqual({ + code: "planner-not-attached", + message, + }); +}); diff --git a/apps/server/src/storage/contract.ts b/apps/server/src/storage/contract.ts index 36c7ce3f6..f9931735a 100644 --- a/apps/server/src/storage/contract.ts +++ b/apps/server/src/storage/contract.ts @@ -1,6 +1,7 @@ import { researchWorkspace } from "./research-inline-contract-fixtures"; import { experimentContract } from "./experiment-contract"; import { imageContract } from "./image-contract"; +import { plannerAttachmentContract } from "./planner-attachment-contract"; import { researchInlineContract } from "./research-inline-contract"; import { describe, expect, it } from "bun:test"; @@ -35,6 +36,7 @@ function attempt(action: () => Promise): Promise { export function storageContract(name: string, factory: Factory): void { experimentContract(name, factory); imageContract(name, factory); + plannerAttachmentContract(name, factory); describe(`${name} storage`, () => { it("supports expiring metadata-only sessions for local authentication", async () => { let storage = await opened(factory); diff --git a/apps/server/src/storage/planner-attachment-contract.ts b/apps/server/src/storage/planner-attachment-contract.ts new file mode 100644 index 000000000..6b1745d48 --- /dev/null +++ b/apps/server/src/storage/planner-attachment-contract.ts @@ -0,0 +1,252 @@ +import { describe, expect, test } from "bun:test"; + +import { Sessions } from "../auth/session"; +import { PlannerAttachments } from "../harness/remote/attachments"; +import { contractId as id, openedStorage } from "./contract-support"; + +import type { AttachmentAuth } from "../harness/remote/attachments"; +import type { PlannerLink } from "../harness/remote/link"; +import type { StorageFactory } from "./contract-support"; +import type { StorageAdapter } from "./port"; + +const GRANT = { accessExpiresIn: 28_800, refreshToken: "refresh", refreshExpiresIn: 86_400 }; + +async function attachable(storage: StorageAdapter) { + let now = new Date(); + let member = { id: id("member"), login: "member" }; + let rival = { id: id("rival"), login: "rival" }; + for (let user of [member, rival]) await storage.users.put({ ...user, avatarUrl: "", now }); + let channelId = id("channel"); + let repository = { id: id("repository"), owner: "octo-org", name: "score" }; + await storage.channels.create({ + id: channelId, + repositoryId: repository.id, + repositoryOwner: repository.owner, + repositoryName: repository.name, + title: "Attached document", + createdBy: member.id, + now, + }); + let skew = { ms: 0 }; + let clock = () => new Date(Date.now() + skew.ms); + let sessions = new Sessions(storage, false, clock); + let signIn = async (user: { id: string }) => + (await sessions.issue(user.id, { ...GRANT, accessToken: `${user.id}-login` })).id; + let logins = { member: await signIn(member), rival: await signIn(rival) }; + let auth: AttachmentAuth = { + config: { origin: "https://chopin.test" }, + storage, + sessions, + clock, + admission: { allowed: async () => true }, + github: { + repositoryAccess: async () => ({ + ...repository, + fullName: "octo-org/score", + private: true, + url: "https://github.test/octo-org/score", + defaultBranch: "main", + permissions: { pull: true, push: true, admin: false }, + }), + }, + }; + let owner = async () => (await storage.channels.readAgent(channelId, new Date()))?.agent; + let request = (user: { id: string; login: string }) => ({ channelId, repository, user }); + return { auth, sessions, member, rival, logins, channelId, owner, request, skew }; +} + +function link(): PlannerLink { + return {} as PlannerLink; +} + +function revocable() { + let refusals: { code: string; message: string }[] = []; + let revocable = { revoke: (code: string, message: string) => refusals.push({ code, message }) }; + return { link: revocable as unknown as PlannerLink, refusals }; +} + +async function released(owner: () => Promise<{ ownerSessionId?: string } | undefined>) { + for (let deadline = Date.now() + 3_000; Date.now() < deadline;) { + if (!(await owner())?.ownerSessionId) return; + await Bun.sleep(10); + } +} + +/** Remote Planner attachments over each storage adapter's generation-guarded ownership. */ +export function plannerAttachmentContract(name: string, factory: StorageFactory): void { + describe(`${name} Planner attachments`, () => { + test("an attachment owns the document, refuses another login, and survives its own reattach", async () => { + let storage = await openedStorage(factory); + let f = await attachable(storage); + let attachments = new PlannerAttachments(f.auth, 60_000); + try { + let granted = await attachments.attach(f.request(f.member), link()); + expect(granted.ok).toBe(true); + let held = await f.owner(); + expect(held).toMatchObject({ ownerSessionId: f.logins.member, status: "ready" }); + expect(await attachments.attach(f.request(f.rival), link())).toMatchObject({ + ok: false, + code: "planner-attached", + message: expect.stringContaining("member is attached"), + }); + if (granted.ok) granted.detach(); + expect(attachments.attached(f.channelId)).toBe(false); + expect(await f.owner()).toEqual(held); + expect(await attachments.attach(f.request(f.rival), link())).toMatchObject({ + ok: false, + code: "planner-attached", + message: expect.stringContaining("member detached"), + }); + expect((await attachments.attach(f.request(f.member), link())).ok).toBe(true); + expect(attachments.attached(f.channelId)).toBe(true); + expect(await f.owner()).toEqual(held); + } finally { + attachments.close(); + await storage.close(); + } + }); + + test("the grace period's end clears only the ownership the attachment still holds", async () => { + let storage = await openedStorage(factory); + let f = await attachable(storage); + let attachments = new PlannerAttachments(f.auth, 30); + try { + let first = await attachments.attach(f.request(f.member), link()); + let generation = (await f.owner())!.generation; + if (first.ok) first.detach(); + await released(f.owner); + expect(await f.owner()).toMatchObject({ + ownerSessionId: undefined, + generation, + status: "unavailable", + }); + expect(attachments.holder(f.channelId)).toBeUndefined(); + + let rival = await attachments.attach(f.request(f.rival), link()); + expect(await f.owner()) + .toMatchObject({ ownerSessionId: f.logins.rival, generation: generation + 1 }); + if (rival.ok) rival.detach(); + let channels = storage.channels; + expect( + await channels.clearAgentOwner(f.channelId, f.logins.rival, generation + 1, new Date()), + ) + .toBe(true); + await channels.claimAgentOwner(f.channelId, f.logins.member, new Date()); + await Bun.sleep(80); + expect(await f.owner()) + .toMatchObject({ ownerSessionId: f.logins.member, generation: generation + 2 }); + } finally { + attachments.close(); + await storage.close(); + } + }); + + test("startup clears an attachment's ownership and a new process starts unattached", async () => { + let storage = await openedStorage(factory); + let f = await attachable(storage); + let attachments = new PlannerAttachments(f.auth, 60_000); + try { + await attachments.attach(f.request(f.member), link()); + let generation = (await f.owner())!.generation; + let lease = await storage.leases.acquire(id("writer"), id("instance"), 60_000); + await storage.sessions.reset(new Date(), lease!, 60_000); + expect(await f.owner()).toMatchObject({ ownerSessionId: undefined, generation }); + + let sessions = new Sessions(storage, false); + let restarted = new PlannerAttachments({ ...f.auth, sessions }, 60_000); + try { + expect(restarted.attached(f.channelId)).toBe(false); + expect(await restarted.attach(f.request(f.member), link())).toMatchObject({ + ok: false, + code: "sign-in-required", + }); + let login = await sessions.issue(f.member.id, { ...GRANT, accessToken: "again" }); + expect((await restarted.attach(f.request(f.member), link())).ok).toBe(true); + expect(await f.owner()) + .toMatchObject({ ownerSessionId: login.id, generation: generation + 1 }); + } finally { + restarted.close(); + } + } finally { + attachments.close(); + await storage.close(); + } + }); + + test("an owner sign-in storage already sees expired releases the attachment as sign-in-required", async () => { + let storage = await openedStorage(factory); + let f = await attachable(storage); + let attachments = new PlannerAttachments(f.auth, 60_000); + try { + let attached = revocable(); + expect((await attachments.attach(f.request(f.member), attached.link)).ok).toBe(true); + let generation = (await f.owner())!.generation; + f.skew.ms = 31 * 24 * 60 * 60 * 1_000; + expect((await storage.channels.readAgent(f.channelId, f.auth.clock()))?.agent) + .toMatchObject({ ownerSessionId: undefined, generation }); + expect(await attachments.verify(f.channelId, attached.link)).toBe(false); + expect(attached.refusals).toEqual([{ + code: "sign-in-required", + message: expect.stringContaining("The Chopin browser sign-in that made member"), + }]); + f.skew.ms = 0; + expect(await f.owner()) + .toMatchObject({ ownerSessionId: undefined, generation, status: "unavailable" }); + expect(attachments.holder(f.channelId)).toBeUndefined(); + } finally { + attachments.close(); + await storage.close(); + } + }); + + test("a reset during a grace reattach refuses it and leaves the document free", async () => { + let storage = await openedStorage(factory); + let f = await attachable(storage); + let reading = Promise.withResolvers(); + let resume = Promise.withResolvers(); + let paused = false; + let channels = { + ...storage.channels, + readAgent: async (channelId: string, now: Date) => { + let snapshot = await storage.channels.readAgent(channelId, now); + if (!paused) return snapshot; + paused = false; + reading.resolve(); + await resume.promise; + return snapshot; + }, + }; + let attachments = new PlannerAttachments({ ...f.auth, storage: { channels } }, 60_000); + try { + let first = await attachments.attach(f.request(f.member), link()); + if (first.ok) first.detach(); + let held = (await f.owner())!; + paused = true; + let reattached = revocable(); + let reattaching = attachments.attach(f.request(f.member), reattached.link); + await reading.promise; + expect( + await storage.channels.clearAgentOwner( + f.channelId, + held.ownerSessionId!, + held.generation, + new Date(), + ), + ).toBe(true); + attachments.ownerReset(f.channelId); + resume.resolve(); + expect(await reattaching).toMatchObject({ ok: false, code: "access-revoked" }); + expect(attachments.attached(f.channelId)).toBe(false); + expect(attachments.holder(f.channelId)).toBeUndefined(); + expect(await f.owner()) + .toMatchObject({ ownerSessionId: undefined, generation: held.generation }); + expect((await attachments.attach(f.request(f.rival), link())).ok).toBe(true); + expect(await f.owner()) + .toMatchObject({ ownerSessionId: f.logins.rival, generation: held.generation + 1 }); + } finally { + attachments.close(); + await storage.close(); + } + }); + }); +} diff --git a/apps/server/src/testing/config.ts b/apps/server/src/testing/config.ts index cc6e64fd6..55cd854f6 100644 --- a/apps/server/src/testing/config.ts +++ b/apps/server/src/testing/config.ts @@ -29,6 +29,7 @@ export function configured(overrides: Record = {}) { JEV_API_KEY: undefined, JEV_MODEL: undefined, JEV_TIMEOUT_MS: undefined, + PLANNER_ATTACH_GRACE_MS: undefined, TYPESAFE_API_KEY: undefined, HARNESS: undefined, HARNESS_AUTH: undefined, diff --git a/apps/server/src/testing/planner-link.ts b/apps/server/src/testing/planner-link.ts index 42626f853..55a4b38a9 100644 --- a/apps/server/src/testing/planner-link.ts +++ b/apps/server/src/testing/planner-link.ts @@ -2,6 +2,8 @@ import { connectPlannerLink, linkSocket, plannerLinkUrl } from "@chopin/planner- import { AdmissionDenied } from "../auth/admission"; import { GitHubError } from "../github/client"; +import { Sessions } from "../auth/session"; +import { PlannerAttachments } from "../harness/remote/attachments"; import { admitPlannerLink, openPlannerLinkSocket } from "../harness/remote/endpoint"; import { MemoryStorage } from "../storage/memory/adapter"; @@ -11,64 +13,139 @@ import type { TurnContext, } from "@chopin/planner-link/client"; import type { Repository } from "../github/client"; +import type { AttachmentAuth } from "../harness/remote/attachments"; import type { PlannerLinkAuth, PlannerLinkSocketData } from "../harness/remote/endpoint"; import type { PlannerLink } from "../harness/remote/link"; -export const READER_TOKEN = "reader-token"; +export const MEMBER_TOKEN = "member-token"; +export const RIVAL_TOKEN = "rival-token"; +export const VIEWER_TOKEN = "viewer-token"; +export const GUEST_TOKEN = "guest-token"; export const OUTSIDER_TOKEN = "outsider-token"; export const DENIED_TOKEN = "denied-token"; +const ACCOUNTS: Record = { + [MEMBER_TOKEN]: { id: "U_member", login: "member", push: true, signedIn: true }, + [RIVAL_TOKEN]: { id: "U_rival", login: "rival", push: true, signedIn: true }, + [VIEWER_TOKEN]: { id: "U_viewer", login: "viewer", push: false, signedIn: true }, + [GUEST_TOKEN]: { id: "U_guest", login: "guest", push: true, signedIn: false }, +}; + +function loginToken(login: string): string { + return `${login}-browser-login`; +} + +function scoreRepository(owner: string, name: string, push: boolean): Repository { + return { + id: "R_score", + owner, + name, + fullName: `${owner}/${name}`, + private: true, + url: `https://github.test/${owner}/${name}`, + defaultBranch: "main", + permissions: { pull: true, push, admin: false }, + }; +} + /** - * GitHub as hosted MCP sees it: the reader can pull `octo-org/score`, the - * outsider is a valid account without access, the denied account fails - * admission, and every other token is rejected by GitHub. + * GitHub as hosted MCP sees it: the member and the rival can push to + * `octo-org/score` and are signed in to Chopin in a browser, the viewer can + * only pull, the guest can push but never signed in, the outsider is a valid + * account without access, the denied account fails admission, and every other + * token is rejected by GitHub. */ export async function linkAuth(channelId?: string) { let storage = new MemoryStorage(); let now = new Date(); - await storage.users.put({ id: "U_reader", login: "reader", avatarUrl: "", now }); + for (let account of Object.values(ACCOUNTS)) { + await storage.users.put({ id: account.id, login: account.login, avatarUrl: "", now }); + } + let clock = { skewMs: 0 }; + let revocations = new Set<(sessionId: string) => Promise>(); + let sessions = new Sessions(storage, false, () => new Date(Date.now() + clock.skewMs), { + onRevoked: async id => { + for (let revoked of revocations) await revoked(id); + }, + }); + let logins = new Map(); + let cookies = new Map(); + for (let account of Object.values(ACCOUNTS)) { + if (!account.signedIn) continue; + let issued = await sessions.issue(account.id, { + accessToken: loginToken(account.login), + accessExpiresIn: 28_800, + refreshToken: "refresh-token", + refreshExpiresIn: 86_400, + }); + logins.set(account.login, issued.id); + cookies.set(account.login, issued.cookie.split(";")[0]!); + } let channel = await storage.channels.create({ id: channelId ?? crypto.randomUUID(), repositoryId: "R_score", repositoryOwner: "octo-org", repositoryName: "score", title: "Linked document", - createdBy: "U_reader", + createdBy: "U_member", now, }); let auth = { config: { origin: "https://chopin.test" }, storage, + sessions, + clock: () => new Date(), admission: { async user(token: string) { if (token === DENIED_TOKEN) throw new AdmissionDenied(); - if (token === READER_TOKEN) return { id: "U_reader", login: "reader", avatarUrl: "" }; + let account = ACCOUNTS[token]; + if (account) return { id: account.id, login: account.login, avatarUrl: "" }; if (token === OUTSIDER_TOKEN) return { id: "U_outsider", login: "outsider", avatarUrl: "" }; throw new GitHubError("Bad credentials", 401); }, + async allowed(token: string) { + return token !== DENIED_TOKEN; + }, }, github: { async repository(token: string, owner: string, name: string): Promise { - if (token !== READER_TOKEN) throw new GitHubError("Not Found", 404); - return { - id: "R_score", - owner, - name, - fullName: `${owner}/${name}`, - private: true, - url: `https://github.test/${owner}/${name}`, - defaultBranch: "main", - permissions: { pull: true, push: false, admin: false }, - }; + let account = ACCOUNTS[token]; + if (!account) throw new GitHubError("Not Found", 404); + return scoreRepository(owner, name, account.push); }, + async repositoryAccess(token: string, owner: string, name: string) { + let account = Object.values(ACCOUNTS).find(value => loginToken(value.login) === token); + return account ? scoreRepository(owner, name, account.push) : undefined; + }, + }, + }; + return { + auth: auth as unknown as PlannerLinkAuth & AttachmentAuth & { github: typeof auth.github }, + storage, + sessions, + logins, + channel, + /** Moves the browser logins' clock, so they expire without the server's clock moving. */ + clock, + /** Called with every browser login that ends, as `main.ts` wires `onSessionRevoked`. */ + revocations, + /** Signs `login` out of the browser, as the logout route does. */ + async signOut(login: string) { + let cookie = cookies.get(login)!; + return sessions.revoke(new Request("https://chopin.test/", { headers: { cookie } })); }, - } as unknown as PlannerLinkAuth; - return { auth, storage, channel }; + }; } /** A Bun server that serves only the Planner link, as the Chopin server does under HARNESS=remote. */ -export function linkServer(auth: PlannerLinkAuth, timeoutMs = 2_000, revalidateEveryMs?: number) { +export function linkServer( + auth: PlannerLinkAuth & AttachmentAuth, + timeoutMs = 2_000, + revalidateEveryMs?: number, + graceMs = 2_000, +) { let links: PlannerLink[] = []; + let attachments = new PlannerAttachments(auth, graceMs); let server = Bun.serve({ hostname: "127.0.0.1", port: 0, @@ -82,6 +159,7 @@ export function linkServer(auth: PlannerLinkAuth, timeoutMs = 2_000, revalidateE websocket: { open(ws) { openPlannerLinkSocket(ws, auth, { + attachments, timeoutMs, revalidateEveryMs, register(_channelId, link) { @@ -102,15 +180,19 @@ export function linkServer(auth: PlannerLinkAuth, timeoutMs = 2_000, revalidateE }); return { url: `http://127.0.0.1:${server.port}`, + attachments, get links() { return links; }, - stop: () => server.stop(true), + stop: () => { + attachments.close(); + return server.stop(true); + }, }; } -/** Raw frames over a socket that authenticated as the reader, for protocol violations. */ -export async function rawLink(url: string, token = READER_TOKEN) { +/** Raw frames over a socket that authenticated as the member, for protocol violations. */ +export async function rawLink(url: string, token = MEMBER_TOKEN) { let socket = linkSocket(plannerLinkUrl(url), token); let messages: any[] = []; let arrivals = new Set<() => void>(); @@ -205,7 +287,7 @@ export function attach( url: string, document: string, handlers: PlannerLinkHandlers, - token = READER_TOKEN, + token = MEMBER_TOKEN, ): Promise { return connectPlannerLink({ url, token, document, handlers }); } @@ -223,7 +305,7 @@ export async function nodeClient(url: string, document: string) { node, new URL("./planner-link-node-client.mjs", import.meta.url).pathname, url, - READER_TOKEN, + MEMBER_TOKEN, document, ], { stdin: "pipe", stdout: "pipe", stderr: "inherit" }); let reader = child.stdout.getReader(); diff --git a/apps/server/src/wire.ts b/apps/server/src/wire.ts index 314aced8a..d0edba037 100644 --- a/apps/server/src/wire.ts +++ b/apps/server/src/wire.ts @@ -11,7 +11,7 @@ */ import type { Server, ServerWebSocket } from "bun"; -import type { Frame, Outgoing } from "@chopin/protocol"; +import type { Frame, Outgoing, Session } from "@chopin/protocol"; import type { DecisionWatch } from "./socket/decision-watch"; export type Identity = { @@ -82,8 +82,13 @@ export function reply(ws: Sender, rid: string, frame: T): vo } /** Refuse one request, on the socket that made it. */ -export function fail(ws: Sender, rid: string, message: string): void { - ws.send(stamp({ kind: "session:error", ts: 0, message }, { rid })); +export function fail( + ws: Sender, + rid: string, + message: string, + code?: Session.Failure["code"], +): void { + ws.send(stamp({ kind: "session:error", ts: 0, message, ...(code ? { code } : {}) }, { rid })); } /** Tell one socket something it did not ask for. */ diff --git a/apps/web/src/chat/references.test.ts b/apps/web/src/chat/references.test.ts index ff4d39172..585fd8ee0 100644 --- a/apps/web/src/chat/references.test.ts +++ b/apps/web/src/chat/references.test.ts @@ -18,6 +18,7 @@ import { selectedReplacement, withNoticesAfter, } from "./references"; +import { WireRefusal } from "../wire"; import type { ReferenceDraft, ReferenceTarget } from "./references"; @@ -366,4 +367,10 @@ describe("acknowledged composer drafts", () => { expect(bounded).toEndWith("..."); expect(boundedChatError(new Error("connection lost"))).toContain("Check the connection"); }); + + test("shows a missing remote Planner's instructions whole, however long", () => { + let message = `No Planner is attached to this document. ${"Run pdd-plan. ".repeat(20)}`.trim(); + expect(boundedChatError(new WireRefusal(message, "planner-not-attached"))).toBe(message); + expect(boundedChatError(new WireRefusal(message))).toEndWith("..."); + }); }); diff --git a/apps/web/src/chat/references.ts b/apps/web/src/chat/references.ts index 21b641a32..ffb666b07 100644 --- a/apps/web/src/chat/references.ts +++ b/apps/web/src/chat/references.ts @@ -364,6 +364,7 @@ export function acknowledgeDraft(current: ComposerDraft, submitted: ComposerDraf export function boundedChatError(error: unknown): string { let detail = error instanceof Error ? error.message.trim() : ""; + if (detail && (error as { code?: unknown }).code === "planner-not-attached") return detail; if (!detail || /^(connection lost|disposed|not connected)$/i.test(detail)) { return "Message not sent. Check the connection and try again."; } diff --git a/apps/web/src/wire.ts b/apps/web/src/wire.ts index f7accad90..7248aa37b 100644 --- a/apps/web/src/wire.ts +++ b/apps/web/src/wire.ts @@ -10,6 +10,8 @@ * same build work on localhost, over a LAN address, and through a tunnel. */ +import type { Session } from "@chopin/protocol"; + export type Status = | "connecting" | "connected" @@ -20,6 +22,17 @@ export type Status = export type Unsubscribe = () => void; +/** A request the server refused; `code` marks a refusal to present in full. */ +export class WireRefusal extends Error { + readonly code: Session.Failure["code"]; + + constructor(message: string, code?: Session.Failure["code"]) { + super(message); + this.name = "WireRefusal"; + this.code = code; + } +} + type Frame = { kind: string; ts: number; rid?: string; sender?: string }; type Listener = (frame: never) => void; @@ -267,7 +280,8 @@ export class Wire { if (waiting) { this.#pending.delete(frame.rid); if (frame.kind === "session:error") { - waiting.reject(new Error(String((frame as { message?: string }).message))); + let failure = frame as Partial; + waiting.reject(new WireRefusal(String(failure.message), failure.code)); } else { waiting.resolve(frame as never); } diff --git a/docs/architecture.md b/docs/architecture.md index ef0cacd61..0499232b3 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -145,8 +145,13 @@ harness sessions; see [Background jobs](background-jobs.md). Under `HARNESS=remote`, `/planner-link` accepts a WebSocket whose upgrade carries a GitHub bearer, checked like `/mcp`. The client attaches to one document and -needs pull access to its repository, rechecked before every session and turn and -periodically while attached. Turns run on the client's model; Chopin's host +needs push or administration access to its repository, rechecked before every +session and turn and periodically while attached. Attaching makes the account's +live browser login the document's Planner owner, one client per document; a +document without one refuses `@chopin` and `invoke_planner` with +`planner-not-attached`. The link closes when that login ends or the document's +Planner is reset. +Turns run on the client's model; Chopin's host tools, Decisions, and room writes still run in the server under the document's Planner owner and the permission checks above. The client sees what a turn sees and steers its document writes during turns writers start; its run reports never diff --git a/docs/hosted-agent.md b/docs/hosted-agent.md index d800024c0..4e04bc00e 100644 --- a/docs/hosted-agent.md +++ b/docs/hosted-agent.md @@ -35,7 +35,8 @@ the default `copilot-sdk` harness, the Copilot entitlement. Under `HARNESS=pi` and `HARNESS=atomic` the owner supplies only the GitHub token; model access comes from the operator's `HARNESS_AUTH` mode. Under `HARNESS=remote` the owner also supplies only the GitHub token, and the model runs in the client attached -to the document. The user must pass instance +to the document; only [attaching](#remote-planner) makes an account the owner +there. The user must pass instance admission and have repository push or administration access. Ownership is assigned atomically in storage and guarded by a generation token. @@ -51,6 +52,20 @@ call follows the same rules. Its instruction is posted as the caller's own message; the turn runs under the channel's current owner, whoever that is. A channel without one is claimed for the caller's live browser login, hosted or local, and without such a login the call is refused and nothing is posted. +Under `HARNESS=remote` only attaching claims ownership. An `@chopin` message or +`invoke_planner` call for a document without an attached Planner is refused with +`planner-not-attached`, and with one it runs as a turn on that Planner under its +owner. Other Planner requests, such as sending a comment to the Planner or a +Build draft, claim nothing either: with a client attached they run on it, and +without one they fail as any request without a usable owner does, not with +`planner-not-attached`. Model-backed research is unavailable, because the remote +Planner requires `BACKGROUND_JOBS=off`; a research request claims no ownership +and is refused as not ready. + +An attachment lasts only as long as its owner. Signing that browser login out, +its expiry, or a repository writer's reset of the document's Planner closes the +attached link at once and frees the document for any eligible account to attach; +see [Planner link ownership](planner-link.md#ownership). Planner ownership stores only the owner session ID, referring to a login loaded in this process. Hosted logins separately persist encrypted credentials in @@ -286,14 +301,55 @@ checked exactly as hosted MCP checks it: the token must resolve through instance admission, a bad or missing token gets HTTP 401, a refused account 403, a foreign `Origin` 403, and GitHub outages 503, all before anything is upgraded. The first message must be `attach` with protocol version 1. Its document is resolved like -an MCP document locator, and the bearer's own account must have pull access to -the document's repository. An unsupported version, an unreadable repository, or -a missing or archived document is refused with a `refused` message and close -code 4426, 4403, or 4404. Neither attaching nor a refusal posts anything to the -document. The account and repository access are checked again before every -session and every turn the link starts, including turns of a session kept for -its runs, and every 5 minutes while the link stays attached; losing either -fails that session or turn and closes the link with `access-revoked`. +an MCP document locator, and the bearer's own account must have push or +administration access to the document's repository. An unsupported version, a +repository it cannot write, or a missing or archived document is refused with a +`refused` message and close code 4426, 4403, or 4404. Neither attaching nor a +refusal posts anything to the document. The account and repository access are +checked again before every session and every turn the link starts, including +turns of a session kept for its runs, and every 5 minutes while the link stays +attached; losing either fails that session or turn and closes the link with +`access-revoked`. + +**Ownership.** Attaching makes the account the document's Planner owner through +the same generation-guarded storage claim as any other owner +(`apps/server/src/harness/remote/attachments.ts`). The owner record names the +account's live browser login, not the link, because Chopin's repository tools +run with that login's GitHub App token and never with the link's bearer; an +account with no live sign-in since the server started is refused with +`sign-in-required` (4401). The login must pass the same eligibility as every +Planner owner: instance admission and push or administration access. This is +stricter than the design's "can read the repository", because an attached +client steers what the Planner writes and runs repository tools as the owner. +An owner left without an attachment, which only a race or an operator reset can +produce, is cleared under its generation and the document is claimed again. + +One client holds a document's Planner. While it is attached, every other +`attach` for that document is refused with `planner-attached` (4409) and a +message naming its GitHub login. A link that detaches or drops keeps the +ownership for `PLANNER_ATTACH_GRACE_MS` (2 minutes by default): the same account +can attach again within it and keeps the same ownership generation, and anyone +else is refused. When it ends, the ownership is cleared under its generation, so +a newer owner is never released by an old timer. An attach, including a grace +reattach, binds only if no sign-out, expiry, or Planner reset of its ownership +landed while it was checking, including the eligibility checks of the sign-in +it is about to claim with; otherwise it is refused like an attached link would +be closed. Attachments live in the server process; startup already clears +every owner reference, which releases every attachment. + +**Unattached documents.** Members, MCP callers, and research requests never claim +ownership under `HARNESS=remote`. An `@chopin` message or `invoke_planner` call +for a document with no attached client, including one within its grace period, +is refused with `planner-not-attached` before anything is posted or queued. +Before accepting either, the server rechecks an attached link as its periodic +recheck would: if the owner's browser sign-in has ended or storage no longer +assigns that owner, it closes the link, releases the ownership, and refuses the +request the same way. The +refusal names the document URL and says to run `pdd-plan` with it as +`plan_document` from a session in a checkout of the repository, which attaches +that session as the Planner; the browser shows that text on the `@chopin` +request. Room messages are unaffected. Once a client is attached, both arrive as +turns on its link and run under its ownership whoever sent them. **Turns and tools.** Chopin's host tools (document, repository, and job tools) still run in the server against the room, under the document's Planner owner and @@ -333,12 +389,10 @@ gains no shell or filesystem access from this harness. The client, in turn, receives everything a Planner turn sees: the conversation, the document, and the results of repository tools that run with the document Planner owner's GitHub App token, limited to the document's repository, which its own account -can already read. Attaching requires only pull access, yet the attached client -decides what the Planner writes to the document during turns writers start, so -attach only clients you would let edit the document. Until one Planner per -document is enforced, the most recently attached link serves new sessions and -earlier ones stay connected. Summary and research workers have no model in the -server, so `HARNESS=remote` requires `BACKGROUND_JOBS=off`. +can already read. The attached client decides what the Planner writes to the +document during turns writers start, which is why attaching requires the push +or administration access every Planner owner needs. Summary and research workers +have no model in the server, so `HARNESS=remote` requires `BACKGROUND_JOBS=off`. ## Permission checks diff --git a/docs/local-agent-mcp.md b/docs/local-agent-mcp.md index 1fa31cc87..2352c9e4c 100644 --- a/docs/local-agent-mcp.md +++ b/docs/local-agent-mcp.md @@ -92,6 +92,22 @@ browser `@chopin` message would: - Without either, the call is refused with `planner-owner-unavailable` and nothing is posted. +A server running `HARNESS=remote` claims no ownership here. Its Planner runs in +a client attached to the document over the [Planner link](planner-link.md), and +only attaching makes an account the owner. With a client attached the turn goes +to it and runs under its owner, whoever invoked it. Without one, including +while a detached Planner's grace period runs and once the attached Planner's +owner signs out of the browser, that sign-in expires, or its Planner is reset, +the call is refused with +`planner-not-attached` and a `message`, and nothing is posted or queued: + +```json +{ + "code": "planner-not-attached", + "message": "No Planner is attached to this document. To attach one, run pdd-plan with plan_document set to https://chopin.example/documents/octo-org/score/release-readiness from a session in a checkout of octo-org/score; that session becomes this document's Planner." +} +``` + The MCP bearer never becomes the owner or supplies its credentials. The bearer needs repository write access; an owner must also pass the GitHub App installation check. @@ -131,8 +147,13 @@ browser need not already have the document open: Chopin keeps its room alive while the invoked turn and queue run. An interrupted turn is not replayed automatically on restart. -Refusals return an object with `code`: +Refusals return an object with `code`, and `planner-not-attached` also a +`message`: +- `planner-not-attached`: under `HARNESS=remote`, no client is attached as the + document's Planner. `message` names the document URL and says to run + `pdd-plan` with it as `plan_document` from a session in a checkout of the + repository, which attaches that session as the Planner. - `planner-owner-unavailable`: the document has no usable Planner owner and the caller has no live browser login to claim it; sign in to Chopin in a browser as the MCP account, or ask the owner to sign in again. diff --git a/docs/planner-link.md b/docs/planner-link.md index 6d8b9f054..1519f9a2e 100644 --- a/docs/planner-link.md +++ b/docs/planner-link.md @@ -81,23 +81,111 @@ The server answers: ``` `attached.document.repository` is `owner/name`, at most 257 characters, and -`url` at most 2048. `attach` succeeds when the bearer's GitHub account has pull -access to the document's repository. After `refused` the server closes the link. - -The server checks the account's admission and pull access again before every -`start` and every `turn`, and every 5 minutes while the link is attached. Once -either is lost it sends `refused` with `access-revoked` and closes the link with -4403; the turn that triggered the check fails. A GitHub outage during a periodic -check leaves the link open. - -| Refusal code | Close code | Meaning | -| ---------------------- | ---------- | ------------------------------------------------- | -| `unsupported-version` | 4426 | The server speaks another `version` | -| `repository-forbidden` | 4403 | The account cannot read the document's repository | -| `document-unavailable` | 4404 | No document matches, or it is being deleted | -| `document-archived` | 4404 | The document is archived | -| `access-revoked` | 4403 | Access was lost after attaching (see below) | -| `unavailable` | 1011 | The server could not check the document | +`url` at most 2048. After `refused` the server closes the link. + +### Ownership + +Attaching makes the client the document's Planner and its account the +document's Planner owner. `attach` succeeds when all of these hold: + +- The bearer's GitHub account has push or administration access to the + document's repository. Pull access alone is refused with + `repository-forbidden`. +- The same account is signed in to Chopin in a browser, on this server, since + the server last started. Chopin's repository tools run with that sign-in's + GitHub App token, never with the link's bearer. Without one, `attach` is + refused with `sign-in-required`; sign in at the server's origin and attach + again. +- That sign-in also passes instance admission and has push or administration + access to the repository. +- No other client holds the document's Planner (see below). + +These are the checks every Planner owner passes. A document has one Planner at a +time. While a client is attached, any other `attach` for that document is +refused with `planner-attached`, including one from the same account, and the +`message` names the holder's GitHub login: + +```text +octocat is attached as this document's Planner. A document has one Planner at a time. +``` + +When the link closes, whether by `detach` or a dropped socket, its account keeps +the ownership for a grace period, 2 minutes unless the operator sets +`PLANNER_ATTACH_GRACE_MS`. During it the same account can attach again and keeps +the ownership; another account is refused with `planner-attached`, and the +`message` says until when the holder keeps it: + +```text +octocat detached from this document's Planner and holds it until 2026-10-10T19:42:00.000Z unless they reattach. A document has one Planner at a time. +``` + +When the grace period ends the ownership is released and any eligible account +can attach. A server restart releases every attachment and ownership at once; +reattach after it. + +The server checks the account's admission and push or administration access +again before every `start` and every `turn`, and every 5 minutes while the link +is attached. Once either is lost, or the document is archived or deleted, it +sends `refused` with `access-revoked` and closes the link with 4403; the turn +that triggered the check fails. A GitHub outage during a periodic check leaves +the link open. + +The ownership also ends, without a grace period, when the browser sign-in it +rests on ends or the document's Planner is reset. The server then sends +`refused` and closes an attached link at once, and any eligible account, +including the same one, can attach again immediately: + +| Cause | Refusal code | Close code | +| -------------------------------------------------------------- | ------------------ | ---------- | +| The owner's browser sign-in signs out or expires | `sign-in-required` | 4401 | +| A repository writer resets the document's Planner | `access-revoked` | 4403 | +| The stored ownership no longer names this attachment's sign-in | `access-revoked` | 4403 | + +The same checks before every `start` and `turn`, and the periodic one, also +find an expired sign-in or a released ownership that nothing reported; an +expired sign-in is reported as `sign-in-required` even when the stored +ownership already reads as released. An `@chopin` message or `invoke_planner` +call makes the same check first, so a link it finds stale is closed this way and +the request is refused as [without an attached Planner](#without-an-attached-planner). +An `attach` that is still in progress when one of these causes lands, including +a reattach within the grace period, is refused with the same code and close code +instead of attaching; a fresh attach whose sign-in ends while the server checks +it is refused with `sign-in-required` (4401). The `message` says what happened: + +```text +The Chopin browser sign-in that made octocat this document's Planner ended, so the Planner was released. Sign in to Chopin at https://chopin.example in a browser as octocat, then attach again. +A repository writer reset this document's Planner, so octocat is no longer attached. Attach again to resume. +This document's Planner ownership was released, so octocat is no longer attached. Attach again to resume. +``` + +| Refusal code | Close code | Meaning | +| ---------------------- | ---------- | ---------------------------------------------------------------------- | +| `unsupported-version` | 4426 | The server speaks another `version` | +| `repository-forbidden` | 4403 | The account cannot push to or administer the document's repository | +| `sign-in-required` | 4401 | The account has no live Chopin browser sign-in, or it ended | +| `planner-attached` | 4409 | Another client holds the document's Planner; `message` names its login | +| `document-unavailable` | 4404 | No document matches, or it is being deleted | +| `document-archived` | 4404 | The document is archived | +| `access-revoked` | 4403 | Access or the Planner ownership was lost after attaching (see above) | +| `unavailable` | 1011 | The server could not check the document | + +### Without an attached Planner + +On a server running `HARNESS=remote`, an `@chopin` message in the browser and +an MCP `invoke_planner` call for a document with no attached client are refused +with the code `planner-not-attached`, and nothing is posted or queued. A +document whose Planner is within its grace period counts as unattached, and so +does one whose attached link the request finds stale: its owner's browser +sign-in has ended or the stored ownership no longer names it. The request closes +that link before it is refused. The refusal's message names the document URL: + +```text +No Planner is attached to this document. To attach one, run pdd-plan with plan_document set to from a session in a checkout of ; that session becomes this document's Planner. +``` + +[Local agent MCP](local-agent-mcp.md#hand-an-instruction-to-the-planner) shows +how `invoke_planner` returns it. Once a client is attached, both arrive as +`turn`s on its link. ## Server to client @@ -185,7 +273,8 @@ check leaves the link open. before `turn-end` so the server keeps the session while runs are live. Reports only change run cards and Chat notices; they never start a Planner turn. - `control-result` answers `stop` or `resume`. -- `detach` closes the link with code 1000. +- `detach` closes the link with code 1000. The account keeps the document's + Planner for the grace period in [Ownership](#ownership). Messages for a session or turn that has ended are ignored, except a `host-tool-call`, which gets an error result (`The turn has ended.` or @@ -321,16 +410,19 @@ ids. When the link closes, the server shows the reported live runs as stopped. | 1008 | `error` with `malformed-message` or `protocol` | Unparseable, invalid, or out-of-order message | | 1009 | `error` with `message-too-large` | A frame over 2 MiB | | 1011 | `refused` with `unavailable` | The server could not check the document | +| 4401 | `refused` | `sign-in-required` | | 4403 | `refused` | `repository-forbidden` or `access-revoked` | | 4404 | `refused` | `document-unavailable` or `document-archived` | +| 4409 | `refused` | `planner-attached` | | 4426 | `refused` | `unsupported-version` | `protocol` covers a message before `attach` is answered, a second `attach`, and a repeated `input-request` id the server remembers. When the link closes for any reason, the server ends a running turn with an error, stops the turn's host tools still running, withdraws open questions and the Decisions those tools -opened, and forgets the link; reattaching starts new sessions. A server never -replays an interrupted turn. +opened, and forgets the link; reattaching starts new sessions. The account keeps +the document's Planner ownership for the grace period described in +[Ownership](#ownership). A server never replays an interrupted turn. A client that receives a frame it cannot parse should close with 4400 and reason `malformed-message`. Node.js's `WebSocket` lets a caller close only with 1000 or diff --git a/docs/self-hosting.md b/docs/self-hosting.md index 203981da6..62da78f2c 100644 --- a/docs/self-hosting.md +++ b/docs/self-hosting.md @@ -255,15 +255,18 @@ bind to a public interface. Startup refuses `HARNESS_AUTH` and requires model. The route exists only with `AGENT` on and `HARNESS=remote`. The trust boundary moves to whoever attaches. A client authenticates with a -GitHub token bearer, checked like hosted MCP, and needs pull access to the -document's repository, rechecked before every session and turn and every 5 -minutes while it stays attached. It then receives what a Planner turn sees, -including repository reads made with the document Planner owner's token, and -decides what the Planner writes during turns writers start; its run reports -never start a turn. The server never runs the client's -tools and gains no shell or filesystem access from this harness. Until one -Planner per document is enforced, the most recently attached link serves new -sessions. Without an attached client, a Planner turn fails as unavailable. See +GitHub token bearer, checked like hosted MCP, and needs push or administration +access to the document's repository, rechecked before every session and turn and +every 5 minutes while it stays attached. The same account must have signed in to +this server in a browser: attaching makes that login the document's Planner +owner, one client per document, and a detached or dropped link keeps it for +`PLANNER_ATTACH_GRACE_MS` so the same account can reattach. The client then +receives what a Planner turn sees, including repository reads made with that +owner's token, and decides what the Planner writes during turns writers start; +its run reports never start a turn. The server never runs the client's tools and +gains no shell or filesystem access from this harness. Without an attached +client, `@chopin` and `invoke_planner` are refused with `planner-not-attached` +and instructions to attach one. See [Remote Planner](hosted-agent.md#remote-planner) for the behaviour and [Planner link](planner-link.md) for the wire format. The link shares the application's origin and port with `/ws` and `/mcp`, so a TLS-terminating proxy @@ -339,6 +342,7 @@ the image. | `PORT` | `8787` | Source-process HTTP and WebSocket port. The supplied image and health check expect internal port 8787. | | `MODEL` | `gpt-6-luna` | Model requested for hosted agent sessions. Required under `HARNESS=pi` and `HARNESS=atomic`; under `atomic` it must be `provider/model` from Atomic's catalog. Optional under `HARNESS=remote`, where the attached client chooses its model and a set value is only passed along as the turn's request. | | `HARNESS` | `copilot-sdk` | Adapter name selected from Chopin's harness map (`copilot-sdk`, `pi`, `atomic`, or `remote`). An unknown name refuses at startup. `remote` runs Planner turns in a client attached at `/planner-link`; see [The remote Planner](#the-remote-planner). `atomic` gives every Planner session shell and filesystem access as the server process's user, hosted instances included; see [Choose and trust a harness](#choose-and-trust-a-harness). | +| `PLANNER_ATTACH_GRACE_MS` | `120000` | Under `HARNESS=remote`, how long a detached or dropped Planner link keeps the document's Planner ownership so the same account can reattach, in milliseconds, an integer between 0 and 3600000. | | `HARNESS_AUTH` | unset | Auth mode forwarded to the selected adapter. For `copilot-sdk`, `direct` and `ai-gateway` are allowed on any bind and `auto` requires a loopback-only `SERVER_HOST`; the adapter does not otherwise consume it. For `pi`, it is required: `auto`, `openai`, `anthropic`, and `custom` require a loopback-only `SERVER_HOST`, only `ai-gateway` is allowed otherwise, and `direct` is always refused. For `atomic`, it is required and must be `auto`, which requires a loopback-only `SERVER_HOST`, or `ai-gateway`; every other value is refused. For `remote`, any value is refused. | | `HARNESS_EXTENSIONS` | unset | Extension or package paths every atomic Planner session loads, as Atomic's `--extension` flag would, separated by the platform path delimiter (`:`, or `;` on Windows). A package's extensions, skills, and workflows all register. Each path must be absolute and exist; any other harness refuses the variable at startup. Background workers never load them. The code runs in the server process as its user. | | `AGENT` | on | Set exactly `off` to prevent hosted agent turns, disable the entire background-job runner, and avoid Copilot CLI startup. | diff --git a/packages/planner-link/src/client.ts b/packages/planner-link/src/client.ts index b81a0a113..de6c5b061 100644 --- a/packages/planner-link/src/client.ts +++ b/packages/planner-link/src/client.ts @@ -67,7 +67,7 @@ export type PlannerLinkHandlers = { export type PlannerLinkOptions = { /** The Chopin origin, or the link's own `ws:`/`wss:` URL. */ url: string | URL; - /** A GitHub token whose account can read the document's repository. */ + /** A GitHub token whose account can push to or administer the document's repository. */ token: string; /** The document's id or URL. */ document: string; diff --git a/packages/planner-link/src/protocol.ts b/packages/planner-link/src/protocol.ts index 07ecea1b2..329805662 100644 --- a/packages/planner-link/src/protocol.ts +++ b/packages/planner-link/src/protocol.ts @@ -18,6 +18,7 @@ export const LINK_CLOSE = { unauthorized: 4401, forbidden: 4403, unavailable: 4404, + attached: 4409, unsupportedVersion: 4426, } as const; @@ -252,6 +253,8 @@ export const refusalCodeSchema = z.enum([ "document-unavailable", "document-archived", "access-revoked", + "planner-attached", + "sign-in-required", "unavailable", ]); diff --git a/packages/protocol/index.d.ts b/packages/protocol/index.d.ts index e789f2183..f042b0843 100644 --- a/packages/protocol/index.d.ts +++ b/packages/protocol/index.d.ts @@ -110,6 +110,12 @@ export declare namespace Session { /** A request could not be served. Carries the `rid` it answers. */ export type Failure = KIND<"session:error"> & { message: string; + /** + * Set for a refusal the client presents in full rather than as a generic + * failure. `planner-not-attached`: a Planner request under `HARNESS=remote` + * to a document no client is attached to; `message` says how to attach one. + */ + code?: "planner-not-attached"; }; /** Liveness, and the smallest thing that proves request correlation works. */