From 457d3209b18bd16415f5fe081df3bad1e1dacf37 Mon Sep 17 00:00:00 2001 From: Aashish Mahato <145881415+aashish254@users.noreply.github.com> Date: Fri, 16 Oct 2026 04:56:35 +0545 Subject: [PATCH] fix: name the real reason a connection request expires on reassignment (#14067) 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. > - When an agent's selected AI account is unavailable, the issue thread shows a `connection_intent` card. A person must reconnect the account or pick another connection. > - If the issue assignment changes while that card is still pending, the server expires it. This is the ownership-change path in `expireConnectionIntentsForOwnershipChange`. > - The expired card showed the reason "The task assignment changed". That text is wrong in two ways. It does not say that the connection request was never answered, and it does not say the connection is still missing. > - Worse, the reason never reached the screen at all: the expired branch of the card hard-codes "This request is no longer active.", so any guidance the server stores is discarded. My first round of this PR claimed the opposite, which was incorrect — the UI change is required for the fix to be visible. > - Issue #14040 reported the symptom: users saw a run fail with "assignment" blame and no hint that a connection needed attention. It happened on both `claude_local` and `codex_local` agents, so the message — not one adapter — is the defect. > - This pull request therefore fixes both ends: the server stores a sentence that names what happened and what to do next, per kind of request, and the card prints the stored reason instead of a generic string. > - The issue asks for a larger behavior change (hold reassignment until the card resolves). That needs a product discussion, so this PR keeps the scope to the message and where it is displayed. > - The benefit is that the card tells the truth: the blocker is the connection, not the assignment, and it says so on screen. ## Linked Issues or Issue Description - Fixes: #14040 ## What Changed - `server/src/services/issue-thread-interactions.ts`: `expireConnectionIntentsForOwnershipChange` now selects each pending row's payload before expiring it and writes a per-request reason: "The task was reassigned before the connection request was answered, so the AI account is still not connected. Reconnect it or choose an available connection." for AI-account intents, and the same sentence naming "the `` connection" for tool intents. The subject is chosen from `payload.purpose === "ai"`, falling back to "the requested connection" for a payload that carries neither, because this one path expires both kinds and the recovery step differs. - `ui/src/features/connections/ConnectionIntentInteractionBody.tsx`: the expired branch now renders `interaction.result?.reason` and keeps "This request is no longer active." only as the fallback for results stored before this change. The `superseded` outcome keeps its own wording. This component is the shared body for both the compact task card and the full thread card, so both views get the reason. - `server/src/__tests__/issues-service.test.ts`: the expiry test now expires a tool (`Notion`) and an AI (`claude_local`) intent in the same run and asserts both stored reasons, so neither wording can regress into the other. - `ui/src/features/connections/ConnectionIntentInteractionBody.test.tsx`: new test renders an expired card whose result carries a reason and asserts the reason is on screen and the generic fallback string is not. ## Verification - `npx vitest run server/src/__tests__/issues-service.test.ts ui/src/features/connections/ConnectionIntentInteractionBody.test.tsx` — 151/151 passed (127 server, 24 UI; 33.4s, Node 22.22.2, local). My first revision of this body cited 168 for the same set; that count came from a run on the wrong branch, and 151 is what this command prints on `fix/connection-intent-expiry-reason`. - The pinned server test drives the real path: it inserts pending `connection_intent` rows plus a live `toolOauthStates` row, reassigns the issue through `svc.update(...)`, then asserts both expired reasons and the deleted OAuth state row. - Mutation checks: forcing the AI subject for every row makes the expiry test fail on the tool-row assertion (`1 failed | 126 skipped`, `expected [ { …29 fields } ] to deeply equal [ ObjectContaining{…} ]`), and deleting the `result.reason` branch in the card fails exactly `shows the stored reason on an expired request` (`1 failed | 23 passed`). Both revert clean. The reverse wording mutation (tool subject forced onto AI rows) was not run, so that half of the test is pinned by its assertion rather than by a demonstrated mutation. - No schema, wire-format, or payload change. Only the free-text `result.reason` value differs and the card now reads the field it was already given. ## Risks - Low risk. The value is display text on a terminal interaction outcome. No code parses the old string, and no stored data is migrated; cards that expired before this change keep their old text and now render it as the stored reason they carry. - The deeper request in #14040 (do not expire a pending `connection_intent` on reassignment) is intentionally out of scope here and needs a maintainer decision. ## Model Used - Anthropic Claude (Opus 4.x, agentic CLI coding assistant with file edit and shell tool use) assisted with the code change, the test updates, and this PR text. A human reviewed the diff and the issue context before submission. ## 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- server/src/__tests__/issues-service.test.ts | 26 ++++++++++- .../src/services/issue-thread-interactions.ts | 46 ++++++++++++++++--- .../ConnectionIntentInteractionBody.test.tsx | 12 +++++ .../ConnectionIntentInteractionBody.tsx | 5 +- 4 files changed, 81 insertions(+), 8 deletions(-) diff --git a/server/src/__tests__/issues-service.test.ts b/server/src/__tests__/issues-service.test.ts index daa556d4231..ed2ec590efd 100644 --- a/server/src/__tests__/issues-service.test.ts +++ b/server/src/__tests__/issues-service.test.ts @@ -886,6 +886,17 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => { requestingAgentId: agentId, requestingAgentName: "ConnectionRequester", phase: "authorizing", }, }).returning(); + // A second pending request on the same issue asks for an AI account. The + // expiry must tell the two apart: only the AI one blocks the run. + const [aiInteraction] = await db.insert(issueThreadInteractions).values({ + companyId, issueId: issue.id, kind: "connection_intent", status: "pending", + createdByAgentId: agentId, addresseeUserId: "local-board", + payload: { + version: 1, purpose: "ai", serviceSlug: "connection:claude_local", + serviceName: "Claude", requestingAgentId: agentId, + requestingAgentName: "ConnectionRequester", phase: "authorizing", + }, + }).returning(); const oauthState = randomUUID(); await db.insert(toolOauthStates).values({ state: oauthState, companyId, connectionId: connection!.id, issueId: issue.id, @@ -901,9 +912,22 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => { await expect(db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, interaction!.id))) .resolves.toEqual([expect.objectContaining({ status: "expired", - result: { version: 1, outcome: "expired", reason: "The task assignment changed" }, + result: { + version: 1, + outcome: "expired", + reason: "The task was reassigned before the connection request was answered, so the Notion connection is still not connected. Reconnect it or choose an available connection.", + }, resolvedAt: expect.any(Date), })]); + await expect(db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, aiInteraction!.id))) + .resolves.toEqual([expect.objectContaining({ + status: "expired", + result: { + version: 1, + outcome: "expired", + reason: "The task was reassigned before the connection request was answered, so the AI account is still not connected. Reconnect it or choose an available connection.", + }, + })]); await expect(db.select().from(toolOauthStates).where(eq(toolOauthStates.state, oauthState))).resolves.toEqual([]); }); diff --git a/server/src/services/issue-thread-interactions.ts b/server/src/services/issue-thread-interactions.ts index 96049788e73..015ba84cf45 100644 --- a/server/src/services/issue-thread-interactions.ts +++ b/server/src/services/issue-thread-interactions.ts @@ -4746,12 +4746,46 @@ export function issueThreadInteractionService( }, expireConnectionIntentsForOwnershipChange: async (issue: { id: string; companyId: string }) => { - const expired = await db.update(issueThreadInteractions).set({ - status: "expired", result: { version: 1, outcome: "expired", reason: "The task assignment changed" }, - resolvedAt: now(), updatedAt: now(), - }).where(and(eq(issueThreadInteractions.companyId, issue.companyId), eq(issueThreadInteractions.issueId, issue.id), - eq(issueThreadInteractions.kind, "connection_intent"), eq(issueThreadInteractions.status, "pending"))).returning(); - if (expired.length) await db.delete(toolOauthStates).where(inArray(toolOauthStates.interactionId, expired.map((row) => row.id))); + // A pending connection_intent asks for either an AI account or a tool + // account, and the two leave different things broken. One bulk write gave + // every row the AI wording, so an expired Notion request told the reader + // the AI account was missing. Read the pending rows first and name the + // subject each row actually asked for. + const pending = await db + .select({ id: issueThreadInteractions.id, payload: issueThreadInteractions.payload }) + .from(issueThreadInteractions) + .where(and( + eq(issueThreadInteractions.companyId, issue.companyId), + eq(issueThreadInteractions.issueId, issue.id), + eq(issueThreadInteractions.kind, "connection_intent"), + eq(issueThreadInteractions.status, "pending"), + )); + const expired: typeof issueThreadInteractions.$inferSelect[] = []; + for (const row of pending) { + const payload = row.payload as { purpose?: string; serviceName?: string } | null; + const subject = payload?.purpose === "ai" + ? "the AI account" + : `the ${payload?.serviceName?.trim() || "requested"} connection`; + const [updated] = await db + .update(issueThreadInteractions) + .set({ + status: "expired", + result: { + version: 1, + outcome: "expired", + reason: `The task was reassigned before the connection request was answered, so ${subject} is still not connected. Reconnect it or choose an available connection.`, + }, + resolvedAt: now(), + updatedAt: now(), + }) + .where(and( + eq(issueThreadInteractions.id, row.id), + eq(issueThreadInteractions.status, "pending"), + )) + .returning(); + if (updated) expired.push(updated); + } + if (expired.length) await db.delete(toolOauthStates).where(inArray(toolOauthStates.interactionId, expired.map((item) => item.id))); return expired; }, expirePendingInteractionsForTerminalIssue: async ( diff --git a/ui/src/features/connections/ConnectionIntentInteractionBody.test.tsx b/ui/src/features/connections/ConnectionIntentInteractionBody.test.tsx index 406eb0fcf63..f9ac280ec45 100644 --- a/ui/src/features/connections/ConnectionIntentInteractionBody.test.tsx +++ b/ui/src/features/connections/ConnectionIntentInteractionBody.test.tsx @@ -259,6 +259,18 @@ describe("ConnectionIntentInteractionBody states and audience", () => { expect(document.body.textContent).toContain(expected); }); + it("shows the stored reason on an expired request", () => { + // The server writes what actually expired the request and what to do next. + // A generic "no longer active" line left the reader with no next step. + const reason = "The task was reassigned before the connection request was answered, so the Notion connection is still not connected. Reconnect it or choose an available connection."; + renderBody({ + ...terminal("expired", "expired"), + result: { version: 1, outcome: "expired", reason }, + } as ConnectionIntentInteraction); + expect(document.body.textContent).toContain(reason); + expect(document.body.textContent).not.toContain("This request is no longer active."); + }); + it("shows non-addressees only the waiting state and never loads setup", () => { renderBody(pendingConnectionIntentInteraction, "other-user"); expect(document.body.textContent).toContain("Waiting for Carol"); diff --git a/ui/src/features/connections/ConnectionIntentInteractionBody.tsx b/ui/src/features/connections/ConnectionIntentInteractionBody.tsx index 48a56139bed..a87af21f786 100644 --- a/ui/src/features/connections/ConnectionIntentInteractionBody.tsx +++ b/ui/src/features/connections/ConnectionIntentInteractionBody.tsx @@ -268,7 +268,10 @@ export function ConnectionIntentInteractionBody({ body: resultOutcome === "superseded" ? "This request was replaced. Use the latest connection card instead." - : "This request is no longer active.", + : // The server stores the reason an expiry happened and what to + // do next; without it the card only says the request is gone. + interaction.result?.reason?.trim() + || "This request is no longer active.", } : null; const StatusIcon = status?.icon;