diff --git a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts index 821a5c37f0a..43ff1745882 100644 --- a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts +++ b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts @@ -9,6 +9,9 @@ const ownerAgentId = "33333333-3333-4333-8333-333333333333"; const peerAgentId = "44444444-4444-4444-8444-444444444444"; const ownerRunId = "55555555-5555-4555-8555-555555555555"; const recoveryActionId = "77777777-7777-4777-8777-777777777777"; +const issueBoundaryError = "Issue is outside this actor's authorization boundary"; +const issueBoundaryHint = + "Do not retry. Route instead: create a child issue assigned to the owner, comment on your own issue naming the ask, or open an issue-thread interaction for board escalation."; const mockIssueService = vi.hoisted(() => ({ addComment: vi.fn(), @@ -356,6 +359,14 @@ function boardActor() { }; } +function expectIssueBoundaryDenied(body: Record, reason: string) { + expect(body).toEqual({ + error: issueBoundaryError, + reason, + hint: issueBoundaryHint, + }); +} + describe("agent issue mutation checkout ownership", () => { beforeEach(() => { vi.resetModules(); @@ -805,7 +816,26 @@ describe("agent issue mutation checkout ownership", () => { .send({ body: "I was not mentioned." }); expect(res.status, JSON.stringify(res.body)).toBe(403); - expect(res.body.error).toBe("Issue is outside this actor's authorization boundary"); + expectIssueBoundaryDenied(res.body, "deny_missing_grant"); + expect(mockIssueService.addComment).not.toHaveBeenCalled(); + }); + + it("passes through the issue mutate denial reason for peer agents", async () => { + mockIssueService.getById.mockResolvedValue(makeIssue({ status: "todo", assigneeAgentId: ownerAgentId })); + mockAccessService.decide.mockImplementation(async (input: { action: string }) => ({ + allowed: input.action !== "issue:mutate", + action: input.action, + reason: input.action === "issue:mutate" ? "deny_scope" : "allow_explicit_grant", + explanation: input.action === "issue:mutate" ? "Issue is outside scope." : "Allowed by test default.", + })); + + const res = await request(await createApp(peerActor())) + .patch(`/api/issues/${issueId}`) + .send({ status: "done" }); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + expectIssueBoundaryDenied(res.body, "deny_scope"); + expect(mockIssueService.update).not.toHaveBeenCalled(); expect(mockIssueService.addComment).not.toHaveBeenCalled(); }); @@ -821,7 +851,7 @@ describe("agent issue mutation checkout ownership", () => { .get(`/api/issues/${issueId}/comments`); expect(res.status, JSON.stringify(res.body)).toBe(403); - expect(res.body.error).toBe("Issue is outside this actor's authorization boundary"); + expectIssueBoundaryDenied(res.body, "deny_low_trust_boundary"); expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ action: "issue:read" })); }); @@ -837,7 +867,7 @@ describe("agent issue mutation checkout ownership", () => { .get(`/api/issues/${issueId}/interactions`); expect(res.status, JSON.stringify(res.body)).toBe(403); - expect(res.body.error).toBe("Issue is outside this actor's authorization boundary"); + expectIssueBoundaryDenied(res.body, "deny_low_trust_boundary"); expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ action: "issue:read" })); expect(mockIssueThreadInteractionService.listForIssue).not.toHaveBeenCalled(); }); @@ -883,7 +913,7 @@ describe("agent issue mutation checkout ownership", () => { .get(`/api/issues/${issueId}/comments/comment-1`); expect(res.status, JSON.stringify(res.body)).toBe(403); - expect(res.body.error).toBe("Issue is outside this actor's authorization boundary"); + expectIssueBoundaryDenied(res.body, "deny_low_trust_boundary"); expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ action: "issue:read" })); expect(mockIssueService.getComment).not.toHaveBeenCalled(); }); diff --git a/server/src/__tests__/issue-comment-reopen-routes.test.ts b/server/src/__tests__/issue-comment-reopen-routes.test.ts index 3a0b0907480..2b6b05fbdb4 100644 --- a/server/src/__tests__/issue-comment-reopen-routes.test.ts +++ b/server/src/__tests__/issue-comment-reopen-routes.test.ts @@ -635,7 +635,11 @@ describe.sequential("issue comment reopen routes", () => { .send({ body: "Please continue this closed issue.", ...intent }); expect(res.status, JSON.stringify(res.body)).toBe(403); - expect(res.body).toEqual({ error: "Issue is outside this actor's authorization boundary" }); + expect(res.body).toEqual({ + error: "Issue is outside this actor's authorization boundary", + reason: "deny_missing_grant", + hint: "Do not retry. Route instead: create a child issue assigned to the owner, comment on your own issue naming the ask, or open an issue-thread interaction for board escalation.", + }); expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ action: "issue:comment" })); expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ action: "issue:mutate" })); expect(mockIssueService.update).not.toHaveBeenCalled(); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 7d67db47344..9dff56bbe26 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -153,7 +153,7 @@ import { ISSUE_WAKE_DIAGNOSTICS_MAX_WAKE_REQUESTS, readAcceptedPlanConfirmationTarget, } from "../services/issues.js"; -import { authorizationDeniedDetails } from "../services/authorization.js"; +import { authorizationDeniedDetails, type AuthorizationDecision } from "../services/authorization.js"; import { environmentService } from "../services/environments.js"; import { environmentRuntimeService } from "../services/environment-runtime.js"; import { redactSensitiveText } from "../redaction.js"; @@ -185,6 +185,9 @@ import { import { externalObjectService } from "../services/external-objects.js"; const MAX_ISSUE_COMMENT_LIMIT = 500; +const ISSUE_AUTHORIZATION_BOUNDARY_ERROR = "Issue is outside this actor's authorization boundary"; +const ISSUE_AUTHORIZATION_BOUNDARY_HINT = + "Do not retry. Route instead: create a child issue assigned to the owner, comment on your own issue naming the ask, or open an issue-thread interaction for board escalation."; const updateIssueRouteSchema = updateIssueSchema.extend({ interrupt: z.boolean().optional(), }); @@ -3282,10 +3285,18 @@ export function issueRoutes( async function assertIssueReadAllowed(req: Request, res: Response, issue: Parameters[1]) { const decision = await decideIssueAccess(req, issue, "issue:read"); if (decision.allowed) return true; - res.status(403).json({ error: "Issue is outside this actor's authorization boundary" }); + sendIssueAuthorizationBoundaryDenied(res, decision); return false; } + function sendIssueAuthorizationBoundaryDenied(res: Response, decision: AuthorizationDecision) { + res.status(403).json({ + error: ISSUE_AUTHORIZATION_BOUNDARY_ERROR, + reason: decision.reason, + hint: ISSUE_AUTHORIZATION_BOUNDARY_HINT, + }); + } + async function assertAgentIssueCommentAllowed( req: Request, res: Response, @@ -3322,7 +3333,7 @@ export function issueRoutes( } const boundaryDecision = await decideIssueAccess(req, issue, "issue:comment"); if (!boundaryDecision.allowed) { - res.status(403).json({ error: "Issue is outside this actor's authorization boundary" }); + sendIssueAuthorizationBoundaryDenied(res, boundaryDecision); return false; } return boundaryDecision; @@ -3411,7 +3422,7 @@ export function issueRoutes( } const boundaryDecision = await decideIssueAccess(req, issue, "issue:mutate"); if (!boundaryDecision.allowed) { - res.status(403).json({ error: "Issue is outside this actor's authorization boundary" }); + sendIssueAuthorizationBoundaryDenied(res, boundaryDecision); return false; } if (issue.assigneeAgentId === null) { diff --git a/skills/paperclip/SKILL.md b/skills/paperclip/SKILL.md index 3c9b66555cd..3cad8aad915 100644 --- a/skills/paperclip/SKILL.md +++ b/skills/paperclip/SKILL.md @@ -301,6 +301,7 @@ For commands, response fields, and MCP tools, read: ## Critical Rules - **Never retry a 409.** The task belongs to someone else. +- **403 boundary -> never retry; follow the hint in the error body.** - **Never look for unassigned work.** No assignments = exit. - **Self-assign only for explicit @-mention handoff.** Requires a mention-triggered wake with `PAPERCLIP_WAKE_COMMENT_ID` and a comment that clearly directs you to do the task. Use checkout (never direct assignee patch). - **Honor "send it back to me" requests from board users.** If a board/user asks for review handoff (e.g. "let me review it", "assign it back to me"), reassign to them with `assigneeAgentId: null` and `assigneeUserId: ""`, typically setting status to `in_review` instead of `done`. Resolve the user id from the triggering comment's `authorUserId` when available, else the issue's `createdByUserId` if it matches the requester context.