diff --git a/layers/registry/server/utils/repository-index.ts b/layers/registry/server/utils/repository-index.ts index a3389374..8cd49cd3 100644 --- a/layers/registry/server/utils/repository-index.ts +++ b/layers/registry/server/utils/repository-index.ts @@ -53,28 +53,48 @@ export async function findIndexedRepositorySkills( })) } +/** + * A request for an indexed Repository rechecks it at most this often. The + * scheduled sync never admits Skills added to a submitted Repository, so the + * owner's request is the path that brings them in. The wait keeps repeated + * requests from spending GitHub quota. + */ +export const RESUBMISSION_INTERVAL_SECONDS = 15 * 60 + +async function repositoryLastCheckedAt( + db: D1Database, + repository: Pick, +): Promise { + const row = await db.prepare( + `SELECT repo_meta_synced_at FROM repos WHERE owner = ? AND repo = ?`, + ).bind(repository.owner, repository.repo).first<{ repo_meta_synced_at: number | null }>() + return row?.repo_meta_synced_at ?? null +} + /** * Ask the registry to index one Repository. If it already holds Skills from - * the Repository, the answer lists them. If not, the answer is the queued job - * to poll. The skilld.dev search box and the public API both send requests - * through here, so the two cannot disagree on when a Repository counts as in. + * the Repository, the answer lists them, and a stale Repository is resubmitted + * in the background so Skills added since then can enter. If not, the answer + * is the queued job to poll. The skilld.dev search box and the public API both + * send requests through here, so the two cannot disagree on when a Repository + * counts as in. */ export async function submitRepositoryIndex( platform: Pick, repository: GitHubRepository, + now: number = Math.floor(Date.now() / 1000), ): Promise { + const env = platform.env as Cloudflare.Env & Record + const submission = { operation: 'submit', owner: repository.owner, repo: repository.repo } as const const skills = await findIndexedRepositorySkills(platform.db, repository) - if (skills.length) + if (skills.length) { + const lastCheckedAt = await repositoryLastCheckedAt(platform.db, repository) + if (lastCheckedAt === null || now - lastCheckedAt >= RESUBMISSION_INTERVAL_SECONDS) + await enqueueRegistryRepoJob(env, submission) return { _tag: 'indexed', repository, skills } + } - const queued = await enqueueRegistryRepoJob( - platform.env as Cloudflare.Env & Record, - { - operation: 'submit', - owner: repository.owner, - repo: repository.repo, - }, - ) + const queued = await enqueueRegistryRepoJob(env, submission) return { _tag: 'queued', repository, diff --git a/layers/registry/server/utils/sync-repo.ts b/layers/registry/server/utils/sync-repo.ts index 73ba47db..9ff2652a 100644 --- a/layers/registry/server/utils/sync-repo.ts +++ b/layers/registry/server/utils/sync-repo.ts @@ -786,6 +786,9 @@ export interface SyncRepoOptions { /** * Admit new skills requested through the public repository URL flow without * claiming that the GitHub owner is verified. Trust and SEO remain computed. + * A submission rereads the tree even when the repo cursor marks it unchanged, + * because a scheduled sync may have recorded that tree after declining to admit + * the Skills it added. */ submitted?: boolean /** @@ -908,7 +911,7 @@ export async function syncRepo( // GraphQL gave us the head tree SHA in the same request. If it matches // our cached value, the repo is unchanged and we skip the REST getTree // call entirely. Skill-less candidates deliberately bypass this cursor. - const unchanged = opts.continuation || opts.forceContent + const unchanged = opts.continuation || opts.forceContent || opts.submitted ? null : unchangedRepoStatus({ existing: existingRepo, hasAdmittedSkills, headTreeSha, repoPushedAt }) if (unchanged) @@ -939,7 +942,7 @@ export async function syncRepo( stats.reason = 'tree_changed_during_continuation' return stats } - if (!opts.continuation && !opts.forceContent && hasAdmittedSkills && existingRepo?.last_tree_sha && existingRepo.last_tree_sha === tree.sha) + if (!opts.continuation && !opts.forceContent && !opts.submitted && hasAdmittedSkills && existingRepo?.last_tree_sha && existingRepo.last_tree_sha === tree.sha) return markUnchanged('skipped-tree-sha') // The common unchanged-repo paths above need only the single repos row. diff --git a/test/unit/repository-index-server.test.ts b/test/unit/repository-index-server.test.ts index cea50e51..13e989fe 100644 --- a/test/unit/repository-index-server.test.ts +++ b/test/unit/repository-index-server.test.ts @@ -1,9 +1,16 @@ -import { describe, expect, it } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' import { readRepositoryJobState, repositoryIndexFailureMessage, + submitRepositoryIndex, } from '../../layers/registry/server/utils/repository-index' +const jobs = vi.hoisted(() => ({ + enqueueRegistryRepoJob: vi.fn(async () => ({ jobId: 'job-1', status: 'queued' as const })), +})) + +vi.mock('../../server/utils/registry-jobs-runtime', () => jobs) + function jobDatabase(row: Record | null): D1Database { return { prepare: () => ({ @@ -71,3 +78,39 @@ describe('repository indexing job state', () => { expect(repositoryIndexFailureMessage(reason)).toBe(message) }) }) + +describe('repository index requests', () => { + const now = 1_800_000_000 + const repository = { _tag: 'repository', owner: 'acme', repo: 'skills', url: 'https://github.com/acme/skills' } as const + + function indexedDatabase(lastCheckedAt: number | null): D1Database { + return { + prepare: (sql: string) => ({ + bind: () => ({ + all: async () => ({ + results: [{ name: 'one', slug: 'acme/one', path: 'skills/one/SKILL.md', description: 'One', likeCount: 0 }], + }), + first: async () => sql.includes('repo_meta_synced_at') ? { repo_meta_synced_at: lastCheckedAt } : null, + }), + }), + } as unknown as D1Database + } + + beforeEach(() => { + jobs.enqueueRegistryRepoJob.mockClear() + }) + + it('queues a resubmission when an indexed repository was last checked over fifteen minutes ago', async () => { + const result = await submitRepositoryIndex({ db: indexedDatabase(now - 16 * 60), env: {} } as never, repository, now) + + expect(result).toMatchObject({ _tag: 'indexed', skills: [{ name: 'one' }] }) + expect(jobs.enqueueRegistryRepoJob).toHaveBeenCalledWith({}, { operation: 'submit', owner: 'acme', repo: 'skills' }) + }) + + it('answers from stored Skills without queueing work when the repository was checked recently', async () => { + const result = await submitRepositoryIndex({ db: indexedDatabase(now - 60), env: {} } as never, repository, now) + + expect(result).toMatchObject({ _tag: 'indexed', skills: [{ name: 'one' }] }) + expect(jobs.enqueueRegistryRepoJob).not.toHaveBeenCalled() + }) +}) diff --git a/test/unit/sync-repo-correctness.test.ts b/test/unit/sync-repo-correctness.test.ts index 91573a1c..3bad9c92 100644 --- a/test/unit/sync-repo-correctness.test.ts +++ b/test/unit/sync-repo-correctness.test.ts @@ -378,6 +378,30 @@ describe('syncRepo content acknowledgement', () => { .toEqual({ broken_since: null, repo_skill_count: 0 }) }) + it('admits a Skill added to an indexed repository when its owner resubmits a tree the sync already saw', async () => { + // The scheduled sync recorded this tree but dropped the new Skill at admission. + insertRepo(sqlite, 'new-tree') + insertSkill(sqlite, 'one', 'one-sha') + setRendered(sqlite, 'one', 'skills/one/SKILL.md') + github.getTree.mockResolvedValue(tree([ + { path: 'skills/one/SKILL.md', sha: 'one-sha' }, + { path: 'skills/two/SKILL.md', sha: 'two-sha' }, + ])) + github.getBlobsBatch.mockResolvedValue({ + status: 200, + data: new Map([['skills/two/SKILL.md', rawSkill('Two')]]), + unreadable: new Set(), + rateLimit: null, + notModified: false, + }) + + const result = await syncRepo('acme', 'skills', {}, db, { submitted: true }) + + expect(result.status).toBe('indexed') + expect(sqlite.prepare(`SELECT name FROM skills WHERE source_resolved = 1 ORDER BY name`).pluck().all()) + .toEqual(['one', 'two']) + }) + it('indexes a Skill added back to a repository whose rows were all retired', async () => { insertRepo(sqlite, 'fixture-tree') insertSkill(sqlite, 'seo-audit', 'fixture-sha')