perf(artifact-delivery): spend GitHub quota once per Skill and commit

layers/artifact-delivery/server/utils/build.ts

@@ -9,7 +9,7 @@ import type {
 import type { ArtifactSigner } from './attestation'
 import type { CheckedArtifactSource } from './checks'
 import type { ArtifactSourceFile, PublicGithubSourceClient, SourceRejection } from './github-source'
-import type { ReadyBuildLookup, ResolutionPatch, ResolutionRow } from './state'
+import type { ReadyBuildLookup, ReadyPublicBuild, ResolutionPatch, ResolutionRow } from './state'
 import type { TrustedRoot } from './trusted-root'
 import { createWideEvent } from '@harlan-zw/nuxt-wide-events/standalone'
 import { emitOperationalEvent } from '#server/utils/operational-event'
@@ -28,16 +28,19 @@ import {
 } from './attestation'
 import { checkArtifactSource, checksBlockArtifact, checksPermitSigning } from './checks'
 import { canonicalJson, digestHex } from './encoding'
-import { isRetryableProblem } from './github-source'
+import { isRetryableProblem, storedFilesPassLoadRules } from './github-source'
 import {
   ARTIFACT_POLICY_VERSION,
+  BYTE_COMPATIBLE_POLICY_VERSIONS,
   findReadyPublicBuild,
   getResolution,
+  hasLeadingBuild,
+  LEADING_BUILD_FRESH_SECONDS,
   parseCheckResults,
   publishArtifactRecord,
   transitionResolution,
 } from './state'
-import { createDeterministicUstar } from './ustar'
+import { createDeterministicUstar, readDeterministicUstar } from './ustar'
 
 export interface ArtifactBuildDependencies {
   db: D1Database
@@ -81,10 +84,17 @@ export type ReuseMissReason
 export type ArtifactBuildReuseReport
   = { _tag: 'hit', lookup: ReadyBuildLookup['_tag'], resolutionId: string, reusedFrom: string }
     | { _tag: 'miss', lookup: ReadyBuildLookup['_tag'], resolutionId: string, reason: ReuseMissReason }
+    /** The stored bytes of a ready build were checked again, with no GitHub read. */
+    | { _tag: 'recheck', lookup: ReadyBuildLookup['_tag'], resolutionId: string, reusedFrom: string }
 
 export type ArtifactBuildOutcome
   = { _tag: 'ready' | 'blocked' | 'failed' | 'unchanged', resolutionId: string }
     | { _tag: 'superseded', resolutionId: string }
+    /** An earlier build of the same Skill at the same commit is running. Process this one again later. */
+    | { _tag: 'deferred', resolutionId: string, delaySeconds: number }
+
+/** How long a build waits before it looks again at the earlier build it follows. */
+export const LEADING_BUILD_WAIT_SECONDS = 2
 
 /**
  * A ready public build of the same commit, verified against the current trusted
@@ -122,6 +132,9 @@ type BuildLoad
     | ReusedBuild
     | { _tag: 'rejected', rejection: SourceRejection }
 
+/** An earlier build of the same source is running, so this one waits for it. */
+interface FollowingBuild { _tag: 'following' }
+
 type StagedStatement
   = { _tag: 'staged', patch: ResolutionPatch }
     | { _tag: 'content-changed' }
@@ -160,9 +173,12 @@ export async function processArtifactBuild(
       // A request pinned to a commit that was built before needs no GitHub
       // read. An unpinned request always resolves on GitHub, so it never gets
       // an older commit than the branch or tag names now.
-      const pinned = await reusePinnedBuild(dependencies, row)
+      const pinned = await startPinnedBuild(dependencies, row)
+      if (pinned?._tag === 'following')
+        return { _tag: 'deferred', resolutionId, delaySeconds: LEADING_BUILD_WAIT_SECONDS }
       if (pinned) {
-        const advanced = await transitionResolution(dependencies.db, row, 'fetching', resolvedSourcePatch(pinned.build.source), now)
+        const pinnedSource = pinned._tag === 'reused' ? pinned.build.source : pinned.source
+        const advanced = await transitionResolution(dependencies.db, row, 'fetching', resolvedSourcePatch(pinnedSource), now)
         if (advanced._tag === 'superseded')
           return { _tag: 'superseded', resolutionId }
         row = advanced.row
@@ -186,7 +202,10 @@ export async function processArtifactBuild(
     }
 
     if (row.state === 'fetching') {
-      const loaded: BuildLoad = carried ?? await reuseOrLoad(dependencies, row)
+      const next: BuildLoad | FollowingBuild = carried ?? await reuseOrLoad(dependencies, row)
+      if (next._tag === 'following')
+        return { _tag: 'deferred', resolutionId, delaySeconds: LEADING_BUILD_WAIT_SECONDS }
+      const loaded: BuildLoad = next
       carried = loaded._tag === 'rejected' ? null : loaded
       const checkResults = loaded._tag === 'loaded'
         ? loaded.checked.checkResults
@@ -380,40 +399,140 @@ function stageReusedStatement(
   }
 }
 
-/** A pinned public request reuses a ready build of its commit before any GitHub read. */
-async function reusePinnedBuild(
+/**
+ * A pinned public request, before any GitHub read: reuse a ready build of its
+ * commit, check stored bytes again, or wait for an earlier build of it.
+ * Null means the request resolves on GitHub.
+ */
+async function startPinnedBuild(
   dependencies: ArtifactBuildDependencies,
   row: ResolutionRow,
-): Promise<ReusedBuild | null> {
+): Promise<ReusedBuild | LoadedBuild | FollowingBuild | null> {
   if (row.visibility !== 'public')
     return null
   const request = sourceRequestFromRow(row)
   if (request.ref?.type !== 'commit')
     return null
-  const decision = await decideReuse(dependencies, row, {
+  const lookup: ReadyBuildLookup = {
     _tag: 'pinned',
     owner: request.owner,
     repository: request.repository,
     commitSha: request.ref.value,
     selector: request.selector,
-  })
-  return decision._tag === 'hit' ? { _tag: 'reused', build: decision.build } : null
+  }
+  const decision = await decideReuse(dependencies, row, lookup)
+  if (decision._tag === 'hit')
+    return { _tag: 'reused', build: decision.build }
+  return (bytesMayBeReused(decision.reason) ? await loadStoredBuild(dependencies, row, lookup) : null)
+    ?? (await followsLeadingBuild(dependencies, row, lookup) ? { _tag: 'following' } : null)
 }
 
-/** After GitHub resolves a public request, reuse a ready build of that exact source, or load it. */
+/**
+ * After GitHub resolves a public request: reuse a ready build of that exact
+ * source, check its stored bytes again, wait for an earlier build of it, or
+ * load it.
+ */
 async function reuseOrLoad(
   dependencies: ArtifactBuildDependencies,
   row: ResolutionRow,
-): Promise<BuildLoad> {
+): Promise<BuildLoad | FollowingBuild> {
   const source = resolvedSourceFromRow(row)
   if (row.visibility === 'public') {
-    const decision = await decideReuse(dependencies, row, { _tag: 'resolved', source })
+    const lookup: ReadyBuildLookup = { _tag: 'resolved', source }
+    const decision = await decideReuse(dependencies, row, lookup)
     if (decision._tag === 'hit')
       return { _tag: 'reused', build: decision.build }
+    const stored = bytesMayBeReused(decision.reason) ? await loadStoredBuild(dependencies, row, lookup) : null
+    if (stored)
+      return stored
+    if (await followsLeadingBuild(dependencies, row, lookup))
+      return { _tag: 'following' }
   }
   return await loadAndCheck(dependencies, row, source)
 }
 
+/**
+ * Whether an earlier build of the same source is running, so this one should
+ * wait for it rather than spend GitHub quota on the same reads. A build waits
+ * at most {@link LEADING_BUILD_FRESH_SECONDS} from its creation.
+ */
+async function followsLeadingBuild(
+  dependencies: ArtifactBuildDependencies,
+  row: ResolutionRow,
+  lookup: ReadyBuildLookup,
+): Promise<boolean> {
+  const now = dependencies.now()
+  if (row.created_at < now - LEADING_BUILD_FRESH_SECONDS)
+    return false
+  return await hasLeadingBuild(dependencies.db, row, lookup, now)
+}
+
+/** A ready build missed only because its policy or checks changed, so its bytes may still serve. */
+function bytesMayBeReused(reason: ReuseMissReason): boolean {
+  return reason === 'policy-changed' || reason === 'checks-changed'
+}
+
+/**
+ * The files of a ready public build, read back from R2 and checked under the
+ * current checks, when its policy packed the same bytes as the current one.
+ *
+ * A policy bump that changes only checks then costs no GitHub read. The
+ * attestation must still verify, R2 must hold the exact bytes, and the files
+ * must pass every rule a GitHub load applies today. Null means load from
+ * GitHub.
+ */
+async function loadStoredBuild(
+  dependencies: ArtifactBuildDependencies,
+  row: ResolutionRow,
+  lookup: ReadyBuildLookup,
+): Promise<LoadedBuild | null> {
+  const ready = await findReadyPublicBuild(dependencies.db, lookup)
+  if (!ready)
+    return null
+  const attestation = parseStoredAttestation(ready.attestationJson)
+  if (
+    !attestation
+    || (attestation.policyVersion !== ARTIFACT_POLICY_VERSION && !BYTE_COMPATIBLE_POLICY_VERSIONS.has(attestation.policyVersion))
+    || !omittedNothing(attestation)
+    || !attestationMatchesRecord(attestation, ready)
+    || !await verifyArtifactAttestation(attestation, dependencies.trustedRoot, dependencies.now())
+  ) {
+    return null
+  }
+  const object = await dependencies.bucket.get(ready.r2Key)
+  if (!object)
+    return null
+  const bytes = new Uint8Array(await object.arrayBuffer())
+  if (bytes.byteLength !== ready.contentBytes || await digestHex('SHA-256', bytes) !== ready.contentSha256)
+    return null
+  const files = await readDeterministicUstar(bytes)
+  if (!files || !storedFilesPassLoadRules(files, ready.source.skillPath))
+    return null
+  const reportReuse = dependencies.reportReuse ?? emitReuseEvent
+  reportReuse({
+    _tag: 'recheck',
+    lookup: lookup._tag,
+    resolutionId: row.id,
+    reusedFrom: ready.resolutionId,
+  })
+  return {
+    _tag: 'loaded',
+    source: ready.source,
+    files,
+    checked: await checkArtifactSource(ready.source, files, []),
+  }
+}
+
+/**
+ * Whether a stored build left no file out. R2 holds only the files it packed,
+ * so a build that left some out cannot be checked again from its bytes.
+ * Policies before the `omitted-files` check never left a file out.
+ */
+function omittedNothing(attestation: ArtifactAttestation): boolean {
+  const omitted = attestation.checkResults.find(check => check.name === 'omitted-files')
+  return omitted === undefined || omitted.outcome === 'pass'
+}
+
 /**
  * The source for a build that resumes after `fetching` in a new invocation.
  * A ready build is reused only when it matches the check results and content
@@ -480,16 +599,8 @@ async function findReusableBuild(
     return { _tag: 'miss', reason: 'policy-changed' }
   if (!checksPermitSigning(attestation.checkResults))
     return { _tag: 'miss', reason: 'checks-changed' }
-  if (
-    attestation.artifactId !== ready.artifactId
-    || ready.artifactId !== `sha256:${ready.contentSha256}`
-    || attestation.contentSha256 !== ready.contentSha256
-    || attestation.contentBytes !== ready.contentBytes
-    || ready.r2Key !== artifactR2Key(ready.contentSha256)
-    || canonicalJson(attestation.source) !== canonicalJson(ready.source)
-  ) {
+  if (!attestationMatchesRecord(attestation, ready))
     return { _tag: 'miss', reason: 'record-mismatch' }
-  }
   const stored = await hasImmutableArtifact(dependencies.bucket, {
     key: ready.r2Key,
     contentSha256: ready.contentSha256,
@@ -511,6 +622,16 @@ async function findReusableBuild(
   }
 }
 
+/** Whether a stored attestation names exactly the bytes and source of its D1 record. */
+function attestationMatchesRecord(attestation: ArtifactAttestation, ready: ReadyPublicBuild): boolean {
+  return attestation.artifactId === ready.artifactId
+    && ready.artifactId === `sha256:${ready.contentSha256}`
+    && attestation.contentSha256 === ready.contentSha256
+    && attestation.contentBytes === ready.contentBytes
+    && ready.r2Key === artifactR2Key(ready.contentSha256)
+    && canonicalJson(attestation.source) === canonicalJson(ready.source)
+}
+
 function parseStoredAttestation(value: string): ArtifactAttestation | null {
   const parsed = artifactAttestationSchema.safeParse(parseJson(value))
   return parsed.success ? parsed.data : null
@@ -528,10 +649,10 @@ function parseJson(value: string): unknown {
 
 /** One wide event per reuse decision, so the daily check-in can count hits. */
 function emitReuseEvent(report: ArtifactBuildReuseReport): void {
-  if (report._tag === 'hit') {
+  if (report._tag === 'hit' || report._tag === 'recheck') {
     emitOperationalEvent(createWideEvent({
       'operation': 'artifact-build-reuse',
-      'outcome': 'hit',
+      'outcome': report._tag,
       'artifact.resolutionId': report.resolutionId,
       'artifact.reuseLookup': report.lookup,
       'artifact.reusedFrom': report.reusedFrom,

layers/artifact-delivery/server/utils/github-source.ts

@@ -184,6 +184,20 @@ interface GithubClientOptions {
   random?: () => number
   /** Unix seconds. A Retry-After delay becomes an absolute retry time against it. */
   now?: () => number
+  /**
+   * Holds GitHub answers that never change: a commit and a tree, each read by
+   * its SHA. Pass it to a public client only, so private trees stay out.
+   */
+  cache?: GithubObjectCache
+}
+
+/**
+ * A store for immutable GitHub answers. `get` answers null for a miss. The
+ * caller parses every value again, so a stale or foreign value only misses.
+ */
+export interface GithubObjectCache {
+  get: (key: string) => Promise<unknown>
+  put: (key: string, value: unknown) => Promise<void>
 }
 
 export function createPublicGithubSourceClient(options: GithubClientOptions): PublicGithubSourceClient {
@@ -283,9 +297,29 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi
     throw new Error(`GitHub read failed at ${step} ${endpoint}: ${result.reason}`)
   }
 
+  /**
+   * A read whose answer the SHA in its key fixes forever. Only an answer that
+   * parsed is stored; a miss, a limit or a refusal always asks GitHub again.
+   */
+  const requestImmutableJson = async <T>(key: string, step: GithubReadStep, path: string, schema: z.ZodType<T>): Promise<ReadOutcome<T>> => {
+    const cache = options.cache
+    if (cache) {
+      const cached = schema.safeParse(await cache.get(key))
+      if (cached.success)
+        return { _tag: 'ok', value: cached.data }
+    }
+    const response = await requestJson(step, path, schema)
+    if (cache && response._tag === 'ok')
+      await cache.put(key, response.value)
+    return response
+  }
+
+  // A tree SHA names its listing in any Repository that holds it, so the key
+  // needs no Repository. The commit that names the tree is read per Repository.
   const getTree = async (owner: string, repository: string, sha: string, recursive: boolean) => {
     const suffix = recursive ? '?recursive=1' : ''
-    const response = await requestJson(
+    const response = await requestImmutableJson(
+      `tree:${sha}${recursive ? ':recursive' : ''}`,
       'tree',
       `/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repository)}/git/trees/${sha}${suffix}`,
       treeResponseSchema,
@@ -555,7 +589,10 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi
         : request.ref.value
       if (typeof requestedCommit !== 'string')
         return requestedCommit
-      const commit = await requestJson(
+      // A commit is keyed by Repository ID, so it proves the commit exists in
+      // this Repository, whatever its name is now.
+      const commit = await requestImmutableJson(
+        `${repository.value.id}:commit:${requestedCommit}`,
         'commit',
         `/repos/${encodeURIComponent(repository.value.owner.login)}/${encodeURIComponent(repository.value.name)}/commits/${requestedCommit}`,
         commitResponseSchema,
@@ -1038,6 +1075,23 @@ function githubBlobUrl(source: ResolvedSource, path: string): string {
   return `https://github.com/${encodeURIComponent(source.owner)}/${encodeURIComponent(source.repository)}/blob/${source.commitSha}/${repositoryPath.split('/').map(encodeURIComponent).join('/')}`
 }
 
+/**
+ * Whether files read back from a stored Artifact pass every rule a GitHub load
+ * applies today, and the load would leave none of them out. A Skill checked
+ * again from its stored bytes then packs exactly what a load would pack.
+ */
+export function storedFilesPassLoadRules(files: ArtifactSourceFile[], skillPath: string): boolean {
+  const entries = files.map(file => ({
+    path: file.path,
+    mode: file.mode === 493 ? '100755' : '100644',
+    type: 'blob' as const,
+    sha: file.gitBlobSha,
+    size: file.bytes.byteLength,
+  }))
+  const selected = selectArtifactEntries(entries, skillPath)
+  return selected._tag === 'selected' && selected.omitted.length === 0
+}
+
 function skillFolderLabel(skillPath: string): string {
   return skillPath === '.' ? 'The Skill folder' : `The Skill folder \`${skillPath}\``
 }