From ee29e2850d0edc6cf11be9a8166a7829bce9c311 Mon Sep 17 00:00:00 2001 From: PodWarden Date: Fri, 23 Oct 2026 20:06:16 -0700 Subject: [PATCH] fix: keep GitHub guest provenance from failing tool discovery (#17340) 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 reach their tools through the MCP tool gateway. Every run starts with `tools/list`. > - Tool discovery runs a GitHub guest sponsor probe (`githubGuestBotConnectionForSession`) before it lists any connection. > - The probe walked the run's identity chain before it knew whether the task had a guest sponsored GitHub message at all. > - When agent A works a task and the task is reassigned to agent B, B's run chain continues A's run. The chain walk refuses with a plain `HttpError`. The gateway returns 403 for `tools/list` with no audit record. B loses every tool, built-in and connected, even when the company has no GitHub endpoint. > - This pull request makes the probe look up guest sponsored GitHub messages for the task and agent first. When there are none, the probe answers `null` and does not consult provenance. > - The benefit is that a reassigned or continued task keeps its tools, while a guest sponsored task still refuses a broken chain. ## Linked Issues or Issue Description Fixes #17307 **What happened?** `tools/list` on the MCP gateway returned `403 {"error":"Chat tools cannot be inherited by another task"}` for every run whose identity chain contained a run of another agent or another task. No audit event was recorded. The agent received no tools at all, including unrelated MCP connections. The company had no GitHub chat endpoint. **Expected behavior** A run on a task with no GitHub guest sponsor lists its tools. The guest probe only decides whether the run must be limited to the sponsoring bot's connection. When the task has no guest sponsored GitHub message, the answer is "no sponsor" and discovery continues. **Steps to reproduce** 1. Let agent A run on a task. The run gets an accepted identity context. 2. Reassign the task to agent B and let B run. B's identity context has A's context as parent. 3. Call `tools/list` for B's run. Before this change the gateway returns 403 with the message above. The same happens when the parent context belongs to a run on another task (see #17307). **Call path** `handleMcpGatewayProtocol` → `listToolsForNamedGateway` → `listToolsForContext` → `buildToolsForContext` → `connectionToolsForContext` → `githubGuestBotConnectionForSession` → `chatRunSourceMessageId`, which throws `forbidden(...)`. The error is an `HttpError`, not a `ToolGatewayHttpError`, so the route sends it as a bare 403 and the discovery audit never runs. **Paperclip version or commit** `d69631b01b9c658f80bf8bed389863dce82643e1` (image `sha-d69631b`), Docker, authenticated mode with PostgreSQL. ## What Changed - `server/src/services/chat-github-tools.ts`: `githubGuestBotConnectionForSession` first selects the inbound GitHub message links on the task whose delivery carries `githubAuthority.guest = true`, for endpoints assigned to the session agent. When there are none it returns `null` without reading the run or its provenance chain. When there are some, it verifies provenance as before and returns the connection of the link that matches the verified source message. - `server/src/services/chat-channels/run-origin.ts` is unchanged. The cross-agent and cross-task refusals stay in place for Slack authority, GitHub reviews and response comments, and for guest sponsored tasks. - New test `server/src/services/chat-github-tools.test.ts` (embedded PostgreSQL, same pattern as `run-origin.test.ts`): - a run whose chain continues another agent, or another task, with no guest message → `null` (fails before this change with the 403 above, passes after); - a guest sponsored task whose chain continues another agent → still rejects with status 403; - a run sponsored by a GitHub guest → returns the bot connection; the same run with `guest: false` → `null`. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/services/chat-github-tools.test.ts`: 2 failed / 3 passed before the fix, 5 passed after. - `pnpm --filter @paperclipai/server exec vitest run src/services/chat-channels/run-origin.test.ts src/services/chat-github-tools.test.ts`: all passed. - `pnpm --filter @paperclipai/plugin-sdk ensure-build-deps && pnpm --filter @paperclipai/server exec tsc --noEmit`: clean. - Manual: with a patched image, a run on a task that was reassigned between two agents receives its full tool list through `tools/list` and a `discovery` audit event is written. Before the patch the same kind of run received the 403 and no audit event. ## Risks - Low risk. The probe's result is unchanged for every session where it returned a value before. The only behavior change is for sessions that previously threw: when the task has no guest sponsored GitHub message, they now get `null`, which is the value the old code could only have returned for them anyway, because the final match also required a guest link on this task for this agent. - One extra query per discovery on guest sponsored tasks. On every other session the run and provenance queries are skipped. ## Model Used - Claude (Anthropic), Claude Code agent session; model id `claude-fable-5-1`, extended thinking, tool use (shell, git, vitest with embedded PostgreSQL). ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LoF4rrmx35JJqbhEpaLQ9Q --------- Co-authored-by: Ingvar Petrov Co-authored-by: Claude Opus 5.5 (1M context) --- server/src/services/chat-github-tools.test.ts | 97 +++++++++++++++++++ server/src/services/chat-github-tools.ts | 60 ++++++------ 2 files changed, 126 insertions(+), 31 deletions(-) create mode 100644 server/src/services/chat-github-tools.test.ts diff --git a/server/src/services/chat-github-tools.test.ts b/server/src/services/chat-github-tools.test.ts new file mode 100644 index 00000000000..67165c1bb8c --- /dev/null +++ b/server/src/services/chat-github-tools.test.ts @@ -0,0 +1,97 @@ +import { randomUUID } from "node:crypto"; +import { eq } from "drizzle-orm"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { + agents, + chatConversations, + chatDeliveries, + chatEndpoints, + chatMessageLinks, + companies, + createDb, + heartbeatRuns, + issueComments, + issues, + runIdentityContexts, + toolApplications, + toolConnections, +} from "@paperclipai/db"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "../__tests__/helpers/embedded-postgres.js"; +import { githubGuestBotConnectionForSession } from "./chat-github-tools.js"; + +const support = await getEmbeddedPostgresTestSupport(); +(support.supported ? describe : describe.skip)("GitHub guest bot connection discovery", () => { + let database: Awaited>; + let db: ReturnType; + + beforeAll(async () => { + database = await startEmbeddedPostgresTestDatabase("paperclip-github-guest-"); + db = createDb(database.connectionString); + }, 30_000); + afterAll(async () => { await database?.cleanup(); }, 60_000); + + async function fixture() { + const [company] = await db.insert(companies).values({ name: "Guest discovery", issuePrefix: randomUUID().slice(0, 8) }).returning(); + const [first, second] = await db.insert(agents).values([ + { companyId: company.id, name: "First agent", role: "general", adapterType: "codex_local" }, + { companyId: company.id, name: "Second agent", role: "general", adapterType: "codex_local" }, + ]).returning(); + const [issue, other] = await db.insert(issues).values([ + { companyId: company.id, title: "Reassigned task", assigneeAgentId: second.id }, + { companyId: company.id, title: "Other task", assigneeAgentId: second.id }, + ]).returning(); + return { company, first, second, issue, other }; + } + + async function continuation(f: Awaited>, parent: { agentId: string; issueId: string }) { + const [source, continued] = await db.insert(heartbeatRuns).values([ + { companyId: f.company.id, agentId: parent.agentId, status: "succeeded", nativeIssueId: parent.issueId, contextSnapshot: {}, responsibleUserId: "requester" }, + { companyId: f.company.id, agentId: f.second.id, status: "running", nativeIssueId: f.issue.id, contextSnapshot: {}, responsibleUserId: "requester" }, + ]).returning(); + const [origin] = await db.insert(runIdentityContexts).values({ companyId: f.company.id, runId: source.id, revision: 1, responsibleUserId: "requester", cause: "dispatch", correlationId: "dispatch" }).returning(); + const [context] = await db.insert(runIdentityContexts).values({ companyId: f.company.id, runId: continued.id, revision: 1, responsibleUserId: "requester", parentContextId: origin.id, cause: "dispatch", correlationId: "dispatch" }).returning(); + await db.update(heartbeatRuns).set({ activeIdentityContextId: context.id }).where(eq(heartbeatRuns.id, continued.id)); + return { companyId: f.company.id, agentId: f.second.id, runId: continued.id, issueId: f.issue.id }; + } + + async function guestSource(f: Awaited>, guest: boolean) { + const endpointId = randomUUID(); + const [application] = await db.insert(toolApplications).values({ companyId: f.company.id, applicationKey: `chat:github:${endpointId}`, name: `GitHub ${endpointId}`, type: "chat", status: "active" }).returning(); + const [connection] = await db.insert(toolConnections).values({ companyId: f.company.id, applicationId: application.id, name: "GitHub", uid: `chat-github-${endpointId}`, connectionPurpose: "channel", transport: "chat_sdk", status: "active", enabled: true }).returning(); + await db.insert(chatEndpoints).values({ id: endpointId, companyId: f.company.id, connectionId: connection.id, provider: "github", publicId: randomUUID(), assignedAgentId: f.second.id, status: "active" }); + const [conversation] = await db.insert(chatConversations).values({ companyId: f.company.id, endpointId, issueId: f.issue.id, externalConversationId: `github:owner/repo:${endpointId}`, externalLabel: "Guest thread" }).returning(); + const [comment] = await db.insert(issueComments).values({ companyId: f.company.id, issueId: f.issue.id, authorType: "user", authorUserId: "requester", body: "Please review" }).returning(); + const [delivery] = await db.insert(chatDeliveries).values({ companyId: f.company.id, endpointId, conversationId: conversation.id, providerEventId: randomUUID(), deduplicationKey: randomUUID(), eventKind: "message", normalizedEvent: { githubAuthority: { guest } }, state: "processed" }).returning(); + await db.insert(chatMessageLinks).values({ companyId: f.company.id, endpointId, conversationId: conversation.id, deliveryId: delivery.id, commentId: comment.id, providerMessageId: randomUUID(), direction: "inbound" }); + return { connectionId: connection.id, commentId: comment.id }; + } + + async function sponsoredRun(f: Awaited>, commentId: string) { + const [run] = await db.insert(heartbeatRuns).values({ companyId: f.company.id, agentId: f.second.id, status: "running", nativeIssueId: f.issue.id, contextSnapshot: {}, responsibleUserId: "requester" }).returning(); + const [context] = await db.insert(runIdentityContexts).values({ companyId: f.company.id, runId: run.id, revision: 1, responsibleUserId: "requester", messageId: commentId, cause: "instruction", correlationId: "message" }).returning(); + await db.update(heartbeatRuns).set({ activeIdentityContextId: context.id }).where(eq(heartbeatRuns.id, run.id)); + return { companyId: f.company.id, agentId: f.second.id, runId: run.id, issueId: f.issue.id }; + } + + it.each(["agent", "task"])("finds no guest sponsor when the run chain continues another %s", async (fault) => { + const f = await fixture(); + const session = await continuation(f, fault === "agent" + ? { agentId: f.first.id, issueId: f.issue.id } + : { agentId: f.second.id, issueId: f.other.id }); + expect(await githubGuestBotConnectionForSession(db, session)).toBeNull(); + }); + + it("still refuses a guest-sponsored task whose run chain continues another agent", async () => { + const f = await fixture(); + await guestSource(f, true); + const session = await continuation(f, { agentId: f.first.id, issueId: f.issue.id }); + await expect(githubGuestBotConnectionForSession(db, session)).rejects.toMatchObject({ status: 403 }); + }); + + it.each([true, false])("resolves the bot connection only for a run sponsored by a GitHub guest (guest=%s)", async (guest) => { + const f = await fixture(); + const source = await guestSource(f, guest); + const session = await sponsoredRun(f, source.commentId); + expect(await githubGuestBotConnectionForSession(db, session)).toBe(guest ? source.connectionId : null); + }); +}); diff --git a/server/src/services/chat-github-tools.ts b/server/src/services/chat-github-tools.ts index f87a9f1c663..a221016caf4 100644 --- a/server/src/services/chat-github-tools.ts +++ b/server/src/services/chat-github-tools.ts @@ -385,6 +385,33 @@ export async function githubGuestBotConnectionForSession( >, ): Promise { if (!session.runId || !session.agentId || !session.issueId) return null; + const guestSource = (commentId?: string) => db + .select({ connectionId: chatEndpoints.connectionId }) + .from(chatMessageLinks) + .innerJoin( + chatDeliveries, + eq(chatDeliveries.id, chatMessageLinks.deliveryId), + ) + .innerJoin( + chatConversations, + eq(chatConversations.id, chatMessageLinks.conversationId), + ) + .innerJoin( + chatEndpoints, + eq(chatEndpoints.id, chatConversations.endpointId), + ) + .where( + and( + eq(chatMessageLinks.companyId, session.companyId), + ...(commentId ? [eq(chatMessageLinks.commentId, commentId)] : []), + eq(chatConversations.issueId, session.issueId!), + eq(chatEndpoints.assignedAgentId, session.agentId!), + eq(chatEndpoints.provider, "github"), + sql`${chatDeliveries.normalizedEvent} -> 'githubAuthority' ->> 'guest' = 'true'`, + ), + ) + .limit(1); + if (!(await guestSource()).length) return null; const [run] = await db .select({ id: heartbeatRuns.id, @@ -406,35 +433,6 @@ export async function githubGuestBotConnectionForSession( ); const commentId = run ? await chatRunSourceMessageId(db, run, { allowLegacyWakeComment: true, provider: "github" }) : null; if (typeof commentId !== "string") return null; - const [source] = await db - .select({ - event: chatDeliveries.normalizedEvent, - connectionId: chatEndpoints.connectionId, - }) - .from(chatMessageLinks) - .innerJoin( - chatDeliveries, - eq(chatDeliveries.id, chatMessageLinks.deliveryId), - ) - .innerJoin( - chatConversations, - eq(chatConversations.id, chatMessageLinks.conversationId), - ) - .innerJoin( - chatEndpoints, - eq(chatEndpoints.id, chatConversations.endpointId), - ) - .where( - and( - eq(chatMessageLinks.companyId, session.companyId), - eq(chatMessageLinks.commentId, commentId), - eq(chatConversations.issueId, session.issueId), - eq(chatEndpoints.assignedAgentId, session.agentId), - eq(chatEndpoints.provider, "github"), - ), - ); - return (source?.event.githubAuthority as { guest?: boolean } | undefined) - ?.guest === true - ? source!.connectionId - : null; + const [source] = await guestSource(commentId); + return source?.connectionId ?? null; }