diff --git a/server/src/__tests__/issue-assignee-invokability-routes.test.ts b/server/src/__tests__/issue-assignee-invokability-routes.test.ts index 8fff514a23..a52310427d 100644 --- a/server/src/__tests__/issue-assignee-invokability-routes.test.ts +++ b/server/src/__tests__/issue-assignee-invokability-routes.test.ts @@ -14,6 +14,7 @@ const agentStatusById: Record = { const mockIssueService = vi.hoisted(() => ({ getById: vi.fn(), + findOpenAncestorCreatedByAgent: vi.fn(), update: vi.fn(), create: vi.fn(), createChild: vi.fn(), @@ -148,6 +149,18 @@ function agentActor(): Actor { }; } +// Minimal chainable/thenable db stub: any query resolves to an empty row set. +// Needed because some route paths (e.g. source-trust resolution for agent +// actors) query the db directly rather than through the mocked services. +function stubDb(): any { + const query: any = {}; + for (const method of ["select", "from", "where", "innerJoin", "leftJoin", "orderBy", "limit", "groupBy", "for"]) { + query[method] = () => query; + } + query.then = (resolve: (rows: unknown[]) => unknown) => Promise.resolve(resolve([])); + return { select: () => query }; +} + function createApp(actor: Actor) { const app = express(); app.use(express.json()); @@ -155,7 +168,7 @@ function createApp(actor: Actor) { (req as any).actor = actor; next(); }); - app.use("/api", issueRoutes({} as any, {} as any)); + app.use("/api", issueRoutes(stubDb() as any, {} as any)); app.use(errorHandler); return app; } @@ -184,6 +197,8 @@ function makeIssue(overrides: Record = {}) { describe("issue assignee invokability guard", () => { beforeEach(() => { mockIssueService.getById.mockReset(); + mockIssueService.findOpenAncestorCreatedByAgent.mockReset(); + mockIssueService.findOpenAncestorCreatedByAgent.mockResolvedValue(null); mockIssueService.update.mockReset(); mockIssueService.create.mockReset(); mockIssueService.createChild.mockReset(); @@ -250,3 +265,79 @@ describe("issue assignee invokability guard", () => { expect(mockIssueService.update).toHaveBeenCalled(); }); }); + +describe("agent delegation cycle guard", () => { + beforeEach(() => { + mockIssueService.getById.mockReset(); + mockIssueService.findOpenAncestorCreatedByAgent.mockReset(); + mockIssueService.findOpenAncestorCreatedByAgent.mockResolvedValue(null); + mockIssueService.create.mockReset(); + mockIssueService.createChild.mockReset(); + }); + + it("refuses an agent child assigned to the creator of an open ancestor", async () => { + const parent = makeIssue(); + mockIssueService.getById.mockResolvedValue(parent); + mockIssueService.findOpenAncestorCreatedByAgent.mockResolvedValue({ + id: "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb", + identifier: "PAP-100", + parentId: null, + createdByAgentId: IDLE_AGENT_ID, + status: "in_progress", + }); + + const res = await request(createApp(agentActor())) + .post(`/api/issues/${parent.id}/children`) + .send({ + title: "Hand it back", + description: "Bounce", + assigneeAgentId: IDLE_AGENT_ID, + }); + + expect(res.status).toBe(409); + expect(res.body.error).toContain("Delegation cycle"); + expect(res.body.details).toMatchObject({ code: "delegation_cycle" }); + expect(mockIssueService.createChild).not.toHaveBeenCalled(); + expect(mockIssueService.findOpenAncestorCreatedByAgent).toHaveBeenCalledWith(parent.id, IDLE_AGENT_ID); + }); + + it("allows the same child when no open ancestor was created by the assignee", async () => { + const parent = makeIssue(); + mockIssueService.getById.mockResolvedValue(parent); + mockIssueService.createChild.mockResolvedValue({ + issue: makeIssue({ id: "cccccccc-cccc-4ccc-8ccc-cccccccccccc", parentId: parent.id, assigneeAgentId: IDLE_AGENT_ID }), + parentBlockerAdded: false, + }); + + const res = await request(createApp(agentActor())) + .post(`/api/issues/${parent.id}/children`) + .send({ + title: "Legit subtask", + description: "Fine", + assigneeAgentId: IDLE_AGENT_ID, + }); + + expect(res.status).toBe(201); + expect(mockIssueService.createChild).toHaveBeenCalled(); + }); + + it("does not consult the guard for board actors", async () => { + const parent = makeIssue({ assigneeAgentId: null }); + mockIssueService.getById.mockResolvedValue(parent); + mockIssueService.createChild.mockResolvedValue({ + issue: makeIssue({ id: "dddddddd-dddd-4ddd-8ddd-dddddddddddd", parentId: parent.id, assigneeAgentId: IDLE_AGENT_ID }), + parentBlockerAdded: false, + }); + + const res = await request(createApp(boardActor())) + .post(`/api/issues/${parent.id}/children`) + .send({ + title: "Human-created child", + description: "Deliberate", + assigneeAgentId: IDLE_AGENT_ID, + }); + + expect(res.status).toBe(201); + expect(mockIssueService.findOpenAncestorCreatedByAgent).not.toHaveBeenCalled(); + }); +}); diff --git a/server/src/__tests__/issue-execution-policy-routes.test.ts b/server/src/__tests__/issue-execution-policy-routes.test.ts index 7302f13264..214f231c93 100644 --- a/server/src/__tests__/issue-execution-policy-routes.test.ts +++ b/server/src/__tests__/issue-execution-policy-routes.test.ts @@ -5,6 +5,7 @@ import { normalizeIssueExecutionPolicy } from "../services/issue-execution-polic const mockIssueService = vi.hoisted(() => ({ getById: vi.fn(), + findOpenAncestorCreatedByAgent: vi.fn(async () => null), assertCheckoutOwner: vi.fn(), update: vi.fn(), createChild: vi.fn(), diff --git a/server/src/__tests__/issues-service.test.ts b/server/src/__tests__/issues-service.test.ts index e3069cdb6a..fe97cc4b75 100644 --- a/server/src/__tests__/issues-service.test.ts +++ b/server/src/__tests__/issues-service.test.ts @@ -2451,6 +2451,127 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => { }); }); +describeEmbeddedPostgres("issueService.findOpenAncestorCreatedByAgent", () => { + let db!: ReturnType; + let svc!: ReturnType; + let tempDb: Awaited> | null = null; + + beforeAll(async () => { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-issues-ancestor-"); + db = createDb(tempDb.connectionString); + svc = issueService(db); + await ensureIssueRelationsTable(db); + }, 20_000); + + afterEach(async () => { + await db.delete(issues); + await db.delete(agents); + await db.delete(companies); + }); + + afterAll(async () => { + await tempDb?.cleanup(); + }); + + async function seedChain() { + const companyId = randomUUID(); + const delegatorId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Chain Co", + issuePrefix: `C${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + await db.insert(agents).values({ + id: delegatorId, + companyId, + name: "Delegator", + role: "engineer", + status: "idle", + adapterType: "codex_local", + adapterConfig: {}, + runtimeConfig: {}, + permissions: {}, + }); + const rootId = randomUUID(); + const midId = randomUUID(); + await db.insert(issues).values([ + { + id: rootId, + companyId, + title: "Root task", + status: "in_progress", + priority: "medium", + issueNumber: 1, + identifier: "CHAIN-1", + }, + { + id: midId, + companyId, + title: "Delegated review", + status: "in_progress", + priority: "medium", + parentId: rootId, + createdByAgentId: delegatorId, + issueNumber: 2, + identifier: "CHAIN-2", + }, + ]); + return { companyId, delegatorId, rootId, midId }; + } + + it("finds an open ancestor created by the agent, at any depth", async () => { + const { delegatorId, midId } = await seedChain(); + + const direct = await svc.findOpenAncestorCreatedByAgent(midId, delegatorId); + expect(direct?.id).toBe(midId); + + const other = await svc.findOpenAncestorCreatedByAgent(midId, randomUUID()); + expect(other).toBeNull(); + }); + + it("ignores closed ancestors created by the agent", async () => { + const { delegatorId, midId } = await seedChain(); + await db.update(issues).set({ status: "done" }).where(eq(issues.id, midId)); + + expect(await svc.findOpenAncestorCreatedByAgent(midId, delegatorId)).toBeNull(); + }); + + it("finds the ancestor through an arbitrarily deep chain", async () => { + const { companyId, delegatorId, midId } = await seedChain(); + // Extend the chain 60 levels below the delegator-created ancestor so the + // match sits far above the new child's parent. + let parentId = midId; + for (let index = 0; index < 60; index += 1) { + const childId = randomUUID(); + await db.insert(issues).values({ + id: childId, + companyId, + title: `Deep child ${index}`, + status: "in_progress", + priority: "medium", + parentId, + issueNumber: index + 10, + identifier: `CHAIN-${index + 10}`, + }); + parentId = childId; + } + + const found = await svc.findOpenAncestorCreatedByAgent(parentId, delegatorId); + expect(found?.id).toBe(midId); + }); + + it("terminates on a corrupted parent-graph cycle", async () => { + const { delegatorId, rootId, midId } = await seedChain(); + // Corrupt the graph: root's parent points back at mid. + await db.update(issues).set({ parentId: midId }).where(eq(issues.id, rootId)); + + const found = await svc.findOpenAncestorCreatedByAgent(midId, delegatorId); + expect(found?.id).toBe(midId); + expect(await svc.findOpenAncestorCreatedByAgent(midId, randomUUID())).toBeNull(); + }); +}); + describeEmbeddedPostgres("issueService.create workspace inheritance", () => { let db!: ReturnType; let svc!: ReturnType; diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index e314a6d42f..64ec351386 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -4396,6 +4396,34 @@ export function issueRoutes( }; } + /** + * Refuse an agent creating a child issue assigned to the agent that created + * a still-open ancestor in the same chain. That shape is a delegation cycle + * (A delegates to B, B delegates the same work back to A): each agent lacks + * something the other assumed it had, the chain of blocked issues grows, + * and no one tells the human. Humans are unaffected, and closed ancestors + * do not count — re-engaging the creator of finished work is normal. + */ + async function assertNoAgentDelegationCycle(input: { + actorType: string; + parentIssueId: string | null | undefined; + assigneeAgentId: string | null | undefined; + }) { + if (input.actorType !== "agent") return; + if (!input.parentIssueId || !input.assigneeAgentId) return; + const ancestor = await svc.findOpenAncestorCreatedByAgent(input.parentIssueId, input.assigneeAgentId); + if (!ancestor) return; + throw conflict( + `Delegation cycle: ${ancestor.identifier ?? "an ancestor issue"} in this chain was created by the agent this child would be assigned to. ` + + "Complete the remaining work in your own issue, leave the child unassigned, or escalate to a board operator — do not delegate the work back to the agent that delegated it to you.", + { + code: "delegation_cycle", + ancestorIssueId: ancestor.id, + assigneeAgentId: input.assigneeAgentId, + }, + ); + } + async function normalizeIssueAssigneeAgentReference( companyId: string, rawAssigneeAgentId: string | null | undefined, @@ -7143,6 +7171,11 @@ export function issueRoutes( rawCreateBody.assigneeAgentId as string | null | undefined, { actorType: req.actor.type }, ); + await assertNoAgentDelegationCycle({ + actorType: req.actor.type, + parentIssueId: typeof effectiveParentId === "string" ? effectiveParentId : null, + assigneeAgentId: normalizedAssigneeAgentId ?? null, + }); const actor = getActorInfo(req); const runWorkspaceInheritanceSourceIssueId = hasExplicitIssueWorkspaceCreateSelection(rawCreateBody) ? null @@ -7359,6 +7392,11 @@ export function issueRoutes( sanitizedBody.assigneeAgentId as string | null | undefined, { actorType: req.actor.type }, ); + await assertNoAgentDelegationCycle({ + actorType: req.actor.type, + parentIssueId: parent.id, + assigneeAgentId: normalizedAssigneeAgentId ?? null, + }); const createBody = { ...sanitizedBody, ...(normalizedAssigneeAgentId !== undefined ? { assigneeAgentId: normalizedAssigneeAgentId } : {}), diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index 4acf9bec04..34b1e594b8 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -5340,6 +5340,52 @@ export function issueService(db: Db) { return activeInboxArchiveFields(archive, lastActivityAt); }, + /** + * Walk the full parent chain from `parentIssueId` (inclusive) looking for + * a still-open ancestor created by `agentId`. Used to refuse agent + * delegation cycles: an agent assigning a new child to the agent that + * created an open ancestor is handing the same work back to its own + * delegator (A→B→A hot-potato). One recursive query covers the entire + * chain regardless of depth; `UNION` (not `UNION ALL`) deduplicates + * revisited rows, so a parent graph corrupted into a loop terminates + * instead of recursing forever. + */ + findOpenAncestorCreatedByAgent: async ( + parentIssueId: string, + agentId: string, + ): Promise<{ + id: string; + identifier: string | null; + parentId: string | null; + createdByAgentId: string | null; + status: string; + } | null> => { + const rows = await db.execute(sql` + WITH RECURSIVE ancestors(id, parent_id) AS ( + SELECT id, parent_id FROM issues WHERE id = ${parentIssueId} + UNION + SELECT parent.id, parent.parent_id + FROM issues parent + JOIN ancestors ON parent.id = ancestors.parent_id + ) + SELECT i.id, i.identifier, i.parent_id, i.created_by_agent_id, i.status + FROM issues i + JOIN ancestors a ON a.id = i.id + WHERE i.created_by_agent_id = ${agentId} + AND i.status NOT IN ('done', 'cancelled') + LIMIT 1 + `); + const first = (Array.isArray(rows) ? rows[0] : null) as Record | null; + if (!first) return null; + return { + id: String(first.id), + identifier: typeof first.identifier === "string" ? first.identifier : null, + parentId: typeof first.parent_id === "string" ? first.parent_id : null, + createdByAgentId: typeof first.created_by_agent_id === "string" ? first.created_by_agent_id : null, + status: String(first.status), + }; + }, + getById: async (raw: string) => { const id = raw.trim(); const identifier = normalizeIssueReferenceIdentifier(id);