Files
PaperClipAI/server/src/__tests__/heartbeat-reviewed-chat-binding.integration.test.ts
T
Devin FoleyandPaperclip 110d176fc9 fix: preserve chat message bindings in review recovery (#13818)
## Thinking Path

> - Paperclip manages AI agents and their work.
> - External chat messages can start work on tasks that remain in review.
> - Paperclip queues one recovery run when a task loses its review path.
> - That recovery keeps the chat source but loses the admitted message IDs.
> - The authorization check then rejects the recovery before execution starts.
> - This change retains the message IDs so the existing check can verify current access.

## Linked Issues or Issue Description

Refs #13809.

**What happened?**

A successful external-chat run can leave an active task in review without a maintained review path. Its automatic recovery then fails with `reviewed_chat_execution_binding_not_authorized`. The recovery context retains `chat:slack` but drops `wakeCommentIds`.

**Expected behavior**

An eligible recovery should retain its admitted message references and pass a new authorization check. Missing or revoked access must still prevent execution.

**Steps to reproduce**

1. Finish a chat run whose task remains in review with no maintained review path.
2. Build the bounded review recovery from that run's context.
3. Dispatch the recovery through the reviewed-chat authorization check.

The new regression tests fail before this patch. Related PR #13809 handles answered conversations that become idle. This patch handles recovery when the conversation remains active.

## What Changed

- Retain the admitted message batch for external-chat review recovery. Derive the current comment reference from that batch.
- Share the existing supported-provider selector between recovery and run-bound chat authorization. AgentMail remains on its separate email inbox path.
- Keep the existing authorization check. Do not copy prior checkout, authorization, session, or prompt state.
- Test Slack and Discord recovery, missing batches, revoked access, and changed task or company bindings.
- Document the recovery authorization contract.

## Verification

- The new tests reproduced the missing-message failure before the fix.
- Six focused suites passed: 257 tests covering review recovery, reviewed-chat authorization, issue liveness, Slack lifecycle, comment-wake batching, and external-chat waits. PostgreSQL integration tests ran against a temporary local PostgreSQL database.
- Focused test files: `review-path-recovery.test.ts`, `heartbeat-reviewed-chat-binding.integration.test.ts`, `heartbeat-issue-liveness-escalation.test.ts`, `slack-conversation-lifecycle.test.ts`, `heartbeat-comment-wake-batching.test.ts`, and `external-chat-wait.integration.test.ts`.
- Server TypeScript check passed with scratch configuration that resolves this checkout's workspace packages. Existing dependency links point to another checkout; the default check reports stale shared-type errors.
- `node scripts/check-module-boundaries.mjs` and `git diff --check` passed.
- `git diff | gitleaks stdin --redact --no-banner` passed. The diff was also checked for private identifiers and user data.
- [Full PR CI](https://github.com/paperclipai/paperclip/actions/runs/35760757895) passed on the latest commit, including build, workspace typecheck, general and serialized tests, Rust checks, and all eight browser shards. The unchanged local-service readiness test and agent-chat page-load assertion passed when their failed shards were retried. The local-service suite also passed locally (6 tests). The first, superseded run lost a chat runner; its stuck browser job was cancelled to unblock the current run.
- Full local workspace typecheck, tests, and build were not run. The machine has about 2 GiB free, and those commands include Rust builds. The full PR CI checks passed before merge.

## Risks

Small context-construction change. Dispatch still checks current execution ownership and access for every admitted message. The recovery remains bounded to one attempt per consumed path. No schema changes. Existing failed runs are not retried by this patch.

## Model Used

OpenAI GPT-6 (Codex), with tool use and code execution.

## 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>
2026-09-22 10:46:13 -07:00

607 lines
18 KiB
TypeScript

import { randomUUID } from "node:crypto";
import { eq } from "drizzle-orm";
import { afterAll, beforeAll, describe, expect, it } from "vitest";
import {
agents,
companies,
createDb,
heartbeatRuns,
issues,
issueComments,
issueThreadInteractions,
toolApplications,
toolConnections,
chatEndpoints,
chatEndpointResources,
chatConversations,
chatExternalPrincipals,
chatIdentityLinks,
companyMemberships,
chatDeliveries,
chatMessageLinks,
} from "@paperclipai/db";
import { renderPaperclipWakePrompt } from "@paperclipai/adapter-utils/server-utils";
import { startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
import {
attestReviewedExternalChatRun,
buildPaperclipWakePayload,
} from "../services/heartbeat.js";
import {
listAuthorizedChatAttachments,
resolveExternalChatResponseWaitAuthorization,
} from "../services/native-runtime/chat-attachment-reuse.js";
import { decideIssueReviewPathRecovery } from "../services/recovery/review-path-recovery.js";
import { resolveCurrentWakeCommentsBinding } from "../services/native-runtime/current-wake-comments.js";
describe.each(["slack", "discord"] as const)("reviewed %s execution binding", (provider) => {
let temporary: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
let db: ReturnType<typeof createDb>;
const companyId = randomUUID(),
agentId = randomUUID(),
issueId = randomUUID(),
otherIssueId = randomUUID(),
runId = randomUUID();
const endpointId = randomUUID(),
resourceId = randomUUID(),
conversationId = randomUUID();
const principalId = randomUUID(),
commentId = randomUUID(),
deliveryId = randomUUID();
const interactionId = randomUUID(),
userId = "reviewed-chat-user";
const context = {
issueId,
source: `chat:${provider}`,
wakeReason: "External chat message received",
wakeCommentIds: [commentId],
commentId,
};
const binding = { companyId, agentId, issueId, runId };
const attest = (contextSnapshot: Record<string, unknown> = context) =>
attestReviewedExternalChatRun({ db, ...binding, contextSnapshot });
beforeAll(async () => {
temporary = await startEmbeddedPostgresTestDatabase(
"reviewed-chat-binding-",
);
db = createDb(temporary.connectionString);
await db
.insert(companies)
.values({
id: companyId,
name: "Reviewed chat",
issuePrefix: "RCB",
issueCounter: 1,
});
await db
.insert(agents)
.values({
id: agentId,
companyId,
name: "Chat runner",
adapterType: "paperclip_runner",
adapterConfig: { provider: "codex" },
status: "active",
});
await db
.insert(issues)
.values({
id: issueId,
companyId,
title: "Reviewed conversation",
issueNumber: 1,
identifier: "RCB-1",
status: "in_review",
workMode: "standard",
assigneeAgentId: agentId,
});
await db.insert(issues).values({
id: otherIssueId,
companyId,
title: "Unrelated conversation",
issueNumber: 2,
identifier: "RCB-2",
status: "in_review",
assigneeAgentId: agentId,
});
await db
.insert(heartbeatRuns)
.values({
id: runId,
companyId,
agentId,
status: "running",
runtimeMode: "native",
nativeIssueId: issueId,
invocationSource: "assignment",
triggerDetail: "system",
contextSnapshot: context,
});
await db
.update(issues)
.set({ executionRunId: runId })
.where(eq(issues.id, issueId));
await db
.insert(issueThreadInteractions)
.values({
id: interactionId,
companyId,
issueId,
sourceRunId: runId,
kind: "request_confirmation",
status: "pending",
title: "Real pending governance",
payload: {
version: 1,
prompt: "Review completion before closing this task.",
},
});
const applicationId = randomUUID(),
connectionId = randomUUID();
await db
.insert(toolApplications)
.values({
id: applicationId,
companyId,
applicationKey: `chat:${provider}:${endpointId}`,
name: "Discord",
type: "chat",
status: "active",
});
await db
.insert(toolConnections)
.values({
id: connectionId,
companyId,
applicationId,
name: "Discord",
uid: `chat-${provider}-${endpointId}`,
connectionPurpose: "channel",
transport: "chat_sdk",
status: "active",
enabled: true,
});
await db
.insert(chatEndpoints)
.values({
id: endpointId,
companyId,
connectionId,
provider,
publicId: randomUUID(),
assignedAgentId: agentId,
status: "active",
providerAccountId: "guild-1",
allowUnlinkedPeople: false,
});
await db
.insert(chatEndpointResources)
.values({
id: resourceId,
companyId,
endpointId,
type: "channel",
providerResourceId: "channel-1",
label: "#review",
availability: "available",
enabled: true,
});
await db
.insert(chatConversations)
.values({
id: conversationId,
companyId,
endpointId,
resourceId,
issueId,
externalConversationId: "channel-1",
externalThreadId: "thread-1",
externalLabel: "Review thread",
state: "active",
});
await db
.insert(chatExternalPrincipals)
.values({
id: principalId,
companyId,
provider,
providerAccountId: "guild-1",
externalId: "user-1",
kind: "user",
});
await db
.insert(chatIdentityLinks)
.values({
companyId,
endpointId,
principalId,
paperclipUserId: userId,
status: "linked",
});
await db
.insert(companyMemberships)
.values({
companyId,
principalType: "user",
principalId: userId,
status: "active",
membershipRole: "member",
});
await db
.insert(issueComments)
.values({
id: commentId,
companyId,
issueId,
authorType: "user",
authorUserId: userId,
body: "Please inspect the earlier photo, without approving completion.",
});
await db
.insert(chatDeliveries)
.values({
id: deliveryId,
companyId,
endpointId,
conversationId,
principalId,
providerEventId: "review-followup",
deduplicationKey: "review-followup",
eventKind: "message",
normalizedEvent: {},
state: "processed",
attempts: 1,
processedAt: new Date(),
});
await db
.insert(chatMessageLinks)
.values({
companyId,
endpointId,
conversationId,
deliveryId,
commentId,
providerMessageId: "review-message",
direction: "inbound",
});
}, 30_000);
afterAll(async () => {
await temporary?.cleanup();
});
it("attests existing execution ownership while preserving real pending governance and checkout state", async () => {
const [before] = await db
.select()
.from(issues)
.where(eq(issues.id, issueId));
const [gateBefore] = await db
.select()
.from(issueThreadInteractions)
.where(eq(issueThreadInteractions.id, interactionId));
await expect(attest()).resolves.toBe(true);
expect(
(await db.select().from(issues).where(eq(issues.id, issueId)))[0],
).toEqual(before);
expect(
(
await db
.select()
.from(issueThreadInteractions)
.where(eq(issueThreadInteractions.id, interactionId))
)[0],
).toEqual(gateBefore);
expect(before).toMatchObject({
status: "in_review",
checkoutRunId: null,
executionRunId: runId,
});
const wake = await buildPaperclipWakePayload({
db,
companyId,
agentId,
contextSnapshot: {
...context,
paperclipExternalChatExecutionBound: true,
},
});
expect(wake).toMatchObject({
checkedOutByHarness: false,
externalChatExecutionBound: true,
externalChatProvider: provider,
});
const prompt = renderPaperclipWakePrompt(wake);
expect(prompt).toContain("not a checkout, approval");
expect(prompt).toContain("task remains in review");
expect(prompt).not.toContain("checked out the issue for this run");
});
async function withReviewRecovery(
check: (recovery: Record<string, unknown>) => Promise<void>,
) {
const decision = decideIssueReviewPathRecovery({
issueId,
sourceRunId: randomUUID(),
assigneeAgentId: agentId,
contextSnapshot: {
...context,
paperclipHarnessCheckedOut: true,
paperclipExternalChatExecutionBound: true,
},
reviewAttention: { state: "stalled", paths: [], reason: "Review path consumed" },
existingWake: false,
});
expect(decision.kind).toBe("enqueue");
if (decision.kind !== "enqueue") return;
await db.update(heartbeatRuns)
.set({ contextSnapshot: decision.contextSnapshot })
.where(eq(heartbeatRuns.id, runId));
try {
await check(decision.contextSnapshot);
} finally {
await db.update(heartbeatRuns)
.set({ contextSnapshot: context })
.where(eq(heartbeatRuns.id, runId));
}
}
it("reauthorizes a review recovery from its retained message batch without approving the task", async () => {
const [before] = await db.select().from(issues).where(eq(issues.id, issueId));
await withReviewRecovery(async (recovery) => {
expect(recovery).not.toHaveProperty("paperclipHarnessCheckedOut");
expect(recovery).not.toHaveProperty("paperclipExternalChatExecutionBound");
await expect(attest(recovery)).resolves.toBe(true);
expect((await db.select().from(issues).where(eq(issues.id, issueId)))[0]).toEqual(before);
expect((await db.select().from(issueThreadInteractions)
.where(eq(issueThreadInteractions.id, interactionId)))[0].status).toBe("pending");
});
});
it("denies recovery after access revocation or a changed company, task, or message binding", async () => {
await withReviewRecovery(async (recovery) => {
await expect(attest(recovery)).resolves.toBe(true);
await expect(attestReviewedExternalChatRun({
db, ...binding, companyId: randomUUID(), contextSnapshot: recovery,
})).resolves.toBe(false);
await expect(attestReviewedExternalChatRun({
db, ...binding, issueId: otherIssueId, contextSnapshot: recovery,
})).resolves.toBe(false);
await expect(attest({ ...recovery, wakeCommentIds: [randomUUID()] })).resolves.toBe(false);
await db.update(companyMemberships).set({ status: "suspended" })
.where(eq(companyMemberships.principalId, userId));
try {
await expect(attest(recovery)).resolves.toBe(false);
} finally {
await db.update(companyMemberships).set({ status: "active" })
.where(eq(companyMemberships.principalId, userId));
}
await db.update(chatEndpointResources).set({ enabled: false })
.where(eq(chatEndpointResources.id, resourceId));
try {
await expect(attest(recovery)).resolves.toBe(false);
} finally {
await db.update(chatEndpointResources).set({ enabled: true })
.where(eq(chatEndpointResources.id, resourceId));
}
await db.update(chatConversations).set({ issueId: otherIssueId })
.where(eq(chatConversations.id, conversationId));
try {
await expect(attest(recovery)).resolves.toBe(false);
} finally {
await db.update(chatConversations).set({ issueId })
.where(eq(chatConversations.id, conversationId));
}
});
});
it("does not trust a supplied marker, owner mismatch, different wake batch, or wrong provider", async () => {
await db
.update(issues)
.set({ executionRunId: null })
.where(eq(issues.id, issueId));
try {
await expect(
attest({ ...context, paperclipExternalChatExecutionBound: true }),
).resolves.toBe(false);
} finally {
await db
.update(issues)
.set({ executionRunId: runId })
.where(eq(issues.id, issueId));
}
await expect(
attest({ ...context, wakeCommentIds: [randomUUID()] }),
).resolves.toBe(false);
await expect(
attest({
...context,
source: "chat:telegram",
paperclipExternalChatExecutionBound: true,
}),
).resolves.toBe(false);
await db
.update(chatEndpoints)
.set({ provider: "telegram" })
.where(eq(chatEndpoints.id, endpointId));
try {
await expect(attest()).resolves.toBe(false);
} finally {
await db
.update(chatEndpoints)
.set({ provider })
.where(eq(chatEndpoints.id, endpointId));
}
});
it("rejects wrong or revoked principals and disabled provider reach", async () => {
await db
.update(chatExternalPrincipals)
.set({ providerAccountId: "another-guild" })
.where(eq(chatExternalPrincipals.id, principalId));
try {
await expect(attest()).resolves.toBe(false);
} finally {
await db
.update(chatExternalPrincipals)
.set({ providerAccountId: "guild-1" })
.where(eq(chatExternalPrincipals.id, principalId));
}
await db
.update(companyMemberships)
.set({ status: "suspended" })
.where(eq(companyMemberships.principalId, userId));
try {
await expect(attest()).resolves.toBe(false);
} finally {
await db
.update(companyMemberships)
.set({ status: "active" })
.where(eq(companyMemberships.principalId, userId));
}
await db
.update(chatEndpointResources)
.set({ enabled: false })
.where(eq(chatEndpointResources.id, resourceId));
try {
await expect(attest()).resolves.toBe(false);
} finally {
await db
.update(chatEndpointResources)
.set({ enabled: true })
.where(eq(chatEndpointResources.id, resourceId));
}
});
it("retries contended ownership outside the transaction and rechecks policy after lock release", async () => {
for (const revoke of [false, true]) {
let release!: () => void;
let locked!: () => void;
const lockReady = new Promise<void>((resolve) => {
locked = resolve;
});
const gate = new Promise<void>((resolve) => {
release = resolve;
});
const blocker = db.transaction(async (tx) => {
await tx
.select()
.from(issues)
.where(eq(issues.id, issueId))
.for("update");
locked();
await gate;
if (revoke)
await tx
.update(companyMemberships)
.set({ status: "suspended" })
.where(eq(companyMemberships.principalId, userId));
});
await lockReady;
let finished = false;
const pending = attest().then((value) => {
finished = true;
return value;
});
try {
await new Promise((resolve) => setTimeout(resolve, 150));
expect(finished).toBe(false);
release();
await blocker;
expect(await pending).toBe(!revoke);
} finally {
release();
await blocker;
await pending;
await db
.update(companyMemberships)
.set({ status: "active" })
.where(eq(companyMemberships.principalId, userId));
}
}
});
it("waits for exact inbound delivery processing to commit before attesting", async () => {
await db
.update(chatDeliveries)
.set({ state: "processing", processedAt: null })
.where(eq(chatDeliveries.id, deliveryId));
let finished = false;
const pending = attest().then((value) => {
finished = true;
return value;
});
try {
await new Promise((resolve) => setTimeout(resolve, 150));
expect(finished).toBe(false);
await db
.update(chatDeliveries)
.set({ state: "processed", processedAt: new Date() })
.where(eq(chatDeliveries.id, deliveryId));
expect(await pending).toBe(true);
await db
.update(chatDeliveries)
.set({ state: "retry" })
.where(eq(chatDeliveries.id, deliveryId));
expect(await attest()).toBe(false);
} finally {
await db
.update(chatDeliveries)
.set({ state: "processed", processedAt: new Date() })
.where(eq(chatDeliveries.id, deliveryId));
await pending;
}
});
it("binds closed historical/current readers without pretending checkout occurred", async () => {
expect(await attest()).toBe(true);
const wake = await buildPaperclipWakePayload({
db,
companyId,
agentId,
contextSnapshot: {
...context,
paperclipExternalChatExecutionBound: true,
},
});
await db
.update(heartbeatRuns)
.set({
contextSnapshot: {
...context,
paperclipExternalChatExecutionBound: true,
paperclipWake: { ...wake, fallbackFetchNeeded: true },
},
})
.where(eq(heartbeatRuns.id, runId));
try {
await expect(
listAuthorizedChatAttachments({ db, binding, limit: 20 }),
).resolves.toMatchObject({ attachments: [], complete: true });
await expect(
resolveExternalChatResponseWaitAuthorization({ db, binding }),
).resolves.toBe("authorized");
await expect(
resolveCurrentWakeCommentsBinding(db, binding),
).resolves.toMatchObject({
provider,
commentIds: [commentId],
});
await db
.update(companyMemberships)
.set({ status: "suspended" })
.where(eq(companyMemberships.principalId, userId));
await expect(
listAuthorizedChatAttachments({ db, binding, limit: 20 }),
).rejects.toThrow("principal_denied");
} finally {
await db
.update(companyMemberships)
.set({ status: "active" })
.where(eq(companyMemberships.principalId, userId));
await db
.update(heartbeatRuns)
.set({ contextSnapshot: context })
.where(eq(heartbeatRuns.id, runId));
}
});
});