Skip to content

fix(interactions): refuse to resolve a card whose issue is already closed - #11071

Open
juancarlosrial76-code wants to merge 1 commit into
paperclipai:masterfrom
juancarlosrial76-code:fix/reject-interaction-resolution-on-closed-issue
Open

fix(interactions): refuse to resolve a card whose issue is already closed#11071
juancarlosrial76-code wants to merge 1 commit into
paperclipai:masterfrom
juancarlosrial76-code:fix/reject-interaction-resolution-on-closed-issue

Conversation

@juancarlosrial76-code

@juancarlosrial76-code juancarlosrial76-code commented Aug 8, 2026

Copy link
Copy Markdown

Thinking Path

  • Paperclip is the open source app people use to manage AI agents for work
  • The board and its humans unblock agents through issue-thread interactions — confirmations, question sets, suggested tasks — and an answered card resumes the agent through a continuation wakeup
  • queueResolvedInteractionContinuationWakeup deliberately drops that wakeup when the issue is done/cancelled, but the accept, reject, respond and verdicts routes never checked the issue status, so a resolution there flipped the card, wrote the activity entry and answered 200 — while waking nobody
  • feat(interactions): add interaction withdrawal and terminal-issue expiry #10251 closed the forward-looking half by expiring pending cards when an issue goes terminal; it does not cover the rows filed before it shipped, which are still pending on issues that closed long ago and still render as answerable
  • On the instance this was found on, 154 such cards sit on 145 closed issues; 146 carry continuationPolicy: wake_assignee, and three ask the reader in their prompt to reply — every one of them is a trap that costs a person a real answer and gives back silence
  • This pull request makes the resolution paths read the issue status and answer 409 instead of recording an answer that cannot travel, while leaving withdraw/cancel open so the stale cards can still be retired
  • The benefit is that a resolution which does nothing no longer looks like one that worked

Linked Issues or Issue Description

Refs #10251 (the auto-expire-on-close hook this backstops), #10920 (open — backfills
the pre-#10251 residue this guard makes visible). No public GitHub issue tracks the
resolution-side hole; describing it inline per the bug template:

What happened?
POST /api/issues/:id/interactions/:interactionId/accept (and reject, respond,
verdicts) on an interaction that is still pending while its issue is done or
cancelled returns 200, sets the card to accepted/rejected/answered and writes
the activity entry — but queueResolvedInteractionContinuationWakeup returns early on
isClosedIssueStatus(issue.status), so no agent is woken. The answer is recorded and
has no effect.

Expected behavior
An answer that cannot resume anything should not report success. Either the card is
unanswerable, or answering it does something.

Steps to reproduce

  1. Create an issue, assign it to an agent.
  2. Create a request_confirmation on it with continuationPolicy: "wake_assignee".
  3. Write the issue to status: "cancelled" by a path that predates or bypasses the
    feat(interactions): add interaction withdrawal and terminal-issue expiry #10251 close hook — or simply hold any interaction filed before feat(interactions): add interaction withdrawal and terminal-issue expiry #10251 landed,
    which is still pending on an issue that closed earlier.
  4. POST .../accept. Response is 200, the card shows accepted, no wakeup is queued
    and no error is surfaced anywhere.

Paperclip version
Reproduced against @paperclipai/server 2026.609.0 (dist/routes/issues.js, the
bundle the live process loads); the code path is unchanged on current master.

Deployment mode
Self-hosted, local instance.

What Changed

  • assertIssueOpenForInteractionResolution() in
    server/src/services/issue-thread-interactions.ts reads the issue's status from the
    database and throws 409 ({ issueId, issueStatus } in the error details) when it is
    done/cancelled.
  • Wired into the four resolution entry points: getPendingInteractionForResolution
    (covers acceptInteraction and rejectInteraction, and through them the
    suggest-tasks and confirmation branches), answerQuestions, and submitItemVerdicts
    (inside its existing transaction, on tx).
  • Placed after the existing status !== "pending" check, so an already-resolved card
    keeps its more specific "Interaction has already been resolved" message and this
    change only affects the case that is currently silent.
  • withdrawInteraction and cancelQuestions are deliberately untouched: retiring a
    stale card on a closed issue is the cleanup those exist for.
  • The status is read from the database rather than taken from the caller's issue
    object, so the plugin host's accept/reject path gets the same guard, not just the
    HTTP routes.
  • skills/paperclip/references/api-reference.md: documents the 409 next to the
    existing issue_closed outcome, including that withdraw/cancel stay available.

Verification

npx vitest run server/src/services/issue-thread-interactions.test.ts
#   12 passed  (5 new)
npx vitest run server/src/__tests__/issue-thread-interaction-routes.test.ts \
               server/src/__tests__/issue-thread-interactions-service.test.ts \
               server/src/__tests__/issues-service.test.ts \
               server/src/__tests__/tool-gateway-service.test.ts
