fix(artifact-delivery): keep healthy runs inside their limits

.dev.vars.example

@@ -11,6 +11,9 @@ NUXT_OG_IMAGE_SECRET=
 NUXT_IMAGE_PROXY_KEY=
 NUXT_AI_READY_RUNTIME_SYNC_SECRET=
 GITHUB_TOKEN=
+# Public Artifact builds read GitHub with this token when it is set. Use a token
+# from another GitHub account, so runs keep a quota the registry sync cannot spend.
+ARTIFACT_GITHUB_TOKEN=
 ARTIFACT_PRIVATE_ACCESS_ENABLED=false
 GITHUB_APP_ID=
 GITHUB_APP_CLIENT_ID=

docs/adr/0012-resolution-poll-allowance.md

@@ -0,0 +1,32 @@
+# ADR-0012: Resolution poll allowance
+
+Date: 2026-10-07
+
+Amends [ADR-0008](0008-api-rate-limits.md).
+
+## Context
+
+ADR-0008 gives every v1 operation one shared allowance.
+A guest gets 60 requests per 60 seconds, keyed by `CF-Connecting-IP`.
+
+`skilld run` polls `GET /api/v1/resolutions/{id}` at the rate the server names in `pollAfterMs`.
+The server names 1000 ms. The CLI waits up to 60 seconds for a Resolution.
+One build that takes a minute therefore costs about 60 polls.
+That spends the whole guest allowance, so the run fails with `RATE_LIMITED`.
+A second run from the same network fails the same way.
+
+## Decision
+
+A poll of one Resolution draws from a separate binding, `API_RESOLUTION_POLL_RATE_LIMIT`.
+It allows 600 requests per 60 seconds.
+It uses the ADR-0008 key: the verified account ID, or the guest IP.
+Ten runs from one network can poll through a one-minute build.
+
+A poll is a `GET` of `/api/v1/resolutions/{id}`.
+Every other v1 request keeps the ADR-0008 allowance, including the `POST` that starts a Resolution.
+An exhausted poll allowance answers like an exhausted shared one: HTTP 429, `RATE_LIMITED`, and `Retry-After: 60`.
+
+## Consequences
+
+A poll is one indexed D1 read. 600 polls per minute is 10 reads per second for one key.
+The starting `POST` stays in the shared allowance, so the number of builds one network can start does not change.

docs/arch/README.md

