fix(ci): narrow the version scan and keep a crashing check inside its mode

The scan now reads exactly the manifests the release tooling rewrites, names
every directory and file it cannot read, and refuses an empty corpus; the
command takes the root it checks so the exit code is tested, and the workflow
reports a crashed check as a warning in report mode.

.github/workflows/pr-check-on-head-branch.yml

@@ -107,4 +107,12 @@ jobs:
             echo "::warning title=Pre-Merge Check::this tree has no scripts/pre-merge-check.ts, so nothing was installed, compiled, or checked"
             exit 0
           fi
-          exec node scripts/pre-merge-check.ts
+          if node scripts/pre-merge-check.ts; then
+            exit 0
+          fi
+          # A check that crashes reports nothing, so the step reports its exit the way the mode asks.
+          if [ "$CHECK_MODE" = block ]; then
+            echo "::error title=Pre-Merge Check::scripts/pre-merge-check.ts exited without reporting, so the merged tree is unchecked"
+            exit 1
+          fi
+          echo "::warning title=Pre-Merge Check::scripts/pre-merge-check.ts exited without reporting, so the merged tree is unchecked"

scripts/ci-workflow.spec.ts

@@ -1700,7 +1700,7 @@ describe('Policy workflow shells', () => {
         for (const step of job.steps) {
           if (!isRecord(step) || typeof step.run !== 'string') continue
           const parsed = spawnSync('bash', ['-n'], { input: step.run, encoding: 'utf8' })
-          expect(`${String(step.name ?? step.id)}: ${parsed.stderr}`).toBe(`${String(step.name ?? step.id)}: `)
+          expect(`${String(step.name ?? step.id)}: ${parsed.status}: ${parsed.stderr}`).toBe(`${String(step.name ?? step.id)}: 0: `)
         }
       }
     },
@@ -1750,7 +1750,8 @@ describe('Policy workflows', () => {
     const run = steps[build]?.run
     if (typeof run !== 'string') throw new TypeError('the pre-merge check step must run a command')
     expect(run).toContain('node scripts/pre-merge-check.ts')
-    // The script annotates its own failure, so the step decides by its exit code alone.
-    expect(run).not.toContain('the merged tree fails its pre-merge check')
+    // The script annotates its own failures; the step keeps a mode-aware guard for a check that crashes.
+    expect(run).toContain('exited without reporting')
+    expect(run).toContain('"$CHECK_MODE" = block')
   })
 })

scripts/pre-merge-check.spec.ts