#   187 passed
npx vitest run server/src/__tests__/issue-thread-interactions-telemetry.test.ts
#   7 passed
npx tsc --noEmit -p server/tsconfig.json     # no diagnostics in the touched file

Red before the fix: with the service change reverted and the new tests in place,
the four refusal tests fail (4 failed | 8 passed) — they are measuring the guard,
not the harness.

New tests in server/src/services/issue-thread-interactions.test.ts:

  • accept on a done issue and on a cancelled issue → 409, and the card must still
    be pending afterwards (the silent failure is precisely a card that moves while
    nothing is woken, so "does not throw" alone would not catch a regression)
  • reject on a closed issue → 409, no card update
  • respond (answerQuestions) on a closed issue → 409, no card update
  • withdraw on a closed issue still succeeds — the administrative path stays open
  • an already-expired card on a closed issue still reports "Interaction has already
    been resolved", pinning the ordering of the two conflicts

The test double's select() grew table awareness for the issues table, opt-in via a
new issueRows argument, so every existing call-ordered expectation in that file is
unchanged.

Risks

Low, and deliberately narrow.

  • Behavioral shift: a resolution that previously returned 200 now returns 409. It
    only fires for a card that is pending on a done/cancelled issue — a state that
    post-feat(interactions): add interaction withdrawal and terminal-issue expiry #10251 issues do not reach through the close path, and whose 200 was never
    effective. No caller loses a working outcome; a silent no-op becomes a visible one.
  • One extra SELECT per resolution, primary-key lookup on issues, on a path that
    already runs several statements.
  • Fails open when the issue row is not found: the routes resolve the issue before
    calling in, so a missing row means an inconsistency, not a closed issue; refusing
    there would turn a read anomaly into a blocked answer.
  • Race window is unchanged, not widened: an issue closing between the guard and the
    card update still lands on the close hook's expiry, and the existing
    WHERE status = 'pending' guard on the update turns that into the same 409. The
    pre-check is aimed at rows that are already pending on an already closed issue.
  • No migration, no schema change, no change to any resolved-card result shape.

Model Used

Claude Opus 5 (claude-opus-5[1m], 1M context), extended thinking, with tool use
(repository search, local vitest/tsc runs against a throwaway worktree of this base).

Checklist

  • I have included a thinking path that traces from project context to this change
  • I have specified the model used (with version and capability details)
  • I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
  • I have searched GitHub for duplicate or related prior PRs and linked them above
  • I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
  • 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
  • I have run tests locally and they pass
  • I have added or updated tests where applicable
  • I have updated relevant documentation to reflect my changes
  • I have considered and documented any risks above
  • All Paperclip CI gates are green — 29 checks SUCCESS, 1 SKIPPED (Storybook visual regression; no UI files in this PR), 0 failures on 9ec5c99d2
  • Greptile is 5/5 with no open P2s, recommendations, or follow-ups — reviewed 9ec5c99d2, confidence 5/5, no inline comments raised
  • I will address all Greptile and reviewer comments before requesting merge

Note on the base: this branch is cut from 4eace88f6 rather than current master,
because the token available here has no workflow scope and pushing the intervening
.github/workflows/ commits to the fork is rejected. 4eace88f6 already contains
#10251, and the functions this PR touches are byte-identical between that commit and
master. A maintainer pressing "Update branch" will rebase it cleanly.

…osed

`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 (paperclipai#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>
@greptile-apps

greptile-apps Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR prevents pending interactions from being resolved after their issue has entered a terminal state, avoiding successful-looking answers that cannot resume work.

  • Adds a company-scoped issue-status check to accept, reject, question-response, and item-verdict resolution paths.
  • Preserves withdraw and cancel as cleanup operations for stale interactions.
  • Adds regression tests for closed-issue resolution and updates the API reference with the new 409 behavior.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete changed-code defect identified.

The new company-scoped terminal-status check covers all intended resolution entry points, preserves cleanup operations, and is supported by focused regression tests and synchronized API documentation.

Important Files Changed

Filename Overview
server/src/services/issue-thread-interactions.ts Adds the terminal-issue guard to every user-facing interaction-resolution path while preserving intentional administrative cleanup paths.
server/src/services/issue-thread-interactions.test.ts Extends the database test double and verifies refusal, non-mutation, error ordering, and permitted withdrawal behavior.
skills/paperclip/references/api-reference.md Documents the closed-issue 409 response and the continued availability of withdraw and cancel operations.

Reviews (1): Last reviewed commit: "fix(interactions): refuse to resolve a c..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant