diff --git a/layers/artifact-delivery/server/utils/github-source.ts b/layers/artifact-delivery/server/utils/github-source.ts index 70f4d79a..c01d5344 100644 --- a/layers/artifact-delivery/server/utils/github-source.ts +++ b/layers/artifact-delivery/server/utils/github-source.ts @@ -22,6 +22,16 @@ const GITHUB_API_VERSION = '2026-03-10' const MAX_GITHUB_JSON_BYTES = 8 * 1024 * 1024 const MAX_TREE_ENTRIES = 2000 const MAX_TREE_REQUESTS = 128 +/** + * The tree reads one search by name may spend. Every read comes out of the + * site's GitHub quota, and anyone can send a name. Measured 2026-10-07 on the + * split walk: n8n-io/n8n needs 27, vercel/next.js 37, openshift/hypershift 45. + * posthog/posthog needs more than 128 and stops after about 40. + */ +const MAX_NAME_SEARCH_TREE_READS = 64 +// One tree read can hold 8 MiB of JSON and the tree parsed from it. Two at a +// time keep a split walk well inside a Worker's 128 MiB. +const TREE_WALK_CONCURRENCY = 2 const MAX_SKILL_PATH_SEGMENTS = 64 // One invocation loads every blob once. With one repository read per load and // resolve, a handful of tree reads, the D1 state transitions and the R2 write, @@ -97,6 +107,12 @@ const blobResponseSchema = z.object({ type TreeEntry = z.infer +/** Where the GitHub API serves a Repository now. */ +interface RepositoryName { + owner: string + repository: string +} + interface ListedTree { _tag: 'listed' entries: TreeEntry[] @@ -178,17 +194,8 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi outcome._tag === 'rate-limited' ? rateLimitRejection(outcome.resetAt) : sourceReadRejection(outcome._tag, expectedVisibility) - const requestJson = async (step: GithubReadStep, path: string, schema: z.ZodType): Promise< - { _tag: 'ok', value: T } - | { _tag: 'not-found' } - | { _tag: 'access-denied' } - | { _tag: 'rate-limited', resetAt: number | null } - > => { - type Outcome - = | { _tag: 'ok', value: T } - | { _tag: 'not-found' } - | { _tag: 'access-denied' } - | { _tag: 'rate-limited', resetAt: number | null } + const requestJson = async (step: GithubReadStep, path: string, schema: z.ZodType): Promise> => { + type Outcome = ReadOutcome const headers = new Headers({ 'Accept': 'application/vnd.github+json', 'User-Agent': 'skilld.dev', @@ -198,11 +205,22 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi headers.set('Authorization', `Bearer ${options.token}`) // Every try requests this same URL, so a retry can never read another ref. const url = `${GITHUB_API}${path}` + const fetchOnce = (target: string) => fetchNoRedirect(options.fetch, target, { + headers, + signal: AbortSignal.timeout(GITHUB_READ_TRY_TIMEOUT_MS), + }) + // A renamed or transferred Repository answers its old name with a 301 to + // `/repositories/`. The read follows that one hop, so a run of the old + // name keeps working. Every other redirect stays fatal. + const fetchFollowingMove = async () => { + const fetched = await fetchOnce(url) + if (fetched._tag !== 'unexpected-redirect') + return fetched + const moved = movedRepositoryUrl(fetched.location) + return moved ? await fetchOnce(moved) : fetched + } const tryOnce = async (): Promise> => { - const sent = await fetchNoRedirect(options.fetch, url, { - headers, - signal: AbortSignal.timeout(GITHUB_READ_TRY_TIMEOUT_MS), - }).then( + const sent = await fetchFollowingMove().then( fetched => ({ _tag: 'sent' as const, fetched }), (error: unknown) => ({ _tag: 'threw' as const, error }), ) @@ -233,10 +251,13 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi status: response.status, } } - const body = await readBoundedJson(response, MAX_GITHUB_JSON_BYTES).then( - value => ({ _tag: 'json' as const, value }), - (error: unknown) => ({ _tag: 'unreadable' as const, error }), - ) + const body = await readBoundedJson(response, MAX_GITHUB_JSON_BYTES) + .catch((error: unknown) => ({ _tag: 'unreadable' as const, error })) + // A body over the limit is a fact about the Git object, so it settles + // as a value each caller can answer. It used to throw, which spent the + // whole queue retry ladder and then failed as SERVICE_UNAVAILABLE. + if (body._tag === 'too-large') + return { _tag: 'settled', value: { _tag: 'too-large' } } if (body._tag === 'unreadable') { const transient = !isBodyLimitError(body.error) return { @@ -322,14 +343,16 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi rootTreeSha: string, ): Promise => { const recursive = await getTree(owner, repository, rootTreeSha, true) - if (recursive._tag !== 'ok') + if (recursive._tag !== 'ok' && recursive._tag !== 'too-large') return readRejection(recursive) - if (!recursive.value.truncated) { + if (recursive._tag === 'ok' && !recursive.value.truncated) { if (recursive.value.tree.length > MAX_TREE_ENTRIES) return sourceLimitRejection(`The Skill folder has more than ${MAX_TREE_ENTRIES} entries.`) return { _tag: 'listed', entries: recursive.value.tree, truncated: false } } + // GitHub truncated the listing, or it passed the read limit. Both mean + // one response could not hold it, so walk one tree per directory. const entries: TreeEntry[] = [] const queue: Array<{ sha: string, prefix: string }> = [{ sha: rootTreeSha, prefix: '' }] let requests = 0 @@ -338,10 +361,10 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi return sourceLimitRejection(`The Skill folder needs more than ${MAX_TREE_REQUESTS} tree reads.`) const current = queue.shift()! const response = await getTree(owner, repository, current.sha, false) + if (response._tag === 'too-large' || (response._tag === 'ok' && response.value.truncated)) + return sourceLimitRejection('GitHub returned an incomplete Skill tree.') if (response._tag !== 'ok') return readRejection(response) - if (response.value.truncated) - return sourceLimitRejection('GitHub returned an incomplete Skill tree.') for (const entry of response.value.tree) { const path = current.prefix ? `${current.prefix}/${entry.path}` : entry.path const nested = { ...entry, path } @@ -391,6 +414,9 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi * * Copies of one Skill in several folders resolve to the canonical copy. * They used to fail as "More than one Skill matched". + * + * The registry answers a name it admitted before this search runs, so the + * search serves names it does not hold, such as a private Skill. */ const resolveNamedSkill = async ( owner: string, @@ -398,22 +424,20 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi treeSha: string, name: string, ): Promise => { - const tree = await getTree(owner, repository, treeSha, true) - if (tree._tag !== 'ok') - return readRejection(tree) - if (tree.value.truncated) { + const folders = await listSkillFolders(owner, repository, treeSha) + if (folders._tag === 'unlistable') { return reject( 'INVALID_SOURCE', 'The Repository is too large to find a Skill by name. Name the Skill by its path.', [name], ) } + if (folders._tag === 'rejected') + return folders // The registry lists a Skill by its folder name made into a slug, so // `emailAndPassword` runs as `emailandpassword`. An exact folder name // still matches for a caller that sends one. - const matches = tree.value.tree - .filter(entry => entry.type === 'blob' && isRegistrySkillPath(entry.path)) - .map(entry => entry.path === 'SKILL.md' ? '.' : entry.path.slice(0, -'/SKILL.md'.length)) + const matches = folders.folders .filter((path) => { const folderName = path === '.' ? repository : path.split('/').at(-1)! return folderName === name || slugifySkillName(folderName) === name @@ -422,6 +446,72 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi ?? reject('SOURCE_NOT_FOUND', 'No Skill matched the requested name.', [name]) } + /** + * Every registry Skill folder in a tree, in as few reads as GitHub allows. + * + * One recursive read lists most Repositories. GitHub truncates a recursive + * listing past 100,000 entries or 7 MB, and a large one can pass the 8 MiB + * read limit: posthog/posthog, n8n-io/n8n, vercel/next.js. Each failed the + * search outright, whatever the Skill size. A tree that one response cannot + * hold now splits: one read lists its own level, and each folder in it gets + * its own recursive read, which can split again. + * + * `MAX_NAME_SEARCH_TREE_READS` bounds the walk. Each folder still to visit + * costs at least one read, so the walk stops as soon as the folders it + * knows about cannot fit, before it spends reads it cannot finish. + */ + const listSkillFolders = async ( + owner: string, + repository: string, + rootTreeSha: string, + ): Promise<{ _tag: 'listed', folders: string[] } | { _tag: 'unlistable' } | SourceRejection> => { + type Visit + = | { _tag: 'listed', folders: string[], children: Array<{ sha: string, prefix: string }> } + | { _tag: 'unlistable' } + | SourceRejection + let requests = 0 + const read = (sha: string, recursive: boolean) => { + requests++ + return getTree(owner, repository, sha, recursive) + } + const visit = async ({ sha, prefix }: { sha: string, prefix: string }): Promise => { + const join = (path: string) => prefix ? `${prefix}/${path}` : path + const whole = await read(sha, true) + if (whole._tag === 'ok' && !whole.value.truncated) + return { _tag: 'listed', folders: skillFolders(whole.value.tree.map(entry => ({ ...entry, path: join(entry.path) }))), children: [] } + if (whole._tag !== 'ok' && whole._tag !== 'too-large') + return readRejection(whole) + const level = await read(sha, false) + if (level._tag === 'too-large' || (level._tag === 'ok' && level.value.truncated)) + return { _tag: 'unlistable' } + if (level._tag !== 'ok') + return readRejection(level) + const entries = level.value.tree.map(entry => ({ ...entry, path: join(entry.path) })) + return { + _tag: 'listed', + folders: skillFolders(entries), + children: entries.filter(entry => entry.type === 'tree').map(entry => ({ sha: entry.sha, prefix: entry.path })), + } + } + + const folders: string[] = [] + const pending = [{ sha: rootTreeSha, prefix: '' }] + while (pending.length > 0) { + // A split folder costs a second read, so the wave adds its own size once more. + if (requests + pending.length + Math.min(pending.length, TREE_WALK_CONCURRENCY) > MAX_NAME_SEARCH_TREE_READS) + return { _tag: 'unlistable' } + const wave = pending.splice(0, TREE_WALK_CONCURRENCY) + const visited = await Promise.all(wave.map(visit)) + for (const result of visited) { + if (result._tag !== 'listed') + return result + folders.push(...result.folders) + pending.push(...result.children) + } + } + return { _tag: 'listed', folders } + } + return { async resolve(request) { const normalizedPath = request.selector.type === 'path' @@ -444,6 +534,8 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi ? reject('SOURCE_NOT_FOUND', 'The Repository was not found.', []) : reject('SOURCE_ACCESS_DENIED', 'GitHub denied access to the Repository.', []) } + if (repository._tag === 'too-large') + return readRejection(repository) if (repository.value.private !== (expectedVisibility === 'private')) { return expectedVisibility === 'private' ? reject('SOURCE_NOT_FOUND', 'The Repository was not found.', []) @@ -471,6 +563,8 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi return reject('SOURCE_NOT_FOUND', 'The requested Git reference was not found.', [request.ref?.value ?? repository.value.default_branch]) if (commit._tag === 'access-denied') return reject('SOURCE_ACCESS_DENIED', 'GitHub denied access to the requested Git reference.', []) + if (commit._tag === 'too-large') + return readRejection(commit) if (commit.value.sha !== requestedCommit) { return reject('INVALID_SOURCE', 'GitHub returned another commit identity.', [ requestedCommit, @@ -489,13 +583,18 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi if (typeof skillPath !== 'string') return skillPath + // A moved Repository keeps the name the request used. GitHub confirmed + // that the name leads to this Repository ID, and the released CLI + // checks the attested name against its own request. + const moved = !sameGithubName(request.owner, repository.value.owner.login) + || !sameGithubName(request.repository, repository.value.name) return { _tag: 'resolved', source: { provider: 'github', repositoryId: repository.value.id, - owner: repository.value.owner.login, - repository: repository.value.name, + owner: moved ? request.owner : repository.value.owner.login, + repository: moved ? request.repository : repository.value.name, visibility: expectedVisibility, commitSha: commit.value.sha, treeSha: commit.value.commit.tree.sha, @@ -519,19 +618,21 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi ? reject('SOURCE_NOT_FOUND', 'The Repository was not found.', []) : reject('SOURCE_ACCESS_DENIED', 'This Artifact source is no longer public.', []) } + if (repository._tag === 'too-large') + return readRejection(repository) if (repository.value.private !== (expectedVisibility === 'private')) return reject('SOURCE_ACCESS_DENIED', 'The Repository visibility changed.', []) - if ( - repository.value.id !== source.repositoryId - || repository.value.owner.login !== source.owner - || repository.value.name !== source.repository - ) { + // The Repository ID is the identity. A name can move while the build + // runs, or after a reused build recorded it, and the commit and tree + // digests still pin every byte. + if (repository.value.id !== source.repositoryId) return reject('INVALID_SOURCE', 'The Repository identity changed during Artifact creation.', []) - } - const skillTree = await findTreeAtPath(source.owner, source.repository, source.treeSha, source.skillPath) + // Every later read uses the current name, so none of them redirects. + const at: RepositoryName = { owner: repository.value.owner.login, repository: repository.value.name } + const skillTree = await findTreeAtPath(at.owner, at.repository, source.treeSha, source.skillPath) if (typeof skillTree !== 'string') return skillTree - const listed = await listTreeBounded(source.owner, source.repository, skillTree) + const listed = await listTreeBounded(at.owner, at.repository, skillTree) if (listed._tag !== 'listed') return listed const selected = selectArtifactEntries(listed.entries, source.skillPath) @@ -549,7 +650,7 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi totalBlobBytes: selected.entries.reduce((total, entry) => total + entry.size, 0), }) if (choice._tag === 'tarball') { - const extracted = await loadFromTarball(source, selected.entries) + const extracted = await loadFromTarball(source, at, selected.entries) if (extracted._tag === 'extracted') return { _tag: 'loaded', value: { source, files: extracted.files, omitted } } // The tarball is an optimisation, never an authority. Anything it got @@ -565,13 +666,14 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi findings: extracted.findings.slice(0, 20), }) } - const fromBlobs = await loadFromBlobs(source, selected.entries) + const fromBlobs = await loadFromBlobs(source, at, selected.entries) return fromBlobs._tag === 'loaded' ? { _tag: 'loaded', value: { source, files: fromBlobs.files, omitted } } : fromBlobs }, } async function loadFromTarball( source: ResolvedSource, + at: RepositoryName, entries: Array, ): Promise { const headers = new Headers({ @@ -585,7 +687,7 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi // redirects. The bytes it returns are verified against the tree digests, // which is what makes an unauthenticated byte host acceptable. const response = await options.fetch( - `${GITHUB_API}/repos/${encodeURIComponent(source.owner)}/${encodeURIComponent(source.repository)}/tarball/${source.commitSha}`, + `${GITHUB_API}/repos/${encodeURIComponent(at.owner)}/${encodeURIComponent(at.repository)}/tarball/${source.commitSha}`, { headers, redirect: 'follow', signal: AbortSignal.timeout(TARBALL_REQUEST_TIMEOUT_MS) }, ).catch((thrown: unknown) => { // A refused connection or the request timeout lands here. The build has @@ -613,6 +715,7 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi async function loadFromBlobs( source: ResolvedSource, + at: RepositoryName, entries: Array, ): Promise<{ _tag: 'loaded', files: ArtifactSourceFile[] } | SourceRejection> { const files: ArtifactSourceFile[] = [] @@ -621,7 +724,7 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi const loaded = await Promise.all(batch.map(async (entry): Promise => { const response = await requestJson( 'blob', - `/repos/${encodeURIComponent(source.owner)}/${encodeURIComponent(source.repository)}/git/blobs/${entry.sha}`, + `/repos/${encodeURIComponent(at.owner)}/${encodeURIComponent(at.repository)}/git/blobs/${entry.sha}`, blobResponseSchema, ) if (response._tag !== 'ok') @@ -692,6 +795,18 @@ function artifactFileMode(treeMode: string): 420 | 493 { return treeMode === '100755' ? 493 : 420 } +/** The folder of every registry SKILL.md among tree entries, with `.` for the root. */ +function skillFolders(entries: TreeEntry[]): string[] { + return entries + .filter(entry => entry.type === 'blob' && isRegistrySkillPath(entry.path)) + .map(entry => entry.path === 'SKILL.md' ? '.' : entry.path.slice(0, -'/SKILL.md'.length)) +} + +/** GitHub owner and Repository names match without case. */ +function sameGithubName(left: string, right: string): boolean { + return left.toLowerCase() === right.toLowerCase() +} + function normalizeRequestedSkillPath(input: string): string | null { const path = input.normalize('NFC') if (path === '.') @@ -953,10 +1068,17 @@ function comparePath(left: string, right: string): number { return left < right ? -1 : left > right ? 1 : 0 } -async function readBoundedJson(response: Response, maximumBytes: number): Promise { +async function readBoundedJson( + response: Response, + maximumBytes: number, +): Promise<{ _tag: 'json', value: unknown } | { _tag: 'too-large' }> { const declared = Number(response.headers.get('content-length')) - if (Number.isFinite(declared) && declared > maximumBytes) - throw new Error('GitHub response exceeded the byte limit') + if (Number.isFinite(declared) && declared > maximumBytes) { + await response.body?.cancel().catch(() => { + // The declared size already decided the outcome. A body that will not close changes nothing. + }) + return { _tag: 'too-large' } + } if (!response.body) throw new Error('GitHub returned an empty response') const reader = response.body.getReader() @@ -969,7 +1091,7 @@ async function readBoundedJson(response: Response, maximumBytes: number): Promis size += next.value.byteLength if (size > maximumBytes) { await reader.cancel('response too large') - throw new Error('GitHub response exceeded the byte limit') + return { _tag: 'too-large' } } chunks.push(next.value) } @@ -979,11 +1101,26 @@ async function readBoundedJson(response: Response, maximumBytes: number): Promis bytes.set(chunk, offset) offset += chunk.byteLength } - return JSON.parse(new TextDecoder().decode(bytes)) as unknown + return { _tag: 'json', value: JSON.parse(new TextDecoder().decode(bytes)) as unknown } +} + +/** + * The API URL a moved Repository answers with, or null for any other redirect. + * + * Only `/repositories/` on the API origin passes, so the token never + * leaves api.github.com and no redirect reads an arbitrary path. + */ +function movedRepositoryUrl(location: string | null): string | null { + if (!location) + return null + const url = URL.parse(location, GITHUB_API) + if (!url || url.origin !== GITHUB_API || url.username || url.password) + return null + return /^\/repositories\/[1-9]\d*(?:\/|$)/.test(url.pathname) ? url.href : null } function sourceReadRejection( - reason: 'not-found' | 'access-denied' | 'identity-mismatch', + reason: 'not-found' | 'access-denied' | 'identity-mismatch' | 'too-large', visibility: 'public' | 'private', ): SourceRejection { if (visibility === 'private' && (reason === 'not-found' || reason === 'access-denied')) @@ -992,6 +1129,8 @@ function sourceReadRejection( return reject('SOURCE_NOT_FOUND', 'The Git object was not found.', []) if (reason === 'access-denied') return reject('SOURCE_ACCESS_DENIED', 'GitHub denied access to the Git object.', []) + if (reason === 'too-large') + return reject('INVALID_SOURCE', 'The GitHub response is larger than the read limit.', []) return reject('INVALID_SOURCE', 'GitHub returned another Git object identity.', []) } @@ -1000,8 +1139,17 @@ function sourceLimitRejection(summary: string, findings: string[] = []): SourceR } type FailedReadOutcome - = { _tag: 'not-found' | 'access-denied' | 'identity-mismatch' } + = { _tag: 'not-found' | 'access-denied' | 'identity-mismatch' | 'too-large' } + | { _tag: 'rate-limited', resetAt: number | null } + +/** One GitHub JSON read that reached an answer. A failed path to GitHub throws instead. */ +type ReadOutcome + = | { _tag: 'ok', value: T } + | { _tag: 'not-found' } + | { _tag: 'access-denied' } | { _tag: 'rate-limited', resetAt: number | null } + /** The body passed `MAX_GITHUB_JSON_BYTES`. A retry gets the same body. */ + | { _tag: 'too-large' } function epochHeader(headers: Headers, name: string): number | null { const raw = headers.get(name) diff --git a/test/unit/artifact-github-source-scale.test.ts b/test/unit/artifact-github-source-scale.test.ts new file mode 100644 index 00000000..fd5770e2 --- /dev/null +++ b/test/unit/artifact-github-source-scale.test.ts @@ -0,0 +1,284 @@ +import { createHash } from 'node:crypto' +import { describe, expect, it, vi } from 'vitest' +import { createPublicGithubSourceClient } from '../../layers/artifact-delivery/server/utils/github-source' + +const commitSha = '0123456789abcdef0123456789abcdef01234567' +const rootTreeSha = '89abcdef0123456789abcdef0123456789abcdef' +const docsTreeSha = '4444444444444444444444444444444444444444' +const skillsTreeSha = '1111111111111111111111111111111111111111' +const skillTreeSha = '2222222222222222222222222222222222222222' +const skillText = '---\nname: demo\ndescription: Use this Skill for demo work.\n---\n' +const skillBlobSha = gitBlobSha(skillText) +const API = 'https://api.github.com' + +describe('a Repository tree larger than one GitHub response', () => { + // posthog/posthog, n8n-io/n8n, vercel/next.js and openshift/hypershift + // answer a recursive tree read with more than 8 MiB. + it('finds a named Skill when the recursive tree exceeds the read limit', async () => { + const fetchMock = splitRepositoryFetch({ root: 'oversized' }) + const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) + + const result = await client.resolve(namedRequest('demo')) + + expect(result).toMatchObject({ _tag: 'resolved', source: { skillPath: 'skills/demo' } }) + }) + + it('finds a named Skill when GitHub truncates the recursive tree', async () => { + const fetchMock = splitRepositoryFetch({ root: 'truncated' }) + const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) + + const result = await client.resolve(namedRequest('demo')) + + expect(result).toMatchObject({ _tag: 'resolved', source: { skillPath: 'skills/demo' } }) + }) + + it('asks for a path when one directory is too large to list', async () => { + const fetchMock = splitRepositoryFetch({ root: 'oversized', rootLevel: 'truncated' }) + const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) + + const result = await client.resolve(namedRequest('demo')) + + expect(result).toMatchObject({ + _tag: 'rejected', + code: 'INVALID_SOURCE', + summary: 'The Repository is too large to find a Skill by name. Name the Skill by its path.', + }) + }) + + it('stops before it spends reads the search cannot finish', async () => { + // posthog/posthog splits into more folders than one search may read. + const folders = Array.from({ length: 80 }, (_, index) => tree(`product-${index}`, index.toString(16).padStart(40, '5'))) + const fetchMock = vi.fn(async (input: RequestInfo | URL) => { + const url = String(input) + if (url === `${API}/repos/skilld-dev/skills`) + return json(repository('skilld-dev', 'skills')) + if (url.endsWith('/git/ref/heads/main')) + return json({ ref: 'refs/heads/main', object: { type: 'commit', sha: commitSha } }) + if (url.endsWith(`/commits/${commitSha}`)) + return json({ sha: commitSha, commit: { tree: { sha: rootTreeSha } } }) + if (url.endsWith(`/git/trees/${rootTreeSha}?recursive=1`)) + return oversized() + if (url.endsWith(`/git/trees/${rootTreeSha}`)) + return json({ sha: rootTreeSha, tree: folders }) + return json({}, 404) + }) + const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) + + const result = await client.resolve(namedRequest('demo')) + + expect(result).toMatchObject({ _tag: 'rejected', code: 'INVALID_SOURCE' }) + expect(fetchMock.mock.calls.filter(call => String(call[0]).includes('/git/trees/'))).toHaveLength(2) + }) + + it('loads a small Skill folder whose recursive listing exceeds the read limit', async () => { + const fetchMock = vi.fn(async (input: RequestInfo | URL) => { + const url = String(input) + if (url === `${API}/repos/skilld-dev/skills`) + return json(repository('skilld-dev', 'skills')) + if (url.endsWith(`/git/trees/${rootTreeSha}`)) + return json({ sha: rootTreeSha, tree: [tree('skills', skillsTreeSha)] }) + if (url.endsWith(`/git/trees/${skillsTreeSha}`)) + return json({ sha: skillsTreeSha, tree: [tree('demo', skillTreeSha)] }) + if (url.endsWith(`/git/trees/${skillTreeSha}?recursive=1`)) + return oversized() + if (url.endsWith(`/git/trees/${skillTreeSha}`)) + return json({ sha: skillTreeSha, tree: [blob('SKILL.md', skillBlobSha, skillText.length)] }) + if (url.endsWith(`/git/blobs/${skillBlobSha}`)) + return json(blobResponse(skillText, skillBlobSha)) + return json({}, 404) + }) + const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) + + const result = await client.load(resolvedSource('skilld-dev', 'skills')) + + expect(result._tag).toBe('loaded') + }) +}) + +describe('a moved Repository', () => { + // facebook/react moved to react/react. GitHub answers the old name with a + // 301 to /repositories/. + it('resolves under the name the request used', async () => { + const fetchMock = movedRepositoryFetch() + const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) + + const result = await client.resolve({ + provider: 'github', + owner: 'facebook', + repository: 'react', + selector: { type: 'path', path: 'skills/demo' }, + ref: { type: 'commit', value: commitSha }, + }) + + expect(result).toEqual({ + _tag: 'resolved', + source: { + provider: 'github', + repositoryId: 10270250, + owner: 'facebook', + repository: 'react', + visibility: 'public', + commitSha, + treeSha: rootTreeSha, + skillPath: 'skills/demo', + }, + }) + }) + + it('loads every Git object from the new name', async () => { + const fetchMock = movedRepositoryFetch() + const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) + + const result = await client.load(resolvedSource('facebook', 'react', 10270250)) + + expect(result._tag).toBe('loaded') + const reads = fetchMock.mock.calls.map(call => String(call[0])) + expect(reads.filter(url => url.startsWith(`${API}/repos/facebook/react`))).toEqual([`${API}/repos/facebook/react`]) + }) + + it('rejects a moved name that now leads to another Repository', async () => { + const fetchMock = movedRepositoryFetch() + const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) + + const result = await client.load(resolvedSource('facebook', 'react', 999)) + + expect(result).toMatchObject({ _tag: 'rejected', code: 'INVALID_SOURCE' }) + }) + + it('never follows a redirect off the GitHub API', async () => { + const fetchMock = vi.fn(async (input: RequestInfo | URL) => { + const url = String(input) + if (url === `${API}/repos/facebook/react`) + return new Response(null, { status: 301, headers: { location: 'https://example.com/repositories/10270250' } }) + return json(repository('react', 'react', 10270250)) + }) + const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) + + await expect(client.resolve({ + provider: 'github', + owner: 'facebook', + repository: 'react', + selector: { type: 'path', path: 'skills/demo' }, + ref: { type: 'commit', value: commitSha }, + })).rejects.toThrow('GitHub redirected 301') + expect(fetchMock.mock.calls.map(call => String(call[0]))).toEqual([`${API}/repos/facebook/react`]) + }) +}) + +/** + * A Repository whose root is `docs/` and `skills/demo/SKILL.md`. The root + * recursive read is the large one; each top-level folder lists in one read. + */ +function splitRepositoryFetch(options: { root: 'oversized' | 'truncated', rootLevel?: 'truncated' }) { + return vi.fn(async (input: RequestInfo | URL) => { + const url = String(input) + if (url === `${API}/repos/skilld-dev/skills`) + return json(repository('skilld-dev', 'skills')) + if (url.endsWith('/git/ref/heads/main')) + return json({ ref: 'refs/heads/main', object: { type: 'commit', sha: commitSha } }) + if (url.endsWith(`/commits/${commitSha}`)) + return json({ sha: commitSha, commit: { tree: { sha: rootTreeSha } } }) + if (url.endsWith(`/git/trees/${rootTreeSha}?recursive=1`)) { + return options.root === 'oversized' + ? oversized() + : json({ sha: rootTreeSha, tree: [tree('docs', docsTreeSha)], truncated: true }) + } + if (url.endsWith(`/git/trees/${rootTreeSha}`)) { + return json({ + sha: rootTreeSha, + tree: [blob('README.md', skillBlobSha, 10), tree('docs', docsTreeSha), tree('skills', skillsTreeSha)], + truncated: options.rootLevel === 'truncated', + }) + } + if (url.endsWith(`/git/trees/${docsTreeSha}?recursive=1`)) + return json({ sha: docsTreeSha, tree: [blob('guide.md', skillBlobSha, 10)], truncated: false }) + if (url.endsWith(`/git/trees/${skillsTreeSha}?recursive=1`)) { + return json({ + sha: skillsTreeSha, + tree: [tree('demo', skillTreeSha), blob('demo/SKILL.md', skillBlobSha, skillText.length)], + truncated: false, + }) + } + return json({}, 404) + }) +} + +/** facebook/react answers 301; react/react serves every Git object. */ +function movedRepositoryFetch() { + return vi.fn(async (input: RequestInfo | URL) => { + const url = String(input) + if (url.startsWith(`${API}/repos/facebook/react`)) + return new Response(null, { status: 301, headers: { location: `${API}/repositories/10270250` } }) + if (url === `${API}/repositories/10270250`) + return json(repository('react', 'react', 10270250)) + const moved = `${API}/repos/react/react` + if (url === `${moved}/commits/${commitSha}`) + return json({ sha: commitSha, commit: { tree: { sha: rootTreeSha } } }) + if (url === `${moved}/git/trees/${rootTreeSha}`) + return json({ sha: rootTreeSha, tree: [tree('skills', skillsTreeSha)] }) + if (url === `${moved}/git/trees/${skillsTreeSha}`) + return json({ sha: skillsTreeSha, tree: [tree('demo', skillTreeSha)] }) + if (url === `${moved}/git/trees/${skillTreeSha}?recursive=1`) + return json({ sha: skillTreeSha, tree: [blob('SKILL.md', skillBlobSha, skillText.length)], truncated: false }) + if (url === `${moved}/git/blobs/${skillBlobSha}`) + return json(blobResponse(skillText, skillBlobSha)) + return json({}, 404) + }) +} + +function namedRequest(name: string) { + return { + provider: 'github' as const, + owner: 'skilld-dev', + repository: 'skills', + selector: { type: 'named-skill' as const, name }, + } +} + +function resolvedSource(owner: string, name: string, repositoryId = 123) { + return { + provider: 'github' as const, + repositoryId, + owner, + repository: name, + visibility: 'public' as const, + commitSha, + treeSha: rootTreeSha, + skillPath: 'skills/demo', + } +} + +function repository(owner: string, name: string, id = 123) { + return { id, name, owner: { login: owner }, private: false, default_branch: 'main' } +} + +/** A tree response one byte past the 8 MiB read limit. */ +function oversized(): Response { + return new Response(`{"pad":"${'x'.repeat(8 * 1024 * 1024)}"}`, { + status: 200, + headers: { 'content-type': 'application/json' }, + }) +} + +function tree(path: string, sha: string) { + return { path, mode: '040000', type: 'tree', sha } +} + +function blob(path: string, sha: string, size: number) { + return { path, mode: '100644', type: 'blob', sha, size } +} + +function blobResponse(value: string, sha: string) { + return { sha, size: value.length, encoding: 'base64', content: btoa(value) } +} + +function gitBlobSha(value: string): string { + return createHash('sha1').update(`blob ${value.length}\0${value}`).digest('hex') +} + +function json(value: unknown, status = 200): Response { + return new Response(JSON.stringify(value), { + status, + headers: { 'content-type': 'application/json' }, + }) +} diff --git a/test/unit/artifact-named-skill.test.ts b/test/unit/artifact-named-skill.test.ts index 85826b80..b70513a0 100644 --- a/test/unit/artifact-named-skill.test.ts +++ b/test/unit/artifact-named-skill.test.ts @@ -77,7 +77,9 @@ describe('named Skill resolution', () => { expect(result).toMatchObject({ _tag: 'resolved', source: { skillPath: '.' } }) }) - it('asks for a path when GitHub cannot list the whole Repository', async () => { + // A truncated recursive tree is split level by level; only a level GitHub + // also truncates leaves the name search nothing to read. + it('asks for a path when GitHub truncates even one tree level', async () => { const result = await resolveNamed('demo', [blob('skills/demo/SKILL.md')], true) expect(result).toMatchObject({ @@ -99,6 +101,8 @@ async function resolveNamed(name: string, entries: object[], truncated = false, return json({ sha: commitSha, commit: { tree: { sha: rootTreeSha } } }) if (url.endsWith(`/git/trees/${rootTreeSha}?recursive=1`)) return json({ sha: rootTreeSha, tree: entries, truncated }) + if (url.endsWith(`/git/trees/${rootTreeSha}`)) + return json({ sha: rootTreeSha, tree: entries, truncated }) return json({}, 404) }) const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch })