Fix stale closure-comment wakeups on done issue updates (#8656)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The issue update route is part of the workflow layer that records board state changes and emits follow-up wakes for agents. > - A single `PATCH /api/issues/:id` request can both close an issue and add the closure comment that explains the final disposition. > - The bug was that the comment-wakeup decision used the issue's pre-update status, so a request that changed `in_progress` to `done` could still enqueue an `issue_commented` wake as if the issue remained open. > - That stale wake could cause already-completed Sentry-family follow-up issues to drift back into active work even though the closure comment was the only new activity. > - This pull request makes the wake suppression decision use the post-update issue status and covers the closure-comment path with a focused regression test. > - The benefit is that terminal issue updates stay terminal unless a separate explicit reopen or resume path is used. ## Linked Issues or Issue Description No public GitHub issue exists for this instance-specific workflow bug, so the issue is described inline. Bug report: - What happened: when an issue was marked `done` with a closure comment in the same `PATCH /api/issues/:id` request, the route could still enqueue an `issue_commented` wake because it checked the pre-update status. - Expected behavior: a closure comment written as part of the terminal update should not wake the assignee again or clear the terminal disposition. - Steps to reproduce: start with an assigned issue in `in_progress`, patch it to `done` while including a comment, then inspect whether an `issue_commented` wake is emitted for the assignee. - Deployment mode: local Paperclip workflow/API behavior. - Related public PRs found during duplicate search: #6657 appears to address a broader stale closeout-comment reopen path; this PR is narrower and targets the same-request post-update status decision in `PATCH /api/issues/:id`. ## What Changed - Use the post-update issue status when deciding whether a PATCH comment should enqueue an `issue_commented` wake. - Add a regression test covering `in_progress` to `done` with a closure comment so the assignee is not woken again after the issue is already closed. ## Verification - `bin/ci`: absent in this repo, so I used the repo's targeted test-equivalent commands for the touched API route. - `pnpm install --frozen-lockfile --ignore-scripts`: passed, with non-fatal warnings about missing `paperclip-plugin-dev-server` bins because `packages/plugins/sdk/dist/dev-cli.js` is not built under `--ignore-scripts`. - `pnpm run preflight:workspace-links && pnpm exec vitest run server/src/__tests__/issue-update-comment-wakeup-routes.test.ts`: passed (`Test Files 1 passed`, `Tests 8 passed`). - GitHub PR workflow checks for build, typecheck, server tests, workspace tests, serialized suites, e2e, canary dry run, security scans, and policy are green on commit `5a8bd799edd606731fd5e215ea97417a655338ea`. - A normal `pnpm install --frozen-lockfile` is blocked on this host before tests because `sharp` attempts a native build under Node `26.1.0` / Python `3.14.5` and fails on missing Python `distutils`; the route-level verification above used `--ignore-scripts` to avoid that local toolchain issue. ## Risks Low risk. The behavior change is limited to comment-wakeup suppression during issue update handling and only narrows wake emission when the post-update status is terminal. The main edge case is that a same-request terminal update with a comment will no longer wake the assignee; explicit reopen or resume flows should remain the correct way to restart completed work. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex via the managed local Codex adapter, model `gpt-5.5` with repository tool use and shell execution. The implementation and PR update were produced with AI assistance under the TechWright CTO Architect role. ## 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) - [ ] 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 Checklist notes: - The branch was already opened as `worker/TEC-1440-reopen-drift`; I am leaving the box unchecked rather than hiding that the live PR branch includes an internal coordination id. - The only non-green automated check before this body update was the automated review/template gate. Greptile was 4/5 because of this PR-description issue, with no code change requested. --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
166f381d3f
commit
b5bb236bc1
|
|
@ -477,6 +477,41 @@ describe("issue update comment wakeups", () => {
|
|||
);
|
||||
});
|
||||
|
||||
it("does not wake the assignee when a closure comment marks the issue done", async () => {
|
||||
const existing = makeIssue({
|
||||
assigneeAgentId: ASSIGNEE_AGENT_ID,
|
||||
assigneeUserId: null,
|
||||
status: "in_progress",
|
||||
});
|
||||
const updated = {
|
||||
...existing,
|
||||
status: "done",
|
||||
completedAt: new Date("2026-06-26T16:30:00.000Z"),
|
||||
};
|
||||
mockIssueService.getById.mockResolvedValue(existing);
|
||||
mockIssueService.update.mockResolvedValue(updated);
|
||||
mockIssueService.addComment.mockResolvedValue({
|
||||
id: "comment-close-1",
|
||||
issueId: existing.id,
|
||||
companyId: existing.companyId,
|
||||
body: "Closing this out.",
|
||||
});
|
||||
|
||||
const res = await request(await createApp())
|
||||
.patch(`/api/issues/${existing.id}`)
|
||||
.send({
|
||||
status: "done",
|
||||
comment: "Closing this out.",
|
||||
});
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
const issueCommentedWakeCalls = mockHeartbeatService.wakeup.mock.calls.filter(
|
||||
([, wakeup]: [string, { reason?: string }]) => wakeup?.reason === "issue_commented",
|
||||
);
|
||||
expect(issueCommentedWakeCalls).toEqual([]);
|
||||
});
|
||||
|
||||
it("wakes the assignee on top-level board issue comments", async () => {
|
||||
const existing = makeIssue({
|
||||
assigneeAgentId: ASSIGNEE_AGENT_ID,
|
||||
|
|
|
|||
|
|
@ -9972,7 +9972,10 @@ export function issueRoutes(
|
|||
const assigneeId = issue.assigneeAgentId;
|
||||
const actorIsAgent = actor.actorType === "agent";
|
||||
const selfComment = actorIsAgent && actor.actorId === assigneeId;
|
||||
const skipAssigneeCommentWake = selfComment || isClosed;
|
||||
// Re-derive closed-ness from the post-update issue so a status change
|
||||
// like in_progress -> done with a closure comment does not enqueue a
|
||||
// stale issue_commented wake for an already-completed issue.
|
||||
const skipAssigneeCommentWake = selfComment || isClosedIssueStatus(issue.status);
|
||||
|
||||
if (assigneeId && !assigneeChanged && (reopened || !skipAssigneeCommentWake)) {
|
||||
addWakeup(assigneeId, {
|
||||
|
|
|
|||
Loading…
Reference in New Issue