diff --git a/VISION.md b/VISION.md index 1a9bf3ca..68294f13 100644 --- a/VISION.md +++ b/VISION.md @@ -95,7 +95,7 @@ The filter for skilld-scoped work, precise enough to hold a proposal next to it ### 1. Provenance is the quality signal -A Skill is trustworthy because you can see who wrote it and read it before you run it. Every surface that shows a Skill names the author. The Skill page links the exact source file in its repository; other surfaces link the Skill page, which is where a visitor inspects a Skill. Registry identity is frozen at admission (ADR-0002) so provenance URLs never rot. Machine-generated metadata is labeled as such and never dressed as authorship. +A Skill is trustworthy because you can see who wrote it and read it before you run it. Every surface that shows a Skill names the author. The Skill page links the exact source file in its repository; other surfaces link the Skill page, which is where a visitor inspects a Skill. When GitHub moves a Repository, its Skills move to the new name and every old URL answers 301 (ADR-0015), so provenance URLs never rot. Machine-generated metadata is labeled as such and never dressed as authorship. **How to apply:** no Skill card, search result, or digest entry ships without its author and a link to its Skill page or its source. A layout that drops provenance to save space fails review. Trust signals (official repository, owner-verified) are earned facts, never editorial favors. diff --git a/docs/adr/0015-follow-moved-repositories.md b/docs/adr/0015-follow-moved-repositories.md new file mode 100644 index 00000000..e2bd4c71 --- /dev/null +++ b/docs/adr/0015-follow-moved-repositories.md @@ -0,0 +1,75 @@ +# ADR-0015: Follow moved Repositories + +**Status:** Accepted +**Date:** 2026-10-07 + +Replaces [ADR-0002](0002-freeze-registry-repository-identity.md). + +## Context + +ADR-0002 froze the registry identity at admission. A renamed or transferred +Repository kept its old `owner/repo` in every table and URL, and +`repos.source_owner` and `repos.source_repo` carried the new name for [GitHub](https://github.com) +reads. + +That left pages and run commands on names GitHub no longer serves. On +2026-10-07 the run sweep found 229 Skills in 13 Repositories failing on GitHub's +301, `facebook/react` among them. Production held 503 Repositories under a name +that GitHub had moved. In two cases the new name was already a separate +registry Repository with the same Skills: `hyf0/vue-skills` and +`vuejs-ai/skills`, and `shadcn/ui` and `shadcn-ui/ui`. Those were duplicate +indexable pages. + +## Decision + +The registry follows GitHub. When sync reads a Repository and GitHub answers +with another name, every row moves to that name in one D1 batch. + +- GitHub's numeric Repository ID survives renames and transfers. Sync stores it + in `repos.repository_id` and records it on the alias. +- The old name stays as a `repo_aliases` row. `/gh//`{lang="html"} and + every path under it answer 301 to the same path under the new name, in one + hop. A later move rewrites older aliases to the newest name. A move back + deletes the alias. +- When the registry already holds the new name, the rows merge into it. A clash + keeps the row under the new name. +- A root Skill takes the Repository name, so a rename renames it. The alias + keeps both names, so its old page redirects too. +- `skilld run //`{lang="html"} keeps working. The registry + answers the old name with the new identity, and delivery follows GitHub's + 301 to the Repository ID. +- Discovery evidence, skills.sh observations, and sync checkpoints keep the + name they recorded. + +`layers/registry/server/utils/repository-move.ts` holds the move. +`scripts/move-repositories.ts` runs the same statements as a one-off backfill. + +## Consequences + +Pages, sitemaps, and run commands show the name GitHub serves. The 301 carries +the old URL's links to the new one. The move adds no indexable page: each Skill +keeps its admission, and a merge removes a duplicate. VISION principle 2 holds. + +Every moved Skill is queued in `skill_dirty`, so trust and indexability are +recomputed for the new identity. The `embedding` marker stays under the old +name. The vector ID hashes the name, so the nightly prune deletes the old vector +and the next generation embeds the new name. + +Static lists keyed by name, such as `official-repos.ts` and the track pins in +`clusters.ts`, must name the new Repository. An official Repository lists both +names until no row carries the old one. + +Moves come from GitHub, not from a person. A Repository whose old name GitHub +gives to another Repository becomes live again once the registry admits it, and +its alias stops redirecting. + +Cull path: there is none while old URLs receive traffic. Dropping +`repo_aliases` ends every redirect. + +## Rejected alternatives + +Keeping ADR-0002 meant run commands and pages named Repositories GitHub no +longer serves, and duplicate Repositories stayed as duplicate pages. + +A redirect without a move would answer every page view with a lookup and keep +two names for one Repository in every table. diff --git a/docs/arch/README.md b/docs/arch/README.md index 6d9ec8c4..5984ea66 100644 --- a/docs/arch/README.md +++ b/docs/arch/README.md @@ -83,4 +83,6 @@ See [ADR-0007](../adr/0007-contract-in-the-cli-repository.md). | Framework page | `/frameworks/[name]` | | Comparison | `/compare/` | +A Repository that GitHub renamed or transferred moves to its new name, and every `/gh` URL under the old name answers 301 to the same path under the new one ([ADR-0015](../adr/0015-follow-moved-repositories.md)). + The legacy `/people/[handle]` and `/people/[handle]/collections/[slug]` URLs are 410 Gone (with a 301 special-case for `/people/harlanzw.com → /@harlanzw`). diff --git a/layers/registry/server/data/clusters.ts b/layers/registry/server/data/clusters.ts index 74f6cc71..17239c2f 100644 --- a/layers/registry/server/data/clusters.ts +++ b/layers/registry/server/data/clusters.ts @@ -153,7 +153,7 @@ export const CLUSTERS: Cluster[] = [ 'ibelick/ui-skills/fixing-accessibility', 'ibelick/ui-skills/baseline-ui', 'nutlope/hallmark/hallmark', - 'shadcn/ui/shadcn', + 'shadcn-ui/ui/shadcn', ], }, { @@ -200,7 +200,7 @@ export const CLUSTERS: Cluster[] = [ pinnedExamples: [ 'dmmulroy/anti-slop/install-anti-slop', 'cursor/plugins/thermo-nuclear-code-quality-review', - 'brianlovin/claude-config/deslop', + 'brianlovin/agent-config/deslop', 'cursor/plugins/deslop', 'davila7/claude-code-templates/deslop', // Registry identity follows `docs/SKILL.md`; its frontmatter name @@ -264,7 +264,7 @@ export const CLUSTERS: Cluster[] = [ 'pbakaus/agent-reviews/resolve-agent-reviews', 'obra/superpowers/requesting-code-review', 'obra/superpowers/receiving-code-review', - 'brianlovin/claude-config/fix-sentry-issues', + 'brianlovin/agent-config/fix-sentry-issues', 'dimillian/skills/review-and-simplify-changes', 'github/awesome-copilot/refactor', ], @@ -594,7 +594,7 @@ export const FRAMEWORK_PINNED_SKILLS: string[] = [ // /frameworks/react and /frameworks/nextjs 'vercel-labs/agent-skills/react-best-practices', 'vercel-labs/agent-skills/composition-patterns', - 'vercel-labs/vercel-plugin/nextjs', + 'vercel/vercel-plugin/nextjs', 'deckardger/tanstack-agent-skills/tanstack-query', 'vercel/next.js/next-cache-components-optimizer', ] diff --git a/layers/registry/server/data/official-repos.ts b/layers/registry/server/data/official-repos.ts index 34fa5612..4bd683d9 100644 --- a/layers/registry/server/data/official-repos.ts +++ b/layers/registry/server/data/official-repos.ts @@ -18,6 +18,10 @@ export interface OfficialRepo { * Org entries seeded from skills.sh/official on 2026-03-30. User entries * seeded from registry on 2026-04-25, then maintained as a reviewed list. * Maintained manually. + * + * A Repository GitHub moved keeps its old name here beside the new one, so + * its Skills stay official on both sides of the registry move (ADR-0015). + * Delete an old name once no registry row carries it. */ export const officialRepos: OfficialRepo[] = [ { owner: 'anthropics', repo: 'skills', kind: 'org' }, @@ -29,7 +33,9 @@ export const officialRepos: OfficialRepo[] = [ { owner: 'base', repo: 'skills', kind: 'org' }, { owner: 'better-auth', repo: 'skills', kind: 'org' }, { owner: 'bitwarden', repo: 'ai-plugins', kind: 'org' }, + // Old name. ADR-0015 moves its rows to the entry below. { owner: 'box', repo: 'box-for-ai', kind: 'org' }, + { owner: 'box', repo: 'skills', kind: 'org' }, { owner: 'brave', repo: 'brave-search-skills', kind: 'org' }, { owner: 'browser-use', repo: 'browser-use', kind: 'org' }, { owner: 'browserbase', repo: 'skills', kind: 'org' }, @@ -46,11 +52,15 @@ export const officialRepos: OfficialRepo[] = [ { owner: 'elevenlabs', repo: 'skills', kind: 'org' }, { owner: 'encoredev', repo: 'skills', kind: 'org' }, { owner: 'expo', repo: 'skills', kind: 'org' }, + // Old name. ADR-0015 moves its rows to the entry below. { owner: 'facebook', repo: 'react', kind: 'org' }, + { owner: 'react', repo: 'react', kind: 'org' }, { owner: 'figma', repo: 'mcp-server-guide', kind: 'org' }, { owner: 'firebase', repo: 'agent-skills', kind: 'org' }, { owner: 'firecrawl', repo: 'cli', kind: 'org' }, + // Old name. ADR-0015 moves its rows to the entry below. { owner: 'flutter', repo: 'skills', kind: 'org' }, + { owner: 'flutter', repo: 'agent-plugins', kind: 'org' }, { owner: 'getsentry', repo: 'skills', kind: 'org' }, { owner: 'github', repo: 'awesome-copilot', kind: 'org' }, { owner: 'google-gemini', repo: 'gemini-skills', kind: 'org' }, @@ -61,7 +71,9 @@ export const officialRepos: OfficialRepo[] = [ { owner: 'kotlin', repo: 'kotlin-agent-skills', kind: 'org' }, { owner: 'langchain-ai', repo: 'langchain-skills', kind: 'org' }, { owner: 'langfuse', repo: 'skills', kind: 'org' }, + // Old name. ADR-0015 moves its rows to the entry below. { owner: 'launchdarkly', repo: 'agent-skills', kind: 'org' }, + { owner: 'launchdarkly', repo: 'ai-tooling', kind: 'org' }, { owner: 'livekit', repo: 'agent-skills', kind: 'org' }, { owner: 'makenotion', repo: 'claude-code-notion-plugin', kind: 'org' }, { owner: 'mapbox', repo: 'mapbox-agent-skills', kind: 'org' }, @@ -90,7 +102,9 @@ export const officialRepos: OfficialRepo[] = [ { owner: 'streamlit', repo: 'agent-skills', kind: 'org' }, { owner: 'stripe', repo: 'ai', kind: 'org' }, { owner: 'supabase', repo: 'agent-skills', kind: 'org' }, + // Old name. ADR-0015 moves its rows to the entry below. { owner: 'sveltejs', repo: 'mcp', kind: 'org' }, + { owner: 'sveltejs', repo: 'ai-tools', kind: 'org' }, { owner: 'vuejs-ai', repo: 'skills', kind: 'org' }, { owner: 'vueuse', repo: 'skills', kind: 'org' }, { owner: 'tavily-ai', repo: 'skills', kind: 'org' }, @@ -101,7 +115,9 @@ export const officialRepos: OfficialRepo[] = [ { owner: 'vercel', repo: 'ai', kind: 'org' }, { owner: 'vercel', repo: 'next.js', kind: 'org' }, { owner: 'vercel-labs', repo: 'agent-skills', kind: 'org' }, + // Old name. ADR-0015 moves its rows to the entry below. { owner: 'vercel-labs', repo: 'vercel-plugin', kind: 'org' }, + { owner: 'vercel', repo: 'vercel-plugin', kind: 'org' }, { owner: 'vercel-labs', repo: 'agent-browser', kind: 'org' }, { owner: 'webflow', repo: 'webflow-skills', kind: 'org' }, { owner: 'wix', repo: 'skills', kind: 'org' }, @@ -118,9 +134,12 @@ export const officialRepos: OfficialRepo[] = [ { owner: 'addyosmani', repo: 'web-quality-skills', kind: 'user' }, { owner: 'mattpocock', repo: 'skills', kind: 'user' }, { owner: 'ibelick', repo: 'ui-skills', kind: 'user' }, + // Old name. ADR-0015 moves its rows to the entry below. { owner: 'brianlovin', repo: 'claude-config', kind: 'user' }, + { owner: 'brianlovin', repo: 'agent-config', kind: 'user' }, { owner: 'dimillian', repo: 'skills', kind: 'user' }, { owner: 'onmax', repo: 'nuxt-skills', kind: 'user' }, + // Old name. ADR-0015 merges its rows into vuejs-ai/skills above. { owner: 'hyf0', repo: 'vue-skills', kind: 'user' }, { owner: 'emilkowalski', repo: 'skills', kind: 'user' }, { owner: 'nutlope', repo: 'hallmark', kind: 'user' }, diff --git a/layers/registry/server/middleware/gh-moved-repository-redirect.ts b/layers/registry/server/middleware/gh-moved-repository-redirect.ts new file mode 100644 index 00000000..87135c8e --- /dev/null +++ b/layers/registry/server/middleware/gh-moved-repository-redirect.ts @@ -0,0 +1,29 @@ +import { + indexRepositoryAliases, + loadRepositoryAliases, + repositoryPathSegments, + resolveMovedRepositoryRoute, +} from '../utils/repository-aliases' + +/** + * Answer every URL under a moved Repository with a 301 to its new name, so + * the old URLs pass their link equity on (ADR-0015). Paths without an owner + * and a Repository segment skip the cache read. + */ +export default defineEventHandler(async (event) => { + const url = getRequestURL(event) + if (!repositoryPathSegments(url.pathname)) + return + + const db = event.context.platform?.db + if (!db) + return + + const aliases = await loadRepositoryAliases(useStorage('cache'), db) + if (!aliases?.length) + return + + const decision = resolveMovedRepositoryRoute(url.pathname, url.search, indexRepositoryAliases(aliases)) + if (decision._tag === 'redirect') + return sendRedirect(event, decision.location, 301) +}) diff --git a/layers/registry/server/presenters/repository-v1.ts b/layers/registry/server/presenters/repository-v1.ts index ebf2e87a..2bf16b4a 100644 --- a/layers/registry/server/presenters/repository-v1.ts +++ b/layers/registry/server/presenters/repository-v1.ts @@ -28,9 +28,8 @@ function byLastChange(left: ProfileSkill, right: ProfileSkill): number { /** * One Repository profile. * - * `source` is where the Repository lives on GitHub now. The registry keeps the - * identity it first indexed (ADR-0002), and the two differ after a rename or - * a transfer. + * `source` is where the Repository lives on GitHub now. After a rename or a + * transfer the two differ until sync moves the registry rows (ADR-0015). */ export function presentRepository( profile: LegacyOwnerProfile, diff --git a/layers/registry/server/utils/embedding-parity.ts b/layers/registry/server/utils/embedding-parity.ts index 68663338..ddef44ae 100644 --- a/layers/registry/server/utils/embedding-parity.ts +++ b/layers/registry/server/utils/embedding-parity.ts @@ -328,6 +328,10 @@ export async function runEmbeddingParityCheck( * hashes owner/repo/name with no sha in it, so any skill still carrying an * `embedding` marker while failing the eligibility predicate is exactly one * strandable vector, addressable by id without reading the index at all. + * + * A marker with no Skill row at all is one too. A Repository move leaves the + * marker under the old name on purpose (`repository-move.ts`), because the + * vector ID still hashes that name. */ const PRUNE_CANDIDATE_SQL = ` SELECT s.owner, s.repo, s.name @@ -345,7 +349,15 @@ const PRUNE_CANDIDATE_SQL = ` AND s.rendered_status = 'ok' AND s.seo_indexable = 1 ) - ORDER BY s.owner ASC, s.repo ASC, s.name ASC + UNION ALL + SELECT marker.owner, marker.repo, marker.name + FROM skill_generated marker + WHERE marker.kind = 'embedding' + AND NOT EXISTS ( + SELECT 1 FROM skills s + WHERE s.owner = marker.owner AND s.repo = marker.repo AND s.name = marker.name + ) + ORDER BY owner ASC, repo ASC, name ASC ` /** diff --git a/layers/registry/server/utils/github-client.ts b/layers/registry/server/utils/github-client.ts index 16b5a8a4..40845d2f 100644 --- a/layers/registry/server/utils/github-client.ts +++ b/layers/registry/server/utils/github-client.ts @@ -234,6 +234,7 @@ export async function getRepo( } interface RepoSummaryGql { + databaseId: number | null name: string nameWithOwner: string url: string @@ -251,16 +252,28 @@ interface RepoSummaryGql { } | null } -const REPO_SUMMARY_FIELDS = `name nameWithOwner url owner{login} +const REPO_SUMMARY_FIELDS = `databaseId name nameWithOwner url owner{login} description stargazerCount forkCount pushedAt createdAt isArchived isFork defaultBranchRef{name target{... on Commit{oid tree{oid}}}}` export interface RepoSummary { meta: RepoMeta + /** + * GitHub's numeric Repository ID. It survives every rename and transfer, so + * it confirms that an old name and a new name are one Repository. + */ + repositoryId: number headTreeSha: string | null } -function repoSummaryFromGql(r: RepoSummaryGql): RepoSummary { +/** + * Parse one GraphQL Repository into a summary, or null without a Repository + * ID. The schema allows a null ID, and a Repository without one cannot be + * followed across a move. + */ +function repoSummaryFromGql(r: RepoSummaryGql): RepoSummary | null { + if (typeof r.databaseId !== 'number') + return null const branch = r.defaultBranchRef?.name || 'main' const meta: RepoMeta = { name: r.name, @@ -276,7 +289,7 @@ function repoSummaryFromGql(r: RepoSummaryGql): RepoSummary { archived: r.isArchived, fork: r.isFork, } - return { meta, headTreeSha: r.defaultBranchRef?.target?.tree.oid ?? null } + return { meta, repositoryId: r.databaseId, headTreeSha: r.defaultBranchRef?.target?.tree.oid ?? null } } /** @@ -311,7 +324,11 @@ export async function getRepoSummary( const r = out.data?.repository if (!r) return { status: 404, data: null, rateLimit: out.rateLimit, notModified: false } - return { status: 200, data: repoSummaryFromGql(r), rateLimit: out.rateLimit, notModified: false } + // A Repository without an ID fails like any other malformed answer. + const summary = repoSummaryFromGql(r) + return summary + ? { status: 200, data: summary, rateLimit: out.rateLimit, notModified: false } + : { status: 502, data: null, rateLimit: out.rateLimit, notModified: false } } /** @@ -377,6 +394,8 @@ export async function getRepoSummariesBatch( return { _tag: 'failed', status: 502, rateLimit, requests: sent } for (let i = 0; i < batch.length; i++) { const r = out.data?.[`r${i}`] + // A summary without a usable Repository ID cannot be followed across a + // move, so it reads as no summary and the per-repository sync owns it. summaries.push(r ? repoSummaryFromGql(r) : null) } } diff --git a/layers/registry/server/utils/repository-aliases.ts b/layers/registry/server/utils/repository-aliases.ts new file mode 100644 index 00000000..8c3c08b8 --- /dev/null +++ b/layers/registry/server/utils/repository-aliases.ts @@ -0,0 +1,161 @@ +/// + +import type { ReadThroughCache } from '#shared/server/cache' +import type { RepositoryName } from './repository-move' +import { readCache, writeCache } from '#shared/server/cache' + +/** An old Repository name and the registry identity it moved to. */ +export interface RepositoryAlias { + owner: string + repo: string + targetOwner: string + targetRepo: string + /** The root Skill name under the old name, when the move renamed it. */ + rootSkill: string | null + targetRootSkill: string | null +} + +interface RepositoryAliasRow { + owner: string + repo: string + target_owner: string + target_repo: string + root_skill: string | null + target_root_skill: string | null +} + +export type MovedRepositoryRoute + = | { _tag: 'pass' } + | { _tag: 'redirect', location: string } + +export const REPOSITORY_ALIASES_CACHE_KEY = 'repos:aliases:v1' +/** Seconds. A new alias waits at most this long before its URLs redirect. */ +export const REPOSITORY_ALIASES_CACHE_TTL = 15 * 60 + +// A registry row under an old name means GitHub gave that name to another +// Repository and the registry admitted it, so the name is live again. +const ALIAS_COLUMNS = `a.owner, a.repo, a.target_owner, a.target_repo, a.root_skill, a.target_root_skill` +const LIVE_NAME = `EXISTS (SELECT 1 FROM repos r WHERE a.owner = r.owner AND a.repo = r.repo)` + +export const REPOSITORY_ALIASES_SQL = ` + SELECT ${ALIAS_COLUMNS} + FROM repo_aliases a + WHERE NOT ${LIVE_NAME} +` + +function toAlias(row: RepositoryAliasRow): RepositoryAlias { + return { + owner: row.owner, + repo: row.repo, + targetOwner: row.target_owner, + targetRepo: row.target_repo, + rootSkill: row.root_skill, + targetRootSkill: row.target_root_skill, + } +} + +/** The alias an old name answers with, matched without case, or null. */ +export async function findRepositoryAlias(db: D1Database, name: RepositoryName): Promise { + const row = await db + .prepare(`SELECT ${ALIAS_COLUMNS} FROM repo_aliases a WHERE a.owner = ?1 AND a.repo = ?2 AND NOT ${LIVE_NAME}`) + .bind(name.owner, name.repo) + .first() + return row ? toAlias(row) : null +} + +export async function selectRepositoryAliases(db: D1Database): Promise { + const rows = await db.prepare(REPOSITORY_ALIASES_SQL).bind().all() + return (rows.results ?? []).map(toAlias) +} + +/** + * Every live alias for this request, or null when neither store answers. + * + * This runs on every `/gh` request, so the set comes from the cache. A + * failure costs one request the redirect: the old URL renders what it did + * before the move. Failing the request would cost the visitor the page. The + * wide event carries the reason, so an outage shows up as a spike. + */ +export async function loadRepositoryAliases( + cache: ReadThroughCache, + db: D1Database, +): Promise { + const cached = await readCache(cache, REPOSITORY_ALIASES_CACHE_KEY) + if (Array.isArray(cached)) + return cached + + try { + const aliases = await selectRepositoryAliases(db) + await writeCache(cache, REPOSITORY_ALIASES_CACHE_KEY, aliases, { ttl: REPOSITORY_ALIASES_CACHE_TTL }) + return aliases + } + catch (error) { + emitOperationalEvent(createWideEvent({ + operation: 'repository-aliases', + outcome: 'failed', + reason: error instanceof Error ? error.message : String(error), + })) + return null + } +} + +const aliasKey = (owner: string, repo: string) => `${owner}/${repo}`.toLowerCase() + +export function indexRepositoryAliases(aliases: readonly RepositoryAlias[]): ReadonlyMap { + return new Map(aliases.map(alias => [aliasKey(alias.owner, alias.repo), alias])) +} + +/** + * The owner and Repository segments of a `/gh` path, when it has both. Only + * these paths can sit under a moved Repository. + */ +export function repositoryPathSegments(pathname: string): { owner: string, repo: string, rest: string[] } | null { + if (!pathname.startsWith('/gh/')) + return null + const [owner, repo, ...rest] = pathname.slice('/gh/'.length).split('/') + return owner && repo ? { owner, repo, rest } : null +} + +/** + * Send a URL under a moved Repository to the same place under its new name, + * in one 301. The Repository hub, every Skill page, file deep links, and the + * agent `.md` copies all move. The query string stays. + */ +export function resolveMovedRepositoryRoute( + pathname: string, + search: string, + aliases: ReadonlyMap, +): MovedRepositoryRoute { + const path = repositoryPathSegments(pathname) + if (!path) + return { _tag: 'pass' } + + const exact = aliases.get(aliasKey(path.owner, path.repo)) + // `/gh//.md` is the agent copy of the Repository hub. + const markdown = !exact && path.rest.length === 0 && path.repo.toLowerCase().endsWith('.md') + ? aliases.get(aliasKey(path.owner, path.repo.slice(0, -'.md'.length))) + : undefined + const alias = exact ?? markdown + if (!alias) + return { _tag: 'pass' } + + const repo = markdown ? `${alias.targetRepo}.md` : alias.targetRepo + const [skill, ...below] = path.rest + const rest = skill === undefined ? [] : [movedSkillSegment(skill, alias), ...below] + return { + _tag: 'redirect', + location: `/gh/${[alias.targetOwner, repo, ...rest].join('/')}${search}`, + } +} + +/** A root Skill takes the Repository name, so its segment moves with it. */ +function movedSkillSegment(segment: string, alias: RepositoryAlias): string { + if (!alias.rootSkill || !alias.targetRootSkill) + return segment + const lower = segment.toLowerCase() + if (lower === alias.rootSkill) + return alias.targetRootSkill + if (lower === `${alias.rootSkill}.md`) + return `${alias.targetRootSkill}.md` + return segment +} diff --git a/layers/registry/server/utils/repository-move.ts b/layers/registry/server/utils/repository-move.ts new file mode 100644 index 00000000..d5dcf8ce --- /dev/null +++ b/layers/registry/server/utils/repository-move.ts @@ -0,0 +1,317 @@ +/// + +import { registrySkillName } from './skill-frontmatter' + +export interface RepositoryName { + owner: string + repo: string +} + +/** + * One Repository move: every registry row under `from` moves to `to` in one + * D1 batch, and `from` stays as an alias that answers its URLs with a 301. + * ADR-0015 says why the registry follows GitHub here. + */ +export interface RepositoryMove { + /** The registry identity the rows carry now, exactly as stored. */ + from: RepositoryName + /** The registry identity the rows carry after the move. */ + to: RepositoryName + /** GitHub's own spelling of the new name, kept as the source identity. */ + source: RepositoryName + /** GitHub's numeric Repository ID, which survives renames and transfers. */ + repositoryId: number + /** Unix seconds. */ + movedAt: number +} + +export type MoveSqlValue = string | number + +export interface MoveStatement { + sql: string + params: MoveSqlValue[] +} + +/** GitHub compares owner and Repository names without case. */ +export function sameRepositoryName(left: RepositoryName, right: RepositoryName): boolean { + return left.owner.toLowerCase() === right.owner.toLowerCase() + && left.repo.toLowerCase() === right.repo.toLowerCase() +} + +/** + * The registry identity for a Repository GitHub now serves as `current`. + * + * The registry stores names in lowercase. When it already holds the new name, + * in any case, the rows merge into that row and keep its spelling. + */ +export function movedRegistryName(current: RepositoryName, held: RepositoryName | null): RepositoryName { + return held ?? { owner: current.owner.toLowerCase(), repo: current.repo.toLowerCase() } +} + +/** The row the registry already holds under a name, with its GitHub identity. */ +export interface HeldRepositoryName extends RepositoryName { + /** The GitHub Repository ID the held row belongs to, when the registry knows it. */ + repositoryId: number | null +} + +/** + * What a move into the registry may do. A held name merges only when the + * held row is not provably another Repository's: a stale row GitHub no + * longer serves can occupy a name, and merging into it would absorb a + * different Repository's rows. The move is refused for review instead. + */ +export type RepositoryMovePlan + = | { _tag: 'merge', to: RepositoryName } + | { _tag: 'held', heldBy: number } + +export function planRepositoryMove( + current: RepositoryName, + held: HeldRepositoryName | null, + repositoryId: number, +): RepositoryMovePlan { + if (held?.repositoryId != null && held.repositoryId !== repositoryId) + return { _tag: 'held', heldBy: held.repositoryId } + return { _tag: 'merge', to: movedRegistryName(current, held) } +} + +/** + * The registry row that already holds a name, matched without case, or null. + */ +export async function findHeldRepositoryName(db: D1Database, name: RepositoryName): Promise { + const row = await db + .prepare(` + SELECT owner, repo, repository_id + FROM repos + WHERE owner = ?1 COLLATE NOCASE AND repo = ?2 COLLATE NOCASE + ORDER BY (owner = lower(?1) AND repo = lower(?2)) DESC + LIMIT 1`) + .bind(name.owner, name.repo) + .first<{ owner: string, repo: string, repository_id: number | null }>() + return row ? { owner: row.owner, repo: row.repo, repositoryId: row.repository_id } : null +} + +/** Apply one move as a single D1 batch, so it lands whole or not at all. */ +export async function moveRepository(db: D1Database, move: RepositoryMove): Promise { + await db.batch(repositoryMoveStatements(move).map(statement => db.prepare(statement.sql).bind(...statement.params))) +} + +// The registry name of the root Skill under the old name. A root Skill takes +// the Repository name, so a renamed Repository renames it. +const ROOT_SKILL = `(SELECT name FROM skills WHERE owner = :from_owner AND repo = :from_repo AND rendered_skill_path = 'SKILL.md' LIMIT 1)` +const movedSkillName = (column: string) => `CASE WHEN ${column} = ${ROOT_SKILL} THEN :root_name ELSE ${column} END` +const movedRowName = (row = '') => `CASE WHEN ${row}rendered_skill_path = 'SKILL.md' THEN :root_name ELSE ${row}name END` + +interface KeyedTable { + table: string + repo?: string + /** The Skill name column, for a table that holds Skills. */ + name?: string + /** Rows the move leaves under the old name. */ + keep?: string +} + +const skillTable = (table: string, options: Omit = {}): KeyedTable => ({ table, name: 'name', ...options }) + +/** + * Tables that hold one row per Skill. A merge keeps the row already under the + * new name, so `UPDATE OR IGNORE` skips a clash and the delete clears it. + * + * The `embedding` marker stays under the old name on purpose. The vector ID + * hashes owner, Repository, and name, so the old vector is an orphan the + * nightly prune finds through that marker, and the missing marker under the + * new name queues a fresh embedding. + */ +const SKILL_TABLES: KeyedTable[] = [ + skillTable('skill_likes'), + skillTable('skill_revisions'), + skillTable('skill_generated', { keep: `kind = 'embedding'` }), + skillTable('skill_dirty'), + skillTable('skill_trending_admissions'), + skillTable('skill_trending_awards'), + skillTable('weekly_skill_sends'), + skillTable('supported_skills'), + skillTable('activity'), + skillTable('collection_skills_v2'), + skillTable('artifact_run_checks', { repo: 'repository' }), + skillTable('x_post_skills', { name: 'slug' }), +] + +/** + * Tables that hold one row per Repository. + * + * Discovery rows and skills.sh observations stay: they record the name a + * source used. `repo_sync_progress` stays too, because a running sync job + * reads its checkpoint by the name it was queued under. + */ +const REPOSITORY_TABLES: KeyedTable[] = [ + { table: 'repo_star_observations' }, + { table: 'repo_star_surges' }, + { table: 'repo_kind_overrides' }, + { table: 'repo_trust_overrides' }, + { table: 'skill_repo_eligibility' }, + { table: 'skill_repo_focus' }, + { table: 'skill_repo_review_sync_outbox' }, + { table: 'supported_repos' }, + { table: 'skillgen_repositories' }, + { table: 'user_starred_repos' }, + { table: 'skill_subscriptions' }, + { table: 'x_post_repos' }, +] + +// Every column but the key, so the copy under the new name keeps them all. +const REPOS_COLUMNS = [ + 'default_branch', + 'stars', + 'forks', + 'pushed_at', + 'repo_created_at', + 'repo_meta_synced_at', + 'last_tree_sha', + 'repo_kind', + 'repo_kind_source', + 'repo_skill_count', + 'broken_since', + 'source_owner', + 'source_repo', + 'description', + 'tree_truncated_at', + 'repository_id', +].join(', ') + +function moveRows({ table, repo = 'repo', name, keep }: KeyedTable): string[] { + const where = `owner = :from_owner AND ${repo} = :from_repo${keep ? ` AND NOT (${keep})` : ''}` + const rename = name === undefined ? '' : `, ${name} = ${movedSkillName(name)}` + return [ + `UPDATE OR IGNORE ${table} SET owner = :to_owner, ${repo} = :to_repo${rename} WHERE ${where}`, + `DELETE FROM ${table} WHERE ${where}`, + ] +} + +const MOVE_SQL: string[] = [ + // The row under the new name has to exist before any row that references it. + `INSERT OR IGNORE INTO repos (owner, repo, ${REPOS_COLUMNS}) + SELECT :to_owner, :to_repo, ${REPOS_COLUMNS} FROM repos WHERE owner = :from_owner AND repo = :from_repo`, + `UPDATE repos SET repository_id = :repository_id, source_owner = :source_owner, source_repo = :source_repo + WHERE owner = :to_owner AND repo = :to_repo`, + + // The old name becomes an alias. A repeat of the same move keeps the first + // date and the root Skill it saw. + `INSERT INTO repo_aliases (owner, repo, repository_id, target_owner, target_repo, root_skill, target_root_skill, moved_at) + SELECT :from_owner, :from_repo, :repository_id, :to_owner, :to_repo, root.name, + CASE WHEN root.name IS NULL THEN NULL ELSE :root_name END, :moved_at + FROM (SELECT ${ROOT_SKILL} AS name) AS root + WHERE true + ON CONFLICT (owner, repo) DO UPDATE SET + repository_id = excluded.repository_id, + target_owner = excluded.target_owner, + target_repo = excluded.target_repo, + root_skill = COALESCE(excluded.root_skill, repo_aliases.root_skill), + target_root_skill = CASE WHEN COALESCE(excluded.root_skill, repo_aliases.root_skill) IS NULL THEN NULL ELSE :root_name END, + moved_at = CASE + WHEN repo_aliases.target_owner = excluded.target_owner AND repo_aliases.target_repo = excluded.target_repo + THEN repo_aliases.moved_at + ELSE excluded.moved_at + END`, + // An older alias points straight at the new name, so no redirect chains. + `UPDATE repo_aliases + SET target_owner = :to_owner, target_repo = :to_repo, + target_root_skill = CASE WHEN target_root_skill IS NULL THEN NULL ELSE :root_name END + WHERE target_owner = :from_owner AND target_repo = :from_repo`, + // The new name is live, so it is nobody's alias. This also ends a move back. + `DELETE FROM repo_aliases WHERE owner = :to_owner AND repo = :to_repo`, + + // Social posts key a Skill by its `owner/name` slug. + `UPDATE OR IGNORE skill_social_posts + SET skill_slug = ( + SELECT :to_owner || '/' || ${movedRowName('s.')} + FROM skills s + WHERE s.owner = :from_owner AND s.repo = :from_repo AND s.slug = skill_social_posts.skill_slug + LIMIT 1) + WHERE skill_slug IN (SELECT slug FROM skills WHERE owner = :from_owner AND repo = :from_repo)`, + `DELETE FROM skill_social_posts + WHERE skill_slug IN ( + SELECT slug FROM skills + WHERE owner = :from_owner AND repo = :from_repo AND slug <> :to_owner || '/' || ${movedRowName()})`, + + // A collection that already lists the Skill under the new name keeps one entry. + `DELETE FROM collection_skills_v2 + WHERE owner = :from_owner AND repo = :from_repo + AND EXISTS ( + SELECT 1 FROM collection_skills_v2 kept + WHERE kept.collection_id = collection_skills_v2.collection_id + AND kept.owner = :to_owner AND kept.repo = :to_repo + AND kept.name = ${movedSkillName('collection_skills_v2.name')})`, + ...SKILL_TABLES.flatMap(moveRows), + + `UPDATE OR IGNORE skills + SET owner = :to_owner, repo = :to_repo, name = ${movedRowName()}, slug = :to_owner || '/' || ${movedRowName()} + WHERE owner = :from_owner AND repo = :from_repo`, + `DELETE FROM skills WHERE owner = :from_owner AND repo = :from_repo`, + // Trust, indexability, and the counters read the moved and merged rows. + `INSERT OR REPLACE INTO skill_dirty (owner, repo, name, reason, queued_at, attempts) + SELECT owner, repo, name, 'repository_moved', :moved_at, 0 FROM skills WHERE owner = :to_owner AND repo = :to_repo`, + // The agent sitemap lists these routes, and the old ones now answer 301. + // '0' sorts right after '/', so the range is every route under the old name + // and stays on the route index. + `DELETE FROM ai_ready_pages + WHERE route = :old_route OR (route >= :old_route || '/' AND route < :old_route || '0')`, + + ...REPOSITORY_TABLES.flatMap(moveRows), + `DELETE FROM repos WHERE owner = :from_owner AND repo = :from_repo`, +] + +const TOKEN = /:([a-z_]+)/g + +/** + * The statements of one move, in order, each with exactly the parameters it + * uses. D1 rejects a statement bound with more values than it names. + */ +export function repositoryMoveStatements(move: RepositoryMove): MoveStatement[] { + const values = moveValues(move) + return MOVE_SQL.map((template) => { + const order: string[] = [] + const sql = template.replace(TOKEN, (_, token: string) => { + if (!order.includes(token)) + order.push(token) + return `?${order.indexOf(token) + 1}` + }) + return { sql, params: order.map(token => values[token]!) } + }) +} + +/** + * The same move as one SQL script with the values inlined, for + * `wrangler d1 execute --file`. Wrangler runs a file in one transaction. + */ +export function repositoryMoveSql(move: RepositoryMove): string { + const values = moveValues(move) + const header = `-- Move ${move.from.owner}/${move.from.repo} to ${move.to.owner}/${move.to.repo} (Repository ${move.repositoryId})` + const statements = MOVE_SQL.map(template => `${template.replace(TOKEN, (_, token: string) => sqlLiteral(values[token]!))};`) + return [header, ...statements].join('\n') +} + +function moveValues(move: RepositoryMove): Record { + if (sameRepositoryName(move.from, move.to)) + throw new Error(`A Repository cannot move to its own name: ${move.from.owner}/${move.from.repo}`) + return { + from_owner: move.from.owner, + from_repo: move.from.repo, + to_owner: move.to.owner, + to_repo: move.to.repo, + source_owner: move.source.owner, + source_repo: move.source.repo, + repository_id: move.repositoryId, + moved_at: move.movedAt, + root_name: registrySkillName('.', move.to.repo), + old_route: `/gh/${move.from.owner}/${move.from.repo}`.toLowerCase(), + } +} + +function sqlLiteral(value: MoveSqlValue): string { + if (typeof value === 'number') { + if (!Number.isSafeInteger(value)) + throw new Error(`Expected an integer SQL value, got ${value}`) + return String(value) + } + return `'${value.replaceAll('\'', '\'\'')}'` +} diff --git a/layers/registry/server/utils/skill-page-url.ts b/layers/registry/server/utils/skill-page-url.ts index 928b7615..06e9765e 100644 --- a/layers/registry/server/utils/skill-page-url.ts +++ b/layers/registry/server/utils/skill-page-url.ts @@ -1,3 +1,4 @@ +import { findRepositoryAlias } from './repository-aliases' import { canonicalRepoSkillPath } from './skill-routes' export interface SkillPageSource { @@ -44,10 +45,26 @@ export function skillPageCacheKey(source: SkillPageSource): string { * One indexed read: `idx_skills_owner_nocase` bounds it to one owner's rows. * `rendered_skill_path` is null on rows that never rendered, so the directory * name stands in for those. + * + * Delivery attests the name a run used, which can be the old name of a moved + * Repository. That name answers with the page under the new name. */ export async function findSkillPagePath( db: D1Database, source: SkillPageSource, +): Promise { + const path = await findHeldSkillPagePath(db, source) + if (path) + return path + const alias = await findRepositoryAlias(db, { owner: source.owner, repo: source.repository }) + return alias + ? await findHeldSkillPagePath(db, { ...source, owner: alias.targetOwner, repository: alias.targetRepo }) + : null +} + +async function findHeldSkillPagePath( + db: D1Database, + source: SkillPageSource, ): Promise { const isRoot = source.skillPath === '.' const renderedPath = isRoot ? 'SKILL.md' : `${source.skillPath}/SKILL.md` diff --git a/layers/registry/server/utils/skill-run-identity.ts b/layers/registry/server/utils/skill-run-identity.ts index fe8313c8..62dde761 100644 --- a/layers/registry/server/utils/skill-run-identity.ts +++ b/layers/registry/server/utils/skill-run-identity.ts @@ -1,3 +1,5 @@ +import { findRepositoryAlias } from './repository-aliases' + export interface SkillRunIdentityQuery { owner: string repository: string @@ -31,10 +33,30 @@ interface IdentityRow { * the registry keeps the case it first admitted. A row whose case matches * exactly wins. A row with no rendered path, or whose source is gone, has no * identity to run. + * + * A Repository that moved keeps answering under its old name, so a run + * command printed before the move resolves the identity the page shows now. */ export async function findSkillRunIdentity( db: D1Database, query: SkillRunIdentityQuery, +): Promise { + const identity = await findAdmittedIdentity(db, query) + if (identity) + return identity + const alias = await findRepositoryAlias(db, { owner: query.owner, repo: query.repository }) + if (!alias) + return null + return await findAdmittedIdentity(db, { + owner: alias.targetOwner, + repository: alias.targetRepo, + name: alias.rootSkill === query.name.toLowerCase() && alias.targetRootSkill ? alias.targetRootSkill : query.name, + }) +} + +async function findAdmittedIdentity( + db: D1Database, + query: SkillRunIdentityQuery, ): Promise { const row = await db .prepare(` diff --git a/layers/registry/server/utils/sync-prefetch.ts b/layers/registry/server/utils/sync-prefetch.ts index ec59cf2a..a871bd2b 100644 --- a/layers/registry/server/utils/sync-prefetch.ts +++ b/layers/registry/server/utils/sync-prefetch.ts @@ -4,6 +4,7 @@ import type { GithubBindings, RepoSummary } from './github-client' import type { ExistingRepo } from './sync-repo' import { getRepoSummariesBatch } from './github-client' import { resolveRepoSourceIdentityFromRow } from './repo-source-identity' +import { sameRepositoryName } from './repository-move' import { repoUnchangedStatements, unchangedRepoStatus } from './sync-repo' /** One repository the hourly sync is about to queue. */ @@ -61,8 +62,9 @@ interface PrefetchRow extends ExistingRepo { * the writes here are the same statements `syncRepo` runs on that path. * * A discovery claim, a job still holding the repository's progress row, a - * Skill-less repository and any repository GitHub gave no summary for keep - * their job: `syncRepo` owns those verdicts. + * Skill-less repository, a repository GitHub moved to another name, and any + * repository GitHub gave no summary for keep their job: `syncRepo` owns those + * verdicts, the move included (ADR-0015). */ export async function prefetchUnchangedRepos( dependencies: SyncPrefetchDependencies, @@ -83,13 +85,14 @@ export async function prefetchUnchangedRepos( eligible.forEach((candidate, index) => { const row = rows.get(repoKey(candidate)) const summary = batch.summaries[index] - if (!row || !summary || row.in_progress === 1 || !isUnchanged(row, summary)) + if (!row || !summary || row.in_progress === 1 || isMoved(candidate, summary) || !isUnchanged(row, summary)) return unchanged.add(repoKey(candidate)) writes.push(repoUnchangedStatements(dependencies.db, { owner: candidate.owner, repo: candidate.repo, meta: summary.meta, + repositoryId: summary.repositoryId, checkedAt: dependencies.now, ownerVerified: candidate.ownerVerified, })) @@ -105,6 +108,10 @@ export async function prefetchUnchangedRepos( } } +function isMoved(candidate: SyncCandidate, summary: RepoSummary): boolean { + return !sameRepositoryName(candidate, { owner: summary.meta.owner.login, repo: summary.meta.name }) +} + function isUnchanged(row: PrefetchRow, summary: RepoSummary): boolean { const pushedAt = Date.parse(summary.meta.pushed_at) return unchangedRepoStatus({ diff --git a/layers/registry/server/utils/sync-repo.ts b/layers/registry/server/utils/sync-repo.ts index 359d32f4..b72d581f 100644 --- a/layers/registry/server/utils/sync-repo.ts +++ b/layers/registry/server/utils/sync-repo.ts @@ -1,12 +1,14 @@ /// import type { GithubBindings, RepoMeta } from './github-client' +import type { RepositoryMovePlan, RepositoryName } from './repository-move' import type { SkillTrustTier } from './skill-trust' import { canonicalSkillPaths, isRegistrySkillPath, isSkilldCacheSkill } from '#shared/skill-path' import { isCategoryPinned } from '../data/clusters' import { getBlobsBatch, getCommitsBatch, getRepoSummary, getTree, logRateLimit } from './github-client' import { repoStarObservationStatements } from './repo-history' import { resolveRepoSourceIdentityFromRow } from './repo-source-identity' +import { findHeldRepositoryName, moveRepository, planRepositoryMove, sameRepositoryName } from './repository-move' import { skillContentSha256 } from './skill-content-hash' import { parseSkillFile, registrySkillName } from './skill-frontmatter' import { isOfficialSkillRepo, scoreSkillIndexability } from './skill-indexability' @@ -29,6 +31,11 @@ export interface SyncRepoStats { rateLimitResource?: string rateLimitResetAt?: number continuation?: SyncRepoContinuation + /** + * The registry identity the rows moved to, when GitHub answered the + * requested name with another one. The sync went on under that identity. + */ + movedTo?: RepositoryName } export interface SyncRepoContinuation { @@ -455,6 +462,24 @@ export async function refreshRepoAssets( } } +/** + * Move every row of a Repository GitHub now serves as `current`, and answer + * the registry identity the rows carry after the move. A move into a name + * another Repository holds is refused, so nothing merges. + */ +async function followRepositoryMove( + db: D1Database, + requested: RepositoryName, + current: RepositoryName, + repositoryId: number, +): Promise { + const plan = planRepositoryMove(current, await findHeldRepositoryName(db, current), repositoryId) + if (plan._tag === 'held') + return plan + await moveRepository(db, { from: requested, to: plan.to, source: current, repositoryId, movedAt: nowSec() }) + return plan +} + async function loadExistingRepo(db: D1Database, owner: string, repo: string): Promise { return await db .prepare(` @@ -476,6 +501,7 @@ function markRepoSummaryCheckedStatement( owner: string, repo: string, meta: RepoMeta, + repositoryId: number, pushedAt: number | null, checkedAt: number, ): D1PreparedStatement { @@ -492,7 +518,8 @@ function markRepoSummaryCheckedStatement( broken_since = NULL, tree_truncated_at = NULL, source_owner = ?, - source_repo = ? + source_repo = ?, + repository_id = ? WHERE owner = ? AND repo = ?`, ) .bind( @@ -505,6 +532,7 @@ function markRepoSummaryCheckedStatement( checkedAt, meta.owner.login, meta.name, + repositoryId, owner, repo, ) @@ -551,14 +579,15 @@ export function repoUnchangedStatements( owner: string repo: string meta: RepoMeta + repositoryId: number checkedAt: number ownerVerified: boolean }, ): D1PreparedStatement[] { - const { owner, repo, meta, checkedAt } = input + const { owner, repo, meta, repositoryId, checkedAt } = input const pushedAt = epoch(meta.pushed_at) const statements = [ - markRepoSummaryCheckedStatement(db, owner, repo, meta, pushedAt, checkedAt), + markRepoSummaryCheckedStatement(db, owner, repo, meta, repositoryId, pushedAt, checkedAt), clearRepoMissingSkillsStatement(db, owner, repo, checkedAt), ...repoStarObservationStatements(db, owner, repo, meta.stargazers_count ?? 0, checkedAt), ] @@ -756,15 +785,16 @@ export interface SyncRepoOptions { } export async function syncRepo( - owner: string, - repo: string, + requestedOwner: string, + requestedRepo: string, bindings: GithubBindings, db: D1Database, opts: SyncRepoOptions = {}, ): Promise { + const requested: RepositoryName = { owner: requestedOwner, repo: requestedRepo } const stats: SyncRepoStats = { - owner, - repo, + owner: requestedOwner, + repo: requestedRepo, status: 'failed', skillsSeen: 0, skillsUpserted: 0, @@ -782,10 +812,10 @@ export async function syncRepo( stats.rateLimitResetAt = Math.max(stats.rateLimitResetAt ?? 0, rateLimit.reset) } - const existingRepo = await loadExistingRepo(db, owner, repo) - const requestSource = resolveRepoSourceIdentityFromRow({ owner, repo }, existingRepo) + const requestedRepoRow = await loadExistingRepo(db, requestedOwner, requestedRepo) + const requestSource = resolveRepoSourceIdentityFromRow(requested, requestedRepoRow) const repoRes = await getRepoSummary(requestSource.owner, requestSource.repo, bindings) - logRateLimit(`repo ${owner}/${repo}`, repoRes.rateLimit) + logRateLimit(`repo ${requestedOwner}/${requestedRepo}`, repoRes.rateLimit) trackRateLimit(repoRes.rateLimit) // A 401 is the credential, not the repository. Reporting it per repo made an @@ -805,13 +835,14 @@ export async function syncRepo( if (!repoRes.data) { if (repoRes.status === 404 || repoRes.status === 410) - await markRepoMissing(db, owner, repo, nowSec()) + await markRepoMissing(db, requestedOwner, requestedRepo, nowSec()) stats.status = 'failed' stats.reason = `repo fetch ${repoRes.status}` return stats } const meta = repoRes.data.meta + const repositoryId = repoRes.data.repositoryId const sourceOwner = meta.owner.login const sourceRepo = meta.name const headTreeSha = repoRes.data.headTreeSha @@ -819,10 +850,28 @@ export async function syncRepo( const repoPushedAt = epoch(meta.pushed_at) const checkedAt = opts.continuation?.checkedAt ?? nowSec() + // GitHub answers a renamed or transferred Repository with its new name. + // The rows follow it there, and the sync goes on under the new name, so a + // caller keyed by the old name still gets an outcome (ADR-0015). + const current: RepositoryName = { owner: sourceOwner, repo: sourceRepo } + const movePlan = sameRepositoryName(requested, current) + ? null + : await followRepositoryMove(db, requested, current, repositoryId) + if (movePlan?._tag === 'held') { + stats.status = 'failed' + stats.reason = `move_refused: ${current.owner}/${current.repo} is Repository ${repositoryId} on GitHub, and the registry holds that name for Repository ${movePlan.heldBy}` + return stats + } + const movedTo = movePlan?.to + if (movedTo) + stats.movedTo = movedTo + const { owner, repo } = movedTo ?? requested + const existingRepo = movedTo ? await loadExistingRepo(db, owner, repo) : requestedRepoRow + const hasAdmittedSkills = await repoHasAdmittedSkills(db, owner, repo) const markUnchanged = async (status: 'skipped-tree-sha' | 'skipped-pushed-at'): Promise => { - await db.batch(repoUnchangedStatements(db, { owner, repo, meta, checkedAt, ownerVerified: opts.ownerVerified === true })) + await db.batch(repoUnchangedStatements(db, { owner, repo, meta, repositoryId, checkedAt, ownerVerified: opts.ownerVerified === true })) stats.status = opts.ownerVerified ? 'verified-only' : status return stats } @@ -937,8 +986,8 @@ export async function syncRepo( `INSERT INTO repos ( owner, repo, default_branch, stars, forks, description, pushed_at, repo_created_at, repo_meta_synced_at, last_tree_sha, repo_kind, repo_kind_source, - repo_skill_count, broken_since, source_owner, source_repo - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, NULL, ?, ?) + repo_skill_count, broken_since, source_owner, source_repo, repository_id + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, NULL, ?, ?, ?) ON CONFLICT(owner, repo) DO UPDATE SET default_branch = excluded.default_branch, stars = excluded.stars, @@ -954,7 +1003,8 @@ export async function syncRepo( broken_since = NULL, tree_truncated_at = NULL, source_owner = excluded.source_owner, - source_repo = excluded.source_repo`, + source_repo = excluded.source_repo, + repository_id = excluded.repository_id`, ).bind( owner, repo, @@ -971,6 +1021,7 @@ export async function syncRepo( skillFiles.length, sourceOwner, sourceRepo, + repositoryId, ) if (skillFiles.length === 0) { diff --git a/migrations/0146_repository_moves.sql b/migrations/0146_repository_moves.sql new file mode 100644 index 00000000..5d94df66 --- /dev/null +++ b/migrations/0146_repository_moves.sql @@ -0,0 +1,35 @@ +-- A Repository that GitHub renames or transfers moves in the registry too +-- (ADR-0015). Every row moves to the new owner and name, and the old name +-- stays behind as an alias that answers its URLs with a 301. +-- +-- GitHub's numeric Repository ID survives every rename and transfer. Sync +-- records it from the same GraphQL read that reports the current name. +ALTER TABLE repos ADD COLUMN repository_id INTEGER; + +-- One row per old name. `owner` and `repo` are the old registry identity, and +-- the target is where the Repository lives now. GitHub names compare without +-- case, so these do too. +-- +-- A root Skill takes the Repository name, so a move that renames the +-- Repository renames that Skill. `root_skill` keeps the old Skill name and +-- `target_root_skill` the new one, so its old URL redirects as well. +-- +-- Cull path: a later move rewrites the target, and a move back deletes the +-- row. Dropping the table ends every redirect, so do not drop it while the +-- old URLs still receive traffic. +CREATE TABLE IF NOT EXISTS repo_aliases ( + owner TEXT NOT NULL COLLATE NOCASE, + repo TEXT NOT NULL COLLATE NOCASE, + repository_id INTEGER NOT NULL, + target_owner TEXT NOT NULL COLLATE NOCASE, + target_repo TEXT NOT NULL COLLATE NOCASE, + root_skill TEXT, + target_root_skill TEXT, + -- Unix seconds the registry first moved the rows to this target. + moved_at INTEGER NOT NULL, + PRIMARY KEY (owner, repo), + CHECK ((root_skill IS NULL) = (target_root_skill IS NULL)) +); + +-- A later move of the target rewrites every alias that points at it. +CREATE INDEX IF NOT EXISTS idx_repo_aliases_target ON repo_aliases (target_owner, target_repo); diff --git a/scripts/lib/d1-schema-verifier.ts b/scripts/lib/d1-schema-verifier.ts index 25d886cd..98316fe2 100644 --- a/scripts/lib/d1-schema-verifier.ts +++ b/scripts/lib/d1-schema-verifier.ts @@ -252,6 +252,7 @@ export const RESERVED_MIGRATION_NUMBERS = { '0128': '#320 feat(indexnow-curated) claims 0128_indexnow.sql', '0129': '#322 feat(seo-trending-indexable) claims 0129_skill_trending_admissions.sql', '0135': '#404 fix(classifier-accuracy) claims 0135_relaunch_classifications.sql', + '0145': '#510 feat(artifact-delivery) claims 0145_artifact_resolution_linked_files.sql', } satisfies Readonly> /** diff --git a/scripts/move-repositories.ts b/scripts/move-repositories.ts new file mode 100644 index 00000000..5413a64e --- /dev/null +++ b/scripts/move-repositories.ts @@ -0,0 +1,209 @@ +/** + * One-off backfill: move Repositories that GitHub renamed or transferred to + * their new names (ADR-0015). Sync makes the same move when it next reads a + * moved Repository. This runs it now, for the moves the 2026-10-07 run sweep + * found failing on GitHub's 301. + * + * Every move runs the statements `syncRepo` runs, from + * `layers/registry/server/utils/repository-move.ts`, one SQL file per move. + * Wrangler runs each file in one transaction, so a move lands whole or not + * at all. A move whose rows already moved changes nothing. + * + * Usage: + * pnpm exec tsx scripts/move-repositories.ts [--local] [--out ] + * Dry run. Confirms each new name with GitHub by Repository ID, reads the + * rows each move touches, and writes the SQL files. Reads only. + * pnpm exec tsx scripts/move-repositories.ts --apply [--local] [--out ] + * The same checks, then runs each SQL file. + * --reverse + * Moves each Repository back to its old name. Sync moves it forward again + * on its next read, unless the deploy that follows GitHub is rolled back. + * + * `--local` reads and writes the local D1 that `wrangler dev` uses. + * Set GITHUB_TOKEN to read GitHub with your own allowance instead of the + * anonymous one. + */ + +import type { HeldRepositoryName, RepositoryMove, RepositoryName } from '../layers/registry/server/utils/repository-move' +import { execFileSync } from 'node:child_process' +import { mkdirSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' +import process from 'node:process' +import { parseArgs } from 'node:util' +import { movedRegistryName, planRepositoryMove, repositoryMoveSql, sameRepositoryName } from '../layers/registry/server/utils/repository-move' + +interface KnownMove { + /** The registry identity the rows carry now. */ + from: RepositoryName + /** GitHub's name for the Repository now. */ + to: RepositoryName + repositoryId: number +} + +// Confirmed on 2026-10-07 with `GET /repositories/`. +const KNOWN_MOVES: KnownMove[] = [ + { from: { owner: 'box', repo: 'box-for-ai' }, to: { owner: 'box', repo: 'skills' }, repositoryId: 1187616611 }, + { from: { owner: 'brianlovin', repo: 'claude-config' }, to: { owner: 'brianlovin', repo: 'agent-config' }, repositoryId: 1137712370 }, + { from: { owner: 'dmccreary', repo: 'claude-skills' }, to: { owner: 'dmccreary', repo: 'ibook-skills' }, repositoryId: 1087738380 }, + { from: { owner: 'facebook', repo: 'react' }, to: { owner: 'react', repo: 'react' }, repositoryId: 10270250 }, + { from: { owner: 'flutter', repo: 'skills' }, to: { owner: 'flutter', repo: 'agent-plugins' }, repositoryId: 1167049909 }, + { from: { owner: 'georgeguimaraes', repo: 'claude-code-elixir' }, to: { owner: 'georgeguimaraes', repo: 'elixir-agent-tools' }, repositoryId: 1123032156 }, + { from: { owner: 'hyf0', repo: 'vue-skills' }, to: { owner: 'vuejs-ai', repo: 'skills' }, repositoryId: 1138832642 }, + { from: { owner: 'launchdarkly', repo: 'agent-skills' }, to: { owner: 'launchdarkly', repo: 'ai-tooling' }, repositoryId: 1148493063 }, + { from: { owner: 'palkan', repo: 'skills' }, to: { owner: 'palkan', repo: 'layered-rails-skills' }, repositoryId: 1150861761 }, + { from: { owner: 'shadcn', repo: 'ui' }, to: { owner: 'shadcn-ui', repo: 'ui' }, repositoryId: 585146387 }, + { from: { owner: 'sveltejs', repo: 'mcp' }, to: { owner: 'sveltejs', repo: 'ai-tools' }, repositoryId: 1054419133 }, + { from: { owner: 'vercel-labs', repo: 'vercel-plugin' }, to: { owner: 'vercel', repo: 'vercel-plugin' }, repositoryId: 1172750294 }, + { from: { owner: 'waynesutton', repo: 'convexskills' }, to: { owner: 'waynesutton', repo: 'builder-skills' }, repositoryId: 1133906703 }, +] + +const ACCOUNT_ID = '5904138d55ca25d5670dca6adf99894e' + +// The tables a move rewrites, and the column that holds the Repository name. +const COUNTED_TABLES: Array<[table: string, repo: string]> = [ + ['repos', 'repo'], + ['skills', 'repo'], + ['skill_likes', 'repo'], + ['skill_revisions', 'repo'], + ['skill_generated', 'repo'], + ['activity', 'repo'], + ['collection_skills_v2', 'repo'], + ['skill_subscriptions', 'repo'], + ['skill_trending_admissions', 'repo'], + ['skill_trending_awards', 'repo'], + ['artifact_run_checks', 'repository'], + ['repo_star_observations', 'repo'], + ['x_post_skills', 'repo'], +] + +type Target = { _tag: 'local' } | { _tag: 'remote' } + +interface GithubRepository { + id: number + full_name: string +} + +function d1Args(target: Target): string[] { + return ['exec', 'wrangler', 'd1', 'execute', 'DB', '--config', 'wrangler.jsonc', target._tag === 'local' ? '--local' : '--remote'] +} + +function runWrangler(target: Target, args: string[]): string { + return execFileSync('pnpm', [...d1Args(target), ...args], { + encoding: 'utf8', + maxBuffer: 64 * 1024 * 1024, + env: { ...process.env, CLOUDFLARE_ACCOUNT_ID: process.env.CLOUDFLARE_ACCOUNT_ID ?? ACCOUNT_ID }, + }) +} + +function readRows(target: Target, sql: string): T[] { + const out = runWrangler(target, ['--json', '--command', sql]) + const parsed = JSON.parse(out) as Array<{ results: T[] }> + return parsed[0]?.results ?? [] +} + +const literal = (value: string) => `'${value.replaceAll('\'', '\'\'')}'` + +async function confirmGithubName(move: KnownMove): Promise { + const headers = new Headers({ 'Accept': 'application/vnd.github+json', 'User-Agent': 'skilld.dev move-repositories' }) + if (process.env.GITHUB_TOKEN) + headers.set('Authorization', `Bearer ${process.env.GITHUB_TOKEN}`) + const response = await fetch(`https://api.github.com/repositories/${move.repositoryId}`, { headers }) + if (!response.ok) + throw new Error(`GitHub answered ${response.status} for Repository ${move.repositoryId} (${move.from.owner}/${move.from.repo}).`) + const body = await response.json() as GithubRepository + const [owner, repo] = body.full_name.split('/') as [string, string] + if (body.id !== move.repositoryId || !sameRepositoryName({ owner, repo }, move.to)) { + throw new Error( + `Repository ${move.repositoryId} is ${body.full_name} on GitHub, not ${move.to.owner}/${move.to.repo}. Update KNOWN_MOVES first.`, + ) + } + return { owner, repo } +} + +function heldName(target: Target, name: RepositoryName): HeldRepositoryName | null { + const [row] = readRows(target, ` + SELECT owner, repo, repository_id AS repositoryId FROM repos + WHERE owner = ${literal(name.owner)} COLLATE NOCASE AND repo = ${literal(name.repo)} COLLATE NOCASE + ORDER BY (owner = lower(${literal(name.owner)}) AND repo = lower(${literal(name.repo)})) DESC + LIMIT 1`) + return row ?? null +} + +/** The move writes columns and a table that migration 0146 adds. */ +function assertMigrated(target: Target): void { + const [row] = readRows<{ migrated: number }>(target, `SELECT COUNT(*) AS migrated FROM pragma_table_info('repos') WHERE name = 'repository_id'`) + if (!row?.migrated) + throw new Error(`The ${target._tag} D1 lacks migration 0146_repository_moves.sql. Deploy first, then run this script.`) +} + +function countRows(target: Target, name: RepositoryName): Record { + const counts = COUNTED_TABLES.map(([table, repo]) => + `(SELECT COUNT(*) FROM ${table} WHERE owner = ${literal(name.owner)} AND ${repo} = ${literal(name.repo)}) AS ${table}`) + const [row] = readRows>(target, `SELECT ${counts.join(', ')}`) + return row ?? {} +} + +async function planMove(target: Target, known: KnownMove, reverse: boolean, movedAt: number): Promise { + const current = await confirmGithubName(known) + const from = reverse ? movedRegistryName(current, heldName(target, current)) : known.from + const name = reverse ? known.from : current + // The same check sync runs: never merge into a row another Repository holds. + const plan = planRepositoryMove(name, heldName(target, name), known.repositoryId) + if (plan._tag === 'held') + throw new Error(`The registry holds ${name.owner}/${name.repo} for Repository ${plan.heldBy}, not ${known.repositoryId}. Resolve that row first.`) + return { from, to: plan.to, source: name, repositoryId: known.repositoryId, movedAt } +} + +function describeCounts(counts: Record): string { + const moving = Object.entries(counts).filter(([, count]) => count > 0) + return moving.length ? moving.map(([table, count]) => `${table} ${count}`).join(', ') : 'no rows' +} + +async function main(): Promise { + const { values } = parseArgs({ + options: { + apply: { type: 'boolean', default: false }, + local: { type: 'boolean', default: false }, + reverse: { type: 'boolean', default: false }, + out: { type: 'string' }, + }, + }) + const target: Target = values.local ? { _tag: 'local' } : { _tag: 'remote' } + const outDir = resolve(values.out ?? join(tmpdir(), `skilld-repository-moves-${new Date().toISOString().slice(0, 10)}`)) + mkdirSync(outDir, { recursive: true }) + const movedAt = Math.floor(Date.now() / 1000) + + console.log(`${values.apply ? 'Apply' : 'Dry run'} against the ${target._tag} D1. SQL files go to ${outDir}.`) + assertMigrated(target) + const files: string[] = [] + for (const [index, known] of KNOWN_MOVES.entries()) { + const move = await planMove(target, known, values.reverse, movedAt) + const held = heldName(target, move.to) + console.log(`\n${move.from.owner}/${move.from.repo} -> ${move.to.owner}/${move.to.repo} (Repository ${move.repositoryId})`) + console.log(` rows under the old name: ${describeCounts(countRows(target, move.from))}`) + console.log(held + ? ` the registry already holds ${held.owner}/${held.repo}: ${describeCounts(countRows(target, held))}. The rows merge, and a clash keeps that row.` + : ` the registry does not hold the new name yet.`) + const file = join(outDir, `${String(index + 1).padStart(2, '0')}-${move.from.owner}-${move.from.repo}.sql`) + writeFileSync(file, `${repositoryMoveSql(move)}\n`) + files.push(file) + } + + if (!values.apply) { + console.log(`\nDry run done. Wrote ${files.length} SQL files. Nothing changed.`) + console.log(`To run them, add --apply${values.local ? ' --local' : ''}.`) + return + } + + for (const file of files) { + console.log(`\nRunning ${file}`) + runWrangler(target, ['--file', file, '--yes']) + } + console.log(`\nApplied ${files.length} moves. The redirects start when the alias cache expires, within 15 minutes.`) +} + +main().catch((error: unknown) => { + console.error(error instanceof Error ? error.message : error) + process.exitCode = 1 +}) diff --git a/scripts/seed-homepage-collections.sql b/scripts/seed-homepage-collections.sql index 7718a4d5..54d0b809 100644 --- a/scripts/seed-homepage-collections.sql +++ b/scripts/seed-homepage-collections.sql @@ -243,7 +243,7 @@ INSERT INTO collection_skills_v2 (collection_id, position, owner, repo, name, re ((SELECT id FROM collections_v2 WHERE author_user_id = (SELECT id FROM users WHERE login = 'harlan-zw') AND slug = 'design-engineering-essentials'), 3, 'vercel-labs', 'agent-skills', 'web-design-guidelines', 'Rauno Freiberg''s Web Interface Guidelines as a review command: the checklist design engineers already run by hand.'), ((SELECT id FROM collections_v2 WHERE author_user_id = (SELECT id FROM users WHERE login = 'harlan-zw') AND slug = 'design-engineering-essentials'), 4, 'ibelick', 'ui-skills', 'fixing-motion-performance', 'Julien Thibeaut''s fixing-motion-performance catches the janky transitions and animations that make even a good design feel cheap.'), ((SELECT id FROM collections_v2 WHERE author_user_id = (SELECT id FROM users WHERE login = 'harlan-zw') AND slug = 'design-engineering-essentials'), 5, 'ibelick', 'ui-skills', 'fixing-accessibility', 'Julien Thibeaut''s concrete accessibility pass: keyboard, focus, ARIA, contrast, and form errors, the states agents most often forget.'), -((SELECT id FROM collections_v2 WHERE author_user_id = (SELECT id FROM users WHERE login = 'harlan-zw') AND slug = 'design-engineering-essentials'), 6, 'shadcn', 'ui', 'shadcn', 'The shadcn/ui skill from the source: component composition, theming, and the patterns the library was built around.'), +((SELECT id FROM collections_v2 WHERE author_user_id = (SELECT id FROM users WHERE login = 'harlan-zw') AND slug = 'design-engineering-essentials'), 6, 'shadcn-ui', 'ui', 'shadcn', 'The shadcn/ui skill from the source: component composition, theming, and the patterns the library was built around.'), ((SELECT id FROM collections_v2 WHERE author_user_id = (SELECT id FROM users WHERE login = 'harlan-zw') AND slug = 'design-engineering-essentials'), 7, 'vercel-labs', 'agent-skills', 'react-best-practices', 'Vercel''s own React playbook, maintained by the team that ships the framework''s reference deployments.'), ((SELECT id FROM collections_v2 WHERE author_user_id = (SELECT id FROM users WHERE login = 'harlan-zw') AND slug = 'design-engineering-essentials'), 8, 'millionco', 'react-doctor', 'react-doctor', 'Million''s React Doctor diagnoses render cost, wasted re-renders, and the performance issues that quietly degrade a UI.'), ((SELECT id FROM collections_v2 WHERE author_user_id = (SELECT id FROM users WHERE login = 'harlan-zw') AND slug = 'design-engineering-essentials'), 9, 'microsoft', 'playwright-cli', 'playwright-cli', 'Microsoft''s Playwright CLI skill for proving the change in a real browser: end-to-end flows, selectors, and reliable assertions.'), @@ -288,7 +288,7 @@ INSERT INTO collection_skills_v2 (collection_id, position, owner, repo, name, re -- react ((SELECT id FROM collections_v2 WHERE author_user_id = (SELECT id FROM users WHERE login = 'harlan-zw') AND slug = 'react'), 0, 'vercel-labs', 'agent-skills', 'react-best-practices', 'Vercel''s React playbook is the right default for modern React work, from the team behind Next.js.'), ((SELECT id FROM collections_v2 WHERE author_user_id = (SELECT id FROM users WHERE login = 'harlan-zw') AND slug = 'react'), 1, 'vercel-labs', 'vercel-plugin', 'nextjs', 'Vercel''s canonical Next.js reference: App Router, server components, caching, and current framework conventions.'), -((SELECT id FROM collections_v2 WHERE author_user_id = (SELECT id FROM users WHERE login = 'harlan-zw') AND slug = 'react'), 2, 'shadcn', 'ui', 'shadcn', 'shadcn/ui''s own skill covers component composition, theming, and the patterns the library was designed around.'), +((SELECT id FROM collections_v2 WHERE author_user_id = (SELECT id FROM users WHERE login = 'harlan-zw') AND slug = 'react'), 2, 'shadcn-ui', 'ui', 'shadcn', 'shadcn/ui''s own skill covers component composition, theming, and the patterns the library was designed around.'), ((SELECT id FROM collections_v2 WHERE author_user_id = (SELECT id FROM users WHERE login = 'harlan-zw') AND slug = 'react'), 3, 'vercel-labs', 'agent-skills', 'composition-patterns', 'Vercel''s composition-patterns teaches the component composition that keeps React codebases flexible as they grow.'), ((SELECT id FROM collections_v2 WHERE author_user_id = (SELECT id FROM users WHERE login = 'harlan-zw') AND slug = 'react'), 4, 'deckardger', 'tanstack-agent-skills', 'tanstack-query', 'TanStack Query patterns for the data-fetching layer most React product work sits on.'), ((SELECT id FROM collections_v2 WHERE author_user_id = (SELECT id FROM users WHERE login = 'harlan-zw') AND slug = 'react'), 5, 'millionco', 'react-doctor', 'react-doctor', 'Million''s React Doctor diagnoses wasted re-renders and performance issues on real component trees.'), diff --git a/shared/track-board.ts b/shared/track-board.ts index 14c34a74..dc4ad844 100644 --- a/shared/track-board.ts +++ b/shared/track-board.ts @@ -191,8 +191,9 @@ function memberRow(member: TrackMemberInput): TrendingBoardRow { * draws the signal that ranked it. * * One owner's Skill of one name is one Skill to a reader, even when two - * repositories ship it: a renamed repository keeps its old registry identity - * (ADR-0002), and a plugin bundle copies a Skill under the same name. The + * repositories ship it: a moved Repository sits under both names until sync + * follows the move (ADR-0015), and a plugin bundle copies a Skill under the + * same name. The * first list to claim it keeps it: the talked section, then the pins, then * the most-starred copy. */ diff --git a/test/unit/discovery-size-guard-not-modified.test.ts b/test/unit/discovery-size-guard-not-modified.test.ts index c95c4450..711e3bb0 100644 --- a/test/unit/discovery-size-guard-not-modified.test.ts +++ b/test/unit/discovery-size-guard-not-modified.test.ts @@ -35,6 +35,7 @@ function summaryResponse() { JSON.stringify({ data: { repository: { + databaseId: 1, name: 'repo', nameWithOwner: 'owner/repo', url: 'https://github.com/owner/repo', diff --git a/test/unit/github-repo-identity.test.ts b/test/unit/github-repo-identity.test.ts index c007507c..6b56539c 100644 --- a/test/unit/github-repo-identity.test.ts +++ b/test/unit/github-repo-identity.test.ts @@ -8,6 +8,7 @@ describe('github repository identity', () => { const fetch = vi.fn(async () => Response.json({ data: { repository: { + databaseId: 1072000123, name: 'openclaw', nameWithOwner: 'openclaw/openclaw', url: 'https://github.com/openclaw/openclaw', @@ -36,6 +37,17 @@ describe('github repository identity', () => { html_url: 'https://github.com/openclaw/openclaw', owner: { login: 'openclaw' }, }) + expect(result.data?.repositoryId).toBe(1072000123) expect(fetch.mock.calls[0]?.[1]?.body).toContain('nameWithOwner') }) + + it('fails the read when GitHub names no Repository ID', async () => { + vi.stubGlobal('fetch', vi.fn(async () => Response.json({ + data: { repository: { databaseId: null, name: 'skills', nameWithOwner: 'acme/skills', owner: { login: 'acme' } } }, + }))) + + const result = await getRepoSummary('acme', 'skills', {}) + + expect(result).toMatchObject({ status: 502, data: null }) + }) }) diff --git a/test/unit/github-repo-summary-batch.test.ts b/test/unit/github-repo-summary-batch.test.ts index e300de3b..a4272b6d 100644 --- a/test/unit/github-repo-summary-batch.test.ts +++ b/test/unit/github-repo-summary-batch.test.ts @@ -10,6 +10,7 @@ interface GraphqlBody { function repository(owner: string, name: string, tree: string) { return { + databaseId: 4242, name, nameWithOwner: `${owner}/${name}`, url: `https://github.com/${owner}/${name}`, diff --git a/test/unit/moved-repository-route.test.ts b/test/unit/moved-repository-route.test.ts new file mode 100644 index 00000000..de6d2007 --- /dev/null +++ b/test/unit/moved-repository-route.test.ts @@ -0,0 +1,167 @@ +import type { RepositoryMove } from '../../layers/registry/server/utils/repository-move' +import type { SqliteD1 } from './helpers/d1-sqlite' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { + indexRepositoryAliases, + loadRepositoryAliases, + REPOSITORY_ALIASES_CACHE_KEY, + resolveMovedRepositoryRoute, +} from '../../layers/registry/server/utils/repository-aliases' +import { moveRepository } from '../../layers/registry/server/utils/repository-move' +import { findSkillPagePath } from '../../layers/registry/server/utils/skill-page-url' +import { findSkillRunIdentity } from '../../layers/registry/server/utils/skill-run-identity' +import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' + +let d1: SqliteD1 + +beforeEach(() => { + d1 = createSqliteD1(allMigrations()) +}) + +afterEach(() => d1.close()) + +function memoryCache() { + const store = new Map() + return { + store, + getItem: async (key: string) => (store.get(key) as T | undefined) ?? null, + setItem: async (key: string, value: never) => { + store.set(key, value) + }, + } +} + +function seed(owner: string, repo: string, skills: Array<{ name: string, path: string }>): void { + d1.raw.exec(`INSERT INTO repos (owner, repo, repo_skill_count) VALUES ('${owner}', '${repo}', ${skills.length})`) + for (const skill of skills) { + d1.raw.exec(` + INSERT INTO skills (owner, repo, name, display_name, slug, source_resolved, rendered_skill_path, rendered_status) + VALUES ('${owner}', '${repo}', '${skill.name}', '${skill.name}', '${owner}/${skill.name}', 1, '${skill.path}', 'ok'); + INSERT INTO skill_revisions (owner, repo, name, sha, modified_at) + VALUES ('${owner}', '${repo}', '${skill.name}', '${'a'.repeat(40)}', 1); + `) + } +} + +function move(from: string, to: string, repositoryId = 1): RepositoryMove { + const [fromOwner, fromRepo] = from.split('/') as [string, string] + const [toOwner, toRepo] = to.split('/') as [string, string] + return { + from: { owner: fromOwner, repo: fromRepo }, + to: { owner: toOwner, repo: toRepo }, + source: { owner: toOwner, repo: toRepo }, + repositoryId, + movedAt: 1_790_000_000, + } +} + +async function route(pathname: string, search = '') { + const aliases = await loadRepositoryAliases(memoryCache(), d1.db) + return resolveMovedRepositoryRoute(pathname, search, indexRepositoryAliases(aliases ?? [])) +} + +describe('the old URLs of a moved Repository', () => { + beforeEach(async () => { + seed('facebook', 'react', [ + { name: 'fix', path: 'compiler/skills/fix/SKILL.md' }, + { name: 'verify', path: 'compiler/skills/verify/SKILL.md' }, + ]) + await moveRepository(d1.db, move('facebook/react', 'react/react', 10270250)) + }) + + it('answers 301 from the Repository hub and every Skill page under it', async () => { + expect(await route('/gh/facebook/react')).toEqual({ _tag: 'redirect', location: '/gh/react/react' }) + expect(await route('/gh/facebook/react/fix')).toEqual({ _tag: 'redirect', location: '/gh/react/react/fix' }) + expect(await route('/gh/Facebook/React/verify', '?tab=files')).toEqual({ _tag: 'redirect', location: '/gh/react/react/verify?tab=files' }) + }) + + it('moves file deep links and the agent copies too', async () => { + expect(await route('/gh/facebook/react/fix/-/references/rules.md')).toEqual({ + _tag: 'redirect', + location: '/gh/react/react/fix/-/references/rules.md', + }) + expect(await route('/gh/facebook/react/fix.md')).toEqual({ _tag: 'redirect', location: '/gh/react/react/fix.md' }) + expect(await route('/gh/facebook/react.md')).toEqual({ _tag: 'redirect', location: '/gh/react/react.md' }) + }) + + it('leaves the old owner hub and its other Repositories alone', async () => { + seed('facebook', 'docusaurus', [{ name: 'docs', path: 'skills/docs/SKILL.md' }]) + + expect(await route('/gh/facebook')).toEqual({ _tag: 'pass' }) + expect(await route('/gh/facebook/docusaurus/docs')).toEqual({ _tag: 'pass' }) + expect(await route('/skills/facebook/react/fix')).toEqual({ _tag: 'pass' }) + }) + + it('stops redirecting once the registry holds the old name again', async () => { + seed('facebook', 'react', [{ name: 'other', path: 'skills/other/SKILL.md' }]) + + expect(await route('/gh/facebook/react/other')).toEqual({ _tag: 'pass' }) + }) + + it('reads the aliases once and serves the next request from the cache', async () => { + const cache = memoryCache() + await loadRepositoryAliases(cache, d1.db) + d1.raw.exec(`DELETE FROM repo_aliases`) + + const aliases = await loadRepositoryAliases(cache, d1.db) + + expect(cache.store.has(REPOSITORY_ALIASES_CACHE_KEY)).toBe(true) + expect(aliases).toHaveLength(1) + }) +}) + +describe('a Repository that moves twice', () => { + it('sends the first name straight to the latest one', async () => { + seed('a', 'one', [{ name: 'tool', path: 'skills/tool/SKILL.md' }]) + await moveRepository(d1.db, move('a/one', 'b/two')) + await moveRepository(d1.db, move('b/two', 'c/three')) + + expect(await route('/gh/a/one/tool')).toEqual({ _tag: 'redirect', location: '/gh/c/three/tool' }) + expect(await route('/gh/b/two/tool')).toEqual({ _tag: 'redirect', location: '/gh/c/three/tool' }) + }) + + it('drops the alias when it moves back', async () => { + seed('a', 'one', [{ name: 'tool', path: 'skills/tool/SKILL.md' }]) + await moveRepository(d1.db, move('a/one', 'b/two')) + await moveRepository(d1.db, move('b/two', 'a/one')) + + expect(await route('/gh/a/one/tool')).toEqual({ _tag: 'pass' }) + expect(await route('/gh/b/two/tool')).toEqual({ _tag: 'redirect', location: '/gh/a/one/tool' }) + }) +}) + +describe('a renamed root Skill', () => { + it('redirects its old page to the Skill under the new name', async () => { + seed('palkan', 'skills', [{ name: 'skills', path: 'SKILL.md' }]) + await moveRepository(d1.db, move('palkan/skills', 'palkan/layered-rails-skills')) + + expect(await route('/gh/palkan/skills/skills')).toEqual({ + _tag: 'redirect', + location: '/gh/palkan/layered-rails-skills/layered-rails-skills', + }) + }) +}) + +describe('the Skill page under the new name', () => { + beforeEach(async () => { + seed('facebook', 'react', [ + { name: 'fix', path: 'compiler/skills/fix/SKILL.md' }, + { name: 'verify', path: 'compiler/skills/verify/SKILL.md' }, + ]) + await moveRepository(d1.db, move('facebook/react', 'react/react', 10270250)) + }) + + it('is the page both names link to', async () => { + const page = '/gh/react/react/fix' + + expect(await findSkillPagePath(d1.db, { owner: 'react', repository: 'react', skillPath: 'compiler/skills/fix' })).toBe(page) + expect(await findSkillPagePath(d1.db, { owner: 'facebook', repository: 'react', skillPath: 'compiler/skills/fix' })).toBe(page) + }) + + it('is what a run of the old name resolves', async () => { + expect(await findSkillRunIdentity(d1.db, { owner: 'facebook', repository: 'react', name: 'fix' })).toEqual({ + skillPath: 'compiler/skills/fix', + commitSha: 'a'.repeat(40), + }) + }) +}) diff --git a/test/unit/repository-move.test.ts b/test/unit/repository-move.test.ts new file mode 100644 index 00000000..fb938673 --- /dev/null +++ b/test/unit/repository-move.test.ts @@ -0,0 +1,337 @@ +import type { SqliteD1 } from './helpers/d1-sqlite' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { pruneOrphanEmbeddings } from '../../layers/registry/server/utils/embedding-parity' +import { moveRepository, repositoryMoveSql } from '../../layers/registry/server/utils/repository-move' +import { syncRepo } from '../../layers/registry/server/utils/sync-repo' +import { vectorIdFor } from '../../layers/registry/server/utils/vector-id' +import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' + +const github = vi.hoisted(() => ({ + getBlobsBatch: vi.fn(), + getCommitsBatch: vi.fn(), + getRepoSummary: vi.fn(), + getTree: vi.fn(), + logRateLimit: vi.fn(), +})) + +vi.mock('../../layers/registry/server/utils/github-client', () => github) + +const REACT_ID = 10270250 +const NOW = 1_790_000_000 + +let d1: SqliteD1 + +beforeEach(() => { + vi.useFakeTimers() + vi.setSystemTime(NOW * 1000) + vi.clearAllMocks() + d1 = createSqliteD1(allMigrations()) +}) + +afterEach(() => { + vi.useRealTimers() + d1.close() +}) + +/** GitHub's answer for a Repository whose head tree the registry already read. */ +function githubAnswer(fullName: string, repositoryId: number, headTreeSha = 'tree-1') { + const [owner, name] = fullName.split('/') as [string, string] + return { + status: 200, + data: { + repositoryId, + headTreeSha, + meta: { + name, + full_name: fullName, + html_url: `https://github.com/${fullName}`, + owner: { login: owner }, + default_branch: 'main', + description: 'The library for web and native user interfaces.', + stargazers_count: 240_000, + forks_count: 50_000, + pushed_at: '2026-10-01T00:00:00Z', + created_at: '2013-05-24T16:15:54Z', + }, + }, + rateLimit: null, + notModified: false, + } +} + +function exec(sql: string): void { + d1.raw.exec(sql) +} + +function rows(sql: string): T[] { + return d1.raw.prepare(sql).all() as T[] +} + +function seedUser(id: number): void { + exec(`INSERT INTO users (id, github_id, login, created_at, last_login_at) VALUES (${id}, ${id}, 'user-${id}', 1, 1)`) +} + +function seedRepo(owner: string, repo: string, extra: { source?: string, tree?: string } = {}): void { + const [sourceOwner, sourceRepo] = extra.source?.split('/') ?? [null, null] + exec(` + INSERT INTO repos (owner, repo, default_branch, stars, last_tree_sha, pushed_at, repo_meta_synced_at, repo_skill_count, source_owner, source_repo) + VALUES ('${owner}', '${repo}', 'main', 10, '${extra.tree ?? 'tree-1'}', ${NOW - 100}, ${NOW - 100}, 2, + ${sourceOwner ? `'${sourceOwner}'` : 'NULL'}, ${sourceRepo ? `'${sourceRepo}'` : 'NULL'})`) +} + +function seedSkill(owner: string, repo: string, name: string, path = `skills/${name}/SKILL.md`): void { + exec(` + INSERT INTO skills (owner, repo, name, display_name, slug, description, current_sha, source_resolved, + rendered_skill_path, rendered_status, rendered_raw, seo_indexable) + VALUES ('${owner}', '${repo}', '${name}', '${name}', '${owner}/${name}', 'Does ${name}.', 'sha-${name}', 1, + '${path}', 'ok', '# ${name}', 1)`) +} + +describe('sync follows a moved Repository by its ID', () => { + beforeEach(() => { + seedUser(101) + seedRepo('facebook', 'react', { source: 'react/react' }) + seedSkill('facebook', 'react', 'fix') + seedSkill('facebook', 'react', 'verify') + exec(` + INSERT INTO skill_likes (user_id, owner, repo, name, created_at) VALUES (101, 'facebook', 'react', 'fix', 1); + INSERT INTO skill_revisions (owner, repo, name, sha, modified_at) VALUES ('facebook', 'react', 'fix', 'commit-1', 1); + INSERT INTO skill_generated (owner, repo, name, kind, sha, payload, generated_at) + VALUES ('facebook', 'react', 'fix', 'embedding', 'sha-fix', '{}', '2026-10-01'), + ('facebook', 'react', 'fix', 'faq', 'sha-fix', '[]', '2026-10-01'); + INSERT INTO repo_star_observations (owner, repo, observed_day, stars) VALUES ('facebook', 'react', ${86400 * 20000}, 10); + INSERT INTO skill_subscriptions (user_id, owner, repo, source, created_at) VALUES (101, 'facebook', 'react', 'manual', 1); + `) + github.getRepoSummary.mockResolvedValue(githubAnswer('react/react', REACT_ID)) + }) + + it('moves the Repository and its Skills to the new name and keeps going there', async () => { + const stats = await syncRepo('facebook', 'react', {}, d1.db) + + expect(github.getRepoSummary).toHaveBeenCalledWith('react', 'react', {}) + expect(stats.movedTo).toEqual({ owner: 'react', repo: 'react' }) + expect(stats.status).toBe('skipped-tree-sha') + expect(rows(`SELECT owner, repo, repository_id, source_owner, source_repo, stars, last_tree_sha FROM repos`)).toEqual([ + { owner: 'react', repo: 'react', repository_id: REACT_ID, source_owner: 'react', source_repo: 'react', stars: 240_000, last_tree_sha: 'tree-1' }, + ]) + expect(rows(`SELECT owner, repo, name, slug FROM skills ORDER BY name`)).toEqual([ + { owner: 'react', repo: 'react', name: 'fix', slug: 'react/fix' }, + { owner: 'react', repo: 'react', name: 'verify', slug: 'react/verify' }, + ]) + expect(rows(`SELECT owner, repo, name FROM skill_likes`)).toEqual([{ owner: 'react', repo: 'react', name: 'fix' }]) + expect(rows(`SELECT owner, repo, name FROM skill_revisions`)).toEqual([{ owner: 'react', repo: 'react', name: 'fix' }]) + expect(rows(`SELECT owner, repo FROM skill_subscriptions`)).toEqual([{ owner: 'react', repo: 'react' }]) + expect(rows(`SELECT DISTINCT owner, repo FROM repo_star_observations`)).toEqual([{ owner: 'react', repo: 'react' }]) + }) + + it('keeps the old name as an alias row', async () => { + await syncRepo('facebook', 'react', {}, d1.db) + + expect(rows(`SELECT owner, repo, repository_id, target_owner, target_repo, root_skill, moved_at FROM repo_aliases`)).toEqual([ + { owner: 'facebook', repo: 'react', repository_id: REACT_ID, target_owner: 'react', target_repo: 'react', root_skill: null, moved_at: NOW }, + ]) + }) + + it('queues every moved Skill for a trust and indexability recompute', async () => { + await syncRepo('facebook', 'react', {}, d1.db) + + expect(rows(`SELECT owner, repo, name FROM skill_dirty WHERE reason = 'repository_moved' ORDER BY name`)).toEqual([ + { owner: 'react', repo: 'react', name: 'fix' }, + { owner: 'react', repo: 'react', name: 'verify' }, + ]) + }) + + it('leaves the embedding marker under the old name, so the prune removes the old vector', async () => { + seedRepo('acme', 'skills') + for (const name of ['one', 'two', 'three']) { + seedSkill('acme', 'skills', name) + exec(`INSERT INTO skill_generated (owner, repo, name, kind, sha, payload, generated_at) VALUES ('acme', 'skills', '${name}', 'embedding', 'sha-${name}', '{}', '2026-10-01')`) + } + await syncRepo('facebook', 'react', {}, d1.db) + const deleteByIds = vi.fn(async () => ({ mutationId: 'm' })) + + const outcome = await pruneOrphanEmbeddings({ db: d1.db, vectorize: { deleteByIds } }) + + expect(rows(`SELECT owner, repo, kind FROM skill_generated WHERE name = 'fix' ORDER BY kind`)).toEqual([ + { owner: 'react', repo: 'react', kind: 'faq' }, + ]) + expect(outcome.deleted).toBe(1) + expect(deleteByIds).toHaveBeenCalledWith([await vectorIdFor({ owner: 'facebook', repo: 'react', name: 'fix' })]) + }) + + it('moves nothing when GitHub only changes the case of the name', async () => { + github.getRepoSummary.mockResolvedValue(githubAnswer('Facebook/React', REACT_ID)) + + const stats = await syncRepo('facebook', 'react', {}, d1.db) + + expect(stats.movedTo).toBeUndefined() + expect(rows(`SELECT owner, repo, repository_id FROM repos`)).toEqual([{ owner: 'facebook', repo: 'react', repository_id: REACT_ID }]) + expect(rows(`SELECT COUNT(*) AS aliases FROM repo_aliases`)).toEqual([{ aliases: 0 }]) + }) +}) + +describe('a move into a name the registry already holds', () => { + beforeEach(() => { + seedUser(101) + seedUser(102) + seedRepo('vuejs-ai', 'skills') + seedSkill('vuejs-ai', 'skills', 'vue-best-practices') + seedRepo('hyf0', 'vue-skills', { source: 'vuejs-ai/skills' }) + seedSkill('hyf0', 'vue-skills', 'vue-best-practices') + seedSkill('hyf0', 'vue-skills', 'vue-debug-guides') + exec(` + INSERT INTO skill_likes (user_id, owner, repo, name, created_at) VALUES + (101, 'vuejs-ai', 'skills', 'vue-best-practices', 1), + (101, 'hyf0', 'vue-skills', 'vue-best-practices', 1), + (102, 'hyf0', 'vue-skills', 'vue-best-practices', 1); + INSERT INTO collections_v2 (id, author_user_id, slug, name, created_at, updated_at) VALUES (901, 101, 'vue', 'Vue', 1, 1); + INSERT INTO collection_skills_v2 (collection_id, position, owner, repo, name) VALUES + (901, 0, 'vuejs-ai', 'skills', 'vue-best-practices'), + (901, 1, 'hyf0', 'vue-skills', 'vue-best-practices'), + (901, 2, 'hyf0', 'vue-skills', 'vue-debug-guides'); + `) + github.getRepoSummary.mockResolvedValue(githubAnswer('vuejs-ai/skills', 1138832642)) + }) + + it('merges into the held row and drops the duplicates', async () => { + await syncRepo('hyf0', 'vue-skills', {}, d1.db) + + expect(rows(`SELECT owner, repo FROM repos`)).toEqual([{ owner: 'vuejs-ai', repo: 'skills' }]) + expect(rows(`SELECT owner, repo, name FROM skills ORDER BY name`)).toEqual([ + { owner: 'vuejs-ai', repo: 'skills', name: 'vue-best-practices' }, + { owner: 'vuejs-ai', repo: 'skills', name: 'vue-debug-guides' }, + ]) + expect(rows(`SELECT user_id, owner, repo FROM skill_likes ORDER BY user_id`)).toEqual([ + { user_id: 101, owner: 'vuejs-ai', repo: 'skills' }, + { user_id: 102, owner: 'vuejs-ai', repo: 'skills' }, + ]) + expect(rows(`SELECT position, owner, repo, name FROM collection_skills_v2 WHERE collection_id = 901 ORDER BY position`)).toEqual([ + { position: 0, owner: 'vuejs-ai', repo: 'skills', name: 'vue-best-practices' }, + { position: 2, owner: 'vuejs-ai', repo: 'skills', name: 'vue-debug-guides' }, + ]) + }) +}) + +describe('a move into a name a different Repository holds', () => { + beforeEach(() => { + seedUser(101) + // A stale row: GitHub stopped serving this Repository, markRepoMissing + // kept the row, and its name carries its own Repository ID. + exec(`INSERT INTO repos (owner, repo, repository_id, repo_skill_count, last_tree_sha) VALUES ('vuejs-ai', 'skills', 111, 1, 'tree-1')`) + seedSkill('vuejs-ai', 'skills', 'held-skill') + seedRepo('hyf0', 'vue-skills') + seedSkill('hyf0', 'vue-skills', 'moved-skill') + github.getRepoSummary.mockResolvedValue(githubAnswer('vuejs-ai/skills', 222)) + }) + + it('refuses the move instead of merging another Repository\'s rows', async () => { + const stats = await syncRepo('hyf0', 'vue-skills', {}, d1.db) + + expect(stats.status).toBe('failed') + expect(stats.reason).toContain('move_refused') + expect(rows(`SELECT owner, repo, repository_id FROM repos ORDER BY owner`)).toEqual([ + { owner: 'hyf0', repo: 'vue-skills', repository_id: null }, + { owner: 'vuejs-ai', repo: 'skills', repository_id: 111 }, + ]) + expect(rows(`SELECT owner, repo, name FROM skills ORDER BY name`)).toEqual([ + { owner: 'vuejs-ai', repo: 'skills', name: 'held-skill' }, + { owner: 'hyf0', repo: 'vue-skills', name: 'moved-skill' }, + ]) + expect(rows(`SELECT COUNT(*) AS aliases FROM repo_aliases`)).toEqual([{ aliases: 0 }]) + }) +}) + +describe('a move that renames a root Skill', () => { + it('renames the Skill to the new Repository name and records it on the alias', async () => { + seedRepo('palkan', 'skills', { source: 'palkan/layered-rails-skills' }) + seedSkill('palkan', 'skills', 'skills', 'SKILL.md') + exec(`INSERT INTO skill_revisions (owner, repo, name, sha, modified_at) VALUES ('palkan', 'skills', 'skills', 'commit-1', 1)`) + github.getRepoSummary.mockResolvedValue(githubAnswer('palkan/layered-rails-skills', 1150861761)) + + await syncRepo('palkan', 'skills', {}, d1.db) + + expect(rows(`SELECT owner, repo, name, slug FROM skills`)).toEqual([ + { owner: 'palkan', repo: 'layered-rails-skills', name: 'layered-rails-skills', slug: 'palkan/layered-rails-skills' }, + ]) + expect(rows(`SELECT name FROM skill_revisions`)).toEqual([{ name: 'layered-rails-skills' }]) + expect(rows(`SELECT root_skill, target_root_skill FROM repo_aliases`)).toEqual([ + { root_skill: 'skills', target_root_skill: 'layered-rails-skills' }, + ]) + }) +}) + +describe('the backfill SQL', () => { + const move = { + from: { owner: 'facebook', repo: 'react' }, + to: { owner: 'react', repo: 'react' }, + source: { owner: 'react', repo: 'react' }, + repositoryId: REACT_ID, + movedAt: NOW, + } + + beforeEach(() => { + seedRepo('facebook', 'react') + seedSkill('facebook', 'react', 'fix') + exec(`INSERT INTO skill_social_posts (skill_slug, platform, post_url, post_id, author_handle, text_extract, fetched_at) + VALUES ('facebook/fix', 'bsky', 'https://bsky.app/p/1', 'p1', 'dan', 'try fix', 1)`) + }) + + it('moves the rows the same way the sync batch does', () => { + exec(`INSERT INTO ai_ready_pages (route, route_key, updated_at, indexed_at) VALUES + ('/gh/facebook/react', 'a', '2026-10-01', 1), + ('/gh/facebook/react/fix', 'b', '2026-10-01', 1), + ('/gh/facebook/react-native', 'c', '2026-10-01', 1)`) + + exec(repositoryMoveSql(move)) + + expect(rows(`SELECT owner, repo, name, slug FROM skills`)).toEqual([{ owner: 'react', repo: 'react', name: 'fix', slug: 'react/fix' }]) + expect(rows(`SELECT skill_slug FROM skill_social_posts`)).toEqual([{ skill_slug: 'react/fix' }]) + expect(rows(`SELECT route FROM ai_ready_pages`)).toEqual([{ route: '/gh/facebook/react-native' }]) + expect(rows(`SELECT owner, repo, target_owner, target_repo FROM repo_aliases`)).toEqual([ + { owner: 'facebook', repo: 'react', target_owner: 'react', target_repo: 'react' }, + ]) + }) + + it('changes nothing when it runs a second time', async () => { + exec(repositoryMoveSql(move)) + const before = rows(`SELECT * FROM skills`) + + exec(repositoryMoveSql(move)) + await moveRepository(d1.db, { ...move, movedAt: NOW + 60 }) + + expect(rows(`SELECT * FROM skills`)).toEqual(before) + expect(rows(`SELECT moved_at FROM repo_aliases`)).toEqual([{ moved_at: NOW }]) + }) + + it('keeps every column of the Repository row', async () => { + // Filled from the migrated schema, so a column added later fails here + // until the move copies it too. + const columns = rows<{ name: string, type: string }>(`PRAGMA table_info(repos)`) + .filter(column => !['owner', 'repo'].includes(column.name)) + const value = (column: { name: string, type: string }) => { + if (column.name === 'repo_kind') + return `'catalog'` + if (column.name === 'repo_kind_source') + return `'override'` + return column.type === 'INTEGER' ? '7' : `'${column.name}-value'` + } + exec(`UPDATE repos SET ${columns.map(column => `${column.name} = ${value(column)}`).join(', ')} WHERE owner = 'facebook'`) + const [before] = rows>(`SELECT * FROM repos WHERE owner = 'facebook'`) + + await moveRepository(d1.db, move) + + const [after] = rows>(`SELECT * FROM repos`) + expect(after).toEqual({ + ...before, + owner: 'react', + repo: 'react', + source_owner: 'react', + source_repo: 'react', + repository_id: REACT_ID, + }) + }) + + it('refuses a move to the same name', () => { + expect(() => repositoryMoveSql({ ...move, to: { owner: 'Facebook', repo: 'React' } })).toThrow('cannot move to its own name') + }) +}) diff --git a/test/unit/skill-page-url.test.ts b/test/unit/skill-page-url.test.ts index d43e92b8..8dc0529a 100644 --- a/test/unit/skill-page-url.test.ts +++ b/test/unit/skill-page-url.test.ts @@ -1,5 +1,6 @@ import type { SqliteD1 } from './helpers/d1-sqlite' // @vitest-environment node +import { readFileSync } from 'node:fs' import { IncomingMessage, ServerResponse } from 'node:http' import { Socket } from 'node:net' import { createEvent, fetchWithEvent, getResponseHeader } from 'h3' @@ -16,6 +17,8 @@ function db() { owner TEXT, repo TEXT, name TEXT, rendered_skill_path TEXT, source_resolved INTEGER DEFAULT 1 )`) + harness.raw.exec('CREATE TABLE repos (owner TEXT, repo TEXT)') + harness.raw.exec(readFileSync('migrations/0146_repository_moves.sql', 'utf8')) } return harness } diff --git a/test/unit/skill-run-identity.test.ts b/test/unit/skill-run-identity.test.ts index d1b18dd9..7ee05b02 100644 --- a/test/unit/skill-run-identity.test.ts +++ b/test/unit/skill-run-identity.test.ts @@ -1,6 +1,7 @@ // @vitest-environment node import type { SourceRequest } from '../../layers/artifact-delivery/server/schemas/contracts' import type { SqliteD1 } from './helpers/d1-sqlite' +import { readFileSync } from 'node:fs' import { afterEach, describe, expect, it, vi } from 'vitest' import { admittedSourceRequest } from '../../layers/artifact-delivery/server/utils/admitted-identity' import { findSkillRunIdentity } from '../../layers/registry/server/utils/skill-run-identity' @@ -20,6 +21,8 @@ function db() { CREATE TABLE skill_revisions ( owner TEXT, repo TEXT, name TEXT, sha TEXT, modified_at INTEGER )`) + harness.raw.exec('CREATE TABLE repos (owner TEXT, repo TEXT)') + harness.raw.exec(readFileSync('migrations/0146_repository_moves.sql', 'utf8')) } return harness } diff --git a/test/unit/sync-prefetch.test.ts b/test/unit/sync-prefetch.test.ts index a8d0f640..07d198e6 100644 --- a/test/unit/sync-prefetch.test.ts +++ b/test/unit/sync-prefetch.test.ts @@ -13,6 +13,7 @@ const heads: Record = { 'acme/fresh': 'tree-fresh', 'acme/busy': 'tree-busy', 'acme/discovered': 'tree-discovered', + 'acme/renamed': 'tree-same', } function graphqlAnswer(_input: unknown, init?: RequestInit): Response { @@ -30,6 +31,7 @@ function graphqlAnswer(_input: unknown, init?: RequestInit): Response { } const [owner, name] = fullName.split('/') as [string, string] data[`r${index}`] = { + databaseId: 4242, name, nameWithOwner: fullName, url: `https://github.com/${fullName}`, @@ -97,6 +99,22 @@ describe('prefetchUnchangedRepos', () => { .toEqual({ repo_meta_synced_at: null }) }) + it('queues a Repository GitHub moved, so its sync job can move the rows', async () => { + harness.raw.exec(` + INSERT INTO repos (owner, repo, last_tree_sha, pushed_at, stars, source_owner, source_repo) + VALUES ('acme', 'old-name', 'tree-same', 100, 1, 'acme', 'renamed'); + INSERT INTO skills (owner, repo, name, display_name, slug) VALUES ('acme', 'old-name', 'one', 'One', 'one'); + `) + vi.stubGlobal('fetch', vi.fn(graphqlAnswer)) + + const result = await prefetchUnchangedRepos({ db: harness.db, bindings, now: NOW }, [candidate('old-name')]) + + expect(result).toMatchObject({ _tag: 'prefetched', unchanged: 0 }) + expect(result.queue.map(item => `${item.owner}/${item.repo}`)).toEqual(['acme/old-name']) + expect(harness.raw.prepare(`SELECT repo_meta_synced_at FROM repos WHERE owner = 'acme' AND repo = 'old-name'`).get()) + .toEqual({ repo_meta_synced_at: null }) + }) + it('queues every candidate when GitHub does not answer the batch', async () => { vi.stubGlobal('fetch', vi.fn(async () => new Response('bad gateway', { status: 502 }))) diff --git a/test/unit/sync-repo-correctness.test.ts b/test/unit/sync-repo-correctness.test.ts index c0681727..23cdc3ec 100644 --- a/test/unit/sync-repo-correctness.test.ts +++ b/test/unit/sync-repo-correctness.test.ts @@ -24,6 +24,7 @@ function repoSummary(headTreeSha: string | null = 'new-tree') { return { status: 200, data: { + repositoryId: 1, headTreeSha, meta: { name: 'skills', @@ -109,75 +110,6 @@ describe('syncRepo content acknowledgement', () => { expect(sqlite.prepare(`SELECT last_tree_sha FROM repos`).pluck().get()).toBe('new-tree') }) - it('freezes registry identity while fetching a renamed repository by its canonical source', async () => { - github.getRepoSummary.mockResolvedValue(repoSummary()) - github.getRepoSummary.mockResolvedValueOnce({ - ...repoSummary(), - data: { - ...repoSummary().data, - meta: { - ...repoSummary().data.meta, - name: 'openclaw', - full_name: 'openclaw/openclaw', - html_url: 'https://github.com/openclaw/openclaw', - owner: { login: 'openclaw' }, - }, - }, - }) - github.getTree.mockResolvedValue(tree([{ path: 'skills/one/SKILL.md', sha: 'one-new' }])) - github.getBlobsBatch.mockResolvedValue({ - status: 200, - data: new Map([['skills/one/SKILL.md', rawSkill('One')]]), - unreadable: new Set(), - rateLimit: null, - notModified: false, - }) - - const result = await syncRepo('steipete', 'clawdis', {}, db, { ownerVerified: true }) - - expect(result).toMatchObject({ owner: 'steipete', repo: 'clawdis', status: 'indexed' }) - expect(github.getTree).toHaveBeenCalledWith('openclaw', 'openclaw', 'new-tree', {}) - expect(github.getBlobsBatch).toHaveBeenCalledWith( - 'openclaw', - 'openclaw', - 'main', - ['skills/one/SKILL.md'], - {}, - ) - expect(sqlite.prepare(` - SELECT owner, repo, source_owner, source_repo - FROM repos - `).get()).toEqual({ - owner: 'steipete', - repo: 'clawdis', - source_owner: 'openclaw', - source_repo: 'openclaw', - }) - expect(sqlite.prepare(`SELECT owner, repo FROM skills`).get()).toEqual({ - owner: 'steipete', - repo: 'clawdis', - }) - - vi.clearAllMocks() - github.getRepoSummary.mockResolvedValueOnce({ - ...repoSummary('new-tree'), - data: { - ...repoSummary('new-tree').data, - meta: { - ...repoSummary('new-tree').data.meta, - name: 'openclaw', - full_name: 'openclaw/openclaw', - html_url: 'https://github.com/openclaw/openclaw', - owner: { login: 'openclaw' }, - }, - }, - }) - - await syncRepo('steipete', 'clawdis', {}, db) - - expect(github.getRepoSummary).toHaveBeenCalledWith('openclaw', 'openclaw', {}) - }) - it('does not advance last_tree_sha when the blob batch fails', async () => { insertRepo(sqlite, 'old-tree') insertSkill(sqlite, 'one', 'one-old') @@ -784,7 +716,7 @@ function createDatabase(): Database.Database { forks INTEGER NOT NULL DEFAULT 0, pushed_at INTEGER, repo_created_at INTEGER, repo_meta_synced_at INTEGER, description TEXT, last_tree_sha TEXT, repo_kind TEXT NOT NULL DEFAULT 'creator', repo_kind_source TEXT NOT NULL DEFAULT 'computed', repo_skill_count INTEGER NOT NULL DEFAULT 0, - broken_since INTEGER, tree_truncated_at INTEGER, source_owner TEXT, source_repo TEXT, PRIMARY KEY (owner, repo) + broken_since INTEGER, tree_truncated_at INTEGER, source_owner TEXT, source_repo TEXT, repository_id INTEGER, PRIMARY KEY (owner, repo) ); CREATE TABLE skills ( name TEXT NOT NULL, owner TEXT NOT NULL, repo TEXT NOT NULL, display_name TEXT NOT NULL, diff --git a/test/unit/sync-repo-freshness.test.ts b/test/unit/sync-repo-freshness.test.ts index 8771d110..1a85e6e9 100644 --- a/test/unit/sync-repo-freshness.test.ts +++ b/test/unit/sync-repo-freshness.test.ts @@ -16,6 +16,7 @@ function repoSummary(headTreeSha: string | null = 'same-tree', stars = 42) { return { status: 200, data: { + repositoryId: 1, headTreeSha, meta: { name: 'skills', @@ -84,6 +85,7 @@ describe('syncRepo freshness cursor', () => { tree_truncated_at INTEGER, source_owner TEXT, source_repo TEXT, + repository_id INTEGER, PRIMARY KEY (owner, repo) ); INSERT INTO repos ( diff --git a/test/unit/sync-repo-streaming.test.ts b/test/unit/sync-repo-streaming.test.ts index 836984c2..1b2ee452 100644 --- a/test/unit/sync-repo-streaming.test.ts +++ b/test/unit/sync-repo-streaming.test.ts @@ -21,6 +21,7 @@ function repoSummary() { return { status: 200, data: { + repositoryId: 1, headTreeSha: 'new-tree', meta: { name: 'skills', @@ -284,7 +285,7 @@ function createDatabase(): Database.Database { forks INTEGER NOT NULL DEFAULT 0, pushed_at INTEGER, repo_created_at INTEGER, repo_meta_synced_at INTEGER, description TEXT, last_tree_sha TEXT, repo_kind TEXT NOT NULL DEFAULT 'creator', repo_kind_source TEXT NOT NULL DEFAULT 'computed', repo_skill_count INTEGER NOT NULL DEFAULT 0, - broken_since INTEGER, tree_truncated_at INTEGER, source_owner TEXT, source_repo TEXT, PRIMARY KEY (owner, repo) + broken_since INTEGER, tree_truncated_at INTEGER, source_owner TEXT, source_repo TEXT, repository_id INTEGER, PRIMARY KEY (owner, repo) ); CREATE TABLE skills ( name TEXT NOT NULL, owner TEXT NOT NULL, repo TEXT NOT NULL, display_name TEXT NOT NULL, diff --git a/test/unit/sync-repo-unreadable-blob.test.ts b/test/unit/sync-repo-unreadable-blob.test.ts index 4ea0156f..d89f1fd2 100644 --- a/test/unit/sync-repo-unreadable-blob.test.ts +++ b/test/unit/sync-repo-unreadable-blob.test.ts @@ -37,6 +37,7 @@ function repoSummary(headTreeSha: string | null = 'new-tree') { return { status: 200, data: { + repositoryId: 1, headTreeSha, meta: { name: 'skills', @@ -166,7 +167,7 @@ function createDatabase(): Database.Database { forks INTEGER NOT NULL DEFAULT 0, pushed_at INTEGER, repo_created_at INTEGER, repo_meta_synced_at INTEGER, description TEXT, last_tree_sha TEXT, repo_kind TEXT NOT NULL DEFAULT 'creator', repo_kind_source TEXT NOT NULL DEFAULT 'computed', repo_skill_count INTEGER NOT NULL DEFAULT 0, - broken_since INTEGER, tree_truncated_at INTEGER, source_owner TEXT, source_repo TEXT, PRIMARY KEY (owner, repo) + broken_since INTEGER, tree_truncated_at INTEGER, source_owner TEXT, source_repo TEXT, repository_id INTEGER, PRIMARY KEY (owner, repo) ); CREATE TABLE skills ( name TEXT NOT NULL, owner TEXT NOT NULL, repo TEXT NOT NULL, display_name TEXT NOT NULL,