diff --git a/doc/connections/GITHUB.md b/doc/connections/GITHUB.md index a85a062c5b..f6b4765343 100644 --- a/doc/connections/GITHUB.md +++ b/doc/connections/GITHUB.md @@ -73,9 +73,19 @@ OAuth completion verifies `/user`, every page of `/user/installations`, and ever 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: +repository IDs, full names, installation IDs, and private-repository flags. GitHub stays authoritative: this snapshot never authorizes repository access. +The permissions page shows repositories across authorized accounts by default. +Use the account filter and search to narrow the list. The list scrolls after +about ten rows and marks known private repositories with a lock. Configure on +GitHub opens the app account chooser so users can add or update organization +access. Refresh access after changing the selection. Older snapshots omit the +private flag until refreshed. If a legacy grant lacks its app chooser URL, +**Load GitHub configuration** refreshes access and recovers the app slug from +GitHub installation metadata. The page does not substitute a single-installation +settings URL for the account chooser. + 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 diff --git a/packages/db/src/schema/tool_access.ts b/packages/db/src/schema/tool_access.ts index 59261f2241..086f9af21f 100644 --- a/packages/db/src/schema/tool_access.ts +++ b/packages/db/src/schema/tool_access.ts @@ -195,7 +195,7 @@ export const connectionGrants = pgTable( installationIds: string[]; installationOwnerLogins: string[]; /** Repository metadata visible to this credential; refreshed from GitHub. */ - repositories?: Array<{ id: string; fullName: string; installationId: string }>; + repositories?: Array<{ id: string; fullName: string; installationId: string; private?: boolean }>; installationUrl?: string; managementUrl?: string; appSlug?: string; diff --git a/packages/shared/src/types/tool-access.ts b/packages/shared/src/types/tool-access.ts index 8bb22a496c..c8e16dc6ed 100644 --- a/packages/shared/src/types/tool-access.ts +++ b/packages/shared/src/types/tool-access.ts @@ -225,7 +225,7 @@ export interface ConnectionGrant { installationIds: string[]; installationOwnerLogins: string[]; /** Repository metadata visible to this credential; refreshed from GitHub. */ - repositories?: Array<{ id: string; fullName: string; installationId: string }>; + repositories?: Array<{ id: string; fullName: string; installationId: string; private?: boolean }>; installationUrl?: string; managementUrl?: string; appSlug?: string; diff --git a/server/src/__tests__/github-grant-metadata.test.ts b/server/src/__tests__/github-grant-metadata.test.ts index b77976f337..e7c42d9ec8 100644 --- a/server/src/__tests__/github-grant-metadata.test.ts +++ b/server/src/__tests__/github-grant-metadata.test.ts @@ -24,8 +24,8 @@ describe("GitHub grant metadata", () => { } 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" }], + ? [{ id: 2, full_name: "paperclipai/b", private: true, description: "must-not-persist", clone_url: "must-not-persist" }] + : [{ id: 1, full_name: "paperclipai/a", private: false }], }, !secondPage); } if (url.pathname === "/user/installations/102/repositories") { @@ -45,14 +45,15 @@ describe("GitHub grant metadata", () => { 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" }, + { id: "1", fullName: "paperclipai/a", installationId: "101", private: false }, + { id: "2", fullName: "paperclipai/b", installationId: "101", private: true }, ], installationUrl: "https://github.com/apps/paperclip-development/installations/new", managementUrl: "https://github.com/settings/installations/101", appSlug: "paperclip-development", webhookHealth: "pending", }); + expect(metadata.repositories[0]).not.toHaveProperty("private"); expect(JSON.stringify(metadata)).not.toContain("must-not-persist"); expect(request).toHaveBeenCalledTimes(6); for (const [input, init] of request.mock.calls) { @@ -61,6 +62,17 @@ describe("GitHub grant metadata", () => { } }); + it("recovers a legacy grant's app chooser from GitHub installation metadata", async () => { + const request = vi.fn() + .mockResolvedValueOnce(json({ id: 42, login: "octocat" })) + .mockResolvedValueOnce(json({ installations: [{ id: 101, app_slug: "paperclip-staging", repository_selection: "selected" }] })) + .mockResolvedValueOnce(json({ repositories: [{ id: 1, full_name: "octocat/a" }] })); + await expect(loadGitHubGrantMetadata("ghu_secret", request)).resolves.toMatchObject({ + appSlug: "paperclip-staging", + installationUrl: "https://github.com/apps/paperclip-staging/installations/new", + }); + }); + it("requires at least one installation with an accessible repository", async () => { const request = vi.fn(async (input) => String(input).endsWith("/user") ? json({ id: 42, login: "octocat" }) diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index d117e6f6a9..a1b07af1fa 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -1942,7 +1942,7 @@ export async function loadGitHubGrantMetadata( repositorySelection: "all" | "selected" | "mixed" | "none"; installationIds: string[]; installationOwnerLogins: string[]; - repositories: Array<{ id: string; fullName: string; installationId: string }>; + repositories: Array<{ id: string; fullName: string; installationId: string; private?: boolean }>; installationUrl: string; managementUrl: string; appSlug?: string; @@ -1950,6 +1950,7 @@ export async function loadGitHubGrantMetadata( lastAccessRefreshAt: string; webhookHealth: "pending"; }> { + let resolvedAppSlug = appSlug; const accessRefreshStartedAt = new Date().toISOString(); const github = async (path: string): Promise<{ data: Record; hasNext: boolean }> => { const response = await request(`https://api.github.com${path}`, { @@ -1991,11 +1992,17 @@ export async function loadGitHubGrantMetadata( const owners = new Set(); const selections = new Set<"all" | "selected">(); const managementUrls = new Set(); - const repositories = new Map(); + const repositories = new Map(); for (const installation of installations) { const installationId = githubId(installation.id); if (!installationId) continue; installationIds.push(installationId); + // Older grants predate the broker's appSlug field. GitHub's installation + // response identifies this token's app without choosing an environment. + if (!resolvedAppSlug && typeof installation.app_slug === "string" + && /^[a-z0-9-]{1,100}$/.test(installation.app_slug)) { + resolvedAppSlug = installation.app_slug; + } if (installation.repository_selection === "all" || installation.repository_selection === "selected") { selections.add(installation.repository_selection); } @@ -2009,13 +2016,16 @@ export async function loadGitHubGrantMetadata( 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 }); + repositories.set(id, { + id, fullName, installationId, + ...(typeof repository.private === "boolean" ? { private: repository.private } : {}), + }); } } const repositoryCount = repositories.size; if (installationIds.length === 0 || repositoryCount === 0) { - const installationUrl = appSlug - ? `https://github.com/apps/${appSlug}/installations/new` + const installationUrl = resolvedAppSlug + ? `https://github.com/apps/${resolvedAppSlug}/installations/new` : "https://github.com/settings/installations"; throw unprocessable("GitHub access is required. Install Paperclip and grant at least one repository before refreshing access.", { code: "github_installation_required", @@ -2023,8 +2033,8 @@ export async function loadGitHubGrantMetadata( managementUrl: "https://github.com/settings/installations", }); } - const installationUrl = appSlug - ? `https://github.com/apps/${appSlug}/installations/new` + const installationUrl = resolvedAppSlug + ? `https://github.com/apps/${resolvedAppSlug}/installations/new` : "https://github.com/settings/installations"; return { userId, @@ -2040,7 +2050,7 @@ export async function loadGitHubGrantMetadata( managementUrl: managementUrls.size === 1 ? managementUrls.values().next().value! : "https://github.com/settings/installations", - ...(appSlug ? { appSlug } : {}), + ...(resolvedAppSlug ? { appSlug: resolvedAppSlug } : {}), accessRevision: randomUUID(), lastAccessRefreshAt: accessRefreshStartedAt, webhookHealth: "pending", diff --git a/ui/src/index.css b/ui/src/index.css index f66d4de01e..7f67ca8568 100644 --- a/ui/src/index.css +++ b/ui/src/index.css @@ -2366,6 +2366,8 @@ span.paperclip-mention-chip[data-mention-kind="external-object"] { for the full site list). */ :root { + /* Ten single-line repository rows, including the gaps between them. */ + --sz-github-repository-list: calc(10lh + 9 * var(--spacing) * 2); --sz-320px: 320px; /* Extracted from ui/src/components/ActiveAgentsPanel.tsx (h-[320px]). */ --sz-390px: 390px; --sz-64px: 64px; diff --git a/ui/src/pages/apps/AppDetail.test.tsx b/ui/src/pages/apps/AppDetail.test.tsx index 4cd70abe4f..131f328d5b 100644 --- a/ui/src/pages/apps/AppDetail.test.tsx +++ b/ui/src/pages/apps/AppDetail.test.tsx @@ -25,6 +25,7 @@ const finishAppMock = vi.hoisted(() => vi.fn()); const finalizeOAuthAccessMock = vi.hoisted(() => vi.fn()); const putConnectionInstallsMock = vi.hoisted(() => vi.fn()); const refreshCatalogMock = vi.hoisted(() => vi.fn()); +const checkConnectionHealthMock = vi.hoisted(() => vi.fn()); const startOAuthMock = vi.hoisted(() => vi.fn()); const listConnectionGrantsMock = vi.hoisted(() => vi.fn()); const revokeConnectionGrantMock = vi.hoisted(() => vi.fn()); @@ -67,6 +68,7 @@ vi.mock("@/api/tools", () => ({ putConnectionInstallsMock(connectionId, installs), archiveConnection: vi.fn(), refreshCatalog: (connectionId: string) => refreshCatalogMock(connectionId), + checkConnectionHealth: (connectionId: string) => checkConnectionHealthMock(connectionId), startOAuth: (connectionId: string, input?: unknown) => input === undefined ? startOAuthMock(connectionId) : startOAuthMock(connectionId, input), @@ -282,6 +284,7 @@ function dedicatedGitHubGrant( installationIds: ["456"], installationOwnerLogins: ["paperclipai"], repositories: [{ id: "789", fullName: "paperclipai/test-repo", installationId: "456" }], + installationUrl: "https://github.com/apps/paperclip-test/installations/new", managementUrl: "https://github.com/settings/installations/456", webhookHealth: "pending", lastWebhookAt: null, @@ -409,6 +412,7 @@ describe("AppDetail", () => { finishAppMock.mockResolvedValue({}); finalizeOAuthAccessMock.mockResolvedValue({}); putConnectionInstallsMock.mockResolvedValue({ connectionId: "conn-1", installs: [] }); + checkConnectionHealthMock.mockResolvedValue({ connection: connection(), healthStatus: "ok" }); refreshCatalogMock.mockResolvedValue({ discoveredCount: 0, quarantinedCount: 0, catalog: [] }); startOAuthMock.mockResolvedValue({ connectionId: "conn-1", @@ -1464,8 +1468,8 @@ describe("AppDetail", () => { 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" }, + { id: "1", fullName: "paperclipai/first", installationId: "456", private: true }, + { id: "2", fullName: "paperclipai/second", installationId: "456", private: false }, ], })], capabilities: fullCapabilities(), currentUserId: "user-1", members: [], @@ -1476,6 +1480,82 @@ describe("AppDetail", () => { expect(container.querySelectorAll('ul[aria-label="Accessible GitHub repositories"] li')).toHaveLength(2); expect(container.textContent).toContain("paperclipai/first"); expect(container.textContent).toContain("paperclipai/second"); + expect(container.querySelector('a[href="https://github.com/paperclipai/first"] [aria-label="Private repository"]')).toBeTruthy(); + expect(container.querySelector('a[href="https://github.com/paperclipai/second"] [aria-label="Private repository"]')).toBeNull(); + const configureHint = [...container.querySelectorAll("p a")].find((link) => link.textContent === "Configure access on GitHub"); + expect(configureHint?.getAttribute("href")).toBe("https://github.com/apps/paperclip-test/installations/new"); + }); + + it.each([undefined, "https://github.com/settings/installations"])("loads missing GitHub app configuration instead of linking legacy settings (%s)", async (installationUrl) => { + mockParams.tab = "permissions"; + getConnectionMock.mockResolvedValue(perUserConnection()); + listConnectionGrantsMock.mockResolvedValue({ + connection: { id: "conn-1", uid: "conn-1" }, + grants: [dedicatedGitHubGrant({ kind: "user", subjectAgentId: null, subjectUserId: "user-1" }, { installationUrl })], + capabilities: fullCapabilities(), currentUserId: "user-1", members: [], + }); + await renderAppDetail(); + expect(findButton("Load GitHub configuration")).toBeTruthy(); + expect(container.querySelector('a[href="https://github.com/settings/installations/456"]')).toBeNull(); + expect(container.querySelector('a[href="https://github.com/settings/installations"]')).toBeNull(); + listConnectionGrantsMock.mockResolvedValue({ + connection: { id: "conn-1", uid: "conn-1" }, + grants: [dedicatedGitHubGrant({ kind: "user", subjectAgentId: null, subjectUserId: "user-1" }, { + appSlug: "paperclip-staging", installationUrl: "https://github.com/apps/paperclip-staging/installations/new", + })], + capabilities: fullCapabilities(), currentUserId: "user-1", members: [], + }); + await act(async () => { findButton("Load GitHub configuration")!.click(); }); + await flushReact(); + expect(checkConnectionHealthMock).toHaveBeenCalledWith("conn-1"); + expect(container.querySelector('a[href="https://github.com/apps/paperclip-staging/installations/new"]')?.textContent).toBe("Configure on GitHub"); + expect(findButton("Load GitHub configuration")).toBeUndefined(); + }); + + it("filters the combined GitHub repository list by owner and search without changing access", 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: 3, + installationOwnerLogins: ["paperclipai", "dottabot", "empty-org"], + repositories: [ + { id: "1", fullName: "paperclipai/first", installationId: "456" }, + { id: "2", fullName: "paperclipai/second", installationId: "456" }, + { id: "3", fullName: "dottabot/first", installationId: "789" }, + ], + })], + capabilities: fullCapabilities(), currentUserId: "user-1", members: [], + }); + await renderAppDetail(); + const repositoryNames = () => [...container.querySelectorAll('ul[aria-label="Accessible GitHub repositories"] a')].map((link) => link.textContent); + const selectOwner = async (label: string) => { + await act(async () => { + container.querySelector('[role="combobox"][aria-label="Filter repositories by account or organization"]')! + .dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowDown", bubbles: true })); + }); + const option = [...document.querySelectorAll('[role="option"]')].find((item) => item.textContent === label); + expect(option).toBeTruthy(); + await act(async () => { + option!.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter", bubbles: true })); + }); + }; + expect(repositoryNames()).toEqual(["paperclipai/first", "paperclipai/second", "dottabot/first"]); + await selectOwner("paperclipai"); + expect(repositoryNames()).toEqual(["paperclipai/first", "paperclipai/second"]); + const search = container.querySelector('input[aria-label="Search GitHub repositories"]')!; + await act(async () => { setInputValue(search, "FIRST"); }); + expect(repositoryNames()).toEqual(["paperclipai/first"]); + await selectOwner("All accounts"); + expect(repositoryNames()).toEqual(["paperclipai/first", "dottabot/first"]); + await act(async () => { setInputValue(search, "missing"); }); + expect(container.textContent).toContain("No repositories match your search."); + await act(async () => { setInputValue(search, ""); }); + await selectOwner("empty-org"); + expect(container.textContent).toContain("No accessible repositories for this account or organization."); + expect(container.querySelector('a[href="https://github.com/apps/paperclip-test/installations/new"]')?.textContent).toBe("Configure on GitHub"); + expect(updateConnectionMock).not.toHaveBeenCalled(); }); it("shows dedicated GitHub access as compact action rows and links to the agent", async () => { @@ -1500,9 +1580,9 @@ describe("AppDetail", () => { 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"); - expect(findButton("Refresh access")).toBeTruthy(); + 'a[href="https://github.com/apps/paperclip-test/installations/new"]', + )?.textContent).toBe("Configure on GitHub"); + expect(container.querySelector('button[aria-label="Refresh access"]')).toBeTruthy(); expect(container.textContent).not.toContain("Installation"); expect(container.textContent).not.toContain("Token continuity"); expect(container.textContent).not.toContain("Webhook health"); diff --git a/ui/src/pages/apps/app-detail/IdentitiesSection.tsx b/ui/src/pages/apps/app-detail/IdentitiesSection.tsx index f2abaf0af9..f48815216d 100644 --- a/ui/src/pages/apps/app-detail/IdentitiesSection.tsx +++ b/ui/src/pages/apps/app-detail/IdentitiesSection.tsx @@ -1,5 +1,5 @@ import { useEffect, useMemo, useState, type ReactNode } from "react"; -import { Building2, Loader2, TriangleAlert, UserRound } from "lucide-react"; +import { Building2, Loader2, Lock, RefreshCw, Search, TriangleAlert, UserRound } from "lucide-react"; import type { ConnectionAudienceMember, ConnectionGrant, @@ -8,6 +8,9 @@ import type { } from "@paperclipai/shared"; import { Button } from "@/components/ui/button"; import { Identity } from "@/components/Identity"; +import { GithubIcon } from "@/components/icons/github-icon"; +import { Input } from "@/components/ui/input"; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"; import { Skeleton } from "@/components/ui/skeleton"; import { InlineBanner } from "@/components/InlineBanner"; import { MemberMultiSelect } from "@/components/MemberMultiSelect"; @@ -291,8 +294,29 @@ function GitHubConnectionSummary({ onRefreshAccess?: () => void; refreshPending: boolean; }) { + const [repositoryOwner, setRepositoryOwner] = useState("*"); + const [repositorySearch, setRepositorySearch] = useState(""); + useEffect(() => { + setRepositoryOwner("*"); + setRepositorySearch(""); + }, [grant.id]); const github = grant.providerTenant?.github; if (!github) return null; + const owners = [...new Set([ + ...(github.installationOwnerLogins ?? []), + ...(github.repositories ?? []).map((repository) => repository.fullName.split("/")[0]), + ])].sort((a, b) => a.localeCompare(b)); + const selectedOwner = owners.includes(repositoryOwner) ? repositoryOwner : "*"; + const search = repositorySearch.trim().toLowerCase(); + const visibleRepositories = github.repositories?.filter((repository) => ( + (selectedOwner === "*" || repository.fullName.split("/")[0] === selectedOwner) + && repository.fullName.toLowerCase().includes(search) + )); + const configurationUrl = github.appSlug + ? `https://github.com/apps/${encodeURIComponent(github.appSlug)}/installations/new` + : /^https:\/\/github\.com\/apps\/[a-z0-9-]+\/installations\/new$/.test(github.installationUrl ?? "") + ? github.installationUrl + : null; const repositoryWarning = github.repositorySelection === "all" ? "All current and future repositories" : github.repositorySelection === "mixed" @@ -309,56 +333,81 @@ function GitHubConnectionSummary({ @{github.login} -
-
-
Repositories
- {repositoryWarning ? ( -
-
- ) : ( -
{repositorySummary}
- )} +
+
+
+
Repositories
+ {repositoryWarning ? ( +
+
+ ) : ( +
{repositorySummary}
+ )} +
+
+ {onRefreshAccess && configurationUrl ? ( + + ) : null} + {configurationUrl ? ( + + ) : onRefreshAccess ? ( + + ) : null} +
+
+
+ +
+
- {github.managementUrl ? ( - - ) : null} -
-
{github.repositories ? ( - :

+ {search ? "No repositories match your search." : "No accessible repositories for this account or organization."} +

) : (

Refresh access to load the current repository list.

)} -
-
-
-
Refresh access
-
Sync repository access from GitHub.
-
- {onRefreshAccess ? ( - - ) : null} + {configurationUrl ?

+ Missing an organization or repository? Configure access on GitHub, then refresh this list. +

: null}
);