diff --git a/docs/architecture/authentication-and-tenancy.md b/docs/architecture/authentication-and-tenancy.md index 764625d..70256fe 100644 --- a/docs/architecture/authentication-and-tenancy.md +++ b/docs/architecture/authentication-and-tenancy.md @@ -74,7 +74,7 @@ not reported as completed. Its token exchange policy maps AK `agent:write` to those target scopes. Agent credentials are not used as a substitute for the user's grant. -After Enbor creates the bound identity and Agent, AK sends one GitHub permission POST +After Enbor creates the identity, AK sends one GitHub permission POST containing only the Resource URL, scopes, and persistent lifetime. Realmroot resolves the controller's connected GitHub installation Contexts internally. AK permissions use the existing native automatic authorization; AK neither @@ -88,12 +88,17 @@ and issues read/write. The current Git transport requires workflows write for all pushes. Contexts retain the GitHub App installation's repository selection. No external automatic-approval policy is introduced. -Creation returns success only after permission configuration succeeds. A 409 -`agent-permissions-incomplete` identifies the created Enbor Agent through its -Location header and detail. Retry the same creation and Idempotency-Key to -reuse Enbor resources and complete equivalent permissions. No local Agent or -permission table is added. Existing Agent reads and assignments do not backfill -grants; this change applies only to new creations. +Only after GitHub permissions succeed does AK create the bound Enbor Agent and +return success. Permission failure first deletes the Enbor identity, then the +Realmroot identity (which revokes its grants), and returns 409 +`agent-permissions-failed`. Connect GitHub or correct its scopes and start a new +creation with a new username and Idempotency-Key after successful cleanup. +Realmroot retains deleted historical identities and reserves their usernames. A cleanup failure +returns 502 `agent-creation-cleanup-failed` with both identity IDs and the cause. +A definite Agent creation rejection uses the same cleanup; an uncertain Agent +creation outcome preserves the identity for reconciliation/retry. Enbor identity +deletion refuses identities currently selected by an Agent, protecting an already +created Agent during a concurrent replay. Acceptance: create a new Agent through Toolbox in Demo, inspect its permissions before assignment, then run a repository Task including Issue, PR, CI log, and diff --git a/server/adapters/realmroot/agentPermissions.ts b/server/adapters/realmroot/agentPermissions.ts index ba41586..e9b5849 100644 --- a/server/adapters/realmroot/agentPermissions.ts +++ b/server/adapters/realmroot/agentPermissions.ts @@ -2,6 +2,14 @@ import type { AgentPermissionGateway } from "@server/usecases/agents/defaultPerm export function createAgentPermissionGateway(origin: string, token: string): AgentPermissionGateway { return { + async deleteIdentity(agentId) { + const response = await fetch(new URL(`/api/agents/${encodeURIComponent(agentId)}`, origin), { + method: "DELETE", + headers: { authorization: `Bearer ${token}` }, + signal: AbortSignal.timeout(60_000), + }); + if (!response.ok && response.status !== 404) throw new Error(`Realmroot identity cleanup failed: HTTP ${response.status}`); + }, async grant(agentId, input) { const response = await fetch(new URL(`/api/agents/${encodeURIComponent(agentId)}/permissions`, origin), { method: "POST", diff --git a/server/http/agents/routes.ts b/server/http/agents/routes.ts index 317e42c..aece6fb 100644 --- a/server/http/agents/routes.ts +++ b/server/http/agents/routes.ts @@ -14,7 +14,7 @@ import { externalCreationIdempotencyKey, readJsonBody, } from "@server/http/resource-server/request"; -import { AgentPermissionProvisioningError, grantDefaultAgentPermissions } from "@server/usecases/agents/defaultPermissions"; +import { grantDefaultAgentPermissions } from "@server/usecases/agents/defaultPermissions"; import { createAgencyAgent } from "@server/usecases/agents/projectAgents"; import { AGENCY_RUNTIMES } from "@shared"; import type { Hono } from "hono"; @@ -46,25 +46,22 @@ export function registerAgentRoutes(api: Hono<{ Bindings: Env }>): void { scopes: ["agents:write"], }); const { client } = await agencyDependencies(c, ["identities:write", "agents:write"]); - const agent = await createAgencyAgent(client, { - name, - username, - runtime, - systemPrompt, - description, - provider, - model, - skills, - idempotencyKey, - }); - try { - if (!agent.spec.identity) throw new Error("Enbor did not return a bound Realmroot identity"); - await grantDefaultAgentPermissions(createAgentPermissionGateway(platformResource, permissionToken), agent.spec.identity.agentId, { - githubResource: c.env.GITHUB_RESOURCE, - }); - } catch (error) { - throw new AgentPermissionProvisioningError(agent.metadata.uid, error); - } + const agent = await createAgencyAgent( + client, + { + name, + username, + runtime, + systemPrompt, + description, + provider, + model, + skills, + idempotencyKey, + }, + createAgentPermissionGateway(platformResource, permissionToken), + c.env.GITHUB_RESOURCE, + ); const represented = agentRepresentation(agent, c.req.url); const etag = await representationEtag(represented); await completeExternalCreation(c, "agents", agent.metadata.uid, etag.slice(1, -1), represented); diff --git a/server/http/middleware/errorHandler.ts b/server/http/middleware/errorHandler.ts index 6ecd6ed..c76e803 100644 --- a/server/http/middleware/errorHandler.ts +++ b/server/http/middleware/errorHandler.ts @@ -3,7 +3,7 @@ import { applyRequestIdHeader } from "@server/http/middleware/requestContext"; import { isPublishedV2Operation, v2Problem } from "@server/http/middleware/v2Contract"; import { AgencyProjectInitializationBusy } from "@server/usecases/agency/ensureAgencyProject"; import { RealmrootDelegationFailure } from "@server/usecases/agency/failures"; -import { AgentPermissionProvisioningError } from "@server/usecases/agents/defaultPermissions"; +import { AgentCreationCleanupError, AgentPermissionProvisioningError } from "@server/usecases/agents/defaultPermissions"; import { ApplicationError } from "@server/usecases/applicationError"; import type { ErrorHandler } from "hono"; import { HTTPException } from "hono/http-exception"; @@ -12,9 +12,11 @@ export const apiErrorHandler: ErrorHandler = (error, c) => { applyRequestIdHeader(c); c.set("requestError", error); const published = isPublishedV2Operation(c.req.method, c.req.path); + if (error instanceof AgentCreationCleanupError) { + return v2Problem(c, 502, "agent-creation-cleanup-failed", "Agent creation cleanup failed", error.message); + } if (error instanceof AgentPermissionProvisioningError) { - c.header("Location", new URL(`/api/agents/${encodeURIComponent(error.agentId)}`, c.req.url).toString()); - return v2Problem(c, 409, "agent-permissions-incomplete", "Agent permissions incomplete", error.message); + return v2Problem(c, 409, "agent-permissions-failed", "Agent permissions failed", error.message); } if (error instanceof RealmrootDelegationFailure) { if (error.kind === "user-login-required") { diff --git a/server/http/resource-server/routes.ts b/server/http/resource-server/routes.ts index 23e4109..e86b284 100644 --- a/server/http/resource-server/routes.ts +++ b/server/http/resource-server/routes.ts @@ -189,7 +189,9 @@ function baseDocument(env: Env) { responses: { "200": response("Agent collection", { $ref: "#/components/schemas/AgentCollection" }), ...projectionReadProblems }, }, post: { - ...operation("createAgent", "agent:write", "Agent created with persistent AK and GitHub development permissions", "201"), + ...operation("createAgent", "agent:write", "Agent created after persistent GitHub permissions are granted", "201"), + description: + "Create the identity, grant default GitHub permissions, then create the bound Agent. Permission failure cleans up both identities and returns 409 agent-permissions-failed. After successful cleanup, use a new username and Idempotency-Key; deleted usernames remain reserved. Cleanup failure returns 502 agent-creation-cleanup-failed. An uncertain Agent creation outcome preserves its identity for retry with the original key. AK scopes use native automatic authorization.", parameters: [version, idempotencyKey], requestBody: { required: true, ...json({ $ref: "#/components/schemas/AgentWrite" }) }, responses: { "201": createdResponse("Agent created", { $ref: "#/components/schemas/Agent" }), ...projectionCreateProblems }, diff --git a/server/usecases/agents/defaultPermissions.ts b/server/usecases/agents/defaultPermissions.ts index 6fc8b78..0f96a1e 100644 --- a/server/usecases/agents/defaultPermissions.ts +++ b/server/usecases/agents/defaultPermissions.ts @@ -14,16 +14,28 @@ export const DEFAULT_GITHUB_SCOPES = [ ] as const; export interface AgentPermissionGateway { + deleteIdentity(agentId: string): Promise; grant(agentId: string, input: { resource: string; scopes: readonly string[]; mode: "persistent" }): Promise; } export class AgentPermissionProvisioningError extends Error { + constructor(cause: unknown) { + super( + `GitHub permission configuration failed: ${cause instanceof Error ? cause.message : String(cause)}. Identity cleanup completed; connect GitHub or correct its permissions, then create again with a new username and Idempotency-Key (deleted identity usernames remain reserved).`, + { cause }, + ); + } +} + +export class AgentCreationCleanupError extends Error { constructor( - readonly agentId: string, + readonly identityId: string, + readonly realmrootAgentId: string, cause: unknown, + readonly cleanupCause: unknown, ) { super( - `Agent ${agentId} was created, but permission configuration failed: ${cause instanceof Error ? cause.message : String(cause)}. Retry creation with the same Idempotency-Key to finish configuring this Agent.`, + `Agent creation failed: ${cause instanceof Error ? cause.message : String(cause)}. Cleanup failed for Enbor identity ${identityId} / Realmroot identity ${realmrootAgentId}: ${cleanupCause instanceof Error ? cleanupCause.message : String(cleanupCause)}. Identity cleanup requires attention.`, { cause }, ); } diff --git a/server/usecases/agents/projectAgents.ts b/server/usecases/agents/projectAgents.ts index 9d16644..f80a574 100644 --- a/server/usecases/agents/projectAgents.ts +++ b/server/usecases/agents/projectAgents.ts @@ -1,5 +1,12 @@ import { type Agent, EnborApiError, type EnborClient, type RuntimeName } from "@realmroot/enbor-sdk"; +import { + AgentCreationCleanupError, + type AgentPermissionGateway, + AgentPermissionProvisioningError, + grantDefaultAgentPermissions, +} from "./defaultPermissions"; + const AGENT_KANBAN_SKILL = "saltbo/agent-kanban@agent-kanban"; export interface CreateAgencyAgentInput { @@ -14,7 +21,12 @@ export interface CreateAgencyAgentInput { idempotencyKey: string; } -export async function createAgencyAgent(client: EnborClient, input: CreateAgencyAgentInput): Promise { +export async function createAgencyAgent( + client: EnborClient, + input: CreateAgencyAgentInput, + permissions: AgentPermissionGateway, + githubResource: string, +): Promise { const identity = await client.identities.create( { metadata: { name: input.name }, @@ -22,6 +34,23 @@ export async function createAgencyAgent(client: EnborClient, input: CreateAgency }, await derivedKey(input.idempotencyKey, "identity"), ); + if (!identity.status.descriptor) + throw new EnborApiError(502, `Enbor identity ${identity.metadata.uid} provisioning did not return its Realmroot descriptor`, null); + const realmrootAgentId = identity.status.descriptor.agentId; + const cleanup = async (cause: unknown) => { + try { + await client.identities.delete(identity.metadata.uid); + await permissions.deleteIdentity(realmrootAgentId); + } catch (cleanupCause) { + throw new AgentCreationCleanupError(identity.metadata.uid, realmrootAgentId, cause, cleanupCause); + } + }; + try { + await grantDefaultAgentPermissions(permissions, realmrootAgentId, { githubResource }); + } catch (error) { + await cleanup(error); + throw new AgentPermissionProvisioningError(error); + } let agent: Agent; try { agent = await client.agents.create( @@ -46,7 +75,7 @@ export async function createAgencyAgent(client: EnborClient, input: CreateAgency error.status !== 408 && error.status !== 429 ) { - await client.identities.delete(identity.metadata.uid); + await cleanup(error); } throw error; } diff --git a/spec/agents.feature b/spec/agents.feature index fa265e4..c212f33 100644 --- a/spec/agents.feature +++ b/spec/agents.feature @@ -28,12 +28,14 @@ Feature: Agent projections Scenario: Create an Agent with its Realmroot identity Given an authorized caller supplies complete Agent configuration When the caller creates an Agent through AK - Then AK creates a same-tenant Realmroot Identity and bound Enbor Agent with the Agent Kanban work skill + Then AK creates a same-tenant Realmroot Identity, grants GitHub permissions, then creates the bound Enbor Agent with the Agent Kanban work skill + And a permission failure deletes the Enbor and Realmroot identities before returning an error + And cleanup failure explicitly identifies the remaining identity resources And AK does not create an Inbox Trigger because Task assignment directly creates a Session And replays the compound operation without duplicate resources when its Idempotency-Key is retried And stores no local Agent entity And grants the new identity its default GitHub permissions before returning success - And a permission failure identifies the created Agent and a retry resumes without duplicates + And after successful rollback the caller starts a new creation with a new username and Idempotency-Key @journey:agents/assignment-subject @entrypoint:toolbox @proof:unit Scenario: Assign a Task by projected Agent subject diff --git a/tests/integration/http/agent-machine-projections.test.ts b/tests/integration/http/agent-machine-projections.test.ts index fa589b5..89b395c 100644 --- a/tests/integration/http/agent-machine-projections.test.ts +++ b/tests/integration/http/agent-machine-projections.test.ts @@ -113,7 +113,11 @@ async function browserSessionFor(subject: string) { return auth; } -function delegatedAgencyFetch(scopes: string[], upstream: (request: Request) => Response | Promise) { +function delegatedAgencyFetch( + scopes: string[], + upstream: (request: Request) => Response | Promise, + permissionRequest?: (request: Request) => Response | Promise, +) { return vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { const request = input instanceof Request ? input : new Request(String(input), init); if (request.url === "https://id.realmroot.dev/api/auth/.well-known/openid-configuration") { @@ -133,6 +137,7 @@ function delegatedAgencyFetch(scopes: string[], upstream: (request: Request) => } if (request.url.startsWith("https://id.realmroot.dev/api/agents/")) { expect(request.headers.get("authorization")).toBe("Bearer platform-user-token"); + if (permissionRequest) return permissionRequest(request); expect(request.method).toBe("POST"); expect(new URL(request.url).pathname.endsWith("/permissions")).toBe(true); const body = (await request.json()) as { resource: string; scopes: string[]; mode: string }; @@ -659,6 +664,56 @@ describe("Agent and Machine projection HTTP resources", () => { expect(created.machine).not.toHaveProperty("runners"); }); + it.each([false, true])( + "[spec: agents/create-bound-agent] reports permission failure and identity cleanup outcome (cleanup fails: %s)", + async (cleanupFails) => { + const operations: string[] = []; + vi.stubGlobal( + "fetch", + delegatedAgencyFetch( + ["identities:write", "agents:write"], + async (request) => { + const path = new URL(request.url).pathname; + operations.push(`${request.method} ${path}`); + if (request.method === "POST" && path === "/api/v1/identities") + return Response.json({ + metadata: metadata("identity-failed", "Failed Agent"), + status: { descriptor: { agentId: "realmroot-failed" } }, + }); + if (request.method === "DELETE" && path === "/api/v1/identities/identity-failed") return new Response(null, { status: 204 }); + throw new Error(`Unexpected request ${request.method} ${path}`); + }, + async (request) => { + const path = new URL(request.url).pathname; + operations.push(`${request.method} ${path}`); + if (request.method === "DELETE") return new Response(null, { status: cleanupFails ? 503 : 204 }); + return Response.json( + { error: { message: "The controller must connect the external resource account before granting Agent permissions." } }, + { status: 400 }, + ); + }, + ), + ); + const response = await browserPost( + "/agents", + { name: "Failed Agent", username: "failed-agent", runtime: "codex", systemPrompt: "Handle assigned work" }, + "failed-permission-create", + ); + expect(response.status).toBe(cleanupFails ? 502 : 409); + expect(response.headers.get("Location")).toBeNull(); + await expect(response.json()).resolves.toMatchObject({ + type: `${resource}/problems/${cleanupFails ? "agent-creation-cleanup-failed" : "agent-permissions-failed"}`, + detail: expect.stringContaining(cleanupFails ? "realmroot-failed" : "Identity cleanup completed"), + }); + expect(operations).toEqual([ + "POST /api/v1/identities", + "POST /api/agents/realmroot-failed/permissions", + "DELETE /api/v1/identities/identity-failed", + "DELETE /api/agents/realmroot-failed", + ]); + }, + ); + it("[spec: agents/authoritative-projection] [spec: agents/create-bound-agent] replays the winning Agent response when identical external creations complete concurrently", async () => { const synchronizeAgentCreations = twoRequestBarrier(); let identityCreates = 0; @@ -673,7 +728,10 @@ describe("Agent and Machine projection HTTP resources", () => { if (path === "/api/v1/identities") { identityCreates += 1; identityUpstreamKeys.push(request.headers.get("Idempotency-Key")!); - return Response.json({ metadata: metadata("identity-concurrent", "Concurrent Agent") }); + return Response.json({ + metadata: metadata("identity-concurrent", "Concurrent Agent"), + status: { descriptor: { agentId: "realmroot-concurrent" } }, + }); } if (path === "/api/v1/agents") { agentCreates += 1; diff --git a/tests/unit/application/agent-default-permissions.test.ts b/tests/unit/application/agent-default-permissions.test.ts index 74655b9..6f8a6f9 100644 --- a/tests/unit/application/agent-default-permissions.test.ts +++ b/tests/unit/application/agent-default-permissions.test.ts @@ -5,7 +5,7 @@ import { DEFAULT_GITHUB_SCOPES, grantDefaultAgentPermissions } from "../../../se describe("new Agent permissions", () => { it("[spec: agents/default-permissions] grants only GitHub scopes and leaves AK to native automatic authorization", async () => { const grant = vi.fn().mockResolvedValue(undefined); - await grantDefaultAgentPermissions({ grant }, "new-identity", { githubResource: "https://github.test/api" }); + await grantDefaultAgentPermissions({ grant, deleteIdentity: vi.fn() }, "new-identity", { githubResource: "https://github.test/api" }); expect(grant.mock.calls).toEqual([["new-identity", { resource: "https://github.test/api", scopes: DEFAULT_GITHUB_SCOPES, mode: "persistent" }]]); }); it("[spec: agents/default-permissions] validates all returned scopes and propagates authority failures", async () => { diff --git a/tests/unit/application/agent-sdk-orchestration.test.ts b/tests/unit/application/agent-sdk-orchestration.test.ts index c03f575..01a8dc5 100644 --- a/tests/unit/application/agent-sdk-orchestration.test.ts +++ b/tests/unit/application/agent-sdk-orchestration.test.ts @@ -1,8 +1,8 @@ import { type Agent, EnborApiError, type EnborClient, type Identity } from "@realmroot/enbor-sdk"; -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { createAgencyAgent } from "../../../server/usecases/agents/projectAgents"; -const identity = { metadata: { uid: "identity-1" } } as Identity; +const identity = { metadata: { uid: "identity-1" }, status: { descriptor: { agentId: "realmroot-1" } } } as Identity; const agent = { metadata: { uid: "agent-1" } } as Agent; const trigger = { status: { subscription: { phase: "active" } } }; @@ -19,6 +19,8 @@ function harness() { return { client, createIdentity, deleteIdentity, createAgent, createTrigger }; } +const permissions = { grant: vi.fn().mockResolvedValue(undefined), deleteIdentity: vi.fn().mockResolvedValue(undefined) }; + const input = { name: "Backend", description: "Builds APIs", @@ -32,10 +34,52 @@ const input = { }; describe("Agent SDK orchestration", () => { + beforeEach(() => { + vi.clearAllMocks(); + permissions.grant.mockResolvedValue(undefined); + permissions.deleteIdentity.mockResolvedValue(undefined); + }); + it("[spec: agents/create-bound-agent] rolls back both identities when GitHub is not connected, without creating an Agent", async () => { + const { client, createIdentity, createAgent, deleteIdentity } = harness(); + permissions.grant.mockRejectedValueOnce(new Error("Connect GitHub first")); + await expect(createAgencyAgent(client, input, permissions, "https://github.test/api")).rejects.toThrow("Connect GitHub first"); + expect(createAgent).not.toHaveBeenCalled(); + expect(deleteIdentity).toHaveBeenCalledWith("identity-1"); + expect(permissions.deleteIdentity).toHaveBeenCalledWith("realmroot-1"); + expect(createIdentity.mock.invocationCallOrder[0]).toBeLessThan(permissions.grant.mock.invocationCallOrder[0]!); + expect(deleteIdentity.mock.invocationCallOrder[0]).toBeLessThan(permissions.deleteIdentity.mock.invocationCallOrder[0]!); + }); + it("[spec: agents/create-bound-agent] surfaces both the permission failure and cleanup failure without deleting a possibly bound Realmroot identity", async () => { + const { client, createAgent, deleteIdentity } = harness(); + permissions.grant.mockRejectedValueOnce(new Error("Connect GitHub first")); + deleteIdentity.mockRejectedValueOnce(new Error("Identity is in use")); + await expect(createAgencyAgent(client, input, permissions, "https://github.test/api")).rejects.toMatchObject({ + identityId: "identity-1", + realmrootAgentId: "realmroot-1", + message: expect.stringContaining("Identity is in use"), + cause: expect.objectContaining({ message: "Connect GitHub first" }), + }); + expect(createAgent).not.toHaveBeenCalled(); + expect(permissions.deleteIdentity).not.toHaveBeenCalled(); + }); + + it("[spec: agents/create-bound-agent] reports Realmroot cleanup failure after local identity deletion", async () => { + const { client, createAgent, deleteIdentity } = harness(); + permissions.grant.mockRejectedValueOnce(new Error("Missing GitHub scopes")); + permissions.deleteIdentity.mockRejectedValueOnce(new Error("Realmroot unavailable")); + await expect(createAgencyAgent(client, input, permissions, "https://github.test/api")).rejects.toMatchObject({ + identityId: "identity-1", + realmrootAgentId: "realmroot-1", + cleanupCause: expect.objectContaining({ message: "Realmroot unavailable" }), + }); + expect(deleteIdentity).toHaveBeenCalledWith("identity-1"); + expect(createAgent).not.toHaveBeenCalled(); + }); + it("[spec: agents/create-bound-agent] creates the SDK Identity before the bound SDK Agent", async () => { const { client, createIdentity, deleteIdentity, createAgent, createTrigger } = harness(); - await expect(createAgencyAgent(client, input)).resolves.toBe(agent); + await expect(createAgencyAgent(client, input, permissions, "https://github.test/api")).resolves.toBe(agent); expect(createIdentity).toHaveBeenCalledWith( { metadata: { name: "Backend" }, spec: { username: "backend", runtime: "codex" } }, expect.stringMatching(/^ak-[a-f0-9]{64}$/), @@ -53,6 +97,7 @@ describe("Agent SDK orchestration", () => { }, expect.stringMatching(/^ak-[a-f0-9]{64}$/), ); + expect(permissions.grant.mock.invocationCallOrder[0]).toBeLessThan(createAgent.mock.invocationCallOrder[0]!); expect(createTrigger).not.toHaveBeenCalled(); expect(createIdentity.mock.calls[0]![1]).not.toBe(createAgent.mock.calls[0]![1]); expect(deleteIdentity).not.toHaveBeenCalled(); @@ -63,8 +108,9 @@ describe("Agent SDK orchestration", () => { const rejection = new EnborApiError(422, "invalid Agent", { type: "validation" }); createAgent.mockRejectedValue(rejection); - await expect(createAgencyAgent(client, input)).rejects.toBe(rejection); + await expect(createAgencyAgent(client, input, permissions, "https://github.test/api")).rejects.toBe(rejection); expect(deleteIdentity).toHaveBeenCalledWith("identity-1"); + expect(permissions.deleteIdentity).toHaveBeenCalledWith("realmroot-1"); }); it.each([ @@ -78,7 +124,7 @@ describe("Agent SDK orchestration", () => { const { client, createAgent, deleteIdentity } = harness(); createAgent.mockRejectedValue(rejection); - await expect(createAgencyAgent(client, input)).rejects.toBe(rejection); + await expect(createAgencyAgent(client, input, permissions, "https://github.test/api")).rejects.toBe(rejection); expect(deleteIdentity).not.toHaveBeenCalled(); }); });