Skip to content

Commit 9ec5c99

Browse files
AURDVIN Dev AgentPaperclip-Paperclipclaude
committed
fix(interactions): refuse to resolve a card whose issue is already closed
`queueResolvedInteractionContinuationWakeup` drops the continuation wakeup when the issue is `done`/`cancelled`. The `accept`, `reject`, `respond` and `verdicts` routes never looked at the issue status, so a resolution there ran all the way through: the card flipped to `accepted`/`rejected`/`answered`, `logActivity` recorded it, the route answered 200 — and nobody was woken. The card reads as answerable in the UI, someone answers it in good faith, and nothing happens. No error, no hint. Closing an issue expires its pending cards (#10251), so in the normal case the `status !== "pending"` check already refuses these. What it does not cover is the residue: rows that were filed before that expiry shipped are still `pending` on issues that closed months ago. On the instance this was found on, 154 such cards sit on 145 closed issues, 146 of them carrying `continuationPolicy: wake_assignee` — every one of them a trap that looks answerable and is not. Three of them ask the reader in their prompt to reply. So the guard is a second line rather than the first: it reads the issue status after the pending check and answers 409 with the closed status in the error details, which is the honest outcome for an answer that cannot travel. An already-resolved card still reports "Interaction has already been resolved" — that message is more specific, and the ordering keeps this change to the one case that is currently silent. `withdraw` and `cancel` are deliberately left alone. Retiring a stale card on a closed issue is exactly the cleanup those routes exist for, and blocking them would strand the residue this guard makes visible. The status is read from the database inside the resolution path rather than taken from the caller's issue object, so the plugin host's accept/reject path is covered by the same check and not just the HTTP routes. Co-Authored-By: Paperclip <noreply@paperclip.ing> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 4eace88 commit 9ec5c99

3 files changed

Lines changed: 174 additions & 13 deletions

File tree

server/src/services/issue-thread-interactions.test.ts

Lines changed: 135 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,25 +11,35 @@ vi.mock("./issues.js", () => ({
1111

1212
type SelectRow = Record<string, unknown>;
1313

14-
function createSelectChain(rows: SelectRow[]) {
14+
function createWhereChain(rows: SelectRow[]) {
1515
return {
16-
from() {
16+
where() {
1717
return {
18-
where() {
19-
return {
20-
then(callback: (rows: SelectRow[]) => unknown) {
21-
return Promise.resolve(callback(rows));
22-
},
23-
};
18+
then(callback: (rows: SelectRow[]) => unknown) {
19+
return Promise.resolve(callback(rows));
2420
},
2521
};
2622
},
2723
};
2824
}
2925

26+
function createSelectChain(rows: SelectRow[]) {
27+
return {
28+
from() {
29+
return createWhereChain(rows);
30+
},
31+
};
32+
}
33+
3034
function createFakeDb(args: {
3135
interactionRow: Record<string, unknown>;
3236
parentRows?: SelectRow[];
37+
/**
38+
* Rows the `issues` table answers with. Only consulted when set, so the
39+
* call-ordered defaults above stay untouched for the tests that do not care
40+
* about the issue's status.
41+
*/
42+
issueRows?: SelectRow[];
3343
}) {
3444
let interactionRow = { ...args.interactionRow };
3545
const issueTouches: Array<Record<string, unknown>> = [];
@@ -38,10 +48,15 @@ function createFakeDb(args: {
3848
let selectCallCount = 0;
3949

4050
const db: any = {
41-
select: vi.fn(() => {
42-
selectCallCount += 1;
43-
return createSelectChain(selectCallCount === 1 ? [interactionRow] : (args.parentRows ?? []));
44-
}),
51+
select: vi.fn(() => ({
52+
from(table: unknown) {
53+
if (args.issueRows && getTableName(table as never) === "issues") {
54+
return createWhereChain(args.issueRows);
55+
}
56+
selectCallCount += 1;
57+
return createWhereChain(selectCallCount === 1 ? [interactionRow] : (args.parentRows ?? []));
58+
},
59+
})),
4560
update: vi.fn((table: unknown) => ({
4661
set(values: Record<string, unknown>) {
4762
return {
@@ -327,4 +342,112 @@ describe("issueThreadInteractionService", () => {
327342
expect(state.toolActionRequestUpdates).toHaveLength(1);
328343
expect(state.toolActionRequestUpdates[0]).toMatchObject({ status: "expired", resolvedByUserId: "local-board" });
329344
});
345+
346+
describe("resolving a card whose issue is already closed", () => {
347+
const ISSUE_ID = "11111111-1111-4111-8111-111111111111";
348+
349+
function confirmationRow(overrides: Record<string, unknown> = {}) {
350+
return {
351+
id: "interaction-closed-carrier", companyId: "company-1", issueId: ISSUE_ID,
352+
kind: "request_confirmation", status: "pending", continuationPolicy: "wake_assignee",
353+
sourceCommentId: null, sourceRunId: null, title: null, summary: null,
354+
createdByAgentId: "agent-1", createdByUserId: null, resolvedByAgentId: null, resolvedByUserId: null,
355+
payload: { version: 1, prompt: "Proceed?" }, result: null, resolvedAt: null,
356+
createdAt: new Date("2026-07-25T10:00:00.000Z"), updatedAt: new Date("2026-07-25T10:00:00.000Z"),
357+
...overrides,
358+
};
359+
}
360+
361+
function questionsRow() {
362+
return confirmationRow({
363+
id: "interaction-closed-questions",
364+
kind: "ask_user_questions",
365+
payload: {
366+
version: 1,
367+
questions: [{
368+
id: "scope", prompt: "Pick one", selectionMode: "single",
369+
options: [{ id: "a", label: "A" }],
370+
}],
371+
},
372+
});
373+
}
374+
375+
for (const issueStatus of ["done", "cancelled"] as const) {
376+
it(`refuses to accept a confirmation on a ${issueStatus} issue`, async () => {
377+
const { issueThreadInteractionService } = await import("./issue-thread-interactions.js");
378+
const state = createFakeDb({ interactionRow: confirmationRow(), issueRows: [{ status: issueStatus }] });
379+
const svc = issueThreadInteractionService(state.db as never);
380+
381+
await expect(svc.acceptInteraction(
382+
{ id: ISSUE_ID, companyId: "company-1", projectId: null, goalId: null },
383+
"interaction-closed-carrier",
384+
{},
385+
{ userId: "local-board" },
386+
)).rejects.toMatchObject({ status: 409 });
387+
// The silent failure this guards against is exactly a card that flips to
388+
// "accepted" while no continuation wakeup fires, so the card must not move.
389+
expect(state.interactionUpdates).toHaveLength(0);
390+
expect(state.getInteractionRow().status).toBe("pending");
391+
});
392+
}
393+
394+
it("refuses to reject a confirmation on a closed issue", async () => {
395+
const { issueThreadInteractionService } = await import("./issue-thread-interactions.js");
396+
const state = createFakeDb({ interactionRow: confirmationRow(), issueRows: [{ status: "done" }] });
397+
const svc = issueThreadInteractionService(state.db as never);
398+
399+
await expect(svc.rejectInteraction(
400+
{ id: ISSUE_ID, companyId: "company-1" },
401+
"interaction-closed-carrier",
402+
{ reason: "No" },
403+
{ userId: "local-board" },
404+
)).rejects.toMatchObject({ status: 409 });
405+
expect(state.interactionUpdates).toHaveLength(0);
406+
});
407+
408+
it("refuses to answer questions on a closed issue", async () => {
409+
const { issueThreadInteractionService } = await import("./issue-thread-interactions.js");
410+
const state = createFakeDb({ interactionRow: questionsRow(), issueRows: [{ status: "cancelled" }] });
411+
const svc = issueThreadInteractionService(state.db as never);
412+
413+
await expect(svc.answerQuestions(
414+
{ id: ISSUE_ID, companyId: "company-1" },
415+
"interaction-closed-questions",
416+
{ answers: [{ questionId: "scope", optionIds: ["a"] }] },
417+
{ userId: "local-board" },
418+
)).rejects.toMatchObject({ status: 409 });
419+
expect(state.interactionUpdates).toHaveLength(0);
420+
});
421+
422+
it("still lets the creator withdraw a card whose issue is closed", async () => {
423+
const { issueThreadInteractionService } = await import("./issue-thread-interactions.js");
424+
const state = createFakeDb({ interactionRow: confirmationRow(), issueRows: [{ status: "done" }] });
425+
const svc = issueThreadInteractionService(state.db as never);
426+
427+
const withdrawn = await svc.withdrawInteraction(
428+
{ id: ISSUE_ID, companyId: "company-1" },
429+
"interaction-closed-carrier",
430+
{ reason: "Superseded" },
431+
{ agentId: "agent-1" },
432+
);
433+
434+
expect(withdrawn.status).toBe("cancelled");
435+
});
436+
437+
it("keeps reporting an already-resolved card as resolved rather than as a closed issue", async () => {
438+
const { issueThreadInteractionService } = await import("./issue-thread-interactions.js");
439+
const state = createFakeDb({
440+
interactionRow: confirmationRow({ status: "expired" }),
441+
issueRows: [{ status: "done" }],
442+
});
443+
const svc = issueThreadInteractionService(state.db as never);
444+
445+
await expect(svc.acceptInteraction(
446+
{ id: ISSUE_ID, companyId: "company-1", projectId: null, goalId: null },
447+
"interaction-closed-carrier",
448+
{},
449+
{ userId: "local-board" },
450+
)).rejects.toMatchObject({ status: 409, message: "Interaction has already been resolved" });
451+
});
452+
});
330453
});

server/src/services/issue-thread-interactions.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ type ResolvedInteractionResult = {
7878

7979
type IssueThreadInteractionRow = typeof issueThreadInteractions.$inferSelect;
8080
type IssueTouchDb = Pick<Db, "update">;
81+
type IssueStatusReadDb = Pick<Db, "select">;
8182

8283
type IssueResolutionContext = {
8384
id: string;
@@ -240,6 +241,40 @@ function isTerminalIssueStatus(status: string) {
240241
return status === "done" || status === "cancelled";
241242
}
242243

244+
/**
245+
* A resolution on a closed issue must not look like it worked.
246+
*
247+
* `queueResolvedInteractionContinuationWakeup` (`server/src/routes/issues.ts`)
248+
* drops the continuation wakeup when the issue is `done`/`cancelled`, so an
249+
* accept/reject/answer there sets the card to a resolved status, writes the
250+
* activity entry, returns 200 — and wakes nobody. The card reads as answerable
251+
* in the UI, someone answers it in good faith, and nothing happens: no error,
252+
* no hint.
253+
*
254+
* Closing an issue expires its pending cards, so in the normal case the
255+
* resolution routes already fail on the `status !== "pending"` check. This guard
256+
* covers what that leaves: rows filed before auto-expiry-on-close shipped, and
257+
* any path that put an issue into a terminal status without going through it.
258+
*
259+
* Deliberately not applied to the administrative resolutions (withdraw, cancel):
260+
* retiring a stale card on a closed issue is exactly the cleanup those exist for.
261+
*/
262+
async function assertIssueOpenForInteractionResolution(
263+
db: IssueStatusReadDb,
264+
issue: { id: string; companyId: string },
265+
) {
266+
const [row] = await db
267+
.select({ status: issues.status })
268+
.from(issues)
269+
.where(and(eq(issues.id, issue.id), eq(issues.companyId, issue.companyId)));
270+
if (!row || !isTerminalIssueStatus(row.status)) return;
271+
throw conflict(
272+
"Cannot resolve an interaction on a closed issue: the answer would not resume any work. "
273+
+ "Reopen the issue first, or withdraw the interaction.",
274+
{ issueId: issue.id, issueStatus: row.status },
275+
);
276+
}
277+
243278
function shouldReturnAcceptedConfirmationToCreatorAgent(args: {
244279
issue: IssueResolutionContext;
245280
current: IssueThreadInteractionRow;
@@ -1028,6 +1063,7 @@ export function issueThreadInteractionService(db: Db) {
10281063
if (current.status !== "pending") {
10291064
throw conflict("Interaction has already been resolved");
10301065
}
1066+
await assertIssueOpenForInteractionResolution(db, args.issue);
10311067
return current;
10321068
}
10331069

@@ -1591,6 +1627,7 @@ export function issueThreadInteractionService(db: Db) {
15911627
}
15921628
throw conflict("Interaction has already been resolved");
15931629
}
1630+
await assertIssueOpenForInteractionResolution(tx, issue);
15941631

15951632
const expired = await expireStaleRequestConfirmationTarget(tx, {
15961633
row: current,
@@ -2129,6 +2166,7 @@ export function issueThreadInteractionService(db: Db) {
21292166
if (current.status !== "pending") {
21302167
throw conflict("Interaction has already been resolved");
21312168
}
2169+
await assertIssueOpenForInteractionResolution(db, issue);
21322170

21332171
const interaction = hydrateInteraction(current) as AskUserQuestionsInteraction;
21342172
const normalizedAnswers = normalizeQuestionAnswers({

skills/paperclip/references/api-reference.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -975,7 +975,7 @@ Resolved result (`RequestCheckboxConfirmationResult`):
975975
Other outcomes match `request_confirmation`:
976976
977977
- `withdrawn` — `{ outcome: "withdrawn", reason }`. Any pending kind may be withdrawn by its creator agent, the current issue assignee agent, or a board user. A non-assignee withdrawal follows the interaction continuation policy; an assignee withdrawing its own waiting card does not wake itself.
978-
- `issue_closed` — `{ outcome: "issue_closed" }`. Transitioning the issue to `done` or `cancelled` expires all pending interactions without continuation wakes; listing a terminal issue also performs a catch-up sweep for historical residue.
978+
- `issue_closed` — `{ outcome: "issue_closed" }`. Transitioning the issue to `done` or `cancelled` expires all pending interactions without continuation wakes; listing a terminal issue also performs a catch-up sweep for historical residue. A card that is still `pending` on a closed issue (filed before that expiry shipped) cannot be resolved: `accept`, `reject`, `respond` and `verdicts` return **409** rather than recording an answer that wakes nobody. `withdraw` and `cancel` stay available so a stale card can still be retired.
979979
980980
- `rejected` — `{ outcome: "rejected", reason, commentId }`. `selectedOptionIds` is absent.
981981
- `superseded_by_comment` — `{ outcome: "superseded_by_comment", commentId }`. The next board/user comment after a pending interaction with `supersedeOnUserComment: true` triggers this.

0 commit comments

Comments
 (0)