fix(server): refuse agent-initiated issue assignment to paused agents (#10648)
## 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
This commit is contained in:
Devin Foley
GitHub
parent
27f8c8dbcf
commit
ada47be764
|
|
@ -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<string, string> = {
|
||||
[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<string, unknown>;
|
||||
|
||||
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<string, unknown> = {}) {
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
|
@ -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 =
|
||||
|
|
|
|||
Loading…
Reference in New Issue