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>

doc/execution-semantics.md

@@ -353,6 +353,12 @@ The valid action-path primitives are:
 - a first-class blocker chain whose unresolved leaf issues are themselves healthy
 - an open explicit recovery action that names the owner and action needed to restore liveness
 
+A bounded review-path recovery for a task from a supported external-chat
+provider retains the source run's admitted message IDs. It does not inherit checkout or authorization
+markers. Before dispatch, Paperclip verifies the recovery run's task ownership
+and current conversation, endpoint, and principal access for every message.
+Missing message references or revoked access still prevent execution.
+
 ### Durable external waits and heartbeat finalization
 
 An external wait counts as a live or waiting path only when the next move survives the current heartbeat and is represented in Paperclip's durable control-plane state. Valid external-wait shapes are:

server/src/__tests__/heartbeat-reviewed-chat-binding.integration.test.ts

@@ -30,14 +30,16 @@ 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("reviewed external-chat execution binding", () => {
+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(),
@@ -49,7 +51,7 @@ describe("reviewed external-chat execution binding", () => {
     userId = "reviewed-chat-user";
   const context = {
     issueId,
-    source: "chat:discord",
+    source: `chat:${provider}`,
     wakeReason: "External chat message received",
     wakeCommentIds: [commentId],
     commentId,
@@ -93,6 +95,15 @@ describe("reviewed external-chat execution binding", () => {
         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({
@@ -132,7 +143,7 @@ describe("reviewed external-chat execution binding", () => {
       .values({
         id: applicationId,
         companyId,
-        applicationKey: `chat:discord:${endpointId}`,
+        applicationKey: `chat:${provider}:${endpointId}`,
         name: "Discord",
         type: "chat",
         status: "active",
@@ -144,7 +155,7 @@ describe("reviewed external-chat execution binding", () => {
         companyId,
         applicationId,
         name: "Discord",
-        uid: `chat-discord-${endpointId}`,
+        uid: `chat-${provider}-${endpointId}`,
         connectionPurpose: "channel",
         transport: "chat_sdk",
         status: "active",
@@ -156,7 +167,7 @@ describe("reviewed external-chat execution binding", () => {
         id: endpointId,
         companyId,
         connectionId,
-        provider: "discord",
+        provider,
         publicId: randomUUID(),
         assignedAgentId: agentId,
         status: "active",
@@ -193,7 +204,7 @@ describe("reviewed external-chat execution binding", () => {
       .values({
         id: principalId,
         companyId,
-        provider: "discord",
+        provider,
         providerAccountId: "guild-1",
         externalId: "user-1",
         kind: "user",
@@ -296,7 +307,7 @@ describe("reviewed external-chat execution binding", () => {
     expect(wake).toMatchObject({
       checkedOutByHarness: false,
       externalChatExecutionBound: true,
-      externalChatProvider: "discord",
+      externalChatProvider: provider,
     });
     const prompt = renderPaperclipWakePrompt(wake);
     expect(prompt).toContain("not a checkout, approval");
@@ -304,6 +315,84 @@ describe("reviewed external-chat execution binding", () => {
     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)
@@ -338,7 +427,7 @@ describe("reviewed external-chat execution binding", () => {
     } finally {
       await db
         .update(chatEndpoints)
-        .set({ provider: "discord" })
+        .set({ provider })
         .where(eq(chatEndpoints.id, endpointId));
     }
   });
@@ -493,7 +582,7 @@ describe("reviewed external-chat execution binding", () => {
       await expect(
         resolveCurrentWakeCommentsBinding(db, binding),
       ).resolves.toMatchObject({
-        provider: "discord",
+        provider,
         commentIds: [commentId],
       });
       await db

server/src/services/native-runtime/chat-attachment-reuse.ts

@@ -29,6 +29,7 @@ import {
 import { getStorageService } from "../../storage/index.js";
 import type { StorageService } from "../../storage/types.js";
 import { issueService } from "../issues.js";
+import { boundExternalChatProvider } from "./external-chat-provider.js";
 import { resolveExternalChatQuestionResponse } from "./external-chat-question-response.js";
 
 export const LIST_CHAT_ATTACHMENTS_TOOL_NAME = "list_chat_attachments";
@@ -418,17 +419,7 @@ export async function authorizeChatConversationForBoundRun(
     context = answer.authorizationContext;
   }
   const source = typeof context.source === "string" ? context.source : "";
-  const provider = [
-    "slack",
-    "github",
-    "discord",
-    "microsoft-teams",
-    "telegram",
-    "imessage-photon",
-  ].find(
-    (candidate) =>
-      source === `chat:${candidate}` || source === `chat:${candidate}:recovery`,
-  );
+  const provider = boundExternalChatProvider(source);
   const commentIds = wakeCommentIds(context);
   if (
     !provider ||
@@ -549,17 +540,7 @@ function externalChatWaitCandidate(
 ): { provider: string; commentIds: string[] } | null {
   const context = record(contextSnapshot);
   const source = typeof context.source === "string" ? context.source : "";
-  const provider = [
-    "slack",
-    "github",
-    "discord",
-    "microsoft-teams",
-    "telegram",
-    "imessage-photon",
-  ].find(
-    (candidate) =>
-      source === `chat:${candidate}` || source === `chat:${candidate}:recovery`,
-  );
+  const provider = boundExternalChatProvider(source);
   const commentIds = wakeCommentIds(context);
   const wake = record(context.paperclipWake);
   const wakeIssue = record(wake.issue);

server/src/services/native-runtime/external-chat-provider.ts

@@ -0,0 +1,16 @@
+// Providers supported by the run-bound external-chat authorization path.
+// AgentMail uses the email inbox path and is not admitted by this boundary.
+const BOUND_EXTERNAL_CHAT_PROVIDERS = [
+  "slack",
+  "github",
+  "discord",
+  "microsoft-teams",
+  "telegram",
+  "imessage-photon",
+] as const;
+
+export function boundExternalChatProvider(source: unknown) {
+  return BOUND_EXTERNAL_CHAT_PROVIDERS.find(
+    (provider) => source === `chat:${provider}` || source === `chat:${provider}:recovery`,
+  ) ?? null;
+}

server/src/services/recovery/review-path-recovery.test.ts

@@ -57,6 +57,88 @@ describe("review-path recovery", () => {
     expect(duplicate).toEqual({ kind: "skip", reason: "review-path recovery wake already exists" });
   });
 
+  it.each(["chat:slack", "chat:slack:recovery", "chat:discord"])(
+    "retains the admitted message batch for %s without inheriting authorization",
+    (source) => {
+      const decision = decideIssueReviewPathRecovery({
+        issueId: "issue-1",
+        sourceRunId: "run-1",
+        assigneeAgentId: "agent-1",
+        contextSnapshot: {
+          issueId: "issue-1",
+          source,
+          wakeCommentIds: ["comment-1", "comment-2", "comment-1"],
+          wakeCommentId: "stale-comment",
+          commentId: "stale-comment",
+          paperclipHarnessCheckedOut: true,
+          paperclipExternalChatExecutionBound: true,
+          paperclipWake: { checkedOutByHarness: true },
+          sessionId: "old-session",
+          instruction: "old instruction",
+        },
+        reviewAttention: stalled,
+        existingWake: false,
+      });
+      expect(decision.kind).toBe("enqueue");
+      if (decision.kind !== "enqueue") return;
+      expect(decision.contextSnapshot).toMatchObject({
+        issueId: "issue-1",
+        source,
+        wakeReason: ISSUE_REVIEW_PATH_LOST_WAKE_REASON,
+        wakeCommentIds: ["comment-1", "comment-2"],
+        wakeCommentId: "comment-2",
+        commentId: "comment-2",
+      });
+      for (const key of [
+        "paperclipHarnessCheckedOut",
+        "paperclipExternalChatExecutionBound",
+        "paperclipWake",
+        "sessionId",
+      ]) expect(decision.contextSnapshot).not.toHaveProperty(key);
+      expect(decision.contextSnapshot.instruction).not.toBe("old instruction");
+    },
+  );
+
+  it.each([undefined, null, "comment-1", [], [null, 1, " "]])(
+    "does not invent an admitted message batch from invalid input %j",
+    (wakeCommentIds) => {
+      const decision = decideIssueReviewPathRecovery({
+        issueId: "issue-1",
+        sourceRunId: "run-1",
+        assigneeAgentId: "agent-1",
+        contextSnapshot: {
+          source: "chat:slack",
+          wakeCommentIds,
+          wakeCommentId: "unproven-comment",
+        },
+        reviewAttention: stalled,
+        existingWake: false,
+      });
+      expect(decision.kind).toBe("enqueue");
+      if (decision.kind !== "enqueue") return;
+      expect(decision.contextSnapshot.source).toBe("chat:slack");
+      expect(decision.contextSnapshot).not.toHaveProperty("wakeCommentIds");
+      expect(decision.contextSnapshot).not.toHaveProperty("wakeCommentId");
+    },
+  );
+
+  it.each(["issue.comment", "chat:agentmail", "chat:agentmail:recovery"])(
+    "does not carry a chat batch for unsupported source %s",
+    (source) => {
+      const decision = decideIssueReviewPathRecovery({
+        issueId: "issue-1",
+        sourceRunId: "run-1",
+        assigneeAgentId: "agent-1",
+        contextSnapshot: { source, wakeCommentIds: ["comment-1"] },
+        reviewAttention: stalled,
+        existingWake: false,
+      });
+      expect(decision.kind).toBe("enqueue");
+      if (decision.kind !== "enqueue") return;
+      expect(decision.contextSnapshot).not.toHaveProperty("wakeCommentIds");
+    },
+  );
+
   it("does not requeue when the bounded recovery run also ends pathless", () => {
     const decision = decideIssueReviewPathRecovery({
       issueId: "issue-1",

server/src/services/recovery/review-path-recovery.ts

@@ -1,5 +1,7 @@
 import { createHash } from "node:crypto";
 import type { IssueReviewAttention } from "@paperclipai/shared";
+import { boundExternalChatProvider } from "../native-runtime/external-chat-provider.js";
+import { extractWakeCommentIds } from "../../modules/run-dispatch/index.js";
 import { withRecoveryContext } from "./status-only-context.js";
 
 export const ISSUE_REVIEW_PATH_LOST_WAKE_REASON = "issue_review_path_lost";
@@ -100,6 +102,9 @@ export function decideIssueReviewPathRecovery(input: {
   });
   if (input.existingWake) return { kind: "skip", reason: "review-path recovery wake already exists" };
 
+  const source = readNonEmptyString(context.source) ?? "heartbeat.review_path_disposition";
+  const chatCommentIds = boundExternalChatProvider(source) ? extractWakeCommentIds(context) : [];
+
   const payload = withRecoveryContext({
     issueId: input.issueId,
     taskId: input.issueId,
@@ -119,7 +124,15 @@ export function decideIssueReviewPathRecovery(input: {
     contextSnapshot: withRecoveryContext({
       ...payload,
       wakeReason: ISSUE_REVIEW_PATH_LOST_WAKE_REASON,
-      source: readNonEmptyString(context.source) ?? "heartbeat.review_path_disposition",
+      source,
+      // Keep the admitted message references, not the source run's authority.
+      // Dispatch must prove the new run owns this task and recheck current
+      // endpoint, conversation, and principal access for the entire batch.
+      ...(chatCommentIds.length > 0 ? {
+        wakeCommentIds: chatCommentIds,
+        wakeCommentId: chatCommentIds.at(-1),
+        commentId: chatCommentIds.at(-1),
+      } : {}),
     }, "normal_model"),
   };
 }