From ada47be7644b92af2d054f25cbeacfdcae81b785 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sat, 1 Aug 2026 15:05:16 -0700 Subject: [PATCH] fix(server): refuse agent-initiated issue assignment to paused agents (#10648) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents can create and assign issues to other agents, and commonly escalate to their org-chart manager (`reports_to`) when they hit something outside their authority > - Issue assignment already refuses terminated and pending-approval assignees, but accepts paused assignees from any actor > - A paused agent never runs, so agent-initiated escalations to a paused manager become invisible dead letters — accepted silently, never picked up, never surfaced > - This pull request refuses paused assignees when the assigning actor is an agent, at the single normalization helper all four assignment paths flow through > - The benefit is that agent-routed work can no longer silently vanish into a paused agent's queue ## Linked Issues or Issue Description Fixes #10641 ## What Changed - `normalizeIssueAssigneeAgentReference` (used by issue create, both child-create routes, and issue update) now throws a 409 when an **agent** actor assigns to a **paused** agent, with a message naming the alternatives: assign an invokable agent, leave the issue unassigned, or escalate to a board operator. - Board/user actors are unchanged and may still assign to paused agents deliberately — the pause state is visible in the UI, and staging work for a later unpause is a legitimate workflow. Terminated / pending-approval / invalid-org-chain refusals are unchanged for all actors. - This matches the existing precedent for watchdogs ("Cannot assign watchdog to an agent that is not invokable") using the same conflict-error shape. ## Verification - `pnpm vitest run server/src/__tests__/issue-assignee-invokability-routes.test.ts` — new coverage: agent PATCH → paused assignee 409 (no update call), agent child-create → paused assignee 409 (no create call), agent assignment to an invokable agent still 200, board assignment to a paused agent still 200. - Neighboring suites unchanged: `issue-update-comment-wakeup-routes`, `issue-agent-mutation-ownership-routes`, `issue-create-deduplication-routes`, `issue-watchdogs-routes` (97 tests). - `cd server && pnpm run typecheck`. ## Risks - Low. The only behavior change is a new 409 for agent actors assigning to paused agents — previously a silent dead-letter. Agents that relied on this (escalation flows) now get an actionable error instead; human workflows are untouched. ## Model Used Claude Fable 5 (`claude-fable-5`, Anthropic) via Claude Code — extended thinking, agentic tool use. No other models involved. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- ...issue-assignee-invokability-routes.test.ts | 252 ++++++++++++++++++ server/src/routes/issues.ts | 17 ++ 2 files changed, 269 insertions(+) create mode 100644 server/src/__tests__/issue-assignee-invokability-routes.test.ts diff --git a/server/src/__tests__/issue-assignee-invokability-routes.test.ts b/server/src/__tests__/issue-assignee-invokability-routes.test.ts new file mode 100644 index 0000000000..8fff514a23 --- /dev/null +++ b/server/src/__tests__/issue-assignee-invokability-routes.test.ts @@ -0,0 +1,252 @@ +import express from "express"; +import request from "supertest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const AGENT_ACTOR_ID = "11111111-1111-4111-8111-111111111111"; +const PAUSED_AGENT_ID = "22222222-2222-4222-8222-222222222222"; +const IDLE_AGENT_ID = "33333333-3333-4333-8333-333333333333"; + +const agentStatusById: Record = { + [AGENT_ACTOR_ID]: "idle", + [PAUSED_AGENT_ID]: "paused", + [IDLE_AGENT_ID]: "idle", +}; + +const mockIssueService = vi.hoisted(() => ({ + getById: vi.fn(), + update: vi.fn(), + create: vi.fn(), + createChild: vi.fn(), + addComment: vi.fn(), + findMentionedAgents: vi.fn(async () => []), + getRelationSummaries: vi.fn(async () => ({ blockedBy: [], blocks: [] })), + listWakeableBlockedDependents: vi.fn(async () => []), + getWakeableParentAfterChildCompletion: vi.fn(async () => null), + getCurrentScheduledRetry: vi.fn(async () => null), + getDependencyReadiness: vi.fn(async () => ({ + blockerIssueIds: [], + isDependencyReady: false, + unresolvedBlockerCount: 0, + })), +})); + +const mockHeartbeatService = vi.hoisted(() => ({ + wakeup: vi.fn(async () => undefined), + reportRunActivity: vi.fn(async () => undefined), + getRun: vi.fn(async () => null), + getActiveRunForAgent: vi.fn(async () => null), + cancelRun: vi.fn(async () => null), +})); + +vi.mock("../services/index.js", () => ({ + companyService: () => ({ + getById: vi.fn(async () => ({ id: "company-1", attachmentMaxBytes: 10 * 1024 * 1024 })), + }), + accessService: () => ({ + canUser: vi.fn(async () => true), + decide: vi.fn(async (input: { action?: string }) => ({ + allowed: true, + action: input.action, + reason: "allow_explicit_grant", + explanation: "Allowed by test grant.", + })), + hasPermission: vi.fn(async () => true), + }), + agentService: () => ({ + getById: vi.fn(async (id: string) => ({ + id, + companyId: "company-1", + status: agentStatusById[id] ?? "idle", + })), + resolveByReference: vi.fn(async (_companyId: string, raw: string) => ({ + ambiguous: false, + agent: { + id: raw, + companyId: "company-1", + status: agentStatusById[raw] ?? "idle", + orgChainHealth: { status: "healthy" }, + }, + })), + }), + companySkillService: () => ({ + completeTestRunForIssue: vi.fn(async () => null), + }), + documentAnnotationService: () => ({ remapOpenThreadsForDocument: async () => [] }), + documentService: () => ({}), + executionWorkspaceService: () => ({}), + feedbackService: () => ({ + listIssueVotesForUser: vi.fn(async () => []), + saveIssueVote: vi.fn(async () => ({ vote: null, consentEnabledNow: false, sharingEnabled: false })), + }), + goalService: () => ({}), + heartbeatService: () => mockHeartbeatService, + instanceSettingsService: () => ({ + get: vi.fn(async () => ({ + id: "instance-settings-1", + general: { + censorUsernameInLogs: false, + feedbackDataSharingPreference: "prompt", + }, + })), + listCompanyIds: vi.fn(async () => ["company-1"]), + }), + issueApprovalService: () => ({}), + issueReferenceService: () => ({ + deleteDocumentSource: async () => undefined, + diffIssueReferenceSummary: () => ({ + addedReferencedIssues: [], + removedReferencedIssues: [], + currentReferencedIssues: [], + }), + emptySummary: () => ({ outbound: [], inbound: [] }), + listIssueReferenceSummary: async () => ({ outbound: [], inbound: [] }), + syncComment: async () => undefined, + syncDocument: async () => undefined, + syncIssue: async () => undefined, + }), + issueRecoveryActionService: () => ({ + getActiveForIssue: vi.fn(async () => null), + listActiveForIssues: vi.fn(async () => new Map()), + }), + issueService: () => mockIssueService, + issueThreadInteractionService: () => ({ + expireRequestConfirmationsSupersededByComment: vi.fn(async () => []), + expireStaleRequestConfirmationsForIssueDocument: vi.fn(async () => []), + expireRequestConfirmationsSupersededByHistoricalComments: vi.fn(async () => []), + }), + logActivity: vi.fn(async () => undefined), + projectService: () => ({}), + routineService: () => ({ + syncRunStatusForIssue: vi.fn(async () => undefined), + }), + workProductService: () => ({}), +})); + +import { errorHandler } from "../middleware/index.js"; +import { issueRoutes } from "../routes/issues.js"; + +type Actor = Record; + +function boardActor(): Actor { + return { + type: "board", + userId: "local-board", + companyIds: ["company-1"], + source: "local_implicit", + isInstanceAdmin: false, + }; +} + +// No runId on purpose: a run-less agent actor exercises the assignment guard +// without engaging watchdog-scope or checkout-ownership lookups. +function agentActor(): Actor { + return { + type: "agent", + agentId: AGENT_ACTOR_ID, + companyId: "company-1", + source: "agent_key", + }; +} + +function createApp(actor: Actor) { + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + (req as any).actor = actor; + next(); + }); + app.use("/api", issueRoutes({} as any, {} as any)); + app.use(errorHandler); + return app; +} + +function makeIssue(overrides: Record = {}) { + return { + id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + companyId: "company-1", + status: "todo", + priority: "medium", + projectId: null, + goalId: null, + parentId: null, + assigneeAgentId: AGENT_ACTOR_ID, + assigneeUserId: null, + createdByUserId: "local-board", + identifier: "PAP-999", + title: "Invokability test", + executionPolicy: null, + executionState: null, + hiddenAt: null, + ...overrides, + }; +} + +describe("issue assignee invokability guard", () => { + beforeEach(() => { + mockIssueService.getById.mockReset(); + mockIssueService.update.mockReset(); + mockIssueService.create.mockReset(); + mockIssueService.createChild.mockReset(); + mockIssueService.addComment.mockReset(); + mockHeartbeatService.wakeup.mockClear(); + }); + + it("refuses an agent assigning an issue to a paused agent", async () => { + const existing = makeIssue(); + mockIssueService.getById.mockResolvedValue(existing); + + const res = await request(createApp(agentActor())) + .patch(`/api/issues/${existing.id}`) + .send({ assigneeAgentId: PAUSED_AGENT_ID }); + + expect(res.status).toBe(409); + expect(res.body.error).toContain("paused agent"); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); + + it("refuses an agent creating a child issue assigned to a paused agent", async () => { + const parent = makeIssue(); + mockIssueService.getById.mockResolvedValue(parent); + + const res = await request(createApp(agentActor())) + .post(`/api/issues/${parent.id}/children`) + .send({ + title: "Escalation for the manager", + description: "Needs a decision", + assigneeAgentId: PAUSED_AGENT_ID, + }); + + expect(res.status).toBe(409); + expect(res.body.error).toContain("paused agent"); + expect(mockIssueService.createChild).not.toHaveBeenCalled(); + expect(mockIssueService.create).not.toHaveBeenCalled(); + }); + + it("still allows an agent to assign to an invokable agent", async () => { + const existing = makeIssue(); + const updated = makeIssue({ assigneeAgentId: IDLE_AGENT_ID }); + mockIssueService.getById.mockResolvedValue(existing); + mockIssueService.update.mockResolvedValue(updated); + + const res = await request(createApp(agentActor())) + .patch(`/api/issues/${existing.id}`) + .send({ assigneeAgentId: IDLE_AGENT_ID }); + + expect(res.status).toBe(200); + expect(mockIssueService.update).toHaveBeenCalled(); + }); + + it("allows a board user to assign to a paused agent deliberately", async () => { + const existing = makeIssue({ assigneeAgentId: null }); + const updated = makeIssue({ assigneeAgentId: PAUSED_AGENT_ID }); + mockIssueService.getById.mockResolvedValue(existing); + mockIssueService.update.mockResolvedValue(updated); + + const res = await request(createApp(boardActor())) + .patch(`/api/issues/${existing.id}`) + .send({ assigneeAgentId: PAUSED_AGENT_ID }); + + expect(res.status).toBe(200); + expect(mockIssueService.update).toHaveBeenCalled(); + }); +}); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 1bb7c5379a..ad04b3f19d 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -4399,6 +4399,7 @@ export function issueRoutes( async function normalizeIssueAssigneeAgentReference( companyId: string, rawAssigneeAgentId: string | null | undefined, + options: { actorType?: string } = {}, ) { if (rawAssigneeAgentId === undefined || rawAssigneeAgentId === null) { return rawAssigneeAgentId; @@ -4422,6 +4423,18 @@ export function issueRoutes( if (resolved.agent.status === "terminated") { throw conflict("Cannot assign work to terminated agents"); } + // Agents must not route work to a paused peer/manager: the assignment is + // accepted silently, nothing will ever run it, and the issue becomes an + // invisible dead letter (e.g. escalation issues assigned to a paused + // manager via the org chart). Humans may still assign to paused agents + // deliberately — the pause state is visible in the UI and staging work + // for a later unpause is a legitimate workflow. + if (options.actorType === "agent" && resolved.agent.status === "paused") { + throw conflict( + "Cannot assign work to a paused agent. Assign an invokable agent, leave the issue unassigned, or escalate to a board operator instead.", + { assigneeAgentId: resolved.agent.id, assigneeStatus: "paused" }, + ); + } if (resolved.agent.orgChainHealth?.status === "invalid_org_chain") { throw conflict( resolved.agent.orgChainHealth?.repairGuidance ?? @@ -7128,6 +7141,7 @@ export function issueRoutes( const normalizedAssigneeAgentId = await normalizeIssueAssigneeAgentReference( companyId, rawCreateBody.assigneeAgentId as string | null | undefined, + { actorType: req.actor.type }, ); const actor = getActorInfo(req); const runWorkspaceInheritanceSourceIssueId = hasExplicitIssueWorkspaceCreateSelection(rawCreateBody) @@ -7343,6 +7357,7 @@ export function issueRoutes( const normalizedAssigneeAgentId = await normalizeIssueAssigneeAgentReference( parent.companyId, sanitizedBody.assigneeAgentId as string | null | undefined, + { actorType: req.actor.type }, ); const createBody = { ...sanitizedBody, @@ -7519,6 +7534,7 @@ export function issueRoutes( const normalizedAssigneeAgentId = await normalizeIssueAssigneeAgentReference( sourceIssue.companyId, sanitizedChild.assigneeAgentId as string | null | undefined, + { actorType: req.actor.type }, ); const childBody = { ...sanitizedChild, @@ -7768,6 +7784,7 @@ export function issueRoutes( const normalizedAssigneeAgentId = await normalizeIssueAssigneeAgentReference( existing.companyId, req.body.assigneeAgentId as string | null | undefined, + { actorType: req.actor.type }, ); const titleOrDescriptionChanged = req.body.title !== undefined || req.body.description !== undefined; const existingRelations =