feat(artifact-delivery): flag a run command that keeps failing

docs/arch/README.md

@@ -18,7 +18,7 @@ Use the names exactly. Drift breeds shallow modules.
 - **app**: `/`, `/community`, `/@<login>/*`, `/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.
 

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,
+})

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<Record<string, string>> = {
+  '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),
+    })),
+  }
+}

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<typeof runCheckFlagsQuerySchema>

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<RunCheckFlagRow[]> {
+  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<Record<string, unknown>>()
+  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
+}

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 }
+  }
+}

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<Map<string, CheckRow>> {
   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<Record<string, unknown>>()
   const rows = new Map<string, CheckRow>()
@@ -231,27 +206,24 @@ async function loadCheckRows(db: D1Database): Promise<Map<string, CheckRow>> {
   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<void> {
+async function writeSettled(db: D1Database, row: CheckRow, columns: RunCheckColumns, now: number): Promise<void> {
   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()
 }