From d6a10dfdd09784ab97820f6db0d8589b9bda1ebe Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 11 Feb 2026 19:52:40 -0600 Subject: [PATCH 1/4] fix api-diff to handle errors better --- tools/deno/api-diff.ts | 112 +++++++++++++++++++++++++++-------------- 1 file changed, 74 insertions(+), 38 deletions(-) diff --git a/tools/deno/api-diff.ts b/tools/deno/api-diff.ts index 811e944af..7dca671cf 100755 --- a/tools/deno/api-diff.ts +++ b/tools/deno/api-diff.ts @@ -42,6 +42,7 @@ const SPEC_RAW_URL = (commit: string, filename: string) => async function resolveCommit(ref?: string | number): Promise { if (ref === undefined) return resolveCommit(await pickPr()) if (typeof ref === 'number') { + console.error(`Resolving PR #${ref} to commit...`) const query = `{ repository(owner: "oxidecomputer", name: "omicron") { pullRequest(number: ${ref}) { headRefOid } @@ -50,27 +51,48 @@ async function resolveCommit(ref?: string | number): Promise { const pr = await $`gh api graphql -f query=${query}`.json() return pr.data.repository.pullRequest.headRefOid } + // Tags, branches, and SHAs are passed through directly — the GitHub + // contents API accepts any ref in the ?ref= param return ref } +/** 5 or fewer digits is a PR number; longer digit strings are short SHAs */ const normalizeRef = (ref?: string | number) => - typeof ref === 'string' && /^\d+$/.test(ref) ? parseInt(ref, 10) : ref + typeof ref === 'string' && /^\d{1,5}$/.test(ref) ? parseInt(ref, 10) : ref + +async function listSchemaDir(ref: string) { + console.error(`Fetching schema list for ${ref}...`) + try { + return await $`gh api ${SPEC_DIR_URL(ref)}`.stderr('null').json() + } catch { + throw new Error( + `Could not list openapi/nexus/ at ref '${ref}'. ` + + `This ref may predate the versioned schema directory.` + ) + } +} -async function getLatestSchema(commit: string) { - const contents = await $`gh api ${SPEC_DIR_URL(commit)}`.json() +async function getLatestSchema(ref: string) { + const contents = await listSchemaDir(ref) + const schemaFiles = contents + .map((f: { name: string }) => f.name) + .filter((n: string) => n.startsWith('nexus-')) + .sort() const latestLink = contents.find((f: { name: string }) => f.name === 'nexus-latest.json') - if (!latestLink) throw new Error('nexus-latest.json not found') + if (!latestLink) { + throw new Error( + `nexus-latest.json not found at ref '${ref}'. ` + + `Available schemas: ${schemaFiles.join(', ') || '(none)'}` + ) + } return (await fetch(latestLink.download_url).then((r) => r.text())).trim() } -/** When diffing a single commit, we diff its latest schema against the previous one */ -async function getLatestAndPreviousSchema(commit: string) { - const contents = await $`gh api ${SPEC_DIR_URL(commit)}`.json() +/** When diffing a single ref, we diff its latest schema against the previous one */ +async function getLatestAndPreviousSchema(ref: string) { + const contents = await listSchemaDir(ref) const latestLink = contents.find((f: { name: string }) => f.name === 'nexus-latest.json') - if (!latestLink) throw new Error('nexus-latest.json not found') - const latest = (await fetch(latestLink.download_url).then((r) => r.text())).trim() - const schemaFiles = contents .filter( (f: { name: string }) => f.name.startsWith('nexus-') && f.name !== 'nexus-latest.json' @@ -78,9 +100,18 @@ async function getLatestAndPreviousSchema(commit: string) { .map((f: { name: string }) => f.name) .sort() + if (!latestLink) { + throw new Error( + `nexus-latest.json not found at ref '${ref}'. ` + + `Available schemas: ${schemaFiles.join(', ') || '(none)'}` + ) + } + const latest = (await fetch(latestLink.download_url).then((r) => r.text())).trim() + const latestIndex = schemaFiles.indexOf(latest) - if (latestIndex === -1) throw new Error(`Latest schema ${latest} not found in dir`) - if (latestIndex === 0) throw new Error('No previous schema version found') + if (latestIndex === -1) + throw new Error(`Latest schema ${latest} not found in dir at ref '${ref}'`) + if (latestIndex === 0) throw new Error(`No previous schema version found at ref '${ref}'`) return { previous: schemaFiles[latestIndex - 1], latest } } @@ -90,24 +121,25 @@ async function resolveTarget(ref1?: string | number, ref2?: string): Promise PR number (e.g., 1234) - Commit SHA - Two refs (commits or PRs), compare latest schema on each + PR number, commit SHA, branch, or tag + Two refs, compare latest schema on each Dependencies: - Deno @@ -193,22 +224,27 @@ Dependencies: }) .arguments('[ref1:string] [ref2:string]') .action(async (options, ref?: string, ref2?: string) => { - const target = await resolveTarget(ref, ref2) - const force = options.force ?? false + try { + const target = await resolveTarget(ref, ref2) + const force = options.force ?? false - const [baseSchema, headSchema] = await Promise.all([ - ensureSchema(target.baseCommit, target.baseSchema, force), - ensureSchema(target.headCommit, target.headSchema, force), - ]) - - if (options.format === 'schema') { - await runDiff(baseSchema, headSchema, target.baseSchema, target.headSchema) - } else { - const [baseClient, headClient] = await Promise.all([ - ensureClient(baseSchema, force), - ensureClient(headSchema, force), + const [baseSchema, headSchema] = await Promise.all([ + ensureSchema(target.baseCommit, target.baseSchema, force), + ensureSchema(target.headCommit, target.headSchema, force), ]) - await runDiff(baseClient, headClient, target.baseSchema, target.headSchema) + + if (options.format === 'schema') { + await runDiff(baseSchema, headSchema, target.baseSchema, target.headSchema) + } else { + const [baseClient, headClient] = await Promise.all([ + ensureClient(baseSchema, force), + ensureClient(headSchema, force), + ]) + await runDiff(baseClient, headClient, target.baseSchema, target.headSchema) + } + } catch (e) { + console.error(`error: ${e instanceof Error ? e.message : e}`) + Deno.exit(1) } }) .parse(Deno.args) From 6cfa4544d3dabe69ff511fb1a7974a7e6520e84a Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 12 Feb 2026 13:39:01 -0600 Subject: [PATCH 2/4] handle refs from before we started versioning --- tools/deno/api-diff.ts | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/tools/deno/api-diff.ts b/tools/deno/api-diff.ts index 7dca671cf..47bc10e5d 100755 --- a/tools/deno/api-diff.ts +++ b/tools/deno/api-diff.ts @@ -36,8 +36,8 @@ type DiffTarget = { const SPEC_DIR_URL = (commit: string) => `https://api.github.com/repos/oxidecomputer/omicron/contents/openapi/nexus?ref=${commit}` -const SPEC_RAW_URL = (commit: string, filename: string) => - `https://raw.githubusercontent.com/oxidecomputer/omicron/${commit}/openapi/nexus/${filename}` +const SPEC_RAW_URL = (ref: string, path: string) => + `https://raw.githubusercontent.com/oxidecomputer/omicron/${ref}/${path}` async function resolveCommit(ref?: string | number): Promise { if (ref === undefined) return resolveCommit(await pickPr()) @@ -60,20 +60,23 @@ async function resolveCommit(ref?: string | number): Promise { const normalizeRef = (ref?: string | number) => typeof ref === 'string' && /^\d{1,5}$/.test(ref) ? parseInt(ref, 10) : ref +const LEGACY_SPEC_PATH = 'openapi/nexus.json' + async function listSchemaDir(ref: string) { console.error(`Fetching schema list for ${ref}...`) try { return await $`gh api ${SPEC_DIR_URL(ref)}`.stderr('null').json() } catch { - throw new Error( - `Could not list openapi/nexus/ at ref '${ref}'. ` + - `This ref may predate the versioned schema directory.` - ) + return null } } async function getLatestSchema(ref: string) { const contents = await listSchemaDir(ref) + if (!contents) { + console.error(`No openapi/nexus/ dir at ${ref}, falling back to ${LEGACY_SPEC_PATH}`) + return LEGACY_SPEC_PATH + } const schemaFiles = contents .map((f: { name: string }) => f.name) .filter((n: string) => n.startsWith('nexus-')) @@ -85,12 +88,19 @@ async function getLatestSchema(ref: string) { `Available schemas: ${schemaFiles.join(', ') || '(none)'}` ) } - return (await fetch(latestLink.download_url).then((r) => r.text())).trim() + const latest = (await fetch(latestLink.download_url).then((r) => r.text())).trim() + return `openapi/nexus/${latest}` } /** When diffing a single ref, we diff its latest schema against the previous one */ async function getLatestAndPreviousSchema(ref: string) { const contents = await listSchemaDir(ref) + if (!contents) { + throw new Error( + `No openapi/nexus/ dir at ref '${ref}'. ` + + `Single-ref mode requires the versioned schema directory.` + ) + } const latestLink = contents.find((f: { name: string }) => f.name === 'nexus-latest.json') const schemaFiles = contents @@ -113,7 +123,10 @@ async function getLatestAndPreviousSchema(ref: string) { throw new Error(`Latest schema ${latest} not found in dir at ref '${ref}'`) if (latestIndex === 0) throw new Error(`No previous schema version found at ref '${ref}'`) - return { previous: schemaFiles[latestIndex - 1], latest } + return { + previous: `openapi/nexus/${schemaFiles[latestIndex - 1]}`, + latest: `openapi/nexus/${latest}`, + } } async function resolveTarget(ref1?: string | number, ref2?: string): Promise { From 2dde308b6aefb63a51b326415ddea6cc2b5d0a00 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 12 Feb 2026 13:45:49 -0600 Subject: [PATCH 3/4] fix overzealous fallback eating non-existent ref errors --- tools/deno/api-diff.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tools/deno/api-diff.ts b/tools/deno/api-diff.ts index 47bc10e5d..5af990e32 100755 --- a/tools/deno/api-diff.ts +++ b/tools/deno/api-diff.ts @@ -67,6 +67,15 @@ async function listSchemaDir(ref: string) { try { return await $`gh api ${SPEC_DIR_URL(ref)}`.stderr('null').json() } catch { + // Verify the ref exists — if it doesn't, error out instead of + // falling back to legacy path for a nonexistent ref + try { + await $`gh api repos/oxidecomputer/omicron/commits/${ref}` + .stderr('null') + .stdout('null') + } catch { + throw new Error(`Ref '${ref}' not found in oxidecomputer/omicron`) + } return null } } @@ -163,7 +172,13 @@ async function ensureSchema(commit: string, specFilename: string, force: boolean if (force || !(await exists(schemaPath))) { await $`mkdir -p ${dir}` console.error(`Downloading ${specFilename}...`) - const content = await fetch(SPEC_RAW_URL(commit, specFilename)).then((r) => r.text()) + const resp = await fetch(SPEC_RAW_URL(commit, specFilename)) + if (!resp.ok) { + throw new Error( + `Failed to download ${specFilename} at ${commit}: ${resp.status} ${resp.statusText}` + ) + } + const content = await resp.text() await Deno.writeTextFile(schemaPath, content) } return schemaPath From 7d65cad33e2a9f439e0a5b1eeafbfb350e2bf864 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 12 Feb 2026 13:56:33 -0600 Subject: [PATCH 4/4] go back to resolving to commits and caching by commit hash --- tools/deno/api-diff.ts | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tools/deno/api-diff.ts b/tools/deno/api-diff.ts index 5af990e32..6fc38cf60 100755 --- a/tools/deno/api-diff.ts +++ b/tools/deno/api-diff.ts @@ -51,9 +51,19 @@ async function resolveCommit(ref?: string | number): Promise { const pr = await $`gh api graphql -f query=${query}`.json() return pr.data.repository.pullRequest.headRefOid } - // Tags, branches, and SHAs are passed through directly — the GitHub - // contents API accepts any ref in the ?ref= param - return ref + // Full SHA is already resolved + if (/^[0-9a-f]{40}$/.test(ref)) return ref + // Resolve branches, tags, and short SHAs to a full commit SHA so the + // cache directory is keyed by immutable commit, not a moving ref + console.error(`Resolving ${ref} to commit...`) + try { + const sha = await $`gh api repos/oxidecomputer/omicron/commits/${ref} --jq .sha` + .stderr('null') + .text() + return sha.trim() + } catch { + throw new Error(`Ref '${ref}' not found in oxidecomputer/omicron`) + } } /** 5 or fewer digits is a PR number; longer digit strings are short SHAs */ @@ -63,19 +73,9 @@ const normalizeRef = (ref?: string | number) => const LEGACY_SPEC_PATH = 'openapi/nexus.json' async function listSchemaDir(ref: string) { - console.error(`Fetching schema list for ${ref}...`) try { return await $`gh api ${SPEC_DIR_URL(ref)}`.stderr('null').json() } catch { - // Verify the ref exists — if it doesn't, error out instead of - // falling back to legacy path for a nonexistent ref - try { - await $`gh api repos/oxidecomputer/omicron/commits/${ref}` - .stderr('null') - .stdout('null') - } catch { - throw new Error(`Ref '${ref}' not found in oxidecomputer/omicron`) - } return null } } @@ -215,7 +215,7 @@ async function runDiff( // use -L to set labels, extracting version from spec filename (e.g., nexus-2026010300.0.0-7599dd.json) const getVersion = (spec: string) => - spec.match(/nexus-([^.]+\.[^.]+\.[^.]+)/)?.[1] ?? spec + spec.match(/nexus-([^.]+\.[^.]+\.[^.]+)/)?.[1] ?? 'unversioned' const baseLabel = `a/${getVersion(baseVersion)}/${filename}` const headLabel = `b/${getVersion(headVersion)}/${filename}` // diff exits 1 when files differ, so noThrow() to avoid breaking the pipe