From 4c45d156af505060f21dfe48c6d0cc890ddc6878 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 21 Aug 2026 01:14:11 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=8C=20refactor:=20Extract=20Git=20Repo?= =?UTF-8?q?sitory=20Adapter=20From=20Skill=20Sync=20(#15052)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🔌 refactor: Extract Git Repository Adapter From Skill Sync Skill sync interleaved GitHub REST calls with orchestration that is not GitHub-specific in any way — discovery, import limits, upsert and stale reconciliation, status accounting. Adding a second provider meant either threading provider branches through that orchestration or forking it. Introduces `GitRepoAdapter` — `resolveCommit`, `fetchTreeEntries`, `fetchFileContent` over a normalized `RepoTreeEntry` — and moves the GitHub REST client behind it. The runner keeps its GitHub source typing; only the transport moved. No behavior change: every pre-existing sync test passes untouched, still driving real GitHub responses through the mocked `fetchFn`. * 🔧 fix: Export GitHubRepoAdapterConfig alongside GitRepoAdapter Self-review: the exported `createAdapter` dep names a config type that consumers could not import, leaving half its signature unnameable. --- .../src/skills/sync/adapters/github.spec.ts | 332 +++++++++++++++++ .../api/src/skills/sync/adapters/github.ts | 245 +++++++++++++ .../api/src/skills/sync/adapters/types.ts | 84 +++++ packages/api/src/skills/sync/errors.ts | 15 + packages/api/src/skills/sync/github.spec.ts | 86 +++++ packages/api/src/skills/sync/github.ts | 343 +++--------------- packages/api/src/skills/sync/index.ts | 1 + packages/api/src/skills/sync/path.ts | 10 + 8 files changed, 833 insertions(+), 283 deletions(-) create mode 100644 packages/api/src/skills/sync/adapters/github.spec.ts create mode 100644 packages/api/src/skills/sync/adapters/github.ts create mode 100644 packages/api/src/skills/sync/adapters/types.ts create mode 100644 packages/api/src/skills/sync/errors.ts create mode 100644 packages/api/src/skills/sync/path.ts diff --git a/packages/api/src/skills/sync/adapters/github.spec.ts b/packages/api/src/skills/sync/adapters/github.spec.ts new file mode 100644 index 0000000000..c2b30d1921 --- /dev/null +++ b/packages/api/src/skills/sync/adapters/github.spec.ts @@ -0,0 +1,332 @@ +import type { SkillSyncGitHubSourceConfig } from 'librechat-data-provider'; +import type { RepoCommit } from './types'; +import { createGitHubRepoAdapter } from './github'; + +function response(body: unknown, status = 200, headers: Record = {}): Response { + const normalizedHeaders = new Map( + Object.entries(headers).map(([key, value]) => [key.toLowerCase(), value]), + ); + return { + ok: status >= 200 && status < 300, + status, + headers: { + get: (key: string) => normalizedHeaders.get(key.toLowerCase()) ?? null, + }, + json: async () => body, + } as unknown as Response; +} + +function treeEntry(overrides: Record = {}) { + return { + path: 'SKILL.md', + mode: '100644', + type: 'blob', + sha: 'blob-sha', + size: 12, + url: 'https://api.github.test/blob', + ...overrides, + }; +} + +function createSource( + overrides: Partial = {}, +): SkillSyncGitHubSourceConfig { + return { + id: 'librechat-skills', + owner: 'LibreChat', + repo: 'skills', + ref: 'main', + paths: ['skills'], + credentialKey: 'github-skills-prod', + ...overrides, + }; +} + +function createAdapter(fetchFn: typeof fetch, source = createSource()) { + return createGitHubRepoAdapter({ source, token: 'github_pat_secret', fetchFn }); +} + +const commit: RepoCommit = { id: 'commit-sha', treeId: 'tree-sha' }; + +describe('createGitHubRepoAdapter', () => { + describe('resolveCommit', () => { + it('resolves the configured ref to its commit and root tree', async () => { + const fetchFn = jest.fn(async () => + response({ sha: 'commit-sha', commit: { tree: { sha: 'tree-sha' } } }), + ) as unknown as typeof fetch; + + await expect(createAdapter(fetchFn).resolveCommit()).resolves.toEqual({ + id: 'commit-sha', + treeId: 'tree-sha', + }); + expect(fetchFn).toHaveBeenCalledWith( + 'https://api.github.com/repos/LibreChat/skills/commits/main', + { + headers: expect.objectContaining({ + Accept: 'application/vnd.github+json', + Authorization: 'Bearer github_pat_secret', + 'X-GitHub-Api-Version': '2022-11-28', + 'User-Agent': 'LibreChat-Skill-Sync', + }), + }, + ); + }); + + it('encodes each ref segment without escaping its separators', async () => { + const fetchFn = jest.fn(async () => + response({ sha: 'commit-sha', commit: { tree: { sha: 'tree-sha' } } }), + ) as unknown as typeof fetch; + + await createAdapter(fetchFn, createSource({ ref: 'release/v1 rc' })).resolveCommit(); + + expect(fetchFn).toHaveBeenCalledWith( + 'https://api.github.com/repos/LibreChat/skills/commits/release/v1%20rc', + expect.anything(), + ); + }); + }); + + describe('fetchTreeEntries', () => { + it('lists the whole repository from the root tree when no path is configured', async () => { + const fetchFn = jest.fn(async () => + response({ sha: 'tree-sha', truncated: false, tree: [treeEntry()] }), + ) as unknown as typeof fetch; + + const entries = await createAdapter(fetchFn).fetchTreeEntries(commit, { + pathPrefix: '', + assertNotCancelled: () => undefined, + }); + + expect(entries).toEqual([{ path: 'SKILL.md', type: 'blob', id: 'blob-sha', size: 12 }]); + expect(fetchFn).toHaveBeenCalledTimes(1); + expect(fetchFn).toHaveBeenCalledWith( + 'https://api.github.com/repos/LibreChat/skills/git/trees/tree-sha?recursive=1', + expect.anything(), + ); + }); + + it('walks one non-recursive listing per path segment before listing recursively', async () => { + const fetchFn = jest.fn(async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes('/git/trees/tree-sha')) { + return response({ + sha: 'tree-sha', + truncated: false, + tree: [treeEntry({ path: 'skills', type: 'tree', sha: 'skills-tree-sha' })], + }); + } + if (url.includes('/git/trees/skills-tree-sha')) { + return response({ + sha: 'skills-tree-sha', + truncated: false, + tree: [treeEntry({ path: 'shared', type: 'tree', sha: 'shared-tree-sha' })], + }); + } + return response({ + sha: 'shared-tree-sha', + truncated: false, + tree: [treeEntry({ path: 'research/SKILL.md', sha: 'skill-md-sha' })], + }); + }) as unknown as typeof fetch; + + const entries = await createAdapter(fetchFn).fetchTreeEntries(commit, { + pathPrefix: 'skills/shared', + assertNotCancelled: () => undefined, + }); + + expect(entries).toEqual([ + { path: 'skills/shared/research/SKILL.md', type: 'blob', id: 'skill-md-sha', size: 12 }, + ]); + const urls = (fetchFn as unknown as jest.Mock).mock.calls.map(([input]) => String(input)); + expect(urls).toEqual([ + 'https://api.github.com/repos/LibreChat/skills/git/trees/tree-sha', + 'https://api.github.com/repos/LibreChat/skills/git/trees/skills-tree-sha', + 'https://api.github.com/repos/LibreChat/skills/git/trees/shared-tree-sha?recursive=1', + ]); + }); + + it('reports a configured path that no tree segment matches', async () => { + const fetchFn = jest.fn(async () => + response({ sha: 'tree-sha', truncated: false, tree: [treeEntry()] }), + ) as unknown as typeof fetch; + + await expect( + createAdapter(fetchFn).fetchTreeEntries(commit, { + pathPrefix: 'skills', + assertNotCancelled: () => undefined, + }), + ).rejects.toMatchObject({ name: 'SkillSyncError', code: 'GITHUB_PATH_NOT_FOUND' }); + }); + + it('refuses a truncated listing rather than syncing a partial repository', async () => { + const fetchFn = jest.fn(async () => + response({ sha: 'tree-sha', truncated: true, tree: [treeEntry()] }), + ) as unknown as typeof fetch; + + await expect( + createAdapter(fetchFn).fetchTreeEntries(commit, { + pathPrefix: '', + assertNotCancelled: () => undefined, + }), + ).rejects.toMatchObject({ name: 'SkillSyncError', code: 'GITHUB_TREE_TRUNCATED' }); + }); + + it('reports submodule entries as neither files nor directories', async () => { + const fetchFn = jest.fn(async () => + response({ + sha: 'tree-sha', + truncated: false, + tree: [ + treeEntry({ path: 'vendor', type: 'commit', sha: 'submodule-sha', size: undefined }), + treeEntry({ path: 'skills', type: 'tree', sha: 'dir-sha', size: undefined }), + ], + }), + ) as unknown as typeof fetch; + + const entries = await createAdapter(fetchFn).fetchTreeEntries(commit, { + pathPrefix: '', + assertNotCancelled: () => undefined, + }); + + expect(entries.map((entry) => entry.type)).toEqual(['submodule', 'tree']); + }); + + it('checks for cancellation between round trips', async () => { + const fetchFn = jest.fn(async (input: RequestInfo | URL) => + String(input).includes('recursive=1') + ? response({ sha: 'skills-tree-sha', truncated: false, tree: [treeEntry()] }) + : response({ + sha: 'tree-sha', + truncated: false, + tree: [treeEntry({ path: 'skills', type: 'tree', sha: 'skills-tree-sha' })], + }), + ) as unknown as typeof fetch; + const assertNotCancelled = jest.fn(); + + await createAdapter(fetchFn).fetchTreeEntries(commit, { + pathPrefix: 'skills', + assertNotCancelled, + }); + + expect(assertNotCancelled.mock.calls.length).toBeGreaterThanOrEqual(4); + }); + + it('stops listing as soon as cancellation is signalled', async () => { + const fetchFn = jest.fn(async () => + response({ + sha: 'tree-sha', + truncated: false, + tree: [treeEntry({ path: 'skills', type: 'tree', sha: 'skills-tree-sha' })], + }), + ) as unknown as typeof fetch; + + await expect( + createAdapter(fetchFn).fetchTreeEntries(commit, { + pathPrefix: 'skills', + assertNotCancelled: () => { + throw new Error('cancelled'); + }, + }), + ).rejects.toThrow('cancelled'); + expect(fetchFn).not.toHaveBeenCalled(); + }); + }); + + describe('fetchFileContent', () => { + it('decodes base64 blob content, ignoring the wrapping whitespace GitHub inserts', async () => { + const fetchFn = jest.fn(async () => + response({ + sha: 'blob-sha', + encoding: 'base64', + size: 5, + content: `${Buffer.from('hello').toString('base64')}\n`, + }), + ) as unknown as typeof fetch; + + const buffer = await createAdapter(fetchFn).fetchFileContent(commit, { + path: 'skills/research/SKILL.md', + type: 'blob', + id: 'skill-md-sha', + }); + + expect(buffer.toString('utf-8')).toBe('hello'); + expect(fetchFn).toHaveBeenCalledWith( + 'https://api.github.com/repos/LibreChat/skills/git/blobs/skill-md-sha', + expect.anything(), + ); + }); + + it('refuses a blob encoding it cannot decode', async () => { + const fetchFn = jest.fn(async () => + response({ sha: 'blob-sha', encoding: 'utf-8', size: 5, content: 'hello' }), + ) as unknown as typeof fetch; + + await expect( + createAdapter(fetchFn).fetchFileContent(commit, { + path: 'skills/research/SKILL.md', + type: 'blob', + id: 'skill-md-sha', + }), + ).rejects.toMatchObject({ name: 'SkillSyncError', code: 'GITHUB_UNSUPPORTED_BLOB' }); + }); + }); + + describe('error classification', () => { + it.each([ + { + label: 'an exhausted rate limit budget', + status: 403, + headers: { 'x-ratelimit-remaining': '0' }, + code: 'GITHUB_RATE_LIMITED', + }, + { + label: 'a retry-after directive', + status: 403, + headers: { 'retry-after': '60' }, + code: 'GITHUB_RATE_LIMITED', + }, + { + label: 'an explicit rate limit status', + status: 429, + headers: {}, + code: 'GITHUB_RATE_LIMITED', + }, + { label: 'a rejected credential', status: 401, headers: {}, code: 'GITHUB_AUTH_FAILED' }, + { label: 'a forbidden repository', status: 403, headers: {}, code: 'GITHUB_AUTH_FAILED' }, + { label: 'a missing repository', status: 404, headers: {}, code: 'GITHUB_NOT_FOUND' }, + { label: 'an upstream outage', status: 500, headers: {}, code: 'GITHUB_REQUEST_FAILED' }, + ])('maps $label to $code', async ({ status, headers, code }) => { + const fetchFn = jest.fn(async () => + response({ message: 'nope' }, status, headers as Record), + ) as unknown as typeof fetch; + + await expect(createAdapter(fetchFn).resolveCommit()).rejects.toMatchObject({ + name: 'SkillSyncError', + code, + }); + }); + + it('treats a rate limit explained only in the body as a rate limit', async () => { + const fetchFn = jest.fn(async () => + response({ message: 'You have exceeded a secondary rate limit' }, 403), + ) as unknown as typeof fetch; + + await expect(createAdapter(fetchFn).resolveCommit()).rejects.toMatchObject({ + name: 'SkillSyncError', + code: 'GITHUB_RATE_LIMITED', + }); + }); + + it('reports a transport failure without leaking the underlying error', async () => { + const fetchFn = jest.fn(async () => { + throw new Error('ECONNREFUSED 140.82.121.6:443'); + }) as unknown as typeof fetch; + + await expect(createAdapter(fetchFn).resolveCommit()).rejects.toMatchObject({ + name: 'SkillSyncError', + code: 'GITHUB_REQUEST_FAILED', + message: expect.not.stringContaining('ECONNREFUSED'), + }); + }); + }); +}); diff --git a/packages/api/src/skills/sync/adapters/github.ts b/packages/api/src/skills/sync/adapters/github.ts new file mode 100644 index 0000000000..39b87d040a --- /dev/null +++ b/packages/api/src/skills/sync/adapters/github.ts @@ -0,0 +1,245 @@ +import type { SkillSyncGitHubSourceConfig } from 'librechat-data-provider'; +import type { + RepoCommit, + RepoTreeEntry, + GitRepoAdapter, + AssertNotCancelled, + RepoTreeEntryType, + FetchTreeEntriesParams, +} from './types'; +import { normalizeRepoPath } from '../path'; +import { SkillSyncError } from '../errors'; + +const GITHUB_API_BASE = 'https://api.github.com'; + +export const GITHUB_FINE_GRAINED_TOKEN_RECOMMENDATION = + 'Use a GitHub fine-grained personal access token scoped to the selected repository with read-only Contents and Metadata permissions.'; + +type FetchFn = typeof fetch; + +type GitHubTreeEntry = { + path: string; + mode: string; + type: 'blob' | 'tree' | 'commit'; + sha: string; + size?: number; + url: string; +}; + +type GitHubTreeResponse = { + sha: string; + tree: GitHubTreeEntry[]; + truncated: boolean; +}; + +type GitHubBlobResponse = { + sha: string; + content: string; + encoding: string; + size: number; +}; + +type GitHubCommitResponse = { + sha: string; + commit: { + tree: { + sha: string; + }; + }; +}; + +export type GitHubRepoAdapterConfig = { + source: SkillSyncGitHubSourceConfig; + token: string; + fetchFn: FetchFn; +}; + +function buildGitHubHeaders(token: string): HeadersInit { + return { + Accept: 'application/vnd.github+json', + Authorization: `Bearer ${token}`, + 'X-GitHub-Api-Version': '2022-11-28', + 'User-Agent': 'LibreChat-Skill-Sync', + }; +} + +function buildGitHubUrl(pathname: string): string { + return `${GITHUB_API_BASE}${pathname}`; +} + +function encodeGitHubPath(value: string): string { + return value.split('/').map(encodeURIComponent).join('/'); +} + +async function readGitHubErrorMessage(response: Response): Promise { + try { + const body = (await response.json()) as { message?: unknown }; + return typeof body.message === 'string' ? body.message : undefined; + } catch { + return undefined; + } +} + +function isGitHubRateLimitResponse(params: { + status: number; + remaining: string | null; + retryAfter: string | null; + message?: string; +}): boolean { + if (params.status === 429 || params.remaining === '0' || params.retryAfter) { + return true; + } + const message = params.message?.toLowerCase() ?? ''; + return message.includes('rate limit') || message.includes('abuse detection'); +} + +async function githubJson(params: { + fetchFn: FetchFn; + token: string; + pathname: string; +}): Promise { + let response: Response; + try { + response = await params.fetchFn(buildGitHubUrl(params.pathname), { + headers: buildGitHubHeaders(params.token), + }); + } catch { + throw new SkillSyncError( + 'GITHUB_REQUEST_FAILED', + 'GitHub request failed before receiving a response', + ); + } + if (response.ok) { + return (await response.json()) as T; + } + const remaining = response.headers.get('x-ratelimit-remaining'); + const retryAfter = response.headers.get('retry-after'); + const message = await readGitHubErrorMessage(response); + if (response.status === 401 || response.status === 403 || response.status === 429) { + const code = isGitHubRateLimitResponse({ + status: response.status, + remaining, + retryAfter, + message, + }) + ? 'GITHUB_RATE_LIMITED' + : 'GITHUB_AUTH_FAILED'; + throw new SkillSyncError(code, `GitHub request failed with HTTP ${response.status}`); + } + if (response.status === 404) { + throw new SkillSyncError('GITHUB_NOT_FOUND', 'GitHub repository, ref, or path was not found'); + } + throw new SkillSyncError( + 'GITHUB_REQUEST_FAILED', + `GitHub request failed with HTTP ${response.status}`, + ); +} + +/** GitHub reports submodules as `commit` entries; every other type maps across as-is. */ +function toRepoTreeEntryType(type: GitHubTreeEntry['type']): RepoTreeEntryType { + return type === 'commit' ? 'submodule' : type; +} + +function toRepoTreeEntry(entry: GitHubTreeEntry, path: string): RepoTreeEntry { + return { + path, + type: toRepoTreeEntryType(entry.type), + id: entry.sha, + size: entry.size, + }; +} + +export function createGitHubRepoAdapter(config: GitHubRepoAdapterConfig): GitRepoAdapter { + const { source, token, fetchFn } = config; + const owner = encodeURIComponent(source.owner); + const repo = encodeURIComponent(source.repo); + + async function fetchTree(treeSha: string, recursive: boolean): Promise { + return githubJson({ + fetchFn, + token, + pathname: `/repos/${owner}/${repo}/git/trees/${encodeURIComponent(treeSha)}${ + recursive ? '?recursive=1' : '' + }`, + }); + } + + /** + * GitHub addresses trees by SHA rather than by path, so reaching a configured + * subdirectory means walking one non-recursive listing per path segment before + * the recursive listing that actually enumerates it. + */ + async function resolveTreeShaAtPath( + rootTreeSha: string, + normalizedPath: string, + assertNotCancelled: AssertNotCancelled, + ): Promise { + let treeSha = rootTreeSha; + for (const segment of normalizedPath.split('/')) { + assertNotCancelled(); + const tree = await fetchTree(treeSha, false); + assertNotCancelled(); + if (tree.truncated) { + throw new SkillSyncError('GITHUB_TREE_TRUNCATED', 'GitHub tree response was truncated'); + } + const next = tree.tree.find((entry) => entry.type === 'tree' && entry.path === segment); + if (!next) { + throw new SkillSyncError( + 'GITHUB_PATH_NOT_FOUND', + `Configured GitHub skill path "${normalizedPath}" was not found`, + ); + } + treeSha = next.sha; + } + return treeSha; + } + + async function resolveCommit(): Promise { + const commit = await githubJson({ + fetchFn, + token, + pathname: `/repos/${owner}/${repo}/commits/${encodeGitHubPath(source.ref)}`, + }); + return { id: commit.sha, treeId: commit.commit.tree.sha }; + } + + async function fetchTreeEntries( + commit: RepoCommit, + params: FetchTreeEntriesParams, + ): Promise { + const normalizedPath = normalizeRepoPath(params.pathPrefix); + const treeSha = normalizedPath + ? await resolveTreeShaAtPath(commit.treeId, normalizedPath, params.assertNotCancelled) + : commit.treeId; + + params.assertNotCancelled(); + const tree = await fetchTree(treeSha, true); + params.assertNotCancelled(); + if (tree.truncated) { + throw new SkillSyncError('GITHUB_TREE_TRUNCATED', 'GitHub tree response was truncated'); + } + if (!normalizedPath) { + return tree.tree.map((entry) => toRepoTreeEntry(entry, entry.path)); + } + return tree.tree.map((entry) => + toRepoTreeEntry(entry, `${normalizedPath}/${normalizeRepoPath(entry.path)}`), + ); + } + + async function fetchFileContent(_commit: RepoCommit, entry: RepoTreeEntry): Promise { + const blob = await githubJson({ + fetchFn, + token, + pathname: `/repos/${owner}/${repo}/git/blobs/${encodeURIComponent(entry.id)}`, + }); + if (blob.encoding !== 'base64') { + throw new SkillSyncError( + 'GITHUB_UNSUPPORTED_BLOB', + `Unsupported GitHub blob encoding "${blob.encoding}"`, + ); + } + return Buffer.from(blob.content.replace(/\s/g, ''), 'base64'); + } + + return { resolveCommit, fetchTreeEntries, fetchFileContent }; +} diff --git a/packages/api/src/skills/sync/adapters/types.ts b/packages/api/src/skills/sync/adapters/types.ts new file mode 100644 index 0000000000..8676235631 --- /dev/null +++ b/packages/api/src/skills/sync/adapters/types.ts @@ -0,0 +1,84 @@ +/** + * `submodule` covers entries that are neither readable files nor descendable + * directories (a Git submodule, which GitHub reports as `commit`). They are + * carried through rather than dropped so path-existence checks keep seeing + * everything the repository actually contains. + */ +export type RepoTreeEntryType = 'blob' | 'tree' | 'submodule'; + +/** + * One entry in a repository tree, normalized across providers. + * + * `id` is the provider's identifier for the entry's content (a GitHub blob SHA, + * a GitLab blob id). It is round-tripped back into `fetchFileContent` and + * persisted in `sourceMetadata`, which lets a later run skip re-downloading a + * file whose id has not moved. Treat it as opaque outside the adapter. + * + * `size` is the blob's byte length when the provider reports it in the tree + * listing. Skill import limits are enforced against it before any content is + * downloaded, so an adapter whose tree endpoint omits size must populate it + * some other way rather than leave it undefined. + */ +export type RepoTreeEntry = { + path: string; + type: RepoTreeEntryType; + id: string; + size?: number; +}; + +/** The commit a single sync run is pinned to. */ +export type RepoCommit = { + /** Persisted as the run's `commitSha`; opaque outside the adapter. */ + id: string; + /** + * Root tree identifier as of this commit — where `fetchTreeEntries` starts + * walking. Distinct from `id` on providers whose commit and tree objects are + * addressed separately (GitHub); equal to `id` where they are not. + */ + treeId: string; +}; + +/** + * Throws when the run has been superseded or shut down. Adapters call it + * between network round trips so a long listing or download loop stops + * promptly instead of running to completion against a cancelled run. + */ +export type AssertNotCancelled = () => void; + +export type FetchTreeEntriesParams = { + /** Repository-root-relative directory to list, or `''` for the whole repository. */ + pathPrefix: string; + assertNotCancelled: AssertNotCancelled; +}; + +/** + * The provider-specific surface a skill sync source needs, and nothing more. + * Everything else in a run — skill discovery, import limits, database upsert + * and reconciliation, status accounting — is provider-agnostic and depends only + * on this interface. + * + * An adapter is bound to one configured source (its repository coordinates, + * ref, and credentials) when constructed, so no call takes them again. + */ +export interface GitRepoAdapter { + /** + * Resolves the source's configured ref to the commit that pins this run, so + * every entry listed and file fetched within it stays consistent even if the + * upstream ref moves mid-sync. + */ + resolveCommit(): Promise; + + /** + * Lists every entry beneath `pathPrefix` recursively, as of `commit`. Paths + * are returned relative to the repository root rather than to `pathPrefix`, + * so entries listed from several configured paths can be merged without + * ambiguity. + */ + fetchTreeEntries(commit: RepoCommit, params: FetchTreeEntriesParams): Promise; + + /** + * Fetches one file's raw bytes. The caller has already checked `entry.size` + * against the skill import limits, so implementations do not repeat that. + */ + fetchFileContent(commit: RepoCommit, entry: RepoTreeEntry): Promise; +} diff --git a/packages/api/src/skills/sync/errors.ts b/packages/api/src/skills/sync/errors.ts new file mode 100644 index 0000000000..2282c51583 --- /dev/null +++ b/packages/api/src/skills/sync/errors.ts @@ -0,0 +1,15 @@ +/** + * Failure raised anywhere in a skill sync run — repository access, skill + * preparation, or database reconciliation. `code` is persisted on the sync + * status document and surfaced through the admin API, so it must stay a stable + * machine-readable identifier rather than a message. + */ +export class SkillSyncError extends Error { + code: string; + + constructor(code: string, message: string) { + super(message); + this.name = 'SkillSyncError'; + this.code = code; + } +} diff --git a/packages/api/src/skills/sync/github.spec.ts b/packages/api/src/skills/sync/github.spec.ts index 76e4c16787..44b05e710b 100644 --- a/packages/api/src/skills/sync/github.spec.ts +++ b/packages/api/src/skills/sync/github.spec.ts @@ -12,6 +12,7 @@ import type { UpdateSkillResult, UpsertSkillFileInput, } from '@librechat/data-schemas'; +import type { RepoTreeEntry, GitRepoAdapter } from './adapters/types'; import type { GitHubSkillSyncDeps } from './github'; import { DEFAULT_SKILL_IMPORT_LIMITS } from '../limits'; import { createGitHubSkillSyncRunner } from './github'; @@ -3418,3 +3419,88 @@ describe('createGitHubSkillSyncRunner', () => { } }); }); + +describe('repository adapter seam', () => { + /** Stands in for any provider: a flat repository held in memory. */ + function createFakeAdapter(files: Record): GitRepoAdapter { + const entries: RepoTreeEntry[] = Object.entries(files).map(([path, content]) => ({ + path, + type: 'blob', + id: `${path}@1`, + size: Buffer.byteLength(content), + })); + return { + resolveCommit: async () => ({ id: 'fake-commit', treeId: 'fake-tree' }), + fetchTreeEntries: async (_commit, { pathPrefix }) => + entries.filter((entry) => !pathPrefix || entry.path.startsWith(`${pathPrefix}/`)), + fetchFileContent: async (_commit, entry) => Buffer.from(files[entry.path]), + }; + } + + it('publishes skills read through any repository client, with no provider requests', async () => { + const deps = createDeps({ + createAdapter: () => + createFakeAdapter({ + 'skills/research/SKILL.md': + '---\nname: research\ndescription: Research things\n---\nBody', + 'skills/research/scripts/run.sh': 'echo hi', + }), + }); + + const result = await createGitHubSkillSyncRunner(deps).runOnce(); + + expect(result.status).toBe('completed'); + expect(deps.fetchFn).not.toHaveBeenCalled(); + expect(deps.createSkill).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'research', + sourceMetadata: expect.objectContaining({ + commitSha: 'fake-commit', + skillBlobSha: 'skills/research/SKILL.md@1', + }), + }), + ); + expect(deps.upsertSkillFile).toHaveBeenCalledWith( + expect.objectContaining({ + relativePath: 'scripts/run.sh', + sourceMetadata: expect.objectContaining({ + commitSha: 'fake-commit', + blobSha: 'skills/research/scripts/run.sh@1', + }), + }), + ); + }); + + it('re-downloads a file only when the adapter reports a new content id', async () => { + const deps = createDeps({ + createAdapter: () => + createFakeAdapter({ + 'skills/research/SKILL.md': + '---\nname: research\ndescription: Research things\n---\nBody', + 'skills/research/scripts/run.sh': 'echo hi', + }), + getSkillFileByPath: jest.fn(async () => ({ + _id: new Types.ObjectId(), + skillId: new Types.ObjectId(), + relativePath: 'scripts/run.sh', + file_id: 'existing-file-id', + filename: 'run.sh', + filepath: '/uploads/existing-file-id__run.sh', + source: 'local', + sourceMetadata: { blobSha: 'skills/research/scripts/run.sh@1' }, + mimeType: 'application/x-sh', + bytes: 7, + category: 'script' as const, + isExecutable: false, + author: new Types.ObjectId(), + createdAt: new Date(), + updatedAt: new Date(), + })), + }); + + const result = await createGitHubSkillSyncRunner(deps).runOnce(); + + expect(result.status).toBe('completed'); + expect(deps.upsertSkillFile).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/api/src/skills/sync/github.ts b/packages/api/src/skills/sync/github.ts index 4009b555ea..82c518bd84 100644 --- a/packages/api/src/skills/sync/github.ts +++ b/packages/api/src/skills/sync/github.ts @@ -24,10 +24,22 @@ import type { SkillSyncStatusInput, } from '@librechat/data-schemas'; import type { SkillSyncConfig, SkillSyncGitHubSourceConfig } from 'librechat-data-provider'; +import type { + RepoCommit, + RepoTreeEntry, + GitRepoAdapter, + AssertNotCancelled, +} from './adapters/types'; +import type { GitHubRepoAdapterConfig } from './adapters/github'; +import { + GITHUB_FINE_GRAINED_TOKEN_RECOMMENDATION, + createGitHubRepoAdapter, +} from './adapters/github'; import { DEFAULT_SKILL_IMPORT_LIMITS } from '../limits'; import { parseSkillMarkdown } from '../parse'; +import { normalizeRepoPath } from './path'; +import { SkillSyncError } from './errors'; -const GITHUB_API_BASE = 'https://api.github.com'; const SYSTEM_AUTHOR_NAME = 'GitHub Sync'; let systemAuthorId: Types.ObjectId | undefined; @@ -51,42 +63,11 @@ const VALIDATION_ISSUE_FIELD_MAX = 100; const VALIDATION_ISSUE_CODE_MAX = 64; const VALIDATION_ISSUE_MESSAGE_MAX = 250; -export const GITHUB_FINE_GRAINED_TOKEN_RECOMMENDATION = - 'Use a GitHub fine-grained personal access token scoped to the selected repository with read-only Contents and Metadata permissions.'; +export { GITHUB_FINE_GRAINED_TOKEN_RECOMMENDATION }; +export type { GitHubRepoAdapterConfig }; type FetchFn = typeof fetch; -type GitHubTreeEntry = { - path: string; - mode: string; - type: 'blob' | 'tree' | 'commit'; - sha: string; - size?: number; - url: string; -}; - -type GitHubTreeResponse = { - sha: string; - tree: GitHubTreeEntry[]; - truncated: boolean; -}; - -type GitHubBlobResponse = { - sha: string; - content: string; - encoding: string; - size: number; -}; - -type GitHubCommitResponse = { - sha: string; - commit: { - tree: { - sha: string; - }; - }; -}; - type SyncCounters = { syncedSkillCount: number; syncedFileCount: number; @@ -95,12 +76,10 @@ type SyncCounters = { skippedSkillCount: number; }; -type AssertNotCancelled = () => void; - type DiscoveredSkill = { rootPath: string; - skillMd: GitHubTreeEntry; - files: GitHubTreeEntry[]; + skillMd: RepoTreeEntry; + files: RepoTreeEntry[]; }; type UpsertRemoteSkillResult = { @@ -239,6 +218,12 @@ export type GitHubSkillSyncDeps = { grantedBy: string | Types.ObjectId; }) => Promise; fetchFn?: FetchFn; + /** + * Builds the repository client a source is synced through. Defaults to the + * GitHub adapter; overridable so the orchestration can be exercised against a + * fake repository without standing up provider HTTP responses. + */ + createAdapter?: (config: GitHubRepoAdapterConfig) => GitRepoAdapter; lockOwner?: string; allowServerCredentials?: boolean; }; @@ -263,21 +248,6 @@ export type GitHubSkillSyncRunner = { runOnce: () => Promise; }; -class SkillSyncError extends Error { - code: string; - - constructor(code: string, message: string) { - super(message); - this.name = 'SkillSyncError'; - this.code = code; - } -} - -function normalizeRepoPath(value: string): string { - const trimmed = value.trim().replace(/^\/+|\/+$/g, ''); - return trimmed === '.' ? '' : trimmed; -} - function isSafeRelativePath(value: string): boolean { if (!value || value.startsWith('/') || value.startsWith('\\')) { return false; @@ -382,7 +352,7 @@ function getLimitMegabytes(bytes: number): number { return Math.round(bytes / 1024 / 1024); } -function assertGitHubBlobSize(entry: GitHubTreeEntry, relativePath: string): number { +function assertGitHubBlobSize(entry: RepoTreeEntry, relativePath: string): number { if (typeof entry.size !== 'number' || !Number.isFinite(entry.size) || entry.size < 0) { throw new SkillSyncError( 'GITHUB_BLOB_SIZE_UNKNOWN', @@ -439,7 +409,7 @@ function getSkillMdPath(discovered: DiscoveredSkill): string { return discovered.rootPath ? `${discovered.rootPath}/SKILL.md` : 'SKILL.md'; } -function getDiscoveredRelativePath(discovered: DiscoveredSkill, entry: GitHubTreeEntry): string { +function getDiscoveredRelativePath(discovered: DiscoveredSkill, entry: RepoTreeEntry): string { const prefix = discovered.rootPath ? `${discovered.rootPath}/` : ''; const normalized = normalizeRepoPath(entry.path); return prefix ? normalized.slice(prefix.length) : normalized; @@ -630,186 +600,23 @@ function truncateSkipName(name: string | undefined): string | undefined { return sanitized.length > SKIP_NAME_MAX ? `${sanitized.slice(0, SKIP_NAME_MAX - 1)}…` : sanitized; } -function buildGitHubHeaders(token: string): HeadersInit { - return { - Accept: 'application/vnd.github+json', - Authorization: `Bearer ${token}`, - 'X-GitHub-Api-Version': '2022-11-28', - 'User-Agent': 'LibreChat-Skill-Sync', - }; -} - -function buildGitHubUrl(pathname: string): string { - return `${GITHUB_API_BASE}${pathname}`; -} - -function encodeGitHubPath(value: string): string { - return value.split('/').map(encodeURIComponent).join('/'); -} - -async function readGitHubErrorMessage(response: Response): Promise { - try { - const body = (await response.json()) as { message?: unknown }; - return typeof body.message === 'string' ? body.message : undefined; - } catch { - return undefined; - } -} - -function isGitHubRateLimitResponse(params: { - status: number; - remaining: string | null; - retryAfter: string | null; - message?: string; -}): boolean { - if (params.status === 429 || params.remaining === '0' || params.retryAfter) { - return true; - } - const message = params.message?.toLowerCase() ?? ''; - return message.includes('rate limit') || message.includes('abuse detection'); -} - -async function githubJson(params: { - fetchFn: FetchFn; - token: string; - pathname: string; -}): Promise { - let response: Response; - try { - response = await params.fetchFn(buildGitHubUrl(params.pathname), { - headers: buildGitHubHeaders(params.token), - }); - } catch { - throw new SkillSyncError( - 'GITHUB_REQUEST_FAILED', - 'GitHub request failed before receiving a response', - ); - } - if (response.ok) { - return (await response.json()) as T; - } - const remaining = response.headers.get('x-ratelimit-remaining'); - const retryAfter = response.headers.get('retry-after'); - const message = await readGitHubErrorMessage(response); - if (response.status === 401 || response.status === 403 || response.status === 429) { - const code = isGitHubRateLimitResponse({ - status: response.status, - remaining, - retryAfter, - message, - }) - ? 'GITHUB_RATE_LIMITED' - : 'GITHUB_AUTH_FAILED'; - throw new SkillSyncError(code, `GitHub request failed with HTTP ${response.status}`); - } - if (response.status === 404) { - throw new SkillSyncError('GITHUB_NOT_FOUND', 'GitHub repository, ref, or path was not found'); - } - throw new SkillSyncError( - 'GITHUB_REQUEST_FAILED', - `GitHub request failed with HTTP ${response.status}`, - ); -} - -async function fetchCommit(params: { - fetchFn: FetchFn; - token: string; - source: SkillSyncGitHubSourceConfig; -}): Promise { - const owner = encodeURIComponent(params.source.owner); - const repo = encodeURIComponent(params.source.repo); - const ref = encodeGitHubPath(params.source.ref); - return githubJson({ - fetchFn: params.fetchFn, - token: params.token, - pathname: `/repos/${owner}/${repo}/commits/${ref}`, - }); -} - -async function fetchTree(params: { - fetchFn: FetchFn; - token: string; - source: SkillSyncGitHubSourceConfig; - treeSha: string; - recursive?: boolean; -}): Promise { - const owner = encodeURIComponent(params.source.owner); - const repo = encodeURIComponent(params.source.repo); - const treeSha = encodeURIComponent(params.treeSha); - const recursive = params.recursive ?? true; - return githubJson({ - fetchFn: params.fetchFn, - token: params.token, - pathname: `/repos/${owner}/${repo}/git/trees/${treeSha}${recursive ? '?recursive=1' : ''}`, - }); -} - -async function fetchTreeAtPath(params: { - fetchFn: FetchFn; - token: string; - source: SkillSyncGitHubSourceConfig; - rootTreeSha: string; - repoPath: string; - assertNotCancelled: AssertNotCancelled; -}): Promise { - const normalizedPath = normalizeRepoPath(params.repoPath); - let treeSha = params.rootTreeSha; - if (normalizedPath) { - for (const segment of normalizedPath.split('/')) { - params.assertNotCancelled(); - const tree = await fetchTree({ - fetchFn: params.fetchFn, - token: params.token, - source: params.source, - treeSha, - recursive: false, - }); - params.assertNotCancelled(); - if (tree.truncated) { - throw new SkillSyncError('GITHUB_TREE_TRUNCATED', 'GitHub tree response was truncated'); - } - const next = tree.tree.find((entry) => entry.type === 'tree' && entry.path === segment); - if (!next) { - throw new SkillSyncError( - 'GITHUB_PATH_NOT_FOUND', - `Configured GitHub skill path "${normalizedPath}" was not found`, - ); - } - treeSha = next.sha; - } - } - - params.assertNotCancelled(); - const tree = await fetchTree({ - fetchFn: params.fetchFn, - token: params.token, - source: params.source, - treeSha, - recursive: true, - }); - params.assertNotCancelled(); - if (tree.truncated) { - throw new SkillSyncError('GITHUB_TREE_TRUNCATED', 'GitHub tree response was truncated'); - } - if (!normalizedPath) { - return tree.tree; - } - return tree.tree.map((entry) => ({ - ...entry, - path: `${normalizedPath}/${normalizeRepoPath(entry.path)}`, - })); -} - +/** + * Merges the recursive listings of every configured path into one entry set. + * Configured paths may nest, so entries are deduplicated on their normalized + * repository path rather than concatenated. + */ async function fetchConfiguredTreeEntries(params: { - fetchFn: FetchFn; - token: string; + adapter: GitRepoAdapter; + commit: RepoCommit; source: SkillSyncGitHubSourceConfig; - rootTreeSha: string; assertNotCancelled: AssertNotCancelled; -}): Promise { - const entriesByPath = new Map(); +}): Promise { + const entriesByPath = new Map(); for (const repoPath of params.source.paths) { - const entries = await fetchTreeAtPath({ ...params, repoPath }); + const entries = await params.adapter.fetchTreeEntries(params.commit, { + pathPrefix: repoPath, + assertNotCancelled: params.assertNotCancelled, + }); for (const entry of entries) { const normalizedPath = normalizeRepoPath(entry.path); entriesByPath.set(normalizedPath, { ...entry, path: normalizedPath }); @@ -818,29 +625,6 @@ async function fetchConfiguredTreeEntries(params: { return [...entriesByPath.values()]; } -async function fetchBlob(params: { - fetchFn: FetchFn; - token: string; - source: SkillSyncGitHubSourceConfig; - sha: string; -}): Promise { - const owner = encodeURIComponent(params.source.owner); - const repo = encodeURIComponent(params.source.repo); - const sha = encodeURIComponent(params.sha); - const blob = await githubJson({ - fetchFn: params.fetchFn, - token: params.token, - pathname: `/repos/${owner}/${repo}/git/blobs/${sha}`, - }); - if (blob.encoding !== 'base64') { - throw new SkillSyncError( - 'GITHUB_UNSUPPORTED_BLOB', - `Unsupported GitHub blob encoding "${blob.encoding}"`, - ); - } - return Buffer.from(blob.content.replace(/\s/g, ''), 'base64'); -} - function isSkillRootWithinDiscoveryDepth( rootPath: string, basePath: string, @@ -860,12 +644,12 @@ function isSkillRootWithinDiscoveryDepth( } function discoverSkills( - tree: GitHubTreeEntry[], + tree: RepoTreeEntry[], source: SkillSyncGitHubSourceConfig, ): DiscoveredSkill[] { const basePaths = source.paths.map(normalizeRepoPath); const skillDiscoveryDepth = source.skillDiscoveryDepth ?? SKILL_SYNC_DEFAULT_DISCOVERY_DEPTH; - const skillMdByRoot = new Map(); + const skillMdByRoot = new Map(); for (const entry of tree) { if (entry.type !== 'blob') { continue; @@ -911,7 +695,7 @@ function discoverSkills( } function assertConfiguredPathsExist( - tree: GitHubTreeEntry[], + tree: RepoTreeEntry[], source: SkillSyncGitHubSourceConfig, ): void { for (const configuredPath of source.paths.map(normalizeRepoPath)) { @@ -1015,7 +799,7 @@ async function prepareRemoteSkill(params: { ref: source.ref, skillPath: discovered.rootPath, commitSha, - skillBlobSha: discovered.skillMd.sha, + skillBlobSha: discovered.skillMd.id, syncedAt: serializeDate(syncedAt), syncStatus: 'synced', }; @@ -1485,16 +1269,15 @@ async function deleteNameConflictingStaleSkill(params: { async function syncSkillFiles(params: { deps: GitHubSkillSyncDeps; - token: string; + adapter: GitRepoAdapter; + commit: RepoCommit; source: SkillSyncGitHubSourceConfig; skill: ISkill & { _id: Types.ObjectId }; discovered: DiscoveredSkill; - commitSha: string; - fetchFn: FetchFn; assertNotCancelled: AssertNotCancelled; journal?: SyncSkillFilesJournal; }): Promise { - const { deps, token, source, skill, discovered, commitSha, fetchFn, assertNotCancelled } = params; + const { deps, adapter, commit, source, skill, discovered, assertNotCancelled } = params; const journal = params.journal ?? { staleFiles: [], savedFiles: [] }; const remotePaths = new Set(); let syncedFileCount = 0; @@ -1511,10 +1294,10 @@ async function syncSkillFiles(params: { assertCumulativeGitHubFileSize(totalFileBytes); remotePaths.add(relativePath); const existing = await deps.getSkillFileByPath(skill._id, relativePath); - if (existing && getSourceMetadataString(existing, 'blobSha') === entry.sha) { + if (existing && getSourceMetadataString(existing, 'blobSha') === entry.id) { continue; } - const buffer = await fetchBlob({ fetchFn, token, source, sha: entry.sha }); + const buffer = await adapter.fetchFileContent(commit, entry); assertNotCancelled(); assertGitHubBufferSize(buffer, relativePath); const fileId = crypto.randomUUID(); @@ -1543,8 +1326,8 @@ async function syncSkillFiles(params: { provider: PROVIDER, sourceId: source.id, upstreamId: makeUpstreamId(source, discovered.rootPath), - commitSha, - blobSha: entry.sha, + commitSha: commit.id, + blobSha: entry.id, path: entry.path, }, mimeType, @@ -1646,6 +1429,7 @@ async function syncSource(params: { assertNotCancelled: AssertNotCancelled; }): Promise { const { deps, source, fetchFn, assertNotCancelled } = params; + const createAdapter = deps.createAdapter ?? createGitHubRepoAdapter; const startedAt = new Date(); const counts: SyncCounters = { syncedSkillCount: 0, @@ -1667,13 +1451,13 @@ async function syncSource(params: { getMissingCredentialMessage(source, allowServerCredentials), ); } - const commit = await fetchCommit({ fetchFn, token, source }); + const adapter = createAdapter({ source, token, fetchFn }); + const commit = await adapter.resolveCommit(); assertNotCancelled(); const treeEntries = await fetchConfiguredTreeEntries({ - fetchFn, - token, + adapter, + commit, source, - rootTreeSha: commit.commit.tree.sha, assertNotCancelled, }); assertConfiguredPathsExist(treeEntries, source); @@ -1777,12 +1561,7 @@ async function syncSource(params: { try { assertGitHubSkillPackageManifest(discovered); const skillMdPath = getSkillMdPath(discovered); - const skillMdBuffer = await fetchBlob({ - fetchFn, - token, - source, - sha: discovered.skillMd.sha, - }); + const skillMdBuffer = await adapter.fetchFileContent(commit, discovered.skillMd); assertNotCancelled(); assertGitHubBufferSize(skillMdBuffer, skillMdPath); const prepared = await prepareRemoteSkill({ @@ -1790,7 +1569,7 @@ async function syncSource(params: { source, discovered, skillMdContent: skillMdBuffer.toString('utf-8'), - commitSha: commit.sha, + commitSha: commit.id, syncedAt, }); preparedSkills.push({ discovered, prepared }); @@ -1907,12 +1686,11 @@ async function syncSource(params: { try { fileCounts = await syncSkillFiles({ deps, - token, + adapter, + commit, source, skill: effectivePrepared.existing, discovered, - commitSha: commit.sha, - fetchFn, assertNotCancelled, journal, }); @@ -1990,12 +1768,11 @@ async function syncSource(params: { try { const fileCounts = await syncSkillFiles({ deps, - token, + adapter, + commit, source, skill, discovered, - commitSha: commit.sha, - fetchFn, assertNotCancelled, }); await ensurePublicViewer(deps, skill._id); diff --git a/packages/api/src/skills/sync/index.ts b/packages/api/src/skills/sync/index.ts index 8cfe4a506b..5fe904921d 100644 --- a/packages/api/src/skills/sync/index.ts +++ b/packages/api/src/skills/sync/index.ts @@ -1,3 +1,4 @@ +export * from './adapters/types'; export * from './github'; export * from './orchestrator'; export * from './scheduler'; diff --git a/packages/api/src/skills/sync/path.ts b/packages/api/src/skills/sync/path.ts new file mode 100644 index 0000000000..e610880fce --- /dev/null +++ b/packages/api/src/skills/sync/path.ts @@ -0,0 +1,10 @@ +/** + * Repository paths arrive from provider APIs and admin config with inconsistent + * leading/trailing slashes, and `.` for the repository root. Normalizing both + * sides through this keeps configured paths, tree entry paths, and stored skill + * paths directly comparable. + */ +export function normalizeRepoPath(value: string): string { + const trimmed = value.trim().replace(/^\/+|\/+$/g, ''); + return trimmed === '.' ? '' : trimmed; +}