@@ -43,7 +43,11 @@ describe('pre-merge check', () => {
   })
 
   it('checks the versions before it installs, and installs before the builds', () => {
-    expect(preMergeSteps('/repo').map(step => step.name)).toEqual(['versions', 'install', 'host contracts', 'client'])
+    const steps = preMergeSteps('/repo')
+    expect(steps.map(step => step.name)).toEqual(['versions', 'install', 'host contracts', 'client'])
+    expect(steps[0]).toEqual({ name: 'versions', command: process.execPath, args: ['/repo/scripts/verify-version-sync.ts', '/repo'] })
+    expect(steps[1]).toEqual({ name: 'install', command: 'pnpm', args: ['install', '--frozen-lockfile'] })
+    expect(steps[3]?.args).toEqual(['exec', 'tsx', 'scripts/compile-referenced-projects.ts', 'client'])
   })
 
   it('annotates a failure as a warning and passes in report mode', () => {
@@ -52,6 +56,7 @@ describe('pre-merge check', () => {
     expect(checkMode('off')).toBe('off')
     expect(checkMode('nonsense')).toBe('report')
     expect(checkMode(undefined)).toBe('report')
+    expect(checkMode('')).toBe('report')
     expect(failureDecision('report', 'the install step failed')).toEqual({
       line: '::warning title=Pre-Merge Check::the merged tree fails its pre-merge check: the install step failed',
       exitCode: 0,

scripts/pre-merge-check.ts

@@ -28,7 +28,7 @@ export interface PreMergeStep {
  */
 export function preMergeSteps(root: string): PreMergeStep[] {
   return [
-    { name: 'versions', command: process.execPath, args: [join(root, 'scripts', 'verify-version-sync.ts')] },
+    { name: 'versions', command: process.execPath, args: [join(root, 'scripts', 'verify-version-sync.ts'), root] },
     { name: 'install', command: 'pnpm', args: ['install', '--frozen-lockfile'] },
     { name: 'host contracts', command: 'pnpm', args: ['exec', 'tsx', 'scripts/compile-referenced-projects.ts', 'host-contracts'] },
     { name: 'client', command: 'pnpm', args: ['exec', 'tsx', 'scripts/compile-referenced-projects.ts', 'client'] },
@@ -49,7 +49,7 @@ export interface FailureDecision {
  * @param value - The `CHECK_MODE` value, when the environment provides one.
  * @returns The mode; a missing or unknown value reports instead of blocking.
  */
-export function checkMode(value: string | undefined = process.env.CHECK_MODE): CheckMode {
+export function checkMode(value: string | undefined): CheckMode {
   return value === 'off' || value === 'block' ? value : 'report'
 }
 
@@ -124,7 +124,7 @@ if (isMain) {
   if (outcome.ok) {
     process.stdout.write('pre-merge-check: passed\n')
   } else {
-    const decision = failureDecision(checkMode(), outcome.reason ?? 'unknown reason')
+    const decision = failureDecision(checkMode(process.env.CHECK_MODE), outcome.reason ?? 'unknown reason')
     ;(decision.exitCode === 0 ? process.stdout : process.stderr).write(`${decision.line}\n`)
     process.exitCode = decision.exitCode
   }

scripts/verify-version-sync.spec.ts

@@ -1,8 +1,12 @@
+import { spawnSync } from 'node:child_process'
 import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { dirname, join } from 'node:path'
 import { describe, expect, it } from 'vitest'
-import { describeVersions, mismatchedVersions, readVersions, releasedManifests } from './verify-version-sync.ts'
+import { describeVersions, mismatchedVersions, readVersions, releasedManifests, runVersionCheck, versionsSynchronized } from './verify-version-sync.ts'
+
+/** Path of the command the pre-merge check spawns. */
+const script = join(import.meta.dirname, 'verify-version-sync.ts')
 
 /**
  * Build a repository holding the given manifests.
@@ -20,12 +24,14 @@ function repository(files: Record<string, string | null>): string {
 }
 
 describe('released manifests', () => {
-  it('collects the packages and applications of the root version and skips other version lines', () => {
+  it('collects the package groups and the applications, and nothing deeper or beside them', () => {
     const root = repository({
       'package.json': '1.0.0',
       'packages/core/session/package.json': '1.0.0',
-      'packages/AGENTS.md': null,
+      'packages/core/package.json': null,
+      'packages/README.md': null,
       'apps/cli/package.json': '1.0.0',
+      'apps/cli/nested/package.json': '1.0.0',
       'vendor/cordis/package.json': '4.0.5',
       'native/system/package.json': '0.1.2',
     })
@@ -35,10 +41,41 @@ describe('released manifests', () => {
       rmSync(root, { recursive: true, force: true })
     }
   })
+
+  it('fails instead of reporting a narrowed corpus', () => {
+    const root = repository({ 'package.json': '1.0.0', 'packages/README.md': null, 'apps/README.md': null })
+    try {
+      expect(() => releasedManifests(root)).toThrow(/no released manifest/)
+    } finally {
+      rmSync(root, { recursive: true, force: true })
+    }
+  })
+
+  it('fails when a directory cannot be read', () => {
+    const root = repository({ 'package.json': '1.0.0', 'packages/core/session/package.json': '1.0.0' })
+    try {
+      rmSync(join(root, 'apps'), { recursive: true, force: true })
+      expect(() => releasedManifests(root)).toThrow(/cannot read apps\//)
+    } finally {
+      rmSync(root, { recursive: true, force: true })
+    }
+  })
 })
 
 describe('version synchronization', () => {
-  it('names a manifest left on another version or without one', () => {
+  it('accepts a tree whose released manifests all carry the root version', () => {
+    const root = repository({ 'package.json': '3.0.0', 'packages/core/session/package.json': '3.0.0', 'apps/web/package.json': '3.0.0' })
+    try {
+      const report = readVersions(root)
+      expect(mismatchedVersions(report.rootVersion ?? '', report.manifests)).toEqual([])
+      expect(versionsSynchronized(report)).toBe(true)
+      expect(describeVersions(report)).toEqual(['2 released manifest(s) carry 3.0.0'])
+    } finally {
+      rmSync(root, { recursive: true, force: true })
+    }
+  })
+
+  it('rejects a manifest on another version or without one', () => {
     const root = repository({
       'package.json': '2.0.0',
       'packages/core/session/package.json': '2.0.0',
@@ -46,7 +83,9 @@ describe('version synchronization', () => {
       'apps/cli/package.json': null,
     })
     try {
-      expect(describeVersions(readVersions(root))).toEqual([
+      const report = readVersions(root)
+      expect(versionsSynchronized(report)).toBe(false)
+      expect(describeVersions(report)).toEqual([
         '2 of 3 released manifest(s) do not carry the root version 2.0.0:',
         '  apps/cli/package.json: no version',
         '  packages/core/agent/package.json: 1.0.0',
@@ -56,17 +95,37 @@ describe('version synchronization', () => {
     }
   })
 
-  it('accepts a tree whose released manifests all carry the root version', () => {
-    const root = repository({
-      'package.json': '3.0.0',
-      'packages/core/session/package.json': '3.0.0',
-      'apps/web/package.json': '3.0.0',
-      'vendor/cordis/package.json': '4.0.5',
-    })
+  it('rejects an empty corpus and a missing root version', () => {
+    expect(versionsSynchronized({ rootVersion: '1.0.0', manifests: [] })).toBe(false)
+    expect(versionsSynchronized({ rootVersion: undefined, manifests: [{ path: 'apps/cli/package.json', version: '1.0.0' }] })).toBe(false)
+    expect(versionsSynchronized({ rootVersion: undefined, manifests: [] })).toBe(false)
+  })
+})
+
+describe('verify-version-sync command', () => {
+  it('exits 0 on a synchronized tree and 1 with the mismatch named', () => {
+    const good = repository({ 'package.json': '1.0.0', 'packages/core/session/package.json': '1.0.0', 'apps/web/package.json': '1.0.0' })
+    const bad = repository({ 'package.json': '2.0.0', 'packages/core/session/package.json': '1.0.0', 'apps/web/package.json': '2.0.0' })
     try {
-      const report = readVersions(root)
-      expect(mismatchedVersions(report.rootVersion ?? '', report.manifests)).toEqual([])
-      expect(describeVersions(report)).toEqual(['2 released manifest(s) carry 3.0.0'])
+      expect(runVersionCheck(good).ok).toBe(true)
+      const passed = spawnSync(process.execPath, [script, good], { encoding: 'utf8' })
+      expect(passed.status).toBe(0)
+      expect(passed.stdout).toContain('2 released manifest(s) carry 1.0.0')
+      const failed = spawnSync(process.execPath, [script, bad], { encoding: 'utf8' })
+      expect(failed.status).toBe(1)
+      expect(failed.stderr).toContain('packages/core/session/package.json: 1.0.0')
+    } finally {
+      rmSync(good, { recursive: true, force: true })
+      rmSync(bad, { recursive: true, force: true })
+    }
+  })
+
+  it('exits 1 when the tree cannot be read', () => {
+    const root = mkdtempSync(join(tmpdir(), 'dsh-version-sync-empty-'))
+    try {
+      const result = spawnSync(process.execPath, [script, root], { encoding: 'utf8' })
+      expect(result.status).toBe(1)
+      expect(result.stderr).toContain('verify-version-sync:')
     } finally {
       rmSync(root, { recursive: true, force: true })
     }

scripts/verify-version-sync.ts

@@ -1,10 +1,11 @@
 /**
- * Check that every released manifest carries the root version. A release
- * synchronization bumps them together, so a mismatch means a merge combined two
- * version lines: a pull request that bumps versions merged onto a master that
- * added a package, for instance.
+ * Check that every released manifest carries the root version. A release synchronization bumps
+ * them together, so a mismatch means a merge combined two version lines: a pull request that bumps
+ * versions merged onto a master that added a package, for instance.
  *
- * The check reads manifests only, so it needs no installed dependency.
+ * The released manifests are the ones the release tooling rewrites, `packages/<group>/<package>`
+ * and `apps/<application>`. Reading them needs no installed dependency, so the check runs before an
+ * install.
  */
 
 import { readFileSync, readdirSync, statSync } from 'node:fs'
@@ -19,14 +20,31 @@ export interface ManifestVersion {
   readonly version: string | undefined
 }
 
+/** Whether a tree is synchronized, with the lines describing it. */
+export interface VersionOutcome {
+  readonly ok: boolean
+  readonly lines: string[]
+}
+
 /** The root version and every released manifest's version. */
 export interface VersionReport {
   readonly rootVersion: string | undefined
   readonly manifests: ManifestVersion[]
 }
 
-/** Directories that hold released manifests below the repository root. */
-const MANIFEST_DIRECTORIES = ['packages', 'apps'] as const
+/** Where released manifests live: the package groups and the applications. */
+const PACKAGE_ROOT = 'packages'
+const APPLICATION_ROOT = 'apps'
+
+/**
+ * Report whether an error only means the path is not there.
+ * @param error - Caught value.
+ * @returns Whether the entry is absent, as a missing file or a parent that is one.
+ */
+function isMissing(error: unknown): boolean {
+  const code = (error as NodeJS.ErrnoException).code
+  return code === 'ENOENT' || code === 'ENOTDIR'
+}
 
 /**
  * Describe an error for a reader.
@@ -37,60 +55,85 @@ function messageOf(error: unknown): string {
   return error instanceof Error ? error.message : String(error)
 }
 
+/**
+ * Test whether a path is a regular file, treating only "no such file" as a no.
+ * @param path - Absolute path.
+ * @returns Whether the file exists.
+ */
+function isFile(path: string): boolean {
+  try {
+    return statSync(path).isFile()
+  } catch (error) {
+    if (isMissing(error)) return false
+    throw new Error(`cannot read ${path}: ${messageOf(error)}`)
+  }
+}
+
+/**
+ * Test whether a path is a directory.
+ * @param path - Absolute path.
+ * @returns Whether the entry is a directory.
+ */
+function isDirectory(path: string): boolean {
+  try {
+    return statSync(path).isDirectory()
+  } catch (error) {
+    if (isMissing(error)) return false
+    throw new Error(`cannot read ${path}: ${messageOf(error)}`)
+  }
+}
+
+/**
+ * List the entries of a directory, naming the directory when it cannot be read.
+ * @param path - Absolute directory path.
+ * @param label - Repository-relative label for the message.
+ * @returns Entry names.
+ */
+function entries(path: string, label: string): string[] {
+  try {
+    return readdirSync(path)
+  } catch (error) {
+    throw new Error(`cannot read ${label}: ${messageOf(error)}`)
+  }
+}
+
 /**
  * Read the version one `package.json` declares.
  * @param path - Absolute path of the manifest.
  * @returns The declared version, or `undefined` when it declares none.
  */
 function versionOf(path: string): string | undefined {
-  const manifest: unknown = JSON.parse(readFileSync(path, 'utf8'))
-  if (typeof manifest !== 'object' || manifest === null) return undefined
-  const version: unknown = (manifest as { version?: unknown }).version
-  return typeof version === 'string' ? version : undefined
+  try {
+    const manifest: unknown = JSON.parse(readFileSync(path, 'utf8'))
+    if (typeof manifest !== 'object' || manifest === null) return undefined
+    const version: unknown = (manifest as { version?: unknown }).version
+    return typeof version === 'string' ? version : undefined
+  } catch (error) {
+    throw new Error(`cannot read ${path}: ${messageOf(error)}`)
+  }
 }
 
 /**
- * Collect the manifests that must carry the root version: `packages/<group>/<package>` and
- * `apps/<application>`. A directory that cannot be read, and a walk that finds nothing, are
- * errors rather than a clean tree, so a narrowed corpus cannot pass the check.
+ * Collect the manifests the release tooling rewrites: `packages/<group>/<package>/package.json`
+ * and `apps/<application>/package.json`. A directory that cannot be read, and a walk that finds
+ * nothing, are errors rather than a clean tree, so a narrowed corpus cannot pass the check.
  * @param root - Repository root.
  * @returns Manifest paths relative to the root, sorted.
  */
 export function releasedManifests(root: string): string[] {
   const paths: string[] = []
-  for (const directory of MANIFEST_DIRECTORIES) {
-    let groups: string[]
-    try {
-      groups = readdirSync(join(root, directory))
-    } catch (error) {
-      throw new Error(`cannot read ${directory}/: ${messageOf(error)}`)
-    }
-    for (const group of groups) {
-      const groupDirectory = join(root, directory, group)
-      let isDirectory: boolean
-      try {
-        isDirectory = statSync(groupDirectory).isDirectory()
-      } catch (error) {
-        throw new Error(`cannot read ${directory}/${group}: ${messageOf(error)}`)
-      }
-      if (!isDirectory) continue
-      const manifest = join(groupDirectory, 'package.json')
-      try {
-        if (statSync(manifest).isFile()) paths.push(`${directory}/${group}/package.json`)
-      } catch {
-        // A directory without a manifest is not a package.
-      }
-      for (const entry of readdirSync(groupDirectory)) {
-        const nested = `${directory}/${group}/${entry}/package.json`
-        try {
-          if (statSync(join(root, nested)).isFile()) paths.push(nested)
-        } catch {
-          // Not a package directory.
-        }
-      }
+  for (const group of entries(join(root, PACKAGE_ROOT), `${PACKAGE_ROOT}/`)) {
+    if (!isDirectory(join(root, PACKAGE_ROOT, group))) continue
+    for (const entry of entries(join(root, PACKAGE_ROOT, group), `${PACKAGE_ROOT}/${group}/`)) {
+      const path = `${PACKAGE_ROOT}/${group}/${entry}/package.json`
+      if (isFile(join(root, path))) paths.push(path)
     }
   }
-  if (paths.length === 0) throw new Error(`no released manifest was found under ${MANIFEST_DIRECTORIES.map(directory => `${directory}/`).join(' or ')}`)
+  for (const application of entries(join(root, APPLICATION_ROOT), `${APPLICATION_ROOT}/`)) {
+    const path = `${APPLICATION_ROOT}/${application}/package.json`
+    if (isFile(join(root, path))) paths.push(path)
+  }
+  if (paths.length === 0) throw new Error(`no released manifest was found under ${PACKAGE_ROOT}/ or ${APPLICATION_ROOT}/`)
   return paths.sort()
 }
 
@@ -116,9 +159,10 @@ export function readVersions(root: string): VersionReport {
 }
 
 /**
- * Report whether every released manifest carries the root version.
+ * Report whether every released manifest carries the root version. A tree without a root version,
+ * or with no released manifest at all, is not synchronized.
  * @param report - Root version and manifests.
- * @returns Whether the tree is synchronized; a tree without a root version or without manifests is not.
+ * @returns Whether the tree is synchronized.
  */
 export function versionsSynchronized(report: VersionReport): boolean {
   const { rootVersion, manifests } = report
@@ -126,7 +170,7 @@ export function versionsSynchronized(report: VersionReport): boolean {
 }
 
 /**
- * Describe a version mismatch for a reader.
+ * Describe a version report for a reader.
  * @param report - Root version and manifests.
  * @returns One line when the tree is consistent, otherwise a header and one line per mismatched manifest.
  */
@@ -141,14 +185,24 @@ export function describeVersions(report: VersionReport): string[] {
   ]
 }
 
+/**
+ * Run the check over one repository.
+ * @param root - Repository root.
+ * @returns Whether the tree is synchronized, with the lines describing it.
+ */
+export function runVersionCheck(root: string): VersionOutcome {
+  const report = readVersions(root)
+  return { ok: versionsSynchronized(report), lines: describeVersions(report) }
+}
+
 const invokedPath = process.argv[1]
 const isMain = invokedPath !== undefined && import.meta.url === pathToFileURL(resolve(invokedPath)).href
 if (isMain) {
+  const root = process.argv[2] ?? resolve(import.meta.dirname, '..')
   try {
-    const report = readVersions(resolve(import.meta.dirname, '..'))
-    const synchronized = versionsSynchronized(report)
-    for (const line of describeVersions(report)) (synchronized ? console.log : console.error)(`verify-version-sync: ${line}`)
-    if (!synchronized) process.exitCode = 1
+    const { ok, lines } = runVersionCheck(resolve(root))
+    for (const line of lines) (ok ? console.log : console.error)(`verify-version-sync: ${line}`)
+    if (!ok) process.exitCode = 1
   } catch (error) {
     console.error(`verify-version-sync: ${messageOf(error)}`)
     process.exitCode = 1