From 134a5d000dd015690d161232def9e215bc08ac8d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 05:23:29 +0000 Subject: [PATCH 1/2] Revert "fix(dashboard): revalidate GitHub caches when installation access changes" This reverts commit 4f54751e1cd00eef690c6d89fb1912b84d4405a1. --- apps/dashboard/src/lib/github.functions.ts | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/apps/dashboard/src/lib/github.functions.ts b/apps/dashboard/src/lib/github.functions.ts index 1fdac3c..a14750d 100644 --- a/apps/dashboard/src/lib/github.functions.ts +++ b/apps/dashboard/src/lib/github.functions.ts @@ -4258,10 +4258,7 @@ async function getMyPullsResult({ resource: "pulls.mine.graphql.v2", params: { username }, freshForMs: githubCachePolicy.list.staleTimeMs, - signalKeys: [ - githubRevalidationSignalKeys.pullsMine, - githubRevalidationSignalKeys.installationAccess, - ], + signalKeys: [githubRevalidationSignalKeys.pullsMine], namespaceKeys: [githubRevalidationSignalKeys.pullsMine], cacheMode: "split", merge: (existing, fresh) => mergeMyPullsResults([existing, fresh]), @@ -4449,10 +4446,7 @@ async function getMyIssuesResult({ resource: "issues.mine.graphql.v2", params: { username }, freshForMs: githubCachePolicy.list.staleTimeMs, - signalKeys: [ - githubRevalidationSignalKeys.issuesMine, - githubRevalidationSignalKeys.installationAccess, - ], + signalKeys: [githubRevalidationSignalKeys.issuesMine], namespaceKeys: [githubRevalidationSignalKeys.issuesMine], cacheMode: "split", merge: (existing, fresh) => mergeMyIssuesResults([existing, fresh]), @@ -4748,18 +4742,12 @@ export const getInstallationAccess = createServerFn({ export const refreshInstallationAccess = createServerFn({ method: "POST", }).handler(async () => { - const context = await getGitHubContext(); - if (!context) { - return { ok: false as const }; - } - await markGitHubRevalidationSignals([ githubRevalidationSignalKeys.installationAccess, ]); - await bustGitHubCache(context.session.user.id, "installationAccess", null); debug( "refreshInstallationAccess", - "marked installationAccess for revalidation and busted cache row", + "marked installationAccess for revalidation", ); return { ok: true }; }); @@ -4777,7 +4765,6 @@ export const getUserRepos = createServerFn({ method: "GET" }).handler( resource: "repos.list", params: { sort: "updated", perPage: 10 }, freshForMs: githubCachePolicy.reposList.staleTimeMs, - signalKeys: [githubRevalidationSignalKeys.installationAccess], namespaceKeys: ["repos.list"], cacheMode: "split", request: (headers) => From 31bf18c76a4aeda1c64155d049e2d408460dde05 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 05:23:36 +0000 Subject: [PATCH 2/2] Revert "feat: filter private repos by GitHub App installation access" This reverts commit 96314e92c5740f82a801b48614a51243efb4ed09. --- .../layouts/github-access-dialog.tsx | 3 - apps/dashboard/src/lib/github-access.test.ts | 92 --- apps/dashboard/src/lib/github-access.ts | 54 -- apps/dashboard/src/lib/github-cache-policy.ts | 4 - apps/dashboard/src/lib/github-revalidation.ts | 9 - apps/dashboard/src/lib/github.functions.ts | 578 +++--------------- apps/dashboard/src/lib/github.types.ts | 2 - .../src/lib/use-refresh-on-return.ts | 59 -- apps/dashboard/src/routes/setup.tsx | 2 - 9 files changed, 69 insertions(+), 734 deletions(-) delete mode 100644 apps/dashboard/src/lib/use-refresh-on-return.ts diff --git a/apps/dashboard/src/components/layouts/github-access-dialog.tsx b/apps/dashboard/src/components/layouts/github-access-dialog.tsx index 43f2ea3..6221f28 100644 --- a/apps/dashboard/src/components/layouts/github-access-dialog.tsx +++ b/apps/dashboard/src/components/layouts/github-access-dialog.tsx @@ -24,7 +24,6 @@ import { useGitHubAccessPrompt, } from "#/lib/github-access-modal-store"; import { useHasMounted } from "#/lib/use-has-mounted"; -import { useRefreshOnReturn } from "#/lib/use-refresh-on-return"; function getExternalLinkProps(href: string) { if (href.startsWith("http://") || href.startsWith("https://")) { @@ -40,8 +39,6 @@ export function GitHubAccessDialog({ userId }: { userId: string }) { const [showOrgSetup, setShowOrgSetup] = useShowOrgSetupQueryState(); const isOpen = showOrgSetup; - useRefreshOnReturn({ enabled: isOpen }); - const accessQuery = useQuery({ queryKey: ["github-app-access-state", userId], queryFn: () => getGitHubAppAccessState(), diff --git a/apps/dashboard/src/lib/github-access.test.ts b/apps/dashboard/src/lib/github-access.test.ts index 996bf68..cd70cc0 100644 --- a/apps/dashboard/src/lib/github-access.test.ts +++ b/apps/dashboard/src/lib/github-access.test.ts @@ -4,9 +4,7 @@ import { buildGitHubOrganizationInstallationsUrl, findInstallationForOwner, type GitHubAppAccessState, - type GitHubInstallationAccessIndex, getAccessHrefForOwner, - isRepoVisibleWithInstallationAccess, } from "./github-access"; const state: GitHubAppAccessState = { @@ -108,93 +106,3 @@ describe("buildGitHubOrganizationInstallationsUrl", () => { ); }); }); - -describe("isRepoVisibleWithInstallationAccess", () => { - const index: GitHubInstallationAccessIndex = { - available: true, - allAccessOwners: new Set(["supabase"]), - selectedRepos: new Set(["adn/private-app", "adn/secret-tool"]), - }; - - it("always allows public repos regardless of index", () => { - expect( - isRepoVisibleWithInstallationAccess(index, "random-org", "repo", false), - ).toBe(true); - }); - - it("allows private repos from an owner with 'all' access", () => { - expect( - isRepoVisibleWithInstallationAccess( - index, - "supabase", - "private-repo", - true, - ), - ).toBe(true); - }); - - it("allows private repos in the selected set", () => { - expect( - isRepoVisibleWithInstallationAccess(index, "adn", "private-app", true), - ).toBe(true); - }); - - it("blocks private repos not in the selected set", () => { - expect( - isRepoVisibleWithInstallationAccess(index, "adn", "other-private", true), - ).toBe(false); - }); - - it("blocks private repos from owners without any installation", () => { - expect( - isRepoVisibleWithInstallationAccess( - index, - "vercel", - "private-repo", - true, - ), - ).toBe(false); - }); - - it("fails open when the index is unavailable", () => { - const unavailable: GitHubInstallationAccessIndex = { - available: false, - allAccessOwners: new Set(), - selectedRepos: new Set(), - }; - expect( - isRepoVisibleWithInstallationAccess( - unavailable, - "any-org", - "private-repo", - true, - ), - ).toBe(true); - }); - - it("treats unknown visibility (null) as potentially private", () => { - expect( - isRepoVisibleWithInstallationAccess(index, "adn", "private-app", null), - ).toBe(true); - expect( - isRepoVisibleWithInstallationAccess(index, "adn", "other-private", null), - ).toBe(false); - expect( - isRepoVisibleWithInstallationAccess(index, "vercel", "some-repo", null), - ).toBe(false); - }); - - it("is case-insensitive for owner and repo matching", () => { - expect( - isRepoVisibleWithInstallationAccess( - index, - "Supabase", - "Private-Repo", - true, - ), - ).toBe(true); - expect( - isRepoVisibleWithInstallationAccess(index, "ADN", "Private-App", true), - ).toBe(true); - }); -}); diff --git a/apps/dashboard/src/lib/github-access.ts b/apps/dashboard/src/lib/github-access.ts index 095605f..c6f66fd 100644 --- a/apps/dashboard/src/lib/github-access.ts +++ b/apps/dashboard/src/lib/github-access.ts @@ -99,57 +99,3 @@ export function getAccessHrefForOwner( return state.publicInstallUrl ?? fallbackHref ?? null; } - -// --------------------------------------------------------------------------- -// Installation access index -// --------------------------------------------------------------------------- - -/** - * A pre-computed index of which repos/owners are accessible via the GitHub App - * installations. Used to filter private repos so the OAuth token doesn't leak - * access beyond what the user configured in their App installation. - */ -export type GitHubInstallationAccessIndex = { - /** `false` when the app-user token isn't available (no app auth yet). */ - available: boolean; - /** Normalized owner logins with `repositorySelection: "all"`. */ - allAccessOwners: Set; - /** Normalized `owner/repo` strings for `repositorySelection: "selected"`. */ - selectedRepos: Set; -}; - -const EMPTY_INSTALLATION_ACCESS_INDEX: GitHubInstallationAccessIndex = { - available: false, - allAccessOwners: new Set(), - selectedRepos: new Set(), -}; - -export function emptyInstallationAccessIndex(): GitHubInstallationAccessIndex { - return EMPTY_INSTALLATION_ACCESS_INDEX; -} - -/** - * Returns `true` when a repo should be visible given the current installation - * access index. - * - * Rules: - * - Public repos always pass. - * - When the index isn't available (no app setup), all repos pass (fail-open). - * - Private repos pass only when the owning account has an "all" installation - * **or** the specific repo is in the "selected" set. - */ -export function isRepoVisibleWithInstallationAccess( - index: GitHubInstallationAccessIndex, - owner: string, - repo: string, - isPrivate: boolean | null, -): boolean { - // Only skip the check when the repo is *explicitly* public. - // `null` (unknown visibility) is treated as potentially private. - if (isPrivate === false) return true; - if (!index.available) return true; - - const normalizedOwner = normalizeLogin(owner); - if (index.allAccessOwners.has(normalizedOwner)) return true; - return index.selectedRepos.has(`${normalizedOwner}/${repo.toLowerCase()}`); -} diff --git a/apps/dashboard/src/lib/github-cache-policy.ts b/apps/dashboard/src/lib/github-cache-policy.ts index 3af82a3..64ad4db 100644 --- a/apps/dashboard/src/lib/github-cache-policy.ts +++ b/apps/dashboard/src/lib/github-cache-policy.ts @@ -35,8 +35,4 @@ export const githubCachePolicy = { staleTimeMs: 30 * 60 * 1000, gcTimeMs: 24 * 60 * 60 * 1000, }, - installationAccess: { - staleTimeMs: 30 * 60 * 1000, - gcTimeMs: 24 * 60 * 60 * 1000, - }, } as const; diff --git a/apps/dashboard/src/lib/github-revalidation.ts b/apps/dashboard/src/lib/github-revalidation.ts index c0dc9b9..d94b40e 100644 --- a/apps/dashboard/src/lib/github-revalidation.ts +++ b/apps/dashboard/src/lib/github-revalidation.ts @@ -22,7 +22,6 @@ export const githubRevalidationSignalKeys = { `workflowJob:${input.owner}/${input.repo}#${input.jobId}`, repoCode: (input: { owner: string; repo: string }) => `repoCode:${input.owner}/${input.repo}`, - installationAccess: "installationAccess", } as const; function isRecord(value: unknown): value is Record { @@ -162,14 +161,6 @@ export function getGitHubWebhookRevalidationSignalKeys( event: string, payload: unknown, ) { - if ( - event === "installation" || - event === "installation_repositories" || - event === "github_app_authorization" - ) { - return [githubRevalidationSignalKeys.installationAccess]; - } - const repository = getRepositoryIdentity(payload); if (!repository) { return []; diff --git a/apps/dashboard/src/lib/github.functions.ts b/apps/dashboard/src/lib/github.functions.ts index a14750d..4a39feb 100644 --- a/apps/dashboard/src/lib/github.functions.ts +++ b/apps/dashboard/src/lib/github.functions.ts @@ -54,13 +54,10 @@ import type { import { buildGitHubAppAuthorizePath, buildGitHubAppInstallUrl, - emptyInstallationAccessIndex, type GitHubAppAccessState, type GitHubAppInstallation, - type GitHubInstallationAccessIndex, type GitHubInstallationTargetType, type GitHubOrganization, - isRepoVisibleWithInstallationAccess, } from "./github-access"; import { getGitHubAppSlug } from "./github-app.server"; import { @@ -70,7 +67,6 @@ import { type GitHubConditionalHeaders, type GitHubFetchResult, getOrRevalidateGitHubResource, - markGitHubRevalidationSignals, } from "./github-cache"; import { githubCachePolicy } from "./github-cache-policy"; import { githubRevalidationSignalKeys } from "./github-revalidation"; @@ -128,7 +124,6 @@ type GitHubGraphQLRepositoryRef = { name: string; nameWithOwner: string; url: string; - isPrivate: boolean; owner: { login: string; }; @@ -494,7 +489,6 @@ type GitHubUserInstallationPayload = { }; type GitHubUserInstallationsPayload = { - total_count?: number; installations?: GitHubUserInstallationPayload[]; }; @@ -675,20 +669,17 @@ function buildRepositoryRef( owner: string, repo: string, url?: string | null, - isPrivate: boolean | null = null, ): RepositoryRef { return { name: repo, owner, fullName: `${owner}/${repo}`, url: url ?? `https://github.com/${owner}/${repo}`, - isPrivate, }; } function parseRepositoryRef( repositoryUrl?: string | null, - isPrivate: boolean | null = null, ): RepositoryRef | null { if (!repositoryUrl) { return null; @@ -703,7 +694,6 @@ function parseRepositoryRef( match[1], match[2], `https://github.com/${match[1]}/${match[2]}`, - isPrivate, ); } @@ -731,7 +721,6 @@ function mapGraphQLRepositoryRef( owner, fullName: repository.nameWithOwner, url: repository.url, - isPrivate: repository.isPrivate, }; } @@ -1514,64 +1503,37 @@ function mapGitHubAppInstallations( async function getGitHubAppUserInstallations(userId: string): Promise<{ installations: GitHubAppInstallation[]; - /** `true` when the app-user token is configured and the API responded. */ installationsAvailable: boolean; - /** The app-user Octokit instance (for follow-up calls like listing repos). */ - appUserOctokit: GitHubClient | null; }> { - const { getGitHubAppUserClientByUserId } = await import("./auth-runtime"); - const appUserOctokit = await getGitHubAppUserClientByUserId(userId); - if (!appUserOctokit) { - debug("github-access", "no app user client, skipping installations"); + try { + const { getGitHubAppUserClientByUserId } = await import("./auth-runtime"); + const appUserOctokit = await getGitHubAppUserClientByUserId(userId); + if (!appUserOctokit) { + debug("github-access", "no app user client, skipping installations"); + return { installations: [], installationsAvailable: false }; + } + + const installationsResponse = await appUserOctokit.request( + "GET /user/installations", + { + per_page: 100, + }, + ); + const installations = mapGitHubAppInstallations( + installationsResponse.data as GitHubUserInstallationsPayload, + ); + debug("github-access", "loaded app installations", { + count: installations.length, + owners: installations.map((i) => i.account.login), + }); return { - installations: [], - installationsAvailable: false, - appUserOctokit: null, + installations, + installationsAvailable: true, }; + } catch (error) { + console.error("[github-access] failed to load app installations", error); + return { installations: [], installationsAvailable: false }; } - - // The app-user token exists — any failures from here on are transient - // and should propagate so the cache layer can serve stale data or the - // outer catch handles them (rather than silently failing open). - const PAGE_SIZE = 100; - const firstResponse = await appUserOctokit.request( - "GET /user/installations", - { per_page: PAGE_SIZE }, - ); - const firstPayload = firstResponse.data as GitHubUserInstallationsPayload; - const firstPage = firstPayload.installations ?? []; - const allRawInstallations = [...firstPage]; - - if (firstPage.length >= PAGE_SIZE) { - let page = 2; - while (true) { - const response = await appUserOctokit.request("GET /user/installations", { - per_page: PAGE_SIZE, - page, - }); - const payload = response.data as GitHubUserInstallationsPayload; - const pageItems = payload.installations ?? []; - allRawInstallations.push(...pageItems); - - if (pageItems.length < PAGE_SIZE) { - break; - } - page += 1; - } - } - - const installations = mapGitHubAppInstallations({ - installations: allRawInstallations, - }); - debug("github-access", "loaded app installations", { - count: installations.length, - owners: installations.map((i) => i.account.login), - }); - return { - installations, - installationsAvailable: true, - appUserOctokit, - }; } async function getGitHubAuthenticatedOrganizations( @@ -1605,184 +1567,6 @@ async function getGitHubAuthenticatedOrganizations( } } -// --------------------------------------------------------------------------- -// Installation access index — cached list of repos accessible via the app -// --------------------------------------------------------------------------- - -type SerializableInstallationAccessIndex = { - available: boolean; - allAccessOwners: string[]; - selectedRepos: string[]; -}; - -function syntheticGitHubResponseMetadata() { - return { - etag: null, - lastModified: null, - rateLimitRemaining: null, - rateLimitReset: null, - statusCode: 200, - }; -} - -async function getInstallationAccessIndex( - context: GitHubContext, -): Promise { - try { - const serializable = - await getOrRevalidateGitHubResource({ - userId: context.session.user.id, - resource: "installationAccess", - params: null, - freshForMs: githubCachePolicy.installationAccess.staleTimeMs, - signalKeys: [githubRevalidationSignalKeys.installationAccess], - namespaceKeys: [githubRevalidationSignalKeys.installationAccess], - cacheMode: "split", - fetcher: async () => { - debug("installation-access", "fetching access index (cache miss)"); - const { installations, installationsAvailable, appUserOctokit } = - await getGitHubAppUserInstallations(context.session.user.id); - - if (!installationsAvailable) { - debug( - "installation-access", - "app-user token unavailable, index not available (fail-open)", - ); - return { - kind: "success", - data: { - available: false, - allAccessOwners: [], - selectedRepos: [], - }, - metadata: syntheticGitHubResponseMetadata(), - }; - } - - debug("installation-access", "processing installations", { - count: installations.length, - owners: installations.map((i) => i.account.login), - }); - - const allAccessOwners: string[] = []; - const selectedRepos: string[] = []; - - for (const installation of installations) { - if (installation.suspendedAt) { - debug("installation-access", "skipping suspended installation", { - owner: installation.account.login, - installationId: installation.id, - }); - continue; - } - - const ownerLogin = installation.account.login.toLowerCase(); - - if (installation.repositorySelection === "all") { - debug( - "installation-access", - `owner "${ownerLogin}" has "all" repo access`, - ); - allAccessOwners.push(ownerLogin); - continue; - } - - if (installation.repositorySelection === "selected") { - try { - // Use the app-user client (not the OAuth client) — - // this endpoint requires a GitHub App user-to-server token. - const repos = await listPaginatedGitHubItems({ - request: (page) => - appUserOctokit!.rest.apps.listInstallationReposForAuthenticatedUser( - { - installation_id: installation.id, - page, - per_page: 100, - }, - ), - getItems: (payload) => - ((payload as GitHubInstallationRepositoriesPayload) - .repositories ?? []) as NonNullable< - GitHubInstallationRepositoriesPayload["repositories"] - >, - label: `installation-access repos ${installation.id}`, - }); - - const repoNames: string[] = []; - for (const repo of repos) { - const fullName = - repo.full_name ?? - (repo.owner?.login && repo.name - ? `${repo.owner.login}/${repo.name}` - : null); - if (fullName) { - const normalized = fullName.toLowerCase(); - selectedRepos.push(normalized); - repoNames.push(normalized); - } - } - - debug( - "installation-access", - `owner "${ownerLogin}" has "selected" repo access`, - { - installationId: installation.id, - repoCount: repoNames.length, - repos: repoNames, - }, - ); - } catch (error) { - console.error( - `[installation-access] failed to list repos for installation ${installation.id}`, - error, - ); - } - } - } - - debug("installation-access", "access index built", { - allAccessOwners, - selectedRepoCount: selectedRepos.length, - selectedRepos, - }); - - return { - kind: "success", - data: { - available: true, - allAccessOwners, - selectedRepos, - }, - metadata: syntheticGitHubResponseMetadata(), - }; - }, - }); - - debug("installation-access", "resolved access index", { - available: serializable.available, - allAccessOwners: serializable.allAccessOwners, - selectedRepoCount: serializable.selectedRepos.length, - selectedRepos: serializable.selectedRepos, - }); - - return { - available: serializable.available, - allAccessOwners: new Set(serializable.allAccessOwners), - selectedRepos: new Set(serializable.selectedRepos), - }; - } catch (error) { - // Transient failure (network, 500, etc.) — not cached, so the next - // request will retry. Fail-open so the current request doesn't block - // all private repos for the user. - debug( - "installation-access", - "transient error building access index, failing open", - ); - console.error("[installation-access] failed to build access index", error); - return emptyInstallationAccessIndex(); - } -} - async function getGitHubContextForInstallation( baseContext: GitHubContext, installation: GitHubAppInstallation, @@ -3567,7 +3351,6 @@ async function getPullPageDataViaGraphQL( name nameWithOwner url - isPrivate owner { login } } reviewThreads(first: 1) { totalCount } @@ -3871,7 +3654,6 @@ async function getIssuePageDataViaGraphQL( name nameWithOwner url - isPrivate owner { login } } assignees(first: 20) { @@ -4349,7 +4131,6 @@ async function getMyPullsResult({ name nameWithOwner url - isPrivate owner { login } @@ -4528,7 +4309,6 @@ async function getMyIssuesResult({ name nameWithOwner url - isPrivate owner { login } @@ -4712,46 +4492,6 @@ export const getGitHubAppAccessState = createServerFn({ }; }); -export type SerializedInstallationAccessIndex = { - available: boolean; - allAccessOwners: string[]; - selectedRepos: string[]; -}; - -export const getInstallationAccess = createServerFn({ - method: "GET", -}).handler(async (): Promise => { - const context = await getGitHubContext(); - if (!context) { - return { available: false, allAccessOwners: [], selectedRepos: [] }; - } - - const index = await getInstallationAccessIndex(context); - return { - available: index.available, - allAccessOwners: [...index.allAccessOwners], - selectedRepos: [...index.selectedRepos], - }; -}); - -/** - * Invalidates the server-side installation access cache so the next request - * fetches fresh data from GitHub. Called when the user returns from changing - * permissions on GitHub (e.g. from /setup or the access dialog). - */ -export const refreshInstallationAccess = createServerFn({ - method: "POST", -}).handler(async () => { - await markGitHubRevalidationSignals([ - githubRevalidationSignalKeys.installationAccess, - ]); - debug( - "refreshInstallationAccess", - "marked installationAccess for revalidation", - ); - return { ok: true }; -}); - export const getUserRepos = createServerFn({ method: "GET" }).handler( async (): Promise => { const context = await getGitHubContext(); @@ -4759,69 +4499,35 @@ export const getUserRepos = createServerFn({ method: "GET" }).handler( return []; } - const [repos, accessIndex] = await Promise.all([ - getCachedGitHubRequest({ - context, - resource: "repos.list", - params: { sort: "updated", perPage: 10 }, - freshForMs: githubCachePolicy.reposList.staleTimeMs, - namespaceKeys: ["repos.list"], - cacheMode: "split", - request: (headers) => - context.octokit.rest.repos.listForAuthenticatedUser({ - sort: "updated", - per_page: 10, - headers, + return getCachedGitHubRequest({ + context, + resource: "repos.list", + params: { sort: "updated", perPage: 10 }, + freshForMs: githubCachePolicy.reposList.staleTimeMs, + namespaceKeys: ["repos.list"], + cacheMode: "split", + request: (headers) => + context.octokit.rest.repos.listForAuthenticatedUser({ + sort: "updated", + per_page: 10, + headers, + }), + mapData: (repos) => + repos.map( + (repo: AuthenticatedUserRepo): UserRepoSummary => ({ + id: repo.id, + name: repo.name, + fullName: repo.full_name, + description: repo.description, + stars: repo.stargazers_count, + language: repo.language, + updatedAt: repo.updated_at, + isPrivate: repo.private, + url: repo.html_url, + owner: repo.owner.login, }), - mapData: (repos) => - repos.map( - (repo: AuthenticatedUserRepo): UserRepoSummary => ({ - id: repo.id, - name: repo.name, - fullName: repo.full_name, - description: repo.description, - stars: repo.stargazers_count, - language: repo.language, - updatedAt: repo.updated_at, - isPrivate: repo.private, - url: repo.html_url, - owner: repo.owner.login, - }), - ), - }), - getInstallationAccessIndex(context), - ]); - - const filtered = repos.filter((repo) => - isRepoVisibleWithInstallationAccess( - accessIndex, - repo.owner, - repo.name, - repo.isPrivate, - ), - ); - - const removedCount = repos.length - filtered.length; - if (removedCount > 0) { - debug("installation-access", "getUserRepos filtered", { - total: repos.length, - kept: filtered.length, - removed: removedCount, - removedRepos: repos - .filter( - (repo) => - !isRepoVisibleWithInstallationAccess( - accessIndex, - repo.owner, - repo.name, - repo.isPrivate, - ), - ) - .map((repo) => repo.fullName), - }); - } - - return filtered; + ), + }); }, ); @@ -4842,7 +4548,7 @@ export const searchCommandPaletteGitHub = createServerFn({ method: "GET" }) const login = viewer.login; const perPage = clampCommandSearchPerPage(data.perPage); - const [pullItems, issueItems, accessIndex] = await Promise.all([ + const [pullItems, issueItems] = await Promise.all([ safeCommandPaletteSearch({ label: "pull requests", fallback: [] as SearchItem[], @@ -4873,87 +4579,14 @@ export const searchCommandPaletteGitHub = createServerFn({ method: "GET" }) return response.data.items; }, }), - getInstallationAccessIndex(context), ]); return { - pulls: filterItemsByInstallationAccess( - mapPullSearchItems(pullItems), - accessIndex, - ), - issues: filterItemsByInstallationAccess( - mapIssueSearchItems(issueItems), - accessIndex, - ), + pulls: mapPullSearchItems(pullItems), + issues: mapIssueSearchItems(issueItems), }; }); -function filterItemsByInstallationAccess< - T extends { repository: RepositoryRef }, ->(items: T[], accessIndex: GitHubInstallationAccessIndex): T[] { - const filtered = items.filter((item) => - isRepoVisibleWithInstallationAccess( - accessIndex, - item.repository.owner, - item.repository.name, - item.repository.isPrivate, - ), - ); - - const removedCount = items.length - filtered.length; - if (removedCount > 0) { - const removed = items - .filter( - (item) => - !isRepoVisibleWithInstallationAccess( - accessIndex, - item.repository.owner, - item.repository.name, - item.repository.isPrivate, - ), - ) - .map((item) => item.repository.fullName); - - debug("installation-access", "filtered items by access scope", { - total: items.length, - kept: filtered.length, - removed: removedCount, - removedRepos: [...new Set(removed)], - }); - } - - return filtered; -} - -function filterMyPullsResult( - result: MyPullsResult, - accessIndex: GitHubInstallationAccessIndex, -): MyPullsResult { - return { - ...result, - reviewRequested: filterItemsByInstallationAccess( - result.reviewRequested, - accessIndex, - ), - assigned: filterItemsByInstallationAccess(result.assigned, accessIndex), - authored: filterItemsByInstallationAccess(result.authored, accessIndex), - mentioned: filterItemsByInstallationAccess(result.mentioned, accessIndex), - involved: filterItemsByInstallationAccess(result.involved, accessIndex), - }; -} - -function filterMyIssuesResult( - result: MyIssuesResult, - accessIndex: GitHubInstallationAccessIndex, -): MyIssuesResult { - return { - ...result, - assigned: filterItemsByInstallationAccess(result.assigned, accessIndex), - authored: filterItemsByInstallationAccess(result.authored, accessIndex), - mentioned: filterItemsByInstallationAccess(result.mentioned, accessIndex), - }; -} - function toInstallationTargetType( value: string | undefined, ): GitHubInstallationTargetType { @@ -4978,11 +4611,7 @@ export const getMyPulls = createServerFn({ method: "GET" }).handler( } const viewer = await getViewer(context); - const [result, accessIndex] = await Promise.all([ - getMyPullsResult({ context, username: viewer.login }), - getInstallationAccessIndex(context), - ]); - return filterMyPullsResult(result, accessIndex); + return getMyPullsResult({ context, username: viewer.login }); }, ); @@ -5136,11 +4765,7 @@ export const getMyIssues = createServerFn({ method: "GET" }).handler( } const viewer = await getViewer(context); - const [result, accessIndex] = await Promise.all([ - getMyIssuesResult({ context, username: viewer.login }), - getInstallationAccessIndex(context), - ]); - return filterMyIssuesResult(result, accessIndex); + return getMyIssuesResult({ context, username: viewer.login }); }, ); @@ -6602,8 +6227,6 @@ export const getUserPinnedRepos = createServerFn({ method: "GET" }) return []; } - const accessIndex = await getInstallationAccessIndex(context); - try { const response: { user: { @@ -6644,37 +6267,7 @@ export const getUserPinnedRepos = createServerFn({ method: "GET" }) { username: data.username }, ); - const allPinned = response.user.pinnedItems.nodes; - const visiblePinned = allPinned.filter((repo) => - isRepoVisibleWithInstallationAccess( - accessIndex, - repo.owner.login, - repo.name, - repo.isPrivate, - ), - ); - - const removedCount = allPinned.length - visiblePinned.length; - if (removedCount > 0) { - debug("installation-access", "getUserPinnedRepos filtered", { - total: allPinned.length, - kept: visiblePinned.length, - removed: removedCount, - removedRepos: allPinned - .filter( - (repo) => - !isRepoVisibleWithInstallationAccess( - accessIndex, - repo.owner.login, - repo.name, - repo.isPrivate, - ), - ) - .map((repo) => `${repo.owner.login}/${repo.name}`), - }); - } - - return visiblePinned.map((repo) => ({ + return response.user.pinnedItems.nodes.map((repo) => ({ name: repo.name, description: repo.description, stars: repo.stargazerCount, @@ -7571,14 +7164,14 @@ export const getNotifications = createServerFn({ method: "GET" }) return { notifications: [] }; } - const [response, accessIndex] = await Promise.all([ - context.octokit.rest.activity.listNotificationsForAuthenticatedUser({ - all: data.all ?? false, - participating: data.participating ?? false, - per_page: 50, - }), - getInstallationAccessIndex(context), - ]); + const response = + await context.octokit.rest.activity.listNotificationsForAuthenticatedUser( + { + all: data.all ?? false, + participating: data.participating ?? false, + per_page: 50, + }, + ); // Batch-fetch participants for PR/Issue notifications in parallel const participantMap = new Map(); @@ -7695,40 +7288,7 @@ export const getNotifications = createServerFn({ method: "GET" }) url: n.url, })); - const filteredNotifications = notifications.filter((notification) => - isRepoVisibleWithInstallationAccess( - accessIndex, - notification.repository.owner.login, - notification.repository.name, - notification.repository.private, - ), - ); - - const removedCount = notifications.length - filteredNotifications.length; - if (removedCount > 0) { - debug("installation-access", "getNotifications filtered", { - total: notifications.length, - kept: filteredNotifications.length, - removed: removedCount, - removedRepos: [ - ...new Set( - notifications - .filter( - (n) => - !isRepoVisibleWithInstallationAccess( - accessIndex, - n.repository.owner.login, - n.repository.name, - n.repository.private, - ), - ) - .map((n) => n.repository.fullName), - ), - ], - }); - } - - return { notifications: filteredNotifications }; + return { notifications }; }); type MarkNotificationReadInput = { threadId: string }; diff --git a/apps/dashboard/src/lib/github.types.ts b/apps/dashboard/src/lib/github.types.ts index 7873330..7fd11a4 100644 --- a/apps/dashboard/src/lib/github.types.ts +++ b/apps/dashboard/src/lib/github.types.ts @@ -3,8 +3,6 @@ export type RepositoryRef = { owner: string; fullName: string; url: string; - /** `null` means the visibility is unknown (e.g. REST search doesn't return it). */ - isPrivate: boolean | null; }; export type GitHubActor = { diff --git a/apps/dashboard/src/lib/use-refresh-on-return.ts b/apps/dashboard/src/lib/use-refresh-on-return.ts deleted file mode 100644 index 62a6c49..0000000 --- a/apps/dashboard/src/lib/use-refresh-on-return.ts +++ /dev/null @@ -1,59 +0,0 @@ -import { useQueryClient } from "@tanstack/react-query"; -import { useRouter } from "@tanstack/react-router"; -import { useCallback, useEffect, useRef } from "react"; -import { refreshInstallationAccess } from "./github.functions"; -import { githubQueryKeys } from "./github.query"; - -/** - * Listens for the tab becoming visible again (user returning from an external - * site like GitHub) and refreshes the installation access cache + all GitHub - * queries so the UI reflects any permission changes. - * - * Only fires once per hidden→visible transition to avoid duplicate work. - */ -export function useRefreshOnReturn({ - enabled = true, -}: { - enabled?: boolean; -} = {}) { - const queryClient = useQueryClient(); - const router = useRouter(); - const wasHiddenRef = useRef(false); - - const refresh = useCallback(async () => { - try { - await refreshInstallationAccess(); - } finally { - void queryClient.invalidateQueries({ - queryKey: githubQueryKeys.all, - }); - void queryClient.invalidateQueries({ - queryKey: ["github-app-access-state"], - }); - void router.invalidate(); - } - }, [queryClient, router]); - - useEffect(() => { - if (!enabled) return; - - function handleVisibilityChange() { - if (document.hidden) { - wasHiddenRef.current = true; - return; - } - - if (wasHiddenRef.current) { - wasHiddenRef.current = false; - void refresh(); - } - } - - document.addEventListener("visibilitychange", handleVisibilityChange); - return () => { - document.removeEventListener("visibilitychange", handleVisibilityChange); - }; - }, [enabled, refresh]); - - return refresh; -} diff --git a/apps/dashboard/src/routes/setup.tsx b/apps/dashboard/src/routes/setup.tsx index cae42e2..ee1adc4 100644 --- a/apps/dashboard/src/routes/setup.tsx +++ b/apps/dashboard/src/routes/setup.tsx @@ -11,7 +11,6 @@ import { getAccessHrefForOwner, } from "#/lib/github-access"; import { buildSeo, formatPageTitle, PRIVATE_ROUTE_HEADERS } from "#/lib/seo"; -import { useRefreshOnReturn } from "#/lib/use-refresh-on-return"; function SetupPageLoading() { return ( @@ -53,7 +52,6 @@ export const Route = createFileRoute("/setup")({ function SetupPage() { const { accessState: state } = Route.useLoaderData(); - useRefreshOnReturn(); const hasInstallations = state?.installationsAvailable === true &&