From f6ab82d490cb314f021a0187f07c56096a70f24e Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 27 Jul 2026 19:04:46 -0500 Subject: [PATCH] feat(interactions): add interaction withdrawal and terminal-issue expiry (#10251) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents and boards coordinate through issue-thread interactions (request_confirmation, ask_user_questions, suggest_tasks, …) that wait as `pending` cards until someone resolves them > - Two lifecycle gaps existed: an interaction's creator could not take back a card it no longer stands behind, and interactions left `pending` on issues that reached a terminal status lingered forever as live-looking approval requests > - Stale pending cards mislead humans (they look actionable), distort attention/liveness signals, and in the worst case invite acting on a proposal whose issue is already closed or cancelled > - This pull request adds an explicit withdraw route for pending interactions and automatically expires pending interactions when their issue reaches a terminal status (including a catch-up sweep for issues closed before this change) > - The benefit is that interaction cards now faithfully reflect reality: only genuinely actionable requests stay pending, and creators can retract requests that events have overtaken ## Linked Issues or Issue Description Fixes #5787 Refs #7403 Related prior PRs found while searching for duplicates (all overlap partially; none combine both lifecycle paths or the route-level authorization used here): #6709 and #7312 (creator-withdraw attempts), #8169 (terminal expiry), #8081 and #5137 (generalized cancel/expire endpoints), #6094 (stale confirmation auto-resolve). Related merged context: #9568 (agent cancel for ask_user_questions), #10119 (tolerating legacy `withdrawn_by_creator` result outcomes — the reader side of the outcome this PR writes). ## What Changed - New route `POST /issues/:id/interactions/:interactionId/withdraw` that resolves a `pending` interaction to status `withdrawn` with a structured result (`outcome: "withdrawn"`, optional trimmed `reason`), stamps `resolvedBy*`/`resolvedAt`, touches the issue, logs activity, and emits resolved-interaction telemetry - Withdrawal authorization: board users, the interaction's creator agent, or the issue's current assignee agent (assignees additionally pass the standard issue-mutation gate); task-watchdog runs are explicitly rejected, and authorization-boundary plus low-trust control-plane checks apply - Withdrawing an already-resolved interaction returns `409`; unknown/cross-issue/cross-company interaction ids return `404` - New service method `expirePendingInteractionsForTerminalIssue`: when an issue transitions to a terminal status, all of its `pending` interactions are resolved to `expired` with `outcome: "issue_closed"`, guarded by a `status = 'pending'` predicate so concurrent resolutions are not overwritten - The same expiry runs as a catch-up when interactions are listed on an already-terminal issue, so cards stranded by issues closed before this change also get cleaned up; expired request_confirmations are logged with a distinguishing source - Shared package: new `withdrawIssueThreadInteractionSchema` validator, `WithdrawIssueThreadInteraction` type, and `withdrawn` / `issue_closed` result-outcome support for all interaction kinds (kind-aware result shapes for `ask_user_questions` and `request_item_verdicts`) - UI helper `ui/src/lib/issue-thread-interactions.ts` recognizes the new outcomes for card rendering - Docs: bundled skill API reference updated with the withdraw endpoint - Review follow-up: terminal expiry moved from the HTTP route hooks into `issueService.update`'s status-transition block, so direct service callers (tree control, recovery, pipelines, status cards) expire pending cards too; the list-endpoint catch-up remains for issues closed before this change - Review follow-up: withdrawing or issue-close-expiring a `request_confirmation` also settles its linked `tool_action_requests` row (withdraw -> `cancelled`, issue closed -> `expired`), so a parked tool call cannot stay approvable after its card is gone - Review follow-up: interaction cards render dedicated copy for the new outcomes ("Withdrawn" with the reason, "Expired when issue closed") instead of falling through to superseded-by-comment / stale-target variants; withdrawn plan reviews badge as "Withdrawn" rather than "Changes requested" ## Screenshots Card states rendered from a local ux-lab harness with mocked data ([full gallery](https://pages.paperclip.ing/pr-10251-interaction-withdrawal-cards/)): | Light | Dark | | --- | --- | | ![All card states, light](https://pages.paperclip.ing/pr-10251-interaction-withdrawal-cards/all-light.png) | ![All card states, dark](https://pages.paperclip.ing/pr-10251-interaction-withdrawal-cards/all-dark.png) | ## Verification - `pnpm --filter @paperclipai/shared build` — clean tsc - `cd server && npx vitest run src/__tests__/issue-thread-interaction-routes.test.ts` — 22 tests pass, including new coverage for: creator-agent withdraw success, non-creator/non-assignee agent 403, watchdog-run 403, double-withdraw 409, and board-user withdraw - `cd server && npx vitest run src/services/issue-thread-interactions.test.ts` — 4 tests pass, including terminal-issue expiry writing `issue_closed` results and leaving already-resolved interactions untouched - `cd ui && pnpm typecheck` — clean - `cd server && npx vitest run src/__tests__/issues-service.test.ts` — includes a new embedded-Postgres test proving a direct `issueService.update` terminal transition expires pending interactions and writes the activity-log entry - `cd ui && npx vitest run src/components/IssueThreadInteractionCard.test.tsx` — 32 tests, including new coverage for withdrawn / issue-closed confirmation and question cards - `cd server && npx tsc --noEmit` — matches the pre-existing repo error baseline exactly (no new errors) - Manual: `POST /issues/:id/interactions/:interactionId/withdraw` with `{"reason":"superseded"}` as the creator agent resolves the card to `withdrawn`; closing an issue with a pending confirmation flips it to `expired` with `outcome: "issue_closed"` ## Risks - Interactions on terminal issues now auto-expire (including retroactively via the list-time catch-up), so consumers that expected to resolve a pending interaction on a closed issue will get `409`; this is the intended semantics and matches how the attention feed already wants to treat dead cards - New result outcomes (`withdrawn`, `issue_closed`) are written to stored results; readers were already made tolerant of these outcome strings in #10119, so mixed-version reads are safe - No schema/migration changes; per-row conditional updates (`status = 'pending'`) avoid clobbering concurrent resolutions - Withdrawal is a new mutation surface, but it is strictly narrower than existing resolve paths (board, creator, or assignee only; watchdog runs blocked) ## Model Used Claude Fable 5 (`claude-fable-5`, Anthropic Mythos-class tier) with extended thinking and agentic tool use (Claude Code harness); commit authored in a Paperclip-managed engineering session. ## 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] 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 --- packages/shared/src/index.ts | 2 + packages/shared/src/types/issue.ts | 9 +- packages/shared/src/validators/index.ts | 2 + packages/shared/src/validators/issue.ts | 14 +- .../issue-thread-interaction-routes.test.ts | 70 +++++ .../issue-thread-interactions-service.test.ts | 22 ++ server/src/__tests__/issues-service.test.ts | 58 ++++ .../__tests__/tool-gateway-service.test.ts | 37 +++ server/src/routes/issues.ts | 99 ++++++- server/src/routes/openapi.ts | 9 + .../issue-thread-interactions.test.ts | 115 ++++++++ .../src/services/issue-thread-interactions.ts | 265 ++++++++++++++++-- server/src/services/issues.ts | 30 ++ server/src/services/tool-gateway.ts | 55 +++- skills/paperclip/SKILL.md | 3 +- skills/paperclip/references/api-reference.md | 4 + .../IssueThreadInteractionCard.test.tsx | 70 +++++ .../components/IssueThreadInteractionCard.tsx | 50 +++- ui/src/lib/issue-thread-interactions.ts | 5 + 19 files changed, 882 insertions(+), 37 deletions(-) diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 66c1227f41..1c0241ad4a 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -1615,6 +1615,7 @@ export { acceptIssueThreadInteractionSchema, rejectIssueThreadInteractionSchema, cancelIssueThreadInteractionSchema, + withdrawIssueThreadInteractionSchema, respondIssueThreadInteractionSchema, submitIssueThreadInteractionVerdictsSchema, linkIssueApprovalSchema, @@ -1677,6 +1678,7 @@ export { type AcceptIssueThreadInteraction, type RejectIssueThreadInteraction, type CancelIssueThreadInteraction, + type WithdrawIssueThreadInteraction, type RespondIssueThreadInteraction, type SubmitIssueThreadInteractionVerdicts, type LinkIssueApproval, diff --git a/packages/shared/src/types/issue.ts b/packages/shared/src/types/issue.ts index e4639f87f7..e1e5466ae0 100644 --- a/packages/shared/src/types/issue.ts +++ b/packages/shared/src/types/issue.ts @@ -996,6 +996,8 @@ export interface SuggestTasksResultCreatedTask { export interface SuggestTasksResult { version: 1; + outcome?: "withdrawn" | "issue_closed"; + reason?: string | null; createdTasks?: SuggestTasksResultCreatedTask[]; skippedClientKeys?: string[]; rejectionReason?: string | null; @@ -1032,6 +1034,8 @@ export interface AskUserQuestionsAnswer { export interface AskUserQuestionsResult { version: 1; + outcome?: "withdrawn" | "issue_closed"; + reason?: string | null; answers: AskUserQuestionsAnswer[]; cancelled?: true; cancellationReason?: string | null; @@ -1166,7 +1170,7 @@ export interface RequestItemVerdictsPayload { export interface RequestConfirmationResult { version: 1; - outcome: "accepted" | "rejected" | "superseded_by_comment" | "stale_target"; + outcome: "accepted" | "rejected" | "superseded_by_comment" | "stale_target" | "withdrawn" | "issue_closed"; reason?: string | null; commentId?: string | null; staleTarget?: RequestConfirmationTarget | null; @@ -1198,7 +1202,8 @@ export interface RequestItemVerdictsResultItem { export interface RequestItemVerdictsResult { version: 1; - outcome: "resolved" | "superseded_by_comment" | "stale_target" | "cancelled"; + outcome: "resolved" | "superseded_by_comment" | "stale_target" | "cancelled" | "withdrawn" | "issue_closed"; + reason?: string | null; complete: boolean; items: RequestItemVerdictsResultItem[]; commentId?: string | null; diff --git a/packages/shared/src/validators/index.ts b/packages/shared/src/validators/index.ts index 69eac14bdf..d9c0195094 100644 --- a/packages/shared/src/validators/index.ts +++ b/packages/shared/src/validators/index.ts @@ -419,6 +419,7 @@ export { acceptIssueThreadInteractionSchema, rejectIssueThreadInteractionSchema, cancelIssueThreadInteractionSchema, + withdrawIssueThreadInteractionSchema, respondIssueThreadInteractionSchema, submitIssueThreadInteractionVerdictsSchema, linkIssueApprovalSchema, @@ -442,6 +443,7 @@ export { type AcceptIssueThreadInteraction, type RejectIssueThreadInteraction, type CancelIssueThreadInteraction, + type WithdrawIssueThreadInteraction, type RespondIssueThreadInteraction, type SubmitIssueThreadInteractionVerdicts, type LinkIssueApproval, diff --git a/packages/shared/src/validators/issue.ts b/packages/shared/src/validators/issue.ts index af6b7efcff..ed935a8b38 100644 --- a/packages/shared/src/validators/issue.ts +++ b/packages/shared/src/validators/issue.ts @@ -718,6 +718,8 @@ export const suggestTasksResultCreatedTaskSchema = z.object({ export const suggestTasksResultSchema = z.object({ version: z.literal(1), + outcome: z.enum(["withdrawn", "issue_closed"]).optional(), + reason: z.string().trim().max(4000).nullable().optional(), createdTasks: z.array(suggestTasksResultCreatedTaskSchema).max(50).optional(), skippedClientKeys: z.array(z.string().trim().min(1).max(120)).max(50).optional(), rejectionReason: z.string().trim().max(4000).nullable().optional(), @@ -778,6 +780,8 @@ export const askUserQuestionsAnswerSchema = z.object({ export const askUserQuestionsResultSchema = z.object({ version: z.literal(1), + outcome: z.enum(["withdrawn", "issue_closed"]).optional(), + reason: z.string().trim().max(4000).nullable().optional(), answers: z.array(askUserQuestionsAnswerSchema).max(20), cancelled: z.literal(true).optional(), cancellationReason: z.string().trim().max(4000).nullable().optional(), @@ -975,7 +979,7 @@ export const requestConfirmationToolActionResultSchema = z.object({ export const requestConfirmationResultSchema = z.object({ version: z.literal(1), - outcome: z.enum(["accepted", "rejected", "superseded_by_comment", "stale_target"]), + outcome: z.enum(["accepted", "rejected", "superseded_by_comment", "stale_target", "withdrawn", "issue_closed"]), reason: z.string().trim().max(4000).nullable().optional(), commentId: z.string().uuid().nullable().optional(), staleTarget: requestConfirmationTargetSchema.nullable().optional(), @@ -1097,7 +1101,8 @@ export const requestItemVerdictsResultItemSchema = z.object({ export const requestItemVerdictsResultSchema = z.object({ version: z.literal(1), - outcome: z.enum(["resolved", "superseded_by_comment", "stale_target", "cancelled"]), + outcome: z.enum(["resolved", "superseded_by_comment", "stale_target", "cancelled", "withdrawn", "issue_closed"]), + reason: z.string().trim().max(4000).nullable().optional(), complete: z.boolean(), items: z.array(requestItemVerdictsResultItemSchema) .max(REQUEST_ITEM_VERDICTS_ITEM_LIMIT), @@ -1216,6 +1221,11 @@ export const cancelIssueThreadInteractionSchema = z.object({ }); export type CancelIssueThreadInteraction = z.infer; +export const withdrawIssueThreadInteractionSchema = z.object({ + reason: z.string().trim().max(4000).optional(), +}); +export type WithdrawIssueThreadInteraction = z.infer; + export const respondIssueThreadInteractionSchema = z.object({ answers: z.array(askUserQuestionsAnswerSchema).max(20), summaryMarkdown: multilineTextSchema.pipe(z.string().max(20000)).nullable().optional(), diff --git a/server/src/__tests__/issue-thread-interaction-routes.test.ts b/server/src/__tests__/issue-thread-interaction-routes.test.ts index 70577f6e72..9476303e82 100644 --- a/server/src/__tests__/issue-thread-interaction-routes.test.ts +++ b/server/src/__tests__/issue-thread-interaction-routes.test.ts @@ -11,15 +11,18 @@ const mockIssueService = vi.hoisted(() => ({ const mockInteractionService = vi.hoisted(() => ({ listForIssue: vi.fn(), + getForIssue: vi.fn(), create: vi.fn(), acceptInteraction: vi.fn(), acceptSuggestedTasks: vi.fn(), rejectInteraction: vi.fn(), rejectSuggestedTasks: vi.fn(), expireRequestConfirmationsSupersededByHistoricalComments: vi.fn(), + expirePendingInteractionsForTerminalIssue: vi.fn(), answerQuestions: vi.fn(), submitItemVerdicts: vi.fn(), cancelQuestions: vi.fn(), + withdrawInteraction: vi.fn(), })); const mockHeartbeatService = vi.hoisted(() => ({ @@ -186,6 +189,24 @@ describe.sequential("issue thread interaction routes", () => { mockIssueService.getById.mockResolvedValue(createIssue()); mockInteractionService.listForIssue.mockResolvedValue([]); mockInteractionService.expireRequestConfirmationsSupersededByHistoricalComments.mockResolvedValue([]); + mockInteractionService.expirePendingInteractionsForTerminalIssue.mockResolvedValue([]); + mockInteractionService.getForIssue.mockResolvedValue({ + id: "interaction-withdraw", + createdByAgentId: CREATED_AGENT_ID, + continuationPolicy: "wake_assignee", + status: "pending", + }); + mockInteractionService.withdrawInteraction.mockResolvedValue({ + id: "interaction-withdraw", + companyId: "company-1", + issueId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + kind: "request_confirmation", + createdByAgentId: CREATED_AGENT_ID, + status: "cancelled", + continuationPolicy: "wake_assignee", + payload: { version: 1, prompt: "Proceed?" }, + result: { version: 1, outcome: "withdrawn", reason: "Replanning" }, + }); mockInteractionService.create.mockResolvedValue({ id: "interaction-1", companyId: "company-1", @@ -574,6 +595,55 @@ describe.sequential("issue thread interaction routes", () => { ); }); + it("allows a board user to withdraw and wakes the assignee", async () => { + const app = await createApp(); + const res = await request(app) + .post("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/interactions/interaction-withdraw/withdraw") + .send({ reason: "Replanning" }); + + expect(res.status).toBe(200); + expect(mockInteractionService.withdrawInteraction).toHaveBeenCalledWith( + expect.objectContaining({ id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa" }), + "interaction-withdraw", + { reason: "Replanning" }, + expect.objectContaining({ userId: "local-board" }), + ); + expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith(ASSIGNEE_AGENT_ID, expect.objectContaining({ + payload: expect.objectContaining({ interactionStatus: "cancelled" }), + })); + expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ + action: "issue.thread_interaction_withdrawn", + })); + }); + + it("allows the creator agent to withdraw and wakes a different assignee", async () => { + const app = await createApp({ type: "agent", agentId: CREATED_AGENT_ID, companyId: "company-1", runId: "run-1" }); + const res = await request(app) + .post("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/interactions/interaction-withdraw/withdraw") + .send({}); + expect(res.status).toBe(200); + expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith(ASSIGNEE_AGENT_ID, expect.anything()); + }); + + it("allows the assignee agent to withdraw without waking itself", async () => { + mockIssueService.getById.mockResolvedValueOnce(createIssue({ status: "todo" })); + const app = await createApp({ type: "agent", agentId: ASSIGNEE_AGENT_ID, companyId: "company-1", runId: "run-2" }); + const res = await request(app) + .post("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/interactions/interaction-withdraw/withdraw") + .send({}); + expect(res.status).toBe(200); + expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); + }); + + it("rejects withdrawal by an unrelated agent", async () => { + const app = await createApp({ type: "agent", agentId: "33333333-3333-4333-8333-333333333333", companyId: "company-1", runId: "run-3" }); + const res = await request(app) + .post("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/interactions/interaction-withdraw/withdraw") + .send({}); + expect(res.status).toBe(403); + expect(mockInteractionService.withdrawInteraction).not.toHaveBeenCalled(); + }); + it("cancels question interactions and emits a continuation wake", async () => { const app = await createApp(); diff --git a/server/src/__tests__/issue-thread-interactions-service.test.ts b/server/src/__tests__/issue-thread-interactions-service.test.ts index f44b968cf4..096dd95a11 100644 --- a/server/src/__tests__/issue-thread-interactions-service.test.ts +++ b/server/src/__tests__/issue-thread-interactions-service.test.ts @@ -892,6 +892,28 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { expect(rows[0]?.idempotencyKey).toBe("run-1:questionnaire"); }); + it("refuses to create an interaction on a closed issue", async () => { + const { companyId, issueId } = await seedConfirmationIssue("Closed issue create guard"); + await db.update(issues).set({ status: "done" }).where(eq(issues.id, issueId)); + + await expect(interactionsSvc.create({ + id: issueId, + companyId, + }, { + kind: "request_confirmation", + continuationPolicy: "wake_assignee", + payload: { version: 1, prompt: "Approve after close?" }, + }, { + userId: "local-board", + })).rejects.toMatchObject({ status: 409 }); + + const rows = await db + .select() + .from(issueThreadInteractions) + .where(eq(issueThreadInteractions.issueId, issueId)); + expect(rows).toHaveLength(0); + }); + it("accepts request_confirmation interactions without creating child issues", async () => { const companyId = randomUUID(); const goalId = randomUUID(); diff --git a/server/src/__tests__/issues-service.test.ts b/server/src/__tests__/issues-service.test.ts index f238c96493..677d8a8d18 100644 --- a/server/src/__tests__/issues-service.test.ts +++ b/server/src/__tests__/issues-service.test.ts @@ -464,6 +464,64 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => { }); }); + it("expires pending thread interactions on any service-level terminal transition", async () => { + const companyId = await seedAssignableAgentCompany(); + const issue = await svc.create(companyId, { + title: "Close me with a pending card", + description: null, + status: "todo", + priority: "medium", + }); + const interactionId = randomUUID(); + await db.insert(issueThreadInteractions).values({ + id: interactionId, + companyId, + issueId: issue.id, + kind: "ask_user_questions", + status: "pending", + continuationPolicy: "wake_assignee", + payload: { + version: 1, + questions: [{ + id: "scope", + prompt: "Pick one", + selectionMode: "single", + options: [{ id: "a", label: "A" }], + }], + } as never, + }); + + // Direct service callers (tree control, recovery, pipelines, status cards) + // never pass through the HTTP routes, so the expiry must fire here. + const updated = await svc.update(issue.id, { status: "cancelled", actorUserId: "local-board" }); + expect(updated?.status).toBe("cancelled"); + + const interaction = await db + .select() + .from(issueThreadInteractions) + .where(eq(issueThreadInteractions.id, interactionId)) + .then((rows) => rows[0] ?? null); + expect(interaction).toMatchObject({ + status: "expired", + resolvedByUserId: "local-board", + }); + expect(interaction?.result).toMatchObject({ version: 1, outcome: "issue_closed" }); + expect(interaction?.resolvedAt).not.toBeNull(); + + const logged = await db + .select() + .from(activityLog) + .where(eq(activityLog.action, "issue.thread_interaction_expired")); + expect(logged).toHaveLength(1); + // details.source is dot-separated and trips the JWT-shaped redaction + // heuristic in sanitizeRecord, so assert the identifying fields instead. + expect(logged[0]?.details).toMatchObject({ + interactionId, + interactionKind: "ask_user_questions", + interactionStatus: "expired", + }); + }); + it("rejects moving an existing terminated assignment into progress without clearing it", async () => { const companyId = await seedAssignableAgentCompany(); const assigneeAgentId = randomUUID(); diff --git a/server/src/__tests__/tool-gateway-service.test.ts b/server/src/__tests__/tool-gateway-service.test.ts index 2ce5d2f126..01d953032a 100644 --- a/server/src/__tests__/tool-gateway-service.test.ts +++ b/server/src/__tests__/tool-gateway-service.test.ts @@ -437,6 +437,43 @@ describeEmbeddedPostgres("tool gateway service", () => { expect(executionEvents).toHaveLength(1); }); + it("refuses to execute an approved action after its issue closes", async () => { + const { company, agent, issue, run } = await createRunFixture(db); + await db.insert(toolPolicies).values({ + companyId: company.id, + name: "Review note writes", + policyType: "require_approval", + selectors: { toolName: "mcp-remote-fixture:update_note" }, + }); + const gateway = createTestToolGatewayService(db); + const session = await gateway.createSession({ companyId: company.id, agentId: agent.id, runId: run.id }); + const parameters = { noteId: "n1", body: "post-close body" }; + + await expect(gateway.executeTool({ + sessionToken: session.token, + tool: "mcp-remote-fixture:update_note", + parameters, + })).rejects.toMatchObject({ reasonCode: "approval_required" }); + const [actionRequest] = await db.select().from(toolActionRequests); + const now = new Date(); + await db.update(toolActionRequests).set({ status: "approved", decidedAt: now, resolvedAt: now }).where(eq(toolActionRequests.id, actionRequest.id)); + await db.update(issues).set({ status: "done" }).where(eq(issues.id, issue.id)); + + await expect(gateway.executeTool({ + sessionToken: session.token, + tool: "mcp-remote-fixture:update_note", + parameters, + })).rejects.toMatchObject({ reasonCode: "action_issue_closed" }); + + const [settled] = await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, actionRequest.id)); + expect(settled?.status).toBe("expired"); + const executionEvents = await db.select().from(toolCallEvents).where(and( + eq(toolCallEvents.actionRequestId, actionRequest.id), + eq(toolCallEvents.reasonCode, "approved_action_executed"), + )); + expect(executionEvents).toHaveLength(0); + }); + it("keeps pre-execute-on-approve approved requests inert", async () => { const { company, agent, run } = await createRunFixture(db); await db.insert(toolPolicies).values({ diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 6af7134cda..adaaa35ace 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -31,6 +31,7 @@ import { acceptIssueThreadInteractionSchema, attachmentArtifactWorkProductMetadataSchema, cancelIssueThreadInteractionSchema, + withdrawIssueThreadInteractionSchema, companySearchExtractQuerySchema, companySearchQuerySchema, createIssueAttachmentMetadataSchema, @@ -3733,6 +3734,41 @@ export function issueRoutes( return true; } + async function assertIssueThreadInteractionWithdrawalAllowed( + req: Request, + res: Response, + issue: Parameters[2], + interaction: { createdByAgentId?: string | null }, + ) { + if (req.actor.type !== "agent") { + assertBoard(req); + return true; + } + const actorAgentId = req.actor.agentId; + if (!actorAgentId || !requireAgentRunId(req, res)) return false; + + const watchdogScope = await resolveTaskWatchdogMutationScope(db, req.actor); + if (watchdogScope.kind !== "none") { + res.status(403).json({ error: "Task-watchdog runs cannot withdraw issue-thread interactions" }); + return false; + } + const boundaryDecision = await decideIssueAccess(req, issue, "issue:mutate"); + if (!boundaryDecision.allowed) { + res.status(403).json({ error: "Issue is outside this actor's authorization boundary" }); + return false; + } + if (await assertLowTrustControlPlaneDenied(req, res, issue.companyId, issue)) return false; + + const isCreator = interaction.createdByAgentId === actorAgentId; + const isAssignee = issue.assigneeAgentId === actorAgentId; + if (!isCreator && !isAssignee) { + res.status(403).json({ error: "Only the interaction creator, current issue assignee, or a board user may withdraw it" }); + return false; + } + if (isAssignee) return assertAgentIssueMutationAllowed(req, res, issue); + return true; + } + async function assertTaskWatchdogCreateIssueAllowed( req: Request, res: Response, @@ -9211,13 +9247,23 @@ export function issueRoutes( if (!(await assertIssueReadAllowed(req, res, issue))) return; const actor = getActorInfo(req); const interactionSvc = issueThreadInteractionService(db); - const expiredInteractions = await interactionSvc.expireRequestConfirmationsSupersededByHistoricalComments(issue); + const supersededInteractions = await interactionSvc.expireRequestConfirmationsSupersededByHistoricalComments(issue); await logExpiredRequestConfirmations({ issue, - interactions: expiredInteractions, + interactions: supersededInteractions, actor, source: "issue.interactions.catchup_superseded_by_comment", }); + const closedIssueInteractions = await interactionSvc.expirePendingInteractionsForTerminalIssue(issue, { + agentId: actor.agentId, + userId: actor.actorType === "user" ? actor.actorId : null, + }); + await logExpiredRequestConfirmations({ + issue, + interactions: closedIssueInteractions, + actor, + source: "issue.interactions.catchup_issue_closed", + }); const interactions = await interactionSvc.listForIssue(id); res.json(interactions); @@ -9586,6 +9632,55 @@ export function issueRoutes( }, ); + router.post( + "/issues/:id/interactions/:interactionId/withdraw", + validate(withdrawIssueThreadInteractionSchema), + async (req, res) => { + const id = req.params.id as string; + const interactionId = req.params.interactionId as string; + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; + + const interactionSvc = issueThreadInteractionService(db); + const current = await interactionSvc.getForIssue(issue, interactionId); + if (!(await assertIssueThreadInteractionWithdrawalAllowed(req, res, issue, current))) return; + + const actor = getActorInfo(req); + const interaction = await interactionSvc.withdrawInteraction(issue, interactionId, req.body, { + agentId: actor.agentId, + userId: actor.actorType === "user" ? actor.actorId : null, + }); + await logActivity(db, { + companyId: issue.companyId, + actorType: actor.actorType, + actorId: actor.actorId, + agentId: actor.agentId, + runId: actor.runId, + agentApiKeyId: actor.agentApiKeyId, + action: "issue.thread_interaction_withdrawn", + entityType: "issue", + entityId: issue.id, + details: { + interactionId: interaction.id, + interactionKind: interaction.kind, + interactionStatus: interaction.status, + reason: interaction.result && "reason" in interaction.result ? interaction.result.reason ?? null : null, + }, + }); + + if (actor.agentId !== issue.assigneeAgentId) { + queueResolvedInteractionContinuationWakeup({ + heartbeat, + issue, + interaction, + actor, + source: "issue.interaction.withdraw", + }); + } + res.json(interaction); + }, + ); + router.post( "/issues/:id/interactions/:interactionId/cancel", validate(cancelIssueThreadInteractionSchema), diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index 9ec800e9f8..6db43c6761 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -164,6 +164,7 @@ import { createAcceptedPlanDecompositionSchema, resolveIssueRecoveryActionSchema, cancelIssueThreadInteractionSchema, + withdrawIssueThreadInteractionSchema, // Secret provider configs and remote import createSecretProviderConfigSchema, updateSecretProviderConfigSchema, @@ -5909,6 +5910,14 @@ registerCurrentRoute({ body: cancelIssueThreadInteractionSchema, }); +registerCurrentRoute({ + method: "post", + path: "/api/issues/{id}/interactions/{interactionId}/withdraw", + tags: ["issues"], + summary: "Withdraw a pending issue thread interaction", + body: withdrawIssueThreadInteractionSchema, +}); + for (const route of [ ["get", "/api/routines/{id}/revisions", "List routine revisions"], ["post", "/api/routines/{id}/revisions/{revisionId}/restore", "Restore a routine revision"], diff --git a/server/src/services/issue-thread-interactions.test.ts b/server/src/services/issue-thread-interactions.test.ts index d2cd4205ce..27d132867b 100644 --- a/server/src/services/issue-thread-interactions.test.ts +++ b/server/src/services/issue-thread-interactions.test.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; +import { getTableName } from "drizzle-orm"; const mockCreateChild = vi.fn(); @@ -33,6 +34,7 @@ function createFakeDb(args: { let interactionRow = { ...args.interactionRow }; const issueTouches: Array> = []; const interactionUpdates: Array> = []; + const toolActionRequestUpdates: Array> = []; let selectCallCount = 0; const db: any = { @@ -44,6 +46,10 @@ function createFakeDb(args: { set(values: Record) { return { where() { + if (getTableName(table as never) === "tool_action_requests") { + toolActionRequestUpdates.push(values); + return Promise.resolve(undefined); + } if ("status" in values || "result" in values || "resolvedAt" in values) { interactionUpdates.push(values); interactionRow = { ...interactionRow, ...values }; @@ -69,6 +75,7 @@ function createFakeDb(args: { getInteractionRow: () => interactionRow, issueTouches, interactionUpdates, + toolActionRequestUpdates, }; } @@ -212,4 +219,112 @@ describe("issueThreadInteractionService", () => { expect(state.interactionUpdates).toHaveLength(1); expect(state.issueTouches).toHaveLength(1); }); + + it("withdraws a pending interaction with attribution and rejects repeats", async () => { + const { issueThreadInteractionService } = await import("./issue-thread-interactions.js"); + const interactionRow = { + id: "interaction-withdraw", companyId: "company-1", issueId: "11111111-1111-4111-8111-111111111111", + kind: "request_confirmation", status: "pending", continuationPolicy: "wake_assignee", + sourceCommentId: null, sourceRunId: null, title: null, summary: null, + createdByAgentId: "agent-1", createdByUserId: null, resolvedByAgentId: null, resolvedByUserId: null, + payload: { version: 1, prompt: "Proceed?" }, result: null, resolvedAt: null, + createdAt: new Date("2026-07-25T10:00:00.000Z"), updatedAt: new Date("2026-07-25T10:00:00.000Z"), + }; + const state = createFakeDb({ interactionRow }); + const svc = issueThreadInteractionService(state.db as never); + const withdrawn = await svc.withdrawInteraction({ id: interactionRow.issueId, companyId: "company-1" }, interactionRow.id, { reason: "Replanning" }, { agentId: "agent-1" }); + expect(withdrawn.status).toBe("cancelled"); + expect(withdrawn.result).toEqual({ version: 1, outcome: "withdrawn", reason: "Replanning" }); + expect(withdrawn.resolvedByAgentId).toBe("agent-1"); + expect(state.toolActionRequestUpdates).toHaveLength(1); + expect(state.toolActionRequestUpdates[0]).toMatchObject({ status: "cancelled", resolvedByAgentId: "agent-1" }); + const resolvedState = createFakeDb({ interactionRow: { ...interactionRow, status: "accepted" } }); + const resolvedSvc = issueThreadInteractionService(resolvedState.db as never); + await expect(resolvedSvc.withdrawInteraction( + { id: interactionRow.issueId, companyId: "company-1" }, + interactionRow.id, + {}, + { agentId: "agent-1" }, + )).rejects.toMatchObject({ status: 409 }); + }); + + it("refuses withdrawal when the linked tool action is already executing", async () => { + const { issueThreadInteractionService } = await import("./issue-thread-interactions.js"); + const interactionRow = { + id: "interaction-executing", companyId: "company-1", issueId: "11111111-1111-4111-8111-111111111111", + kind: "request_confirmation", status: "pending", continuationPolicy: "wake_assignee", + sourceCommentId: null, sourceRunId: null, title: null, summary: null, + createdByAgentId: "agent-1", createdByUserId: null, resolvedByAgentId: null, resolvedByUserId: null, + payload: { version: 1, prompt: "Proceed?" }, result: null, resolvedAt: null, + createdAt: new Date("2026-07-25T10:00:00.000Z"), updatedAt: new Date("2026-07-25T10:00:00.000Z"), + }; + const state = createFakeDb({ interactionRow, parentRows: [{ id: "action-request-1" }] }); + const svc = issueThreadInteractionService(state.db as never); + await expect(svc.withdrawInteraction( + { id: interactionRow.issueId, companyId: "company-1" }, + interactionRow.id, + {}, + { agentId: "agent-1" }, + )).rejects.toMatchObject({ status: 409 }); + expect(state.interactionUpdates).toHaveLength(0); + }); + + it("expires pending interactions when the issue is terminal", async () => { + const { issueThreadInteractionService } = await import("./issue-thread-interactions.js"); + const interactionRow = { + id: "interaction-close", companyId: "company-1", issueId: "11111111-1111-4111-8111-111111111111", + kind: "ask_user_questions", status: "pending", continuationPolicy: "wake_assignee", + sourceCommentId: null, sourceRunId: null, title: null, summary: null, + createdByAgentId: "agent-1", createdByUserId: null, resolvedByAgentId: null, resolvedByUserId: null, + payload: { version: 1, questions: [{ id: "q", prompt: "Q?", selectionMode: "single", options: [{ id: "a", label: "A" }] }] }, + result: null, resolvedAt: null, createdAt: new Date("2026-07-25T10:00:00.000Z"), updatedAt: new Date("2026-07-25T10:00:00.000Z"), + }; + const state = createFakeDb({ interactionRow }); + const svc = issueThreadInteractionService(state.db as never); + const expired = await svc.expirePendingInteractionsForTerminalIssue({ id: interactionRow.issueId, companyId: "company-1", status: "done" }); + expect(expired).toHaveLength(1); + expect(expired[0]?.status).toBe("expired"); + expect(expired[0]?.result).toMatchObject({ version: 1, outcome: "issue_closed", answers: [] }); + expect(state.toolActionRequestUpdates).toHaveLength(0); + }); + + it("expires the linked tool action request when a terminal issue closes a confirmation card", async () => { + const { issueThreadInteractionService } = await import("./issue-thread-interactions.js"); + const interactionRow = { + id: "interaction-tool", companyId: "company-1", issueId: "11111111-1111-4111-8111-111111111111", + kind: "request_confirmation", status: "pending", continuationPolicy: "wake_assignee", + sourceCommentId: null, sourceRunId: null, title: null, summary: null, + createdByAgentId: "agent-1", createdByUserId: null, resolvedByAgentId: null, resolvedByUserId: null, + payload: { + version: 1, + prompt: "Run the parked tool call?", + toolAction: { + version: 1, + actionRequestId: "33333333-3333-4333-8333-333333333333", + invocationId: "44444444-4444-4444-8444-444444444444", + toolName: "deploy", + toolDisplayName: "Deploy", + connectionId: null, + applicationId: null, + appDisplayName: null, + risk: "write", + previewMarkdown: "Deploy the current build.", + argumentsSummaryJson: "{}", + argumentsHash: "hash-1", + expiresAt: "2026-07-25T11:00:00.000Z", + }, + }, + result: null, resolvedAt: null, createdAt: new Date("2026-07-25T10:00:00.000Z"), updatedAt: new Date("2026-07-25T10:00:00.000Z"), + }; + const state = createFakeDb({ interactionRow }); + const svc = issueThreadInteractionService(state.db as never); + const expired = await svc.expirePendingInteractionsForTerminalIssue( + { id: interactionRow.issueId, companyId: "company-1", status: "cancelled" }, + { userId: "local-board" }, + ); + expect(expired).toHaveLength(1); + expect(expired[0]?.result).toMatchObject({ version: 1, outcome: "issue_closed" }); + expect(state.toolActionRequestUpdates).toHaveLength(1); + expect(state.toolActionRequestUpdates[0]).toMatchObject({ status: "expired", resolvedByUserId: "local-board" }); + }); }); diff --git a/server/src/services/issue-thread-interactions.ts b/server/src/services/issue-thread-interactions.ts index 3d3ef7fd7b..5c7820b34f 100644 --- a/server/src/services/issue-thread-interactions.ts +++ b/server/src/services/issue-thread-interactions.ts @@ -9,6 +9,7 @@ import { issueDocuments, issueThreadInteractions, issues, + toolActionRequests, } from "@paperclipai/db"; import { trackInteractionResolved } from "@paperclipai/shared/telemetry"; import type { @@ -29,6 +30,7 @@ import type { SuggestTasksInteraction, SuggestTasksResultCreatedTask, SubmitIssueThreadInteractionVerdicts, + WithdrawIssueThreadInteraction, } from "@paperclipai/shared"; import { acceptIssueThreadInteractionSchema, @@ -46,6 +48,7 @@ import { suggestTasksPayloadSchema, suggestTasksResultSchema, submitIssueThreadInteractionVerdictsSchema, + withdrawIssueThreadInteractionSchema, } from "@paperclipai/shared"; import { z } from "zod"; import { conflict, notFound, unprocessable } from "../errors.js"; @@ -345,6 +348,67 @@ function buildStaleTargetResult( } as const; } +function buildAdministrativeOutcomeResult( + row: IssueThreadInteractionRow, + outcome: "withdrawn" | "issue_closed", + reason: string | null = null, +) { + if (row.kind === "ask_user_questions") { + return { version: 1, outcome, reason, answers: [], summaryMarkdown: null } as const; + } + if (row.kind === "request_item_verdicts") { + const interaction = hydrateInteraction(row) as RequestItemVerdictsInteraction; + return { + version: 1, + outcome, + reason, + complete: false, + items: interaction.result?.items ?? [], + } satisfies RequestItemVerdictsResult; + } + return { version: 1, outcome, reason } as const; +} + +// Rollback sentinel: the interaction was resolved by another actor between the +// pending-rows read and the conditional update, so the enclosing transaction's +// tool-action revocation must be undone. +class InteractionResolvedConcurrentlyError extends Error { + constructor() { + super("Interaction was resolved concurrently"); + } +} + +// A request_confirmation card can govern a parked tool call via a linked +// tool_action_requests row. Administrative resolutions (withdraw, terminal-issue +// expiry) must settle that row too, or the parked call stays approvable under +// its own one-hour lifecycle after its card is gone. +async function resolveLinkedToolActionRequests( + db: Pick, + interaction: Pick, + outcome: { + status: "expired" | "cancelled"; + fromStatuses: Array<"pending" | "approved">; + actor: InteractionActor; + now: Date; + }, +) { + if (interaction.kind !== "request_confirmation") return; + await db + .update(toolActionRequests) + .set({ + status: outcome.status, + resolvedByAgentId: outcome.actor.agentId ?? null, + resolvedByUserId: outcome.actor.userId ?? null, + resolvedAt: outcome.now, + updatedAt: outcome.now, + }) + .where(and( + eq(toolActionRequests.companyId, interaction.companyId), + eq(toolActionRequests.interactionId, interaction.id), + inArray(toolActionRequests.status, outcome.fromStatuses), + )); +} + function resolveActorKind(interaction: Pick) { if (interaction.resolvedByAgentId) return "agent"; if (interaction.resolvedByUserId) return "user"; @@ -900,6 +964,18 @@ export function issueThreadInteractionService(db: Db) { .then((rows) => rows[0] ?? null); } + async function getForIssue(issue: { id: string; companyId: string }, interactionId: string) { + const current = await db + .select() + .from(issueThreadInteractions) + .where(eq(issueThreadInteractions.id, interactionId)) + .then((rows) => rows[0] ?? null); + if (!current || current.companyId !== issue.companyId || current.issueId !== issue.id) { + throw notFound("Interaction not found"); + } + return hydrateInteraction(current); + } + async function assertIssueWorkspaceFinalizedForAccept(args: { db: Pick; issue: { id: string; companyId: string }; @@ -1110,6 +1186,7 @@ export function issueThreadInteractionService(db: Db) { } return { + getForIssue, listForIssue: async (issueId: string) => { const rows = await db .select() @@ -1194,24 +1271,42 @@ export function issueThreadInteractionService(db: Db) { let created: IssueThreadInteractionRow; try { - [created] = await db - .insert(issueThreadInteractions) - .values({ - companyId: issue.companyId, - issueId: issue.id, - kind: data.kind, - status: "pending", - continuationPolicy: data.continuationPolicy, - idempotencyKey: data.idempotencyKey ?? null, - sourceCommentId: data.sourceCommentId ?? null, - sourceRunId: data.sourceRunId ?? null, - title: data.title ?? null, - summary: data.summary ?? null, - createdByAgentId: actor.agentId ?? null, - createdByUserId: actor.userId ?? null, - payload: data.payload, - }) - .returning(); + // A terminal issue must not regain pending actionable cards. FOR SHARE + // on the issue row serializes this insert against the terminal status + // transition's row lock: either the close committed first and this + // read rejects the create, or the insert commits before the close + // proceeds and the close's expiry sweep collects the new row. + // Idempotent reuse above stays allowed so retries of a pre-close + // create keep returning the (by now expired) original. + created = await db.transaction(async (tx) => { + const [issueRow] = await tx + .select({ status: issues.status }) + .from(issues) + .where(and(eq(issues.id, issue.id), eq(issues.companyId, issue.companyId))) + .for("share"); + if (!issueRow || isTerminalIssueStatus(issueRow.status)) { + throw conflict("Cannot create an interaction on a closed issue"); + } + const [row] = await tx + .insert(issueThreadInteractions) + .values({ + companyId: issue.companyId, + issueId: issue.id, + kind: data.kind, + status: "pending", + continuationPolicy: data.continuationPolicy, + idempotencyKey: data.idempotencyKey ?? null, + sourceCommentId: data.sourceCommentId ?? null, + sourceRunId: data.sourceRunId ?? null, + title: data.title ?? null, + summary: data.summary ?? null, + createdByAgentId: actor.agentId ?? null, + createdByUserId: actor.userId ?? null, + payload: data.payload, + }) + .returning(); + return row; + }); } catch (error) { if (!data.idempotencyKey || !isIssueThreadInteractionIdempotencyConflict(error)) { throw error; @@ -1878,6 +1973,140 @@ export function issueThreadInteractionService(db: Db) { return expired; }, + expirePendingInteractionsForTerminalIssue: async ( + issue: { id: string; companyId: string; status: string }, + actor: InteractionActor = {}, + ) => { + if (!isTerminalIssueStatus(issue.status)) return []; + const rows = await db + .select() + .from(issueThreadInteractions) + .where(and( + eq(issueThreadInteractions.companyId, issue.companyId), + eq(issueThreadInteractions.issueId, issue.id), + eq(issueThreadInteractions.status, "pending"), + )); + if (rows.length === 0) return []; + + const now = new Date(); + const expired: IssueThreadInteraction[] = []; + for (const row of rows) { + // Same ordering as withdrawal: revoke the linked tool action before + // resolving the card, inside one transaction. A concurrent gateway + // claim (approved -> executing) blocks on the revocation's row lock + // and then aborts; if the card was concurrently resolved instead, the + // no-row update below rolls the revocation back. A claim that already + // committed is in flight and cannot be recalled — the card still + // expires and the execution result lands on it via the gateway's + // lifecycle reflection. + const updated = await db.transaction(async (tx) => { + await resolveLinkedToolActionRequests(tx, row, { + status: "expired", + fromStatuses: ["pending", "approved"], + actor, + now, + }); + const [resolved] = await tx + .update(issueThreadInteractions) + .set({ + status: "expired", + result: buildAdministrativeOutcomeResult(row, "issue_closed"), + resolvedByAgentId: actor.agentId ?? null, + resolvedByUserId: actor.userId ?? null, + resolvedAt: now, + updatedAt: now, + }) + .where(and( + eq(issueThreadInteractions.id, row.id), + eq(issueThreadInteractions.status, "pending"), + )) + .returning(); + if (!resolved) throw new InteractionResolvedConcurrentlyError(); + return resolved; + }).catch((err: unknown) => { + if (err instanceof InteractionResolvedConcurrentlyError) return null; + throw err; + }); + if (updated) expired.push(hydrateInteraction(updated)); + } + if (expired.length > 0) { + await touchIssue(db, issue.id); + await emitResolvedInteractionsTelemetry(db, expired); + } + return expired; + }, + + withdrawInteraction: async ( + issue: { id: string; companyId: string }, + interactionId: string, + input: WithdrawIssueThreadInteraction, + actor: InteractionActor, + ) => { + const data = withdrawIssueThreadInteractionSchema.parse(input); + const current = await db + .select() + .from(issueThreadInteractions) + .where(eq(issueThreadInteractions.id, interactionId)) + .then((rows) => rows[0] ?? null); + if (!current || current.companyId !== issue.companyId || current.issueId !== issue.id) { + throw notFound("Interaction not found"); + } + if (current.status !== "pending") throw conflict("Interaction has already been resolved"); + + const reason = data.reason?.trim() || null; + const now = new Date(); + // One transaction, linked tool action first: revoking pending/approved + // requests before resolving the card means a concurrent gateway claim + // (approved -> executing) either loses to the revocation's row lock or + // is detected below and aborts the withdrawal; a concurrent card + // resolution rolls the revocation back via the status="pending" guard. + // "approved" is revoked too — the request can be approved from the tool + // review queue while the card is still pending, and an executable + // request must not outlive a withdrawn card. + const updated = await db.transaction(async (tx) => { + await resolveLinkedToolActionRequests(tx, current, { + status: "cancelled", + fromStatuses: ["pending", "approved"], + actor, + now, + }); + if (current.kind === "request_confirmation") { + const active = await tx + .select({ id: toolActionRequests.id }) + .from(toolActionRequests) + .where(and( + eq(toolActionRequests.companyId, current.companyId), + eq(toolActionRequests.interactionId, current.id), + inArray(toolActionRequests.status, ["executing", "executed"]), + )) + .then((rows) => rows[0] ?? null); + if (active) throw conflict("The linked tool action is already executing and can no longer be withdrawn"); + } + const [row] = await tx + .update(issueThreadInteractions) + .set({ + status: "cancelled", + result: buildAdministrativeOutcomeResult(current, "withdrawn", reason), + resolvedByAgentId: actor.agentId ?? null, + resolvedByUserId: actor.userId ?? null, + resolvedAt: now, + updatedAt: now, + }) + .where(and( + eq(issueThreadInteractions.id, interactionId), + eq(issueThreadInteractions.status, "pending"), + )) + .returning(); + if (!row) throw conflict("Interaction has already been resolved"); + return row; + }); + + await touchIssue(db, issue.id); + const withdrawn = hydrateInteraction(updated); + await emitInteractionResolvedTelemetry(db, withdrawn); + return withdrawn; + }, + answerQuestions: async ( issue: { id: string; companyId: string }, interactionId: string, diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index aa9cf5a89f..b5e6811cf3 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -109,6 +109,7 @@ import { classifyIssueGraphLiveness, type IssueLivenessFinding } from "./recover import { visibleIssueCondition } from "./issue-visibility.js"; import { finalizeStatusCardsForStalledGeneration } from "./status-card-finalization.js"; import { finalizeSummarySlotsForTerminalIssue } from "./summary-slot-finalization.js"; +import { logActivity } from "./activity-log.js"; const ALL_ISSUE_STATUSES = ["backlog", "todo", "in_progress", "in_review", "blocked", "done", "cancelled"]; const MAX_ISSUE_COMMENT_PAGE_LIMIT = 500; @@ -6747,6 +6748,35 @@ export function issueService(db: Db) { if (existing.status !== updated.status) { if (updated.status === "done" || updated.status === "cancelled") { await finalizeSummarySlotsForTerminalIssue(tx, updated); + // Every terminal transition funnels through here, including direct + // service callers (tree control, recovery, pipelines, status cards) + // that never touch the HTTP routes, so pending interaction cards + // cannot outlive their issue. Dynamic import breaks the module + // cycle (issue-thread-interactions.js imports issueService). + const { issueThreadInteractionService } = await import("./issue-thread-interactions.js"); + const expiredInteractions = await issueThreadInteractionService(tx).expirePendingInteractionsForTerminalIssue( + updated, + { agentId: actorAgentId ?? null, userId: actorUserId ?? null }, + ); + for (const interaction of expiredInteractions) { + await logActivity(tx as unknown as Db, { + companyId: updated.companyId, + actorType: actorAgentId ? "agent" : actorUserId ? "user" : "system", + actorId: actorAgentId ?? actorUserId ?? "issue_service", + agentId: actorAgentId ?? null, + action: "issue.thread_interaction_expired", + entityType: "issue", + entityId: updated.id, + details: { + identifier: updated.identifier ?? null, + interactionId: interaction.id, + interactionKind: interaction.kind, + interactionStatus: interaction.status, + source: "issue.status_transition.issue_closed", + result: interaction.result ?? null, + }, + }); + } } // A status-card generation task that goes done/cancelled/blocked stops // making progress; release the card's generation claim so the board tile diff --git a/server/src/services/tool-gateway.ts b/server/src/services/tool-gateway.ts index 9ec481000a..dcb7680694 100644 --- a/server/src/services/tool-gateway.ts +++ b/server/src/services/tool-gateway.ts @@ -4070,6 +4070,39 @@ export function createToolGatewayService( return { reasonCode, message }; } + // Guard for approved-action execution: the issue must still be open. Expires + // the claimed request and reflects the interaction lifecycle when it is not. + // Called twice — after winning the executing claim, and again immediately + // before provider dispatch, because tool/snapshot resolution between the two + // involves network calls and leaves a seconds-wide window for the issue to + // close. + async function assertIssueOpenForApprovedAction(input: { + claimed: typeof toolActionRequests.$inferSelect; + invocation: typeof toolInvocations.$inferSelect; + }): Promise<{ projectId: string | null }> { + const { claimed, invocation } = input; + const [issue] = await db + .select({ status: issues.status, projectId: issues.projectId }) + .from(issues) + .where(and(eq(issues.id, invocation.issueId!), eq(issues.companyId, invocation.companyId))) + .limit(1); + if (issue && issue.status !== "done" && issue.status !== "cancelled") { + return { projectId: issue.projectId }; + } + const expiredAt = new Date(); + await db + .update(toolActionRequests) + .set({ status: "expired", resolvedAt: expiredAt, updatedAt: expiredAt }) + .where(eq(toolActionRequests.id, claimed.id)); + await reflectToolActionInteractionLifecycle({ actionRequestId: claimed.id, status: "expired" }); + throw new ToolGatewayHttpError( + 409, + "The issue for this tool action is closed; the approval has expired", + "action_issue_closed", + { actionRequestId: claimed.id, invocationId: invocation.id }, + ); + } + async function executeApprovedAgentInvocation(input: { actionRequest: typeof toolActionRequests.$inferSelect; invocation: typeof toolInvocations.$inferSelect; @@ -4105,6 +4138,12 @@ export function createToolGatewayService( throw new ToolGatewayHttpError(409, "Tool action request was already consumed", "action_already_consumed"); } + // Terminal-issue expiry revokes pending/approved requests, but a claim that + // committed just before the issue closed slips past that revocation. Recheck + // the issue after winning the claim so a governed action never runs external + // side effects for an issue that is already done or cancelled. + const issue = await assertIssueOpenForApprovedAction({ claimed, invocation }); + const signedPayload = readSignedToolArgumentsPayload({ signedArguments: claimed.signedArguments, invocationId: invocation.id, @@ -4124,11 +4163,6 @@ export function createToolGatewayService( ); } - const [issue] = await db - .select({ projectId: issues.projectId }) - .from(issues) - .where(and(eq(issues.id, invocation.issueId), eq(issues.companyId, invocation.companyId))) - .limit(1); const session: ToolGatewaySession = { id: `approved-action:${claimed.id}`, token: "", @@ -4184,6 +4218,17 @@ export function createToolGatewayService( sensitiveMode: "redact", promptInjectionMode: "ignore", }).summary; + // Final recheck at the last DB write before dispatch: tool and snapshot + // resolution above involve network calls, so re-verify the issue is still + // open now that only the provider call remains. A close that commits after + // this read has raced an execution that was approved, claimed, and verified + // while the issue was open; that instant is irreducible for an external + // side effect gated by DB state (holding a DB lock across a remote provider + // call is not an option), and the accepted linearization is that the + // execution wins — its result still lands on the expired card via + // reflectToolActionInteractionLifecycle. + await assertIssueOpenForApprovedAction({ claimed, invocation }); + const startedAt = Date.now(); await db .update(toolInvocations) diff --git a/skills/paperclip/SKILL.md b/skills/paperclip/SKILL.md index 718b030281..91658f4535 100644 --- a/skills/paperclip/SKILL.md +++ b/skills/paperclip/SKILL.md @@ -234,6 +234,7 @@ Key shared semantics: - **Continuation policy.** `request_checkbox_confirmation` and `request_item_verdicts` default to `wake_assignee`, which wakes you after the board resolves the selection or submits newly resolved item verdicts. `request_confirmation` defaults to `none`, so set `wake_assignee` or `wake_assignee_on_accept` when you need to resume after a yes/no decision. `none` never wakes you — only use it when you truly do not need to resume. - **Target binding and staleness.** `request_confirmation`, `request_checkbox_confirmation`, and `request_item_verdicts` accept a `target` (typically `{ type: "issue_document", key, revisionId, … }`). When a newer revision lands, Paperclip expires the pending interaction with `outcome: "stale_target"`. Rebuild against the latest revision and create a fresh interaction. - **Supersede on user comment.** Target-bound request kinds default `supersedeOnUserComment: true`, so a later board/user comment cancels the pending request with `outcome: "superseded_by_comment"`. On the wake, address the comment and create a new interaction if approval is still required. +- **Withdraw and terminal expiry.** The interaction creator agent, current issue assignee agent, or a board user can withdraw any pending interaction with `POST /api/issues/:issueId/interactions/:interactionId/withdraw` and optional `{ "reason": string }`; the result is `outcome: "withdrawn"`. Closing an issue as `done` or `cancelled` expires all remaining pending interactions with `outcome: "issue_closed"` and never wakes the closed issue. - **Idempotency.** Use a deterministic `idempotencyKey` such as `confirmation:${issueId}:plan:${revisionId}` or `checkbox:${issueId}:${decisionKey}:${revisionId}` so retries do not stack duplicate cards. - **Source issue posture.** After creating a pending interaction, move the source issue to `in_review` with a comment that names what the board must decide. The pending interaction is the explicit waiting path. @@ -501,7 +502,7 @@ If `plan` already exists, fetch the current document first and send its latest ` | Update task | `PATCH /api/issues/:issueId` (optional `comment` field) | | Get comments / delta / single | `GET /api/issues/:issueId/comments[?after=:commentId&order=asc]` • `/comments/:commentId` | | Add comment | `POST /api/issues/:issueId/comments` | -| Issue-thread interactions | `GET\|POST /api/issues/:issueId/interactions` • `POST /api/issues/:issueId/interactions/:interactionId/{accept,reject,respond}` | +| Issue-thread interactions | `GET\|POST /api/issues/:issueId/interactions` • `POST /api/issues/:issueId/interactions/:interactionId/{accept,reject,respond,withdraw}` | | Create subtask | `POST /api/companies/:companyId/issues` | | Release task | `POST /api/issues/:issueId/release` | | Search issues | `GET /api/companies/:companyId/issues?q=search+term` | diff --git a/skills/paperclip/references/api-reference.md b/skills/paperclip/references/api-reference.md index 9ee8821c10..a21a3eae6a 100644 --- a/skills/paperclip/references/api-reference.md +++ b/skills/paperclip/references/api-reference.md @@ -974,6 +974,9 @@ Resolved result (`RequestCheckboxConfirmationResult`): Other outcomes match `request_confirmation`: +- `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. +- `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. + - `rejected` — `{ outcome: "rejected", reason, commentId }`. `selectedOptionIds` is absent. - `superseded_by_comment` — `{ outcome: "superseded_by_comment", commentId }`. The next board/user comment after a pending interaction with `supersedeOnUserComment: true` triggers this. - `stale_target` — `{ outcome: "stale_target", staleTarget }`. Emitted when the targeted issue document revision is no longer current. @@ -1210,6 +1213,7 @@ Terminal states: `done`, `cancelled` | POST | `/api/issues/:issueId/interactions/:interactionId/reject` | Reject suggested tasks or confirmation | | POST | `/api/issues/:issueId/interactions/:interactionId/respond` | Respond to structured questions | | POST | `/api/issues/:issueId/interactions/:interactionId/verdicts` | Submit partial item verdicts for `request_item_verdicts` | +| POST | `/api/issues/:issueId/interactions/:interactionId/withdraw` | Withdraw any pending interaction; optional `{ "reason": string }`; creator agent, current assignee agent, or board user | | GET | `/api/issues/:issueId/documents` | List issue documents | | GET | `/api/issues/:issueId/documents/:key` | Get issue document by key | | PUT | `/api/issues/:issueId/documents/:key` | Create or update issue document (send `baseRevisionId` when updating) | diff --git a/ui/src/components/IssueThreadInteractionCard.test.tsx b/ui/src/components/IssueThreadInteractionCard.test.tsx index ec2f79a7ee..e2ebc7b882 100644 --- a/ui/src/components/IssueThreadInteractionCard.test.tsx +++ b/ui/src/components/IssueThreadInteractionCard.test.tsx @@ -236,6 +236,76 @@ describe("IssueThreadInteractionCard", () => { expect(host.textContent).not.toContain("Questions expired by comment"); }); + it("renders withdrawn confirmations with the withdraw reason", () => { + const host = renderCard({ + interaction: { + ...pendingRequestConfirmationInteraction, + status: "cancelled", + result: { version: 1, outcome: "withdrawn", reason: "Superseded by the hotfix plan." }, + }, + onAcceptInteraction: vi.fn(), + onRejectInteraction: vi.fn(), + }); + + expect(host.textContent).toContain("Withdrawn"); + expect(host.textContent).toContain("Superseded by the hotfix plan."); + expect(host.textContent).not.toContain("Decline"); + }); + + it("renders confirmations expired by issue closure with dedicated copy", () => { + const host = renderCard({ + interaction: { + ...pendingRequestConfirmationInteraction, + status: "expired", + result: { version: 1, outcome: "issue_closed", reason: null }, + }, + }); + + expect(host.textContent).toContain("Expired when issue closed"); + expect(host.textContent).toContain("The issue was closed before this confirmation was resolved."); + expect(host.textContent).not.toContain("Expired by target change"); + }); + + it("renders withdrawn question interactions with the withdraw reason", () => { + const host = renderCard({ + interaction: { + ...pendingAskUserQuestionsInteraction, + status: "cancelled", + result: { + version: 1, + outcome: "withdrawn", + reason: "Scope was decided on the parent issue.", + answers: [], + summaryMarkdown: null, + }, + }, + }); + + expect(host.textContent).toContain("Questions withdrawn"); + expect(host.textContent).toContain("Scope was decided on the parent issue."); + expect(host.textContent).not.toContain("Question cancelled"); + }); + + it("renders question interactions expired by issue closure with dedicated copy", () => { + const host = renderCard({ + interaction: { + ...pendingAskUserQuestionsInteraction, + status: "expired", + result: { + version: 1, + outcome: "issue_closed", + reason: null, + answers: [], + summaryMarkdown: null, + }, + }, + }); + + expect(host.textContent).toContain("Questions expired when issue closed"); + expect(host.textContent).toContain("The issue was closed before these questions were answered."); + expect(host.textContent).not.toContain("expired by comment"); + }); + it("makes child tasks explicit in suggested task trees", () => { const host = renderCard({ interaction: pendingSuggestedTasksInteraction, diff --git a/ui/src/components/IssueThreadInteractionCard.tsx b/ui/src/components/IssueThreadInteractionCard.tsx index ed946654c8..364a68a173 100644 --- a/ui/src/components/IssueThreadInteractionCard.tsx +++ b/ui/src/components/IssueThreadInteractionCard.tsx @@ -192,6 +192,7 @@ function requestConfirmationResumeFailure(interaction: IssueThreadInteraction) { function planStatusClasses( status: IssueThreadInteraction["status"], resumeFailure?: ReturnType, + outcome?: string | null, ) { switch (status) { case "accepted": @@ -215,7 +216,7 @@ function planStatusClasses( return { shell: "border-2 border-red-500/80 bg-transparent", badge: "border-red-500/60 bg-red-500/10 text-red-900 dark:bg-red-500/15 dark:text-red-100", - label: "Changes requested", + label: outcome === "withdrawn" ? "Withdrawn" : "Changes requested", Icon: XCircle, }; case "failed": @@ -1154,9 +1155,15 @@ function AskUserQuestionsCard({ ) : interaction.status === "cancelled" ? (
-
Question cancelled
+
+ {interaction.result?.outcome === "withdrawn" + ? questions.length === 1 ? "Question withdrawn" : "Questions withdrawn" + : "Question cancelled"} +
{interaction.result?.cancellationReason ? (

{interaction.result.cancellationReason}

+ ) : interaction.result?.reason ? ( +

{interaction.result.reason}

) : (

No answer was recorded.

)} @@ -1165,10 +1172,14 @@ function AskUserQuestionsCard({
- {questions.length === 1 ? "Question expired by comment" : "Questions expired by comment"} + {interaction.result?.outcome === "issue_closed" + ? questions.length === 1 ? "Question expired when issue closed" : "Questions expired when issue closed" + : questions.length === 1 ? "Question expired by comment" : "Questions expired by comment"}

- A later board/user comment superseded this question request. Create a fresh request if answers are still needed. + {interaction.result?.outcome === "issue_closed" + ? "The issue was closed before these questions were answered." + : "A later board/user comment superseded this question request. Create a fresh request if answers are still needed."}

{interaction.result?.commentId ? ( +
+ Withdrawn + +
+ {interaction.result?.reason ? ( +
+ {interaction.result.reason} +
+ ) : null} +
+ ); + } + if (interaction.status === "expired") { const expiredByComment = outcome === "superseded_by_comment"; + const expiredWithIssue = outcome === "issue_closed"; const expiredByTargetChange = outcome === "stale_target"; return (
- {expiredByComment ? "Expired by comment" : "Expired by target change"} + {expiredByComment ? "Expired by comment" : expiredWithIssue ? "Expired when issue closed" : "Expired by target change"}

{expiredByComment ? "A board comment superseded this confirmation before it was resolved." - : "The requested target changed before this confirmation was resolved."} + : expiredWithIssue + ? "The issue was closed before this confirmation was resolved." + : "The requested target changed before this confirmation was resolved."}

{expiredByComment && interaction.result?.commentId ? (