From 70a7dbd136f67bb821d665ca3f1ad828ccdb2b6d Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Wed, 7 Oct 2026 01:55:26 +1100 Subject: [PATCH 1/2] feat(artifact-delivery): flag a run command that keeps failing The sweep from #481 records how each Skill's run check settles, but a visitor still saw a run command that fails every time. Two checks in a row that fail for a reason a retry cannot change now flag the run command on the Skill page, and the curated surfaces leave the Skill out until one check passes. A retryable failure never counts. --- docs/arch/README.md | 2 +- .../server/api/run-checks/flags.get.ts | 26 ++++ .../server/presenters/run-check-flags.ts | 45 +++++++ .../server/schemas/run-check-flags.ts | 11 ++ .../server/utils/run-check-flags.ts | 50 ++++++++ .../server/utils/run-check-state.ts | 115 ++++++++++++++++++ .../server/utils/run-sweep.ts | 82 ++++--------- .../registry/app/components/SkillDetail.vue | 17 ++- .../app/components/_SkillCommandPanel.vue | 18 +++ .../registry/app/components/_SkillRunFlag.vue | 52 ++++++++ .../server/api/skill-demos/index.get.ts | 17 ++- migrations/0143_artifact_run_check_streak.sql | 20 +++ nuxt.config.ts | 4 +- .../by-author/[login]/[slug]/index.get.ts | 12 +- server/api/feed/trending.get.ts | 28 ++++- shared/run-check-flags.ts | 31 +++++ shared/server/run-check-flags.ts | 18 +++ test/unit/run-check-state.test.ts | 49 ++++++++ test/unit/skill-run-sweep.test.ts | 22 ++++ 19 files changed, 552 insertions(+), 67 deletions(-) create mode 100644 layers/artifact-delivery/server/api/run-checks/flags.get.ts create mode 100644 layers/artifact-delivery/server/presenters/run-check-flags.ts create mode 100644 layers/artifact-delivery/server/schemas/run-check-flags.ts create mode 100644 layers/artifact-delivery/server/utils/run-check-flags.ts create mode 100644 layers/artifact-delivery/server/utils/run-check-state.ts create mode 100644 layers/registry/app/components/_SkillRunFlag.vue create mode 100644 migrations/0143_artifact_run_check_streak.sql create mode 100644 shared/run-check-flags.ts create mode 100644 shared/server/run-check-flags.ts create mode 100644 test/unit/run-check-state.test.ts diff --git a/docs/arch/README.md b/docs/arch/README.md index 8d5ce47d..7c330dd6 100644 --- a/docs/arch/README.md +++ b/docs/arch/README.md @@ -18,7 +18,7 @@ Use the names exactly. Drift breeds shallow modules. - **app**: `/`, `/community`, `/@/*`, `/collections/new`, `api/community|collections|feed|*`. Owns native data. - **marketing**: `/skills`, `/skills/*`, `/frameworks/*`, `/learn/*`, `/compare/*`. Comark renders its Markdown. Owns SEO content. [Comparison requirements](comparisons.md) cover editorial evidence and admission. - **admin**: existing. -- **artifact delivery:** `/api/v1/resolutions|artifacts|trusted-root|github/connections`. Owns exact source resolution, checks, signing, grants, and transient delivery. +- **artifact delivery:** `/api/v1/resolutions|artifacts|trusted-root|github/connections`, and the internal `/api/run-checks/flags`. Owns exact source resolution, checks, signing, grants, transient delivery, and the run checks behind the run check flag. Cross-layer reads go via HTTP (`$fetch('/api/...')`), never shared server utilities. Each layer is deletion-testable. diff --git a/layers/artifact-delivery/server/api/run-checks/flags.get.ts b/layers/artifact-delivery/server/api/run-checks/flags.get.ts new file mode 100644 index 00000000..95f0476f --- /dev/null +++ b/layers/artifact-delivery/server/api/run-checks/flags.get.ts @@ -0,0 +1,26 @@ +import { setHeader } from 'h3' +import { defineApiHandler } from '#shared/server/handler' +import { presentRunCheckFlags } from '../../presenters/run-check-flags' +import { runCheckFlagsQuerySchema } from '../../schemas/run-check-flags' +import { loadRunCheckFlags } from '../../utils/run-check-flags' + +/** + * The Skills whose last two run checks failed for a reason a retry cannot + * change. The Skill page asks for its own Skill and flags its run command. + * The demo, trending and collection routes ask for every flag and leave + * those Skills out. + * + * Public and read only: every answer is a fact the Skill page shows anyway, + * so it has no policy. It reads only flagged rows, through the streak index. + */ +export default defineApiHandler({ + schema: runCheckFlagsQuerySchema, + handler: async ({ event, body, platform }) => { + // The sweep settles checks every 15 minutes, so one minute of reuse is safe. + setHeader(event, 'cache-control', 'public, max-age=60') + const [owner, repository, name] = body.skill?.split('/') ?? [] + const skill = owner && repository && name ? { owner, repository, name } : null + return await loadRunCheckFlags(platform.db, skill) + }, + presenter: presentRunCheckFlags, +}) diff --git a/layers/artifact-delivery/server/presenters/run-check-flags.ts b/layers/artifact-delivery/server/presenters/run-check-flags.ts new file mode 100644 index 00000000..a5f36801 --- /dev/null +++ b/layers/artifact-delivery/server/presenters/run-check-flags.ts @@ -0,0 +1,45 @@ +import type { RunCheckFlag, RunCheckFlagsResponse } from '#shared/run-check-flags' +import type { RunCheckFlagRow } from '../utils/run-check-flags' + +/** + * The reason a run fails, in plain words, for each tag a retry cannot change. + * + * Keys are the tags `settleResolution` writes. A source policy failure has no + * entry: its check summary already names the rule and the file. + */ +const REASONS: Readonly> = { + 'CHECK_BLOCKED:credential-material': 'A file in the Skill holds a private key.', + 'CHECK_BLOCKED:path-policy': 'A file path in the Skill is too long for skilld to pack.', + 'SOURCE_NOT_FOUND': 'skilld could not find this Skill on GitHub.', + 'SOURCE_ACCESS_DENIED': 'GitHub denied access to the Repository.', + 'AUTH_REQUIRED': 'The Repository needs a GitHub sign-in.', + 'INVALID_SOURCE': 'GitHub returned a source that skilld could not use.', + 'ARTIFACT_REVOKED': 'skilld withdrew this version of the Skill.', +} + +/** Why a flagged run fails, in plain words. */ +export function runCheckReason(tag: string, detail: string | null): string { + const known = REASONS[tag] + if (known) + return known + if (tag === 'CHECK_BLOCKED:source-policy') + return detail ?? 'The Skill folder breaks a source rule, such as a size limit.' + if (tag.startsWith('CHECK_BLOCKED:')) + return 'A required check stopped the run.' + return `skilld run stopped with the code ${tag}.` +} + +const CHECKED_ON = new Intl.DateTimeFormat('en-GB', { day: 'numeric', month: 'short', year: 'numeric', timeZone: 'UTC' }) + +export function presentRunCheckFlags(rows: RunCheckFlagRow[]): RunCheckFlagsResponse { + return { + items: rows.map((row): RunCheckFlag => ({ + owner: row.owner, + repository: row.repository, + name: row.name, + checkedAt: new Date(row.failedAt * 1000).toISOString(), + checkedOn: CHECKED_ON.format(row.failedAt * 1000), + reason: runCheckReason(row.tag, row.detail), + })), + } +} diff --git a/layers/artifact-delivery/server/schemas/run-check-flags.ts b/layers/artifact-delivery/server/schemas/run-check-flags.ts new file mode 100644 index 00000000..7f4f4afc --- /dev/null +++ b/layers/artifact-delivery/server/schemas/run-check-flags.ts @@ -0,0 +1,11 @@ +import { z } from 'zod' + +/** `owner/repository/name`, with the segments GitHub and the registry allow. */ +const SKILL_REF = /^[a-z0-9-]{1,39}\/[\w.-]{1,100}\/[^/\s]{1,64}$/i + +/** No `skill` lists every flag. A `skill` limits the answer to that Skill. */ +export const runCheckFlagsQuerySchema = z.object({ + skill: z.string().regex(SKILL_REF).optional(), +}).strict() + +export type RunCheckFlagsQuery = z.infer diff --git a/layers/artifact-delivery/server/utils/run-check-flags.ts b/layers/artifact-delivery/server/utils/run-check-flags.ts new file mode 100644 index 00000000..2213ac5d --- /dev/null +++ b/layers/artifact-delivery/server/utils/run-check-flags.ts @@ -0,0 +1,50 @@ +import type { RunnableSkill } from './run-sweep' +import { z } from 'zod' +import { FLAG_AFTER_FAILURES, runCheckFlag, runCheckStateOf } from './run-check-state' + +/** One flagged Skill: its run checks failed in a row for a reason a retry cannot change. */ +export interface RunCheckFlagRow extends RunnableSkill { + tag: string + detail: string | null + /** Unix seconds the last failing check settled. */ + failedAt: number +} + +const flagRowSchema = z.object({ + owner: z.string(), + repository: z.string(), + name: z.string(), + outcome: z.enum(['ready', 'failing']).nullable(), + tag: z.string().nullable(), + detail: z.string().nullable(), + retryable: z.union([z.literal(0), z.literal(1)]), + failing_since: z.number().int().nullable(), + failure_streak: z.number().int().nonnegative(), + failed_at: z.number().int().nullable(), + settled_at: z.number().int().nullable(), +}) + +/** + * Every flagged Skill, or the one named, matched without regard to case. + * + * The streak index limits the read to rows that can flag, so a call reads the + * flagged rows and no others. `runCheckFlag` stays the one rule. + */ +export async function loadRunCheckFlags(db: D1Database, skill: RunnableSkill | null): Promise { + const result = await db.prepare( + `SELECT owner, repository, name, outcome, tag, detail, retryable, failing_since, + failure_streak, failed_at, settled_at + FROM artifact_run_checks + WHERE failure_streak >= ?1 + AND (?2 IS NULL OR (owner = ?2 COLLATE NOCASE AND repository = ?3 COLLATE NOCASE AND name = ?4 COLLATE NOCASE)) + ORDER BY owner, repository, name`, + ).bind(FLAG_AFTER_FAILURES, skill?.owner ?? null, skill?.repository ?? null, skill?.name ?? null).all>() + const flags: RunCheckFlagRow[] = [] + for (const value of result.results ?? []) { + const row = flagRowSchema.parse(value) + const flag = runCheckFlag(runCheckStateOf(row)) + if (flag._tag === 'flagged') + flags.push({ owner: row.owner, repository: row.repository, name: row.name, tag: flag.tag, detail: flag.detail, failedAt: flag.failedAt }) + } + return flags +} diff --git a/layers/artifact-delivery/server/utils/run-check-state.ts b/layers/artifact-delivery/server/utils/run-check-state.ts new file mode 100644 index 00000000..d77cb714 --- /dev/null +++ b/layers/artifact-delivery/server/utils/run-check-state.ts @@ -0,0 +1,115 @@ +/** + * The state of one Skill's run checks, and the flag it earns. + * + * A run check is the Resolution request `skilld run OWNER/REPOSITORY/NAME` + * sends, made by the `sweep-skill-runs` task. This module is the pure core: + * the sweep stores what it returns, and the flag route reads the result. + */ + +/** Failures in a row, each for a reason a retry cannot change, that flag a Skill. */ +export const FLAG_AFTER_FAILURES = 2 + +export type RunCheckState + = | { _tag: 'unchecked' } + | { _tag: 'ready' } + /** The last check failed for a reason a retry can change, such as `RATE_LIMITED`. */ + | { _tag: 'transient', tag: string, detail: string | null, since: number } + /** The last check that said anything about the Skill failed for a reason a retry cannot change. */ + | { + _tag: 'failing' + tag: string + detail: string | null + /** Unix seconds this tag was first seen in a row. */ + since: number + /** Unix seconds the last failure in the streak settled. */ + failedAt: number + /** Checks in a row that failed this way, with any tag. */ + streak: number + } + +/** How one check ended. */ +export type RunCheckOutcome + = | { _tag: 'ready' } + | { _tag: 'failing', tag: string, detail: string | null, retryable: boolean } + +export type RunCheckFlag + = | { _tag: 'clear' } + | { _tag: 'flagged', tag: string, detail: string | null, failedAt: number } + +/** + * The state after one more check settles. + * + * A pass clears everything. A failure a retry cannot change adds one to the + * streak. A failure a retry can change says nothing about the Skill: over a + * known failure it keeps that failure and its streak, so a spent quota never + * hides a broken Skill and never counts toward a flag. + */ +export function nextRunCheckState(previous: RunCheckState, outcome: RunCheckOutcome, now: number): RunCheckState { + if (outcome._tag === 'ready') + return { _tag: 'ready' } + if (outcome.retryable) { + if (previous._tag === 'failing') + return previous + const since = previous._tag === 'transient' && previous.tag === outcome.tag ? previous.since : now + return { _tag: 'transient', tag: outcome.tag, detail: outcome.detail, since } + } + const streak = previous._tag === 'failing' ? previous.streak + 1 : 1 + const since = previous._tag === 'failing' && previous.tag === outcome.tag ? previous.since : now + return { _tag: 'failing', tag: outcome.tag, detail: outcome.detail, since, failedAt: now, streak } +} + +/** The flag a Skill page shows beside its run command, if any. */ +export function runCheckFlag(state: RunCheckState): RunCheckFlag { + if (state._tag !== 'failing' || state.streak < FLAG_AFTER_FAILURES) + return { _tag: 'clear' } + return { _tag: 'flagged', tag: state.tag, detail: state.detail, failedAt: state.failedAt } +} + +/** What one `artifact_run_checks` row stores for its state. */ +export interface RunCheckColumns { + outcome: 'ready' | 'failing' | null + tag: string | null + detail: string | null + retryable: 0 | 1 + failing_since: number | null + failure_streak: number + failed_at: number | null +} + +/** + * The state one row stores. + * + * A row the sweep wrote before the streak existed holds a streak of 0 over a + * failure, so it counts that failure once. + */ +export function runCheckStateOf(row: RunCheckColumns & { settled_at: number | null }): RunCheckState { + if (row.outcome === null) + return { _tag: 'unchecked' } + if (row.outcome === 'ready') + return { _tag: 'ready' } + const tag = row.tag ?? 'UNKNOWN' + const since = row.failing_since ?? row.settled_at ?? 0 + if (row.retryable === 1) + return { _tag: 'transient', tag, detail: row.detail, since } + return { + _tag: 'failing', + tag, + detail: row.detail, + since, + failedAt: row.failed_at ?? row.settled_at ?? since, + streak: Math.max(1, row.failure_streak), + } +} + +export function runCheckColumns(state: RunCheckState): RunCheckColumns { + switch (state._tag) { + case 'unchecked': + return { outcome: null, tag: null, detail: null, retryable: 0, failing_since: null, failure_streak: 0, failed_at: null } + case 'ready': + return { outcome: 'ready', tag: null, detail: null, retryable: 0, failing_since: null, failure_streak: 0, failed_at: null } + case 'transient': + return { outcome: 'failing', tag: state.tag, detail: state.detail, retryable: 1, failing_since: state.since, failure_streak: 0, failed_at: null } + case 'failing': + return { outcome: 'failing', tag: state.tag, detail: state.detail, retryable: 0, failing_since: state.since, failure_streak: state.streak, failed_at: state.failedAt } + } +} diff --git a/layers/artifact-delivery/server/utils/run-sweep.ts b/layers/artifact-delivery/server/utils/run-sweep.ts index 3ae3af2e..096cb87d 100644 --- a/layers/artifact-delivery/server/utils/run-sweep.ts +++ b/layers/artifact-delivery/server/utils/run-sweep.ts @@ -1,7 +1,9 @@ import type { SourceRequest } from '../schemas/contracts' import type { ResolutionAccess } from './request-resolution' +import type { RunCheckColumns } from './run-check-state' import type { CreateResolutionResult, ResolutionRow } from './state' import { z } from 'zod' +import { nextRunCheckState, runCheckColumns, runCheckStateOf } from './run-check-state' import { ACTIVE_BUILD_STATES, getResolution, parseCheckResults } from './state' /** One Skill whose page shows `skilld run OWNER/REPOSITORY/NAME`. */ @@ -60,6 +62,8 @@ const checkRowSchema = z.object({ detail: z.string().nullable(), retryable: z.union([z.literal(0), z.literal(1)]), failing_since: z.number().int().nullable(), + failure_streak: z.number().int().nonnegative(), + failed_at: z.number().int().nullable(), settled_at: z.number().int().nullable(), pending_resolution_id: z.string().nullable(), requested_at: z.number().int().nullable(), @@ -105,56 +109,27 @@ export async function runSkillRunSweep(dependencies: SkillRunSweepDependencies): continue report.settled++ const skill = { owner: row.owner, repository: row.repository, name: row.name } + const previous = runCheckStateOf(row) if (settled._tag === 'ready') { - if (row.outcome === 'failing' && row.retryable === 0) + if (previous._tag === 'failing') report.recovered.push(skill) - await writeSettled(dependencies.db, row, { outcome: 'ready', tag: null, detail: null, retryable: false, failingSince: null }, now) - rows.set(skillKey(skill), { ...row, outcome: 'ready', tag: null, detail: null, retryable: 0, pending_resolution_id: null, settled_at: now }) - continue } - const failure = { ...skill, tag: settled.tag, detail: settled.detail } - const sameFailure = row.outcome === 'failing' && row.tag === settled.tag - if (settled.retryable) - report.transientFailures.push(failure) - else if (!sameFailure) - report.newFailures.push(failure) - // A retryable failure says nothing about the Skill. Over a known failure it - // keeps that failure, so the next check that sees it again is not new. - if (settled.retryable && row.outcome === 'failing' && row.retryable === 0) { - await writeSettled(dependencies.db, row, { - outcome: 'failing', - tag: row.tag, - detail: row.detail, - retryable: false, - failingSince: row.failing_since, - }, now) - rows.set(skillKey(skill), { ...row, pending_resolution_id: null, settled_at: now }) - continue + else { + const failure = { ...skill, tag: settled.tag, detail: settled.detail } + if (settled.retryable) + report.transientFailures.push(failure) + else if (!(previous._tag === 'failing' && previous.tag === settled.tag)) + report.newFailures.push(failure) } - const failingSince = sameFailure ? row.failing_since ?? now : now - await writeSettled(dependencies.db, row, { - outcome: 'failing', - tag: settled.tag, - detail: settled.detail, - retryable: settled.retryable, - failingSince, - }, now) - rows.set(skillKey(skill), { - ...row, - outcome: 'failing', - tag: settled.tag, - detail: settled.detail, - retryable: settled.retryable ? 1 : 0, - failing_since: failingSince, - pending_resolution_id: null, - settled_at: now, - }) + const next = runCheckColumns(nextRunCheckState(previous, settled, now)) + await writeSettled(dependencies.db, row, next, now) + rows.set(skillKey(skill), { ...row, ...next, pending_resolution_id: null, settled_at: now }) } const runnable = await dependencies.listRunnableSkills() report.failing = runnable.filter((skill) => { const row = rows.get(skillKey(skill)) - return row?.outcome === 'failing' && row.retryable === 0 + return row !== undefined && runCheckStateOf(row)._tag === 'failing' }).length const due = runnable @@ -220,7 +195,7 @@ export function settleResolution(resolution: ResolutionRow | null, pendingSecond async function loadCheckRows(db: D1Database): Promise> { const result = await db.prepare( `SELECT owner, repository, name, outcome, tag, detail, retryable, failing_since, - settled_at, pending_resolution_id, requested_at + failure_streak, failed_at, settled_at, pending_resolution_id, requested_at FROM artifact_run_checks`, ).all>() const rows = new Map() @@ -231,27 +206,24 @@ async function loadCheckRows(db: D1Database): Promise> { return rows } -async function writeSettled( - db: D1Database, - row: CheckRow, - settled: { outcome: 'ready' | 'failing', tag: string | null, detail: string | null, retryable: boolean, failingSince: number | null }, - now: number, -): Promise { +async function writeSettled(db: D1Database, row: CheckRow, columns: RunCheckColumns, now: number): Promise { await db.prepare( `UPDATE artifact_run_checks SET outcome = ?4, tag = ?5, detail = ?6, retryable = ?7, failing_since = ?8, - settled_at = ?9, settled_resolution_id = pending_resolution_id, - pending_resolution_id = NULL + failure_streak = ?9, failed_at = ?10, settled_at = ?11, + settled_resolution_id = pending_resolution_id, pending_resolution_id = NULL WHERE owner = ?1 AND repository = ?2 AND name = ?3`, ).bind( row.owner, row.repository, row.name, - settled.outcome, - settled.tag, - settled.detail, - settled.retryable ? 1 : 0, - settled.failingSince, + columns.outcome, + columns.tag, + columns.detail, + columns.retryable, + columns.failing_since, + columns.failure_streak, + columns.failed_at, now, ).run() } diff --git a/layers/registry/app/components/SkillDetail.vue b/layers/registry/app/components/SkillDetail.vue index 2dcc0650..335e1135 100644 --- a/layers/registry/app/components/SkillDetail.vue +++ b/layers/registry/app/components/SkillDetail.vue @@ -1,5 +1,6 @@ + + diff --git a/layers/registry/server/api/skill-demos/index.get.ts b/layers/registry/server/api/skill-demos/index.get.ts index d445c80b..9c98d782 100644 --- a/layers/registry/server/api/skill-demos/index.get.ts +++ b/layers/registry/server/api/skill-demos/index.get.ts @@ -1,5 +1,7 @@ import type { SkillDemoView } from '../../utils/skill-demos' +import { runCheckFlagKey } from '#shared/run-check-flags' import { defineApiHandler } from '#shared/server/handler' +import { fetchFlaggedSkillKeys } from '#shared/server/run-check-flags' import { listSkillDemos, presentSkillDemo } from '../../utils/skill-demos' export interface SkillDemoListResponse { @@ -7,15 +9,18 @@ export interface SkillDemoListResponse { } /** - * Every published demo, newest first, for the homepage demo section. The - * manifest ships with the build, so the list changes only on deploy. The list - * reads no D1 row, so it never knows the current commit and marks nothing - * outdated; the Skill page does that. + * Every published demo, newest first, for the homepage demo section and + * `/skills/demos`. The manifest ships with the build. A demo whose Skill holds + * a run check flag stays out until a check passes: a visitor who watches it + * would copy a run command that fails. The list reads no D1 row of its own, + * so it never knows the current commit and marks nothing outdated; the Skill + * page does that. */ export default defineApiHandler({ - handler: ({ event }) => { + handler: async ({ event }) => { setHeader(event, 'Cache-Control', 'public, max-age=300, s-maxage=3600') - return listSkillDemos() + const flagged = await fetchFlaggedSkillKeys('skill-demos') + return listSkillDemos().filter(demo => !flagged.has(runCheckFlagKey(demo.owner, demo.repo, demo.name))) }, presenter: (demos): SkillDemoListResponse => ({ items: demos.map(demo => presentSkillDemo(demo, null)), diff --git a/migrations/0143_artifact_run_check_streak.sql b/migrations/0143_artifact_run_check_streak.sql new file mode 100644 index 00000000..986c802c --- /dev/null +++ b/migrations/0143_artifact_run_check_streak.sql @@ -0,0 +1,20 @@ +-- The run check flag. The Skill page flags its run command once two checks in +-- a row failed for a reason a retry cannot change, and the curated surfaces +-- leave the Skill out while the flag stands. One passing check clears it. +-- +-- A retryable failure, such as a spent GitHub quota, neither counts nor +-- resets the streak, because it says nothing about the Skill. +-- Cull path: drop the flag route, then drop these columns and the index. +ALTER TABLE artifact_run_checks ADD COLUMN failure_streak INTEGER NOT NULL DEFAULT 0; +-- Unix seconds the last failure in the streak settled. +ALTER TABLE artifact_run_checks ADD COLUMN failed_at INTEGER; + +-- A row whose last check failed that way already has one failure in a row. +UPDATE artifact_run_checks +SET failure_streak = 1, failed_at = settled_at +WHERE outcome = 'failing' AND retryable = 0; + +-- The flag route reads only flagged rows. Nearly every row holds 0, so the +-- index keeps that read to the flagged rows instead of the whole table. +CREATE INDEX IF NOT EXISTS idx_artifact_run_checks_failure_streak + ON artifact_run_checks (failure_streak); diff --git a/nuxt.config.ts b/nuxt.config.ts index 2b45c2e2..c741962e 100644 --- a/nuxt.config.ts +++ b/nuxt.config.ts @@ -573,7 +573,9 @@ export default defineNuxtConfig({ // inside the 36000s nuxt-skew-protection keeps old chunks for. // `scripts/check-edge-cache.ts` proves it after each deploy. '/skills/trending': edgeCache({ maxAge: 60, staleWhileRevalidate: 3600 }), - // Demos change only on deploy. + // Demos change on deploy, and when a run check flag hides one. A flag + // takes two failed checks, so a day of stale serving on a quiet colo is + // within the time the checks themselves take. '/skills/demos': edgeCache({ maxAge: 300, staleWhileRevalidate: 86400 }), // The homepage is the same shape as the board: rendered signed out, no // cookie read, and fed by the same feeds, so the same lifetimes hold. A diff --git a/server/api/collections/by-author/[login]/[slug]/index.get.ts b/server/api/collections/by-author/[login]/[slug]/index.get.ts index e0ac41b8..f68e566d 100644 --- a/server/api/collections/by-author/[login]/[slug]/index.get.ts +++ b/server/api/collections/by-author/[login]/[slug]/index.get.ts @@ -1,6 +1,8 @@ import { collectionDetailPresenter } from '~~/server/presenters/collection' import { loadCollectionDetail } from '~~/server/utils/collections' +import { runCheckFlagKey } from '#shared/run-check-flags' import { defineApiHandler } from '#shared/server/handler' +import { fetchFlaggedSkillKeys } from '#shared/server/run-check-flags' export default defineApiHandler({ handler: async ({ event, platform }) => { @@ -9,10 +11,16 @@ export default defineApiHandler({ if (!login || !slug) throw createError({ statusCode: 400, message: 'Missing login or slug' }) - const detail = await loadCollectionDetail(platform.db, login, slug) + const [detail, flagged] = await Promise.all([ + loadCollectionDetail(platform.db, login, slug), + fetchFlaggedSkillKeys('collection-detail'), + ]) if (!detail) throw createError({ statusCode: 404, message: 'Collection not found' }) - return collectionDetailPresenter(detail.collection, detail.skills) + // A Skill whose run checks keep failing leaves the page until a check + // passes. A Repository entry names no single Skill, so it stays. + const skills = detail.skills.filter(skill => !skill.name || !flagged.has(runCheckFlagKey(skill.owner, skill.repo, skill.name))) + return collectionDetailPresenter(detail.collection, skills) }, }) diff --git a/server/api/feed/trending.get.ts b/server/api/feed/trending.get.ts index 925398fe..a9c1739a 100644 --- a/server/api/feed/trending.get.ts +++ b/server/api/feed/trending.get.ts @@ -7,12 +7,15 @@ * catalog. The admin review surface reads the same loader without the filter. */ +import type { TrendingBoard } from '#shared/server/trending-board' import type { FallbackSkill } from '#shared/server/trending-fallback' import type { TrendingRepo } from '#shared/server/trending-repos' import type { TrendingSkill, TrendingSkillEvidence } from '#shared/server/trending-skills' import type { StarPoint } from '#shared/trending-range' import { getDB } from '#server/utils/db' import { cachedFeed } from '#server/utils/feed-cache' +import { runCheckFlagKey } from '#shared/run-check-flags' +import { fetchFlaggedSkillKeys } from '#shared/server/run-check-flags' import { loadStarSeries, starSeriesKey } from '#shared/server/star-series' import { loadTrendingBoard } from '#shared/server/trending-board' import { clipPostText, DEFAULT_WINDOW_HOURS } from '#shared/server/trending-skills' @@ -246,6 +249,25 @@ function toItem(entry: TrendingRepo): TrendingFeedItem { } } +/** + * The board without Skills that hold a run check flag. A board row hands out a + * run command, so a Skill whose run keeps failing stays off it until a check + * passes. A repository card that loses every Skill leaves too. + */ +function withoutFlaggedSkills(board: TrendingBoard, flagged: ReadonlySet): TrendingBoard { + if (flagged.size === 0) + return board + const runs = (owner: string, repo: string, name: string) => !flagged.has(runCheckFlagKey(owner, repo, name)) + return { + ...board, + entries: board.entries + .map(entry => ({ ...entry, skills: entry.skills.filter(skill => runs(entry.owner, entry.repo, skill.name)) })) + .filter(entry => entry.skills.length > 0), + namedSkills: board.namedSkills.filter(entry => runs(entry.owner, entry.repo, entry.slug)), + fallback: board.fallback.filter(entry => runs(entry.owner, entry.repo, entry.slug)), + } +} + export default defineEventHandler( async (event): Promise => { const limit = Math.min(Number(getQuery(event).limit) || 24, 50) @@ -263,7 +285,11 @@ export default defineEventHandler( return cachedFeed(event, 'trending', async () => { const db = getDB(event) const now = Math.floor(Date.now() / 1000) - const { entries, namedSkills, fallback: fallbackSkills } = await loadTrendingBoard({ db, now, limit, windowHours }) + const [board, flagged] = await Promise.all([ + loadTrendingBoard({ db, now, limit, windowHours }), + fetchFlaggedSkillKeys('feed-trending'), + ]) + const { entries, namedSkills, fallback: fallbackSkills } = withoutFlaggedSkills(board, flagged) // The sparkline covers the same window the ranking read, from its first // whole UTC day, so a line never starts before the board does. diff --git a/shared/run-check-flags.ts b/shared/run-check-flags.ts new file mode 100644 index 00000000..9c2fbe09 --- /dev/null +++ b/shared/run-check-flags.ts @@ -0,0 +1,31 @@ +/** + * The run check flag, as artifact delivery answers it at + * `GET /api/run-checks/flags`. + * + * A flagged Skill failed its last two run checks for a reason a retry cannot + * change. Its Skill page keeps the run command and flags it. The curated + * surfaces leave the Skill out while the flag stands. + */ +export interface RunCheckFlag { + owner: string + repository: string + name: string + /** ISO 8601 time the last failing check settled. */ + checkedAt: string + /** The same time as a UTC date, such as `7 Oct 2026`. */ + checkedOn: string + /** Why the run failed, in plain words. */ + reason: string +} + +export interface RunCheckFlagsResponse { + items: RunCheckFlag[] +} + +/** + * One key per Skill, without regard to case, so a route that names a Skill + * with other casing still matches its flag. + */ +export function runCheckFlagKey(owner: string, repository: string, name: string): string { + return `${owner}/${repository}/${name}`.toLowerCase() +} diff --git a/shared/server/run-check-flags.ts b/shared/server/run-check-flags.ts new file mode 100644 index 00000000..79b6cd92 --- /dev/null +++ b/shared/server/run-check-flags.ts @@ -0,0 +1,18 @@ +import type { RunCheckFlagsResponse } from '#shared/run-check-flags' +import { runCheckFlagKey } from '#shared/run-check-flags' + +/** + * The keys of every flagged Skill (`runCheckFlagKey`), for a curated surface + * that leaves those Skills out. + * + * Artifact delivery owns the run checks, so other layers read them over HTTP + * (ADR-0001). A failed read logs under `surface` and returns no keys. The + * surface then shows every Skill, and each Skill page still flags its own run. + */ +export async function fetchFlaggedSkillKeys(surface: string): Promise> { + const response = await $fetch('/api/run-checks/flags').catch(() => { + emitOperationalEvent(createWideEvent({ operation: 'run-check-flags-read', outcome: 'failed', reason: surface })) + return null + }) + return new Set(response?.items.map(flag => runCheckFlagKey(flag.owner, flag.repository, flag.name))) +} diff --git a/test/unit/run-check-state.test.ts b/test/unit/run-check-state.test.ts new file mode 100644 index 00000000..b4a5966d --- /dev/null +++ b/test/unit/run-check-state.test.ts @@ -0,0 +1,49 @@ +import type { RunCheckOutcome, RunCheckState } from '../../layers/artifact-delivery/server/utils/run-check-state' +import { describe, expect, it } from 'vitest' +import { nextRunCheckState, runCheckFlag } from '../../layers/artifact-delivery/server/utils/run-check-state' + +const tooLarge: RunCheckOutcome = { _tag: 'failing', tag: 'CHECK_BLOCKED:source-policy', detail: 'The Skill folder packs to 12 MiB. The limit is 10 MiB.', retryable: false } +const gone: RunCheckOutcome = { _tag: 'failing', tag: 'SOURCE_NOT_FOUND', detail: null, retryable: false } +const spentQuota: RunCheckOutcome = { _tag: 'failing', tag: 'RATE_LIMITED', detail: null, retryable: true } +const unavailable: RunCheckOutcome = { _tag: 'failing', tag: 'SERVICE_UNAVAILABLE', detail: null, retryable: true } +const passed: RunCheckOutcome = { _tag: 'ready' } + +/** Settle each check in order, one minute apart from t=1000. */ +function afterChecks(outcomes: RunCheckOutcome[]): RunCheckState { + return outcomes.reduce( + (state, outcome, index) => nextRunCheckState(state, outcome, 1000 + index * 60), + { _tag: 'unchecked' }, + ) +} + +describe('run check flag', () => { + it('flags a Skill after two checks in a row fail for a reason a retry cannot change', () => { + expect(runCheckFlag(afterChecks([tooLarge]))).toEqual({ _tag: 'clear' }) + expect(runCheckFlag(afterChecks([tooLarge, tooLarge]))).toEqual({ + _tag: 'flagged', + tag: 'CHECK_BLOCKED:source-policy', + detail: 'The Skill folder packs to 12 MiB. The limit is 10 MiB.', + failedAt: 1060, + }) + }) + + it('flags two failures in a row even when the reason changed, and shows the latest', () => { + expect(runCheckFlag(afterChecks([tooLarge, gone]))).toEqual({ _tag: 'flagged', tag: 'SOURCE_NOT_FOUND', detail: null, failedAt: 1060 }) + }) + + it('clears the flag after one passing check, and one later failure does not bring it back', () => { + expect(runCheckFlag(afterChecks([tooLarge, tooLarge, passed]))).toEqual({ _tag: 'clear' }) + expect(runCheckFlag(afterChecks([tooLarge, tooLarge, passed, tooLarge]))).toEqual({ _tag: 'clear' }) + }) + + it('never flags for failures a retry can change', () => { + expect(runCheckFlag(afterChecks([spentQuota, unavailable, spentQuota, unavailable]))).toEqual({ _tag: 'clear' }) + // A spent quota between two real failures does not count as either of them. + expect(runCheckFlag(afterChecks([tooLarge, spentQuota]))).toEqual({ _tag: 'clear' }) + expect(runCheckFlag(afterChecks([tooLarge, spentQuota, tooLarge]))).toMatchObject({ _tag: 'flagged', failedAt: 1120 }) + }) + + it('keeps a flag through a spent quota, dated to the last real failure', () => { + expect(runCheckFlag(afterChecks([tooLarge, tooLarge, spentQuota, unavailable]))).toMatchObject({ _tag: 'flagged', failedAt: 1060 }) + }) +}) diff --git a/test/unit/skill-run-sweep.test.ts b/test/unit/skill-run-sweep.test.ts index 9d426ced..55302f16 100644 --- a/test/unit/skill-run-sweep.test.ts +++ b/test/unit/skill-run-sweep.test.ts @@ -5,6 +5,7 @@ import type { RunnableSkill, SkillRunSweepDependencies } from '../../layers/arti import { describe, expect, it, vi } from 'vitest' import { processArtifactBuild } from '../../layers/artifact-delivery/server/utils/build' import { requestResolution } from '../../layers/artifact-delivery/server/utils/request-resolution' +import { loadRunCheckFlags } from '../../layers/artifact-delivery/server/utils/run-check-flags' import { runSkillRunSweep } from '../../layers/artifact-delivery/server/utils/run-sweep' import { createSqliteD1 } from './helpers/d1-sqlite' @@ -15,6 +16,7 @@ const MIGRATIONS = [ 'migrations/0112_private_artifact_keys.sql', 'migrations/0122_artifact_resolution_retry_after.sql', 'migrations/0142_artifact_run_checks.sql', + 'migrations/0143_artifact_run_check_streak.sql', ] const NOW = 1_791_000_000 const archify: RunnableSkill = { owner: 'tt-a1i', repository: 'archify', name: 'archify' } @@ -79,6 +81,26 @@ describe('skilld run sweep', () => { harness.close() }) + it('flags a Skill once two checks in a row fail, and never for a spent quota', async () => { + const harness = createHarness([archify, busy]) + const blocked: SourceRejection = { _tag: 'rejected', code: 'INVALID_SOURCE', summary: 'Too large.', findings: [] } + const spent: SourceRejection = { _tag: 'rejected', code: 'RATE_LIMITED', summary: 'Spent.', findings: [] } + + await runSkillRunSweep(harness.dependencies) + await harness.buildEach({ archify: blocked, busy: spent }) + await runSkillRunSweep(harness.dependencies) + const afterOne = await loadRunCheckFlags(harness.dependencies.db, null) + await harness.buildEach({ archify: blocked, busy: spent }) + await runSkillRunSweep(harness.dependencies) + const afterTwo = await loadRunCheckFlags(harness.dependencies.db, null) + const named = await loadRunCheckFlags(harness.dependencies.db, { owner: 'TT-A1I', repository: 'Archify', name: 'archify' }) + + expect(afterOne).toEqual([]) + expect(afterTwo).toEqual([{ ...archify, tag: 'CHECK_BLOCKED:source-policy', detail: 'Too large.', failedAt: NOW + 2 * 15 * 60 }]) + expect(named).toEqual(afterTwo) + harness.close() + }) + it('checks the Skill checked longest ago first', async () => { const harness = createHarness([archify, busy], 1) From a097da4479e10865a585e32b4f93cc464310c3a8 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Wed, 7 Oct 2026 02:01:48 +1100 Subject: [PATCH 2/2] fix(registry): read the run flag when the fetch has no data A failed or mocked fetch leaves the data null, so the page must not assume the default answer. --- layers/registry/app/components/SkillDetail.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/layers/registry/app/components/SkillDetail.vue b/layers/registry/app/components/SkillDetail.vue index 335e1135..41736905 100644 --- a/layers/registry/app/components/SkillDetail.vue +++ b/layers/registry/app/components/SkillDetail.vue @@ -312,7 +312,7 @@ const runFlagFetch = useFetch('/api/run-checks/flags', { watch: [slug], default: () => ({ items: [] }), }) -const runFlag = computed(() => runFlagFetch.data.value.items[0] ?? null) +const runFlag = computed(() => runFlagFetch.data.value?.items[0] ?? null) const liveSkillFetch = useAsyncData( () => `skill-live:${slug.value}`,