From 10c5d59e5be7385fb6a4d6d7296c24c772e2225a Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Thu, 8 Oct 2026 23:57:14 +1100 Subject: [PATCH 1/3] fix(registry): complete large repository source refresh --- layers/registry/server/utils/github-client.ts | 50 +++++++++++++- .../registry/server/utils/sync-candidates.ts | 18 +++-- test/unit/github-client-tree.test.ts | 65 +++++++++++++++++++ test/unit/sync-candidates.test.ts | 28 ++++++-- 4 files changed, 148 insertions(+), 13 deletions(-) create mode 100644 test/unit/github-client-tree.test.ts diff --git a/layers/registry/server/utils/github-client.ts b/layers/registry/server/utils/github-client.ts index fbdb4c33..3794caa3 100644 --- a/layers/registry/server/utils/github-client.ts +++ b/layers/registry/server/utils/github-client.ts @@ -531,11 +531,57 @@ export async function getTree( bindings: GithubBindings, options?: GithubReadOptions, ): Promise> { - return ghRequest( + // A page deadline covers the entire fallback, including credential reads. + const init = readInit(options) + const initial = await ghRequest( `${API_BASE}/repos/${owner}/${repo}/git/trees/${ref}?recursive=1`, bindings, - readInit(options), + init, ) + if (!initial.data?.truncated) + return initial + + // GitHub caps recursive trees at 100,000 entries or 7 MB. Resolve each + // truncated subtree from its immutable SHA. Never merge a partial response + // into the inventory: sync uses absent paths to retire Skills. + const rootSha = initial.data.sha + const incomplete: TreeResponse = { sha: rootSha, tree: [], truncated: true } + const pending = [{ sha: rootSha, prefix: '', recursive: false }] + const entries: TreeEntry[] = [] + let rateLimit = initial.rateLimit + let requests = 1 + while (pending.length > 0) { + // Bound quota and isolate memory. Larger trees retain their verdict and + // retry on the general refresh clock instead of yielding a false snapshot. + if (requests >= 64) + return { ...initial, data: incomplete, rateLimit, notModified: false } + const next = pending.pop()! + requests++ + const result = await ghRequest( + `${API_BASE}/repos/${owner}/${repo}/git/trees/${next.sha}${next.recursive ? '?recursive=1' : ''}`, + bindings, + init, + ) + if (!result.data) + return result + if (result.rateLimit && (!rateLimit || result.rateLimit.remaining < rateLimit.remaining)) + rateLimit = result.rateLimit + if (result.data.truncated) { + if (!next.recursive) + return { ...result, data: incomplete, rateLimit, notModified: false } + pending.push({ ...next, recursive: false }) + continue + } + if (entries.length + result.data.tree.length > 250_000) + return { ...result, data: incomplete, rateLimit, notModified: false } + for (const entry of result.data.tree) { + const path = next.prefix ? `${next.prefix}/${entry.path}` : entry.path + entries.push({ path, type: entry.type, sha: entry.sha }) + if (!next.recursive && entry.type === 'tree') + pending.push({ sha: entry.sha, prefix: path, recursive: true }) + } + } + return { ...initial, data: { sha: rootSha, tree: entries, truncated: false }, rateLimit, notModified: false } } export interface GetCommitsOpts { diff --git a/layers/registry/server/utils/sync-candidates.ts b/layers/registry/server/utils/sync-candidates.ts index e79273df..a6d90c2e 100644 --- a/layers/registry/server/utils/sync-candidates.ts +++ b/layers/registry/server/utils/sync-candidates.ts @@ -17,8 +17,8 @@ interface HistoricalDiscoveryStageOptions extends RepoSyncPriorityOptions { /** * Subscribed repos get the short freshness window supplied by the caller. * Grouping collapses multiple subscribers watching the same repo. Repos with - * a recorded too-large tree verdict stay out: GitHub will truncate their tree - * response on every attempt, so re-picking them only burns rate limit. + * a recorded too-large tree verdict use the slower general refresh pool. + * Their bounded subtree fallback costs more reads than a normal tree fetch. */ export const SUBSCRIBED_SYNC_CANDIDATES_SQL = ` SELECT r.owner, r.repo, r.repo_meta_synced_at AS ls, @@ -47,8 +47,9 @@ export const SUBSCRIBED_SYNC_CANDIDATES_SQL = ` * forever. EXISTS keeps empty/retired repo rows out without grouping skills. * A repo whose every row is `path_missing` has no Skills left: its tree was * readable but held none, so it is not broken and not refreshed either. - * Repos with a recorded too-large tree verdict stay out for the same reason - * as the subscribed sweep above. + * Truncated repositories retry on this slower clock. The tree reader now + * resolves truncated responses through bounded immutable subtree reads. + * Public references without a source commit lead this pool until repaired. */ export const GENERAL_SYNC_CANDIDATES_SQL = ` SELECT r.owner, r.repo, r.repo_meta_synced_at AS ls, @@ -57,14 +58,19 @@ export const GENERAL_SYNC_CANDIDATES_SQL = ` LEFT JOIN discovery_candidates dc ON dc.owner = r.owner AND dc.repo = r.repo WHERE r.broken_since IS NULL - AND r.tree_truncated_at IS NULL AND (r.repo_meta_synced_at IS NULL OR r.repo_meta_synced_at < ?1) AND EXISTS ( SELECT 1 FROM skills s WHERE s.owner = r.owner AND s.repo = r.repo AND COALESCE(s.sync_status, '') != 'path_missing' ) - ORDER BY r.repo_meta_synced_at IS NULL DESC, r.repo_meta_synced_at ASC + ORDER BY EXISTS ( + SELECT 1 FROM skills s + WHERE s.owner = r.owner AND s.repo = r.repo + AND s.seo_indexable = 1 AND s.references_count > 0 + AND s.rendered_commit_sha IS NULL + AND COALESCE(s.sync_status, '') != 'path_missing' + ) DESC, r.repo_meta_synced_at IS NULL DESC, r.repo_meta_synced_at ASC LIMIT ?2` /** diff --git a/test/unit/github-client-tree.test.ts b/test/unit/github-client-tree.test.ts new file mode 100644 index 00000000..4b58442c --- /dev/null +++ b/test/unit/github-client-tree.test.ts @@ -0,0 +1,65 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { getTree } from '../../layers/registry/server/utils/github-client' + +afterEach(() => vi.unstubAllGlobals()) + +describe('getTree', () => { + it('reconstructs truncated trees from immutable subtree SHAs', async () => { + const answers = new Map([ + ['root?recursive=1', { sha: 'root', truncated: true, tree: [{ path: 'partial', type: 'blob', sha: 'discard' }] }], + ['root', { sha: 'root', tree: [{ path: 'skills', type: 'tree', sha: 'skills-tree' }] }], + ['skills-tree?recursive=1', { sha: 'skills-tree', truncated: true, tree: [] }], + ['skills-tree', { sha: 'skills-tree', tree: [{ path: 'one', type: 'tree', sha: 'one-tree' }] }], + ['one-tree?recursive=1', { sha: 'one-tree', tree: [{ path: 'SKILL.md', type: 'blob', sha: 'skill' }, { path: 'references/a.md', type: 'blob', sha: 'reference' }] }], + ]) + const reads: string[] = [] + vi.stubGlobal('fetch', vi.fn(async (url: string) => { + const key = url.split('/git/trees/')[1]! + reads.push(key) + const answer = answers.get(key) + if (!answer) + throw new Error(`Unexpected tree read: ${key}`) + return Response.json(answer) + })) + + const result = await getTree('acme', 'large', 'root', {}) + + expect(result.data).toEqual({ sha: 'root', truncated: false, tree: [ + { path: 'skills', type: 'tree', sha: 'skills-tree' }, + { path: 'skills/one', type: 'tree', sha: 'one-tree' }, + { path: 'skills/one/SKILL.md', type: 'blob', sha: 'skill' }, + { path: 'skills/one/references/a.md', type: 'blob', sha: 'reference' }, + ] }) + expect(reads).toEqual([...answers.keys()]) + }) + + it('propagates a subtree read failure without presenting a partial tree', async () => { + let reads = 0 + vi.stubGlobal('fetch', vi.fn(async () => { + reads++ + return reads === 1 + ? Response.json({ sha: 'root', truncated: true, tree: [] }) + : new Response('', { status: 429 }) + })) + + const result = await getTree('acme', 'large', 'root', {}) + + expect(result.status).toBe(429) + expect(result.data).toBeNull() + }) + + it('stops bounded traversal without accepting an incomplete inventory', async () => { + const fetchMock = vi.fn(async (url: string) => Response.json({ + sha: url.split('/git/trees/')[1]!.split('?')[0], + truncated: url.includes('?'), + tree: url.includes('?') ? [] : [{ path: 'nested', type: 'tree', sha: 'next' }], + })) + vi.stubGlobal('fetch', fetchMock) + + const result = await getTree('acme', 'large', 'root', {}) + + expect(result.data?.truncated).toBe(true) + expect(fetchMock.mock.calls.length).toBeGreaterThan(1) + expect(fetchMock.mock.calls.length).toBeLessThanOrEqual(64) + }) +}) diff --git a/test/unit/sync-candidates.test.ts b/test/unit/sync-candidates.test.ts index 96c6c474..d96b6cdc 100644 --- a/test/unit/sync-candidates.test.ts +++ b/test/unit/sync-candidates.test.ts @@ -30,6 +30,9 @@ describe('github sync candidate selection', () => { repo TEXT NOT NULL, name TEXT NOT NULL, sync_status TEXT, + seo_indexable INTEGER DEFAULT 0, + references_count INTEGER DEFAULT 0, + rendered_commit_sha TEXT, PRIMARY KEY (owner, repo, name) ); CREATE TABLE skill_subscriptions ( @@ -120,7 +123,7 @@ describe('github sync candidate selection', () => { .prepare(GENERAL_SYNC_CANDIDATES_SQL) .all({ 1: 1_000_000 - 36 * 3600, 2: 250 }) as Array<{ owner: string, repo: string }> - expect(rows.map(row => row.repo)).toEqual(['never-checked', 'general-due']) + expect(rows.map(row => row.repo)).toEqual(['never-checked', 'general-due', 'too-large']) expect(rows.map(row => row.repo)).not.toContain('general-recent') expect(rows.map(row => row.repo)).not.toContain('broken') expect(rows.map(row => row.repo)).not.toContain('empty-retired') @@ -143,10 +146,10 @@ describe('github sync candidate selection', () => { .all({ 1: 1_000_000 - 36 * 3600, 2: 250 }) as Array<{ repo: string }> expect(subscribed.map(row => row.repo)).not.toContain('all-retired') - expect(general.map(row => row.repo)).toEqual(['never-checked', 'general-due']) + expect(general.map(row => row.repo)).toEqual(['never-checked', 'general-due', 'too-large']) }) - it('excludes repos with a recorded too-large verdict from both sweeps', () => { + it('retries truncated repositories on the general refresh clock', () => { const subscribed = sqlite .prepare(SUBSCRIBED_SYNC_CANDIDATES_SQL) .all({ 1: 1_000_000 - 3600, 2: 250 }) as Array<{ repo: string }> @@ -154,9 +157,24 @@ describe('github sync candidate selection', () => { .prepare(GENERAL_SYNC_CANDIDATES_SQL) .all({ 1: 1_000_000 - 36 * 3600, 2: 250 }) as Array<{ repo: string }> - // acme/too-large is due on both clocks, so only the verdict keeps it out. expect(subscribed.map(row => row.repo)).not.toContain('too-large') - expect(general.map(row => row.repo)).not.toContain('too-large') + expect(general.map(row => row.repo)).toContain('too-large') + sqlite.exec('UPDATE repos SET repo_meta_synced_at = 999000 WHERE repo = \'too-large\'') + const rechecked = sqlite.prepare(GENERAL_SYNC_CANDIDATES_SQL) + .all({ 1: 1_000_000 - 36 * 3600, 2: 250 }) as Array<{ repo: string }> + expect(rechecked.map(row => row.repo)).not.toContain('too-large') + }) + + it('prioritizes public references without a source commit over older ordinary work', () => { + sqlite.exec(` + UPDATE skills SET seo_indexable = 1, references_count = 2 WHERE repo = 'too-large'; + UPDATE skills SET seo_indexable = 1, references_count = 2, rendered_commit_sha = 'known' WHERE repo = 'general-due'; + INSERT INTO skills (owner, repo, name, seo_indexable, references_count, sync_status) + VALUES ('acme', 'never-checked', 'retired', 1, 2, 'path_missing'); + `) + const rows = sqlite.prepare(GENERAL_SYNC_CANDIDATES_SQL) + .all({ 1: 1_000_000 - 36 * 3600, 2: 1 }) as Array<{ repo: string }> + expect(rows.map(row => row.repo)).toEqual(['too-large']) }) it('walks the due-work partial index', () => { From a056d6bf736f7d73647665e96f649d06c089bc24 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Fri, 9 Oct 2026 00:17:55 +1100 Subject: [PATCH 2/3] fix(registry): preserve tree sizes and bounded page reads --- .../server/api/repos/[owner]/[repo].get.ts | 6 +++++- layers/registry/server/utils/github-client.ts | 7 ++++--- test/unit/github-client-tree.test.ts | 17 ++++++++++++++--- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/layers/registry/server/api/repos/[owner]/[repo].get.ts b/layers/registry/server/api/repos/[owner]/[repo].get.ts index 8d391141..62f97edc 100644 --- a/layers/registry/server/api/repos/[owner]/[repo].get.ts +++ b/layers/registry/server/api/repos/[owner]/[repo].get.ts @@ -143,7 +143,11 @@ export default defineCachedEventHandler(async (event) => { if (cachedScan) return profileFrom(meta, cachedScan) satisfies RepoSourceProfile - const treeRes = await getTree(meta.owner.login, meta.name, treeSha ?? meta.default_branch, bindings, { timeoutMs: GITHUB_PAGE_READ_TIMEOUT_MS }) + const treeRes = await getTree(meta.owner.login, meta.name, treeSha ?? meta.default_branch, bindings, { + timeoutMs: GITHUB_PAGE_READ_TIMEOUT_MS, + // Repository pages can show a partial listing. Durable sync owns expansion. + expandTruncated: false, + }) .catch((error: unknown) => githubReadFailed('repo-source-profile-tree', error)) const scan = treeRes.data ? scanTree(treeRes.data) : null if (scan && treeSha) diff --git a/layers/registry/server/utils/github-client.ts b/layers/registry/server/utils/github-client.ts index 3794caa3..cf2bb7b5 100644 --- a/layers/registry/server/utils/github-client.ts +++ b/layers/registry/server/utils/github-client.ts @@ -175,6 +175,7 @@ export interface TreeEntry { path: string type: 'blob' | 'tree' | 'commit' sha: string + size?: number } export interface TreeResponse { @@ -529,7 +530,7 @@ export async function getTree( repo: string, ref: string, bindings: GithubBindings, - options?: GithubReadOptions, + options?: GithubReadOptions & { expandTruncated?: boolean }, ): Promise> { // A page deadline covers the entire fallback, including credential reads. const init = readInit(options) @@ -538,7 +539,7 @@ export async function getTree( bindings, init, ) - if (!initial.data?.truncated) + if (!initial.data?.truncated || options?.expandTruncated === false) return initial // GitHub caps recursive trees at 100,000 entries or 7 MB. Resolve each @@ -576,7 +577,7 @@ export async function getTree( return { ...result, data: incomplete, rateLimit, notModified: false } for (const entry of result.data.tree) { const path = next.prefix ? `${next.prefix}/${entry.path}` : entry.path - entries.push({ path, type: entry.type, sha: entry.sha }) + entries.push({ path, type: entry.type, sha: entry.sha, size: entry.size }) if (!next.recursive && entry.type === 'tree') pending.push({ sha: entry.sha, prefix: path, recursive: true }) } diff --git a/test/unit/github-client-tree.test.ts b/test/unit/github-client-tree.test.ts index 4b58442c..51f4c89a 100644 --- a/test/unit/github-client-tree.test.ts +++ b/test/unit/github-client-tree.test.ts @@ -10,7 +10,7 @@ describe('getTree', () => { ['root', { sha: 'root', tree: [{ path: 'skills', type: 'tree', sha: 'skills-tree' }] }], ['skills-tree?recursive=1', { sha: 'skills-tree', truncated: true, tree: [] }], ['skills-tree', { sha: 'skills-tree', tree: [{ path: 'one', type: 'tree', sha: 'one-tree' }] }], - ['one-tree?recursive=1', { sha: 'one-tree', tree: [{ path: 'SKILL.md', type: 'blob', sha: 'skill' }, { path: 'references/a.md', type: 'blob', sha: 'reference' }] }], + ['one-tree?recursive=1', { sha: 'one-tree', tree: [{ path: 'SKILL.md', type: 'blob', sha: 'skill', size: 120 }, { path: 'references/a.md', type: 'blob', sha: 'reference', size: 42 }] }], ]) const reads: string[] = [] vi.stubGlobal('fetch', vi.fn(async (url: string) => { @@ -27,12 +27,23 @@ describe('getTree', () => { expect(result.data).toEqual({ sha: 'root', truncated: false, tree: [ { path: 'skills', type: 'tree', sha: 'skills-tree' }, { path: 'skills/one', type: 'tree', sha: 'one-tree' }, - { path: 'skills/one/SKILL.md', type: 'blob', sha: 'skill' }, - { path: 'skills/one/references/a.md', type: 'blob', sha: 'reference' }, + { path: 'skills/one/SKILL.md', type: 'blob', sha: 'skill', size: 120 }, + { path: 'skills/one/references/a.md', type: 'blob', sha: 'reference', size: 42 }, ] }) expect(reads).toEqual([...answers.keys()]) }) + it('returns a partial tree without expansion for a page read', async () => { + const tree = { sha: 'root', truncated: true, tree: [{ path: 'SKILL.md', type: 'blob', sha: 'skill', size: 120 }] } + const fetchMock = vi.fn(async () => Response.json(tree)) + vi.stubGlobal('fetch', fetchMock) + + const result = await getTree('acme', 'large', 'root', {}, { expandTruncated: false }) + + expect(result.data).toEqual(tree) + expect(fetchMock).toHaveBeenCalledTimes(1) + }) + it('propagates a subtree read failure without presenting a partial tree', async () => { let reads = 0 vi.stubGlobal('fetch', vi.fn(async () => { From 11c04fbaf279fced63c7efbe75873c923c244df7 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Fri, 9 Oct 2026 01:58:24 +1100 Subject: [PATCH 3/3] fix(registry): limit tree expansion to durable refresh --- layers/registry/server/utils/github-client.ts | 2 +- layers/registry/server/utils/sync-repo.ts | 4 ++-- test/unit/github-client-tree.test.ts | 10 +++++----- test/unit/sync-repo-freshness.test.ts | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/layers/registry/server/utils/github-client.ts b/layers/registry/server/utils/github-client.ts index cf2bb7b5..ca078b41 100644 --- a/layers/registry/server/utils/github-client.ts +++ b/layers/registry/server/utils/github-client.ts @@ -539,7 +539,7 @@ export async function getTree( bindings, init, ) - if (!initial.data?.truncated || options?.expandTruncated === false) + if (!initial.data?.truncated || options?.expandTruncated !== true) return initial // GitHub caps recursive trees at 100,000 entries or 7 MB. Resolve each diff --git a/layers/registry/server/utils/sync-repo.ts b/layers/registry/server/utils/sync-repo.ts index f50ed82a..c0b197f7 100644 --- a/layers/registry/server/utils/sync-repo.ts +++ b/layers/registry/server/utils/sync-repo.ts @@ -373,7 +373,7 @@ export async function refreshRepoAssets( SET source_owner = ?, source_repo = ? WHERE owner = ? AND repo = ? `).bind(sourceOwner, sourceRepo, owner, repo).run() - const treeRes = await getTree(sourceOwner, sourceRepo, repoRes.data.headTreeSha ?? branch, bindings) + const treeRes = await getTree(sourceOwner, sourceRepo, repoRes.data.headTreeSha ?? branch, bindings, { expandTruncated: true }) logRateLimit(`asset-backfill tree ${owner}/${repo}`, treeRes.rateLimit) const rateLimitRemaining = Math.min( rate.rateLimitRemaining ?? Number.POSITIVE_INFINITY, @@ -895,7 +895,7 @@ export async function syncRepo( // The head tree SHA names the exact tree the summary saw. A branch name // can move between the two reads, and a SHA URL is immutable, so its ETag // cache entry answers 304 for as long as the tree stands. - const treeRes = await getTree(sourceOwner, sourceRepo, headTreeSha ?? branch, bindings) + const treeRes = await getTree(sourceOwner, sourceRepo, headTreeSha ?? branch, bindings, { expandTruncated: true }) logRateLimit(`tree ${owner}/${repo}`, treeRes.rateLimit) trackRateLimit(treeRes.rateLimit) diff --git a/test/unit/github-client-tree.test.ts b/test/unit/github-client-tree.test.ts index 51f4c89a..1cd04e19 100644 --- a/test/unit/github-client-tree.test.ts +++ b/test/unit/github-client-tree.test.ts @@ -22,7 +22,7 @@ describe('getTree', () => { return Response.json(answer) })) - const result = await getTree('acme', 'large', 'root', {}) + const result = await getTree('acme', 'large', 'root', {}, { expandTruncated: true }) expect(result.data).toEqual({ sha: 'root', truncated: false, tree: [ { path: 'skills', type: 'tree', sha: 'skills-tree' }, @@ -33,12 +33,12 @@ describe('getTree', () => { expect(reads).toEqual([...answers.keys()]) }) - it('returns a partial tree without expansion for a page read', async () => { + it('returns a partial tree with one request unless durable refresh opts in', async () => { const tree = { sha: 'root', truncated: true, tree: [{ path: 'SKILL.md', type: 'blob', sha: 'skill', size: 120 }] } const fetchMock = vi.fn(async () => Response.json(tree)) vi.stubGlobal('fetch', fetchMock) - const result = await getTree('acme', 'large', 'root', {}, { expandTruncated: false }) + const result = await getTree('acme', 'large', 'root', {}) expect(result.data).toEqual(tree) expect(fetchMock).toHaveBeenCalledTimes(1) @@ -53,7 +53,7 @@ describe('getTree', () => { : new Response('', { status: 429 }) })) - const result = await getTree('acme', 'large', 'root', {}) + const result = await getTree('acme', 'large', 'root', {}, { expandTruncated: true }) expect(result.status).toBe(429) expect(result.data).toBeNull() @@ -67,7 +67,7 @@ describe('getTree', () => { })) vi.stubGlobal('fetch', fetchMock) - const result = await getTree('acme', 'large', 'root', {}) + const result = await getTree('acme', 'large', 'root', {}, { expandTruncated: true }) expect(result.data?.truncated).toBe(true) expect(fetchMock.mock.calls.length).toBeGreaterThan(1) diff --git a/test/unit/sync-repo-freshness.test.ts b/test/unit/sync-repo-freshness.test.ts index 1a85e6e9..6d7995d4 100644 --- a/test/unit/sync-repo-freshness.test.ts +++ b/test/unit/sync-repo-freshness.test.ts @@ -217,7 +217,7 @@ describe('syncRepo freshness cursor', () => { FROM repos WHERE owner = 'acme' AND repo = 'skills' `).get() - expect(github.getTree).toHaveBeenCalledWith('acme', 'skills', 'moved-tree', {}) + expect(github.getTree.mock.calls[0]?.[2]).toBe('moved-tree') expect(result.status).toBe('failed') expect(result.reason).toBe('tree_truncated') expect(row).toEqual({