fix: review findings on release gates (#1-#5, #7-#8, #10-#12)

scripts/lib/distribution-set.ts

@@ -1,4 +1,9 @@
-import { LAUNCHER_MANIFEST_PATH, type LauncherManifest, type Target, TARGETS_PATH } from './shared.ts'
+import {
+  LAUNCHER_MANIFEST_PATH,
+  type LauncherManifest,
+  type Target,
+  TARGETS_PATH,
+} from './shared.ts'
 
 export interface PackageTarget {
   name: string

scripts/lib/matrix-rows.ts

@@ -3,40 +3,70 @@ import { parse as parseYaml } from '@std/yaml'
 export interface MatrixRow {
   target: string
   suffix: string
+  runner: string
 }
 
 /**
- * Extract the release matrix include rows (target + suffix) from a workflow
- * document by parsing YAML, not scraping text (issue #8).
+ * Extract the release matrix include rows (target + suffix + runner) from a
+ * workflow document by parsing YAML, not scraping text (issue #8).
  *
  * Formatting cannot break agreement: flow-style lists, quoted keys or values,
- * key reordering, and comments all parse to the same typed rows. Returns an
- * empty list when no job carries a `strategy.matrix.include` list — callers
- * must treat that as a failure, since an empty matrix cannot agree with the
- * targets table.
+ * key reordering, and comments all parse to the same typed rows. Only the
+ * job named `release` is consulted, so a decoy matrix-bearing job elsewhere
+ * in the workflow cannot satisfy the gate (issue #8 refit). Every include
+ * row must carry string target/suffix/runner; a missing or non-conforming
+ * row, an empty include, a missing `release` job, or malformed YAML throws —
+ * the gate must fail loudly rather than compare against a silent subset.
  */
 export function matrixRows(workflowText: string): MatrixRow[] {
   const doc: unknown = parseYaml(workflowText)
-  if (typeof doc !== 'object' || doc === null) return []
+  if (typeof doc !== 'object' || doc === null) {
+    throw new Error('workflow document is not a mapping')
+  }
   const jobs = (doc as Record<string, unknown>).jobs
-  if (typeof jobs !== 'object' || jobs === null) return []
-  for (const job of Object.values(jobs as Record<string, unknown>)) {
-    if (typeof job !== 'object' || job === null) continue
-    const strategy = (job as Record<string, unknown>).strategy
-    if (typeof strategy !== 'object' || strategy === null) continue
-    const matrix = (strategy as Record<string, unknown>).matrix
-    if (typeof matrix !== 'object' || matrix === null) continue
-    const include = (matrix as Record<string, unknown>).include
-    if (!Array.isArray(include)) continue
-    const rows: MatrixRow[] = []
-    for (const row of include) {
-      if (typeof row !== 'object' || row === null) continue
-      const record = row as Record<string, unknown>
-      if (typeof record.target === 'string' && typeof record.suffix === 'string') {
-        rows.push({ target: record.target, suffix: record.suffix })
-      }
+  if (typeof jobs !== 'object' || jobs === null) {
+    throw new Error('workflow has no jobs mapping')
+  }
+  const releaseJob = (jobs as Record<string, unknown>).release
+  if (typeof releaseJob !== 'object' || releaseJob === null) {
+    throw new Error('workflow has no release job')
+  }
+  const strategy = (releaseJob as Record<string, unknown>).strategy
+  if (typeof strategy !== 'object' || strategy === null) {
+    throw new Error('release job has no strategy')
+  }
+  const matrix = (strategy as Record<string, unknown>).matrix
+  if (typeof matrix !== 'object' || matrix === null) {
+    throw new Error('release job has no strategy.matrix')
+  }
+  const include = (matrix as Record<string, unknown>).include
+  if (!Array.isArray(include)) {
+    throw new Error('release job has no strategy.matrix.include')
+  }
+  if (include.length === 0) {
+    throw new Error('release matrix include is empty')
+  }
+  const rows: MatrixRow[] = []
+  for (const row of include) {
+    if (typeof row !== 'object' || row === null) {
+      throw new Error(`malformed include row (not a mapping): ${JSON.stringify(row)}`)
+    }
+    const record = row as Record<string, unknown>
+    const target = record.target
+    const suffix = record.suffix
+    const runner = record.runner
+    if (
+      typeof target !== 'string' ||
+      typeof suffix !== 'string' ||
+      typeof runner !== 'string'
+    ) {
+      throw new Error(
+        `malformed include row: target/suffix/runner must be strings, got ${
+          JSON.stringify(record)
+        }`,
+      )
     }
-    if (rows.length > 0) return rows
+    rows.push({ target, suffix, runner })
   }
-  return []
+  return rows
 }

scripts/lib/shared.ts

@@ -18,6 +18,7 @@ export interface Target {
   os: string
   cpu: string
   libc?: string
+  runner: string
   bin: string
 }
 

scripts/lib/targets.json

@@ -5,6 +5,7 @@
     "os": "linux",
     "cpu": "x64",
     "libc": "glibc",
+    "runner": "ubuntu-latest",
     "bin": "comment-checker"
   },
   {
@@ -13,27 +14,31 @@
     "os": "linux",
     "cpu": "arm64",
     "libc": "glibc",
+    "runner": "ubuntu-24.04-arm",
     "bin": "comment-checker"
   },
   {
     "target": "x86_64-apple-darwin",
     "suffix": "darwin-x64",
     "os": "darwin",
     "cpu": "x64",
+    "runner": "macos-14",
     "bin": "comment-checker"
   },
   {
     "target": "aarch64-apple-darwin",
     "suffix": "darwin-arm64",
     "os": "darwin",
     "cpu": "arm64",
+    "runner": "macos-14",
     "bin": "comment-checker"
   },
   {
     "target": "x86_64-pc-windows-msvc",
     "suffix": "win32-x64",
     "os": "win32",
     "cpu": "x64",
+    "runner": "windows-2022",
     "bin": "comment-checker.exe"
   }
 ]

scripts/tools/check-matrix.test.ts

@@ -1,10 +1,12 @@
 import { assertEquals, assertThrows } from '@std/assert'
 import { matrixRows } from '../lib/matrix-rows.ts'
 
-// Issue #8 regression fixture: the workflow must be parsed as YAML, so
+// Issue #8 regression fixtures: the workflow must be parsed as YAML, so
 // formatting variants (flow-style include list, quoted target, reordered
-// keys, comments) resolve to the same typed rows the regex scraper used to
-// miss — which let the gate pass vacuously.
+// keys, comments) resolve to the same typed rows. The gate now reads ONLY
+// the named `release` job, requires every row to carry target/suffix/runner,
+// and throws (fails loudly) on any malformed/empty matrix.
+
 const FLOW_STYLE = `name: Release
 on:
   push:
@@ -18,9 +20,9 @@ jobs:
         include: [
           { target: "x86_64-unknown-linux-gnu", suffix: linux-x64, runner: ubuntu-latest },
           { target: aarch64-unknown-linux-gnu, runner: "ubuntu-24.04-arm", suffix: linux-arm64 },
-          { suffix: darwin-x64, target: x86_64-apple-darwin },
-          { target: aarch64-apple-darwin, suffix: darwin-arm64 },
-          { target: x86_64-pc-windows-msvc, suffix: win32-x64 },
+          { suffix: darwin-x64, target: x86_64-apple-darwin, runner: macos-14 },
+          { target: aarch64-apple-darwin, suffix: darwin-arm64, runner: macos-14 },
+          { target: x86_64-pc-windows-msvc, suffix: win32-x64, runner: windows-2022 },
         ]
 `
 
@@ -43,7 +45,7 @@ jobs:
             runner: ubuntu-24.04-arm
           - target: x86_64-apple-darwin
             suffix: darwin-x64
-            runner: macos-13
+            runner: macos-14
           - target: aarch64-apple-darwin
             suffix: darwin-arm64
             runner: macos-14
@@ -52,30 +54,88 @@ jobs:
             runner: windows-2022
 `
 
-const EXPECTED: Array<[string, string]> = [
-  ['x86_64-unknown-linux-gnu', 'linux-x64'],
-  ['aarch64-unknown-linux-gnu', 'linux-arm64'],
-  ['x86_64-apple-darwin', 'darwin-x64'],
-  ['aarch64-apple-darwin', 'darwin-arm64'],
-  ['x86_64-pc-windows-msvc', 'win32-x64'],
+const EXPECTED: Array<[string, string, string]> = [
+  ['x86_64-unknown-linux-gnu', 'linux-x64', 'ubuntu-latest'],
+  ['aarch64-unknown-linux-gnu', 'linux-arm64', 'ubuntu-24.04-arm'],
+  ['x86_64-apple-darwin', 'darwin-x64', 'macos-14'],
+  ['aarch64-apple-darwin', 'darwin-arm64', 'macos-14'],
+  ['x86_64-pc-windows-msvc', 'win32-x64', 'windows-2022'],
 ]
 
 Deno.test('matrixRows parses the block-style workflow', () => {
   const rows = matrixRows(BLOCK_STYLE)
-  assertEquals(rows.map((r) => [r.target, r.suffix]), EXPECTED)
+  assertEquals(
+    rows.map((r) => [r.target, r.suffix, r.runner]),
+    EXPECTED,
+  )
 })
 
 Deno.test('matrixRows parses the flow-style workflow (issue #8 regression)', () => {
   const rows = matrixRows(FLOW_STYLE)
-  assertEquals(rows.map((r) => [r.target, r.suffix]), EXPECTED)
+  assertEquals(
+    rows.map((r) => [r.target, r.suffix, r.runner]),
+    EXPECTED,
+  )
 })
 
 Deno.test('matrixRows throws on malformed YAML (gate must fail loudly)', () => {
-  // A parse error must propagate so the check-matrix CLI reports FAIL rather
-  // than comparing against an empty row set.
   assertThrows(() => matrixRows('jobs: [unclosed'))
 })
 
-Deno.test('matrixRows ignores jobs without a matrix include', () => {
-  assertEquals(matrixRows('jobs:\n  lint:\n    runs-on: ubuntu-latest\n'), [])
+Deno.test('matrixRows throws when the release job is missing (issue #8 refit)', () => {
+  // A decoy job with a matrix must NOT satisfy the gate: only the named
+  // `release` job is authoritative, so a workflow without it throws.
+  assertThrows(() =>
+    matrixRows(
+      'jobs:\n' +
+        '  build:\n' +
+        '    strategy:\n' +
+        '      matrix:\n' +
+        '        include:\n' +
+        '          - target: x86_64-unknown-linux-gnu\n' +
+        '            suffix: linux-x64\n' +
+        '            runner: ubuntu-latest\n',
+    )
+  )
+})
+
+Deno.test('matrixRows throws when the release matrix include is empty', () => {
+  assertThrows(() =>
+    matrixRows(
+      'jobs:\n' +
+        '  release:\n' +
+        '    strategy:\n' +
+        '      matrix:\n' +
+        '        include: []\n',
+    )
+  )
+})
+
+Deno.test('matrixRows throws when a release row lacks a runner (#5)', () => {
+  assertThrows(() =>
+    matrixRows(
+      'jobs:\n' +
+        '  release:\n' +
+        '    strategy:\n' +
+        '      matrix:\n' +
+        '        include:\n' +
+        '          - target: x86_64-unknown-linux-gnu\n' +
+        '            suffix: linux-x64\n',
+    )
+  )
+})
+
+Deno.test('matrixRows throws when a release row has a non-string suffix (#11)', () => {
+  assertThrows(() =>
+    matrixRows(
+      'jobs:\n' +
+        '  release:\n' +
+        '    strategy:\n' +
+        '      matrix:\n' +
+        '        include:\n' +
+        '          - target: x86_64-unknown-linux-gnu\n' +
+        '            suffix: [linux, x64]\n' +
+        '            runner: ubuntu-latest\n',
+    )
+  )
 })

scripts/tools/check-matrix.ts

@@ -14,6 +14,11 @@ import {
 // derived from the table under check.
 const EXPECTED_SUFFIXES = ['linux-x64', 'linux-arm64', 'darwin-x64', 'darwin-arm64', 'win32-x64']
 
+// Known-good hosted runners. The release workflow's per-row runner must match
+// the table's canonical runner AND be one of these — a retired or mistyped
+// label (e.g. macos-13) must fail the gate, not pass it (issue #8 refit).
+const KNOWN_RUNNERS = new Set(['ubuntu-latest', 'ubuntu-24.04-arm', 'macos-14', 'windows-2022'])
+
 const failures: string[] = []
 const fail = (reason: string) => failures.push(reason)
 const note = (message: string) => console.error(`check-matrix: note: ${message}`)
@@ -60,6 +65,13 @@ function checkTable(targets: Target[]) {
         `target ${entry.target}: suffix "${entry.suffix}" must equal os-cpu "${entry.os}-${entry.cpu}"`,
       )
     }
+    if (!KNOWN_RUNNERS.has(entry.runner)) {
+      fail(
+        `target ${entry.target}: runner "${entry.runner}" is not a known runner; known are ${
+          [...KNOWN_RUNNERS].join(', ')
+        }`,
+      )
+    }
     if ((entry.os === 'win32') !== (entry.bin === 'comment-checker.exe')) {
       fail(
         `target ${entry.target}: bin must be comment-checker.exe iff os is win32 (os: ${entry.os}, bin: ${entry.bin})`,
@@ -117,30 +129,38 @@ async function checkWorkflow(workflowPath: string, targets: Target[]) {
     await Deno.lstat(workflowPath)
     const content = await Deno.readTextFile(workflowPath)
     // Typed YAML parse (issue #8): formatting variants (flow style, quoting,
-    // key order) must not change what rows are seen, and malformed YAML must
-    // fail the gate instead of yielding an empty match set.
-    const workflowPairs = new Map(matrixRows(content).map((row) => [row.target, row.suffix]))
-    const tablePairs = new Map(targets.map((t) => [t.target, t.suffix]))
-    for (const [target, suffix] of tablePairs) {
-      if (!workflowPairs.has(target)) {
-        fail(`release.yml does not list release target ${target}`)
-      } else if (workflowPairs.get(target) !== suffix) {
+    // key order) must not change what rows are seen, and any malformed or
+    // empty matrix — or a missing release job — throws inside matrixRows and
+    // fails the gate instead of yielding an empty match set.
+    const workflowRows = matrixRows(content)
+    const tableRows = new Map(targets.map((t) => [t.target, t]))
+    if (workflowRows.length !== targets.length) {
+      fail(
+        `release.yml lists ${workflowRows.length} matrix rows; targets.json has ${targets.length}`,
+      )
+    }
+    for (const row of workflowRows) {
+      const entry = tableRows.get(row.target)
+      if (!entry) {
+        fail(`release.yml lists ${row.target}, which is not a row in targets.json`)
+        continue
+      }
+      if (row.suffix !== entry.suffix) {
         fail(
-          `release.yml lists ${target} with suffix ${
-            workflowPairs.get(target)
-          }, table says ${suffix}`,
+          `release.yml lists ${row.target} with suffix ${row.suffix}, table says ${entry.suffix}`,
         )
       }
-    }
-    for (const [target, suffix] of workflowPairs) {
-      if (!tablePairs.has(target)) {
-        fail(`release.yml lists ${target}, which is not a row in targets.json`)
-      } else if (suffix !== tablePairs.get(target)) {
+      if (row.runner !== entry.runner) {
         fail(
-          `release.yml lists ${target} with suffix ${suffix}, table says ${tablePairs.get(target)}`,
+          `release.yml lists ${row.target} on runner ${row.runner}, table says ${entry.runner}`,
         )
       }
     }
+    for (const entry of targets) {
+      if (!workflowRows.some((row) => row.target === entry.target)) {
+        fail(`release.yml does not list release target ${entry.target}`)
+      }
+    }
   } catch (error) {
     if (error instanceof Deno.errors.NotFound) {
       note(`skipped: ${workflowPath} not found (workflow agreement not checked)`)