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 <noreply@paperclip.ing>
This commit is contained in:
Nicky Leach
@@ -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 () => {
|
||||
|
||||
@@ -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<ReturnType<typeof decideIssueAccess>>) {
|
||||
return decision !== true && decision.reason === "allow_issue_mention_grant";
|
||||
}
|
||||
|
||||
async function filterIssuesForActor<T extends Parameters<typeof decideIssueAccess>[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,
|
||||
|
||||
Reference in New Issue
Block a user