From fb0e3fb02a61a4ea872847103b0c0d4042b15755 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Fri, 19 Jun 2026 16:47:06 -0700 Subject: [PATCH] Fix mention-granted closed issue comments with resume intent (#8350) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Issue comments are part of the control plane boundary between agent collaboration and task mutation. > - Mention grants intentionally let a mentioned peer agent answer on an issue thread without taking ownership of the task. > - Closed issue comments have a second mutation guard because comments can also request resume or reopen behavior. > - A mention-granted comment should stay append-only unless the actor also has mutation authority. > - This pull request preserves the narrow comment path while keeping explicit `resume` and `reopen` intent behind mutation authorization. > - The benefit is a smaller authorization exception with regression coverage for both allowed and denied paths. ## Linked Issues or Issue Description Refs #2884 Related prior authorization work: #8024, #7863, #6113, #7998. Bug fix description: - What happened: a mention-granted non-assignee agent could be treated as authorized for the closed issue comment route without preserving a clean distinction between append-only comments and explicit reopen/resume mutation intent. - Expected behavior: a mention grant allows a plain comment on a closed issue, but `resume: true` or `reopen: true` still requires mutation authorization. - Steps to reproduce: create a closed issue assigned to one agent, give a different agent a valid mention-scoped comment grant, then POST a comment as that agent with and without `resume`/`reopen` intent. - Deployment mode: server route behavior; covered by focused Vitest regression tests. ## What Changed - Preserve the `issue:comment` authorization decision in `POST /api/issues/:id/comments` so the route can identify legitimate mention-grant decisions. - Skip the closed-issue non-assignee mutation fallback only for inert mention-granted comments. - Continue requiring mutation authorization when a mention-granted closed issue comment includes explicit `resume` or `reopen` intent. - Add regression coverage for the allowed inert comment and denied resume/reopen cases. ## Verification - `pnpm vitest run server/src/__tests__/issue-comment-reopen-routes.test.ts` passed locally: 72 tests. ## Risks - Low risk: scoped to the issue comment route and tests for a specific authorization decision reason. - Residual risk: CI should run the full PR suite. ## Model Used OpenAI GPT-5 Codex via Paperclip Git Expert agent, with tool use for code inspection, test execution, Git, and GitHub operations. ## 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] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- .../issue-comment-reopen-routes.test.ts | 77 +++++++++++++++++++ server/src/routes/issues.ts | 28 +++++-- 2 files changed, 99 insertions(+), 6 deletions(-) diff --git a/server/src/__tests__/issue-comment-reopen-routes.test.ts b/server/src/__tests__/issue-comment-reopen-routes.test.ts index 594fdef5..30693674 100644 --- a/server/src/__tests__/issue-comment-reopen-routes.test.ts +++ b/server/src/__tests__/issue-comment-reopen-routes.test.ts @@ -552,6 +552,83 @@ describe.sequential("issue comment reopen routes", () => { expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); }); + it("allows mention-granted non-assignee agent POST comments on closed issues without reopening", async () => { + const mentionedAgentId = "33333333-3333-4333-8333-333333333333"; + mockIssueService.getById.mockResolvedValue(makeIssue("done")); + mockIssueService.addComment.mockResolvedValue({ + id: "comment-1", + issueId: "11111111-1111-4111-8111-111111111111", + companyId: "company-1", + body: "I can answer the mention without reopening.", + createdAt: new Date(), + updatedAt: new Date(), + authorAgentId: mentionedAgentId, + authorUserId: null, + }); + mockAccessService.decide.mockImplementation(async (input: { action?: string }) => { + const allowed = input.action === "issue:comment"; + return { + allowed, + action: input.action, + reason: allowed ? "allow_issue_mention_grant" : "deny_missing_grant", + explanation: allowed ? "Allowed by a mention-scoped issue comment grant." : "Missing permission.", + }; + }); + + const res = await request(await installActor(createApp(), agentActor(mentionedAgentId))) + .post("/api/issues/11111111-1111-4111-8111-111111111111/comments") + .send({ body: "I can answer the mention without reopening." }); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + expect(mockIssueService.addComment).toHaveBeenCalled(); + expect(mockIssueService.update).not.toHaveBeenCalled(); + expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); + expect(mockAccessService.decide).not.toHaveBeenCalledWith(expect.objectContaining({ action: "issue:mutate" })); + }); + + it.each([ + ["resume", { resume: true }], + ["reopen", { reopen: true }], + ])( + // Mention grants are append-only; explicit lifecycle intent still requires mutation authority. + "denies mention-granted non-assignee agent POST comments on closed issues with %s intent", + async (_name, intent) => { + const mentionedAgentId = "33333333-3333-4333-8333-333333333333"; + mockIssueService.getById.mockResolvedValue(makeIssue("done")); + mockIssueService.addComment.mockResolvedValue({ + id: "comment-1", + issueId: "11111111-1111-4111-8111-111111111111", + companyId: "company-1", + body: "Please continue this closed issue.", + createdAt: new Date(), + updatedAt: new Date(), + authorAgentId: mentionedAgentId, + authorUserId: null, + }); + mockAccessService.decide.mockImplementation(async (input: { action?: string }) => { + const allowed = input.action === "issue:comment"; + return { + allowed, + action: input.action, + reason: allowed ? "allow_issue_mention_grant" : "deny_missing_grant", + explanation: allowed ? "Allowed by a mention-scoped issue comment grant." : "Missing permission.", + }; + }); + + const res = await request(await installActor(createApp(), agentActor(mentionedAgentId))) + .post("/api/issues/11111111-1111-4111-8111-111111111111/comments") + .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(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ action: "issue:comment" })); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ action: "issue:mutate" })); + expect(mockIssueService.update).not.toHaveBeenCalled(); + expect(mockIssueService.addComment).not.toHaveBeenCalled(); + expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); + }, + ); + // POST self-comment from the assignee agent on a done issue with explicit // reopen=true is the same log-class signal — the guard must suppress reopen. it("does not reopen via POST comment+reopen when the assignee agent is the actor on a done issue", async () => { diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 1a4897b0..fbce62cc 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -1915,7 +1915,11 @@ export function issueRoutes( res.status(403).json({ error: "Issue is outside this actor's authorization boundary" }); return false; } - return true; + return boundaryDecision; + } + + function isIssueMentionGrantDecision(decision: true | Awaited>) { + return decision !== true && decision.reason === "allow_issue_mention_grant"; } async function filterIssuesForActor[1]>(req: Request, rows: T[]) { @@ -7359,7 +7363,8 @@ export function issueRoutes( return; } assertCompanyAccess(req, issue.companyId); - if (!(await assertAgentIssueCommentAllowed(req, res, issue))) return; + const commentAccessDecision = await assertAgentIssueCommentAllowed(req, res, issue); + if (!commentAccessDecision) return; if (!assertStructuredCommentFieldsAllowed(req, res, { presentation: req.body.presentation, metadata: req.body.metadata, @@ -7376,19 +7381,30 @@ export function issueRoutes( const interruptRequested = req.body.interrupt === true; const isClosed = isClosedIssueStatus(issue.status); const isBlocked = issue.status === "blocked"; + const mentionGrantedPeerAgentCommentOnly = + isClosed && + req.actor.type === "agent" && + issue.assigneeAgentId !== null && + issue.assigneeAgentId !== req.actor.agentId && + !reopenRequested && + !resumeRequested && + isIssueMentionGrantDecision(commentAccessDecision); + const effectiveReopenRequested = mentionGrantedPeerAgentCommentOnly ? false : reopenRequested; + const effectiveResumeRequested = mentionGrantedPeerAgentCommentOnly ? false : resumeRequested; if ( isClosed && req.actor.type === "agent" && issue.assigneeAgentId !== null && - issue.assigneeAgentId !== req.actor.agentId + issue.assigneeAgentId !== req.actor.agentId && + !mentionGrantedPeerAgentCommentOnly ) { if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; } - if (resumeRequested === true && !(await assertExplicitResumeIntentAllowed(req, res, issue))) return; - if (resumeRequested !== true && reopenRequested === true && req.actor.type === "agent") { + if (effectiveResumeRequested === true && !(await assertExplicitResumeIntentAllowed(req, res, issue))) return; + if (effectiveResumeRequested !== true && effectiveReopenRequested === true && req.actor.type === "agent") { if (!(await assertExplicitResumeIntentAllowed(req, res, issue))) return; } - const explicitMoveToTodoRequested = reopenRequested || resumeRequested === true; + const explicitMoveToTodoRequested = effectiveReopenRequested || effectiveResumeRequested === true; const scheduledRetryForHumanComment = shouldHumanCommentResumeInProgressScheduledRetry({ hasComment: true,