From d37f4be30942a1374bd22878b30307e6e9bd5421 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Sat, 10 Oct 2026 19:28:03 +1100 Subject: [PATCH 1/5] feat(skills): surface private frontmatter validation warnings --- COPY.md | 5 ++ layers/identity/app/pages/me/index.vue | 47 ++++++++++ layers/identity/app/queries/account.ts | 6 ++ layers/identity/server/api/me/email.patch.ts | 20 +++++ .../identity/server/api/me/validation.get.ts | 16 ++++ .../server/presenters/skill-validation.ts | 5 ++ .../identity/server/routes/auth/github.get.ts | 26 +++++- .../server/utils/skill-validation-email.ts | 51 +++++++++++ .../identity/server/utils/skill-validation.ts | 61 +++++++++++++ layers/identity/shared/contracts/account.ts | 15 ++++ layers/registry/server/utils/sync-repo.ts | 10 ++- migrations/0152_skill_validation_delivery.sql | 9 ++ shared/skill-validation-copy.ts | 7 ++ shared/skill-validation.ts | 89 +++++++++++++++++++ test/unit/account-skill-validation.test.ts | 74 +++++++++++++++ test/unit/skill-validation.test.ts | 48 ++++++++++ test/unit/sync-repo-correctness.test.ts | 2 +- test/unit/sync-repo-freshness.test.ts | 1 + test/unit/sync-repo-streaming.test.ts | 2 +- test/unit/sync-repo-unreadable-blob.test.ts | 2 +- 20 files changed, 489 insertions(+), 7 deletions(-) create mode 100644 layers/identity/server/api/me/validation.get.ts create mode 100644 layers/identity/server/presenters/skill-validation.ts create mode 100644 layers/identity/server/utils/skill-validation-email.ts create mode 100644 layers/identity/server/utils/skill-validation.ts create mode 100644 migrations/0152_skill_validation_delivery.sql create mode 100644 shared/skill-validation-copy.ts create mode 100644 shared/skill-validation.ts create mode 100644 test/unit/account-skill-validation.test.ts create mode 100644 test/unit/skill-validation.test.ts diff --git a/COPY.md b/COPY.md index 1e052a67..6ae2b39b 100644 --- a/COPY.md +++ b/COPY.md @@ -11,6 +11,11 @@ contradicts this file is a bug. ## Canonical assets +Skill validation in `/me` uses "Skill validation" and "Some of your Skills have validation issues." +The action is "View validation issues". Source links use "Open SKILL.md". +Email summaries use the subject "Your Skills have validation issues" and link to `/me?view=repositories`. +Missing optional fields are not issues. Claude Code extensions are portability notices. + Comparison navigation uses “Compare Humanizer, Stop Slop, and No AI Slop” on the homepage, writing track, and featured Skill pages. The writing comparison credits “Harlan Wilton” as its author, with an agent research and drafting disclosure. diff --git a/layers/identity/app/pages/me/index.vue b/layers/identity/app/pages/me/index.vue index be5f3010..7a988f6a 100644 --- a/layers/identity/app/pages/me/index.vue +++ b/layers/identity/app/pages/me/index.vue @@ -8,6 +8,7 @@ import type { IdentitySubscriptionRef, } from '../../../shared/contracts/account' import type { StarsSyncResponse } from '../../utils/sync-starred-repos' +import { SKILL_VALIDATION_COPY } from '#shared/skill-validation-copy' import { accountDeletionConfirmed } from '../../../shared/contracts/account' import SkillgenRepositories from '../../components/_SkillgenRepositories.vue' import { identityAccountQueries, identityAccountQueryOptions } from '../../queries/account' @@ -27,6 +28,8 @@ interface LikedSkill { const { data: me, error: accountError, status: accountStatus, refresh: retryAccount } = await useNuxtRpcQuery(identityAccountQueries.me(), identityAccountQueryOptions) const { data: subs, error: subscriptionsError, status: subscriptionsStatus, refresh: retrySubscriptions } = await useNuxtRpcQuery(identityAccountQueries.subscriptions(), identityAccountQueryOptions) +const { data: validation, error: validationError, status: validationStatus, refresh: retryValidation } = await useNuxtRpcQuery(identityAccountQueries.validation()) +const invalidSkillCount = computed(() => validation.value?.items.filter(item => item.issues.some(issue => issue.severity === 'error')).length ?? 0) const { data: likes, error: likesError, @@ -260,6 +263,14 @@ async function deleteAccount() {
+

{{ viewTitle }}

@@ -484,6 +495,42 @@ async function deleteAccount() {
+
+

+ {{ SKILL_VALIDATION_COPY.heading }} +

+ +

+ Checking your Skills +

+
+

+ {{ validation.checked }} {{ validation.checked === 1 ? 'Skill' : 'Skills' }} checked. GitHub forks are excluded. +

+

+ {{ validation.pending }} {{ validation.pending === 1 ? 'Skill awaits' : 'Skills await' }} source or GitHub metadata refresh. +

+

+ No frontmatter issues found in the checked Skills. +

+
+ + {{ item.repository }}/{{ item.name }} + +
    +
  • + {{ issue.message }} +
    • +
+ {{ SKILL_VALIDATION_COPY.source }} +
+
+

{{ watchedRepositoryCount }} {{ watchedRepositoryCount === 1 ? 'repository supports' : 'repositories support' }} your skill updates.

diff --git a/layers/identity/app/queries/account.ts b/layers/identity/app/queries/account.ts index 478509dc..7cffd322 100644 --- a/layers/identity/app/queries/account.ts +++ b/layers/identity/app/queries/account.ts @@ -9,6 +9,7 @@ import { identityMutationResponseSchema, identityOwnedRepoScanResponseSchema, identityPrivacyPatchBodySchema, + identitySkillValidationSchema, identitySubscriptionsSchema, } from '../../shared/contracts/account' @@ -27,6 +28,11 @@ export const identityAccountQueries = defineNuxtQueryGroup('identity:account', { path: '/api/me/subscriptions', response: identitySubscriptionsSchema, }), + validation: () => defineNuxtRpcQuery({ + key: ['identity', 'validation'], + path: '/api/me/validation', + response: identitySkillValidationSchema, + }), saveEmail: () => defineNuxtRpcMutation({ body: identityEmailPatchBodySchema, method: 'PATCH', diff --git a/layers/identity/server/api/me/email.patch.ts b/layers/identity/server/api/me/email.patch.ts index 487d3d77..3a54628c 100644 --- a/layers/identity/server/api/me/email.patch.ts +++ b/layers/identity/server/api/me/email.patch.ts @@ -2,6 +2,8 @@ import { defineApiHandler } from '#shared/server/handler' import { identityEmailPatchBodySchema, identityMutationResponseSchema } from '../../../shared/contracts/account' import { authenticated } from '../../policies/authenticated' import { planAccountSettings, updateAccountSettings } from '../../utils/account-settings' +import { sendEmailWithEnv } from '../../utils/email' +import { sendSkillValidationSummary } from '../../utils/skill-validation-email' import { requireUserRow } from '../../utils/users' export default defineApiHandler({ @@ -21,6 +23,24 @@ export default defineApiHandler({ if (plan._tag === 'MissingAddress') throw createError({ statusCode: 400, message: 'Add a valid email address to receive emails' }) await updateAccountSettings(platform.db, u.id, plan.columns) + const config = useRuntimeConfig(event) + const updated = await requireUserRow(event) + const summary = sendSkillValidationSummary({ + db: platform.db, + user: updated, + now: Math.floor(Date.now() / 1000), + send: input => sendEmailWithEnv(platform.env, { ...input, from: config.email.from }), + }).then((outcome) => { + if (outcome === 'rejected' || outcome === 'uncertain') + emitOperationalEvent(createWideEvent({ operation: 'skill-validation-email', outcome: 'failed' })) + }).catch(() => { + emitOperationalEvent(createWideEvent({ operation: 'skill-validation-email', outcome: 'failed' })) + }) + const context = (event.context as { cloudflare?: { context?: { waitUntil?: (p: Promise) => void } } }).cloudflare?.context + if (context?.waitUntil) + context.waitUntil(summary) + else + await summary return { ok: true as const } }, }) diff --git a/layers/identity/server/api/me/validation.get.ts b/layers/identity/server/api/me/validation.get.ts new file mode 100644 index 00000000..aab7ab28 --- /dev/null +++ b/layers/identity/server/api/me/validation.get.ts @@ -0,0 +1,16 @@ +import { defineApiHandler } from '#shared/server/handler' +import { identitySkillValidationSchema } from '../../../shared/contracts/account' +import { authenticated } from '../../policies/authenticated' +import { skillValidationPresenter } from '../../presenters/skill-validation' +import { loadAccountSkillValidation } from '../../utils/skill-validation' +import { requireUserRow } from '../../utils/users' + +export default defineApiHandler({ + policy: [authenticated], + response: identitySkillValidationSchema, + presenter: skillValidationPresenter, + handler: async ({ event, platform }) => { + const user = await requireUserRow(event) + return loadAccountSkillValidation(platform.db, user.login) + }, +}) diff --git a/layers/identity/server/presenters/skill-validation.ts b/layers/identity/server/presenters/skill-validation.ts new file mode 100644 index 00000000..643cb910 --- /dev/null +++ b/layers/identity/server/presenters/skill-validation.ts @@ -0,0 +1,5 @@ +import type { AccountSkillValidation } from '../utils/skill-validation' + +export function skillValidationPresenter(result: AccountSkillValidation): AccountSkillValidation { + return result +} diff --git a/layers/identity/server/routes/auth/github.get.ts b/layers/identity/server/routes/auth/github.get.ts index 70af369b..92f1b606 100644 --- a/layers/identity/server/routes/auth/github.get.ts +++ b/layers/identity/server/routes/auth/github.get.ts @@ -2,8 +2,10 @@ import type { H3Event } from 'h3' import { useSession } from 'h3' import { z } from 'zod' import { parseReturnTo } from '#shared/return-to' +import { sendEmailWithEnv } from '../../utils/email' import { fetchVerifiedPrimaryEmail } from '../../utils/github-emails' import { ownedRepoScanWarning, scanOwnedRepos } from '../../utils/scan-owned-repos' +import { sendSkillValidationSummary } from '../../utils/skill-validation-email' import { upsertUserFromGithub } from '../../utils/users' import { handleWatchAction } from '../../utils/watch-actions' @@ -62,6 +64,18 @@ const githubHandler = defineOAuthGitHubEventHandler({ clientId: platform.env.NUXT_OAUTH_GITHUB_CLIENT_ID, scopes, }) + const config = useRuntimeConfig(event) + const sendValidationSummary = () => sendSkillValidationSummary({ + db: platform.db, + user: row, + now: Math.floor(Date.now() / 1000), + send: input => sendEmailWithEnv(platform.env, { ...input, from: config.email.from }), + }).then((outcome) => { + if (outcome === 'rejected' || outcome === 'uncertain') + emitOperationalEvent(createWideEvent({ operation: 'skill-validation-email', outcome: 'failed' })) + }).catch(() => { + emitOperationalEvent(createWideEvent({ operation: 'skill-validation-email', outcome: 'failed' })) + }) // First sign-in: scan the account's public repositories for SKILL.md and // index what it finds. This is how the registry grows, and the files are @@ -76,7 +90,7 @@ const githubHandler = defineOAuthGitHubEventHandler({ }).then((result) => { if (ownedRepoScanWarning(result)) emitOperationalEvent(createWideEvent({ operation: 'oauth-owned-repo-scan', outcome: 'incomplete' })) - }).catch(() => { + }).then(sendValidationSummary).catch(() => { emitOperationalEvent(createWideEvent({ operation: 'oauth-owned-repo-scan', outcome: 'failed' })) }) const cfCtx = (event.context as { cloudflare?: { context?: { waitUntil?: (p: Promise) => void } } }).cloudflare?.context @@ -85,6 +99,16 @@ const githubHandler = defineOAuthGitHubEventHandler({ else void scanPromise } + else { + const summaryPromise = sendValidationSummary().catch(() => { + emitOperationalEvent(createWideEvent({ operation: 'skill-validation-email', outcome: 'failed' })) + }) + const cfCtx = (event.context as { cloudflare?: { context?: { waitUntil?: (p: Promise) => void } } }).cloudflare?.context + if (cfCtx?.waitUntil) + cfCtx.waitUntil(summaryPromise) + else + await summaryPromise + } const intent = await loginIntentSession(event) const action = typeof intent.data.action === 'string' ? intent.data.action : '' diff --git a/layers/identity/server/utils/skill-validation-email.ts b/layers/identity/server/utils/skill-validation-email.ts new file mode 100644 index 00000000..e74346e1 --- /dev/null +++ b/layers/identity/server/utils/skill-validation-email.ts @@ -0,0 +1,51 @@ +import type { SendEmailInput, SendEmailResult } from './email' +import type { UserRow } from './users' +import { SKILL_VALIDATION_COPY } from '#shared/skill-validation-copy' +import { loadAccountSkillValidation } from './skill-validation' +import { weeklyDeliveryActive } from './weekly-select' + +interface ValidationEmailInput { + db: D1Database + user: UserRow + send: (input: SendEmailInput) => Promise + now: number +} + +/** One consented connection summary, claimed atomically before delivery. */ +export async function sendSkillValidationSummary(input: ValidationEmailInput): Promise<'skipped' | SendEmailResult['_tag']> { + const { db, user, send, now } = input + if (!user.repo_indexing || (!user.email_opt_in && !weeklyDeliveryActive(user))) + return 'skipped' + const to = (user.digest_email ?? user.email ?? '').trim() + if (!to) + return 'skipped' + const summary = await loadAccountSkillValidation(db, user.login) + const invalid = summary.items.filter(item => item.issues.some(issue => issue.severity === 'error')) + if (!invalid.length) + return 'skipped' + const claimed = await db.prepare(` + INSERT INTO skill_validation_email_deliveries (user_id, status, attempted_at) + VALUES (?1, 'claimed', ?2) + ON CONFLICT(user_id) DO UPDATE SET status = 'claimed', attempted_at = excluded.attempted_at, error = NULL + WHERE skill_validation_email_deliveries.status = 'rejected' + RETURNING user_id + `).bind(user.id, now).first<{ user_id: number }>() + if (!claimed) + return 'skipped' + const link = 'https://skilld.dev/me?view=repositories' + const count = `${invalid.length} ${invalid.length === 1 ? 'Skill has' : 'Skills have'} frontmatter issues.` + const outcome = await send({ + to, + subject: SKILL_VALIDATION_COPY.subject, + html: `

${SKILL_VALIDATION_COPY.subject}

${count}

${SKILL_VALIDATION_COPY.action}

`, + text: `${count}\n\n${SKILL_VALIDATION_COPY.action}: ${link}`, + }).catch((error): SendEmailResult => ({ + // A thrown send may have reached the provider. Do not risk duplicate delivery. + _tag: 'uncertain', + error: error instanceof Error ? error.message : String(error), + })) + await db.prepare(`UPDATE skill_validation_email_deliveries SET status = ?1, error = ?2 WHERE user_id = ?3`) + .bind(outcome._tag, outcome._tag === 'accepted' ? null : outcome.error, user.id) + .run() + return outcome._tag +} diff --git a/layers/identity/server/utils/skill-validation.ts b/layers/identity/server/utils/skill-validation.ts new file mode 100644 index 00000000..e1f22b37 --- /dev/null +++ b/layers/identity/server/utils/skill-validation.ts @@ -0,0 +1,61 @@ +import type { SkillValidationIssue } from '#shared/skill-validation' +import { validateSkillFrontmatter } from '#shared/skill-validation' + +export interface AccountSkillValidation { + checked: number + pending: number + items: Array<{ + repository: string + name: string + sourceUrl: string + issues: SkillValidationIssue[] + }> +} + +interface SourceRow { + repo: string + name: string + rendered_raw: string | null + rendered_skill_path: string | null + rendered_commit_sha: string | null + source_owner: string | null + source_repo: string | null + is_fork: number | null +} + +/** Only the signed-in account's own Repository sources enter this summary. */ +export async function loadAccountSkillValidation(db: D1Database, login: string): Promise { + const result: AccountSkillValidation = { checked: 0, pending: 0, items: [] } + let after = '' + // Page source bytes so one account cannot exceed D1's response size limit. + for (;;) { + const page = await db.prepare(` + SELECT s.repo, s.name, s.rendered_raw, s.rendered_skill_path, + s.rendered_commit_sha, r.source_owner, r.source_repo, r.is_fork + FROM skills s JOIN repos r ON r.owner = s.owner AND r.repo = s.repo + WHERE s.owner = ?1 COLLATE NOCASE AND s.source_resolved = 1 + AND (r.is_fork IS NULL OR r.is_fork = 0) + AND (s.repo || char(0) || s.name) > ?2 + ORDER BY s.repo, s.name LIMIT 25 + `).bind(login, after).all() + const rows = page.results ?? [] + for (const row of rows) { + if (row.is_fork === null || !row.rendered_raw || !row.rendered_skill_path || !row.rendered_commit_sha) { + result.pending++ + continue + } + result.checked++ + const issues = validateSkillFrontmatter(row.rendered_raw, row.rendered_skill_path, row.source_repo ?? row.repo) + if (issues.length) { + const owner = row.source_owner ?? login + const repo = row.source_repo ?? row.repo + const sourceUrl = `https://github.com/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/blob/${encodeURIComponent(row.rendered_commit_sha)}/${row.rendered_skill_path.split('/').map(encodeURIComponent).join('/')}` + result.items.push({ repository: `${login}/${row.repo}`, name: row.name, sourceUrl, issues }) + } + } + if (rows.length < 25) + return result + const last = rows.at(-1)! + after = `${last.repo}\0${last.name}` + } +} diff --git a/layers/identity/shared/contracts/account.ts b/layers/identity/shared/contracts/account.ts index 2c2b81ad..1e668f47 100644 --- a/layers/identity/shared/contracts/account.ts +++ b/layers/identity/shared/contracts/account.ts @@ -78,6 +78,21 @@ export const identityMutationResponseSchema = z.object({ ok: z.literal(true), }) +export const identitySkillValidationSchema = z.object({ + checked: z.number().int().nonnegative(), + pending: z.number().int().nonnegative(), + items: z.array(z.object({ + repository: z.string(), + name: z.string(), + sourceUrl: z.string().url(), + issues: z.array(z.object({ + field: z.string(), + severity: z.enum(['error', 'notice']), + message: z.string(), + })), + })), +}) + /** * Account deletion asks for the GitHub login, typed by hand. A login that does * not match deletes nothing. diff --git a/layers/registry/server/utils/sync-repo.ts b/layers/registry/server/utils/sync-repo.ts index f71d2719..73ba47db 100644 --- a/layers/registry/server/utils/sync-repo.ts +++ b/layers/registry/server/utils/sync-repo.ts @@ -529,6 +529,7 @@ function markRepoSummaryCheckedStatement( SET default_branch = ?, stars = ?, forks = ?, + is_fork = ?, description = ?, pushed_at = ?, repo_created_at = ?, @@ -544,6 +545,7 @@ function markRepoSummaryCheckedStatement( meta.default_branch || 'main', meta.stargazers_count ?? 0, meta.forks_count ?? 0, + typeof meta.fork === 'boolean' ? Number(meta.fork) : null, meta.description?.trim() || null, pushedAt, epoch(meta.created_at), @@ -1011,8 +1013,8 @@ export async function syncRepo( `INSERT INTO repos ( owner, repo, default_branch, stars, forks, description, pushed_at, repo_created_at, repo_meta_synced_at, last_tree_sha, repo_kind, repo_kind_source, - repo_skill_count, broken_since, source_owner, source_repo, repository_id - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, NULL, ?, ?, ?) + repo_skill_count, broken_since, source_owner, source_repo, repository_id, is_fork + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, NULL, ?, ?, ?, ?) ON CONFLICT(owner, repo) DO UPDATE SET default_branch = excluded.default_branch, stars = excluded.stars, @@ -1029,7 +1031,8 @@ export async function syncRepo( tree_truncated_at = NULL, source_owner = excluded.source_owner, source_repo = excluded.source_repo, - repository_id = excluded.repository_id`, + repository_id = excluded.repository_id, + is_fork = excluded.is_fork`, ).bind( owner, repo, @@ -1047,6 +1050,7 @@ export async function syncRepo( sourceOwner, sourceRepo, repositoryId, + typeof meta.fork === 'boolean' ? Number(meta.fork) : null, ) if (skillFiles.length === 0) { diff --git a/migrations/0152_skill_validation_delivery.sql b/migrations/0152_skill_validation_delivery.sql new file mode 100644 index 00000000..6ee55195 --- /dev/null +++ b/migrations/0152_skill_validation_delivery.sql @@ -0,0 +1,9 @@ +-- Unknown fork status stays unknown until GitHub metadata confirms it. +ALTER TABLE repos ADD COLUMN is_fork INTEGER CHECK (is_fork IN (0, 1)); + +CREATE TABLE skill_validation_email_deliveries ( + user_id INTEGER PRIMARY KEY REFERENCES users(id) ON DELETE CASCADE, + status TEXT NOT NULL CHECK (status IN ('claimed', 'accepted', 'rejected', 'uncertain')), + attempted_at INTEGER NOT NULL, + error TEXT +); diff --git a/shared/skill-validation-copy.ts b/shared/skill-validation-copy.ts new file mode 100644 index 00000000..b73f1eca --- /dev/null +++ b/shared/skill-validation-copy.ts @@ -0,0 +1,7 @@ +export const SKILL_VALIDATION_COPY = { + heading: 'Skill validation', + summary: 'Some of your Skills have validation issues.', + action: 'View validation issues', + source: 'Open SKILL.md', + subject: 'Your Skills have validation issues', +} as const diff --git a/shared/skill-validation.ts b/shared/skill-validation.ts new file mode 100644 index 00000000..5674074c --- /dev/null +++ b/shared/skill-validation.ts @@ -0,0 +1,89 @@ +import { parseDocument } from 'yaml' + +export interface SkillValidationIssue { + field: string + severity: 'error' | 'notice' + message: string +} + +const BASE_FIELDS = new Set(['name', 'description', 'license', 'compatibility', 'metadata', 'allowed-tools']) +// https://code.claude.com/docs/en/skills#frontmatter-reference +const CLAUDE_FIELDS = new Set(['argument-hint', 'disable-model-invocation', 'user-invocable', 'model', 'context', 'agent', 'hooks', 'effort']) + +/** Validate source bytes, without the registry's lenient display parser. */ +export function validateSkillFrontmatter(raw: string, path: string, repository: string): SkillValidationIssue[] { + const issues: SkillValidationIssue[] = [] + const add = (field: string, message: string, severity: SkillValidationIssue['severity'] = 'error') => issues.push({ field, message, severity }) + const match = raw.match(/^---\r?\n([\s\S]*?)\r?\n---(?:\r?\n|$)/) + if (!match) { + add('frontmatter', 'Add a YAML frontmatter block before the instructions.') + return issues + } + const document = parseDocument(match[1]!, { uniqueKeys: true, logLevel: 'silent' }) + if (document.errors.length || document.warnings.length) { + add('yaml', 'Fix malformed YAML or duplicate frontmatter fields.') + return issues + } + let mapping: unknown + try { + mapping = document.toJS({ mapAsMap: true, maxAliasCount: 100 }) + } + catch { + // Alias expansion is expected invalid author input, not an infrastructure failure. + add('yaml', 'Replace recursive or excessive YAML aliases.') + return issues + } + if (!(mapping instanceof Map)) { + add('frontmatter', 'Use a YAML mapping for frontmatter.') + return issues + } + const sourceName = mapping.get('name') + const name = typeof sourceName === 'string' ? sourceName.normalize('NFKC') : sourceName + const expected = path === 'SKILL.md' ? repository : path.split('/').at(-2) + if (typeof name !== 'string' || !name || [...name].length > 64 || name !== name.toLowerCase() + || !/^[\p{L}\p{N}]+(?:-[\p{L}\p{N}]+)*$/u.test(name)) { + add('name', 'Use 1 to 64 lowercase letters, numbers, or single hyphens for name.') + } + else if (name.normalize('NFKC') !== expected?.normalize('NFKC')) { + add('name', `Set name to the Skill folder name: ${expected}.`) + } + const description = mapping.get('description') + if (typeof description !== 'string' || !description.trim() || [...description].length > 1024) + add('description', 'Use a nonempty description with at most 1024 characters.') + for (const field of ['license', 'allowed-tools']) { + if (mapping.has(field) && typeof mapping.get(field) !== 'string') + add(field, `Use a string for ${field}.`) + } + if (mapping.has('compatibility')) { + const value = mapping.get('compatibility') + if (typeof value !== 'string' || !value.trim() || [...value].length > 500) + add('compatibility', 'Use 1 to 500 characters for compatibility, or omit it.') + } + if (mapping.has('metadata')) { + const value = mapping.get('metadata') + if (!(value instanceof Map) || [...value].some(([key, item]) => typeof key !== 'string' || typeof item !== 'string')) + add('metadata', 'Use string keys and string values in metadata. Quote version numbers.') + } + let claudeExtensions = false + for (const key of mapping.keys()) { + if (typeof key !== 'string') { + add('frontmatter', 'Use string keys for frontmatter fields.') + } + else if (CLAUDE_FIELDS.has(key)) { + claudeExtensions = true + } + else if (!BASE_FIELDS.has(key)) { + const message = key === 'user_invocable' + ? 'Replace user_invocable with the Claude Code field user-invocable, or omit it.' + : key === 'version' + ? 'Move version into metadata.version and quote its value.' + : `Move the unsupported field ${key} into metadata, or remove it.` + add(key, message) + } + } + if (claudeExtensions) + add('claude-code', 'Claude Code fields may not work in other Agents.', 'notice') + if (raw.split(/\r?\n/).length > 500) + add('body', 'Keep SKILL.md under 500 lines. Move detail into linked reference files.', 'notice') + return issues +} diff --git a/test/unit/account-skill-validation.test.ts b/test/unit/account-skill-validation.test.ts new file mode 100644 index 00000000..e8222eb5 --- /dev/null +++ b/test/unit/account-skill-validation.test.ts @@ -0,0 +1,74 @@ +import type { UserRow } from '../../layers/identity/server/utils/users' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { loadAccountSkillValidation } from '../../layers/identity/server/utils/skill-validation' +import { sendSkillValidationSummary } from '../../layers/identity/server/utils/skill-validation-email' +import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' + +const raw = '---\nname: example\ndescription: Works.\nversion: "1"\n---\nInstructions' + +describe('account Skill validation', () => { + let harness: ReturnType + beforeEach(() => { + harness = createSqliteD1(allMigrations()) + harness.raw.exec(`INSERT INTO users (id,github_id,login,created_at,last_login_at) VALUES (10001,10001,'owner',1,1)`) + }) + afterEach(() => harness.close()) + + function source(owner = 'owner', repo = 'skills', fork: number | null = 0, body = raw, name = 'example') { + harness.raw.prepare(`INSERT OR IGNORE INTO repos (owner,repo,is_fork) VALUES (?,?,?)`).run(owner, repo, fork) + harness.raw.prepare(`INSERT INTO skills (owner,repo,name,display_name,slug,source_resolved,rendered_raw,rendered_skill_path,rendered_commit_sha) + VALUES (?,?,?,?,?,1,?,?,?)`).run(owner, repo, name, name, name, body, `skills/${name}/SKILL.md`, 'commit-sha') + } + + function user(overrides: Partial = {}): UserRow { + return { id: 10001, login: 'owner', email: 'owner@example.com', digest_email: null, email_opt_in: 1, weekly_opt_out: 1, repo_indexing: 1, ...overrides } as UserRow + } + + it('checks only account-owned nonfork sources and reports unknown status as pending', async () => { + source() + source('other', 'private') + source('owner', 'fork', 1) + source('owner', 'unknown', null) + const summary = await loadAccountSkillValidation(harness.db, 'OWNER') + expect(summary).toMatchObject({ checked: 1, pending: 1, items: [{ repository: 'OWNER/skills', name: 'example', sourceUrl: 'https://github.com/OWNER/skills/blob/commit-sha/skills/example/SKILL.md' }] }) + expect(summary.items).toHaveLength(1) + }) + + it('validates every page and clears issues when the current source changes', async () => { + for (let index = 0; index < 31; index++) { + const name = `example-${index}` + source('owner', 'skills', 0, raw.replace('name: example', `name: ${name}`), name) + } + expect((await loadAccountSkillValidation(harness.db, 'owner')).checked).toBe(31) + harness.raw.prepare(`UPDATE skills SET rendered_raw = replace(rendered_raw, 'version: "1"', '')`).run() + expect((await loadAccountSkillValidation(harness.db, 'owner')).items).toEqual([]) + }) + + it('sends one summary after consent and never sends a captured address alone', async () => { + source() + const send = vi.fn(async () => ({ _tag: 'accepted' as const, messageId: 'message-1' })) + expect(await sendSkillValidationSummary({ db: harness.db, user: user({ email_opt_in: 0, weekly_opt_out: 0 }), send, now: 10 })).toBe('skipped') + expect(send).not.toHaveBeenCalled() + await Promise.all([1, 2].map(() => sendSkillValidationSummary({ db: harness.db, user: user(), send, now: 10 }))) + expect(send).toHaveBeenCalledTimes(1) + expect(send.mock.calls[0]?.[0]).toMatchObject({ subject: 'Your Skills have validation issues', text: expect.stringContaining('1 Skill has frontmatter issues.') }) + expect(await sendSkillValidationSummary({ db: harness.db, user: user(), send, now: 20 })).toBe('skipped') + }) + + it('does not email portability notices or opted-out accounts', async () => { + source('owner', 'skills', 0, raw.replace('version: "1"', 'context: fork')) + const send = vi.fn(async () => ({ _tag: 'accepted' as const, messageId: 'message-1' })) + expect(await sendSkillValidationSummary({ db: harness.db, user: user(), send, now: 10 })).toBe('skipped') + expect(await sendSkillValidationSummary({ db: harness.db, user: user({ repo_indexing: 0 }), send, now: 10 })).toBe('skipped') + expect(send).not.toHaveBeenCalled() + }) + + it('retries rejected delivery but preserves an uncertain provider result', async () => { + source() + const send = vi.fn().mockResolvedValueOnce({ _tag: 'rejected', error: 'provider unavailable' }).mockResolvedValueOnce({ _tag: 'uncertain', error: 'missing receipt' }) + expect(await sendSkillValidationSummary({ db: harness.db, user: user(), send, now: 10 })).toBe('rejected') + expect(await sendSkillValidationSummary({ db: harness.db, user: user(), send, now: 20 })).toBe('uncertain') + expect(await sendSkillValidationSummary({ db: harness.db, user: user(), send, now: 30 })).toBe('skipped') + expect(send).toHaveBeenCalledTimes(2) + }) +}) diff --git a/test/unit/skill-validation.test.ts b/test/unit/skill-validation.test.ts new file mode 100644 index 00000000..a9f6a419 --- /dev/null +++ b/test/unit/skill-validation.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from 'vitest' +import { validateSkillFrontmatter } from '../../shared/skill-validation' + +const valid = '---\nname: example\ndescription: Convert HTML when a task needs Markdown.\n---\nInstructions' + +describe('skill frontmatter validation', () => { + it('accepts omitted optional fields and decoded YAML scalars', () => { + expect(validateSkillFrontmatter(valid, 'skills/example/SKILL.md', 'repo')).toEqual([]) + expect(validateSkillFrontmatter(valid.replace('---\nInstructions', `metadata:\n version: "1"\nallowed-tools: Read Bash(git:*)\ncompatibility: "${'😀'.repeat(500)}"\n---\nInstructions`), 'skills/example/SKILL.md', 'repo')).toEqual([]) + }) + + it.each([ + ['name: example\nname: duplicate', 'yaml'], + ['name: [', 'yaml'], + ['name: example\ndescription: true', 'description'], + ['name: Example\ndescription: Works.', 'name'], + ['name: example\ndescription: Works.\nmetadata:\n version: 1', 'metadata'], + ['name: example\ndescription: Works.\nmetadata:\n 1: value', 'metadata'], + ['name: example\ndescription: Works.\nallowed-tools: [Read]', 'allowed-tools'], + ['name: example\ndescription: Works.\nuser_invocable: true', 'user_invocable'], + ['name: example\ndescription: Works.\nversion: "1"', 'version'], + ['name: example\ndescription: Works.\ncompatibility: ""', 'compatibility'], + ])('reports actionable invalid metadata: %s', (frontmatter, field) => { + const result = validateSkillFrontmatter(`---\n${frontmatter}\n---\nInstructions`, 'skills/example/SKILL.md', 'repo') + expect(result.some(issue => issue.field === field && issue.severity === 'error')).toBe(true) + }) + + it('reports missing frontmatter and a mismatched folder', () => { + expect(validateSkillFrontmatter('Instructions', 'skills/example/SKILL.md', 'repo')[0]?.field).toBe('frontmatter') + expect(validateSkillFrontmatter(valid, 'skills/different/SKILL.md', 'repo')).toEqual(expect.arrayContaining([expect.objectContaining({ field: 'name' })])) + }) + + it('keeps documented Claude Code extensions as portability notices', () => { + const result = validateSkillFrontmatter(valid.replace('---\nInstructions', 'context: fork\nagent: Explore\nuser-invocable: false\n---\nInstructions'), 'skills/example/SKILL.md', 'repo') + expect(result.filter(issue => issue.severity === 'error')).toEqual([]) + expect(result).toEqual([expect.objectContaining({ field: 'claude-code', severity: 'notice' })]) + }) + + it('counts Unicode characters and treats long instructions as guidance', () => { + expect(validateSkillFrontmatter(valid.replace('Convert HTML when a task needs Markdown.', '😀'.repeat(1024)), 'skills/example/SKILL.md', 'repo')).toEqual([]) + expect(validateSkillFrontmatter(`${valid}\n${'Line\n'.repeat(500)}`, 'skills/example/SKILL.md', 'repo')).toEqual([expect.objectContaining({ field: 'body', severity: 'notice' })]) + }) + + it('normalizes Unicode names before checking naming constraints', () => { + expect(validateSkillFrontmatter(valid.replace('name: example', 'name: ｅｘａｍｐｌｅ'), 'skills/example/SKILL.md', 'repo')).toEqual([]) + expect(validateSkillFrontmatter(valid.replace('name: example', `name: ${'ff'.repeat(40)}`), `skills/${'ff'.repeat(40)}/SKILL.md`, 'repo')).toEqual(expect.arrayContaining([expect.objectContaining({ field: 'name' })])) + }) +}) diff --git a/test/unit/sync-repo-correctness.test.ts b/test/unit/sync-repo-correctness.test.ts index a87f35b0..91573a1c 100644 --- a/test/unit/sync-repo-correctness.test.ts +++ b/test/unit/sync-repo-correctness.test.ts @@ -695,7 +695,7 @@ function createDatabase(): Database.Database { forks INTEGER NOT NULL DEFAULT 0, pushed_at INTEGER, repo_created_at INTEGER, repo_meta_synced_at INTEGER, description TEXT, last_tree_sha TEXT, repo_kind TEXT NOT NULL DEFAULT 'creator', repo_kind_source TEXT NOT NULL DEFAULT 'computed', repo_skill_count INTEGER NOT NULL DEFAULT 0, - broken_since INTEGER, tree_truncated_at INTEGER, source_owner TEXT, source_repo TEXT, repository_id INTEGER, PRIMARY KEY (owner, repo) + broken_since INTEGER, tree_truncated_at INTEGER, source_owner TEXT, source_repo TEXT, repository_id INTEGER, is_fork INTEGER, PRIMARY KEY (owner, repo) ); CREATE TABLE skills ( name TEXT NOT NULL, owner TEXT NOT NULL, repo TEXT NOT NULL, display_name TEXT NOT NULL, diff --git a/test/unit/sync-repo-freshness.test.ts b/test/unit/sync-repo-freshness.test.ts index 060da180..2c546b37 100644 --- a/test/unit/sync-repo-freshness.test.ts +++ b/test/unit/sync-repo-freshness.test.ts @@ -86,6 +86,7 @@ describe('syncRepo freshness cursor', () => { source_owner TEXT, source_repo TEXT, repository_id INTEGER, + is_fork INTEGER, PRIMARY KEY (owner, repo) ); INSERT INTO repos ( diff --git a/test/unit/sync-repo-streaming.test.ts b/test/unit/sync-repo-streaming.test.ts index 5b4d734f..1bc3a8eb 100644 --- a/test/unit/sync-repo-streaming.test.ts +++ b/test/unit/sync-repo-streaming.test.ts @@ -349,7 +349,7 @@ function createDatabase(): Database.Database { forks INTEGER NOT NULL DEFAULT 0, pushed_at INTEGER, repo_created_at INTEGER, repo_meta_synced_at INTEGER, description TEXT, last_tree_sha TEXT, repo_kind TEXT NOT NULL DEFAULT 'creator', repo_kind_source TEXT NOT NULL DEFAULT 'computed', repo_skill_count INTEGER NOT NULL DEFAULT 0, - broken_since INTEGER, tree_truncated_at INTEGER, source_owner TEXT, source_repo TEXT, repository_id INTEGER, PRIMARY KEY (owner, repo) + broken_since INTEGER, tree_truncated_at INTEGER, source_owner TEXT, source_repo TEXT, repository_id INTEGER, is_fork INTEGER, PRIMARY KEY (owner, repo) ); CREATE TABLE skills ( name TEXT NOT NULL, owner TEXT NOT NULL, repo TEXT NOT NULL, display_name TEXT NOT NULL, diff --git a/test/unit/sync-repo-unreadable-blob.test.ts b/test/unit/sync-repo-unreadable-blob.test.ts index 616259b4..19b211b9 100644 --- a/test/unit/sync-repo-unreadable-blob.test.ts +++ b/test/unit/sync-repo-unreadable-blob.test.ts @@ -167,7 +167,7 @@ function createDatabase(): Database.Database { forks INTEGER NOT NULL DEFAULT 0, pushed_at INTEGER, repo_created_at INTEGER, repo_meta_synced_at INTEGER, description TEXT, last_tree_sha TEXT, repo_kind TEXT NOT NULL DEFAULT 'creator', repo_kind_source TEXT NOT NULL DEFAULT 'computed', repo_skill_count INTEGER NOT NULL DEFAULT 0, - broken_since INTEGER, tree_truncated_at INTEGER, source_owner TEXT, source_repo TEXT, repository_id INTEGER, PRIMARY KEY (owner, repo) + broken_since INTEGER, tree_truncated_at INTEGER, source_owner TEXT, source_repo TEXT, repository_id INTEGER, is_fork INTEGER, PRIMARY KEY (owner, repo) ); CREATE TABLE skills ( name TEXT NOT NULL, owner TEXT NOT NULL, repo TEXT NOT NULL, display_name TEXT NOT NULL, From 1ff993007a717e248123c8dba2df7478ba8870d8 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Sat, 10 Oct 2026 19:38:48 +1100 Subject: [PATCH 2/5] fix(skills): preserve validation state across account flows --- layers/identity/server/utils/account-deletion.ts | 1 + layers/registry/server/utils/repository-move.ts | 1 + test/unit/account-deletion.nuxt.test.ts | 1 + test/unit/digest-opt-in-address.test.ts | 8 ++++++++ test/unit/me-watchlist.nuxt.test.ts | 9 ++++----- test/unit/repository-move.test.ts | 2 ++ 6 files changed, 17 insertions(+), 5 deletions(-) diff --git a/layers/identity/server/utils/account-deletion.ts b/layers/identity/server/utils/account-deletion.ts index 154cddb2..0ae49f13 100644 --- a/layers/identity/server/utils/account-deletion.ts +++ b/layers/identity/server/utils/account-deletion.ts @@ -27,6 +27,7 @@ const ACCOUNT_ROW_DELETES = [ `DELETE FROM digest_runs WHERE user_id = ?1`, `DELETE FROM weekly_runs WHERE user_id = ?1`, `DELETE FROM email_preference_events WHERE user_id = ?1`, + `DELETE FROM skill_validation_email_deliveries WHERE user_id = ?1`, `DELETE FROM artifact_resolution_requesters WHERE account_id = ?1`, `DELETE FROM artifact_download_grants WHERE account_id = ?1`, `DELETE FROM private_artifact_attestations WHERE account_id = ?1`, diff --git a/layers/registry/server/utils/repository-move.ts b/layers/registry/server/utils/repository-move.ts index b6d3b12f..107a11bf 100644 --- a/layers/registry/server/utils/repository-move.ts +++ b/layers/registry/server/utils/repository-move.ts @@ -165,6 +165,7 @@ const REPOS_COLUMNS = [ 'default_branch', 'stars', 'forks', + 'is_fork', 'pushed_at', 'repo_created_at', 'repo_meta_synced_at', diff --git a/test/unit/account-deletion.nuxt.test.ts b/test/unit/account-deletion.nuxt.test.ts index e6fc9094..615cf3f1 100644 --- a/test/unit/account-deletion.nuxt.test.ts +++ b/test/unit/account-deletion.nuxt.test.ts @@ -148,6 +148,7 @@ function seedAccount(raw: DatabaseSync, account: Account, githubToken: string | NOW, ) run(`INSERT INTO email_preference_events (user_id, list, action, occurred_at) VALUES (?, 'weekly', 'unsubscribed', ?)`, id, NOW) + run(`INSERT INTO skill_validation_email_deliveries (user_id, status, attempted_at) VALUES (?, 'accepted', ?)`, id, NOW) run( `INSERT INTO github_app_installations (installation_id, account_id, github_account_id, state, connected_at, verified_at) diff --git a/test/unit/digest-opt-in-address.test.ts b/test/unit/digest-opt-in-address.test.ts index daf59523..b816cbc7 100644 --- a/test/unit/digest-opt-in-address.test.ts +++ b/test/unit/digest-opt-in-address.test.ts @@ -8,6 +8,14 @@ import { SIGNED_IN_HEADERS } from './helpers/session' const migrationPath = resolve(process.cwd(), 'migrations/0082_digest_opt_in_requires_address.sql') +vi.mock('../../layers/identity/server/utils/skill-validation-email', () => ({ + sendSkillValidationSummary: vi.fn().mockResolvedValue('skipped'), +})) +vi.mock('#app/nuxt', async importOriginal => ({ + ...await importOriginal(), + useRuntimeConfig: () => ({ email: { from: 'agent@example.com' } }), +})) + describe('digest opt-in requires a deliverable address', () => { it('rejects opting in without an address', () => { const result = identityEmailPatchBodySchema.safeParse({ email_opt_in: true }) diff --git a/test/unit/me-watchlist.nuxt.test.ts b/test/unit/me-watchlist.nuxt.test.ts index 8c68366d..8158e8f1 100644 --- a/test/unit/me-watchlist.nuxt.test.ts +++ b/test/unit/me-watchlist.nuxt.test.ts @@ -46,7 +46,6 @@ const likesStatus = ref<'idle' | 'pending' | 'success' | 'error'>('success') let serverAccount = accountFixture() let serverSubscriptions = [{ owner: 'antfu', repo: 'skills', source: 'like' }] let serverLikes = [likedSkillFixture()] -let queryCall = 0 let releaseDelete: (() => void) | undefined let releaseEmailSave: (() => void) | undefined @@ -65,9 +64,10 @@ vi.mock('@harlan-zw/nuxt-use-query/rpc', async (importOriginal) => { mockNuxtImport('$fetch', () => requestFetch) mockNuxtImport('useNuxtRpcQuery', () => { - return () => { - queryCall += 1 - if (queryCall % 2 === 1) + return (operation: { path: string }) => { + if (operation.path === '/api/me/validation') + return { data: ref({ checked: 0, pending: 0, items: [] }), error: ref(), status: ref('success'), refresh: vi.fn() } + if (operation.path === '/api/me') return { data: account, error: accountError, status: ref('success'), refresh: retryAccount } return { data: subscriptions, @@ -160,7 +160,6 @@ function paragraphWithText(wrapper: Awaited>, text: describe('account skill watchlist', () => { beforeEach(() => { - queryCall = 0 accountError.value = undefined subscriptionsError.value = undefined retryAccount.mockClear() diff --git a/test/unit/repository-move.test.ts b/test/unit/repository-move.test.ts index d87c6d84..14ca00e0 100644 --- a/test/unit/repository-move.test.ts +++ b/test/unit/repository-move.test.ts @@ -314,6 +314,8 @@ describe('the backfill SQL', () => { return `'catalog'` if (column.name === 'repo_kind_source') return `'override'` + if (column.name === 'is_fork') + return '1' return column.type === 'INTEGER' ? '7' : `'${column.name}-value'` } exec(`UPDATE repos SET ${columns.map(column => `${column.name} = ${value(column)}`).join(', ')} WHERE owner = 'facebook'`) From 0827ca1e456eb9622a3e8156806b40df39b2cae2 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Sat, 10 Oct 2026 20:56:38 +1100 Subject: [PATCH 3/5] fix(validation): normalize root Skill identity and folder guidance --- layers/identity/app/pages/me/index.vue | 2 +- shared/skill-validation.ts | 22 +++++++++++++++++----- test/unit/skill-validation.test.ts | 15 +++++++++++++++ 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/layers/identity/app/pages/me/index.vue b/layers/identity/app/pages/me/index.vue index 7a988f6a..8901428e 100644 --- a/layers/identity/app/pages/me/index.vue +++ b/layers/identity/app/pages/me/index.vue @@ -523,7 +523,7 @@ async function deleteAccount() { {{ item.repository }}/{{ item.name }}
    -
  • +
  • {{ issue.message }}
diff --git a/shared/skill-validation.ts b/shared/skill-validation.ts index 5674074c..aad5379d 100644 --- a/shared/skill-validation.ts +++ b/shared/skill-validation.ts @@ -1,4 +1,5 @@ import { parseDocument } from 'yaml' +import { slugifySkillName } from './skill-path' export interface SkillValidationIssue { field: string @@ -10,6 +11,11 @@ const BASE_FIELDS = new Set(['name', 'description', 'license', 'compatibility', // https://code.claude.com/docs/en/skills#frontmatter-reference const CLAUDE_FIELDS = new Set(['argument-hint', 'disable-model-invocation', 'user-invocable', 'model', 'context', 'agent', 'hooks', 'effort']) +function isSkillName(name: string): boolean { + return Boolean(name) && [...name].length <= 64 && name === name.toLowerCase() + && /^[\p{L}\p{N}]+(?:-[\p{L}\p{N}]+)*$/u.test(name) +} + /** Validate source bytes, without the registry's lenient display parser. */ export function validateSkillFrontmatter(raw: string, path: string, repository: string): SkillValidationIssue[] { const issues: SkillValidationIssue[] = [] @@ -39,13 +45,19 @@ export function validateSkillFrontmatter(raw: string, path: string, repository: } const sourceName = mapping.get('name') const name = typeof sourceName === 'string' ? sourceName.normalize('NFKC') : sourceName - const expected = path === 'SKILL.md' ? repository : path.split('/').at(-2) - if (typeof name !== 'string' || !name || [...name].length > 64 || name !== name.toLowerCase() - || !/^[\p{L}\p{N}]+(?:-[\p{L}\p{N}]+)*$/u.test(name)) { + // A root Skill uses the registry identity. Nested Skills have an actual + // parent folder, which the base specification requires name to match. + const expected = path === 'SKILL.md' ? slugifySkillName(repository) : path.split('/').at(-2)?.normalize('NFKC') + if (typeof name !== 'string' || !isSkillName(name)) { add('name', 'Use 1 to 64 lowercase letters, numbers, or single hyphens for name.') } - else if (name.normalize('NFKC') !== expected?.normalize('NFKC')) { - add('name', `Set name to the Skill folder name: ${expected}.`) + else if (name !== expected) { + if (path === 'SKILL.md') + add('name', `Set name to the Repository slug: ${expected}.`) + else if (expected && !isSkillName(expected)) + add('name', `Rename the Skill folder ${expected} to ${name} so it matches name.`) + else + add('name', `Set name to the Skill folder name: ${expected}.`) } const description = mapping.get('description') if (typeof description !== 'string' || !description.trim() || [...description].length > 1024) diff --git a/test/unit/skill-validation.test.ts b/test/unit/skill-validation.test.ts index a9f6a419..a746d681 100644 --- a/test/unit/skill-validation.test.ts +++ b/test/unit/skill-validation.test.ts @@ -30,6 +30,21 @@ describe('skill frontmatter validation', () => { expect(validateSkillFrontmatter(valid, 'skills/different/SKILL.md', 'repo')).toEqual(expect.arrayContaining([expect.objectContaining({ field: 'name' })])) }) + it('uses the registry slug for a root Skill in a mixed-case Repository', () => { + expect(validateSkillFrontmatter(valid, 'SKILL.md', 'Example')).toEqual([]) + expect(validateSkillFrontmatter(valid.replace('name: example', 'name: example-repo'), 'SKILL.md', 'Example Repo')).toEqual([]) + }) + + it('gives satisfiable guidance for an invalid nested folder name', () => { + expect(validateSkillFrontmatter(valid, 'skills/Example/SKILL.md', 'repo')).toEqual([ + { field: 'name', severity: 'error', message: 'Rename the Skill folder Example to example so it matches name.' }, + ]) + expect(validateSkillFrontmatter(valid, 'skills/example/SKILL.md', 'repo')).toEqual([]) + expect(validateSkillFrontmatter(valid, 'skills/different/SKILL.md', 'repo')).toEqual([ + { field: 'name', severity: 'error', message: 'Set name to the Skill folder name: different.' }, + ]) + }) + it('keeps documented Claude Code extensions as portability notices', () => { const result = validateSkillFrontmatter(valid.replace('---\nInstructions', 'context: fork\nagent: Explore\nuser-invocable: false\n---\nInstructions'), 'skills/example/SKILL.md', 'repo') expect(result.filter(issue => issue.severity === 'error')).toEqual([]) From 447db1f0a5978954e8dc92df66ff434948deef8c Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Sat, 10 Oct 2026 20:58:34 +1100 Subject: [PATCH 4/5] fix(email): fence validation delivery retries and add opt-out --- layers/identity/server/api/me/email.patch.ts | 3 +- .../identity/server/routes/auth/github.get.ts | 3 +- .../server/utils/skill-validation-email.ts | 31 +++++- test/unit/account-skill-validation.test.ts | 98 +++++++++++++++++-- 4 files changed, 122 insertions(+), 13 deletions(-) diff --git a/layers/identity/server/api/me/email.patch.ts b/layers/identity/server/api/me/email.patch.ts index 3a54628c..2d3f36a1 100644 --- a/layers/identity/server/api/me/email.patch.ts +++ b/layers/identity/server/api/me/email.patch.ts @@ -2,7 +2,7 @@ import { defineApiHandler } from '#shared/server/handler' import { identityEmailPatchBodySchema, identityMutationResponseSchema } from '../../../shared/contracts/account' import { authenticated } from '../../policies/authenticated' import { planAccountSettings, updateAccountSettings } from '../../utils/account-settings' -import { sendEmailWithEnv } from '../../utils/email' +import { sendEmailWithEnv, signUnsubToken } from '../../utils/email' import { sendSkillValidationSummary } from '../../utils/skill-validation-email' import { requireUserRow } from '../../utils/users' @@ -29,6 +29,7 @@ export default defineApiHandler({ db: platform.db, user: updated, now: Math.floor(Date.now() / 1000), + signUnsubscribe: userId => signUnsubToken(userId, config.tokenKey), send: input => sendEmailWithEnv(platform.env, { ...input, from: config.email.from }), }).then((outcome) => { if (outcome === 'rejected' || outcome === 'uncertain') diff --git a/layers/identity/server/routes/auth/github.get.ts b/layers/identity/server/routes/auth/github.get.ts index 92f1b606..6847f6ca 100644 --- a/layers/identity/server/routes/auth/github.get.ts +++ b/layers/identity/server/routes/auth/github.get.ts @@ -2,7 +2,7 @@ import type { H3Event } from 'h3' import { useSession } from 'h3' import { z } from 'zod' import { parseReturnTo } from '#shared/return-to' -import { sendEmailWithEnv } from '../../utils/email' +import { sendEmailWithEnv, signUnsubToken } from '../../utils/email' import { fetchVerifiedPrimaryEmail } from '../../utils/github-emails' import { ownedRepoScanWarning, scanOwnedRepos } from '../../utils/scan-owned-repos' import { sendSkillValidationSummary } from '../../utils/skill-validation-email' @@ -69,6 +69,7 @@ const githubHandler = defineOAuthGitHubEventHandler({ db: platform.db, user: row, now: Math.floor(Date.now() / 1000), + signUnsubscribe: userId => signUnsubToken(userId, config.tokenKey), send: input => sendEmailWithEnv(platform.env, { ...input, from: config.email.from }), }).then((outcome) => { if (outcome === 'rejected' || outcome === 'uncertain') diff --git a/layers/identity/server/utils/skill-validation-email.ts b/layers/identity/server/utils/skill-validation-email.ts index e74346e1..c60f01df 100644 --- a/layers/identity/server/utils/skill-validation-email.ts +++ b/layers/identity/server/utils/skill-validation-email.ts @@ -1,6 +1,7 @@ import type { SendEmailInput, SendEmailResult } from './email' import type { UserRow } from './users' import { SKILL_VALIDATION_COPY } from '#shared/skill-validation-copy' +import { digestEmailHeaders } from './email' import { loadAccountSkillValidation } from './skill-validation' import { weeklyDeliveryActive } from './weekly-select' @@ -9,8 +10,11 @@ interface ValidationEmailInput { user: UserRow send: (input: SendEmailInput) => Promise now: number + signUnsubscribe: (userId: number) => Promise } +const CLAIM_LEASE_SECONDS = 300 + /** One consented connection summary, claimed atomically before delivery. */ export async function sendSkillValidationSummary(input: ValidationEmailInput): Promise<'skipped' | SendEmailResult['_tag']> { const { db, user, send, now } = input @@ -19,19 +23,38 @@ export async function sendSkillValidationSummary(input: ValidationEmailInput): P const to = (user.digest_email ?? user.email ?? '').trim() if (!to) return 'skipped' + const prior = await db.prepare(`SELECT status, attempted_at FROM skill_validation_email_deliveries WHERE user_id = ?1`) + .bind(user.id) + .first<{ status: string, attempted_at: number }>() + if (prior && (prior.status === 'accepted' || prior.status === 'uncertain' + || (prior.status === 'claimed' && prior.attempted_at >= now - CLAIM_LEASE_SECONDS))) { + return 'skipped' + } const summary = await loadAccountSkillValidation(db, user.login) const invalid = summary.items.filter(item => item.issues.some(issue => issue.severity === 'error')) if (!invalid.length) return 'skipped' + const token = await input.signUnsubscribe(user.id) + const list = user.email_opt_in ? 'digest' : 'weekly' + const unsubscribeUrl = `https://skilld.dev/api/unsubscribe?t=${encodeURIComponent(token)}&list=${list}` const claimed = await db.prepare(` INSERT INTO skill_validation_email_deliveries (user_id, status, attempted_at) VALUES (?1, 'claimed', ?2) ON CONFLICT(user_id) DO UPDATE SET status = 'claimed', attempted_at = excluded.attempted_at, error = NULL WHERE skill_validation_email_deliveries.status = 'rejected' + OR (skill_validation_email_deliveries.status = 'claimed' AND skill_validation_email_deliveries.attempted_at < ?3) RETURNING user_id - `).bind(user.id, now).first<{ user_id: number }>() + `).bind(user.id, now, now - CLAIM_LEASE_SECONDS).first<{ user_id: number }>() if (!claimed) return 'skipped' + // Only pre-send claims can expire. Fence old workers before dispatch. + // A crash after this transition leaves ambiguous delivery blocked. + const dispatch = await db.prepare(`UPDATE skill_validation_email_deliveries + SET status = 'uncertain', error = 'Delivery started without a confirmed receipt' + WHERE user_id = ?1 AND attempted_at = ?2 AND status = 'claimed' + RETURNING user_id`).bind(user.id, now).first<{ user_id: number }>() + if (!dispatch) + return 'skipped' const link = 'https://skilld.dev/me?view=repositories' const count = `${invalid.length} ${invalid.length === 1 ? 'Skill has' : 'Skills have'} frontmatter issues.` const outcome = await send({ @@ -39,13 +62,15 @@ export async function sendSkillValidationSummary(input: ValidationEmailInput): P subject: SKILL_VALIDATION_COPY.subject, html: `

${SKILL_VALIDATION_COPY.subject}

${count}

${SKILL_VALIDATION_COPY.action}

`, text: `${count}\n\n${SKILL_VALIDATION_COPY.action}: ${link}`, + headers: digestEmailHeaders(unsubscribeUrl, `skilld-validation:${user.id}`), }).catch((error): SendEmailResult => ({ // A thrown send may have reached the provider. Do not risk duplicate delivery. _tag: 'uncertain', error: error instanceof Error ? error.message : String(error), })) - await db.prepare(`UPDATE skill_validation_email_deliveries SET status = ?1, error = ?2 WHERE user_id = ?3`) - .bind(outcome._tag, outcome._tag === 'accepted' ? null : outcome.error, user.id) + await db.prepare(`UPDATE skill_validation_email_deliveries SET status = ?1, error = ?2 + WHERE user_id = ?3 AND attempted_at = ?4 AND status = 'uncertain'`) + .bind(outcome._tag, outcome._tag === 'accepted' ? null : outcome.error, user.id, now) .run() return outcome._tag } diff --git a/test/unit/account-skill-validation.test.ts b/test/unit/account-skill-validation.test.ts index e8222eb5..12887a81 100644 --- a/test/unit/account-skill-validation.test.ts +++ b/test/unit/account-skill-validation.test.ts @@ -47,28 +47,110 @@ describe('account Skill validation', () => { it('sends one summary after consent and never sends a captured address alone', async () => { source() const send = vi.fn(async () => ({ _tag: 'accepted' as const, messageId: 'message-1' })) - expect(await sendSkillValidationSummary({ db: harness.db, user: user({ email_opt_in: 0, weekly_opt_out: 0 }), send, now: 10 })).toBe('skipped') + expect(await sendSkillValidationSummary({ signUnsubscribe: async () => 'signed-token', db: harness.db, user: user({ email_opt_in: 0, weekly_opt_out: 0 }), send, now: 10 })).toBe('skipped') expect(send).not.toHaveBeenCalled() - await Promise.all([1, 2].map(() => sendSkillValidationSummary({ db: harness.db, user: user(), send, now: 10 }))) + await Promise.all([1, 2].map(() => sendSkillValidationSummary({ signUnsubscribe: async () => 'signed-token', db: harness.db, user: user(), send, now: 10 }))) expect(send).toHaveBeenCalledTimes(1) expect(send.mock.calls[0]?.[0]).toMatchObject({ subject: 'Your Skills have validation issues', text: expect.stringContaining('1 Skill has frontmatter issues.') }) - expect(await sendSkillValidationSummary({ db: harness.db, user: user(), send, now: 20 })).toBe('skipped') + expect(await sendSkillValidationSummary({ signUnsubscribe: async () => 'signed-token', db: harness.db, user: user(), send, now: 20 })).toBe('skipped') }) it('does not email portability notices or opted-out accounts', async () => { source('owner', 'skills', 0, raw.replace('version: "1"', 'context: fork')) const send = vi.fn(async () => ({ _tag: 'accepted' as const, messageId: 'message-1' })) - expect(await sendSkillValidationSummary({ db: harness.db, user: user(), send, now: 10 })).toBe('skipped') - expect(await sendSkillValidationSummary({ db: harness.db, user: user({ repo_indexing: 0 }), send, now: 10 })).toBe('skipped') + expect(await sendSkillValidationSummary({ signUnsubscribe: async () => 'signed-token', db: harness.db, user: user(), send, now: 10 })).toBe('skipped') + expect(await sendSkillValidationSummary({ signUnsubscribe: async () => 'signed-token', db: harness.db, user: user({ repo_indexing: 0 }), send, now: 10 })).toBe('skipped') expect(send).not.toHaveBeenCalled() }) it('retries rejected delivery but preserves an uncertain provider result', async () => { source() const send = vi.fn().mockResolvedValueOnce({ _tag: 'rejected', error: 'provider unavailable' }).mockResolvedValueOnce({ _tag: 'uncertain', error: 'missing receipt' }) - expect(await sendSkillValidationSummary({ db: harness.db, user: user(), send, now: 10 })).toBe('rejected') - expect(await sendSkillValidationSummary({ db: harness.db, user: user(), send, now: 20 })).toBe('uncertain') - expect(await sendSkillValidationSummary({ db: harness.db, user: user(), send, now: 30 })).toBe('skipped') + expect(await sendSkillValidationSummary({ signUnsubscribe: async () => 'signed-token', db: harness.db, user: user(), send, now: 10 })).toBe('rejected') + expect(await sendSkillValidationSummary({ signUnsubscribe: async () => 'signed-token', db: harness.db, user: user(), send, now: 20 })).toBe('uncertain') + expect(await sendSkillValidationSummary({ signUnsubscribe: async () => 'signed-token', db: harness.db, user: user(), send, now: 30 })).toBe('skipped') expect(send).toHaveBeenCalledTimes(2) }) + + it('reclaims stale pre-send claims but keeps active claims and ambiguous delivery untouched', async () => { + source() + harness.raw.prepare(`INSERT INTO skill_validation_email_deliveries (user_id,status,attempted_at) VALUES (10001,'claimed',10)`).run() + const send = vi.fn(async () => { + expect(harness.raw.prepare(`SELECT status FROM skill_validation_email_deliveries WHERE user_id=10001`).get()).toEqual({ status: 'uncertain' }) + return { _tag: 'accepted' as const, messageId: 'receipt' } + }) + expect(await sendSkillValidationSummary({ signUnsubscribe: async () => 'signed-token', db: harness.db, user: user(), send, now: 20 })).toBe('skipped') + expect(await sendSkillValidationSummary({ signUnsubscribe: async () => 'signed-token', db: harness.db, user: user(), send, now: 1000 })).toBe('accepted') + expect(send).toHaveBeenCalledTimes(1) + harness.raw.prepare(`UPDATE skill_validation_email_deliveries SET status='uncertain',attempted_at=10`).run() + expect(await sendSkillValidationSummary({ signUnsubscribe: async () => 'signed-token', db: harness.db, user: user(), send, now: 2000 })).toBe('skipped') + expect(send).toHaveBeenCalledTimes(1) + }) + + it('skips source validation queries after a summary was delivered', async () => { + source() + harness.raw.prepare(`INSERT INTO skill_validation_email_deliveries (user_id,status,attempted_at) VALUES (10001,'accepted',10)`).run() + const prepare = vi.spyOn(harness.db, 'prepare') + const send = vi.fn() + expect(await sendSkillValidationSummary({ signUnsubscribe: async () => 'signed-token', db: harness.db, user: user(), send, now: 1000 })).toBe('skipped') + expect(prepare.mock.calls.every(([sql]) => !sql.includes('rendered_raw'))).toBe(true) + expect(send).not.toHaveBeenCalled() + }) + + it('includes one-click unsubscribe headers for the consented delivery list', async () => { + source() + const send = vi.fn().mockResolvedValue({ _tag: 'accepted', messageId: 'receipt' }) + await sendSkillValidationSummary({ signUnsubscribe: async () => 'signed-token', db: harness.db, user: user(), send, now: 1000 }) + expect(send.mock.calls[0]?.[0].headers).toMatchObject({ + 'List-Unsubscribe': '', + 'List-Unsubscribe-Post': 'List-Unsubscribe=One-Click', + }) + harness.raw.prepare(`DELETE FROM skill_validation_email_deliveries`).run() + await sendSkillValidationSummary({ signUnsubscribe: async () => 'signed-token', db: harness.db, user: user({ email_opt_in: 0, weekly_opt_out: 0, digest_email: 'owner@example.com' }), send, now: 2000 }) + expect(send.mock.calls[1]?.[0].headers['List-Unsubscribe']).toContain('list=weekly') + }) + + it('fences a paused worker when another connection reclaims its expired pre-send claim', async () => { + source() + let resume: () => void = () => {} + let started: () => void = () => {} + const paused = new Promise((resolve) => { + resume = resolve + }) + const waiting = new Promise((resolve) => { + started = resolve + }) + const prepare = harness.db.prepare.bind(harness.db) + const db = { + ...harness.db, + prepare(sql: string) { + const statement = prepare(sql) + if (!sql.includes('Delivery started without')) + return statement + return { + ...statement, + bind(...values: unknown[]) { + const bound = statement.bind(...values) + return { + ...bound, + async first() { + if (values[1] === 10) { + started() + await paused + } + return bound.first() + }, + } + }, + } as D1PreparedStatement + }, + } as D1Database + const send = vi.fn().mockResolvedValue({ _tag: 'accepted', messageId: 'receipt' }) + const oldWorker = sendSkillValidationSummary({ signUnsubscribe: async () => 'token', db, user: user(), send, now: 10 }) + await waiting + expect(await sendSkillValidationSummary({ signUnsubscribe: async () => 'token', db, user: user(), send, now: 1000 })).toBe('accepted') + resume() + expect(await oldWorker).toBe('skipped') + expect(send).toHaveBeenCalledTimes(1) + }) }) From 548be9dc2ddd3f12a956b7dbe6e80454f3c47d29 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Sat, 10 Oct 2026 20:58:53 +1100 Subject: [PATCH 5/5] fix(validation): refresh fork metadata for indexed sources --- .../identity/server/utils/skill-validation.ts | 62 ++--------------- .../0153_validation_fork_metadata_refresh.sql | 12 ++++ shared/server/skill-validation-sources.ts | 66 +++++++++++++++++++ test/unit/repository-skill-validation.test.ts | 24 +++++++ test/unit/sync-prefetch.test.ts | 4 +- test/unit/validation-fork-refresh.test.ts | 27 ++++++++ 6 files changed, 136 insertions(+), 59 deletions(-) create mode 100644 migrations/0153_validation_fork_metadata_refresh.sql create mode 100644 shared/server/skill-validation-sources.ts create mode 100644 test/unit/repository-skill-validation.test.ts create mode 100644 test/unit/validation-fork-refresh.test.ts diff --git a/layers/identity/server/utils/skill-validation.ts b/layers/identity/server/utils/skill-validation.ts index e1f22b37..ba207135 100644 --- a/layers/identity/server/utils/skill-validation.ts +++ b/layers/identity/server/utils/skill-validation.ts @@ -1,61 +1,9 @@ -import type { SkillValidationIssue } from '#shared/skill-validation' -import { validateSkillFrontmatter } from '#shared/skill-validation' +import type { RepositorySkillValidation } from '#shared/server/skill-validation-sources' +import { loadRepositorySkillValidation } from '#shared/server/skill-validation-sources' -export interface AccountSkillValidation { - checked: number - pending: number - items: Array<{ - repository: string - name: string - sourceUrl: string - issues: SkillValidationIssue[] - }> -} - -interface SourceRow { - repo: string - name: string - rendered_raw: string | null - rendered_skill_path: string | null - rendered_commit_sha: string | null - source_owner: string | null - source_repo: string | null - is_fork: number | null -} +export type AccountSkillValidation = RepositorySkillValidation /** Only the signed-in account's own Repository sources enter this summary. */ -export async function loadAccountSkillValidation(db: D1Database, login: string): Promise { - const result: AccountSkillValidation = { checked: 0, pending: 0, items: [] } - let after = '' - // Page source bytes so one account cannot exceed D1's response size limit. - for (;;) { - const page = await db.prepare(` - SELECT s.repo, s.name, s.rendered_raw, s.rendered_skill_path, - s.rendered_commit_sha, r.source_owner, r.source_repo, r.is_fork - FROM skills s JOIN repos r ON r.owner = s.owner AND r.repo = s.repo - WHERE s.owner = ?1 COLLATE NOCASE AND s.source_resolved = 1 - AND (r.is_fork IS NULL OR r.is_fork = 0) - AND (s.repo || char(0) || s.name) > ?2 - ORDER BY s.repo, s.name LIMIT 25 - `).bind(login, after).all() - const rows = page.results ?? [] - for (const row of rows) { - if (row.is_fork === null || !row.rendered_raw || !row.rendered_skill_path || !row.rendered_commit_sha) { - result.pending++ - continue - } - result.checked++ - const issues = validateSkillFrontmatter(row.rendered_raw, row.rendered_skill_path, row.source_repo ?? row.repo) - if (issues.length) { - const owner = row.source_owner ?? login - const repo = row.source_repo ?? row.repo - const sourceUrl = `https://github.com/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/blob/${encodeURIComponent(row.rendered_commit_sha)}/${row.rendered_skill_path.split('/').map(encodeURIComponent).join('/')}` - result.items.push({ repository: `${login}/${row.repo}`, name: row.name, sourceUrl, issues }) - } - } - if (rows.length < 25) - return result - const last = rows.at(-1)! - after = `${last.repo}\0${last.name}` - } +export function loadAccountSkillValidation(db: D1Database, login: string): Promise { + return loadRepositorySkillValidation(db, { _tag: 'owner', owner: login }) } diff --git a/migrations/0153_validation_fork_metadata_refresh.sql b/migrations/0153_validation_fork_metadata_refresh.sql new file mode 100644 index 00000000..928f3aa2 --- /dev/null +++ b/migrations/0153_validation_fork_metadata_refresh.sql @@ -0,0 +1,12 @@ +-- Prioritize confirmation of unknown fork metadata for indexed Repositories. +-- The existing bounded sync sweep obtains GitHub's isFork value. +-- Unchanged sources use the metadata prefetch path. Fork status stays unknown +-- until GitHub confirms it. Existing quota controls and pauses still apply. +UPDATE repos SET repo_meta_synced_at = NULL +WHERE is_fork IS NULL + AND broken_since IS NULL + AND EXISTS ( + SELECT 1 FROM skills s + WHERE s.owner = repos.owner AND s.repo = repos.repo + AND COALESCE(s.sync_status, '') != 'path_missing' + ); diff --git a/shared/server/skill-validation-sources.ts b/shared/server/skill-validation-sources.ts new file mode 100644 index 00000000..3f952bc0 --- /dev/null +++ b/shared/server/skill-validation-sources.ts @@ -0,0 +1,66 @@ +import type { SkillValidationIssue } from '#shared/skill-validation' +import { validateSkillFrontmatter } from '#shared/skill-validation' + +export interface RepositorySkillValidation { + checked: number + pending: number + items: Array<{ + repository: string + name: string + sourceUrl: string + issues: SkillValidationIssue[] + }> +} + +interface SourceRow { + repo: string + name: string + rendered_raw: string | null + rendered_skill_path: string | null + rendered_commit_sha: string | null + source_owner: string | null + source_repo: string | null + is_fork: number | null +} + +/** Select sources explicitly. Callers own authorization and presentation. */ +export type RepositoryValidationSelection + = { _tag: 'owner', owner: string } + | { _tag: 'repository', owner: string, repo: string } + +export async function loadRepositorySkillValidation(db: D1Database, selection: RepositoryValidationSelection): Promise { + const result: RepositorySkillValidation = { checked: 0, pending: 0, items: [] } + let after = '' + // Page source bytes so each read stays below D1's response size limit. + for (;;) { + const page = await db.prepare(` + SELECT s.repo, s.name, s.rendered_raw, s.rendered_skill_path, + s.rendered_commit_sha, r.source_owner, r.source_repo, r.is_fork + FROM skills s JOIN repos r ON r.owner = s.owner AND r.repo = s.repo + WHERE s.owner = ?1 COLLATE NOCASE AND s.source_resolved = 1 + AND (?3 IS NULL OR s.repo = ?3 COLLATE NOCASE) + AND (r.is_fork IS NULL OR r.is_fork = 0) + AND (s.repo || char(0) || s.name) > ?2 + ORDER BY s.repo, s.name LIMIT 25 + `).bind(selection.owner, after, selection._tag === 'repository' ? selection.repo : null).all() + const rows = page.results ?? [] + for (const row of rows) { + if (row.is_fork === null || !row.rendered_raw || !row.rendered_skill_path || !row.rendered_commit_sha) { + result.pending++ + continue + } + result.checked++ + const issues = validateSkillFrontmatter(row.rendered_raw, row.rendered_skill_path, row.source_repo ?? row.repo) + if (issues.length) { + const owner = row.source_owner ?? selection.owner + const repo = row.source_repo ?? row.repo + const sourceUrl = `https://github.com/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/blob/${encodeURIComponent(row.rendered_commit_sha)}/${row.rendered_skill_path.split('/').map(encodeURIComponent).join('/')}` + result.items.push({ repository: `${selection.owner}/${row.repo}`, name: row.name, sourceUrl, issues }) + } + } + if (rows.length < 25) + return result + const last = rows.at(-1)! + after = `${last.repo}\0${last.name}` + } +} diff --git a/test/unit/repository-skill-validation.test.ts b/test/unit/repository-skill-validation.test.ts new file mode 100644 index 00000000..6e84f7c8 --- /dev/null +++ b/test/unit/repository-skill-validation.test.ts @@ -0,0 +1,24 @@ +import { expect, it } from 'vitest' +import { loadRepositorySkillValidation } from '../../shared/server/skill-validation-sources' +import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' + +it('selects one Repository independently of account identity and excludes confirmed forks', async () => { + const harness = createSqliteD1(allMigrations()) + try { + for (const [owner, repo, fork] of [['org', 'wanted', 0], ['org', 'other', 0], ['elsewhere', 'wanted', 0], ['org', 'fork', 1]] as const) { + harness.raw.prepare('INSERT INTO repos (owner,repo,is_fork) VALUES (?,?,?)').run(owner, repo, fork) + harness.raw.prepare(`INSERT INTO skills (owner,repo,name,display_name,slug,source_resolved,rendered_raw,rendered_skill_path,rendered_commit_sha) + VALUES (?,?,?,?,?,1,?,?,?)`) + .run(owner, repo, 'example', 'Example', 'example', '---\nname: example\ndescription: Works.\nversion: "1"\n---\nInstructions', 'skills/example/SKILL.md', 'sha') + } + const selected = await loadRepositorySkillValidation(harness.db, { _tag: 'repository', owner: 'org', repo: 'WANTED' }) + expect(selected).toMatchObject({ checked: 1, pending: 0, items: [{ repository: 'org/wanted', sourceUrl: 'https://github.com/org/wanted/blob/sha/skills/example/SKILL.md' }] }) + expect(selected.items).toHaveLength(1) + expect(await loadRepositorySkillValidation(harness.db, { _tag: 'repository', owner: 'org', repo: 'fork' })).toEqual({ checked: 0, pending: 0, items: [] }) + const owner = await loadRepositorySkillValidation(harness.db, { _tag: 'owner', owner: 'org' }) + expect(owner.checked).toBe(2) + } + finally { + harness.close() + } +}) diff --git a/test/unit/sync-prefetch.test.ts b/test/unit/sync-prefetch.test.ts index 26952ee8..5d55a929 100644 --- a/test/unit/sync-prefetch.test.ts +++ b/test/unit/sync-prefetch.test.ts @@ -94,8 +94,8 @@ describe('prefetchUnchangedRepos', () => { 'gone/missing', ]) expect(result).toMatchObject({ _tag: 'prefetched', unchanged: 1 }) - expect(harness.raw.prepare(`SELECT stars, repo_meta_synced_at FROM repos WHERE owner = 'acme' AND repo = 'unchanged'`).get()) - .toEqual({ stars: 77, repo_meta_synced_at: NOW }) + expect(harness.raw.prepare(`SELECT stars, is_fork, repo_meta_synced_at FROM repos WHERE owner = 'acme' AND repo = 'unchanged'`).get()) + .toEqual({ stars: 77, is_fork: 0, repo_meta_synced_at: NOW }) expect(harness.raw.prepare(`SELECT repo_meta_synced_at FROM repos WHERE owner = 'acme' AND repo = 'changed'`).get()) .toEqual({ repo_meta_synced_at: null }) }) diff --git a/test/unit/validation-fork-refresh.test.ts b/test/unit/validation-fork-refresh.test.ts new file mode 100644 index 00000000..52f24c21 --- /dev/null +++ b/test/unit/validation-fork-refresh.test.ts @@ -0,0 +1,27 @@ +import { readFileSync } from 'node:fs' +import { expect, it } from 'vitest' +import { GENERAL_SYNC_CANDIDATES_SQL } from '../../layers/registry/server/utils/sync-candidates' +import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' + +it('makes indexed unknown-fork repositories due without guessing their fork status', async () => { + const migration = 'migrations/0153_validation_fork_metadata_refresh.sql' + const harness = createSqliteD1(allMigrations().filter(path => path !== migration)) + try { + for (const [repo, fork] of [['unknown', null], ['known', 0], ['fork', 1], ['empty', null]] as const) { + harness.raw.prepare('INSERT INTO repos (owner, repo, is_fork, repo_meta_synced_at) VALUES (?, ?, ?, ?)') + .run('owner', repo, fork, 1000) + if (repo !== 'empty') { + harness.raw.prepare('INSERT INTO skills (owner,repo,name,display_name,slug) VALUES (?, ?, ?, ?, ?)') + .run('owner', repo, 'example', 'Example', 'example') + } + } + harness.raw.exec(readFileSync(migration, 'utf8')) + const due = await harness.db.prepare(GENERAL_SYNC_CANDIDATES_SQL).bind(900, 1).all<{ repo: string }>() + expect(due.results?.map(row => row.repo)).toEqual(['unknown']) + expect(harness.raw.prepare('SELECT is_fork FROM repos WHERE repo = ?').get('unknown')).toEqual({ is_fork: null }) + expect(harness.raw.prepare('SELECT repo_meta_synced_at FROM repos WHERE repo = ?').get('empty')).toEqual({ repo_meta_synced_at: 1000 }) + } + finally { + harness.close() + } +})