From 6b8d0b3d781f606127ab0f454fcac98abdbd6a1c Mon Sep 17 00:00:00 2001 From: ARUNAVO RAY Date: Wed, 9 Sep 2026 11:24:50 +0530 Subject: [PATCH] fix(public-orgs): finish the destination-only paths for tokenless sources (#411) Follow-up to #409. Public-only accounts could add and mirror an organization, then hit the source-token gates that were left behind everywhere after the first mirror. Each item was reproduced against a running instance with only a destination configured and a tokenless GitHub source, and re-checked after the fix. **The paths that still demanded a source token** - `POST /api/job/sync-repo` and `POST /api/job/schedule-sync-repo` answered 400 "Config missing for the user or token", so the Sync button never worked for these accounts. Neither route uses that token; they dispatch to the destination and resolve source clients per repository. Both now gate on the destination token, like the mirror and retry routes already do. - `recovery.ts` threw "GitHub token not found in configuration" before the per-source resolution right below it, so an interrupted mirror never recovered. The gate is gone and a tokenless GitHub source now gets the same anonymous client the job routes build. - `mirrorGitRepoIssuesToGitea` and the pull request, label and milestone functions required the config token even though the client arrives as a parameter, so with the anonymous client they threw and the per-component try/catch logged an error on every sync. They now require only the destination, and public metadata mirrors. **The bulk import** `POST /api/sync` iterated every source, including tokenless ones whose personal listing 401s, counted them as failed, and answered 502 when there was nothing else. It now skips them the way `importRepositoriesFromSources` does, reports them as `skippedPublicSources`, and says so when every source is public only. **The organization pin** `force: true` without a `sourceId` or `provider` re-pinned the organization to the primary source. Reproduced with two sources: an organization pinned to the GitLab source and one deliberately left on every source were both moved onto the primary. Only a request that names a source re-pins now; naming a provider (public mode) still re-pins as #409 intended. Covered by a new route test. **The badge** `invalidateConfigCache()` cleared the cache but nothing re-read it, so the Public only badge appeared only after a reload. Mounted `useConfigStatus` hooks now refresh on invalidation, through one shared in-flight request so a page with several of them still makes a single call. Two things from the review I deliberately did not change: the anonymous 60 requests an hour budget with its long `onRateLimit` waits, and `rediscoverOrganizationRepositories` running for every user rather than only tokenless sources. Both are judgement calls about the feature rather than defects. Tests: 906 pass. Build passes. Type errors in the touched files are unchanged from main. --- CHANGELOG.md | 5 +++ src/hooks/useConfigStatus.ts | 49 ++++++++++++++++++++++--- src/lib/gitea.ts | 27 ++++++-------- src/lib/recovery.ts | 15 ++++---- src/pages/api/job/schedule-sync-repo.ts | 5 ++- src/pages/api/job/sync-repo.ts | 7 +++- src/pages/api/sync/index.ts | 23 ++++++++++-- src/pages/api/sync/organization.test.ts | 45 +++++++++++++++++++++++ src/pages/api/sync/organization.ts | 5 ++- 9 files changed, 145 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76a67a7..1fbf121 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- Public organizations without a source connection (#409) + - The Add Organization dialog offers a Public only mode, the default when no source is connected: pick GitHub, GitLab or Gitea/Forgejo, optionally an instance URL, and the organization is imported anonymously + - A tokenless source row is found or created for that provider and host and the organization is pinned to it, so attribution, locks, the scheduler and cleanup keep working per source + - Anonymous GitHub clients keep the throttling and rate-limit backoff of authenticated ones, and the scheduler, per-repo mirror, retry, sync and recovery paths gate on the destination token instead of a source token + - Sources with no username and no token are saved and shown as Public only, and the organizations and repositories pages work with no configured source - Per-organization source selection for multi-source accounts - Organizations remember which source they import and mirror from; migration 0020 backfills the pin from each organization's repositories when they agree on one - Source picker in the add-organization dialog and on organization cards when more than one source is connected diff --git a/src/hooks/useConfigStatus.ts b/src/hooks/useConfigStatus.ts index 7fb455f..94e239f 100644 --- a/src/hooks/useConfigStatus.ts +++ b/src/hooks/useConfigStatus.ts @@ -34,6 +34,28 @@ let configCache: { data: ConfigApiResponse | null; timestamp: number; userId: st const CACHE_DURATION = 30000; // 30 seconds cache +// Mounted hooks, so invalidating the cache refreshes what is on screen +// instead of only clearing the store behind it. +const cacheSubscribers = new Set<() => void>(); + +// One in-flight GET shared by every mounted hook: an invalidation wakes all +// of them at once, and without this each would fire its own request. +let inFlightRequest: { userId: string; promise: Promise } | null = null; + +function fetchConfigOnce(userId: string): Promise { + if (inFlightRequest && inFlightRequest.userId === userId) { + return inFlightRequest.promise; + } + const promise = apiRequest(`/config?userId=${userId}`, { method: 'GET' }); + const tracked = promise.finally(() => { + if (inFlightRequest?.promise === tracked) { + inFlightRequest = null; + } + }); + inFlightRequest = { userId, promise: tracked }; + return tracked; +} + /** * Hook to check if GitHub and Gitea are properly configured * Returns configuration status and prevents unnecessary API calls when not configured @@ -127,10 +149,7 @@ export function useConfigStatus(): ConfigStatus { setConfigStatus(prev => ({ ...prev, isLoading: true, error: null })); } - const configResponse = await apiRequest( - `/config?userId=${user.id}`, - { method: 'GET' } - ); + const configResponse = await fetchConfigOnce(user.id); // Update cache configCache = { @@ -196,12 +215,32 @@ export function useConfigStatus(): ConfigStatus { checkConfiguration(); }, [checkConfiguration]); + // Re-read after invalidateConfigCache(): saving the configuration or + // adding a public organization (which creates its tokenless source row) + // has to reach the components already on screen, or source-dependent UI + // like the organization card's "Public only" badge stays stale until a + // reload. + useEffect(() => { + const refresh = () => { + void checkConfiguration(); + }; + cacheSubscribers.add(refresh); + return () => { + cacheSubscribers.delete(refresh); + }; + }, [checkConfiguration]); + return configStatus; } -// Export function to invalidate cache when config is updated +// Export function to invalidate cache when config is updated. Mounted hooks +// re-fetch (through the shared in-flight request, so this costs one call). export function invalidateConfigCache() { configCache = { data: null, timestamp: 0, userId: null }; + inFlightRequest = null; + for (const refresh of [...cacheSubscribers]) { + refresh(); + } } // Export function to get cached config data for other hooks diff --git a/src/lib/gitea.ts b/src/lib/gitea.ts index 32c65aa..150fbd4 100644 --- a/src/lib/gitea.ts +++ b/src/lib/gitea.ts @@ -2531,13 +2531,14 @@ export const mirrorGitRepoIssuesToGitea = async ({ giteaRepoName?: string; }) => { //things covered here are- issue, title, body, labels, comments and assignees + // The source client arrives as `octokit` (anonymous for a public-only + // source), so only the destination has to be configured here. if ( - !config.githubConfig?.token || !config.giteaConfig?.token || !config.giteaConfig?.url || !config.giteaConfig?.defaultOwner ) { - throw new Error("Missing GitHub or Gitea configuration."); + throw new Error("Missing destination configuration."); } // Decrypt config tokens for API usage @@ -3601,13 +3602,13 @@ export async function mirrorGitRepoPullRequestsToGitea({ giteaOwner: string; giteaRepoName?: string; }) { + // The source client arrives as `octokit`; see mirrorGitRepoIssuesToGitea. if ( - !config.githubConfig?.token || !config.giteaConfig?.token || !config.giteaConfig?.url || !config.giteaConfig?.defaultOwner ) { - throw new Error("Missing GitHub or Gitea configuration."); + throw new Error("Missing destination configuration."); } // Decrypt config tokens for API usage @@ -4053,12 +4054,9 @@ export async function mirrorGitRepoLabelsToGitea({ giteaOwner: string; giteaRepoName?: string; }) { - if ( - !config.githubConfig?.token || - !config.giteaConfig?.token || - !config.giteaConfig?.url - ) { - throw new Error("Missing GitHub or Gitea configuration."); + // The source client arrives as `octokit`; see mirrorGitRepoIssuesToGitea. + if (!config.giteaConfig?.token || !config.giteaConfig?.url) { + throw new Error("Missing destination configuration."); } // Decrypt config tokens for API usage @@ -4187,12 +4185,9 @@ export async function mirrorGitRepoMilestonesToGitea({ giteaOwner: string; giteaRepoName?: string; }) { - if ( - !config.githubConfig?.token || - !config.giteaConfig?.token || - !config.giteaConfig?.url - ) { - throw new Error("Missing GitHub or Gitea configuration."); + // The source client arrives as `octokit`; see mirrorGitRepoIssuesToGitea. + if (!config.giteaConfig?.token || !config.giteaConfig?.url) { + throw new Error("Missing destination configuration."); } // Decrypt config tokens for API usage diff --git a/src/lib/recovery.ts b/src/lib/recovery.ts index b61d8c9..b0e8c7a 100644 --- a/src/lib/recovery.ts +++ b/src/lib/recovery.ts @@ -8,7 +8,7 @@ import { resetStuckMirrorStatuses } from './stuck-status-recovery'; import { db, repositories, organizations, mirrorJobs, configs } from './db'; import { eq, and, lt, inArray, sql } from 'drizzle-orm'; import { mirrorRepositoryToDestination, syncRepositoryOnDestination } from './mirror-dispatch'; -import { createGitHubClient } from './github'; +import { createGitHubClient, createPublicGitHubClient } from './github'; import type { Octokit } from '@octokit/rest'; import { decryptSourceToken, @@ -272,11 +272,8 @@ async function recoverMirrorJob(job: any, remainingItemIds: string[]) { console.log(`Found ${repos.length} repositories to process for recovery`); - // Validate GitHub configuration before creating client - if (!config.githubConfig?.token) { - throw new Error('GitHub token not found in configuration'); - } - + // No source-token check here: the client is resolved from each + // repository's own source below, and a public-only source has none. // Only GitHub sources use an API client while mirroring (metadata). // Other hosts get code-only mirrors through Gitea, so no client is built. // Repositories picked by bare id can span sources, so the sources are @@ -304,6 +301,10 @@ async function recoverMirrorJob(job: any, remainingItemIds: string[]) { if (repoSource?.provider === 'github') { try { const decryptedToken = decryptSourceToken(repoSource.token); + // A tokenless GitHub source gets the anonymous public client + // (60 req/hr), the same resolution the job routes use; an empty + // token must never reach createGitHubClient, which would send + // `auth: ""` and 401 on every call. octokit = decryptedToken ? createGitHubClient( decryptedToken, @@ -311,7 +312,7 @@ async function recoverMirrorJob(job: any, remainingItemIds: string[]) { repoSource.username || undefined, resolveGitHubApiBaseUrl(repoSource.url) ) - : null; + : createPublicGitHubClient(resolveGitHubApiBaseUrl(repoSource.url)); } catch (error) { throw new Error(`Failed to create GitHub client: ${error instanceof Error ? error.message : String(error)}`); } diff --git a/src/pages/api/job/schedule-sync-repo.ts b/src/pages/api/job/schedule-sync-repo.ts index 16bb8cf..3371b72 100644 --- a/src/pages/api/job/schedule-sync-repo.ts +++ b/src/pages/api/job/schedule-sync-repo.ts @@ -32,11 +32,12 @@ export const POST: APIRoute = async ({ request, locals }) => { const config = configResult[0]; - if (!config || !config.githubConfig.token) { + // Scheduling a sync only needs the destination; see sync-repo.ts. + if (!config || !config.giteaConfig?.token) { return new Response( JSON.stringify({ success: false, - error: "Config missing for the user or GitHub token not found.", + error: "Config missing for the user or destination token not found.", repositories: [], } satisfies ScheduleSyncRepoResponse), { status: 400, headers: { "Content-Type": "application/json" } } diff --git a/src/pages/api/job/sync-repo.ts b/src/pages/api/job/sync-repo.ts index fa6bf93..cd544fd 100644 --- a/src/pages/api/job/sync-repo.ts +++ b/src/pages/api/job/sync-repo.ts @@ -49,9 +49,12 @@ export const POST: APIRoute = async ({ request, locals }) => { const config = configResult[0]; - if (!config || !config.githubConfig.token) { + // Syncing only touches the destination: the source client is resolved + // per repository further down the dispatch, and a public-only source has + // no token at all. Requiring one here locked those accounts out of Sync. + if (!config || !config.giteaConfig?.token) { return new Response( - JSON.stringify({ error: "Config missing for the user or token." }), + JSON.stringify({ error: "Config missing for the user or destination token." }), { status: 400, headers: { "Content-Type": "application/json" } } ); } diff --git a/src/pages/api/sync/index.ts b/src/pages/api/sync/index.ts index 5848592..ffdabf1 100644 --- a/src/pages/api/sync/index.ts +++ b/src/pages/api/sync/index.ts @@ -40,7 +40,7 @@ export const POST: APIRoute = async ({ request, locals }) => { // is the primary source). A source without a token still lists public // repositories through its provider. Configs written outside the sources // API get their first source row seeded here. - const { listSources, ensureSourcesFromConfig, selectSameRunPinsToClear } = await import("@/lib/sources"); + const { listSources, ensureSourcesFromConfig, selectSameRunPinsToClear, decryptSourceToken } = await import("@/lib/sources"); const { createSourceProviderFromSource } = await import("@/lib/source-providers"); await ensureSourcesFromConfig(userId); const sources = (await listSources(userId)).filter((source) => source.enabled); @@ -70,6 +70,7 @@ export const POST: APIRoute = async ({ request, locals }) => { let totalSkippedDisabled = 0; const failedOrgNames: string[] = []; const failedSourceNames: string[] = []; + const skippedPublicSourceNames: string[] = []; // Organization pins this run set itself (inserted, or re-pinned on // recovery), by normalized name. A later source listing the same name @@ -79,6 +80,17 @@ export const POST: APIRoute = async ({ request, locals }) => { for (const source of sources) { try { + // A public-only (tokenless) source has no account to list from: the + // personal and starred listings 401 on every call, which would count + // the source as failed and, when it is the only one, answer 502. Its + // organizations are added through the add-organization route and + // re-listed by the scheduler, which skips these sources the same way. + if (decryptSourceToken(source.token) === "") { + console.log(`Skipping personal discovery for public-only source ${source.name} (${source.id})`); + skippedPublicSourceNames.push(source.name); + continue; + } + const sourceProvider = createSourceProviderFromSource(source, { userId }); const sourceLabel = SOURCE_PROVIDER_LABELS[sourceProvider.kind]; @@ -316,7 +328,8 @@ export const POST: APIRoute = async ({ request, locals }) => { } } - if (sources.length > 0 && failedSourceNames.length === sources.length) { + const attemptedSourceCount = sources.length - skippedPublicSourceNames.length; + if (attemptedSourceCount > 0 && failedSourceNames.length === attemptedSourceCount) { return createSecureErrorResponse( new Error("every enabled source failed to sync"), "source data sync", @@ -327,12 +340,16 @@ export const POST: APIRoute = async ({ request, locals }) => { return jsonResponse({ data: { success: true, - message: "Repositories and organizations synced successfully", + message: + attemptedSourceCount === 0 && skippedPublicSourceNames.length > 0 + ? "Nothing to import: every connected source is public only, so it has no account to list repositories from. Public organizations are imported from the Organizations page." + : "Repositories and organizations synced successfully", newRepositories: totalInsertedRepos, newOrganizations: totalInsertedOrgs, skippedDisabledRepositories: totalSkippedDisabled, failedOrgs: failedOrgNames, failedSources: failedSourceNames, + skippedPublicSources: skippedPublicSourceNames, recoveredOrgs: totalRecoveredOrgs, }, }); diff --git a/src/pages/api/sync/organization.test.ts b/src/pages/api/sync/organization.test.ts index 22e47b4..2851a66 100644 --- a/src/pages/api/sync/organization.test.ts +++ b/src/pages/api/sync/organization.test.ts @@ -454,6 +454,51 @@ describe.skipIf(!isChild)("POST /api/sync/organization public add-org", () => { expect(insertedRepoRows).toHaveLength(0); }); + test("force:true without a named source leaves the existing pin alone", async () => { + const primary = makeSourceRow({ + provider: "github", + url: "https://github.com", + username: "octocat", + token: "enc:gh-token", + }); + const other = makeSourceRow({ + provider: "gitlab", + url: "https://gitlab.com", + username: "release-bot", + token: "enc:gl-token", + }); + sourceRows = [primary, other]; + orgRows = [ + { + id: "org-1", + userId: "user-1", + name: "acme", + normalizedName: "acme", + membershipRole: "admin", + sourceId: other.id, + }, + { + id: "org-2", + userId: "user-1", + name: "widgets", + normalizedName: "widgets", + membershipRole: "admin", + sourceId: null, + }, + ]; + + const pinned = await postOrg({ org: "acme", role: "member", force: true }); + expect(pinned.status).toBe(200); + expect(orgRows[0].sourceId).toBe(other.id); + + const unpinned = await postOrg({ org: "widgets", role: "member", force: true }); + expect(unpinned.status).toBe(200); + expect(orgRows[1].sourceId).toBeNull(); + + // Nothing was created to hold a new pin either. + expect(sourceRows).toHaveLength(2); + }); + test("a rate-limited org metadata fetch (403) still imports with a fallback record", async () => { orgMetadataError = Object.assign( new Error("rate limit exceeded"), diff --git a/src/pages/api/sync/organization.ts b/src/pages/api/sync/organization.ts index 972ca4e..4d50390 100644 --- a/src/pages/api/sync/organization.ts +++ b/src/pages/api/sync/organization.ts @@ -161,7 +161,10 @@ export const POST: APIRoute = async ({ request, locals }) => { .set({ membershipRole: role, normalizedName: normalizedOrg, - ...(source ? { sourceId: source.id } : {}), + // Only a request that named a source (an explicit pin, or public + // mode's provider) re-pins; a bare force re-add must leave the + // organization's own pin, including "every source", alone. + ...(source && (sourceId || body.provider) ? { sourceId: source.id } : {}), updatedAt: new Date(), }) .where(eq(organizations.id, existingOrg.id))