mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 13:24:55 +01:00
feat(interactions): add interaction withdrawal and terminal-issue expiry (#10251)
## 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 | | --- | --- | |  |  | ## 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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
a3b293e26d
commit
f6ab82d490
19 files changed
+882
-37
No files matched your search
@@ -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();
|
||||
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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({
|
||||
|
||||
Reference in new issue
Block a user