diff --git a/doc/connections/GITHUB.md b/doc/connections/GITHUB.md index eb894ce016..a85a062c5b 100644 --- a/doc/connections/GITHUB.md +++ b/doc/connections/GITHUB.md @@ -69,15 +69,26 @@ installation-health failure, not as token expiry. ## Repository access -OAuth completion verifies `/user`, `/user/installations`, and each -installation's accessible repository count. Setup remains incomplete until at -least one installation and repository are available. Paperclip stores user and -installation summaries, not a repository-name cache. GitHub stays authoritative: -removed repository access fails immediately even if a displayed count is stale. +OAuth completion verifies `/user`, every page of `/user/installations`, and every +page of each installation's accessible repositories. Setup remains incomplete +until at least one installation and repository are available. Paperclip stores +the authenticated username and a grant-scoped display snapshot containing only +repository IDs, full names, and installation IDs. GitHub stays authoritative: +this snapshot never authorizes repository access. -The Apps UI links to GitHub's installation management page and offers -**Refresh access**. Selected repositories are recommended. Choosing all -repositories requires an explicit warning in setup. +The permissions page shows the authenticated GitHub account and the complete +accessible repository list. **Refresh access** reloads it from GitHub. Older +grants and grants invalidated by newer installation lifecycle events prompt for a +refresh instead of presenting a stale list. The page links to GitHub's +installation management page. Selected repositories are recommended; all- +repository access retains its warning. + +Fresh local test-drives use production Paperclip Cloud. Instance enrollment +and provider enablement are separate: enrollment alone does not enable GitHub +OAuth. Production must advertise the `github.code` profile (see Cloud's +`docs/github-connector-deploy-bootstrap.md`). If it is unavailable, setup +preserves the sign-in intent and offers a retry instead of silently switching +to a personal access token. A successful retry preserves the chosen audience. ## Webhooks diff --git a/packages/db/src/schema/tool_access.ts b/packages/db/src/schema/tool_access.ts index 939921e26d..59261f2241 100644 --- a/packages/db/src/schema/tool_access.ts +++ b/packages/db/src/schema/tool_access.ts @@ -194,9 +194,12 @@ export const connectionGrants = pgTable( repositorySelection: "all" | "selected" | "mixed" | "none"; installationIds: string[]; installationOwnerLogins: string[]; + /** Repository metadata visible to this credential; refreshed from GitHub. */ + repositories?: Array<{ id: string; fullName: string; installationId: string }>; installationUrl?: string; managementUrl?: string; appSlug?: string; + accessRevision?: string; lastAccessRefreshAt?: string; lastWebhookAt?: string; webhookHealth?: "pending" | "healthy" | "unhealthy"; diff --git a/packages/shared/src/types/tool-access.ts b/packages/shared/src/types/tool-access.ts index e2b288ae8d..8bb22a496c 100644 --- a/packages/shared/src/types/tool-access.ts +++ b/packages/shared/src/types/tool-access.ts @@ -224,9 +224,12 @@ export interface ConnectionGrant { repositorySelection: "all" | "selected" | "mixed" | "none"; installationIds: string[]; installationOwnerLogins: string[]; + /** Repository metadata visible to this credential; refreshed from GitHub. */ + repositories?: Array<{ id: string; fullName: string; installationId: string }>; installationUrl?: string; managementUrl?: string; appSlug?: string; + accessRevision?: string; lastAccessRefreshAt?: string; lastWebhookAt?: string; webhookHealth?: "pending" | "healthy" | "unhealthy"; diff --git a/server/src/__tests__/github-connection-events.test.ts b/server/src/__tests__/github-connection-events.test.ts index f8a12ece61..aef6432061 100644 --- a/server/src/__tests__/github-connection-events.test.ts +++ b/server/src/__tests__/github-connection-events.test.ts @@ -92,6 +92,7 @@ describeEmbeddedPostgres.sequential("GitHub connection event delivery", () => { repositorySelection: "selected", installationIds: ["101"], installationOwnerLogins: ["paperclipai"], + repositories: [{ id: "203", fullName: "paperclipai/removed", installationId: "101" }], webhookHealth: "pending", }, }, @@ -206,7 +207,7 @@ describeEmbeddedPostgres.sequential("GitHub connection event delivery", () => { expect(connector.acknowledgeEvents).toHaveBeenCalledTimes(2); }); - it("applies installation repository deltas transactionally and never reapplies a processed delivery", async () => { + it.each([false, true])("applies installation events once without discarding newer verified access (refreshed: %s)", async (refreshed) => { const companyId = randomUUID(); const applicationId = randomUUID(); const connectionId = randomUUID(); @@ -252,6 +253,7 @@ describeEmbeddedPostgres.sequential("GitHub connection event delivery", () => { repositorySelection: "selected", installationIds: ["101"], installationOwnerLogins: ["paperclipai"], + repositories: [{ id: "203", fullName: "paperclipai/removed", installationId: "101" }], webhookHealth: "pending", }, }, @@ -280,7 +282,16 @@ describeEmbeddedPostgres.sequential("GitHub connection event delivery", () => { refresh: vi.fn(), revoke: vi.fn(), setWebhookBinding: vi.fn(async () => undefined), - leaseEvents: vi.fn(async () => ({ leaseId: `lease-${++poll}`, events: [leasedEvent] })), + leaseEvents: vi.fn(async () => { + if (refreshed) { + const [latest] = await db.select().from(connectionGrants).where(eq(connectionGrants.id, grantId)); + await db.update(connectionGrants).set({ providerTenant: { + ...latest!.providerTenant, + github: { ...latest!.providerTenant!.github!, lastAccessRefreshAt: "2026-09-04T12:00:02.000Z" }, + } }).where(eq(connectionGrants.id, grantId)); + } + return ({ leaseId: `lease-${++poll}`, events: [leasedEvent] }); + }), acknowledgeEvents: vi.fn(async () => 1), } as unknown as PaperclipCloudConnector; let currentTime = new Date("2026-09-04T12:00:05.000Z"); @@ -292,12 +303,17 @@ describeEmbeddedPostgres.sequential("GitHub connection event delivery", () => { await expect(service.pollOnce()).resolves.toMatchObject({ processed: 1, duplicate: 0, failed: 0 }); unsubscribe(); let [grant] = await db.select().from(connectionGrants).where(eq(connectionGrants.id, grantId)); - expect(grant?.providerTenant?.github).toMatchObject({ repositoryCount: 4, webhookHealth: "healthy" }); + expect(grant?.providerTenant?.github).toMatchObject({ repositoryCount: refreshed ? 3 : 4, webhookHealth: "healthy" }); + if (refreshed) { + expect(grant?.providerTenant?.github?.repositories).toHaveLength(1); + } else { + expect(grant?.providerTenant?.github?.repositories).toBeUndefined(); + } currentTime = new Date(currentTime.getTime() + 6_000); await expect(service.pollOnce()).resolves.toMatchObject({ processed: 0, duplicate: 1, failed: 0 }); [grant] = await db.select().from(connectionGrants).where(eq(connectionGrants.id, grantId)); - expect(grant?.providerTenant?.github?.repositoryCount).toBe(4); + expect(grant?.providerTenant?.github?.repositoryCount).toBe(refreshed ? 3 : 4); const [receipt] = await db.select().from(connectionEventDeliveries).where(eq( connectionEventDeliveries.providerDeliveryId, leasedEvent.id, diff --git a/server/src/__tests__/github-grant-metadata.test.ts b/server/src/__tests__/github-grant-metadata.test.ts index 35e5d01671..b77976f337 100644 --- a/server/src/__tests__/github-grant-metadata.test.ts +++ b/server/src/__tests__/github-grant-metadata.test.ts @@ -1,51 +1,62 @@ import { describe, expect, it, vi } from "vitest"; import { loadGitHubGrantMetadata } from "../services/tool-access.js"; -function json(value: unknown, status = 200): Response { +function json(value: unknown, next = false): Response { return new Response(JSON.stringify(value), { - status, - headers: { "content-type": "application/json" }, + headers: { + "content-type": "application/json", + ...(next ? { link: '; rel="next"' } : {}), + }, }); } describe("GitHub grant metadata", () => { - it("keeps only user and installation summaries while counting accessible repositories", async () => { + it("lists every page of installations and repositories, persisting only display metadata", async () => { const request = vi.fn(async (input) => { - const url = String(input); - if (url.endsWith("/user")) return json({ id: 42, login: "octocat", avatar_url: "https://avatars.example/octocat" }); - if (url.includes("/user/installations?")) { - return json({ - installations: [ - { id: 101, repository_selection: "selected", html_url: "https://github.com/settings/installations/101", account: { login: "paperclipai" } }, - { id: 102, repository_selection: "all", account: { login: "octocat" } }, - ], - }); + const url = new URL(String(input)); + const secondPage = url.searchParams.get("page") === "2"; + if (url.pathname === "/user") return json({ id: 42, login: "octocat", avatar_url: "https://avatars.example/octocat" }); + if (url.pathname === "/user/installations") { + return json({ installations: secondPage + ? [{ id: 102, repository_selection: "all", account: { login: "octocat" } }] + : [{ id: 101, repository_selection: "selected", html_url: "https://github.com/settings/installations/101", account: { login: "paperclipai" } }], + }, !secondPage); } - if (url.includes("/user/installations/101/repositories")) { - return json({ total_count: 2, repositories: [{ full_name: "paperclipai/private-name-must-not-persist" }] }); + if (url.pathname === "/user/installations/101/repositories") { + return json({ total_count: 2, repositories: secondPage + ? [{ id: 2, full_name: "paperclipai/b", description: "must-not-persist", clone_url: "must-not-persist" }] + : [{ id: 1, full_name: "paperclipai/a" }], + }, !secondPage); } - if (url.includes("/user/installations/102/repositories")) return json({ total_count: 5 }); - return json({}, 404); + if (url.pathname === "/user/installations/102/repositories") { + return json({ total_count: 1, repositories: [{ id: 3, full_name: "octocat/c" }] }); + } + throw new Error(`Unexpected GitHub path: ${url.pathname}`); }); const metadata = await loadGitHubGrantMetadata("ghu_secret", request, "paperclip-development"); - expect(metadata).toMatchObject({ userId: "42", login: "octocat", installationCount: 2, - repositoryCount: 7, + repositoryCount: 3, repositorySelection: "mixed", installationIds: ["101", "102"], installationOwnerLogins: ["paperclipai", "octocat"], + repositories: [ + { id: "3", fullName: "octocat/c", installationId: "102" }, + { id: "1", fullName: "paperclipai/a", installationId: "101" }, + { id: "2", fullName: "paperclipai/b", installationId: "101" }, + ], installationUrl: "https://github.com/apps/paperclip-development/installations/new", managementUrl: "https://github.com/settings/installations/101", appSlug: "paperclip-development", webhookHealth: "pending", }); - expect(JSON.stringify(metadata)).not.toContain("private-name-must-not-persist"); - expect(request).toHaveBeenCalledTimes(4); - for (const [, init] of request.mock.calls) { + expect(JSON.stringify(metadata)).not.toContain("must-not-persist"); + expect(request).toHaveBeenCalledTimes(6); + for (const [input, init] of request.mock.calls) { + expect(new URL(String(input)).origin).toBe("https://api.github.com"); expect(new Headers(init?.headers).get("authorization")).toBe("Bearer ghu_secret"); } }); @@ -54,9 +65,19 @@ describe("GitHub grant metadata", () => { const request = vi.fn(async (input) => String(input).endsWith("/user") ? json({ id: 42, login: "octocat" }) : json({ installations: [] })); - await expect(loadGitHubGrantMetadata("ghu_secret", request)).rejects.toMatchObject({ details: expect.objectContaining({ code: "github_installation_required" }), }); }); + + it("does not report a partial repository list when a later page fails", async () => { + const request = vi.fn() + .mockResolvedValueOnce(json({ id: 42, login: "octocat" })) + .mockResolvedValueOnce(json({ installations: [{ id: 101, repository_selection: "selected" }] })) + .mockResolvedValueOnce(json({ repositories: [{ id: 1, full_name: "octocat/a" }] }, true)) + .mockResolvedValueOnce(new Response("Unavailable", { status: 503 })); + await expect(loadGitHubGrantMetadata("ghu_secret", request)).rejects.toMatchObject({ + details: expect.objectContaining({ code: "github_access_check_failed" }), + }); + }); }); diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index a5709c0fd2..62150dc800 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -5103,7 +5103,7 @@ describeEmbeddedPostgres("tool access service", () => { } }, 15_000); - it("binds a non-expiring managed GitHub identity and installation to one agent", async () => { + it.each(["none", "event", "same-time-refresh"])("binds a managed GitHub identity and protects refresh from concurrent access changes (%s)", async (concurrentChange) => { const company = await createCompany(db); const userId = `github-manager-${randomUUID()}`; await grantBoardUser(db, company.id, userId, [], "owner"); @@ -5114,6 +5114,7 @@ describeEmbeddedPostgres("tool access service", () => { const githubDefinition = getConnectableAppDefinition("github")!; const previousOwnershipAvailability = githubDefinition.ownershipAvailability; githubDefinition.ownershipAvailability = { ...previousOwnershipAvailability, platform_shared: true }; + let beforeRepositoryResponse = async () => {}; vi.spyOn(globalThis, "fetch").mockImplementation(async (url) => { const href = String(url); if (href === "https://api.github.com/user") { @@ -5128,7 +5129,8 @@ describeEmbeddedPostgres("tool access service", () => { }] }); } if (href.includes("https://api.github.com/user/installations/101/repositories?")) { - return mcpHttpResponse({ total_count: 3, repositories: [{ full_name: "paperclipai/do-not-store" }] }); + await beforeRepositoryResponse(); + return mcpHttpResponse({ total_count: 3, repositories: [1, 2, 3].map((id) => ({ id, full_name: `paperclipai/repo-${id}`, description: "do-not-store" })) }); } if (href === GITHUB_CONNECTOR_PROFILES["github.code"].serverUrl) { return mcpHttpResponse({ @@ -5226,6 +5228,33 @@ describeEmbeddedPostgres("tool access service", () => { eq(toolConnectionInstalls.targetType, "agent"), eq(toolConnectionInstalls.targetId, agent.id), ))).resolves.toHaveLength(1); + vi.mocked(connector.setWebhookBinding).mockClear(); + if (concurrentChange !== "none") { + beforeRepositoryResponse = async () => { + const [latest] = await db.select().from(connectionGrants).where(eq(connectionGrants.id, grant!.id)); + await db.update(connectionGrants).set({ providerTenant: { + ...latest!.providerTenant, + github: { + ...latest!.providerTenant!.github!, + accessRevision: randomUUID(), + // Simulate a refresh with identical timestamps, so only the unique + // access revision can distinguish its newer access snapshot. + ...(concurrentChange === "event" ? { lastWebhookAt: new Date().toISOString() } : {}), + installationIds: [], installationCount: 0, repositoryCount: 0, + repositorySelection: "none", repositories: undefined, webhookHealth: "unhealthy", + }, + } }).where(eq(connectionGrants.id, grant!.id)); + }; + await expect(service.checkHealth(connected.connectionId, actor)) + .rejects.toThrow("GitHub access changed during refresh. Try again."); + const [latest] = await db.select().from(connectionGrants).where(eq(connectionGrants.id, grant!.id)); + expect(latest?.providerTenant?.github).toMatchObject({ installationIds: [], repositoryCount: 0, webhookHealth: "unhealthy" }); + expect(latest?.providerTenant?.github?.repositories).toBeUndefined(); + expect(connector.setWebhookBinding).not.toHaveBeenCalled(); + } else { + await expect(service.checkHealth(connected.connectionId, actor)).resolves.toMatchObject({ connection: { healthStatus: "ok" } }); + expect(connector.setWebhookBinding).toHaveBeenCalled(); + } } finally { githubDefinition.ownershipAvailability = previousOwnershipAvailability; } @@ -5261,7 +5290,7 @@ describeEmbeddedPostgres("tool access service", () => { }] }); } if (href.includes("https://api.github.com/user/installations/101/repositories?")) { - return mcpHttpResponse({ total_count: 1, repositories: [] }); + return mcpHttpResponse({ total_count: 1, repositories: [{ id: 1, full_name: "paperclipai/repo-1" }] }); } if (href === GITHUB_CONNECTOR_PROFILES["github.code"].serverUrl) { return mcpHttpResponse({ diff --git a/server/src/services/github-connection-events.ts b/server/src/services/github-connection-events.ts index 22c27c9579..177492ea69 100644 --- a/server/src/services/github-connection-events.ts +++ b/server/src/services/github-connection-events.ts @@ -1,3 +1,4 @@ +import { randomUUID } from "node:crypto"; import { connectionEventDeliveries, connectionGrants, @@ -269,7 +270,28 @@ export function githubConnectionEventService( } async function applyInstallationEvent(database: Db, binding: GitHubBinding, event: LeasedEvent) { - const github = binding.providerTenant.github!; + // Bindings are loaded before the Cloud request. Lock and read the grant + // again so a refresh completed during that request cannot be overwritten. + const [currentGrant] = await database.select().from(connectionGrants).where(and( + eq(connectionGrants.id, binding.grantId), + eq(connectionGrants.companyId, binding.companyId), + eq(connectionGrants.status, "active"), + )).for("update").limit(1); + const currentProviderTenant = currentGrant?.providerTenant; + const github = currentProviderTenant?.github; + if (!github) return; + // A newly bound instance can receive installation events from before OAuth + // verified its repository list. Those events must not erase newer access. + if (Date.parse(github.lastAccessRefreshAt ?? "") > Date.parse(event.createdAt)) { + await database.update(connectionGrants).set({ + providerTenant: { + ...currentProviderTenant, + github: { ...github, lastWebhookAt: now().toISOString(), webhookHealth: "healthy" }, + }, + updatedAt: now(), + }).where(and(eq(connectionGrants.id, binding.grantId), eq(connectionGrants.companyId, binding.companyId))); + return; + } const unavailable = event.event === "installation" && (event.action === "deleted" || event.action === "suspend"); const installationIds = unavailable ? github.installationIds.filter((id) => id !== binding.installationId) @@ -285,9 +307,13 @@ export function githubConnectionEventService( : github.repositoryCount + added - removed, ); const providerTenant = { - ...binding.providerTenant, + ...currentProviderTenant, github: { ...github, + // Lifecycle webhooks carry IDs, not the user token’s complete repository view. + // Discard the snapshot until Refresh access verifies it again. + accessRevision: randomUUID(), + repositories: undefined, installationIds, installationCount: installationIds.length, repositoryCount, @@ -353,12 +379,19 @@ export function githubConnectionEventService( const github = binding.providerTenant.github; if (!github) continue; await database.update(connectionGrants).set({ - providerTenant: { - ...binding.providerTenant, - github: { ...github, lastWebhookAt: touchedAt.toISOString(), webhookHealth: "healthy" }, - }, + // Update only webhook fields; a concurrent access/token refresh + // owns the remaining metadata and must not be replaced here. + providerTenant: sql`jsonb_set(${connectionGrants.providerTenant}, '{github}', + (${connectionGrants.providerTenant}->'github') || ${JSON.stringify({ + lastWebhookAt: touchedAt.toISOString(), webhookHealth: "healthy", + })}::jsonb)`, updatedAt: touchedAt, - }).where(and(eq(connectionGrants.id, binding.grantId), eq(connectionGrants.companyId, companyId))); + }).where(and( + eq(connectionGrants.id, binding.grantId), + eq(connectionGrants.companyId, companyId), + eq(connectionGrants.status, "active"), + sql`${connectionGrants.providerTenant}->'github' is not null`, + )); } } const finishedAt = now(); diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index 284b3510f2..d117e6f6a9 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -1942,13 +1942,16 @@ export async function loadGitHubGrantMetadata( repositorySelection: "all" | "selected" | "mixed" | "none"; installationIds: string[]; installationOwnerLogins: string[]; + repositories: Array<{ id: string; fullName: string; installationId: string }>; installationUrl: string; managementUrl: string; appSlug?: string; + accessRevision: string; lastAccessRefreshAt: string; webhookHealth: "pending"; }> { - const github = async (path: string): Promise> => { + const accessRefreshStartedAt = new Date().toISOString(); + const github = async (path: string): Promise<{ data: Record; hasNext: boolean }> => { const response = await request(`https://api.github.com${path}`, { headers: { accept: "application/vnd.github+json", @@ -1965,21 +1968,30 @@ export async function loadGitHubGrantMetadata( } const value = await response.json() as unknown; if (!recordValue(value)) throw unprocessable("GitHub returned invalid account metadata", { code: "github_bad_response" }); - return value; + return { data: value, hasNext: /;\s*rel="next"/.test(response.headers.get("link") ?? "") }; }; - const user = await github("/user"); + const list = async (path: string, key: string): Promise[]> => { + const items: Record[] = []; + for (let page = 1; ; page += 1) { + const { data, hasNext } = await github(`${path}?per_page=100&page=${page}`); + const batch = data[key]; + if (!Array.isArray(batch) || !batch.every(recordValue) || (hasNext && batch.length === 0)) { + throw unprocessable("GitHub returned invalid access metadata", { code: "github_bad_response" }); + } + items.push(...batch); + if (!hasNext) return items; + } + }; + const { data: user } = await github("/user"); const userId = githubId(user.id); const login = typeof user.login === "string" ? user.login : null; if (!userId || !login) throw unprocessable("GitHub returned invalid account metadata", { code: "github_bad_response" }); - const installationsResponse = await github("/user/installations?per_page=100"); - const installations = Array.isArray(installationsResponse.installations) - ? installationsResponse.installations.filter(recordValue).slice(0, 100) - : []; + const installations = await list("/user/installations", "installations"); const installationIds: string[] = []; const owners = new Set(); const selections = new Set<"all" | "selected">(); const managementUrls = new Set(); - let repositoryCount = 0; + const repositories = new Map(); for (const installation of installations) { const installationId = githubId(installation.id); if (!installationId) continue; @@ -1991,11 +2003,16 @@ export async function loadGitHubGrantMetadata( if (typeof account?.login === "string") owners.add(account.login); const managementUrl = githubInstallationManagementUrl(installation.html_url); if (managementUrl) managementUrls.add(managementUrl); - const repositories = await github(`/user/installations/${installationId}/repositories?per_page=1`); - if (typeof repositories.total_count === "number" && Number.isSafeInteger(repositories.total_count) && repositories.total_count >= 0) { - repositoryCount += repositories.total_count; + for (const repository of await list(`/user/installations/${installationId}/repositories`, "repositories")) { + const id = githubId(repository.id); + const fullName = typeof repository.full_name === "string" ? repository.full_name : ""; + if (!id || !/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(fullName)) { + throw unprocessable("GitHub returned invalid repository metadata", { code: "github_bad_response" }); + } + repositories.set(id, { id, fullName, installationId }); } } + const repositoryCount = repositories.size; if (installationIds.length === 0 || repositoryCount === 0) { const installationUrl = appSlug ? `https://github.com/apps/${appSlug}/installations/new` @@ -2018,12 +2035,14 @@ export async function loadGitHubGrantMetadata( repositorySelection: selections.size > 1 ? "mixed" : selections.values().next().value ?? "none", installationIds, installationOwnerLogins: [...owners], + repositories: [...repositories.values()].sort((a, b) => a.fullName.localeCompare(b.fullName)), installationUrl, managementUrl: managementUrls.size === 1 ? managementUrls.values().next().value! : "https://github.com/settings/installations", ...(appSlug ? { appSlug } : {}), - lastAccessRefreshAt: new Date().toISOString(), + accessRevision: randomUUID(), + lastAccessRefreshAt: accessRefreshStartedAt, webhookHealth: "pending", }; } @@ -8584,21 +8603,36 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} throw retryError; } } - const previousGitHub = grant.providerTenant?.github; - const providerTenant = { - ...(grant.providerTenant ?? {}), - github: { - ...metadata, - ...(previousGitHub?.lastWebhookAt ? { lastWebhookAt: previousGitHub.lastWebhookAt } : {}), - webhookHealth: previousGitHub?.webhookHealth ?? metadata.webhookHealth, - }, - }; - const [updated] = await db.update(connectionGrants).set({ - providerTenant, - status: "active", - updatedAt: now(), - }).where(and(eq(connectionGrants.id, grant.id), eq(connectionGrants.companyId, grant.companyId))).returning(); - if (!updated) throw notFound("GitHub authorization not found"); + const { updated, previousGitHub } = await db.transaction(async (tx) => { + const [currentGrant] = await tx.select().from(connectionGrants).where(and( + eq(connectionGrants.id, grant.id), eq(connectionGrants.companyId, grant.companyId), + )).for("update").limit(1); + if (!currentGrant || currentGrant.status === "revoked") throw notFound("GitHub authorization not found"); + const previousGitHub = currentGrant.providerTenant?.github; + const initialGitHub = grant.providerTenant?.github; + // No lock is held during provider requests. Reject a snapshot if another + // refresh or webhook changed access while those requests were in flight. + if (previousGitHub?.accessRevision !== initialGitHub?.accessRevision + || previousGitHub?.lastWebhookAt !== initialGitHub?.lastWebhookAt + || previousGitHub?.lastAccessRefreshAt !== initialGitHub?.lastAccessRefreshAt) { + throw conflict("GitHub access changed during refresh. Try again.", { code: "github_access_changed" }); + } + const providerTenant = { + ...(currentGrant.providerTenant ?? {}), + github: { + ...metadata, + ...(previousGitHub?.lastWebhookAt ? { lastWebhookAt: previousGitHub.lastWebhookAt } : {}), + webhookHealth: previousGitHub?.webhookHealth ?? metadata.webhookHealth, + }, + }; + const [updated] = await tx.update(connectionGrants).set({ + providerTenant, + status: "active", + updatedAt: now(), + }).where(and(eq(connectionGrants.id, grant.id), eq(connectionGrants.companyId, grant.companyId))).returning(); + if (!updated) throw notFound("GitHub authorization not found"); + return { updated, previousGitHub }; + }); const cloudConnector = currentCloudConnector(); const subject = updated.kind === "agent" && updated.subjectAgentId diff --git a/ui/src/features/connections/ConnectionSetupFlow.tsx b/ui/src/features/connections/ConnectionSetupFlow.tsx index 1886358b70..b97791408e 100644 --- a/ui/src/features/connections/ConnectionSetupFlow.tsx +++ b/ui/src/features/connections/ConnectionSetupFlow.tsx @@ -961,7 +961,12 @@ export function ConnectionSetupFlow({ [entry, galleryQuery.data, linkUrl], ); - const entryAutomaticOAuthMethod = automaticOAuthMethod(entry); + const selectedSetupMethod = entry ? getAvailableConnectionMethod(entry, connectionMethodKey || null) : null; + // Apps with an advanced PAT option still need OAuth progress and recovery + // screens when their selected method is managed sign-in. + const entryAutomaticOAuthMethod = selectedSetupMethod && connectionMethodSupportsAutomaticOAuth(selectedSetupMethod) + ? selectedSetupMethod + : automaticOAuthMethod(entry); const automaticOAuthEntry = credentialSource === "paperclip_vault" && entryAutomaticOAuthMethod ? entry : null; const directOAuthEntry = credentialSource === "paperclip_vault" && canUseAutomaticOAuthFastPath(entry) ? entry : null; const directOAuthLookupPending = Boolean(directOAuthSource) && ( @@ -1222,9 +1227,8 @@ export function ConnectionSetupFlow({ ); // The enrollment lookup decides whether a hidden managed method means // "enroll this instance" or "that Cloud profile is unavailable here". - // Do not choose a method until that distinction is known: retaining the - // hidden method key after an active enrollment produces an empty setup - // screen with a permanently disabled generic Connect button. + // Preserve managed sign-in intent in both cases; the setup screen explains + // unavailable profiles rather than downgrading to a credential form. if ( requestedDefinitionUsesManagedConnector && !requestedEntryAdvertisesManagedConnector @@ -1234,7 +1238,6 @@ export function ConnectionSetupFlow({ const initialMethod = ( requestedDefinitionUsesManagedConnector && !requestedEntryAdvertisesManagedConnector - && connectorEnrollmentQuery.data?.configured !== true ? recommendedManagedConnectorMethod(fullRequestedDefinition) : null ) ?? recommendedSetupConnectionMethod(methods); @@ -1312,14 +1315,10 @@ export function ConnectionSetupFlow({ hasPrefilledLink: Boolean(prefill.link), zapierSource, })); - } else if ( - connectorEnrollmentQuery.data?.configured === true - && connectionMethodKey - && !methods.some((candidate) => candidate.key === connectionMethodKey) - ) { - // A failed enrollment lookup can select the hidden pre-enrollment - // method. Replace it after a successful refetch proves that the instance - // is enrolled and the current Cloud gallery does not advertise it. + } else if (entryAdvertisesManagedConnector !== requestedEntryAdvertisesManagedConnector) { + // A capability refresh must replace the stale gallery entry as well as + // its method, while preserving the chosen audience and wizard step. + setEntry(requestedEntry); setConnectionMethodKey(initialMethod?.key ?? ""); setConfigValues(defaultMethodConfig(initialMethod)); } @@ -1346,6 +1345,7 @@ export function ConnectionSetupFlow({ connectionMethodKey, credentialSource, entry?.slug, + entryAdvertisesManagedConnector, galleryQuery.data, galleryQuery.isLoading, navigate, @@ -1689,6 +1689,14 @@ export function ConnectionSetupFlow({ && (directOAuthEntry || oauthPhase !== "entry"), ); + const managedConnectorUnavailable = Boolean( + step === "key" + && entry + && requestedDefinitionUsesManagedConnector + && !entryAdvertisesManagedConnector + && connectorEnrollmentQuery.data?.configured === true + ); + const showConnectorEnrollmentStep = Boolean( step === "key" && entry @@ -1938,7 +1946,21 @@ export function ConnectionSetupFlow({ /> )} - {step === "key" && entry && showConnectorEnrollmentStep ? ( + {managedConnectorUnavailable && entry ? ( +
+

