diff --git a/layers/registry/server/api/skill-files/[...slug].get.ts b/layers/registry/server/api/skill-files/[...slug].get.ts index bdc48b16..8f0596b0 100644 --- a/layers/registry/server/api/skill-files/[...slug].get.ts +++ b/layers/registry/server/api/skill-files/[...slug].get.ts @@ -1,123 +1,11 @@ -import type { ReadThroughWindows } from '#shared/server/cache' -import { readThroughCache } from '#shared/server/cache' import { defineApiHandler } from '#shared/server/handler' -import { selectSkillFiles } from '#shared/skill-files' -import { resolveRepoSourceIdentityFromRow } from '../../utils/repo-source-identity' +import { loadStoredSkillRow, readStoredSkillFiles } from '../../utils/skill-stored-source' import { findSkill } from '../../utils/skills-registry' -import { fetchUpstreamTree } from '../../utils/upstream-tree' - -const FILES_CACHE_TTL = 60 * 60 * 6 -// How long past its fresh window a files payload stays servable when its live -// recompute fails. The file list only changes when the repo does, so during a -// cold-key crawler sweep (SKILLD-1F) serving a day-old list beats another -// upstream tree fetch per request. -const FILES_CACHE_STALE_TTL = 60 * 60 * 24 -// An empty file list is only transient when the skill directory was never -// resolved. `skillPath` carries the SKILL.md path found in the upstream tree: -// the stored `rendered_skill_path` when the tree still holds it, else the -// heuristic hit. It is null exactly when no SKILL.md resolved (empty tree, -// moved file, heuristic miss), so those take the short missing window. A -// resolved skill holding only SKILL.md also produces files: [] (the filter -// excludes SKILL.md itself), but its non-null skillPath keeps it on the -// 6-hour fresh window with the day-long stale serve. -const FILES_MISSING_TTL = 60 * 5 -// v4: entries carry a freshness envelope for readThroughCache, so v3 values -// (raw payloads) must never be read as envelopes. -const FILES_CACHE_VERSION = 'v4' -const FILES_RETRY_AFTER = 30 - -interface SkillFile { - path: string - size: number - type: 'markdown' | 'code' | 'image' | 'data' | 'other' -} - -interface SkillFilesPayload { - skillPath: string | null - branch: string - files: SkillFile[] - total: number -} - -interface SkillFilesRow { - default_branch: string | null - rendered_skill_path: string | null - source_owner: string | null - source_repo: string | null - source_resolved: number | null -} - -function classify(path: string): SkillFile['type'] { - const ext = path.toLowerCase().split('.').pop() ?? '' - if (ext === 'md' || ext === 'markdown') - return 'markdown' - if (['png', 'jpg', 'jpeg', 'gif', 'webp', 'svg'].includes(ext)) - return 'image' - if (['json', 'yaml', 'yml', 'toml', 'csv'].includes(ext)) - return 'data' - if (ext) - return 'code' - return 'other' -} - -// Upstream repos spell SKILL.md in any case (skill.md, Skill.MD). Every path -// test goes through these two helpers, so a match and the directory derived -// from it can never disagree on case. -function isSkillMd(path: string): boolean { - return /(?:^|\/)skill\.md$/i.test(path) -} - -function skillDirOf(skillMdPath: string): string { - return skillMdPath.replace(/(?:^|\/)skill\.md$/i, '') -} - -/** - * The SKILL.md path this skill resolves to in the upstream tree, or null when - * nothing does. The stored `rendered_skill_path` counts only when the tree - * still holds it; a moved file falls through to the name heuristic. Returning - * the tree's own entry means a non-null result always names a real file, so an - * empty file list with a non-null skillPath is a genuinely SKILL.md-only - * directory, never a failed resolution. - */ -function resolveSkillPath(tree: readonly { path: string }[], stored: string | null, name: string): string | null { - if (stored) { - const wanted = stored.toLowerCase() - const hit = tree.find(f => f.path.toLowerCase() === wanted) - if (hit) - return hit.path - } - const slugifiedName = name.toLowerCase().replace(/[^a-z0-9]+/g, '-').replace(/^-+|-+$/g, '') - const lowerName = name.toLowerCase() - return tree.find((f) => { - const path = f.path.toLowerCase() - return path.endsWith(`/${slugifiedName}/skill.md`) - || path === `${slugifiedName}/skill.md` - || path.endsWith(`/${lowerName}/skill.md`) - })?.path ?? null -} /** - * The boundary parse for stored cache bytes. A v4 entry whose value is not a - * SkillFilesPayload (corrupt write, foreign format) fails this guard and is - * treated as a miss, so `emptyFilesWindows` never dereferences a shape - * nobody validated. + * The file explorer's list, read from the `assets` the sync stored. No GitHub + * call happens on this path, so the list is as fresh as the last sync. */ -function isSkillFilesPayload(value: unknown): value is SkillFilesPayload { - if (typeof value !== 'object' || value === null) - return false - const payload = value as Record - return (payload.skillPath === null || typeof payload.skillPath === 'string') - && typeof payload.branch === 'string' - && Array.isArray(payload.files) - && typeof payload.total === 'number' -} - -function emptyFilesWindows(payload: SkillFilesPayload): ReadThroughWindows | undefined { - return payload.files.length === 0 && payload.skillPath === null - ? { ttl: FILES_MISSING_TTL, staleTtl: 0 } - : undefined -} - export default defineApiHandler({ handler: async ({ event, platform }) => { const slugParam = getRouterParam(event, 'slug') @@ -132,87 +20,19 @@ export default defineApiHandler({ if (!owner || !repo || !name) throw createError({ statusCode: 400, message: 'Missing path components' }) - const skillSlug = `${owner}/${repo}/${name}` - const skill = await findSkill(event, skillSlug) + const skill = await findSkill(event, `${owner}/${repo}/${name}`) if (!skill) throw createError({ statusCode: 404, message: 'Skill not found' }) - const row = await platform.db - .prepare(`SELECT r.default_branch, r.source_owner, r.source_repo, s.rendered_skill_path, s.source_resolved - FROM skills s JOIN repos r ON r.owner = s.owner AND r.repo = s.repo - WHERE s.owner = ? AND s.repo = ? AND s.name = ?`) - .bind(skill.owner, skill.repo, skill.name) - .first() + const row = await loadStoredSkillRow(platform.db, skill) if (!row) throw createError({ statusCode: 404, message: 'Skill metadata missing' }) // The sync's verdict that this SKILL.md is gone upstream. The page serves // a 410 tombstone on the same verdict, and this endpoint must agree. - // Answer before any upstream call: the registry already knows (SKILLD-11). - if (row.source_resolved === 0) + const stored = readStoredSkillFiles(row) + if (stored._tag === 'gone') throw createError({ statusCode: 410, message: 'Skill source is gone upstream' }) - - const source = resolveRepoSourceIdentityFromRow(skill, row) - const branch = row.default_branch || 'main' - const cacheKey = `skills:files:${FILES_CACHE_VERSION}:${source.owner}/${source.repo}/${skill.name}:${branch}` - - // retry-after is an instruction to re-poll. A failed recompute is - // absorbed by a stale envelope, so the 503 only escapes when nothing - // servable is left; only that escape may carry the header. A 200 served - // from cache must never tell an agent to come back in 30 seconds. - try { - return await readThroughCache( - useStorage('edge-cache'), - cacheKey, - async () => { - const treeResult = await fetchUpstreamTree(source, branch, { operation: 'skill-files-tree-fetch' }) - - if (treeResult._tag === 'gone') { - // The registry has not recorded this deletion yet. The next sync flips - // `source_resolved` and short-circuits earlier. - throw createError({ statusCode: 410, message: 'Skill source is gone upstream' }) - } - - if (treeResult._tag === 'unavailable') - throw createError({ statusCode: 503, message: 'Skill source is unavailable upstream' }) - - const tree = treeResult.files - const skillPath = resolveSkillPath(tree, row.rendered_skill_path, skill.name) - - if (skillPath === null) { - return { skillPath, branch, files: [], total: 0 } satisfies SkillFilesPayload - } - - const skillDir = skillDirOf(skillPath) - const prefix = skillDir ? `${skillDir}/` : '' - const files: SkillFile[] = tree - .filter(f => f.path.startsWith(prefix) && !isSkillMd(f.path)) - .map(f => ({ - path: f.path.slice(prefix.length), - size: f.size ?? 0, - type: classify(f.path), - })) - - const selected = selectSkillFiles(files) - return { - skillPath, - branch, - files: selected.files, - total: selected.total, - } satisfies SkillFilesPayload - }, - { - ttl: FILES_CACHE_TTL, - staleTtl: FILES_CACHE_STALE_TTL, - windowsFor: emptyFilesWindows, - validate: isSkillFilesPayload, - }, - ) - } - catch (error) { - if ((error as { statusCode?: number } | null)?.statusCode === 503) - setHeader(event, 'retry-after', FILES_RETRY_AFTER) - throw error - } + return stored.payload }, }) diff --git a/layers/registry/server/api/skills-raw/[...slug].get.ts b/layers/registry/server/api/skills-raw/[...slug].get.ts index 708f2d11..15c53306 100644 --- a/layers/registry/server/api/skills-raw/[...slug].get.ts +++ b/layers/registry/server/api/skills-raw/[...slug].get.ts @@ -1,54 +1,27 @@ -import { readCache, writeCache } from '#shared/server/cache' +import type { ReadThroughCache } from '#shared/server/cache' +import { readThroughCache } from '#shared/server/cache' import { defineApiHandler } from '#shared/server/handler' import { normalizeSkillAssetFilePath } from '#shared/skill-asset-path' -import { resolveRepoSourceIdentityFromRow } from '../../utils/repo-source-identity' +import { loadStoredSkillRow, readReferencedFile, readStoredSkillMd, resolveReferencedFileTarget } from '../../utils/skill-stored-source' import { findSkill } from '../../utils/skills-registry' import { fetchUpstreamText } from '../../utils/upstream-text' -import { fetchUpstreamTree } from '../../utils/upstream-tree' -const RAW_CACHE_TTL = 60 * 5 -// The cache entry outlives the fresh window by this much, so a blip after -// expiry still has a last-good body to fall back to instead of a 503. -const RAW_STALE_TTL = 60 * 60 -const RAW_MISSING_TTL = 60 +// SKILL.md comes from D1, so this only bounds browser and edge copies. The +// referenced-file path reads GitHub once per cache miss and keeps a last good +// body for an hour, so a blip after expiry does not surface as a 503. +const REFERENCED_FILE_TTL = 60 * 5 +const REFERENCED_FILE_STALE_TTL = 60 * 60 const RAW_RETRY_AFTER = 30 -interface RawCache { - status: 'ok' | 'missing' - body: string | null - branch: string | null - path: string | null +interface ReferencedFile { + body: string + source: string } -/** - * The freshness envelope `readThroughCache` stores, read here by hand rather - * than through the helper: 404 and 410 must propagate even in the stale - * window, so only an upstream outage may fall back to the stale value. - */ -interface RawCacheEnvelope { - storedAt: number - value: RawCache -} - -function parseLastGood(raw: unknown): { value: RawCache, ageSeconds: number } | null { - if (typeof raw !== 'object' || raw === null || !('storedAt' in raw) || !('value' in raw)) - return null - const { storedAt, value } = raw as Record - if (typeof storedAt !== 'number' || !Number.isFinite(storedAt)) - return null - if (typeof value !== 'object' || value === null) - return null - const entry = value as RawCache - if (entry.status !== 'ok' || typeof entry.body !== 'string') - return null - return { value: entry, ageSeconds: (Date.now() - storedAt) / 1000 } -} - -interface SkillSourceRow { - default_branch: string | null - source_owner: string | null - source_repo: string | null - source_resolved: number | null +function isReferencedFile(value: unknown): value is ReferencedFile { + return typeof value === 'object' && value !== null + && typeof (value as ReferencedFile).body === 'string' + && typeof (value as ReferencedFile).source === 'string' } export default defineApiHandler({ @@ -79,123 +52,59 @@ export default defineApiHandler({ if (!skill) throw createError({ statusCode: 404, message: 'Skill not found' }) - const sourceRow = await platform.db - .prepare(` - SELECT r.default_branch, r.source_owner, r.source_repo, s.source_resolved - FROM skills s JOIN repos r ON r.owner = s.owner AND r.repo = s.repo - WHERE s.owner = ? AND s.repo = ? AND s.name = ? - `) - .bind(skill.owner, skill.repo, skill.name) - .first() + const row = await loadStoredSkillRow(platform.db, skill) + if (!row) + throw createError({ statusCode: 404, message: 'Skill not found' }) // The sync's verdict that this SKILL.md is gone upstream. The page serves - // a 410 tombstone on the same verdict, and this endpoint must agree. A - // gone source is permanent, so it must not read as an outage. Answer - // before any upstream call: the registry already knows (SKILLD-11). - if (sourceRow?.source_resolved === 0) + // a 410 tombstone on the same verdict, and this endpoint must agree. + if (row.source_resolved === 0) throw createError({ statusCode: 410, message: 'Skill source is gone upstream' }) - const source = resolveRepoSourceIdentityFromRow(skill, sourceRow ?? undefined) - const branch = sourceRow?.default_branch || 'main' - const cacheKey = filePath - ? `skills:raw:v3:${source.owner}/${source.repo}/${skill.name}:${filePath}` - : `skills:raw:v3:${source.owner}/${source.repo}/${skill.name}` - const lastGood = parseLastGood(await readCache(useStorage('edge-cache'), cacheKey)) - if (lastGood && lastGood.ageSeconds < RAW_CACHE_TTL) { + if (!filePath) { + const stored = readStoredSkillMd(skill, row) + if (stored._tag === 'gone') + throw createError({ statusCode: 410, message: 'Skill source is gone upstream' }) + if (stored._tag === 'missing') + throw createError({ statusCode: 404, message: 'SKILL.md not found in repository' }) setHeader(event, 'content-type', 'text/markdown; charset=utf-8') setHeader(event, 'cache-control', 'public, max-age=300') - setHeader(event, 'x-skilld-source', `${source.owner}/${source.repo}@${lastGood.value.branch}/${lastGood.value.path}`) - return lastGood.value.body + setHeader(event, 'x-skilld-source', stored.source) + return stored.body } - // An upstream outage must not reach the run surface while a last-good - // copy is still readable. A gone or missing verdict still propagates: - // those are facts about the source, not about its availability. - const serveStale = (stage: string) => { - if (!lastGood || lastGood.ageSeconds >= RAW_CACHE_TTL + RAW_STALE_TTL) - return null - emitOperationalEvent(createWideEvent({ - 'operation': 'skill-raw-stale-fallback', - 'outcome': 'degraded', - 'cache.servedStale': true, - 'cache.ageSeconds': Math.round(lastGood.ageSeconds), - 'reason': stage, - })) + // A file beside SKILL.md (e.g. `references/foo.md`) is not stored, so it + // is the one read that still reaches GitHub. Its directory comes from D1. + const target = resolveReferencedFileTarget(skill, row, filePath) + if (target._tag === 'missing') + throw createError({ statusCode: 404, message: 'Referenced file not found in repository' }) + + const cacheKey = `skills:raw:v4:${target.url}` + try { + const file = await readThroughCache( + useStorage('edge-cache') as ReadThroughCache, + cacheKey, + async () => { + const raw = await readReferencedFile(target, fetchUpstreamText) + if (raw._tag === 'missing') + throw createError({ statusCode: 404, message: 'Referenced file not found in repository' }) + if (raw._tag === 'unavailable') { + emitOperationalEvent(createWideEvent({ 'operation': 'skill-raw-content-fetch', 'outcome': 'failed', 'upstream.status': raw.status ?? 0, 'attempt': raw.attempts })) + throw createError({ statusCode: 503, message: 'Referenced file source is unavailable upstream' }) + } + return { body: raw.body, source: target.source } satisfies ReferencedFile + }, + { ttl: REFERENCED_FILE_TTL, staleTtl: REFERENCED_FILE_STALE_TTL, validate: isReferencedFile }, + ) setHeader(event, 'content-type', 'text/markdown; charset=utf-8') - setHeader(event, 'cache-control', 'public, max-age=30') - setHeader(event, 'x-skilld-source', `${source.owner}/${source.repo}@${lastGood.value.branch}/${lastGood.value.path}`) - return lastGood.value.body - } - - const treeResult = await fetchUpstreamTree(source, branch, { operation: 'skill-raw-tree-fetch' }) - - if (treeResult._tag === 'gone') { - // The registry has not recorded this deletion yet. The next sync flips - // `source_resolved` and short-circuits earlier. Until then, the missing - // marker keeps repeat callers off the upstream 404. - await writeCache(useStorage('edge-cache'), cacheKey, { status: 'missing', body: null, branch, path: null } satisfies RawCache, { ttl: RAW_MISSING_TTL }) - throw createError({ statusCode: 410, message: 'Skill source is gone upstream' }) - } - - if (treeResult._tag === 'unavailable') { - const staleBody = serveStale('tree') - if (staleBody !== null) - return staleBody - setHeader(event, 'retry-after', RAW_RETRY_AFTER) - throw createError({ statusCode: 503, message: 'Skill source is unavailable upstream' }) - } - - const files = treeResult.files - const slugifiedName = skill.name.toLowerCase().replace(/[^a-z0-9]+/g, '-').replace(/^-+|-+$/g, '') - const skillPath = files.find(f => - f.path.toLowerCase().endsWith(`/${slugifiedName}/skill.md`) - || f.path.toLowerCase() === `${slugifiedName}/skill.md` - || f.path.toLowerCase().endsWith(`/${skill.name.toLowerCase()}/skill.md`), - )?.path - - if (!skillPath) { - await writeCache(useStorage('edge-cache'), cacheKey, { status: 'missing', body: null, branch, path: null } satisfies RawCache, { ttl: RAW_MISSING_TTL }) - throw createError({ statusCode: 404, message: 'SKILL.md not found in repository' }) - } - - // A bare skill slug serves SKILL.md itself; a trailing path serves a file - // beside it (e.g. `references/foo.md`), resolved relative to SKILL.md's - // own directory rather than the repo root. - const skillDir = skillPath.replace(/\/SKILL\.md$/, '') - const targetPath = filePath ? `${skillDir}/${filePath}` : skillPath - const notFoundMessage = filePath ? 'Referenced file not found in repository' : 'SKILL.md not found in repository' - - const rawUrl = `https://raw.githubusercontent.com/${source.owner}/${source.repo}/${branch}/${targetPath}` - const raw = await fetchUpstreamText(rawUrl) - - if (raw._tag === 'missing') { - emitOperationalEvent(createWideEvent({ 'operation': 'skill-raw-content-fetch', 'outcome': 'missing', 'upstream.status': raw.status })) - await writeCache(useStorage('edge-cache'), cacheKey, { status: 'missing', body: null, branch, path: targetPath } satisfies RawCache, { ttl: RAW_MISSING_TTL }) - throw createError({ statusCode: 404, message: notFoundMessage }) + setHeader(event, 'cache-control', 'public, max-age=300') + setHeader(event, 'x-skilld-source', file.source) + return file.body } - - if (raw._tag === 'unavailable') { - // A GitHub outage must not leave a "missing" marker behind, or the - // document reads as deleted for the rest of the cache window. - emitOperationalEvent(createWideEvent({ 'operation': 'skill-raw-content-fetch', 'outcome': 'failed', 'upstream.status': raw.status ?? 0, 'attempt': raw.attempts })) - const staleBody = serveStale('content') - if (staleBody !== null) - return staleBody - setHeader(event, 'retry-after', RAW_RETRY_AFTER) - throw createError({ statusCode: 503, message: 'SKILL.md source is unavailable upstream' }) + catch (error) { + if ((error as { statusCode?: number } | null)?.statusCode === 503) + setHeader(event, 'retry-after', RAW_RETRY_AFTER) + throw error } - - const body = raw.body - await writeCache( - useStorage('edge-cache'), - cacheKey, - { storedAt: Date.now(), value: { status: 'ok', body, branch, path: targetPath } } satisfies RawCacheEnvelope, - { ttl: RAW_CACHE_TTL + RAW_STALE_TTL }, - ) - - setHeader(event, 'content-type', 'text/markdown; charset=utf-8') - setHeader(event, 'cache-control', 'public, max-age=300') - setHeader(event, 'x-skilld-source', `${source.owner}/${source.repo}@${branch}/${targetPath}`) - return body }, }) diff --git a/layers/registry/server/utils/skill-stored-source.ts b/layers/registry/server/utils/skill-stored-source.ts new file mode 100644 index 00000000..29de3122 --- /dev/null +++ b/layers/registry/server/utils/skill-stored-source.ts @@ -0,0 +1,175 @@ +import type { RepoIdentity } from './repo-source-identity' +import type { UpstreamText } from './upstream-text' +import { selectSkillFiles } from '#shared/skill-files' +import { resolveRepoSourceIdentityFromRow } from './repo-source-identity' + +/** + * What the sync stored for one Skill. The request path for `skills-raw` and + * `skill-files` reads this and nothing else, so a GitHub outage cannot reach + * the run surface or the file explorer. Freshness is "as of the last sync". + */ +export interface StoredSkillRow { + default_branch: string | null + source_owner: string | null + source_repo: string | null + source_resolved: number | null + rendered_status: string | null + rendered_raw: string | null + rendered_skill_path: string | null + /** JSON array of `{ path, size, type }`, written by the sync. */ + assets: string | null +} + +export type SkillFileType = 'markdown' | 'code' | 'image' | 'data' | 'other' + +export interface SkillFile { + path: string + size: number + type: SkillFileType +} + +export interface SkillFilesPayload { + skillPath: string | null + branch: string + files: SkillFile[] + total: number +} + +export async function loadStoredSkillRow( + db: D1Database, + skill: RepoIdentity & { name: string }, +): Promise { + return db + .prepare(` + SELECT r.default_branch, r.source_owner, r.source_repo, + s.source_resolved, s.rendered_status, s.rendered_raw, s.rendered_skill_path, s.assets + FROM skills s JOIN repos r ON r.owner = s.owner AND r.repo = s.repo + WHERE s.owner = ? AND s.repo = ? AND s.name = ? + `) + .bind(skill.owner, skill.repo, skill.name) + .first() +} + +export type StoredSkillMd + = | { _tag: 'ok', body: string, source: string } + | { _tag: 'gone' } + | { _tag: 'missing' } + +/** + * The SKILL.md as of the last sync. A gone verdict wins over a stored body: + * the page serves a 410 tombstone on the same verdict. + */ +export function readStoredSkillMd(skill: RepoIdentity, row: StoredSkillRow): StoredSkillMd { + if (row.source_resolved === 0) + return { _tag: 'gone' } + if (row.rendered_status !== 'ok' || !row.rendered_raw || !row.rendered_skill_path) + return { _tag: 'missing' } + const source = resolveRepoSourceIdentityFromRow(skill, row) + return { + _tag: 'ok', + body: row.rendered_raw, + source: `${source.owner}/${source.repo}@${row.default_branch || 'main'}/${row.rendered_skill_path}`, + } +} + +function isSkillFileType(value: unknown): value is SkillFileType { + return value === 'markdown' || value === 'code' || value === 'image' || value === 'data' || value === 'other' +} + +/** + * Parse the sync's `assets` column once. A malformed entry is dropped; a + * malformed column is an empty list, because the sync rewrites it on change. + */ +export function parseStoredAssets(assets: string | null): SkillFile[] { + if (!assets) + return [] + let parsed: unknown + try { + parsed = JSON.parse(assets) + } + catch { + // A corrupt column reads as no files; the next sync rewrites it. + return [] + } + if (!Array.isArray(parsed)) + return [] + return parsed.flatMap((entry: unknown): SkillFile[] => { + if (typeof entry !== 'object' || entry === null) + return [] + const { path, size, type } = entry as Record + if (typeof path !== 'string') + return [] + return [{ + path, + size: typeof size === 'number' ? size : 0, + type: isSkillFileType(type) ? type : 'other', + }] + }) +} + +export type StoredSkillFiles + = | { _tag: 'ok', payload: SkillFilesPayload } + | { _tag: 'gone' } + +/** + * The file list as of the last sync. A Skill whose SKILL.md sits at the + * repository root has no stored assets, so its list is empty but its + * `skillPath` is set. A Skill with no resolved SKILL.md has `skillPath: null`. + */ +export function readStoredSkillFiles(row: StoredSkillRow): StoredSkillFiles { + if (row.source_resolved === 0) + return { _tag: 'gone' } + const branch = row.default_branch || 'main' + if (row.rendered_status !== 'ok' || !row.rendered_skill_path) + return { _tag: 'ok', payload: { skillPath: null, branch, files: [], total: 0 } } + const selected = selectSkillFiles(parseStoredAssets(row.assets)) + return { + _tag: 'ok', + payload: { skillPath: row.rendered_skill_path, branch, files: selected.files, total: selected.total }, + } +} + +export type ReferencedFileTarget + = | { _tag: 'ok', url: string, path: string, source: string } + | { _tag: 'missing' } + +/** + * Where a file beside SKILL.md lives upstream. Only the file body needs + * GitHub: its directory comes from the stored SKILL.md path, so the ungh tree + * read is gone. + */ +export function resolveReferencedFileTarget( + skill: RepoIdentity, + row: StoredSkillRow, + filePath: string, +): ReferencedFileTarget { + if (!row.rendered_skill_path) + return { _tag: 'missing' } + const source = resolveRepoSourceIdentityFromRow(skill, row) + const branch = row.default_branch || 'main' + const skillDir = row.rendered_skill_path.replace(/(?:^|\/)skill\.md$/i, '') + const path = skillDir ? `${skillDir}/${filePath}` : filePath + return { + _tag: 'ok', + path, + source: `${source.owner}/${source.repo}@${branch}/${path}`, + url: `https://raw.githubusercontent.com/${source.owner}/${source.repo}/${branch}/${path}`, + } +} + +export type ReferencedFileRead + = | { _tag: 'ok', body: string } + | { _tag: 'missing' } + | { _tag: 'unavailable', status: number | null, attempts: number } + +/** + * Read one referenced file. The text reader is a dependency, so a test proves + * which reads happen and the handler passes the real one. + */ +export async function readReferencedFile( + target: Extract, + fetchText: (url: string) => Promise, +): Promise { + const raw = await fetchText(target.url) + return raw._tag === 'ok' ? { _tag: 'ok', body: raw.body } : raw +} diff --git a/nuxt.config.ts b/nuxt.config.ts index 3079d38d..d713d338 100644 --- a/nuxt.config.ts +++ b/nuxt.config.ts @@ -513,6 +513,9 @@ export default defineNuxtConfig({ 'cloudflare-cdn-cache-control': 'public, max-age=300', }, } as any, + // The file list now comes from D1, identical for everyone and only + // changing on a sync, so the edge can hold it for a few minutes. + '/api/skill-files/**': edgeCache({ maxAge: 300, staleWhileRevalidate: 3600 }), '/api/skills/typeahead': { headers: { 'cache-control': 'public, max-age=3600', diff --git a/test/unit/skill-files-cache.test.ts b/test/unit/skill-files-cache.test.ts deleted file mode 100644 index 45cda8c8..00000000 --- a/test/unit/skill-files-cache.test.ts +++ /dev/null @@ -1,345 +0,0 @@ -import type { H3Event } from 'h3' -import type { SqliteD1 } from './helpers/d1-sqlite' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { SKILL_FILE_LIMIT } from '../../shared/skill-files' -import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' -import { SIGNED_IN_HEADERS } from './helpers/session' - -const HOUR = 60 * 60 -const CACHE_KEY = 'skills:files:v4:owner/repo/skill:main' - -let fixture: SqliteD1 -let cache: { - getItem: ReturnType - setItem: ReturnType -} - -beforeEach(() => { - vi.resetModules() - fixture = createSqliteD1(allMigrations()) - fixture.raw.prepare(`INSERT INTO repos (owner, repo) VALUES ('owner', 'repo')`).run() - fixture.raw.prepare( - `INSERT INTO skills (owner, repo, name, slug, display_name, source_resolved) - VALUES ('owner', 'repo', 'skill', 'owner/repo/skill', 'Skill', 1)`, - ).run() - cache = { - getItem: vi.fn().mockResolvedValue(null), - setItem: vi.fn().mockResolvedValue(undefined), - } - - vi.stubGlobal('defineEventHandler', (handler: unknown) => handler) - vi.stubGlobal('getRouterParam', () => 'owner/repo/skill') - vi.stubGlobal('getUserSession', () => Promise.resolve({ user: { id: 1, login: 'tester' } })) - vi.stubGlobal('useStorage', () => cache) - vi.stubGlobal('setHeader', vi.fn()) - vi.stubGlobal('createWideEvent', () => ({ context: {}, setLevel: vi.fn(), emit: vi.fn(() => null) })) - vi.stubGlobal('emitOperationalEvent', vi.fn()) -}) - -afterEach(() => { - fixture.close() - vi.unstubAllGlobals() -}) - -describe('skill-files cache', () => { - it('computes a cold miss and stores a freshness envelope', async () => { - vi.stubGlobal('$fetch', vi.fn().mockResolvedValue({ - files: [ - { path: 'skill/SKILL.md', size: 100 }, - { path: 'skill/assets/cover.png', size: 2048 }, - ], - })) - - const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default - const payload = await handler(event()) - - expect(payload).toEqual({ - skillPath: 'skill/SKILL.md', - branch: 'main', - files: [{ path: 'assets/cover.png', size: 2048, type: 'image' }], - total: 1, - }) - expect(cache.getItem).toHaveBeenCalledWith(CACHE_KEY) - expect(cache.setItem).toHaveBeenCalledWith( - CACHE_KEY, - { storedAt: expect.any(Number), value: payload }, - { ttl: HOUR * 6 + HOUR * 24 }, - ) - }) - - it('serves a fresh envelope without an upstream call', async () => { - const payload = { - skillPath: 'skill', - branch: 'main', - files: [{ path: 'assets/cover.png', size: 2048, type: 'image' as const }], - total: 1, - } - cache.getItem.mockResolvedValue({ storedAt: Date.now() - 1000, value: payload }) - const fetchMock = vi.fn() - vi.stubGlobal('$fetch', fetchMock) - - const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default - - await expect(handler(event())).resolves.toEqual(payload) - expect(fetchMock).not.toHaveBeenCalled() - expect(cache.setItem).not.toHaveBeenCalled() - }) - - it('recomputes when a stored value is not a SkillFilesPayload', async () => { - // Stored bytes are untrusted. A v4 entry whose value is corrupt or - // foreign must be a miss, not a TypeError on every read until the KV - // TTL expires the entry. - cache.getItem.mockResolvedValue({ storedAt: Date.now() - 1000, value: {} }) - vi.stubGlobal('$fetch', vi.fn().mockResolvedValue({ - files: [ - { path: 'skill/SKILL.md', size: 100 }, - { path: 'skill/assets/cover.png', size: 2048 }, - ], - })) - - const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default - - const payload = await handler(event()) - expect(payload).toEqual({ - skillPath: 'skill/SKILL.md', - branch: 'main', - files: [{ path: 'assets/cover.png', size: 2048, type: 'image' }], - total: 1, - }) - expect(cache.setItem).toHaveBeenCalledWith( - CACHE_KEY, - { storedAt: expect.any(Number), value: payload }, - { ttl: HOUR * 6 + HOUR * 24 }, - ) - }) - - it('recomputes a cached no-skillDir empty payload once the missing window expires', async () => { - // Root SKILL.md with rendered_skill_path NULL: the heuristic misses, so - // the first run caches the transient empty fallback. - const fetchMock = vi.fn() - .mockResolvedValueOnce({ files: [{ path: 'SKILL.md', size: 100 }] }) - .mockResolvedValueOnce({ - files: [ - { path: 'skill/SKILL.md', size: 100 }, - { path: 'skill/assets/cover.png', size: 2048 }, - ], - }) - vi.stubGlobal('$fetch', fetchMock) - - const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default - - const empty = await handler(event()) - expect(empty).toEqual({ skillPath: null, branch: 'main', files: [], total: 0 }) - - // reconcile-rendered re-renders exactly the rows this empty payload came - // from. Its envelope is now 10 minutes old, past the 5-minute missing - // window, so the next read must recompute instead of serving it. - fixture.raw.prepare( - `UPDATE skills SET rendered_skill_path = 'skill/SKILL.md' - WHERE owner = 'owner' AND repo = 'repo' AND name = 'skill'`, - ).run() - cache.getItem.mockResolvedValue({ storedAt: Date.now() - 10 * 60 * 1000, value: empty }) - - const payload = await handler(event()) - - expect(payload).toEqual({ - skillPath: 'skill/SKILL.md', - branch: 'main', - files: [{ path: 'assets/cover.png', size: 2048, type: 'image' }], - total: 1, - }) - expect(fetchMock).toHaveBeenCalledTimes(2) - }) - - it('recomputes a cached empty-tree payload once the missing window expires', async () => { - const fetchMock = vi.fn() - .mockResolvedValueOnce({ files: [] }) - .mockResolvedValueOnce({ - files: [ - { path: 'skill/SKILL.md', size: 100 }, - { path: 'skill/assets/cover.png', size: 2048 }, - ], - }) - vi.stubGlobal('$fetch', fetchMock) - - const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default - - const empty = await handler(event()) - expect(empty).toEqual({ skillPath: null, branch: 'main', files: [], total: 0 }) - - cache.getItem.mockResolvedValue({ storedAt: Date.now() - 10 * 60 * 1000, value: empty }) - - const payload = await handler(event()) - - expect(payload).toEqual({ - skillPath: 'skill/SKILL.md', - branch: 'main', - files: [{ path: 'assets/cover.png', size: 2048, type: 'image' }], - total: 1, - }) - expect(fetchMock).toHaveBeenCalledTimes(2) - }) - - it('keeps a resolved SKILL.md-only empty payload on the full fresh and stale windows', async () => { - // A skill directory holding only SKILL.md is fully resolved: the tree - // filter excludes SKILL.md itself, so files: [] with a non-null - // skillPath is the permanent correct answer, not a transient miss. - fixture.raw.prepare( - `UPDATE skills SET rendered_skill_path = 'skill/SKILL.md' - WHERE owner = 'owner' AND repo = 'repo' AND name = 'skill'`, - ).run() - - vi.stubGlobal('$fetch', vi.fn().mockResolvedValue({ files: [{ path: 'skill/SKILL.md', size: 100 }] })) - - const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default - - const resolved = await handler(event()) - expect(resolved).toEqual({ skillPath: 'skill/SKILL.md', branch: 'main', files: [], total: 0 }) - expect(cache.setItem).toHaveBeenCalledWith( - CACHE_KEY, - { storedAt: expect.any(Number), value: resolved }, - { ttl: HOUR * 6 + HOUR * 24 }, - ) - - // Seven hours later the entry is past even the resolved payload's - // 6-hour fresh window. An upstream failure must serve the stale - // envelope, not propagate. - cache.getItem.mockResolvedValue({ storedAt: Date.now() - 7 * HOUR * 1000, value: resolved }) - const error = Object.assign(new Error('404 Not Found'), { status: 404, statusCode: 404 }) - vi.stubGlobal('$fetch', vi.fn().mockRejectedValue(error)) - - await expect(handler(event())).resolves.toEqual(resolved) - }) - - it('caches a heuristic-resolved SKILL.md-only payload with the full fresh and stale windows', async () => { - // rendered_skill_path is NULL but the name heuristic finds skill/SKILL.md: - // the directory is resolved, so the empty file list (the tree filter - // excludes SKILL.md itself) is the permanent correct answer. The payload - // must carry the resolved path so it cannot be mistaken for the - // unresolved fallback, and must ride the 6-hour fresh window with the - // day-long stale serve. - vi.stubGlobal('$fetch', vi.fn().mockResolvedValue({ files: [{ path: 'skill/SKILL.md', size: 100 }] })) - - const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default - - const resolved = await handler(event()) - expect(resolved).toEqual({ skillPath: 'skill/SKILL.md', branch: 'main', files: [], total: 0 }) - expect(cache.setItem).toHaveBeenCalledWith( - CACHE_KEY, - { storedAt: expect.any(Number), value: resolved }, - { ttl: HOUR * 6 + HOUR * 24 }, - ) - - // Seven hours later the entry is past its fresh window. An upstream - // failure must serve the stale envelope, not propagate the 404. - cache.getItem.mockResolvedValue({ storedAt: Date.now() - 7 * HOUR * 1000, value: resolved }) - const error = Object.assign(new Error('404 Not Found'), { status: 404, statusCode: 404 }) - vi.stubGlobal('$fetch', vi.fn().mockRejectedValue(error)) - - await expect(handler(event())).resolves.toEqual(resolved) - }) - - it('lists files for a case-variant skill.md the name heuristic finds', async () => { - vi.stubGlobal('$fetch', vi.fn().mockResolvedValue({ - files: [ - { path: 'skill/skill.md', size: 100 }, - { path: 'skill/assets/cover.png', size: 2048 }, - ], - })) - - const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default - - await expect(handler(event())).resolves.toEqual({ - skillPath: 'skill/skill.md', - branch: 'main', - files: [{ path: 'assets/cover.png', size: 2048, type: 'image' }], - total: 1, - }) - }) - - it('lists files for a case-variant stored rendered_skill_path', async () => { - fixture.raw.prepare( - `UPDATE skills SET rendered_skill_path = 'skill/Skill.MD' - WHERE owner = 'owner' AND repo = 'repo' AND name = 'skill'`, - ).run() - vi.stubGlobal('$fetch', vi.fn().mockResolvedValue({ - files: [ - { path: 'skill/Skill.MD', size: 100 }, - { path: 'skill/assets/cover.png', size: 2048 }, - ], - })) - - const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default - - await expect(handler(event())).resolves.toEqual({ - skillPath: 'skill/Skill.MD', - branch: 'main', - files: [{ path: 'assets/cover.png', size: 2048, type: 'image' }], - total: 1, - }) - }) - - it('keeps an empty list on the missing window when the stored path is absent from the tree', async () => { - // rendered_skill_path points at a SKILL.md the upstream tree no longer - // holds (moved or renamed). The empty list is a failed resolution, so it - // must recheck on the 5-minute window, not ride the 30-hour one. - fixture.raw.prepare( - `UPDATE skills SET rendered_skill_path = 'old/SKILL.md' - WHERE owner = 'owner' AND repo = 'repo' AND name = 'skill'`, - ).run() - vi.stubGlobal('$fetch', vi.fn().mockResolvedValue({ files: [{ path: 'elsewhere/SKILL.md', size: 100 }] })) - - const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default - - const empty = await handler(event()) - expect(empty).toEqual({ skillPath: null, branch: 'main', files: [], total: 0 }) - expect(cache.setItem).toHaveBeenCalledWith( - CACHE_KEY, - { storedAt: expect.any(Number), value: empty }, - { ttl: 60 * 5 }, - ) - }) - - it('bounds a root SKILL.md over a large repo and keeps its shallow files', async () => { - fixture.raw.prepare( - `UPDATE skills SET rendered_skill_path = 'SKILL.md' - WHERE owner = 'owner' AND repo = 'repo' AND name = 'skill'`, - ).run() - const deep = Array.from({ length: SKILL_FILE_LIMIT + 50 }, (_, index) => ({ path: `.github/workflows/${index}.yml`, size: 10 })) - vi.stubGlobal('$fetch', vi.fn().mockResolvedValue({ - files: [...deep, { path: 'README.md', size: 10 }, { path: 'SKILL.md', size: 100 }], - })) - - const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default - const payload = await handler(event()) - - expect(payload.skillPath).toBe('SKILL.md') - expect(payload.total).toBe(SKILL_FILE_LIMIT + 51) - expect(payload.files).toHaveLength(SKILL_FILE_LIMIT) - expect(payload.files.map(f => f.path)).toContain('README.md') - }) - - it('serves the stale envelope when the upstream tree fails', async () => { - const stale = { - skillPath: 'skill', - branch: 'main', - files: [{ path: 'assets/cover.png', size: 2048, type: 'image' as const }], - total: 1, - } - cache.getItem.mockResolvedValue({ storedAt: Date.now() - (HOUR * 6 + 60) * 1000, value: stale }) - const error = Object.assign(new Error('404 Not Found'), { status: 404, statusCode: 404 }) - vi.stubGlobal('$fetch', vi.fn().mockRejectedValue(error)) - - const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default - - await expect(handler(event())).resolves.toEqual(stale) - }) -}) - -function event(): H3Event { - return { - method: 'GET', - context: { platform: { db: fixture.db } }, - node: { req: { headers: { ...SIGNED_IN_HEADERS } } }, - } as unknown as H3Event -} diff --git a/test/unit/skill-files-tree-outage.test.ts b/test/unit/skill-files-tree-outage.test.ts deleted file mode 100644 index b9817ca4..00000000 --- a/test/unit/skill-files-tree-outage.test.ts +++ /dev/null @@ -1,115 +0,0 @@ -import type { H3Event } from 'h3' -import type { SqliteD1 } from './helpers/d1-sqlite' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' -import { SIGNED_IN_HEADERS } from './helpers/session' - -let fixture: SqliteD1 -let cache: { - getItem: ReturnType - setItem: ReturnType -} -const responseHeaders = new Map() - -beforeEach(() => { - vi.resetModules() - responseHeaders.clear() - fixture = createSqliteD1(allMigrations()) - fixture.raw.prepare(`INSERT INTO repos (owner, repo) VALUES ('owner', 'repo')`).run() - fixture.raw.prepare( - `INSERT INTO skills (owner, repo, name, slug, display_name, source_resolved) - VALUES ('owner', 'repo', 'skill', 'owner/repo/skill', 'Skill', 1)`, - ).run() - cache = { - getItem: vi.fn().mockResolvedValue(null), - setItem: vi.fn().mockResolvedValue(undefined), - } - - vi.stubGlobal('defineEventHandler', (handler: unknown) => handler) - vi.stubGlobal('getRouterParam', () => 'owner/repo/skill') - vi.stubGlobal('getUserSession', () => Promise.resolve({ user: { id: 1, login: 'tester' } })) - vi.stubGlobal('useStorage', () => cache) - vi.stubGlobal('setHeader', (_event: H3Event, name: string, value: string | number) => { - responseHeaders.set(name, value) - }) - vi.stubGlobal('createWideEvent', () => ({ context: {}, setLevel: vi.fn(), emit: vi.fn(() => null) })) - vi.stubGlobal('emitOperationalEvent', vi.fn()) -}) - -afterEach(() => { - fixture.close() - vi.unstubAllGlobals() -}) - -describe('skill-files tree outage', () => { - it('returns a retryable error without caching an empty file list', async () => { - vi.stubGlobal('$fetch', vi.fn().mockRejectedValue(new Error('tree unavailable'))) - - const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default - - await expect(handler(event())).rejects.toMatchObject({ statusCode: 503 }) - - expect(cache.setItem).not.toHaveBeenCalled() - expect(responseHeaders.get('retry-after')).toBe(30) - }) -}) - -describe('skill-files stale serve', () => { - it('serves a stale envelope on a failed recompute without retry-after', async () => { - // retry-after is an instruction to re-poll. It belongs to the 503 that - // reaches the agent, never to a 200 served from a stale envelope, or a - // crawler sweep re-polls every 30 seconds against a dead upstream. - const stale = { - skillPath: 'skill', - branch: 'main', - files: [{ path: 'assets/cover.png', size: 2048, type: 'image' }], - total: 1, - } - // Seven hours old: past the 6-hour fresh window, inside the day-long - // stale window, so the failed recompute falls back to this payload. - cache.getItem.mockResolvedValue({ storedAt: Date.now() - 7 * 60 * 60 * 1000, value: stale }) - vi.stubGlobal('$fetch', vi.fn().mockRejectedValue(new Error('tree unavailable'))) - - const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default - - await expect(handler(event())).resolves.toEqual(stale) - expect(responseHeaders.has('retry-after')).toBe(false) - }) -}) - -describe('skill-files source gone', () => { - it('answers 410 from the registry verdict without an upstream call', async () => { - fixture.raw.prepare(`UPDATE skills SET source_resolved = 0 WHERE owner = 'owner' AND repo = 'repo' AND name = 'skill'`).run() - const fetchMock = vi.fn() - vi.stubGlobal('$fetch', fetchMock) - - const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default - - await expect(handler(event())).rejects.toMatchObject({ statusCode: 410 }) - - expect(fetchMock).not.toHaveBeenCalled() - expect(cache.setItem).not.toHaveBeenCalled() - }) - - it('classifies an upstream 404 tree as gone rather than an outage', async () => { - // ungh answers 404 for a deleted repository. ofetch carries the status on - // the rejection. A 503 with retry-after tells an agent to retry a - // permanent condition, which is what raised SKILLD-11. - const error = Object.assign(new Error('404 Not Found'), { status: 404, statusCode: 404 }) - vi.stubGlobal('$fetch', vi.fn().mockRejectedValue(error)) - - const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default - - await expect(handler(event())).rejects.toMatchObject({ statusCode: 410 }) - - expect(responseHeaders.has('retry-after')).toBe(false) - }) -}) - -function event(): H3Event { - return { - method: 'GET', - context: { platform: { db: fixture.db } }, - node: { req: { headers: { ...SIGNED_IN_HEADERS } } }, - } as unknown as H3Event -} diff --git a/test/unit/skill-stored-source.test.ts b/test/unit/skill-stored-source.test.ts new file mode 100644 index 00000000..38582fec --- /dev/null +++ b/test/unit/skill-stored-source.test.ts @@ -0,0 +1,152 @@ +import type { SqliteD1 } from './helpers/d1-sqlite' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + loadStoredSkillRow, + readReferencedFile, + readStoredSkillFiles, + readStoredSkillMd, + resolveReferencedFileTarget, +} from '../../layers/registry/server/utils/skill-stored-source' +import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' + +let fixture: SqliteD1 +const skill = { owner: 'owner', repo: 'repo', name: 'skill' } + +function seed(fields: { raw?: string | null, status?: string, path?: string | null, assets?: unknown, resolved?: number }) { + fixture.raw.prepare( + `INSERT INTO skills (owner, repo, name, slug, display_name, source_resolved, rendered_status, rendered_raw, rendered_skill_path, assets) + VALUES ('owner', 'repo', 'skill', 'owner/repo/skill', 'Skill', ?, ?, ?, ?, ?)`, + ).run( + fields.resolved ?? 1, + fields.status ?? 'ok', + fields.raw === undefined ? '# Stored body' : fields.raw, + fields.path === undefined ? 'skills/skill/SKILL.md' : fields.path, + JSON.stringify(fields.assets ?? []), + ) +} + +beforeEach(() => { + fixture = createSqliteD1(allMigrations()) + fixture.raw.prepare(`INSERT INTO repos (owner, repo, default_branch) VALUES ('owner', 'repo', 'trunk')`).run() +}) + +afterEach(() => { + fixture.close() +}) + +describe('stored SKILL.md', () => { + it('serves the stored body and names its source', async () => { + seed({}) + const row = await loadStoredSkillRow(fixture.db, skill) + + expect(readStoredSkillMd(skill, row!)).toEqual({ + _tag: 'ok', + body: '# Stored body', + source: 'owner/repo@trunk/skills/skill/SKILL.md', + }) + }) + + it('reports missing when no copy is stored', async () => { + seed({ raw: null }) + const row = await loadStoredSkillRow(fixture.db, skill) + + expect(readStoredSkillMd(skill, row!)).toEqual({ _tag: 'missing' }) + }) + + it('reports missing when the last render failed', async () => { + seed({ status: 'error' }) + const row = await loadStoredSkillRow(fixture.db, skill) + + expect(readStoredSkillMd(skill, row!)).toEqual({ _tag: 'missing' }) + }) + + it('reports gone over a stored body', async () => { + seed({ resolved: 0 }) + const row = await loadStoredSkillRow(fixture.db, skill) + + expect(readStoredSkillMd(skill, row!)).toEqual({ _tag: 'gone' }) + expect(readStoredSkillFiles(row!)).toEqual({ _tag: 'gone' }) + }) +}) + +describe('stored file list', () => { + it('builds the payload from the stored assets', async () => { + seed({ assets: [{ path: 'references/a.md', size: 10, type: 'markdown' }, { path: 'run.py', size: 5, type: 'code' }] }) + const row = await loadStoredSkillRow(fixture.db, skill) + + expect(readStoredSkillFiles(row!)).toEqual({ + _tag: 'ok', + payload: { + skillPath: 'skills/skill/SKILL.md', + branch: 'trunk', + files: [ + { path: 'references/a.md', size: 10, type: 'markdown' }, + { path: 'run.py', size: 5, type: 'code' }, + ], + total: 2, + }, + }) + }) + + it('lists no files for a root SKILL.md, because the sync stores none', async () => { + seed({ path: 'SKILL.md', assets: [] }) + const row = await loadStoredSkillRow(fixture.db, skill) + + expect(readStoredSkillFiles(row!)).toEqual({ + _tag: 'ok', + payload: { skillPath: 'SKILL.md', branch: 'trunk', files: [], total: 0 }, + }) + }) + + it('returns an empty list with no skillPath when nothing resolved', async () => { + seed({ path: null, status: 'error' }) + const row = await loadStoredSkillRow(fixture.db, skill) + + expect(readStoredSkillFiles(row!)).toEqual({ + _tag: 'ok', + payload: { skillPath: null, branch: 'trunk', files: [], total: 0 }, + }) + }) + + it('bounds a large list and reports the full count', async () => { + const assets = Array.from({ length: 300 }, (_, i) => ({ path: `references/${i}.md`, size: 1, type: 'markdown' })) + seed({ assets }) + const row = await loadStoredSkillRow(fixture.db, skill) + const result = readStoredSkillFiles(row!) + + expect(result._tag === 'ok' && result.payload.files).toHaveLength(250) + expect(result._tag === 'ok' && result.payload.total).toBe(300) + }) +}) + +describe('referenced file', () => { + it('takes the directory from the stored path and reads one file', async () => { + seed({}) + const row = await loadStoredSkillRow(fixture.db, skill) + const target = resolveReferencedFileTarget(skill, row!, 'references/a.md') + expect(target).toMatchObject({ + _tag: 'ok', + url: 'https://raw.githubusercontent.com/owner/repo/trunk/skills/skill/references/a.md', + }) + + const fetchText = vi.fn().mockResolvedValue({ _tag: 'ok', body: '# A' }) + expect(await readReferencedFile(target as Extract, fetchText)).toEqual({ _tag: 'ok', body: '# A' }) + expect(fetchText).toHaveBeenCalledTimes(1) + }) + + it('resolves a root Skill file at the repository root', async () => { + seed({ path: 'SKILL.md' }) + const row = await loadStoredSkillRow(fixture.db, skill) + + expect(resolveReferencedFileTarget(skill, row!, 'a.md')).toMatchObject({ + url: 'https://raw.githubusercontent.com/owner/repo/trunk/a.md', + }) + }) + + it('reports missing when no SKILL.md path is stored', async () => { + seed({ path: null }) + const row = await loadStoredSkillRow(fixture.db, skill) + + expect(resolveReferencedFileTarget(skill, row!, 'a.md')).toEqual({ _tag: 'missing' }) + }) +}) diff --git a/test/unit/skills-raw-stored.test.ts b/test/unit/skills-raw-stored.test.ts new file mode 100644 index 00000000..977a42f8 --- /dev/null +++ b/test/unit/skills-raw-stored.test.ts @@ -0,0 +1,98 @@ +import type { H3Event } from 'h3' +import type { SqliteD1 } from './helpers/d1-sqlite' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' +import { SIGNED_IN_HEADERS } from './helpers/session' + +let fixture: SqliteD1 +const headers = new Map() +const fetchMock = vi.fn() + +function seed(resolved: number, raw: string | null) { + fixture.raw.prepare(`INSERT INTO repos (owner, repo) VALUES ('owner', 'repo')`).run() + fixture.raw.prepare( + `INSERT INTO skills (owner, repo, name, slug, display_name, source_resolved, rendered_status, rendered_raw, rendered_skill_path, assets) + VALUES ('owner', 'repo', 'skill', 'owner/repo/skill', 'Skill', ?, 'ok', ?, 'skills/skill/SKILL.md', '[{"path":"references/a.md","size":3,"type":"markdown"}]')`, + ).run(resolved, raw) +} + +function event(): H3Event { + return { + method: 'GET', + context: { platform: { db: fixture.db } }, + node: { req: { headers: { ...SIGNED_IN_HEADERS } } }, + } as unknown as H3Event +} + +beforeEach(() => { + vi.resetModules() + headers.clear() + fetchMock.mockReset() + fixture = createSqliteD1(allMigrations()) + vi.stubGlobal('defineEventHandler', (handler: unknown) => handler) + vi.stubGlobal('getRouterParam', () => 'owner/repo/skill') + vi.stubGlobal('getUserSession', () => Promise.resolve({ user: { id: 1, login: 'tester' } })) + vi.stubGlobal('setHeader', (_e: H3Event, name: string, value: string | number) => headers.set(name, value)) + vi.stubGlobal('createWideEvent', () => ({ context: {}, setLevel: vi.fn(), emit: vi.fn(() => null) })) + vi.stubGlobal('emitOperationalEvent', vi.fn()) + vi.stubGlobal('$fetch', fetchMock) + vi.stubGlobal('fetch', fetchMock) + vi.stubGlobal('useStorage', () => ({ getItem: vi.fn().mockResolvedValue(null), setItem: vi.fn().mockResolvedValue(undefined) })) +}) + +afterEach(() => { + fixture.close() + vi.unstubAllGlobals() +}) + +describe('skills-raw handler', () => { + it('serves the stored SKILL.md with the existing headers and no network call', async () => { + seed(1, '# Stored') + const handler = (await import('../../layers/registry/server/api/skills-raw/[...slug].get')).default + + expect(await handler(event())).toBe('# Stored') + expect(headers.get('content-type')).toBe('text/markdown; charset=utf-8') + expect(headers.get('cache-control')).toBe('public, max-age=300') + expect(headers.get('x-skilld-source')).toBe('owner/repo@main/skills/skill/SKILL.md') + expect(fetchMock).not.toHaveBeenCalled() + }) + + it('answers 404 when no copy is stored, without a network call', async () => { + seed(1, null) + const handler = (await import('../../layers/registry/server/api/skills-raw/[...slug].get')).default + + await expect(handler(event())).rejects.toMatchObject({ statusCode: 404 }) + expect(fetchMock).not.toHaveBeenCalled() + }) + + it('answers 410 for a gone Skill, without a network call', async () => { + seed(0, '# Stored') + const handler = (await import('../../layers/registry/server/api/skills-raw/[...slug].get')).default + + await expect(handler(event())).rejects.toMatchObject({ statusCode: 410 }) + expect(fetchMock).not.toHaveBeenCalled() + }) +}) + +describe('skill-files handler', () => { + it('serves the stored list with no network call', async () => { + seed(1, '# Stored') + const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default + + expect(await handler(event())).toEqual({ + skillPath: 'skills/skill/SKILL.md', + branch: 'main', + files: [{ path: 'references/a.md', size: 3, type: 'markdown' }], + total: 1, + }) + expect(fetchMock).not.toHaveBeenCalled() + }) + + it('answers 410 for a gone Skill, without a network call', async () => { + seed(0, '# Stored') + const handler = (await import('../../layers/registry/server/api/skill-files/[...slug].get')).default + + await expect(handler(event())).rejects.toMatchObject({ statusCode: 410 }) + expect(fetchMock).not.toHaveBeenCalled() + }) +}) diff --git a/test/unit/skills-raw-tree-outage.test.ts b/test/unit/skills-raw-tree-outage.test.ts deleted file mode 100644 index f27754c8..00000000 --- a/test/unit/skills-raw-tree-outage.test.ts +++ /dev/null @@ -1,215 +0,0 @@ -import type { H3Event } from 'h3' -import type { SqliteD1 } from './helpers/d1-sqlite' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' -import { SIGNED_IN_HEADERS } from './helpers/session' - -let fixture: SqliteD1 -let cache: { - getItem: ReturnType - setItem: ReturnType -} -const responseHeaders = new Map() - -beforeEach(() => { - vi.resetModules() - responseHeaders.clear() - fixture = createSqliteD1(allMigrations()) - fixture.raw.prepare(`INSERT INTO repos (owner, repo) VALUES ('owner', 'repo')`).run() - fixture.raw.prepare( - `INSERT INTO skills (owner, repo, name, slug, display_name, source_resolved) - VALUES ('owner', 'repo', 'skill', 'owner/repo/skill', 'Skill', 1)`, - ).run() - cache = { - getItem: vi.fn().mockResolvedValue(null), - setItem: vi.fn().mockResolvedValue(undefined), - } - - vi.stubGlobal('defineEventHandler', (handler: unknown) => handler) - vi.stubGlobal('getRouterParam', () => 'owner/repo/skill') - vi.stubGlobal('getUserSession', () => Promise.resolve({ user: { id: 1, login: 'tester' } })) - vi.stubGlobal('useStorage', () => cache) - vi.stubGlobal('setHeader', (_event: H3Event, name: string, value: string | number) => { - responseHeaders.set(name, value) - }) -}) - -afterEach(() => { - fixture.close() - vi.unstubAllGlobals() -}) - -describe('skills-raw tree outage', () => { - it('returns a retryable error without caching the Skill as missing', async () => { - vi.stubGlobal('$fetch', vi.fn().mockRejectedValue(new Error('tree unavailable'))) - - const handler = (await import('../../layers/registry/server/api/skills-raw/[...slug].get')).default - - await expect(handler(event())).rejects.toMatchObject({ statusCode: 503 }) - - expect(cache.setItem).not.toHaveBeenCalled() - expect(responseHeaders.get('retry-after')).toBe(30) - }) -}) - -describe('skills-raw transient tree failure', () => { - it('serves the Skill when the first ungh tree read fails and the next one answers', async () => { - // SKILLD-1E: `dimillian/skills` answered 200 from GitHub and from ungh - // throughout the window, and the endpoint still raised 503. This is the - // run-command surface, so a blip that lasts one request must not reach - // the agent as an instruction to retry. - vi.stubGlobal('createWideEvent', () => ({ context: {}, setLevel: vi.fn(), emit: vi.fn(() => null) })) - vi.stubGlobal('emitOperationalEvent', vi.fn()) - const treeError = Object.assign(new Error('503 Service Unavailable'), { status: 503, statusCode: 503 }) - vi.stubGlobal('$fetch', vi.fn() - .mockRejectedValueOnce(treeError) - .mockResolvedValueOnce({ files: [{ path: 'skills/skill/SKILL.md' }] })) - vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response('# Skill body', { status: 200 }))) - - const handler = (await import('../../layers/registry/server/api/skills-raw/[...slug].get')).default - const body = await handler(event()) - - expect(body).toBe('# Skill body') - expect(responseHeaders.has('retry-after')).toBe(false) - }) -}) - -describe('skills-raw stale fallback', () => { - beforeEach(() => { - vi.stubGlobal('createWideEvent', () => ({ context: {}, setLevel: vi.fn(), emit: vi.fn(() => null) })) - vi.stubGlobal('emitOperationalEvent', vi.fn()) - }) - - it('serves the last-good body when the fresh window passed and upstream is unavailable', async () => { - // The body must survive its own TTL: an upstream blip minutes after the - // last good fetch answers with that copy, not a 503 on the run surface. - cache.getItem.mockResolvedValue({ - storedAt: Date.now() - 10 * 60 * 1000, - value: { status: 'ok', body: '# last good body', branch: 'main', path: 'skills/skill/SKILL.md' }, - }) - const unavailable = Object.assign(new Error('503 Service Unavailable'), { status: 503, statusCode: 503 }) - vi.stubGlobal('$fetch', vi.fn().mockRejectedValue(unavailable)) - - const handler = (await import('../../layers/registry/server/api/skills-raw/[...slug].get')).default - const body = await handler(event()) - - expect(body).toBe('# last good body') - expect(responseHeaders.get('content-type')).toBe('text/markdown; charset=utf-8') - expect(responseHeaders.has('retry-after')).toBe(false) - }) - - it('serves the last-good body when the tree answers but the content read is unavailable', async () => { - // SKILLD-11: raw.githubusercontent.com answered 503 while the ungh tree - // was healthy. The content stage is the observed outage point, so the - // stale copy must survive a content-stage failure, and the failure must - // not leave a "missing" marker behind. - cache.getItem.mockResolvedValue({ - storedAt: Date.now() - 10 * 60 * 1000, - value: { status: 'ok', body: '# last good body', branch: 'main', path: 'skills/skill/SKILL.md' }, - }) - vi.stubGlobal('$fetch', vi.fn().mockResolvedValue({ files: [{ path: 'skills/skill/SKILL.md' }] })) - vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response('upstream down', { status: 503 }))) - - const handler = (await import('../../layers/registry/server/api/skills-raw/[...slug].get')).default - const body = await handler(event()) - - expect(body).toBe('# last good body') - expect(responseHeaders.get('content-type')).toBe('text/markdown; charset=utf-8') - expect(responseHeaders.has('retry-after')).toBe(false) - expect(cache.setItem).not.toHaveBeenCalled() - }) - - it('serves a fresh envelope without calling upstream', async () => { - cache.getItem.mockResolvedValue({ - storedAt: Date.now(), - value: { status: 'ok', body: '# fresh body', branch: 'main', path: 'skills/skill/SKILL.md' }, - }) - const fetchMock = vi.fn() - vi.stubGlobal('$fetch', fetchMock) - - const handler = (await import('../../layers/registry/server/api/skills-raw/[...slug].get')).default - const body = await handler(event()) - - expect(body).toBe('# fresh body') - expect(fetchMock).not.toHaveBeenCalled() - }) -}) - -describe('skills-raw source gone', () => { - let emitEvent: ReturnType - - beforeEach(() => { - emitEvent = vi.fn() - vi.stubGlobal('createWideEvent', () => ({ context: {}, setLevel: vi.fn(), emit: vi.fn(() => null) })) - vi.stubGlobal('emitOperationalEvent', emitEvent) - }) - - it('answers 410 from the registry verdict without an upstream call', async () => { - fixture.raw.prepare(`UPDATE skills SET source_resolved = 0 WHERE owner = 'owner' AND repo = 'repo' AND name = 'skill'`).run() - const fetchMock = vi.fn() - vi.stubGlobal('$fetch', fetchMock) - - const handler = (await import('../../layers/registry/server/api/skills-raw/[...slug].get')).default - - await expect(handler(event())).rejects.toMatchObject({ statusCode: 410 }) - - expect(fetchMock).not.toHaveBeenCalled() - expect(cache.setItem).not.toHaveBeenCalled() - expect(emitEvent).not.toHaveBeenCalled() - }) - - it('classifies an upstream 404 tree as gone rather than an outage', async () => { - // ungh answers 404 for a deleted repository. ofetch carries the status on - // the rejection. Production culprit: dagster-io/erk. GitHub deleted it - // while its registry row still said resolved. - const error = Object.assign(new Error('404 Not Found'), { status: 404, statusCode: 404 }) - vi.stubGlobal('$fetch', vi.fn().mockRejectedValue(error)) - - const handler = (await import('../../layers/registry/server/api/skills-raw/[...slug].get')).default - - await expect(handler(event())).rejects.toMatchObject({ statusCode: 410 }) - - expect(cache.setItem).toHaveBeenCalledWith( - 'skills:raw:v3:owner/repo/skill', - expect.objectContaining({ status: 'missing' }), - expect.anything(), - ) - expect(responseHeaders.has('retry-after')).toBe(false) - }) -}) - -describe('skills-raw default branch', () => { - it('serves a skill whose repository default branch is not main, without a repo metadata round trip', async () => { - fixture.raw.prepare(`UPDATE repos SET default_branch = 'master' WHERE owner = 'owner' AND repo = 'repo'`).run() - // ungh's real wire shape: defaultBranch lives under .repo, and a tree - // listing only exists for the repository's actual default branch. - const fetchMock = vi.fn((url: string) => { - if (url.endsWith('/files/master')) - return Promise.resolve({ files: [{ path: 'skills/skill/SKILL.md' }] }) - if (url.endsWith('/files/main')) - return Promise.reject(new Error('404 Not Found')) - return Promise.resolve({ repo: { defaultBranch: 'master' } }) - }) - vi.stubGlobal('$fetch', fetchMock) - vi.stubGlobal('fetch', vi.fn().mockResolvedValue( - new Response('# Skill body', { status: 200 }), - )) - - const handler = (await import('../../layers/registry/server/api/skills-raw/[...slug].get')).default - const body = await handler(event()) - - expect(body).toBe('# Skill body') - expect(responseHeaders.get('x-skilld-source')).toBe('owner/repo@master/skills/skill/SKILL.md') - const treeUrls = fetchMock.mock.calls.map(([url]) => String(url)) - expect(treeUrls).toEqual(['https://ungh.cc/repos/owner/repo/files/master']) - expect(treeUrls.some(url => url.endsWith('/files/main'))).toBe(false) - }) -}) - -function event(): H3Event { - return { - method: 'GET', - context: { platform: { db: fixture.db } }, - node: { req: { headers: { ...SIGNED_IN_HEADERS } } }, - } as unknown as H3Event -}