From 8be3a1d75019ec564012698382928cf260255f0e Mon Sep 17 00:00:00 2001 From: Harlan GitHub Agent Date: Thu, 12 Nov 2026 16:41:10 +1100 Subject: [PATCH] perf(artifact): run checks only after a push and drop replaced checks A run check of an unchanged repository proves nothing new, but it still writes a Resolution with about 60 KB of attestations. Check a Skill again when its repository's pushed_at changes, with a 30-day backstop for ready Skills and 7 days for failing ones. When a check settles, delete the Resolution of the check it replaced unless it holds the newest attestation of its Artifact, so the sweep keeps one row per Skill. Co-Authored-By: Claude Opus 5.5 --- .../server/tasks/sweep-skill-runs.ts | 7 ++-- .../server/utils/resolution-retention.ts | 30 ++++++++++++--- .../server/utils/run-check-state.ts | 36 +++++++++++------- .../server/utils/run-sweep.ts | 30 +++++++++++---- .../server/presenters/runnable-skills.ts | 14 +++++-- .../registry/server/utils/skills-registry.ts | 6 ++- .../0160_artifact_run_check_revision.sql | 6 +++ test/unit/skill-run-sweep.test.ts | 38 ++++++++++++++++++- 8 files changed, 131 insertions(+), 36 deletions(-) create mode 100644 migrations/0160_artifact_run_check_revision.sql diff --git a/layers/artifact-delivery/server/tasks/sweep-skill-runs.ts b/layers/artifact-delivery/server/tasks/sweep-skill-runs.ts index 7694be06..1c9deb10 100644 --- a/layers/artifact-delivery/server/tasks/sweep-skill-runs.ts +++ b/layers/artifact-delivery/server/tasks/sweep-skill-runs.ts @@ -12,8 +12,9 @@ import { runSkillRunSweep } from '../utils/run-sweep' const CRON = '7,22,37,52 * * * *' /** - * At most 40 checks every 15 minutes. A ready Skill is checked weekly and a - * failing one daily (`RECHECK_AFTER_SECONDS`), so most runs start few checks. + * At most 40 checks every 15 minutes. A Skill is checked again after a push to + * its repository, or after a backstop (`RECHECK_AFTER_SECONDS`), so most runs + * start no checks. A settled check deletes the Resolution of the one it replaced. * Each check still writes a Resolution with about 60 KB of attestations, even * when it reuses a ready build. Checking every Skill hourly filled the 10 GB * database on 2026-11-04. A first check loads from GitHub with about 8 REST @@ -59,7 +60,7 @@ export default defineScheduledTask({ const now = () => Math.floor(Date.now() / 1000) const report = await runSkillRunSweep({ db, - listRunnableSkills: async () => (await $fetch<{ items: Array<{ owner: string, repository: string, name: string }> }>('/api/skills/runnable')).items, + listRunnableSkills: async () => (await $fetch<{ items: Array<{ owner: string, repository: string, name: string, revision: string | null }> }>('/api/skills/runnable')).items, requestResolution: input => requestResolution({ db, lookupAdmitted: fetchAdmittedSkillIdentity(), diff --git a/layers/artifact-delivery/server/utils/resolution-retention.ts b/layers/artifact-delivery/server/utils/resolution-retention.ts index 64d87150..930f5380 100644 --- a/layers/artifact-delivery/server/utils/resolution-retention.ts +++ b/layers/artifact-delivery/server/utils/resolution-retention.ts @@ -27,14 +27,11 @@ export const RESOLUTION_PRUNE_BATCH = 500 * reuse copies and the Artifact routes serve. * * Deleting one cascades to its attestation, check results, and requester rows. - * The first filters repeat `idx_artifact_resolutions_state`, so the scan reads + * The batch prune filters by `idx_artifact_resolutions_state`, so its scan reads * only settled rows older than the window. */ -const PRUNE_CANDIDATES = ` - SELECT r.id - FROM artifact_resolutions r INDEXED BY idx_artifact_resolutions_state - WHERE r.state IN ('ready', 'blocked', 'failed', 'revoked') - AND r.updated_at < ?1 +const UNNEEDED = ` + r.state IN ('ready', 'blocked', 'failed', 'revoked') AND r.visibility = 'public' AND r.id NOT IN ( SELECT pending_resolution_id FROM artifact_run_checks WHERE pending_resolution_id IS NOT NULL @@ -54,6 +51,13 @@ const PRUNE_CANDIDATES = ` WHERE own.resolution_id = r.id ) ) +` + +const PRUNE_CANDIDATES = ` + SELECT r.id + FROM artifact_resolutions r INDEXED BY idx_artifact_resolutions_state + WHERE r.updated_at < ?1 + AND ${UNNEEDED} LIMIT ?2 ` @@ -72,3 +76,17 @@ export async function pruneSettledResolutions( ).bind(cutoff, input.limit ?? RESOLUTION_PRUNE_BATCH).run() return result.meta.changes ?? 0 } + +/** + * Delete one Resolution now, when nothing needs it, without waiting for the + * retention window. The run sweep calls this for the check a newer check + * replaced. Returns whether it deleted the row. + */ +export async function deleteUnneededResolution(db: D1Database, id: string): Promise { + const result = await db.prepare( + `DELETE FROM artifact_resolutions WHERE id IN ( + SELECT r.id FROM artifact_resolutions r WHERE r.id = ?1 AND ${UNNEEDED} + )`, + ).bind(id).run() + return (result.meta.changes ?? 0) > 0 +} diff --git a/layers/artifact-delivery/server/utils/run-check-state.ts b/layers/artifact-delivery/server/utils/run-check-state.ts index 2de9457c..80c10db3 100644 --- a/layers/artifact-delivery/server/utils/run-check-state.ts +++ b/layers/artifact-delivery/server/utils/run-check-state.ts @@ -114,35 +114,45 @@ export function runCheckColumns(state: RunCheckState): RunCheckColumns { } } -/** How long a settled check stands before the sweep checks that Skill again. */ +/** How long a settled check stands when its repository gets no push. */ export interface RecheckAfterSeconds { ready: number failing: number } /** - * A ready Skill is checked weekly, a failing one daily. Each check writes a - * Resolution with its attestations, about 60 KB, so checking every Skill every - * hour filled the 10 GB database in November 2026. + * Backstops for a repository that gets no push. A check of an unchanged + * repository still writes a Resolution with about 60 KB of attestations, and + * checking every Skill hourly filled the 10 GB database in November 2026. The + * backstop catches a change in the checks themselves. */ export const RECHECK_AFTER_SECONDS: RecheckAfterSeconds = { - ready: 7 * 24 * 60 * 60, - failing: 24 * 60 * 60, + ready: 30 * 24 * 60 * 60, + failing: 7 * 24 * 60 * 60, +} + +/** What the sweep knows about one Skill when it decides whether to check it. */ +export interface RecheckInput { + state: RunCheckState + settledAt: number | null + /** The repository revision the last check ran at. */ + checkedRevision: string | null + /** The repository revision the registry holds now. */ + revision: string | null } /** * Whether the sweep should check a Skill again now. * * An unchecked Skill and a transient failure, such as a spent quota, are due - * at once: neither says anything about the Skill yet. + * at once: neither says anything about the Skill yet. Otherwise a push to the + * repository makes the Skill due, and the backstop covers the rest. */ -export function recheckDue( - state: RunCheckState, - settledAt: number | null, - now: number, - after: RecheckAfterSeconds, -): boolean { +export function recheckDue(input: RecheckInput, now: number, after: RecheckAfterSeconds): boolean { + const { state, settledAt } = input if (state._tag === 'unchecked' || state._tag === 'transient' || settledAt === null) return true + if (input.revision !== null && input.revision !== input.checkedRevision) + return true return now - settledAt >= (state._tag === 'ready' ? after.ready : after.failing) } diff --git a/layers/artifact-delivery/server/utils/run-sweep.ts b/layers/artifact-delivery/server/utils/run-sweep.ts index 90371105..b643602a 100644 --- a/layers/artifact-delivery/server/utils/run-sweep.ts +++ b/layers/artifact-delivery/server/utils/run-sweep.ts @@ -3,6 +3,7 @@ import type { ResolutionAccess } from './request-resolution' import type { RecheckAfterSeconds, RunCheckColumns } from './run-check-state' import type { CreateResolutionResult, ResolutionRow } from './state' import { z } from 'zod' +import { deleteUnneededResolution } from './resolution-retention' import { nextRunCheckState, RECHECK_AFTER_SECONDS, recheckDue, runCheckColumns, runCheckStateOf } from './run-check-state' import { ACTIVE_BUILD_STATES, getResolution, parseCheckResults } from './state' @@ -11,6 +12,8 @@ export interface RunnableSkill { owner: string repository: string name: string + /** Changes whenever the repository gets a push. Null or absent when unknown. */ + revision?: string | null } export interface SkillRunFailure extends RunnableSkill { @@ -68,7 +71,9 @@ const checkRowSchema = z.object({ failed_at: z.number().int().nullable(), settled_at: z.number().int().nullable(), pending_resolution_id: z.string().nullable(), + settled_resolution_id: z.string().nullable(), requested_at: z.number().int().nullable(), + checked_revision: z.string().nullable(), }) type CheckRow = z.infer @@ -125,7 +130,11 @@ export async function runSkillRunSweep(dependencies: SkillRunSweepDependencies): } 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 }) + // The new check replaces the last one. Its Resolution goes now unless it + // holds the newest attestation of its Artifact. + if (row.settled_resolution_id) + await deleteUnneededResolution(dependencies.db, row.settled_resolution_id) + rows.set(skillKey(skill), { ...row, ...next, pending_resolution_id: null, settled_resolution_id: row.pending_resolution_id, settled_at: now }) } const runnable = await dependencies.listRunnableSkills() @@ -140,7 +149,12 @@ export async function runSkillRunSweep(dependencies: SkillRunSweepDependencies): const row = rows.get(skillKey(skill)) if (!row) return true - return !row.pending_resolution_id && recheckDue(runCheckStateOf(row), row.settled_at, now, recheckAfter) + return !row.pending_resolution_id && recheckDue({ + state: runCheckStateOf(row), + settledAt: row.settled_at, + checkedRevision: row.checked_revision, + revision: skill.revision ?? null, + }, now, recheckAfter) }) .sort((left, right) => (rows.get(skillKey(left))?.settled_at ?? 0) - (rows.get(skillKey(right))?.settled_at ?? 0) @@ -203,7 +217,8 @@ 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, - failure_streak, failed_at, settled_at, pending_resolution_id, requested_at + failure_streak, failed_at, settled_at, pending_resolution_id, settled_resolution_id, + requested_at, checked_revision FROM artifact_run_checks`, ).all>() const rows = new Map() @@ -238,12 +253,13 @@ async function writeSettled(db: D1Database, row: CheckRow, columns: RunCheckColu async function writePending(db: D1Database, skill: RunnableSkill, resolutionId: string, now: number): Promise { await db.prepare( - `INSERT INTO artifact_run_checks (owner, repository, name, pending_resolution_id, requested_at) - VALUES (?1, ?2, ?3, ?4, ?5) + `INSERT INTO artifact_run_checks (owner, repository, name, pending_resolution_id, requested_at, checked_revision) + VALUES (?1, ?2, ?3, ?4, ?5, ?6) ON CONFLICT (owner, repository, name) DO UPDATE SET pending_resolution_id = excluded.pending_resolution_id, - requested_at = excluded.requested_at`, - ).bind(skill.owner, skill.repository, skill.name, resolutionId, now).run() + requested_at = excluded.requested_at, + checked_revision = excluded.checked_revision`, + ).bind(skill.owner, skill.repository, skill.name, resolutionId, now, skill.revision ?? null).run() } function skillKey(skill: RunnableSkill): string { diff --git a/layers/registry/server/presenters/runnable-skills.ts b/layers/registry/server/presenters/runnable-skills.ts index f2477f35..1aa9f49e 100644 --- a/layers/registry/server/presenters/runnable-skills.ts +++ b/layers/registry/server/presenters/runnable-skills.ts @@ -1,7 +1,15 @@ export interface RunnableSkillsDto { - items: Array<{ owner: string, repository: string, name: string }> + /** `revision` changes whenever the repository gets a push. Null when sync has not read it. */ + items: Array<{ owner: string, repository: string, name: string, revision: string | null }> } -export function presentRunnableSkills(rows: Array<{ owner: string, repo: string, name: string }>): RunnableSkillsDto { - return { items: rows.map(row => ({ owner: row.owner, repository: row.repo, name: row.name })) } +export function presentRunnableSkills(rows: Array<{ owner: string, repo: string, name: string, pushedAt: number | null }>): RunnableSkillsDto { + return { + items: rows.map(row => ({ + owner: row.owner, + repository: row.repo, + name: row.name, + revision: row.pushedAt === null ? null : String(row.pushedAt), + })), + } } diff --git a/layers/registry/server/utils/skills-registry.ts b/layers/registry/server/utils/skills-registry.ts index dd8c86e4..90e3571a 100644 --- a/layers/registry/server/utils/skills-registry.ts +++ b/layers/registry/server/utils/skills-registry.ts @@ -742,6 +742,8 @@ export interface SkillSitemapEntry { repo: string /** Resolved-Skill total for the repo; 1 means the hub URL is canonical. */ repoSkillCount: number + /** Unix seconds of the repository's last push, as the last sync read it. */ + pushedAt: number | null } interface SkillDuplicateRow extends DuplicateCandidate { @@ -899,7 +901,7 @@ function findDuplicateGroupInRows(rows: SkillDuplicateRow[], slug: string): Skil export function listAllSkillsForSitemap(event: H3Event): Promise { return cached({ storage: useStorage('cache'), - key: 'skills:sitemap-all:v5', + key: 'skills:sitemap-all:v6', ttlSeconds: DUPLICATE_CANDIDATES_TTL, staleSeconds: DUPLICATE_CANDIDATES_STALE_TTL, compute: () => queryAllSkillsForSitemap(getDB(event)), @@ -946,7 +948,7 @@ export async function queryAllSkillsForSitemap(db: D1Database): Promise row.repo_kind !== 'aggregator' && !weakerSlugs.has(skillSlug(row))) - .map(row => ({ name: row.name, owner: row.owner, repo: row.repo, repoSkillCount: row.repo_skill_count ?? 0 })) + .map(row => ({ name: row.name, owner: row.owner, repo: row.repo, repoSkillCount: row.repo_skill_count ?? 0, pushedAt: row.pushed_at ?? null })) return entries } diff --git a/migrations/0160_artifact_run_check_revision.sql b/migrations/0160_artifact_run_check_revision.sql new file mode 100644 index 00000000..576b99d2 --- /dev/null +++ b/migrations/0160_artifact_run_check_revision.sql @@ -0,0 +1,6 @@ +-- The repository revision the run sweep last checked a Skill at. The sweep +-- checks a Skill again when its repository gets a push, instead of on a timer, +-- because every check writes a Resolution with about 60 KB of attestations. +-- NULL until the next check records it. +-- Cull path: drop the column once the sweep stops reading it. +ALTER TABLE artifact_run_checks ADD COLUMN checked_revision TEXT; diff --git a/test/unit/skill-run-sweep.test.ts b/test/unit/skill-run-sweep.test.ts index ef250e3a..496807d7 100644 --- a/test/unit/skill-run-sweep.test.ts +++ b/test/unit/skill-run-sweep.test.ts @@ -20,6 +20,7 @@ const MIGRATIONS = [ 'migrations/0142_artifact_run_checks.sql', 'migrations/0143_artifact_run_check_streak.sql', 'migrations/0144_artifact_resolution_requesters.sql', + 'migrations/0160_artifact_run_check_revision.sql', ] const NOW = 1_791_000_000 const archify: RunnableSkill = { owner: 'tt-a1i', repository: 'archify', name: 'archify' } @@ -117,7 +118,7 @@ describe('skilld run sweep', () => { harness.close() }) - it('checks a failing Skill again only after a day, and a spent quota at once', async () => { + it('checks a failing Skill again only after the backstop, and a spent quota at once', async () => { const harness = createHarness([archify, busy], 10, RECHECK_AFTER_SECONDS) await runSkillRunSweep(harness.dependencies) @@ -131,11 +132,43 @@ describe('skilld run sweep', () => { expect(settled.started).toBe(1) expect(harness.requested()).toEqual(['acme/busy/busy', 'tt-a1i/archify/archify', 'acme/busy/busy']) harness.advance(RECHECK_AFTER_SECONDS.failing) - // Busy's pending check timed out over that day, so it is due again too. + // Busy's pending check timed out meanwhile, so it is due again too. expect((await runSkillRunSweep(harness.dependencies)).started).toBe(2) expect(harness.requested().slice(-2)).toContain('tt-a1i/archify/archify') harness.close() }) + + it('checks a settled Skill again when its repository gets a push', async () => { + const skills = [{ ...archify, revision: '100' }] + const harness = createHarness(skills, 10, RECHECK_AFTER_SECONDS) + const blocked: SourceRejection = { _tag: 'rejected', code: 'INVALID_SOURCE', summary: 'Too large.', findings: [] } + + await runSkillRunSweep(harness.dependencies) + await harness.buildEach({ archify: blocked }) + const unchanged = await runSkillRunSweep(harness.dependencies) + skills[0] = { ...archify, revision: '200' } + const pushed = await runSkillRunSweep(harness.dependencies) + + expect(unchanged).toMatchObject({ settled: 1, started: 0 }) + expect(pushed.started).toBe(1) + harness.close() + }) + + it('deletes the Resolution of a check a newer check replaced', async () => { + const harness = createHarness([archify]) + const blocked: SourceRejection = { _tag: 'rejected', code: 'INVALID_SOURCE', summary: 'Too large.', findings: [] } + + await runSkillRunSweep(harness.dependencies) + const [first] = harness.resolutionIds() + await harness.buildEach({ archify: blocked }) + await runSkillRunSweep(harness.dependencies) + await harness.buildEach({ archify: blocked }) + await runSkillRunSweep(harness.dependencies) + + expect(harness.resolutionIds()).toHaveLength(2) + expect(harness.resolutionIds()).not.toContain(first) + harness.close() + }) }) function createHarness(skills: RunnableSkill[], batchSize = 10, recheckAfter = { ready: 0, failing: 0 }) { @@ -206,6 +239,7 @@ function createHarness(skills: RunnableSkill[], batchSize = 10, recheckAfter = { buildEach, requested: () => requestedRefs, advance: (seconds: number) => { clock += seconds }, + resolutionIds: () => (sqlite.raw.prepare('SELECT id FROM artifact_resolutions ORDER BY created_at, id').all() as Array<{ id: string }>).map(row => row.id), close: () => sqlite.close(), } }