fix(config): tighten Zod-backed surfaces after v2.12.0 review (#354)

Cleanup pass on the surfaces the post-v2.12.0 code review flagged but
left as follow-up:

- Convert `ValidationResult` from an interface with optional fields to a
  discriminated union. Callers now narrow with `if (result.success)`
  instead of optional chaining or `as` casts.
- Isolate Zod private-internals reads (`_def.type`, `_def.innerType`,
  `_def.shape`) behind a new `scripts/lib/zod-internals.ts` adapter
  module. The codegen's required-array post-process no longer reaches
  into Zod's private surface inline — one place to update on a future
  zod major bump instead of three.
- Drop the unused `isConfigSchemaError` export. No production consumers
  remained after v2.12.0 landed.
- Add semver validation to the docs generator's `--version` flag,
  mirroring the sibling `generate-config-schema.ts` pattern. Empty or
  malformed values now exit 1 instead of being interpolated into the
  generated `$schema` URL.

While in `generate-config-schema.ts`, the required-array walker was
split into two helpers (`unwrapZodWrappers`, `pruneRequiredArray`) so
the main function stays under the lint cognitive-complexity threshold
after the adapter indirection lands.

Test count: 710 → 723. Schema output byte-identical (drift check passes
with no regeneration needed). Lint warnings: 7 → 6 across the repo.

Author: marcusrbrown

AGENTS.md

@@ -109,7 +109,6 @@ systematic/
 | `loadConfig` | fn | src/lib/config.ts:47 | 5 | JSONC config loading + 3-source merge |
 | `SystematicConfigSchema` | const | src/lib/config-schema.ts | 6 | Canonical Zod schema for user config |
 | `validateConfig` | fn | src/lib/config-schema.ts | 4 | Safe parse wrapper returning ValidationResult |
-| `isConfigSchemaError` | fn | src/lib/config.ts | 2 | Type guard for config validation errors |
 | `SECURITY_OVERLAY_FIELDS` | const | src/lib/config-schema.ts | 3 | Trust-protected overlay field names |
 | `isValidAgentColor` | fn | src/lib/agent-colors.ts | 3 | Color validator (hex or token) |
 | `OPENCODE_AGENT_COLOR_TOKENS` | const | src/lib/agent-colors.ts | 4 | Accepted OpenCode color token enum |

docs/scripts/generate-config-reference.ts

@@ -282,7 +282,14 @@ function renderTopLevelSection(
  *                using the same fallback chain as the JSON Schema generator.
  * @returns The full .mdx content as a string.
  */
+const SEMVER_REGEX = /^\d+\.\d+\.\d+(-[a-zA-Z0-9.-]+)?(\+[a-zA-Z0-9.-]+)?$/
+
 export function generateConfigReference(version?: string): string {
+  if (version !== undefined && !SEMVER_REGEX.test(version)) {
+    throw new Error(
+      `Error: Invalid version format "${version}". Must be valid semver (e.g., 3.0.0)`,
+    )
+  }
   const resolvedVersion = version ?? resolveVersion(null, PROJECT_ROOT)
   const major = getMajorVersion(resolvedVersion)
   const schemaUrl = SCHEMA_ID_TEMPLATE.replace('%MAJOR%', major)

scripts/generate-config-schema.ts

@@ -21,6 +21,11 @@ import path from 'node:path'
 import { fileURLToPath } from 'node:url'
 import { z } from 'zod'
 import { SystematicConfigSchema } from '../src/lib/config-schema.js'
+import {
+  getZodDefaultInnerType,
+  getZodObjectShape,
+  getZodTypeName,
+} from './lib/zod-internals.js'
 
 /**
  * Run Biome's formatter over JSON content via stdin. Used to keep the
@@ -59,6 +64,53 @@ const SEMVER_REGEX = /^\d+\.\d+\.\d+(-[a-zA-Z0-9.-]+)?(\+[a-zA-Z0-9.-]+)?$/
 export const SCHEMA_ID_TEMPLATE =
   'https://fro.bot/systematic/schemas/v%MAJOR%/systematic-config.schema.json'
 
+function resolveVersionFromGit(rootDir: string): string | null {
+  // Prefer `describe` (locates a tag in the commit's ancestry), then fall
+  // back to listing all semver-shaped tags. The fallback covers two real
+  // CI cases that `git describe` cannot:
+  //   - shallow checkout where the tagged commit isn't in local history
+  //   - PR synthetic merge commits that aren't ancestors of any tag
+  for (const cmd of [
+    'git describe --tags --abbrev=0',
+    'git tag -l "v*.*.*" --sort=-v:refname',
+  ]) {
+    let tag = ''
+    try {
+      const out = execSync(cmd, { encoding: 'utf-8', cwd: rootDir }).trim()
+      tag = out.split('\n')[0]?.trim() ?? ''
+    } catch {
+      continue
+    }
+    if (tag.length === 0) continue
+    const normalized = tag.startsWith('v') ? tag.slice(1) : tag
+    if (SEMVER_REGEX.test(normalized)) return normalized
+    throw new Error(
+      `Error: Invalid git tag format "${tag}". Expected semver or v-prefixed semver (e.g., v1.2.3)`,
+    )
+  }
+  return null
+}
+
+function resolveVersionFromPackageJson(rootDir: string): string | null {
+  const pkgPath = path.join(rootDir, 'package.json')
+  try {
+    const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf-8')) as {
+      version?: string
+    }
+    const v = pkg.version
+    if (
+      typeof v === 'string' &&
+      v.length > 0 &&
+      !v.includes('semantic-release')
+    ) {
+      return v
+    }
+  } catch {
+    // fall through
+  }
+  return null
+}
+
 /**
  * Resolve the version to use for schema generation.
  *
@@ -86,48 +138,11 @@ export function resolveVersion(
     return explicit
   }
 
-  // Try git: prefer describe (locates a tag in the commit's ancestry),
-  // fall back to listing all semver-shaped tags sorted by version. The
-  // fallback covers two real CI cases that `git describe` cannot:
-  //   - shallow checkout where the tagged commit isn't in local history
-  //   - PR synthetic merge commits that aren't ancestors of any tag
-  for (const cmd of [
-    'git describe --tags --abbrev=0',
-    'git tag -l "v*.*.*" --sort=-v:refname',
-  ]) {
-    try {
-      const out = execSync(cmd, { encoding: 'utf-8', cwd: rootDir }).trim()
-      const tag = out.split('\n')[0]?.trim() ?? ''
-      if (tag.length === 0) continue
-      const normalized = tag.startsWith('v') ? tag.slice(1) : tag
-      if (SEMVER_REGEX.test(normalized)) {
-        return normalized
-      }
-      throw new Error(
-        `Error: Invalid git tag format "${tag}". Expected semver or v-prefixed semver (e.g., v1.2.3)`,
-      )
-    } catch {
-      // try next strategy
-    }
-  }
+  const fromGit = resolveVersionFromGit(rootDir)
+  if (fromGit != null) return fromGit
 
-  // Try package.json
-  const pkgPath = path.join(rootDir, 'package.json')
-  try {
-    const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf-8')) as {
-      version?: string
-    }
-    const v = pkg.version
-    if (
-      typeof v === 'string' &&
-      v.length > 0 &&
-      !v.includes('semantic-release')
-    ) {
-      return v
-    }
-  } catch {
-    // package.json read failed, fall through to error
-  }
+  const fromPkg = resolveVersionFromPackageJson(rootDir)
+  if (fromPkg != null) return fromPkg
 
   throw new Error(
     'Error: No resolvable version. Specify --version <semver>, ensure a git tag exists, or set a valid semver in package.json.',
@@ -144,53 +159,51 @@ export function getMajorVersion(version: string): string {
 }
 
 /**
- * Walk a JSON Schema object tree and remove fields from `required` arrays
- * when the corresponding Zod shape field has a `.default()` wrapper.
- *
- * A field with a runtime default is optional from the user's perspective:
- * Zod fills it in if absent. Leaving it in `required` causes IDEs to
- * redline valid minimal configs that the runtime accepts fine.
- *
- * Handles both the top-level object and nested objects (e.g., `bootstrap`).
- *
- * @param jsonNode  The JSON Schema object node to process (mutated in-place)
- * @param zodNode   The corresponding Zod schema node (used to detect defaults)
+ * Unwrap `ZodDefault` / `ZodOptional` wrappers to reach the underlying schema
+ * (typically a `ZodObject`). Stops as soon as the inner type is neither.
  */
-function removeDefaultFieldsFromRequired(
+function unwrapZodWrappers(schema: z.ZodType): z.ZodType {
+  let inner: z.ZodType = schema
+  for (;;) {
+    const typeName = getZodTypeName(inner)
+    if (typeName !== 'default' && typeName !== 'optional') return inner
+    const next = getZodDefaultInnerType(inner)
+    if (next == null) return inner
+    inner = next
+  }
+}
+
+/**
+ * Drop keys from `jsonNode.required` whose Zod field has a runtime default.
+ * Removes the `required` key entirely when nothing remains.
+ */
+function pruneRequiredArray(
   jsonNode: Record<string, unknown>,
-  zodNode: z.ZodType,
+  shape: Record<string, z.ZodType>,
 ): void {
-  // Unwrap ZodDefault / ZodOptional wrappers to reach the inner ZodObject.
-  let inner: z.ZodType = zodNode
-  for (;;) {
-    const def = inner._def as { type?: string; innerType?: z.ZodType }
-    if (def.type === 'default' || def.type === 'optional') {
-      inner = def.innerType as z.ZodType
-    } else {
-      break
-    }
+  if (!Array.isArray(jsonNode.required)) return
+  const filtered = (jsonNode.required as string[]).filter((key) => {
+    const field = shape[key]
+    if (field == null) return true
+    return getZodTypeName(field) !== 'default'
+  })
+  if (filtered.length === 0) {
+    delete jsonNode.required
+  } else {
+    jsonNode.required = filtered
   }
+}
 
-  // Only ZodObject has ._def.shape; skip everything else (ZodRecord, ZodArray, …)
+function removeDefaultFieldsFromRequired(
+  jsonNode: Record<string, unknown>,
+  zodNode: z.ZodType,
+): void {
+  // Only ZodObject has a shape; skip everything else (ZodRecord, ZodArray, …)
   // In this Zod 4 build, shape is a plain object (not a lazy function).
-  const innerDef = inner._def as { shape?: Record<string, z.ZodType> }
-  const shape = innerDef.shape
+  const shape = getZodObjectShape(unwrapZodWrappers(zodNode))
   if (shape == null) return
 
-  // Remove keys from `required` when the Zod shape field is ZodDefault
-  if (Array.isArray(jsonNode.required)) {
-    const filtered = (jsonNode.required as string[]).filter((key) => {
-      const field = shape[key]
-      if (field == null) return true
-      const fieldDef = field._def as { type?: string }
-      return fieldDef.type !== 'default'
-    })
-    if (filtered.length === 0) {
-      delete jsonNode.required
-    } else {
-      jsonNode.required = filtered
-    }
-  }
+  pruneRequiredArray(jsonNode, shape)
 
   // Recurse into `properties` to catch nested object schemas (e.g., `bootstrap`)
   const props = jsonNode.properties as

scripts/lib/zod-internals.ts

@@ -0,0 +1,26 @@
+/**
+ * Thin adapters over zod private internals (`_def`). Isolated here so a future
+ * zod major upgrade only needs to change this file. All other code should use
+ * these functions instead of reading `_def` directly.
+ *
+ * Pinned to `zod@4.4.3` exact in package.json; the dependency-version comment
+ * here is the canary — bump zod, re-verify these adapters, then bump the pin.
+ */
+import type { z } from 'zod'
+
+export function getZodTypeName(schema: z.ZodType): string | undefined {
+  return (schema as { _def?: { type?: string } })._def?.type
+}
+
+export function getZodDefaultInnerType(
+  schema: z.ZodType,
+): z.ZodType | undefined {
+  return (schema as { _def?: { innerType?: z.ZodType } })._def?.innerType
+}
+
+export function getZodObjectShape(
+  schema: z.ZodType,
+): Record<string, z.ZodType> | undefined {
+  return (schema as { _def?: { shape?: Record<string, z.ZodType> } })._def
+    ?.shape
+}

src/lib/AGENTS.md

@@ -41,7 +41,7 @@ All discovery follows same pattern: `dir → walkDir() → find files → parseF
 
 | Module | Key Exports | Role |
 |--------|-------------|------|
-| `config.ts` | `loadConfig`, `getConfigPaths`, `SystematicConfig`, `DEFAULT_CONFIG`, `isConfigSchemaError` | JSONC config loading + merging |
+| `config.ts` | `loadConfig`, `getConfigPaths`, `SystematicConfig`, `DEFAULT_CONFIG` | JSONC config loading + merging |
 | `config-schema.ts` | `SystematicConfigSchema`, `validateConfig`, `SECURITY_OVERLAY_FIELDS`, `AgentOverlaySchema`, `CategoryOverlaySchema`, `BootstrapSchema` | Canonical Zod schema for user config; security field list |
 | `agent-colors.ts` | `isValidAgentColor`, `OPENCODE_AGENT_COLOR_TOKENS` | Color validator (hex or named token) + accepted token enum |
 | `config-handler.ts` | `createConfigHandler`, `ConfigHandlerDeps`, `formatAgentDescription`, `toTitleCase` | OpenCode config hook (collects + converts all assets) |

src/lib/config-schema.ts

@@ -281,11 +281,9 @@ export const SystematicConfigSchema = z
     examples: [{ disabled_skills: ['ce:plan'], bootstrap: { enabled: false } }],
   })
 
-export interface ValidationResult {
-  success: boolean
-  data?: z.infer<typeof SystematicConfigSchema>
-  errors?: readonly z.ZodIssue[]
-}
+export type ValidationResult =
+  | { success: true; data: z.infer<typeof SystematicConfigSchema> }
+  | { success: false; errors: readonly z.ZodIssue[] }
 
 export function validateConfig(input: unknown): ValidationResult {
   const result = SystematicConfigSchema.safeParse(input)

src/lib/config.ts

@@ -157,23 +157,6 @@ function throwTopLevelConfigSchemaError(
   })
 }
 
-/**
- * Type guard for errors thrown by the top-level config schema validator.
- * Use this to distinguish schema validation failures from JSONC parse errors
- * or file read errors.
- */
-export function isConfigSchemaError(err: unknown): err is Error & {
-  _tag: 'ConfigSchemaError'
-  filePath: string
-  trust: ConfigSource['trust']
-  issues: readonly z.core.$ZodIssue[]
-} {
-  return (
-    err instanceof Error &&
-    (err as unknown as { _tag: unknown })._tag === 'ConfigSchemaError'
-  )
-}
-
 function loadConfigSource(
   filePath: string,
   trust: ConfigSource['trust'],

tests/unit/config-schema.test.ts

@@ -313,7 +313,7 @@ describe('validateConfig wrapper', () => {
     expect(result.success).toBe(true)
     if (result.success) {
       expect(result.data).toBeDefined()
-      expect(result.data?.bootstrap.enabled).toBe(true)
+      expect(result.data.bootstrap.enabled).toBe(true)
     }
   })
 
@@ -322,7 +322,30 @@ describe('validateConfig wrapper', () => {
     expect(result.success).toBe(false)
     if (!result.success) {
       expect(result.errors).toBeDefined()
-      expect(result.errors?.length ?? 0).toBeGreaterThan(0)
+      expect(result.errors.length).toBeGreaterThan(0)
+    }
+  })
+
+  test('discriminated union: success branch narrows data without optional chaining', () => {
+    const result = validateConfig({})
+    if (result.success) {
+      // After narrowing via result.success, result.data is non-optional.
+      // This test compiles only if ValidationResult is a discriminated union.
+      const enabled: boolean = result.data.bootstrap.enabled
+      expect(enabled).toBe(true)
+    } else {
+      throw new Error('Expected success')
+    }
+  })
+
+  test('discriminated union: failure branch narrows errors without optional chaining', () => {
+    const result = validateConfig({ foo: 'bar' })
+    if (!result.success) {
+      // After narrowing via !result.success, result.errors is non-optional.
+      const count: number = result.errors.length
+      expect(count).toBeGreaterThan(0)
+    } else {
+      throw new Error('Expected failure')
     }
   })
 })

tests/unit/generate-config-reference.test.ts

@@ -481,3 +481,32 @@ describe('schema-described field parity', () => {
     }
   })
 })
+
+describe('--version flag semver validation', () => {
+  let generateFn: (version?: string) => string
+
+  beforeAll(async () => {
+    const mod = await import('../../docs/scripts/generate-config-reference.js')
+    generateFn = mod.generateConfigReference
+  })
+
+  test('valid semver is accepted and produces the expected schema URL', () => {
+    const mdx = generateFn('3.0.0')
+    expect(mdx).toContain('v3')
+  })
+
+  test('pre-release semver is accepted', () => {
+    const mdx = generateFn('3.0.0-alpha.1')
+    expect(mdx).toContain('v3')
+  })
+
+  test('empty version string is rejected with semver error', () => {
+    expect(() => generateFn('')).toThrow('Invalid version format ""')
+  })
+
+  test('garbage version string is rejected with semver error', () => {
+    expect(() => generateFn('garbage')).toThrow(
+      'Invalid version format "garbage"',
+    )
+  })
+})