{entry.name} sign-in is unavailable

+

+ This instance is connected to Paperclip, but {entry.name} sign-in is not currently available. Try again shortly or contact your instance administrator. +

+
+ + +
+
+ ) : step === "key" && entry && showConnectorEnrollmentStep ? (
diff --git a/ui/src/pages/apps/AppDetail.test.tsx b/ui/src/pages/apps/AppDetail.test.tsx index e5c45cadfc..4cd70abe4f 100644 --- a/ui/src/pages/apps/AppDetail.test.tsx +++ b/ui/src/pages/apps/AppDetail.test.tsx @@ -281,6 +281,7 @@ function dedicatedGitHubGrant( repositorySelection: "selected", installationIds: ["456"], installationOwnerLogins: ["paperclipai"], + repositories: [{ id: "789", fullName: "paperclipai/test-repo", installationId: "456" }], managementUrl: "https://github.com/settings/installations/456", webhookHealth: "pending", lastWebhookAt: null, @@ -1455,6 +1456,28 @@ describe("AppDetail", () => { expect(findButton("Revoke")).toBeUndefined(); }); + it("shows the personal GitHub username and every accessible repository", async () => { + mockParams.tab = "permissions"; + getConnectionMock.mockResolvedValue(perUserConnection()); + listConnectionGrantsMock.mockResolvedValue({ + connection: { id: "conn-1", uid: "conn-1" }, + grants: [dedicatedGitHubGrant({ kind: "user", subjectAgentId: null, subjectUserId: "user-1" }, { + repositoryCount: 2, + repositories: [ + { id: "1", fullName: "paperclipai/first", installationId: "456" }, + { id: "2", fullName: "paperclipai/second", installationId: "456" }, + ], + })], + capabilities: fullCapabilities(), currentUserId: "user-1", members: [], + }); + await renderAppDetail(); + expect(container.textContent).toContain("@dottabot"); + expect(container.textContent).toContain("2 selected repositories"); + expect(container.querySelectorAll('ul[aria-label="Accessible GitHub repositories"] li')).toHaveLength(2); + expect(container.textContent).toContain("paperclipai/first"); + expect(container.textContent).toContain("paperclipai/second"); + }); + it("shows dedicated GitHub access as compact action rows and links to the agent", async () => { mockParams.tab = "permissions"; getConnectionMock.mockResolvedValue(connection({ @@ -1473,7 +1496,9 @@ describe("AppDetail", () => { expect(container.querySelector('a[href="/agents/coder"]')?.textContent).toContain("Used only by Coder"); expect(container.textContent).toContain("Repositories"); - expect(container.textContent).toContain("1 selected repositories"); + expect(container.textContent).toContain("1 selected repository"); + expect(container.querySelector('a[href="https://github.com/dottabot"]')?.textContent).toBe("@dottabot"); + expect(container.querySelector('a[href="https://github.com/paperclipai/test-repo"]')?.textContent).toBe("paperclipai/test-repo"); expect(container.querySelector( 'a[href="https://github.com/settings/installations/456"]', )?.textContent).toBe("Manage repositories on GitHub"); diff --git a/ui/src/pages/apps/AppsConnect.test.tsx b/ui/src/pages/apps/AppsConnect.test.tsx index fc20c65fe4..c1102a17ec 100644 --- a/ui/src/pages/apps/AppsConnect.test.tsx +++ b/ui/src/pages/apps/AppsConnect.test.tsx @@ -852,7 +852,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { expect(container.textContent).not.toContain("Connect with Paperclip"); }); - it("uses GitHub's advertised PAT fallback when an enrolled Cloud omits the managed profile", async () => { + it("explains unavailable GitHub sign-in without silently switching to a PAT", async () => { mockSearch.value = "source=github&stage=setup&cloud_connector=enrolled"; listGalleryMock.mockResolvedValueOnce({ apps: [{ @@ -864,19 +864,19 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await render(); - expect(container.textContent).toContain("Your GitHub key"); - expect(container.textContent).not.toContain("Connect with Paperclip"); - expect(container.textContent).not.toContain("Continue to GitHub"); - const connect = buttonByText("Connect"); - expect(connect?.disabled).toBe(true); - const tokenInput = container.querySelector('input[type="password"]'); - expect(tokenInput).toBeTruthy(); - await act(async () => setInputValue(tokenInput!, "github_pat_test")); + expect(container.textContent).toContain("GitHub sign-in is unavailable"); + expect(container.textContent).not.toContain("Your GitHub key"); + expect(container.querySelector('input[type="password"]')).toBeNull(); + expect(buttonByText("Try again")?.disabled).toBe(false); + + listGalleryMock.mockResolvedValue({ apps: [GITHUB_MANAGED] }); + await act(async () => buttonByText("Try again")!.click()); await flushReact(); - expect(connect?.disabled).toBe(false); + expect(container.textContent).toContain("Continue to GitHub"); + expect(container.textContent).not.toContain("GitHub sign-in is unavailable"); }); - it("replaces a hidden managed method after enrollment recovery reveals an advertised PAT fallback", async () => { + it("keeps GitHub sign-in intent when enrollment recovery reveals an unavailable profile", async () => { mockSearch.value = "source=github&stage=setup&cloud_connector=enrolled"; listGalleryMock.mockResolvedValueOnce({ apps: [{ @@ -907,10 +907,9 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }); await flushReact(); - expect(container.textContent).toContain("Your GitHub key"); - expect(container.textContent).not.toContain("Connect with Paperclip"); - expect(container.textContent).not.toContain("Continue to GitHub"); - expect(buttonByText("Connect")?.disabled).toBe(true); + expect(container.textContent).toContain("GitHub sign-in is unavailable"); + expect(container.textContent).not.toContain("Your GitHub key"); + expect(buttonByText("Try again")?.disabled).toBe(false); }); it("preserves a dedicated agent identity across the full-page enrollment callback", async () => { @@ -1645,6 +1644,26 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { ); }); + it("shows installation recovery for GitHub even when an advanced PAT method is available", async () => { + const connectionId = "22222222-2222-4222-8222-222222222222"; + mockSearch.value = `source=github&resume=${connectionId}&oauth=failed&code=github_installation_required&installation_url=https%3A%2F%2Fgithub.com%2Fapps%2Fpaperclip-for-github%2Finstallations%2Fnew`; + listGalleryMock.mockResolvedValue({ apps: [GITHUB_MANAGED] }); + listApplicationsMock.mockResolvedValue({ applications: [{ id: "app-github", status: "draft", metadata: { sourceTemplateKey: "github" } }] }); + listConnectionsMock.mockResolvedValue({ connections: [{ + id: connectionId, applicationId: "app-github", authKind: "oauth", credentialPolicy: "per_user", status: "draft", + config: { sourceTemplateKey: "github", connectionMethodKey: "managed" }, transportConfig: {}, + }] }); + await render(); + await flushReact(); + expect(container.textContent).toContain("Install Paperclip and grant at least one repository"); + expect(container.querySelector('a[href="https://github.com/apps/paperclip-for-github/installations/new"]')?.textContent).toBe("Install Paperclip on GitHub"); + expect(container.textContent).not.toContain("Your GitHub key"); + await act(async () => buttonByText("Try again")!.click()); + await flushReact(); + expect(startOAuthMock).toHaveBeenCalledWith(connectionId, { asCurrentUser: true }); + expect(connectAppMock).not.toHaveBeenCalled(); + }); + it("returns a declined OAuth draft to the same one-action resume checkpoint", async () => { mockSearch.value = "source=notion&resume=22222222-2222-4222-8222-222222222222&oauth=denied&code=oauth_authorization_denied"; listGalleryMock.mockResolvedValueOnce({ apps: [NOTION] }); diff --git a/ui/src/pages/apps/app-detail/IdentitiesSection.tsx b/ui/src/pages/apps/app-detail/IdentitiesSection.tsx index 2f49e0c15a..f2abaf0af9 100644 --- a/ui/src/pages/apps/app-detail/IdentitiesSection.tsx +++ b/ui/src/pages/apps/app-detail/IdentitiesSection.tsx @@ -300,9 +300,15 @@ function GitHubConnectionSummary({ : null; const repositorySummary = github.repositorySelection === "none" ? "No repositories selected" - : `${github.repositoryCount} selected repositories`; + : `${github.repositoryCount} selected ${github.repositoryCount === 1 ? "repository" : "repositories"}`; return (
+
+
GitHub account
+ + @{github.login} + +
Repositories
@@ -327,6 +333,21 @@ function GitHubConnectionSummary({ ) : null}
+
+ {github.repositories ? ( + + ) : ( +

Refresh access to load the current repository list.

+ )} +
Refresh access