@@ -45,7 +45,7 @@ descriptor (skilld-dev/skilld: packages/sdk/src/contract)
   └─ parity test        test/unit/api-v1-parity.test.ts: one route file per operation
 ```
 
-- **Operation**: one `namespace.verb` entry in the contract, such as `skills.get`. Its ID never changes. Public handlers never receive caller identity. Rate-limit middleware verifies credentials only to select an allowance. Account operations reuse that identity. V1 responses bypass Workers Cache so every network request reaches the limiter. See [ADR-0008](../adr/0008-api-rate-limits.md).
+- **Operation**: one `namespace.verb` entry in the contract, such as `skills.get`. Its ID never changes. Public handlers never receive caller identity. Rate-limit middleware verifies credentials only to select an allowance. Account operations reuse that identity. V1 responses bypass Workers Cache so every network request reaches the limiter. See [ADR-0008](../adr/0008-api-rate-limits.md). Resolution polls draw from their own allowance: [ADR-0012](../adr/0012-resolution-poll-allowance.md).
 - **defineApiOperation**: the shape of a `/api/v1` route: `{ operation, handler }`. The descriptor parses the input, checks the credential, and sets the status and cache headers. The handler only loads data. The answer passes the strict producer schema, so an unnamed field fails closed with a 500.
 - **Route file**: lives in the layer that owns the data, at the operation's own path: `GET /api/v1/skills/{owner}/{repository}/{name}` is `layers/registry/server/api/v1/skills/[owner]/[repository]/[name].get.ts`. It calls that layer's utilities, or reads the internal route in process. It never duplicates SQL.
 - **v1 presenter**: a pure function in `layers/<layer>/server/presenters/<thing>-v1.ts`, typed `OperationResult<typeof op>`.

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

@@ -662,6 +662,13 @@ async function failWithRejection(
   )
 }
 
+/**
+ * The wait a retryable failure names when its cause named none. GitHub asks
+ * for at least a minute after a secondary rate limit, and the build queue
+ * gives up only after several minutes of failed attempts.
+ */
+export const DEFAULT_RETRY_AFTER_SECONDS = 60
+
 export async function failResolution(
   dependencies: Pick<ArtifactBuildDependencies, 'db' | 'now'>,
   row: NonNullable<Awaited<ReturnType<typeof getResolution>>>,
@@ -675,9 +682,13 @@ export async function failResolution(
   // The upstream reset header is an absolute epoch; error_retry_after means a
   // relative delay in seconds, like HTTP Retry-After. Store the delay a reader
   // can add to now, and never let a clock skew store zero or a negative one.
-  const errorRetryAfter = retryAtEpochSeconds === undefined
+  // A retryable failure whose cause named no time still names one, so the
+  // CLI never prints "may be retried" without saying when.
+  const errorRetryAfter = !retryable
     ? undefined
-    : Math.max(1, retryAtEpochSeconds - now)
+    : retryAtEpochSeconds === undefined
+      ? DEFAULT_RETRY_AFTER_SECONDS
+      : Math.max(1, retryAtEpochSeconds - now)
   const advanced = await transitionResolution(dependencies.db, row, 'failed', {
     errorCode: code,
     errorRetryable: retryable,

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

@@ -166,6 +166,8 @@ interface GithubClientOptions {
   onReadFailure?: (failure: GithubReadFailure) => void
   sleep?: (milliseconds: number) => Promise<void>
   random?: () => number
+  /** Unix seconds. A Retry-After delay becomes an absolute retry time against it. */
+  now?: () => number
 }
 
 export function createPublicGithubSourceClient(options: GithubClientOptions): PublicGithubSourceClient {
@@ -174,6 +176,7 @@ export function createPublicGithubSourceClient(options: GithubClientOptions): Pu
 
 export function createGithubSourceClient(options: GithubClientOptions): PublicGithubSourceClient {
   const expectedVisibility = options.visibility ?? 'public'
+  const now = options.now ?? (() => Math.floor(Date.now() / 1000))
   const readRejection = (outcome: FailedReadOutcome) =>
     outcome._tag === 'rate-limited'
       ? rateLimitRejection(outcome.resetAt)
@@ -214,15 +217,15 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi
       const response = fetched.response
       if (response.status === 404)
         return { _tag: 'settled', value: { _tag: 'not-found' } }
-      if (response.status === 401 || response.status === 403) {
-        // A spent quota is a fact about this minute, not about the Repository.
-        // It used to throw, which wrote nothing: the Resolution sat in its
-        // current state through the 60, 120, 240, 480 second delivery ladder and
-        // then failed as SERVICE_UNAVAILABLE with no reason on it.
-        if (response.status === 403 && response.headers.get('x-ratelimit-remaining') === '0')
-          return { _tag: 'settled', value: { _tag: 'rate-limited', resetAt: epochHeader(response.headers, 'x-ratelimit-reset') } }
+      // A spent quota is a fact about this minute, not about the Repository.
+      // It used to throw, which wrote nothing: the Resolution sat in its
+      // current state through the delivery retry ladder and then failed as
+      // SERVICE_UNAVAILABLE with no reason on it.
+      const limited = githubRateLimit(response, now())
+      if (limited)
+        return { _tag: 'settled', value: limited }
+      if (response.status === 401 || response.status === 403)
         return { _tag: 'settled', value: { _tag: 'access-denied' } }
-      }
       if (!response.ok) {
         await response.body?.cancel().catch(() => {
           // The status already decided the outcome. A body that will not close changes nothing.
@@ -552,6 +555,10 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi
         const extracted = await loadFromTarball(source, selected.entries)
         if (extracted._tag === 'extracted')
           return { _tag: 'loaded', value: { source, files: extracted.files, omitted } }
+        // The blobs API reads one file per request against the same limit.
+        // Falling back would spend the most quota exactly when none is left.
+        if (extracted._tag === 'rate-limited')
+          return rateLimitRejection(extracted.resetAt)
         // The tarball is an optimisation, never an authority. Anything it got
         // wrong, including a file `.gitattributes export-ignore` removed from
         // the archive, falls through to the blobs API, which serves every blob
@@ -573,7 +580,7 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi
   async function loadFromTarball(
     source: ResolvedSource,
     entries: Array<TreeEntry & { size: number }>,
-  ): Promise<TarballExtraction> {
+  ): Promise<TarballExtraction | GithubRateLimit> {
     const headers = new Headers({
       'Accept': 'application/vnd.github+json',
       'User-Agent': 'skilld.dev',
@@ -594,6 +601,11 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi
     })
     if ('_tag' in response)
       return { _tag: 'unusable', reason: 'unavailable', findings: [response.message] }
+    const limited = githubRateLimit(response, now())
+    if (limited) {
+      await response.body?.cancel()
+      return limited
+    }
     if (!response.ok || !response.body) {
       await response.body?.cancel()
       return { _tag: 'unusable', reason: 'unavailable', findings: [`GitHub returned ${response.status}`] }
@@ -1011,6 +1023,31 @@ function epochHeader(headers: Headers, name: string): number | null {
   return Number.isSafeInteger(value) && value > 0 ? value : null
 }
 
+interface GithubRateLimit {
+  _tag: 'rate-limited'
+  /** Unix seconds GitHub allows the next request, when it said so. */
+  resetAt: number | null
+}
+
+/**
+ * GitHub's rate limit answer, or null for any other response.
+ *
+ * GitHub answers a spent primary quota with 403 or 429 and no remaining
+ * requests, and a secondary limit with 403 or 429 and Retry-After. A 429 is
+ * always a limit. A 403 with neither header denies access.
+ * https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api
+ */
+export function githubRateLimit(response: Pick<Response, 'status' | 'headers'>, now: number): GithubRateLimit | null {
+  if (response.status !== 403 && response.status !== 429)
+    return null
+  const retryAfter = epochHeader(response.headers, 'retry-after')
+  if (retryAfter !== null)
+    return { _tag: 'rate-limited', resetAt: now + retryAfter }
+  if (response.headers.get('x-ratelimit-remaining') === '0')
+    return { _tag: 'rate-limited', resetAt: epochHeader(response.headers, 'x-ratelimit-reset') }
+  return response.status === 429 ? { _tag: 'rate-limited', resetAt: null } : null
+}
+
 function rateLimitRejection(resetAt: number | null): SourceRejection {
   const rejection = reject('RATE_LIMITED', 'GitHub refused the read: its rate limit is spent.', [])
   return resetAt === null ? rejection : { ...rejection, retryAfterSeconds: resetAt }

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

@@ -25,6 +25,17 @@ const artifactBuildMessageSchema = z.object({
   resolutionId: z.string().uuid(),
 }).strict()
 
+/**
+ * The wait before each retry of a build attempt that threw. A fifth failed
+ * attempt fails the Resolution as SERVICE_UNAVAILABLE.
+ *
+ * The CLI waits 60 seconds for a Resolution, so the first retry comes within
+ * that window: one transient GitHub or signer error no longer fails the run.
+ * It used to wait 60 seconds. The later waits still ride out a long GitHub
+ * network fault, such as the one on 2026-09-30.
+ */
+export const ARTIFACT_BUILD_RETRY_DELAYS_SECONDS: readonly number[] = [5, 30, 120, 480]
+
 export async function enqueueArtifactBuild(env: Cloudflare.Env, resolutionId: string): Promise<void> {
   await env.ARTIFACT_BUILD_QUEUE.send({ version: 1, resolutionId })
 }
@@ -59,8 +70,9 @@ export async function consumeArtifactBuildBatch(
       attempt: message.attempts,
       error: outcome.error instanceof Error ? outcome.error.message : String(outcome.error),
     }))
-    if (message.attempts < 5) {
-      message.retry({ delaySeconds: Math.min(3600, 60 * 2 ** Math.max(0, message.attempts - 1)) })
+    const delaySeconds = ARTIFACT_BUILD_RETRY_DELAYS_SECONDS[message.attempts - 1]
+    if (delaySeconds !== undefined) {
+      message.retry({ delaySeconds })
       continue
     }
     const row = await getResolution(dependencies.db, parsed.data.resolutionId)
@@ -86,14 +98,27 @@ export function reportGithubReadFailure(failure: GithubReadFailure): void {
   }))
 }
 
+/**
+ * The token public Artifact builds read GitHub with.
+ *
+ * GitHub counts a personal token's quota per account, and the registry sync
+ * spends `GITHUB_TOKEN` to zero before each hourly reset. Every run that needed
+ * GitHub in those minutes failed RATE_LIMITED. `ARTIFACT_GITHUB_TOKEN`, from
+ * another account, keeps a quota for runs. Until it is set, builds share
+ * `GITHUB_TOKEN`.
+ */
+export function artifactGithubToken(env: Partial<Pick<Cloudflare.Env, 'ARTIFACT_GITHUB_TOKEN' | 'GITHUB_TOKEN'>>): string | undefined {
+  return env.ARTIFACT_GITHUB_TOKEN || env.GITHUB_TOKEN
+}
+
 export function createArtifactBuildDependencies(env: Cloudflare.Env): ArtifactBuildDependencies {
   const runtimeFetch = globalThis.fetch.bind(globalThis)
   const privateDependencies = privateArtifactAccessEnabled(env)
     ? createPrivateBuildDependencies(env, runtimeFetch)
     : {}
   return {
     db: env.DB,
-    github: createPublicGithubSourceClient({ fetch: runtimeFetch, token: env.GITHUB_TOKEN, onReadFailure: reportGithubReadFailure }),
+    github: createPublicGithubSourceClient({ fetch: runtimeFetch, token: artifactGithubToken(env), onReadFailure: reportGithubReadFailure }),
     bucket: env.PUBLIC_ARTIFACTS,
     signer: createArtifactSigner(env.ARTIFACT_SIGNER),
     trustedRoot: parseTrustedRoot(env.ARTIFACT_TRUSTED_ROOT_JSON, Math.floor(Date.now() / 1000)),

shared/server/api-rate-limit.ts

@@ -13,7 +13,10 @@ declare module 'h3' {
   }
 }
 
-/** One allowance across v1 operations, keyed by verified account or Cloudflare's client IP. */
+/**
+ * One allowance across v1 operations, keyed by verified account or Cloudflare's client IP.
+ * Resolution polls draw from a separate allowance under the same key.
+ */
 export function createApiRateLimitHandler(resolveUser: (event: H3Event) => Promise<UserSession['user'] | null>) {
   return defineEventHandler(async (event) => {
     const path = event.path.split('?')[0]!
@@ -30,7 +33,9 @@ export function createApiRateLimitHandler(resolveUser: (event: H3Event) => Promi
     const { env, requestId } = event.context.platform
     // Do not trust X-Forwarded-For. Missing Cloudflare identity shares a conservative bucket.
     const key = user ? `account:${user.id}` : `guest:${getHeader(event, 'cf-connecting-ip') ?? 'unknown'}`
-    const limiter = user ? env.API_ACCOUNT_RATE_LIMIT : env.API_GUEST_RATE_LIMIT
+    const limiter = isResolutionPoll(event.method, path)
+      ? env.API_RESOLUTION_POLL_RATE_LIMIT
+      : user ? env.API_ACCOUNT_RATE_LIMIT : env.API_GUEST_RATE_LIMIT
     const { success } = await limiter.limit({ key })
     if (success)
       return
@@ -52,3 +57,16 @@ export function createApiRateLimitHandler(resolveUser: (event: H3Event) => Promi
     })
   })
 }
+
+const RESOLUTION_POLL_PATH = /^\/api\/v1\/resolutions\/[^/]+$/
+
+/**
+ * A read of one Resolution while its build runs.
+ *
+ * The server sets the poll rate through `pollAfterMs`, so a poll is not a
+ * request the caller chose to make. A one-minute build costs sixty polls,
+ * which used to spend the whole guest allowance (ADR-0012).
+ */
+function isResolutionPoll(method: string, path: string): boolean {
+  return method === 'GET' && RESOLUTION_POLL_PATH.test(path)
+}

test/unit/api-rate-limit.test.ts

@@ -5,12 +5,14 @@ import { createApiRateLimitHandler } from '../../shared/server/api-rate-limit'
 
 const guest = vi.fn()
 const account = vi.fn()
+const poll = vi.fn()
 const resolveUser = vi.fn()
+const RESOLUTION_ID = '0d6f6c2e-5b1a-4c3e-9f1d-2a7b8c9d0e1f'
 
 function serve() {
   const app = createApp()
   app.use(eventHandler((event) => {
-    event.context.platform = { requestId: 'request-test', env: { API_GUEST_RATE_LIMIT: { limit: guest }, API_ACCOUNT_RATE_LIMIT: { limit: account } } } as never
+    event.context.platform = { requestId: 'request-test', env: { API_GUEST_RATE_LIMIT: { limit: guest }, API_ACCOUNT_RATE_LIMIT: { limit: account }, API_RESOLUTION_POLL_RATE_LIMIT: { limit: poll } } } as never
   }))
   app.use(createApiRateLimitHandler(resolveUser))
   app.use(eventHandler(() => ({ ok: true })))
@@ -22,6 +24,7 @@ describe('api rate limits', () => {
   beforeEach(() => {
     guest.mockReset().mockResolvedValue({ success: true })
     account.mockReset().mockResolvedValue({ success: true })
+    poll.mockReset().mockResolvedValue({ success: true })
     resolveUser.mockReset().mockResolvedValue(null)
   })
 
@@ -93,4 +96,35 @@ describe('api rate limits', () => {
     expect(await response.json()).toMatchObject({ code: 'RATE_LIMITED' })
     expect(guest).not.toHaveBeenCalled()
   })
+
+  // A run polls its Resolution once a second while it builds. Sixty polls used
+  // to spend the whole guest allowance, so one slow build failed the run with
+  // RATE_LIMITED, and so did a second run from the same network.
+  it('counts Resolution polls against their own allowance, not the shared one', async () => {
+    const fetch = serve()
+    for (let attempt = 0; attempt < 3; attempt++)
+      await fetch(`/api/v1/resolutions/${RESOLUTION_ID}`, { 'CF-Connecting-IP': '203.0.113.8' })
+    expect(poll.mock.calls).toEqual(Array.from({ length: 3 }, () => [{ key: 'guest:203.0.113.8' }]))
+    expect(guest).not.toHaveBeenCalled()
+  })
+
+  it('keys an account poll by the account', async () => {
+    resolveUser.mockResolvedValue({ id: 7, login: 'octo' })
+    await serve()(`/api/v1/resolutions/${RESOLUTION_ID}`, { Authorization: 'Bearer valid' })
+    expect(poll).toHaveBeenCalledWith({ key: 'account:7' })
+    expect(account).not.toHaveBeenCalled()
+  })
+
+  it('still counts the request that starts a Resolution against the shared allowance', async () => {
+    await serve()('/api/v1/resolutions', { 'CF-Connecting-IP': '203.0.113.8' }, 'POST')
+    expect(guest).toHaveBeenCalledWith({ key: 'guest:203.0.113.8' })
+    expect(poll).not.toHaveBeenCalled()
+  })
+
+  it('limits polls once their own allowance is spent', async () => {
+    poll.mockResolvedValue({ success: false })
+    const response = await serve()(`/api/v1/resolutions/${RESOLUTION_ID}`, { 'CF-Connecting-IP': '203.0.113.8' })
+    expect(response.status).toBe(429)
+    expect(await response.json()).toMatchObject({ code: 'RATE_LIMITED' })
+  })
 })