From 99bc19afaeeeae533e725646be7448daa6bd3844 Mon Sep 17 00:00:00 2001 From: Harlan GitHub Agent Date: Fri, 4 Sep 2026 14:14:09 +1000 Subject: [PATCH 1/5] fix(ci): resolve published CLI help through pnpm dlx instead of a cold npm exec cache --- scripts/check-published-cli-grammar.ts | 118 ++++++++++++++++++------ test/unit/published-cli-grammar.test.ts | 53 +++++++++++ 2 files changed, 144 insertions(+), 27 deletions(-) create mode 100644 test/unit/published-cli-grammar.test.ts diff --git a/scripts/check-published-cli-grammar.ts b/scripts/check-published-cli-grammar.ts index d1c6cf5d..daf4c31a 100644 --- a/scripts/check-published-cli-grammar.ts +++ b/scripts/check-published-cli-grammar.ts @@ -12,7 +12,9 @@ */ import { execFile } from 'node:child_process' +import { resolve } from 'node:path' import process from 'node:process' +import { fileURLToPath } from 'node:url' import { promisify } from 'node:util' import { collectionInstallCmd, @@ -49,6 +51,20 @@ interface ChannelCheck { problems: string[] } +export interface PublishedCliInvocation { + file: string + args: string[] +} + +export interface PublishedCliGrammarDependencies { + readVersion: (tag: ChannelRequirement['tag']) => Promise + readHelp: (version: string, subcommand?: string) => Promise +} + +export type PublishedCliGrammarResult + = | { _tag: 'clean', checks: ChannelCheck[] } + | { _tag: 'blocked', checks: ChannelCheck[] } + function subcommand(command: string): string { return command.split(/\s+/)[2]! } @@ -93,9 +109,25 @@ async function publishedVersion(tag: ChannelRequirement['tag']): Promise return version } +/** + * Runs a published package through `pnpm dlx` rather than `npx`. + * + * `npx` downloads into a per-container npm cache, so every ephemeral CI + * container pays the full download over the shared uplink and a slow one + * overruns the 180s kill and reads as a grammar failure. `pnpm dlx` resolves + * through the shared pnpm store the setup action mounts, so only the first + * container ever downloads a version. + */ +export function publishedCliInvocation(version: string, subcommand?: string): PublishedCliInvocation { + return { + file: 'pnpm', + args: ['--reporter=silent', 'dlx', `${PACKAGE}@${version}`, ...(subcommand ? [subcommand] : []), '--help'], + } +} + async function helpText(version: string, subcommand?: string): Promise { - const args = ['--yes', `${PACKAGE}@${version}`, ...(subcommand ? [subcommand] : []), '--help'] - const result = await run('npx', args, { + const command = publishedCliInvocation(version, subcommand) + const result = await run(command.file, command.args, { timeout: 180_000, env: { ...process.env, NO_COLOR: '1', FORCE_COLOR: '0' }, }).catch((error: { stdout?: string, stderr?: string }) => error) @@ -146,11 +178,14 @@ function publishedAgents(installHelp: string): string[] | null { return values.length > 0 ? values : null } -async function checkChannel(requirement: ChannelRequirement): Promise { - const version = await publishedVersion(requirement.tag) - const published = publishedSubcommands(await helpText(version)) +async function checkChannel( + requirement: ChannelRequirement, + dependencies: PublishedCliGrammarDependencies, +): Promise { + const version = await dependencies.readVersion(requirement.tag) + const published = publishedSubcommands(await dependencies.readHelp(version)) const agents = requirement.agents.length > 0 - ? publishedAgents(await helpText(version, 'install')) + ? publishedAgents(await dependencies.readHelp(version, 'install')) : [] const major = Number(version.split('.')[0]) const problems: string[] = [] @@ -185,28 +220,57 @@ async function checkChannel(requirement: ChannelRequirement): Promise check.problems.length > 0) +/** + * Checks one channel at a time. Each channel's first call downloads its + * package through the shared store, and two parallel downloads contend for + * the same uplink the CI box shares with every other job. + */ +export async function runPublishedCliGrammar( + dependencies: PublishedCliGrammarDependencies, +): Promise { + const checks: ChannelCheck[] = [] + for (const requirement of channelRequirements()) + checks.push(await checkChannel(requirement, dependencies)) + + return checks.some(check => check.problems.length > 0) + ? { _tag: 'blocked', checks } + : { _tag: 'clean', checks } +} + +async function main(): Promise { + const result = await runPublishedCliGrammar({ + readVersion: publishedVersion, + readHelp: helpText, + }) -if (failed.length === 0) { - console.log(JSON.stringify({ - _tag: 'clean', - check: 'published-cli-grammar', - channels: checks.map(({ tag, version, required, requiredAgents }) => ({ tag, version, required, requiredAgents })), - }, null, 2)) - process.exit(0) + if (result._tag === 'clean') { + console.log(JSON.stringify({ + _tag: 'clean', + check: 'published-cli-grammar', + channels: result.checks.map(({ tag, version, required, requiredAgents }) => ({ tag, version, required, requiredAgents })), + }, null, 2)) + return + } + + for (const check of result.checks.filter(check => check.problems.length > 0)) { + console.error(`${PACKAGE}@${check.tag} is ${check.version}: ${check.problems.join('; ')}.`) + if (check.published) + console.error(`It supports: ${check.published.join(', ')}.`) + if (check.publishedAgents && check.publishedAgents.length > 0) + console.error(`Its --agent values: ${check.publishedAgents.join(', ')}.`) + } + console.error('') + console.error('The site prints a command that the published CLI cannot run.') + console.error('Publish the missing CLI grammar under the npm tag named above.') + console.error('For a rejected --agent value, hold its /agents page: raise cliSince in agent-pages.ts.') + console.error('To deploy anyway, run the "Deploy to Cloudflare" workflow by hand.') + process.exitCode = 1 } -for (const check of failed) { - console.error(`${PACKAGE}@${check.tag} is ${check.version}: ${check.problems.join('; ')}.`) - if (check.published) - console.error(`It supports: ${check.published.join(', ')}.`) - if (check.publishedAgents && check.publishedAgents.length > 0) - console.error(`Its --agent values: ${check.publishedAgents.join(', ')}.`) +const invokedPath = process.argv[1] ? resolve(process.argv[1]) : null +if (invokedPath === fileURLToPath(import.meta.url)) { + main().catch((error) => { + console.error(error instanceof Error ? error.message : String(error)) + process.exitCode = 1 + }) } -console.error('') -console.error('The site prints a command that the published CLI cannot run.') -console.error('Publish the missing CLI grammar under the npm tag named above.') -console.error('For a rejected --agent value, hold its /agents page: raise cliSince in agent-pages.ts.') -console.error('To deploy anyway, run the "Deploy to Cloudflare" workflow by hand.') -process.exit(1) diff --git a/test/unit/published-cli-grammar.test.ts b/test/unit/published-cli-grammar.test.ts new file mode 100644 index 00000000..84d44f3c --- /dev/null +++ b/test/unit/published-cli-grammar.test.ts @@ -0,0 +1,53 @@ +// @vitest-environment node +import { describe, expect, it } from 'vitest' +import { + publishedCliInvocation, + runPublishedCliGrammar, +} from '../../scripts/check-published-cli-grammar' + +const betaAgents = [ + 'claude-code', + 'codex', + 'cursor', + 'gemini-cli', + 'github-copilot', + 'hermes', + 'openclaw', + 'opencode', + 'windsurf', +] + +describe('published CLI grammar', () => { + it('runs a published package through pnpm without npm exec', () => { + expect(publishedCliInvocation('2.3.0', 'install')).toEqual({ + file: 'pnpm', + args: ['--reporter=silent', 'dlx', 'skilld@2.3.0', 'install', '--help'], + }) + }) + + it('checks one published package at a time', async () => { + let active = 0 + let peakActive = 0 + const readHelp = async (version: string, subcommand?: string) => { + active += 1 + peakActive = Math.max(peakActive, active) + await new Promise(resolvePromise => setTimeout(resolvePromise, 5)) + active -= 1 + + if (version === '2.3.0') + return 'USAGE skilld [OPTIONS] add|pull\n' + if (subcommand === 'install') { + return `--agent \n Values: ${betaAgents.join(', ')}.\n` + } + return 'Commands:\n install Install a Skill\n run Run a Skill\n\n' + } + + const result = await runPublishedCliGrammar({ + readVersion: async tag => tag === 'latest' ? '2.3.0' : '3.0.0-beta.3', + readHelp, + }) + + expect(result._tag).toBe('clean') + expect(peakActive).toBe(1) + }) +}) From a5beece2d15f37d395c305966ff1a3c31f11bf3c Mon Sep 17 00:00:00 2001 From: Harlan GitHub Agent Date: Sat, 5 Sep 2026 08:30:16 +1000 Subject: [PATCH 2/5] fix: resolve issue #137 --- .../server/api/skills/[...slug].get.ts | 32 ++++--- test/unit/cache-write-best-effort.test.ts | 90 ++++++++++++++++++- 2 files changed, 110 insertions(+), 12 deletions(-) diff --git a/layers/registry/server/api/skills/[...slug].get.ts b/layers/registry/server/api/skills/[...slug].get.ts index b110db38..3116a6df 100644 --- a/layers/registry/server/api/skills/[...slug].get.ts +++ b/layers/registry/server/api/skills/[...slug].get.ts @@ -1,6 +1,8 @@ import type { H3Event } from 'h3' +import type { z } from 'zod' import { LIVE_RENDER_STALE_SECONDS } from '~~/server/utils/sync-thresholds' +import { readCache, writeCache } from '#shared/server/cache' import { defineApiHandler } from '#shared/server/handler' import { githubSkillFileUrl } from '#shared/skill-file-url' import { selectSkillFiles } from '#shared/skill-files' @@ -32,6 +34,9 @@ interface CuratorEndorsement { const ONE_DAY_MS = 1000 * 60 * 60 * 24 +const DETAIL_CACHE_TTL = 60 +type SkillDetailPayload = z.infer + const OFFICIAL_REPO_KEYS = new Set(officialRepos.map(r => `${r.owner}/${r.repo}`)) const OFFICIAL_REPO_KIND = new Map(officialRepos.map(r => [`${r.owner}/${r.repo}`, r.kind])) @@ -183,6 +188,11 @@ const skillDetailHandler = defineApiHandler({ if (!slug) throw createError({ statusCode: 400, message: 'Missing skill slug' }) + const cacheKey = `skills:detail:v1:${slug.toLowerCase()}` + const cached = await readCache(useStorage('cache'), cacheKey) + if (cached) + return cached + const skill = await findSkill(event, slug) if (!skill) throw createError({ statusCode: 404, message: 'Skill not found' }) @@ -326,7 +336,7 @@ const skillDetailHandler = defineApiHandler({ const createdAtIso = epochToIso(row?.repo_created_at) const repoSkillCount = resolvedRepoSkillCount?.skill_count ?? 0 - return { + const detail = { owner: skill.owner, repo: skill.repo, name: skill.name, @@ -447,20 +457,20 @@ const skillDetailHandler = defineApiHandler({ }, duplicateGroup, } + + await writeCache(useStorage('cache'), cacheKey, detail, { ttl: DETAIL_CACHE_TTL }) + return detail }, }) // Skill detail data is public and changes only when indexing or social counts -// update. Cache the assembled response so popular links do not repeat every D1 -// lookup and Markdown render for each reader. -export default defineCachedEventHandler(skillDetailHandler, { - maxAge: 60, - staleMaxAge: 60 * 5, - swr: true, - group: 'skill-detail', - name: 'skill-detail-v1', - getKey: event => (getRouterParam(event, 'slug') ?? '').toLowerCase(), -}) +// update. A read-through KV cache keeps popular links from repeating every D1 +// lookup and Markdown render for each reader. The cache writes go through +// `writeCache` because Nitro's own route cache called storage.setItem bare, +// so a KV `KV PUT failed: 429` escaped the cached handler as a 500 (Sentry +// SKILLD-1V). Losing the entry costs one repeat computation; propagating the +// rejection costs the user the page. +export default skillDetailHandler interface RenderedView { skillPath: string | null diff --git a/test/unit/cache-write-best-effort.test.ts b/test/unit/cache-write-best-effort.test.ts index acb92c68..e74952a5 100644 --- a/test/unit/cache-write-best-effort.test.ts +++ b/test/unit/cache-write-best-effort.test.ts @@ -1,7 +1,10 @@ +import type { H3Event } from 'h3' +import type { SqliteD1 } from './helpers/d1-sqlite' import { readdirSync, readFileSync, statSync } from 'node:fs' import { join } from 'node:path' -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' import { readCache, writeCache } from '../../shared/server/cache' +import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' function sourceFiles(directory: string): string[] { const entries = readdirSync(directory) @@ -103,3 +106,88 @@ describe('best-effort cache reads', () => { expect(offenders).toEqual([]) }) }) + +describe('skill detail route cache', () => { + const NOW_SEC = Math.floor(Date.now() / 1000) + const slug = 'ericzakariasson/scandinavian-design/alpha' + let harness: SqliteD1 + let handler: (event: H3Event) => Promise> + + // Nitro's cached handler: on a miss it computes the response and awaits + // storage.setItem, so a KV PUT rejection travels out of the handler as a + // 500 (Sentry SKILLD-1V). This stub keeps that documented behaviour honest, + // so the regression test fails again if the route ever returns to + // defineCachedEventHandler. + vi.stubGlobal('defineCachedEventHandler', ( + handler: (event: H3Event) => Promise, + opts: { getKey: (event: H3Event) => string }, + ) => { + return async (event: H3Event) => { + const storage = useStorage('cache') + const key = opts.getKey(event) + const cached = await storage.getItem(key) + if (cached != null) + return cached + const result = await handler(event) + await storage.setItem(key, result, { ttl: 60 }) + return result + } + }) + vi.stubGlobal('defineEventHandler', (handler: unknown) => handler) + vi.stubGlobal('createError', (input: Record) => Object.assign(new Error(String(input.message)), input)) + vi.stubGlobal('getUserSession', () => Promise.resolve(null)) + vi.stubGlobal('getRouterParam', (_event: unknown, key: string) => (key === 'slug' ? slug : undefined)) + + beforeEach(async () => { + vi.resetModules() + harness = createSqliteD1(allMigrations()) + harness.raw.prepare(`INSERT INTO repos (owner, repo) VALUES ('ericzakariasson', 'scandinavian-design')`).run() + const raw = `---\nname: alpha\ndescription: A alpha skill.\n---\n\nBody of alpha.\n` + harness.raw.prepare( + `INSERT INTO skills (owner, repo, name, slug, display_name, source_resolved, + rendered_skill_path, rendered_status, rendered_raw, rendered_html, rendered_at) + VALUES ('ericzakariasson', 'scandinavian-design', 'alpha', ?, 'alpha', 1, + 'skills/alpha/SKILL.md', 'ok', ?, '

ok

', ?)`, + ).run(slug, raw, NOW_SEC) + handler = (await import('../../layers/registry/server/api/skills/[...slug].get')).default + }) + + it('serves the page even when the KV cache write is rate limited', async () => { + const setItem = vi.fn(async () => { + throw new Error('KV PUT failed: 429 Too Many Requests') + }) + vi.stubGlobal('useStorage', () => ({ + getItem: async () => null, + setItem, + })) + + const body = await handler(event()) + + expect(body.registryPath).toBe('/gh/ericzakariasson/scandinavian-design') + expect(setItem).toHaveBeenCalledWith( + expect.stringContaining('skills:detail:'), + expect.objectContaining({ registryPath: '/gh/ericzakariasson/scandinavian-design' }), + { ttl: 60 }, + ) + }) + + it('serves the cached response on a hit without recomputing', async () => { + const cacheMap = new Map() + vi.stubGlobal('useStorage', () => ({ + getItem: async (key: string) => cacheMap.get(key) ?? null, + setItem: async (key: string, value: unknown) => { + cacheMap.set(key, value) + }, + })) + + const fresh = await handler(event()) + await expect(handler(event())).resolves.toBe(fresh) + }) + + function event(): H3Event { + return { + context: { platform: { db: harness.db } }, + node: { req: { headers: {} } }, + } as unknown as H3Event + } +}) From 7913a62771791d3711a7b916c50c8789b6fef390 Mon Sep 17 00:00:00 2001 From: Harlan GitHub Agent Date: Sat, 5 Sep 2026 09:16:37 +1000 Subject: [PATCH 3/5] fix(registry): restore SWR and single-flight to the skill detail cache --- .../server/api/skills/[...slug].get.ts | 515 +++++++++--------- shared/server/cache.ts | 109 ++++ test/unit/cache-write-best-effort.test.ts | 29 +- test/unit/skill-detail-swr.test.ts | 114 ++++ 4 files changed, 493 insertions(+), 274 deletions(-) create mode 100644 test/unit/skill-detail-swr.test.ts diff --git a/layers/registry/server/api/skills/[...slug].get.ts b/layers/registry/server/api/skills/[...slug].get.ts index 3116a6df..b17a5722 100644 --- a/layers/registry/server/api/skills/[...slug].get.ts +++ b/layers/registry/server/api/skills/[...slug].get.ts @@ -1,8 +1,9 @@ import type { H3Event } from 'h3' import type { z } from 'zod' +import type { Platform } from '#shared/server/platform' import { LIVE_RENDER_STALE_SECONDS } from '~~/server/utils/sync-thresholds' -import { readCache, writeCache } from '#shared/server/cache' +import { cached } from '#shared/server/cache' import { defineApiHandler } from '#shared/server/handler' import { githubSkillFileUrl } from '#shared/skill-file-url' import { selectSkillFiles } from '#shared/skill-files' @@ -35,6 +36,7 @@ interface CuratorEndorsement { const ONE_DAY_MS = 1000 * 60 * 60 * 24 const DETAIL_CACHE_TTL = 60 +const DETAIL_CACHE_STALE_TTL = 60 * 5 type SkillDetailPayload = z.infer const OFFICIAL_REPO_KEYS = new Set(officialRepos.map(r => `${r.owner}/${r.repo}`)) @@ -188,19 +190,26 @@ const skillDetailHandler = defineApiHandler({ if (!slug) throw createError({ statusCode: 400, message: 'Missing skill slug' }) - const cacheKey = `skills:detail:v1:${slug.toLowerCase()}` - const cached = await readCache(useStorage('cache'), cacheKey) - if (cached) - return cached + return cached({ + storage: useStorage('cache'), + key: `skills:detail:v1:${slug.toLowerCase()}`, + ttlSeconds: DETAIL_CACHE_TTL, + staleSeconds: DETAIL_CACHE_STALE_TTL, + compute: () => loadSkillDetail(event, platform, slug), + schedule: promise => runAfterResponse(event, promise), + }) + }, +}) - const skill = await findSkill(event, slug) - if (!skill) - throw createError({ statusCode: 404, message: 'Skill not found' }) +async function loadSkillDetail(event: H3Event, platform: Platform, slug: string): Promise { + const skill = await findSkill(event, slug) + if (!skill) + throw createError({ statusCode: 404, message: 'Skill not found' }) - const [curators, row, latestCommit, duplicateGroup, faqRow, tagRow, summaryRow, repoSkillRows, resolvedRepoSkillCount, ownerRow] = await Promise.all([ - Promise.resolve([] satisfies CuratorEndorsement[]), - platform.db - .prepare(`SELECT r.stars, r.forks, r.pushed_at, r.repo_created_at, r.default_branch, + const [curators, row, latestCommit, duplicateGroup, faqRow, tagRow, summaryRow, repoSkillRows, resolvedRepoSkillCount, ownerRow] = await Promise.all([ + Promise.resolve([] satisfies CuratorEndorsement[]), + platform.db + .prepare(`SELECT r.stars, r.forks, r.pushed_at, r.repo_created_at, r.default_branch, r.source_owner, r.source_repo, s.current_sha, s.modified_at, s.references_count, s.assets, s.last_synced_at, s.sync_status, s.source_resolved, @@ -211,265 +220,267 @@ const skillDetailHandler = defineApiHandler({ s.rendered_skill_path, s.rendered_status, s.rendered_raw, s.rendered_frontmatter, s.rendered_html, s.rendered_at 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(), - platform.db - .prepare(`SELECT sha FROM skill_revisions WHERE owner = ? AND repo = ? AND name = ? ORDER BY modified_at DESC LIMIT 1`) - .bind(skill.owner, skill.repo, skill.name) - .first<{ sha: string }>(), - findDuplicateGroupForSkill(event, `${skill.owner}/${skill.repo}/${skill.name}`), - getGenerated(platform.db, { owner: skill.owner, repo: skill.repo, name: skill.name, kind: 'faq' }), - getGenerated(platform.db, { owner: skill.owner, repo: skill.repo, name: skill.name, kind: 'tags' }), - getGenerated(platform.db, { owner: skill.owner, repo: skill.repo, name: skill.name, kind: 'summary' }), - platform.db - .prepare(`SELECT name FROM skills WHERE owner = ? AND repo = ? ORDER BY name`) - .bind(skill.owner, skill.repo) - .all(), - // Same definition as loadRepoSkillCounts (source_resolved = 1), so the - // 301 decision matches the hub routing used by feeds and sitemaps. - // repoSkillNames stays unfiltered: it feeds dependency parsing, not - // routing. - platform.db - .prepare(`SELECT COUNT(*) AS skill_count FROM skills WHERE owner = ? AND repo = ? AND source_resolved = 1`) - .bind(skill.owner, skill.repo) - .first(), - // Same `owners` row the owner page shows; the byline reuses it. - platform.db - .prepare(`SELECT name FROM owners WHERE owner = ?`) - .bind(skill.owner) - .first<{ name: string | null }>(), - ]) - - const source = resolveRepoSourceIdentityFromRow(skill, row) - const githubUrl = `https://github.com/${source.owner}/${source.repo}` - const branch = row?.default_branch || 'main' - const repoSkillNames = (repoSkillRows.results ?? []).map(candidate => candidate.name) - - // Warm path: render is in D1. Cold path (legacy rows or fetch_failed - // status): fall back to a live render so the first visit still works, - // then write back to D1. - let rendered: RenderedView - if (row?.rendered_html && row.rendered_status === 'ok') { - const reparsed = row.rendered_raw - ? await parseSkillMd(row.rendered_raw, { - owner: source.owner, - repo: source.repo, - name: skill.name, - branch, - skillDir: row.rendered_skill_path?.replace(/\/SKILL\.md$/, '') ?? '', - filePath: '', - skillNames: repoSkillNames, - registryOwner: skill.owner, - registryRepo: skill.repo, - }) - : null - rendered = { - skillPath: row.rendered_skill_path, - raw: row.rendered_raw, - frontmatter: reparsed?.frontmatter ?? parseFrontmatterJson(row.rendered_frontmatter), - body: reparsed?.body ?? stripFrontmatter(row.rendered_raw ?? ''), - html: reparsed?.html ?? row.rendered_html, - dependencies: reparsed?.dependencies ?? [], - status: 'ok', - } - } - else { - rendered = await renderLive(event, { - sourceOwner: source.owner, - sourceRepo: source.repo, - registryOwner: skill.owner, - registryRepo: skill.repo, - name: skill.name, - branch, - skillNames: repoSkillNames, - }) - // Cache cold-path result back to D1 so subsequent visits hit the warm - // path. Fire-and-forget; missing waitUntil context (e.g. local dev) - // just means we await it inline. - schedulePersist(event, platform.db, skill.owner, skill.repo, skill.name, rendered) + .bind(skill.owner, skill.repo, skill.name) + .first(), + platform.db + .prepare(`SELECT sha FROM skill_revisions WHERE owner = ? AND repo = ? AND name = ? ORDER BY modified_at DESC LIMIT 1`) + .bind(skill.owner, skill.repo, skill.name) + .first<{ sha: string }>(), + findDuplicateGroupForSkill(event, `${skill.owner}/${skill.repo}/${skill.name}`), + getGenerated(platform.db, { owner: skill.owner, repo: skill.repo, name: skill.name, kind: 'faq' }), + getGenerated(platform.db, { owner: skill.owner, repo: skill.repo, name: skill.name, kind: 'tags' }), + getGenerated(platform.db, { owner: skill.owner, repo: skill.repo, name: skill.name, kind: 'summary' }), + platform.db + .prepare(`SELECT name FROM skills WHERE owner = ? AND repo = ? ORDER BY name`) + .bind(skill.owner, skill.repo) + .all(), + // Same definition as loadRepoSkillCounts (source_resolved = 1), so the + // 301 decision matches the hub routing used by feeds and sitemaps. + // repoSkillNames stays unfiltered: it feeds dependency parsing, not + // routing. + platform.db + .prepare(`SELECT COUNT(*) AS skill_count FROM skills WHERE owner = ? AND repo = ? AND source_resolved = 1`) + .bind(skill.owner, skill.repo) + .first(), + // Same `owners` row the owner page shows; the byline reuses it. + platform.db + .prepare(`SELECT name FROM owners WHERE owner = ?`) + .bind(skill.owner) + .first<{ name: string | null }>(), + ]) + + const source = resolveRepoSourceIdentityFromRow(skill, row) + const githubUrl = `https://github.com/${source.owner}/${source.repo}` + const branch = row?.default_branch || 'main' + const repoSkillNames = (repoSkillRows.results ?? []).map(candidate => candidate.name) + + // Warm path: render is in D1. Cold path (legacy rows or fetch_failed + // status): fall back to a live render so the first visit still works, + // then write back to D1. + let rendered: RenderedView + if (row?.rendered_html && row.rendered_status === 'ok') { + const reparsed = row.rendered_raw + ? await parseSkillMd(row.rendered_raw, { + owner: source.owner, + repo: source.repo, + name: skill.name, + branch, + skillDir: row.rendered_skill_path?.replace(/\/SKILL\.md$/, '') ?? '', + filePath: '', + skillNames: repoSkillNames, + registryOwner: skill.owner, + registryRepo: skill.repo, + }) + : null + rendered = { + skillPath: row.rendered_skill_path, + raw: row.rendered_raw, + frontmatter: reparsed?.frontmatter ?? parseFrontmatterJson(row.rendered_frontmatter), + body: reparsed?.body ?? stripFrontmatter(row.rendered_raw ?? ''), + html: reparsed?.html ?? row.rendered_html, + dependencies: reparsed?.dependencies ?? [], + status: 'ok', } + } + else { + rendered = await renderLive(event, { + sourceOwner: source.owner, + sourceRepo: source.repo, + registryOwner: skill.owner, + registryRepo: skill.repo, + name: skill.name, + branch, + skillNames: repoSkillNames, + }) + // Cache cold-path result back to D1 so subsequent visits hit the warm + // path. Fire-and-forget; missing waitUntil context (e.g. local dev) + // just means we await it inline. + schedulePersist(event, platform.db, skill.owner, skill.repo, skill.name, rendered) + } - // Stale refresh: only fire when rendered_at older than threshold. - const renderedAge = secondsAgo(row?.rendered_at) - if (row?.rendered_html && renderedAge != null && renderedAge > LIVE_RENDER_STALE_SECONDS) - scheduleRefresh(event, platform.db, skill, source, skill.name, branch, repoSkillNames) - - const rawAiTags = tagRow?.payload.tags ?? [] - const tags = rawAiTags - .map(s => TAG_BY_SLUG.get(s)) - .filter((t): t is NonNullable => Boolean(t)) - const knownTagSlugs = new Set(tags.map(t => t.slug)) - const keywords = rawAiTags.filter(t => !knownTagSlugs.has(t)) - - const description = frontmatterString(rendered.frontmatter, 'description') ?? skill.description ?? null - const license = frontmatterString(rendered.frontmatter, 'license') - let assets: { path: string, size: number, type: string }[] = [] - if (row?.assets) { - try { - const parsedAssets = JSON.parse(row.assets) as unknown - if (Array.isArray(parsedAssets)) { - assets = parsedAssets.filter((a): a is { path: string, size: number, type: string } => - Boolean(a) && typeof a === 'object' && typeof (a as { path: unknown }).path === 'string') - } - } - catch { - // Ignore malformed JSON; treat as no assets. + // Stale refresh: only fire when rendered_at older than threshold. + const renderedAge = secondsAgo(row?.rendered_at) + if (row?.rendered_html && renderedAge != null && renderedAge > LIVE_RENDER_STALE_SECONDS) + scheduleRefresh(event, platform.db, skill, source, skill.name, branch, repoSkillNames) + + const rawAiTags = tagRow?.payload.tags ?? [] + const tags = rawAiTags + .map(s => TAG_BY_SLUG.get(s)) + .filter((t): t is NonNullable => Boolean(t)) + const knownTagSlugs = new Set(tags.map(t => t.slug)) + const keywords = rawAiTags.filter(t => !knownTagSlugs.has(t)) + + const description = frontmatterString(rendered.frontmatter, 'description') ?? skill.description ?? null + const license = frontmatterString(rendered.frontmatter, 'license') + let assets: { path: string, size: number, type: string }[] = [] + if (row?.assets) { + try { + const parsedAssets = JSON.parse(row.assets) as unknown + if (Array.isArray(parsedAssets)) { + assets = parsedAssets.filter((a): a is { path: string, size: number, type: string } => + Boolean(a) && typeof a === 'object' && typeof (a as { path: unknown }).path === 'string') } } - const allowedTools = parseAllowedTools(rendered.frontmatter) - const capability = classifyAllowedTools(allowedTools) - const selectedAssets = selectSkillFiles(assets) - // `rendered.*` describes the cached copy, which survives the file being - // deleted upstream, so it can only ever say "we can still render this". The - // stored `source_resolved` is the sync's verdict on whether the file is - // still there. Reading only the former reported `resolved: true` for skills - // whose SKILL.md upstream had been 404 for months. - const sourceResolved = isSourceResolved({ - sourceResolved: row?.source_resolved, - renderStatus: rendered.status, - skillPath: rendered.skillPath, - raw: rendered.raw, - }) - const sourceGone = !sourceResolved && row?.source_resolved === 0 - const sourceCommitSha = latestCommit?.sha ?? row?.current_sha ?? null - const pushedAtIso = epochToIso(row?.pushed_at) - const createdAtIso = epochToIso(row?.repo_created_at) - const repoSkillCount = resolvedRepoSkillCount?.skill_count ?? 0 - - const detail = { + catch { + // Ignore malformed JSON; treat as no assets. + } + } + const allowedTools = parseAllowedTools(rendered.frontmatter) + const capability = classifyAllowedTools(allowedTools) + const selectedAssets = selectSkillFiles(assets) + // `rendered.*` describes the cached copy, which survives the file being + // deleted upstream, so it can only ever say "we can still render this". The + // stored `source_resolved` is the sync's verdict on whether the file is + // still there. Reading only the former reported `resolved: true` for skills + // whose SKILL.md upstream had been 404 for months. + const sourceResolved = isSourceResolved({ + sourceResolved: row?.source_resolved, + renderStatus: rendered.status, + skillPath: rendered.skillPath, + raw: rendered.raw, + }) + const sourceGone = !sourceResolved && row?.source_resolved === 0 + const sourceCommitSha = latestCommit?.sha ?? row?.current_sha ?? null + const pushedAtIso = epochToIso(row?.pushed_at) + const createdAtIso = epochToIso(row?.repo_created_at) + const repoSkillCount = resolvedRepoSkillCount?.skill_count ?? 0 + + const detail = { + owner: skill.owner, + repo: skill.repo, + name: skill.name, + registryPath: canonicalRepoSkillPath({ owner: skill.owner, repo: skill.repo, name: skill.name, - registryPath: canonicalRepoSkillPath({ - owner: skill.owner, - repo: skill.repo, - name: skill.name, - repoSkillCount, - }), - displayName: skill.displayName, - authorName: ownerRow?.name ?? null, - githubUrl, - skillPath: rendered.skillPath, - branch, - resolutionStatus: rendered.status, - sourceGone, - contentHtml: rendered.html, - dependencies: rendered.dependencies, - frontmatter: rendered.frontmatter, - raw: rendered.raw, - assets: selectedAssets.files, - assetCount: selectedAssets.total, - curators, - description, - license, - stars: row?.stars ?? 0, - forks: row?.forks ?? 0, - pushedAt: pushedAtIso, - createdAt: createdAtIso, - maturity: computeMaturity(row?.repo_created_at ?? null, row?.pushed_at ?? null), - tier: resolveTier(skill.owner, skill.repo), - sourceFacts: { - description: { - present: Boolean(description?.trim()), - length: description?.trim().length ?? 0, - source: frontmatterString(rendered.frontmatter, 'description') ? 'frontmatter' : skill.description ? 'repository' : null, - }, - repository: { - pushedAt: pushedAtIso, - pushedAgeDays: daysFromSecondsAgo(isoToSecondsAgo(pushedAtIso)), - createdAt: createdAtIso, - stars: row?.stars ?? 0, - forks: row?.forks ?? 0, - defaultBranch: branch, - }, - source: { - resolved: sourceResolved, - gone: sourceGone, - resolutionStatus: rendered.status, - skillPath: rendered.skillPath, - currentSha: row?.current_sha ?? null, - hasCurrentSha: Boolean(row?.current_sha), - latestRevisionSha: latestCommit?.sha ?? null, - modifiedAt: row?.modified_at ?? null, - modifiedAgeDays: daysFromSecondsAgo(secondsAgo(row?.modified_at)), - referencesCount: row?.references_count ?? 0, - lastSyncedAt: row?.last_synced_at ?? null, - lastSyncedAgeDays: daysFromSecondsAgo(secondsAgo(row?.last_synced_at)), - syncStatus: row?.sync_status ?? null, - }, - frontmatter: { - present: Boolean(rendered.frontmatter && Object.keys(rendered.frontmatter).length), - keys: rendered.frontmatter ? Object.keys(rendered.frontmatter).sort() : [], - model: frontmatterString(rendered.frontmatter, 'model'), - allowedTools, - capabilityScopes: capability.scopes, - mcpServers: capability.mcpServers, - }, + repoSkillCount, + }), + displayName: skill.displayName, + authorName: ownerRow?.name ?? null, + githubUrl, + skillPath: rendered.skillPath, + branch, + resolutionStatus: rendered.status, + sourceGone, + contentHtml: rendered.html, + dependencies: rendered.dependencies, + frontmatter: rendered.frontmatter, + raw: rendered.raw, + assets: selectedAssets.files, + assetCount: selectedAssets.total, + curators, + description, + license, + stars: row?.stars ?? 0, + forks: row?.forks ?? 0, + pushedAt: pushedAtIso, + createdAt: createdAtIso, + maturity: computeMaturity(row?.repo_created_at ?? null, row?.pushed_at ?? null), + tier: resolveTier(skill.owner, skill.repo), + sourceFacts: { + description: { + present: Boolean(description?.trim()), + length: description?.trim().length ?? 0, + source: frontmatterString(rendered.frontmatter, 'description') ? 'frontmatter' : skill.description ? 'repository' : null, }, - tags, - keywords, - // Deliberately top-level, not under `seo`: likes are displayed and back - // ?sort=likes, but never feed indexability or trust (ADR-0003). - likeCount: row?.like_count ?? 0, - faqs: faqRow?.payload.faqs ?? [], - summary: summaryRow?.payload?.text - ? { text: summaryRow.payload.text } - : null, - provenance: { - owner: skill.owner, - repo: skill.repo, - branch, + repository: { + pushedAt: pushedAtIso, + pushedAgeDays: daysFromSecondsAgo(isoToSecondsAgo(pushedAtIso)), + createdAt: createdAtIso, + stars: row?.stars ?? 0, + forks: row?.forks ?? 0, + defaultBranch: branch, + }, + source: { + resolved: sourceResolved, + gone: sourceGone, + resolutionStatus: rendered.status, skillPath: rendered.skillPath, - sourceCommitSha, - sourceCommitUrl: sourceCommitSha - ? `${githubUrl}/commit/${sourceCommitSha}` - : null, - skillFileUrl: githubSkillFileUrl({ - owner: source.owner, - repo: source.repo, - skillPath: rendered.skillPath, - ref: sourceCommitSha ?? branch, - }), - historyUrl: rendered.skillPath - ? `${githubUrl}/commits/${branch}/${rendered.skillPath}` - : null, + currentSha: row?.current_sha ?? null, + hasCurrentSha: Boolean(row?.current_sha), + latestRevisionSha: latestCommit?.sha ?? null, modifiedAt: row?.modified_at ?? null, + modifiedAgeDays: daysFromSecondsAgo(secondsAgo(row?.modified_at)), referencesCount: row?.references_count ?? 0, lastSyncedAt: row?.last_synced_at ?? null, + lastSyncedAgeDays: daysFromSecondsAgo(secondsAgo(row?.last_synced_at)), syncStatus: row?.sync_status ?? null, }, - seo: { - indexScore: row?.seo_index_score ?? 0, - indexable: row?.seo_indexable === 1, - reasons: row?.seo_index_reasons ? JSON.parse(row.seo_index_reasons) as string[] : [], - syncedAt: row?.seo_index_synced_at ?? null, - curatorCount: row?.curator_count ?? 0, - curatorReasonCount: row?.curator_reason_count ?? 0, - approvedSocialCount: row?.approved_social_count ?? 0, - authorSocialCount: row?.author_social_count ?? 0, + frontmatter: { + present: Boolean(rendered.frontmatter && Object.keys(rendered.frontmatter).length), + keys: rendered.frontmatter ? Object.keys(rendered.frontmatter).sort() : [], + model: frontmatterString(rendered.frontmatter, 'model'), + allowedTools, + capabilityScopes: capability.scopes, + mcpServers: capability.mcpServers, }, - trust: { - tier: row?.trust_tier ?? 'untrusted', - source: row?.trust_source ?? 'computed', - score: row?.trust_score ?? 0, - reasons: row?.trust_reasons ? JSON.parse(row.trust_reasons) as string[] : [], - syncedAt: row?.trust_synced_at ?? null, - }, - duplicateGroup, - } + }, + tags, + keywords, + // Deliberately top-level, not under `seo`: likes are displayed and back + // ?sort=likes, but never feed indexability or trust (ADR-0003). + likeCount: row?.like_count ?? 0, + faqs: faqRow?.payload.faqs ?? [], + summary: summaryRow?.payload?.text + ? { text: summaryRow.payload.text } + : null, + provenance: { + owner: skill.owner, + repo: skill.repo, + branch, + skillPath: rendered.skillPath, + sourceCommitSha, + sourceCommitUrl: sourceCommitSha + ? `${githubUrl}/commit/${sourceCommitSha}` + : null, + skillFileUrl: githubSkillFileUrl({ + owner: source.owner, + repo: source.repo, + skillPath: rendered.skillPath, + ref: sourceCommitSha ?? branch, + }), + historyUrl: rendered.skillPath + ? `${githubUrl}/commits/${branch}/${rendered.skillPath}` + : null, + modifiedAt: row?.modified_at ?? null, + referencesCount: row?.references_count ?? 0, + lastSyncedAt: row?.last_synced_at ?? null, + syncStatus: row?.sync_status ?? null, + }, + seo: { + indexScore: row?.seo_index_score ?? 0, + indexable: row?.seo_indexable === 1, + reasons: row?.seo_index_reasons ? JSON.parse(row.seo_index_reasons) as string[] : [], + syncedAt: row?.seo_index_synced_at ?? null, + curatorCount: row?.curator_count ?? 0, + curatorReasonCount: row?.curator_reason_count ?? 0, + approvedSocialCount: row?.approved_social_count ?? 0, + authorSocialCount: row?.author_social_count ?? 0, + }, + trust: { + tier: row?.trust_tier ?? 'untrusted', + source: row?.trust_source ?? 'computed', + score: row?.trust_score ?? 0, + reasons: row?.trust_reasons ? JSON.parse(row.trust_reasons) as string[] : [], + syncedAt: row?.trust_synced_at ?? null, + }, + duplicateGroup, + } - await writeCache(useStorage('cache'), cacheKey, detail, { ttl: DETAIL_CACHE_TTL }) - return detail - }, -}) + return detail +} // Skill detail data is public and changes only when indexing or social counts // update. A read-through KV cache keeps popular links from repeating every D1 -// lookup and Markdown render for each reader. The cache writes go through -// `writeCache` because Nitro's own route cache called storage.setItem bare, -// so a KV `KV PUT failed: 429` escaped the cached handler as a 500 (Sentry -// SKILLD-1V). Losing the entry costs one repeat computation; propagating the -// rejection costs the user the page. +// lookup and Markdown render for each reader. `cached` restores what Nitro's +// route cache used to provide here (maxAge 60, staleMaxAge 300, swr) without +// the bare `storage.setItem` that let a KV `KV PUT failed: 429` escape the +// handler as a 500 (Sentry SKILLD-1V): a fresh entry is served as-is, a stale +// entry is served while one background refresh recomputes, and a dead entry +// recomputes once per key per isolate. Without that stale shield every +// concurrent miss after the 60s TTL re-runs about six D1 queries plus a +// possible live GitHub render, and cold-key bursts are the documented D1 +// overload mechanism on these routes (Sentry SKILLD-G/H/J/K/M/N/P/Q). export default skillDetailHandler interface RenderedView { diff --git a/shared/server/cache.ts b/shared/server/cache.ts index 25baf042..8983a981 100644 --- a/shared/server/cache.ts +++ b/shared/server/cache.ts @@ -29,6 +29,115 @@ export async function writeCache( } } +/** + * What {@link cached} stores for one key: the computed value plus the epoch + * seconds it was computed at. Freshness is decided from `t`, so the storage + * TTL only has to carry the entry through the whole fresh + stale window. + */ +interface SwrEntry { + v: T + t: number +} + +function isSwrEntry(value: unknown): value is SwrEntry { + return typeof value === 'object' && value !== null + && 'v' in value && 't' in value + && typeof (value as SwrEntry).t === 'number' +} + +export interface CachedOptions { + storage: ReadThroughCache + key: string + /** Seconds a stored value is served as-is. */ + ttlSeconds: number + /** Extra seconds a stale value is served while one refresh recomputes. Zero means the entry is dead at TTL expiry. */ + staleSeconds?: number + /** The full computation a cache miss must run. */ + compute: () => Promise + /** + * Runs the background refresh promise when a stale value is served. + * Pass the event's `waitUntil` wrapper so Workers keeps the refresh alive; + * the default drops it to fire-and-forget. + */ + schedule?: (promise: Promise) => void + /** Epoch seconds clock. */ + now?: () => number +} + +/** + * One in-flight computation per cache key. + * + * A bare read-through cache turns every TTL expiry into a thundering herd: + * after the entry dies, each concurrent request re-runs the whole compute + * until the first write lands, and on these routes a compute is about six D1 + * queries plus a possible live GitHub render. The ops triage ledger + * attributes recurring D1 overload bursts (Sentry SKILLD-G/H/J/K/M/N/P/Q) to + * exactly that shape, and Nitro's `defineCachedEventHandler` had the same + * hole covered by `swr: true` + `staleMaxAge` before the detail route left it + * (Sentry SKILLD-1V). This helper restores both halves without the bare + * `setItem` that made a KV 429 a 500: + * + * - fresh (`age < ttlSeconds`): serve the stored value. + * - stale (`age < ttlSeconds + staleSeconds`): serve the stored value and + * refresh in the background through `schedule`. + * - dead or absent: recompute, awaited. + * + * All three paths funnel into one shared promise per key, so concurrent + * callers share a single computation, a single D1 pass, and a single KV + * write, which also keeps the write under KV's one-write-per-second limit. + * The map is per isolate, so the guarantee is per isolate; that is still the + * difference between N computes and one. + */ +const inflightComputes = new Map>() + +export function cached(options: CachedOptions): Promise { + const { key, compute } = options + const freshSeconds = options.ttlSeconds + const maxAgeSeconds = freshSeconds + (options.staleSeconds ?? 0) + const nowSeconds = options.now ?? (() => Math.floor(Date.now() / 1000)) + + function computeOnce(): Promise { + const pending = inflightComputes.get(key) + if (pending) + return pending as Promise + const promise = (async () => { + const value = await compute() + await writeCache(options.storage, key, { v: value, t: nowSeconds() } satisfies SwrEntry, { ttl: maxAgeSeconds }) + return value + })() + inflightComputes.set(key, promise) + // Both callbacks return normally, so this derived promise never rejects + // and the original rejection still reaches every caller. + void promise.then(() => inflightComputes.delete(key), () => inflightComputes.delete(key)) + return promise + } + + return (async () => { + const entry = await readCache>(options.storage, key) + if (isSwrEntry(entry)) { + const age = nowSeconds() - entry.t + if (age < freshSeconds) + return entry.v + if (age < maxAgeSeconds) { + const refresh = computeOnce().catch((error: unknown) => { + emitOperationalEvent(createWideEvent({ + 'operation': 'swr-refresh', + 'outcome': 'failed', + 'cache.key': key, + 'reason': error instanceof Error ? error.message : String(error), + })) + }) + if (options.schedule) + options.schedule(refresh) + else + void refresh + return entry.v + } + } + return computeOnce() + })() +} + /** * The read half of unstorage's `Storage`, structurally typed for the same * reason as {@link CacheStorage}. diff --git a/test/unit/cache-write-best-effort.test.ts b/test/unit/cache-write-best-effort.test.ts index e74952a5..b791fef3 100644 --- a/test/unit/cache-write-best-effort.test.ts +++ b/test/unit/cache-write-best-effort.test.ts @@ -113,26 +113,11 @@ describe('skill detail route cache', () => { let harness: SqliteD1 let handler: (event: H3Event) => Promise> - // Nitro's cached handler: on a miss it computes the response and awaits - // storage.setItem, so a KV PUT rejection travels out of the handler as a - // 500 (Sentry SKILLD-1V). This stub keeps that documented behaviour honest, - // so the regression test fails again if the route ever returns to - // defineCachedEventHandler. - vi.stubGlobal('defineCachedEventHandler', ( - handler: (event: H3Event) => Promise, - opts: { getKey: (event: H3Event) => string }, - ) => { - return async (event: H3Event) => { - const storage = useStorage('cache') - const key = opts.getKey(event) - const cached = await storage.getItem(key) - if (cached != null) - return cached - const result = await handler(event) - await storage.setItem(key, result, { ttl: 60 }) - return result - } - }) + // Nitro's cached handler used to guard this route and awaited a bare + // storage.setItem, so a KV PUT rejection travelled out of the handler as a + // 500 (Sentry SKILLD-1V). The route now caches through `cached`, whose + // writes go through the best-effort `writeCache`; this proves the old + // failure mode stays dead. vi.stubGlobal('defineEventHandler', (handler: unknown) => handler) vi.stubGlobal('createError', (input: Record) => Object.assign(new Error(String(input.message)), input)) vi.stubGlobal('getUserSession', () => Promise.resolve(null)) @@ -166,8 +151,8 @@ describe('skill detail route cache', () => { expect(body.registryPath).toBe('/gh/ericzakariasson/scandinavian-design') expect(setItem).toHaveBeenCalledWith( expect.stringContaining('skills:detail:'), - expect.objectContaining({ registryPath: '/gh/ericzakariasson/scandinavian-design' }), - { ttl: 60 }, + expect.objectContaining({ v: expect.objectContaining({ registryPath: '/gh/ericzakariasson/scandinavian-design' }), t: expect.any(Number) }), + { ttl: 360 }, ) }) diff --git a/test/unit/skill-detail-swr.test.ts b/test/unit/skill-detail-swr.test.ts new file mode 100644 index 00000000..8ce6625c --- /dev/null +++ b/test/unit/skill-detail-swr.test.ts @@ -0,0 +1,114 @@ +import type { H3Event } from 'h3' +import type { SqliteD1 } from './helpers/d1-sqlite' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' + +// The skill detail route lost Nitro's stale-while-revalidate shield when it +// moved to a manual KV read-through cache (Sentry SKILLD-1V fix). Without +// SWR or single-flight, every concurrent request that lands after the 60s +// TTL expires re-runs the full handler, about six D1 queries plus a possible +// live GitHub render, and the ops triage ledger attributes recurring D1 +// overload bursts (SKILLD-G/H/J/K/M/N/P/Q) to exactly this shape on these +// routes. These tests pin the replacement behaviour: one computation per key +// at a time, with stale entries served while a single refresh recomputes. +describe('skill detail SWR cache', () => { + const NOW_SEC = Math.floor(Date.now() / 1000) + const slug = 'ericzakariasson/scandinavian-design/alpha' + const cacheKey = `skills:detail:v1:${slug}` + let harness: SqliteD1 + let handler: (event: H3Event) => Promise> + let prepareCalls: number + + vi.stubGlobal('defineEventHandler', (handler: unknown) => handler) + vi.stubGlobal('createError', (input: Record) => Object.assign(new Error(String(input.message)), input)) + vi.stubGlobal('getUserSession', () => Promise.resolve(null)) + vi.stubGlobal('getRouterParam', (_event: unknown, key: string) => (key === 'slug' ? slug : undefined)) + + beforeEach(async () => { + vi.resetModules() + harness = createSqliteD1(allMigrations()) + prepareCalls = 0 + const db = harness.db as unknown as { prepare: (sql: string) => unknown } + const realPrepare = db.prepare.bind(db) + db.prepare = (sql: string) => { + prepareCalls++ + return realPrepare(sql) + } + + harness.raw.prepare(`INSERT INTO repos (owner, repo) VALUES ('ericzakariasson', 'scandinavian-design')`).run() + const raw = `---\nname: alpha\ndescription: A alpha skill.\n---\n\nBody of alpha.\n` + harness.raw.prepare( + `INSERT INTO skills (owner, repo, name, slug, display_name, source_resolved, + rendered_skill_path, rendered_status, rendered_raw, rendered_html, rendered_at) + VALUES ('ericzakariasson', 'scandinavian-design', 'alpha', ?, 'alpha', 1, + 'skills/alpha/SKILL.md', 'ok', ?, '

ok

', ?)`, + ).run(slug, raw, NOW_SEC) + handler = (await import('../../layers/registry/server/api/skills/[...slug].get')).default + }) + + it('shares one computation between concurrent requests when the entry has expired', async () => { + // The 60s TTL has passed: KV answers nothing, every request is a miss. + const setItem = vi.fn(async () => {}) + vi.stubGlobal('useStorage', () => ({ + getItem: async () => null, + setItem, + })) + const detailWrites = () => setItem.mock.calls.filter(([key]) => String(key).startsWith('skills:detail:')).length + + await handler(event()) + const singleRunQueries = prepareCalls + + const writesBeforeBurst = detailWrites() + const beforeBurst = prepareCalls + const results = await Promise.all(Array.from({ length: 8 }, () => handler(event()))) + const burstQueries = prepareCalls - beforeBurst + + expect(new Set(results).size).toBe(1) + expect(burstQueries).toBeLessThanOrEqual(singleRunQueries) + expect(detailWrites() - writesBeforeBurst).toBe(1) + }) + + it('serves a stale entry while exactly one refresh recomputes in the background', async () => { + const cacheMap = new Map() + const setItem = vi.fn(async (key: string, value: unknown) => { + cacheMap.set(key, value) + }) + vi.stubGlobal('useStorage', () => ({ + getItem: async (key: string) => cacheMap.get(key) ?? null, + setItem, + })) + + // Control run: one full compute, stored fresh in the cache. + await handler(event()) + const singleRunQueries = prepareCalls + const fresh = cacheMap.get(cacheKey) as { v: Record, t: number } + expect(fresh.v.registryPath).toBe('/gh/ericzakariasson/scandinavian-design') + + // Age the entry past the 60s TTL, into the 5 minute stale window. + const staleT = Math.floor(Date.now() / 1000) - 61 + cacheMap.set(cacheKey, { v: fresh.v, t: staleT }) + + const beforeBurst = prepareCalls + const results = await Promise.all(Array.from({ length: 8 }, () => handler(event()))) + + // Every request is served the stale value immediately. + expect(results.every(result => result === fresh.v)).toBe(true) + + // One background refresh recomputes and writes the entry back. + await vi.waitFor(() => { + const written = cacheMap.get(cacheKey) as { v: Record, t: number } + expect(written.t).toBeGreaterThan(staleT) + }) + expect(prepareCalls - beforeBurst).toBeLessThanOrEqual(singleRunQueries) + const refreshed = cacheMap.get(cacheKey) as { v: Record, t: number } + expect(refreshed.v.registryPath).toBe('/gh/ericzakariasson/scandinavian-design') + expect(refreshed.t).toBeGreaterThanOrEqual(fresh.t) + }) + + function event(): H3Event { + return { + context: { platform: { db: harness.db } }, + node: { req: { headers: {} } }, + } as unknown as H3Event + } +}) From 5597529f33d3b431350e265892e4f36de30b8ad4 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Sat, 5 Sep 2026 13:14:51 +1000 Subject: [PATCH 4/5] fix(registry): declare the cache.key wide event field The SWR helper emits `cache.key` on a failed background refresh. The field was never added to `wideEvents.fields`, so nuxt-wide-events rejected it at build time and vue-tsc rejected the same literal as TS2353. Both CI jobs failed on one missing declaration. Claude-Session: https://claude.ai/code/session_01Xf6fnPYRB2nFxHA2i8DT5h --- nuxt.config.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/nuxt.config.ts b/nuxt.config.ts index 065ab1f0..06420929 100644 --- a/nuxt.config.ts +++ b/nuxt.config.ts @@ -78,6 +78,7 @@ export default defineNuxtConfig({ fields: [ 'attempt', 'batch.count', + 'cache.key', 'cache.readFailed', 'cache.writeFailed', 'eligible.count', From 3414fc38b94a4862e27ced7ac0ba74937bb2bd81 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Sat, 5 Sep 2026 22:22:58 +1000 Subject: [PATCH 5/5] chore(ci): trigger a fresh run after the runner memory fix The queued run aged past the supervisor's six hour verification window while skilld.dev CI was down. An aged run is now ignored, so this branch needs a fresh one. Claude-Session: https://claude.ai/code/session_01Xf6fnPYRB2nFxHA2i8DT5h