From 417538449d5654f7bfa7ca09295d0e5b328ec8f4 Mon Sep 17 00:00:00 2001 From: Samuel Olabode Date: Thu, 6 Aug 2026 07:38:53 -0600 Subject: [PATCH 1/6] feat: add shared GitHub list pagination helpers Walk every page of GitHub list endpoints so callers can reuse one implementation instead of hard-coding the first 100 items. --- backend/services/githubPagination.js | 77 +++++++++++++++++++++ backend/tests/githubPagination.test.js | 95 ++++++++++++++++++++++++++ 2 files changed, 172 insertions(+) create mode 100644 backend/services/githubPagination.js create mode 100644 backend/tests/githubPagination.test.js diff --git a/backend/services/githubPagination.js b/backend/services/githubPagination.js new file mode 100644 index 0000000..18eb2bb --- /dev/null +++ b/backend/services/githubPagination.js @@ -0,0 +1,77 @@ +/** + * Shared GitHub list pagination helpers. + * + * GitHub caps each page at 100 items. Callers that previously fetched only + * page 1 silently dropped everything after that — these helpers walk pages + * until a short/empty page, keeping the common ≤100 case as a single request. + */ + +const DEFAULT_PER_PAGE = 100; +const DEFAULT_MAX_PAGES = 50; + +/** + * Collect every item across paginated GitHub list responses. + * + * @template T + * @param {(page: number, perPage: number) => Promise} fetchPage + * @param {{ perPage?: number, maxPages?: number }} [options] + * @returns {Promise} + */ +async function collectAllGitHubPages(fetchPage, options = {}) { + const perPage = options.perPage ?? DEFAULT_PER_PAGE; + const maxPages = options.maxPages ?? DEFAULT_MAX_PAGES; + /** @type {T[]} */ + const all = []; + + for (let page = 1; page <= maxPages; page += 1) { + const items = await fetchPage(page, perPage); + if (!Array.isArray(items) || items.length === 0) { + break; + } + all.push(...items); + if (items.length < perPage) { + break; + } + } + + return all; +} + +/** + * Scan paginated results until `predicate` matches, then stop. + * Prefer this when looking for a single repo so large installations + * do not force a full crawl after a hit. + * + * @template T + * @param {(page: number, perPage: number) => Promise} fetchPage + * @param {(item: T) => boolean} predicate + * @param {{ perPage?: number, maxPages?: number }} [options] + * @returns {Promise} + */ +async function findInGitHubPages(fetchPage, predicate, options = {}) { + const perPage = options.perPage ?? DEFAULT_PER_PAGE; + const maxPages = options.maxPages ?? DEFAULT_MAX_PAGES; + + for (let page = 1; page <= maxPages; page += 1) { + const items = await fetchPage(page, perPage); + if (!Array.isArray(items) || items.length === 0) { + return null; + } + const hit = items.find(predicate); + if (hit) { + return hit; + } + if (items.length < perPage) { + return null; + } + } + + return null; +} + +module.exports = { + DEFAULT_PER_PAGE, + DEFAULT_MAX_PAGES, + collectAllGitHubPages, + findInGitHubPages, +}; diff --git a/backend/tests/githubPagination.test.js b/backend/tests/githubPagination.test.js new file mode 100644 index 0000000..8e0b8ba --- /dev/null +++ b/backend/tests/githubPagination.test.js @@ -0,0 +1,95 @@ +/** + * Unit tests for shared GitHub pagination helpers. + */ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const { + collectAllGitHubPages, + findInGitHubPages, + DEFAULT_PER_PAGE, +} = require('../services/githubPagination'); + +test('collectAllGitHubPages returns a single page when under the page size', async () => { + const calls = []; + const items = await collectAllGitHubPages(async (page, perPage) => { + calls.push({ page, perPage }); + return ['a', 'b', 'c']; + }); + assert.deepEqual(items, ['a', 'b', 'c']); + assert.deepEqual(calls, [{ page: 1, perPage: DEFAULT_PER_PAGE }]); +}); + +test('collectAllGitHubPages walks every full page then stops on a short page', async () => { + const calls = []; + const items = await collectAllGitHubPages( + async (page, perPage) => { + calls.push({ page, perPage }); + if (page === 1) return Array.from({ length: perPage }, (_, i) => `p1-${i}`); + if (page === 2) return Array.from({ length: perPage }, (_, i) => `p2-${i}`); + return ['last']; + }, + { perPage: 3 }, + ); + assert.equal(items.length, 7); + assert.deepEqual(calls, [ + { page: 1, perPage: 3 }, + { page: 2, perPage: 3 }, + { page: 3, perPage: 3 }, + ]); + assert.equal(items[0], 'p1-0'); + assert.equal(items[6], 'last'); +}); + +test('collectAllGitHubPages stops on an empty page', async () => { + const calls = []; + const items = await collectAllGitHubPages( + async (page) => { + calls.push(page); + if (page === 1) return ['x', 'y']; + return []; + }, + { perPage: 2 }, + ); + assert.deepEqual(items, ['x', 'y']); + assert.deepEqual(calls, [1, 2]); +}); + +test('collectAllGitHubPages respects maxPages', async () => { + const calls = []; + const items = await collectAllGitHubPages( + async (page, perPage) => { + calls.push(page); + return Array.from({ length: perPage }, (_, i) => `${page}-${i}`); + }, + { perPage: 2, maxPages: 2 }, + ); + assert.equal(items.length, 4); + assert.deepEqual(calls, [1, 2]); +}); + +test('findInGitHubPages returns the first match and stops paginating', async () => { + const calls = []; + const hit = await findInGitHubPages( + async (page, perPage) => { + calls.push(page); + if (page === 1) return Array.from({ length: perPage }, (_, i) => ({ id: i })); + return [{ id: 99 }, { id: 100 }]; + }, + (item) => item.id === 99, + { perPage: 2 }, + ); + assert.deepEqual(hit, { id: 99 }); + assert.deepEqual(calls, [1, 2]); +}); + +test('findInGitHubPages returns null when nothing matches', async () => { + const hit = await findInGitHubPages( + async (page, perPage) => { + if (page === 1) return Array.from({ length: perPage }, (_, i) => ({ id: i })); + return [{ id: 2 }]; + }, + (item) => item.id === 404, + { perPage: 2 }, + ); + assert.equal(hit, null); +}); From 29e2614b1c3847775fc9fdbd1ff9d1acc740df1d Mon Sep 17 00:00:00 2001 From: Samuel Olabode Date: Thu, 6 Aug 2026 07:39:08 -0600 Subject: [PATCH 2/6] feat: paginate GitHub repository listing beyond 100 results listGitHubRepos now walks every page so Connect can show repos past the first hundred without changing the single-page path for small accounts. --- backend/services/storageService.js | 15 ++++++++++----- backend/tests/storageService.test.js | 26 ++++++++++++++++++++++---- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/backend/services/storageService.js b/backend/services/storageService.js index 55de330..73c0ed6 100644 --- a/backend/services/storageService.js +++ b/backend/services/storageService.js @@ -8,6 +8,7 @@ const crypto = require('crypto'); const { randomUUID } = require('crypto'); const { normalizeGitHubRepoName } = require('../../shared/githubRepoName'); +const { collectAllGitHubPages } = require('./githubPagination'); const MANIFEST_PATH = 'vizably.json'; /** Pre-rename store root — still loadable; rewritten to `MANIFEST_PATH` on load. */ @@ -41,11 +42,15 @@ class StorageService { * @returns {Promise>} */ async listGitHubRepos(githubClient) { - const { data } = await githubClient.rest.repos.listForAuthenticatedUser({ - visibility: 'all', - affiliation: 'owner,collaborator,organization_member', - per_page: 100, - sort: 'updated', + const data = await collectAllGitHubPages(async (page, perPage) => { + const { data: pageData } = await githubClient.rest.repos.listForAuthenticatedUser({ + visibility: 'all', + affiliation: 'owner,collaborator,organization_member', + per_page: perPage, + page, + sort: 'updated', + }); + return pageData; }); return data.map((repo) => ({ diff --git a/backend/tests/storageService.test.js b/backend/tests/storageService.test.js index 07abd03..3e0616e 100644 --- a/backend/tests/storageService.test.js +++ b/backend/tests/storageService.test.js @@ -76,16 +76,18 @@ function createMockGitHubClient(initial = {}) { }), }, repos: { - listForAuthenticatedUser: async () => ({ - data: [ + listForAuthenticatedUser: async ({ page = 1, per_page = 100 } = {}) => { + const all = initial.listedRepos ?? [ { node_id: STORAGE_REF.id, full_name: STORAGE_REF.full_name, private: true, html_url: STORAGE_REF.html_url, }, - ], - }), + ]; + const start = (page - 1) * per_page; + return { data: all.slice(start, start + per_page) }; + }, get: async (args) => { if (typeof initial.repoGet === 'function') { return initial.repoGet(args); @@ -296,6 +298,22 @@ test('listGitHubRepos maps node id and repo metadata', async () => { assert.equal(repos[0].full_name, STORAGE_REF.full_name); }); +test('listGitHubRepos paginates beyond the first 100 repos', async () => { + const storageService = new StorageService(); + const listedRepos = Array.from({ length: 105 }, (_, i) => ({ + node_id: `R_${i}`, + full_name: `sam/repo-${i}`, + private: true, + html_url: `https://github.com/sam/repo-${i}`, + })); + const client = createMockGitHubClient({ listedRepos }); + const repos = await storageService.listGitHubRepos(client); + assert.equal(repos.length, 105); + assert.equal(repos[0].full_name, 'sam/repo-0'); + assert.equal(repos[104].full_name, 'sam/repo-104'); + assert.equal(repos[104].id, 'R_104'); +}); + test('checkGitHubRepoNameAvailability returns available on 404', async () => { const storageService = new StorageService(); const client = createMockGitHubClient({ From 138e131ba6af4be8dcb56269fd156ddbfc9d0904 Mon Sep 17 00:00:00 2001 From: Samuel Olabode Date: Thu, 6 Aug 2026 07:39:29 -0600 Subject: [PATCH 3/6] feat: paginate GitHub App installation lookups in StorageService Walk every installation and installation-repo page when probing write access so repos past the first hundred are not treated as uninstalled. --- backend/services/storageService.js | 68 ++++++++++++++++++---------- backend/tests/storageService.test.js | 24 ++++++---- 2 files changed, 60 insertions(+), 32 deletions(-) diff --git a/backend/services/storageService.js b/backend/services/storageService.js index 73c0ed6..23bee99 100644 --- a/backend/services/storageService.js +++ b/backend/services/storageService.js @@ -8,7 +8,7 @@ const crypto = require('crypto'); const { randomUUID } = require('crypto'); const { normalizeGitHubRepoName } = require('../../shared/githubRepoName'); -const { collectAllGitHubPages } = require('./githubPagination'); +const { collectAllGitHubPages, findInGitHubPages } = require('./githubPagination'); const MANIFEST_PATH = 'vizably.json'; /** Pre-rename store root — still loadable; rewritten to `MANIFEST_PATH` on load. */ @@ -228,16 +228,20 @@ class StorageService { */ async _isRepoOnWritableInstallation(octokit, owner, repo) { const fullName = `${owner}/${repo}`; - let data; + let installations; try { - ({ data } = await octokit.rest.apps.listInstallationsForAuthenticatedUser({ - per_page: 100, - })); + installations = await collectAllGitHubPages(async (page, perPage) => { + const { data } = await octokit.rest.apps.listInstallationsForAuthenticatedUser({ + per_page: perPage, + page, + }); + return data.installations ?? []; + }); } catch (err) { throw this._formatGitHubInstallationProbeError(err); } - for (const installation of data.installations ?? []) { + for (const installation of installations) { if (installation.permissions?.contents !== 'write') { continue; } @@ -245,18 +249,25 @@ class StorageService { return true; } - let reposData; + let matched; try { - ({ data: reposData } = - await octokit.rest.apps.listInstallationReposForAuthenticatedUser({ - installation_id: installation.id, - per_page: 100, - })); + matched = await findInGitHubPages( + async (page, perPage) => { + const { data: reposData } = + await octokit.rest.apps.listInstallationReposForAuthenticatedUser({ + installation_id: installation.id, + per_page: perPage, + page, + }); + return reposData.repositories ?? []; + }, + (entry) => entry.full_name === fullName, + ); } catch (err) { throw this._formatGitHubInstallationProbeError(err); } - if (reposData.repositories?.some((r) => r.full_name === fullName)) { + if (matched) { return true; } } @@ -1050,24 +1061,33 @@ class StorageService { let canWrite = false; try { - const { data } = await octokit.rest.apps.listInstallationsForAuthenticatedUser({ - per_page: 100, + const installations = await collectAllGitHubPages(async (page, perPage) => { + const { data } = await octokit.rest.apps.listInstallationsForAuthenticatedUser({ + per_page: perPage, + page, + }); + return data.installations ?? []; }); - for (const installation of data.installations ?? []) { + for (const installation of installations) { const contents = installation.permissions?.contents; if (!contents || contents === 'none') { continue; } - const { data: reposData } = - await octokit.rest.apps.listInstallationReposForAuthenticatedUser({ - installation_id: installation.id, - per_page: 100, - }); - - const included = reposData.repositories?.some((r) => r.full_name === fullName); - if (!included) { + const matched = await findInGitHubPages( + async (page, perPage) => { + const { data: reposData } = + await octokit.rest.apps.listInstallationReposForAuthenticatedUser({ + installation_id: installation.id, + per_page: perPage, + page, + }); + return reposData.repositories ?? []; + }, + (entry) => entry.full_name === fullName, + ); + if (!matched) { continue; } diff --git a/backend/tests/storageService.test.js b/backend/tests/storageService.test.js index 3e0616e..0dd3d1b 100644 --- a/backend/tests/storageService.test.js +++ b/backend/tests/storageService.test.js @@ -258,28 +258,36 @@ function createMockGitHubClient(initial = {}) { }, apps: installationProbe || initial.installationProbeError ? { - listInstallationsForAuthenticatedUser: async () => { + listInstallationsForAuthenticatedUser: async ({ page = 1, per_page = 100 } = {}) => { if (initial.installationProbeError) { throw initial.installationProbeError; } + const all = (installationProbe ?? []).map((entry) => ({ + id: entry.id, + permissions: { contents: entry.contents }, + repository_selection: entry.repository_selection || 'selected', + })); + const start = (page - 1) * per_page; return { data: { - installations: (installationProbe ?? []).map((entry) => ({ - id: entry.id, - permissions: { contents: entry.contents }, - repository_selection: entry.repository_selection || 'selected', - })), + installations: all.slice(start, start + per_page), }, }; }, - listInstallationReposForAuthenticatedUser: async ({ installation_id }) => { + listInstallationReposForAuthenticatedUser: async ({ + installation_id, + page = 1, + per_page = 100, + }) => { if (initial.installationReposError) { throw initial.installationReposError; } const entry = (installationProbe ?? []).find((item) => item.id === installation_id); + const all = (entry?.repos ?? []).map((full_name) => ({ full_name })); + const start = (page - 1) * per_page; return { data: { - repositories: (entry?.repos ?? []).map((full_name) => ({ full_name })), + repositories: all.slice(start, start + per_page), }, }; }, From e526c84acc0e724662125f7dafb4cf5c07f0f57e Mon Sep 17 00:00:00 2001 From: Samuel Olabode Date: Thu, 6 Aug 2026 07:40:08 -0600 Subject: [PATCH 4/6] feat: paginate installation resolution in AuthService _findInstallationIdForRepo now walks every installation and selected-repo page so storage repos beyond the first hundred still resolve to an App token. --- backend/services/authService.js | 30 ++++++++++++++------ backend/tests/authService.test.js | 46 +++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/backend/services/authService.js b/backend/services/authService.js index a9d2bcb..819a024 100644 --- a/backend/services/authService.js +++ b/backend/services/authService.js @@ -10,6 +10,7 @@ const session = require('express-session'); const passport = require('passport'); const GitHubStrategy = require('passport-github2').Strategy; const { Octokit } = require('@octokit/rest'); +const { collectAllGitHubPages, findInGitHubPages } = require('./githubPagination'); const GOOGLE_NOT_AVAILABLE = 'Google auth is not available until Phase 3'; @@ -125,18 +126,29 @@ class AuthService { } } - const { data } = await userOctokit.rest.apps.listInstallationsForAuthenticatedUser({ - per_page: 100, + const installations = await collectAllGitHubPages(async (page, perPage) => { + const { data } = await userOctokit.rest.apps.listInstallationsForAuthenticatedUser({ + per_page: perPage, + page, + }); + return data.installations ?? []; }); - for (const installation of data.installations ?? []) { - const { data: reposData } = - await userOctokit.rest.apps.listInstallationReposForAuthenticatedUser({ - installation_id: installation.id, - per_page: 100, - }); + for (const installation of installations) { + const matched = await findInGitHubPages( + async (page, perPage) => { + const { data: reposData } = + await userOctokit.rest.apps.listInstallationReposForAuthenticatedUser({ + installation_id: installation.id, + per_page: perPage, + page, + }); + return reposData.repositories ?? []; + }, + (entry) => entry.full_name === fullName, + ); - if (reposData.repositories?.some((entry) => entry.full_name === fullName)) { + if (matched) { return installation.id; } } diff --git a/backend/tests/authService.test.js b/backend/tests/authService.test.js index cd68c7c..4e25b24 100644 --- a/backend/tests/authService.test.js +++ b/backend/tests/authService.test.js @@ -219,3 +219,49 @@ test('getInstallationClientForRepo requires storageRef.id', async () => { ); assert.equal(reposGetCalled, false); }); + +test('_findInstallationIdForRepo paginates user installations and repos', async () => { + const authService = new AuthService({ + sessionSecret: TEST_SESSION_SECRET, + encryptionKey: TEST_ENCRYPTION_KEY, + }); + + const installations = Array.from({ length: 101 }, (_, i) => ({ id: i + 1 })); + const reposForTarget = Array.from({ length: 101 }, (_, i) => ({ + full_name: i === 100 ? 'sam/site-audits' : `sam/other-${i}`, + })); + const calls = { installations: [], repos: [] }; + + const userOctokit = { + rest: { + apps: { + listInstallationsForAuthenticatedUser: async ({ page = 1, per_page = 100 } = {}) => { + calls.installations.push({ page, per_page }); + const start = (page - 1) * per_page; + return { + data: { installations: installations.slice(start, start + per_page) }, + }; + }, + listInstallationReposForAuthenticatedUser: async ({ + installation_id, + page = 1, + per_page = 100, + }) => { + calls.repos.push({ installation_id, page, per_page }); + if (installation_id !== 101) { + return { data: { repositories: [] } }; + } + const start = (page - 1) * per_page; + return { + data: { repositories: reposForTarget.slice(start, start + per_page) }, + }; + }, + }, + }, + }; + + const id = await authService._findInstallationIdForRepo(userOctokit, 'sam/site-audits'); + assert.equal(id, 101); + assert.equal(calls.installations.length, 2); + assert.ok(calls.repos.some((c) => c.installation_id === 101 && c.page === 2)); +}); From eb9cf2c3ca9b0e6c326b16f3822c7632fcfe46e1 Mon Sep 17 00:00:00 2001 From: Samuel Olabode Date: Thu, 6 Aug 2026 07:40:23 -0600 Subject: [PATCH 5/6] test: cover create flow when install data spans multiple pages Ensure newly created repos are recognized as writable even when the matching installation or installation-repo sits past the first page. --- backend/tests/storageService.test.js | 34 ++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/backend/tests/storageService.test.js b/backend/tests/storageService.test.js index 0dd3d1b..ce57bf7 100644 --- a/backend/tests/storageService.test.js +++ b/backend/tests/storageService.test.js @@ -423,6 +423,40 @@ test('createGitHubRepository skips install hop when installation covers all repo assert.equal(result.needsInstall, false); }); +test('createGitHubRepository finds writable install when repo is past first page', async () => { + const storageService = new StorageService(); + const repos = Array.from({ length: 101 }, (_, i) => + i === 100 ? 'sam/vizably-new' : `sam/other-${i}`, + ); + const client = createMockGitHubClient({ + installationProbe: [ + { + id: 1, + contents: 'write', + repos, + }, + ], + }); + const result = await storageService.createGitHubRepository('vizably-new', { + githubUserClient: client, + }); + assert.equal(result.needsInstall, false); +}); + +test('createGitHubRepository finds writable install when installation is past first page', async () => { + const storageService = new StorageService(); + const installationProbe = Array.from({ length: 101 }, (_, i) => ({ + id: i + 1, + contents: 'write', + repos: i === 100 ? ['sam/vizably-new'] : [`sam/other-${i}`], + })); + const client = createMockGitHubClient({ installationProbe }); + const result = await storageService.createGitHubRepository('vizably-new', { + githubUserClient: client, + }); + assert.equal(result.needsInstall, false); +}); + test('createGitHubRepository surfaces rate limits instead of needsInstall', async () => { const storageService = new StorageService(); const probeErr = new Error('API rate limit exceeded'); From d35a44e8b4eefb7c7c053708c6fc242175e0501a Mon Sep 17 00:00:00 2001 From: Samuel Olabode Date: Thu, 6 Aug 2026 07:40:38 -0600 Subject: [PATCH 6/6] test: cover capability probe when installation repos span pages validateStorage should still report App write access when the target repo only appears after the first hundred installation repositories. --- backend/tests/storageService.test.js | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/backend/tests/storageService.test.js b/backend/tests/storageService.test.js index ce57bf7..997897c 100644 --- a/backend/tests/storageService.test.js +++ b/backend/tests/storageService.test.js @@ -615,6 +615,30 @@ test('validateStorage probes write access with user client when IO uses installa assert.equal(result.capabilities.canWrite, true); }); +test('validateStorage finds App write access when repo is past first install page', async () => { + const storageService = new StorageService(); + const repos = Array.from({ length: 101 }, (_, i) => + i === 100 ? STORAGE_REF.full_name : `sam/other-${i}`, + ); + const client = createMockGitHubClient({ + repoMeta: { permissions: { pull: true, push: false, admin: false } }, + installationProbe: [ + { + id: 1, + contents: 'write', + repos, + }, + ], + }); + const result = await storageService.validateStorage('github', STORAGE_REF, { + githubClient: client, + githubUserClient: client, + }); + assert.equal(result.status, 'initializable'); + assert.equal(result.capabilities.canRead, true); + assert.equal(result.capabilities.canWrite, true); +}); + test('validateStorage returns unrelated when root has other files', async () => { const storageService = new StorageService(); const client = createMockGitHubClient({