Fix frontmatter scalar validation on clean history - #3134
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07802d143e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Please review exact head |
|
@codex please run the branch's complete governance regression surface without changing files: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3562268ccb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Governance Regression ReportVerified both before and after execution that Results
Outstanding Human Actions
|
|
@codex address the two regression failures without changing the rejection boundary. Preserve the established policy diagnostic by classifying leading/trailing whitespace before the broader control-character check, while internal C0/DEL/C1 characters must still fail as controls. Run both governance test files and the checker. Since your checkout cannot push, reply with the complete final contents of |
|
@codex superseding the narrower diagnostic-only request: address both current unresolved exact-head findings in one bounded repair. Use the default YAML implicit resolver as the compatibility oracle, not broad case-insensitive approximations. At minimum, add red/green counterexamples proving:
Prefer resolver-exact, readable patterns with comments over one permissive case-insensitive regex. Re-run both governance test files, the live checker, syntax check, and diff check. Update the evidence note with exact current counts and remove stale numbers. Your checkout cannot push. Reply with the complete final contents of every changed file, each in its own fenced block labelled with the path, plus exact test results. Do not change workflows, policy files, or rule files. |
#!/usr/bin/env node
import { access, readFile } from 'node:fs/promises'
import { constants as fsConstants } from 'node:fs'
import { resolve } from 'node:path'
import { fileURLToPath } from 'node:url'
export const CI_POLICY_PATH = 'ci/policy.v1.json'
export const CI_CONTROL_RULE_PATH = '.claude/rules/ci-control.md'
const FORBIDDEN_SCALAR_CONTROL = /[\u0000-\u001F\u007F-\u009F]/u
const YAML_NULL_SCALAR = /^(?:~|null)$/i
const YAML_BOOLEAN_SCALAR = /^(?:true|false)$/i
const YAML_INTEGER_SCALAR = /^[+-]?(?:0b[01](?:_?[01])*|0o[0-7](?:_?[0-7])*|0x[0-9a-f](?:_?[0-9a-f])*|[0-9](?:_?[0-9])*)$/i
const YAML_FLOAT_SCALAR = /^[+-]?(?:(?:[0-9](?:_?[0-9])*)?\.[0-9](?:_?[0-9])*(?:e[+-]?[0-9](?:_?[0-9])*)?|[0-9](?:_?[0-9])*\.(?:[0-9](?:_?[0-9])*)?(?:e[+-]?[0-9](?:_?[0-9])*)?|[0-9](?:_?[0-9])*e[+-]?[0-9](?:_?[0-9])*)$/i
const YAML_NON_FINITE_FLOAT_SCALAR = /^[+-]?\.(?:inf|nan)$/i
const YAML_DATE_SCALAR = /^\d{4}-\d{2}-\d{2}$/
const YAML_TIMESTAMP_SCALAR = /^\d{4}-\d{2}-\d{2}(?:[Tt]|[ \t]+)\d{1,2}:\d{2}:\d{2}(?:\.\d+)?(?:[ \t]*(?:[Zz]|[+-]\d{1,2}(?::?\d{2})?))?$/
const requiredDocs = [
'docs/STATUS.md',
'docs/IMPLEMENTATION_MASTERPLAN.md',
'docs/TESTING_GUIDE.md',
'docs/MANUAL_TEST_CHECKLIST.md',
'docs/GOLDEN_PRINCIPLES.md',
]
const errors = []
async function fileExists(path) {
try {
await access(resolve(path), fsConstants.F_OK)
return true
} catch {
return false
}
}
function expectContains(source, token, label) {
if (!source.includes(token)) {
errors.push(`${label} is missing required token: ${token}`)
}
}
/**
* Read `controlPaths` out of the Smart CI policy document.
*
* Fails closed: an unreadable policy, a missing `controlPaths` array, or a non-string entry is an
* error rather than an empty list, because an empty list would make the mirror check vacuously pass.
*/
export function parsePolicyControlPaths(policyText, policyPath = CI_POLICY_PATH) {
let policy
try {
policy = JSON.parse(policyText)
} catch (error) {
return { controlPaths: [], errors: [`${policyPath} is not parseable JSON: ${error.message}`] }
}
const controlPaths = policy?.controlPaths
if (!Array.isArray(controlPaths)) {
return { controlPaths: [], errors: [`${policyPath} does not declare a controlPaths array`] }
}
if (controlPaths.length === 0) {
return { controlPaths: [], errors: [`${policyPath} declares an empty controlPaths array`] }
}
if (controlPaths.some((entry) => typeof entry === 'string' && /^\s|\s$/u.test(entry))) {
return {
controlPaths: [],
errors: [`${policyPath} controlPaths must not contain leading or trailing whitespace`],
}
}
const invalid = controlPaths.filter(
(entry) => typeof entry !== 'string' || entry.length === 0 || FORBIDDEN_SCALAR_CONTROL.test(entry),
)
if (invalid.length > 0) {
return {
controlPaths: [],
errors: [`${policyPath} controlPaths must contain only non-empty strings without control characters`],
}
}
return { controlPaths, errors: [] }
}
/**
* Read the supported single-line scalar subset, not arbitrary YAML.
*
* Quoted strings support JSON double-quote escapes or YAML doubled single quotes. Plain scalars
* keep internal quotes/brackets literally; only leading indicators select YAML structure. Tags,
* aliases, anchors, block/flow collections and multiline scalars are deliberately unsupported.
*/
function trimAsciiWhitespace(value) {
return value.replace(/^[ \t]+|[ \t]+$/g, '')
}
function parsedScalar(value, quoted) {
return FORBIDDEN_SCALAR_CONTROL.test(value)
? { value: null, error: 'forbidden control character', quoted }
: { value, error: null, quoted }
}
function isYamlImplicitNonStringScalar(value) {
return (
YAML_NULL_SCALAR.test(value) ||
YAML_BOOLEAN_SCALAR.test(value) ||
YAML_INTEGER_SCALAR.test(value) ||
YAML_FLOAT_SCALAR.test(value) ||
YAML_NON_FINITE_FLOAT_SCALAR.test(value) ||
YAML_DATE_SCALAR.test(value) ||
YAML_TIMESTAMP_SCALAR.test(value)
)
}
function parseFrontMatterScalar(rawValue) {
const text = trimAsciiWhitespace(rawValue)
if (text === '') {
return { value: null, error: 'empty unquoted scalar', quoted: false }
}
if (FORBIDDEN_SCALAR_CONTROL.test(text)) {
return { value: null, error: 'forbidden control character', quoted: false }
}
if (text.startsWith('"')) {
const quoted = text.match(/^("(?:[^"\\]|\\.)*")(?:[ \t]+#.*)?$/)
if (!quoted) {
return { value: null, error: 'unbalanced quote or trailing content', quoted: true }
}
try {
return parsedScalar(JSON.parse(quoted[1]), true)
} catch {
return { value: null, error: 'unsupported double-quoted escape or control character', quoted: true }
}
}
if (text.startsWith("'")) {
const quoted = text.match(/^'((?:[^']|'')*)'(?:[ \t]+#.*)?$/)
return quoted
? parsedScalar(quoted[1].replaceAll("''", "'"), true)
: { value: null, error: 'unbalanced quote or trailing content', quoted: true }
}
const value = text.replace(/[ \t]+#.*$/, '')
if (/^[\[{]/.test(value)) {
const closer = value[0] === '[' ? ']' : '}'
return {
value: null,
error: value.endsWith(closer)
? 'unsupported flow sequence or mapping'
: 'unterminated flow sequence or mapping',
quoted: false,
}
}
if (/^[!&*|>@`%}\],#]/.test(value) || /^[-?:](?:[ \t]|$)/.test(value)) {
return { value: null, error: 'unsupported leading scalar indicator', quoted: false }
}
if (/:(?:[ \t]|$)/.test(value)) {
return { value: null, error: 'unsupported nested mapping', quoted: false }
}
return parsedScalar(value, false)
}
/**
* Validate the WHOLE front matter block, not just `paths:`. An invalid or unsupported line anywhere
* must fail closed rather than letting the mirror check certify a rule its loader might reject.
*
* Accepted: top-level keys with a separated single-line scalar, or one flat indented scalar list;
* comments and blank lines. Each list chooses its own indentation, but every sibling must match it.
* This dependency-free check intentionally does not implement the complete YAML grammar.
*/
function validateFrontMatterStructure(lines, rulePath) {
const structureErrors = []
const seenKeys = new Set()
let blockKey = null
let blockIndent = null
let reportedOrphanEntry = false
for (const line of lines) {
if (trimAsciiWhitespace(line) === '') {
continue
}
if (/^[ \t]*\t/.test(line)) {
structureErrors.push(`${rulePath} front matter has tab indentation, which this check cannot parse: ${line.trim()}`)
continue
}
if (/^ *#/.test(line)) {
continue
}
if (/^ /.test(line)) {
const entry = line.match(/^( +)-(?:[ \t]+(.*))?$/)
if (!entry) {
structureErrors.push(`${rulePath} front matter has a line this check cannot parse: ${line.trim()}`)
continue
}
if (blockKey === null) {
if (!reportedOrphanEntry) {
reportedOrphanEntry = true
structureErrors.push(
`${rulePath} front matter has a list entry with no preceding key, which this check cannot parse: ${line.trim()} (further orphaned entries not listed)`,
)
}
continue
}
blockIndent ??= entry[1].length
if (entry[1].length !== blockIndent) {
structureErrors.push(`${rulePath} front matter has nested or inconsistent list indentation, which this check cannot parse: ${line.trim()}`)
continue
}
// Empty quoted metadata strings are valid; only the paths consumer requires nonempty values.
const { error } = parseFrontMatterScalar(entry[2] ?? '')
if (error !== null) {
structureErrors.push(`${rulePath} front matter has a list entry this check cannot parse (${error}): ${line.trim()}`)
}
continue
}
if (/^-(?:[ \t]|$)/.test(line)) {
structureErrors.push(
`${rulePath} front matter has a list entry at column 0 that this check cannot parse (entries must be indented): ${line.trim()}`,
)
continue
}
// A colon without separation starts plain scalar text, not a YAML mapping value.
const keyMatch = line.match(/^([A-Za-z0-9_][A-Za-z0-9_.-]*) *:(?:[ \t]+(.*))?$/)
if (!keyMatch) {
structureErrors.push(`${rulePath} front matter has a line this check cannot parse: ${line.trim()}`)
blockKey = null
blockIndent = null
continue
}
const [, key, rawValue = ''] = keyMatch
if (seenKeys.has(key)) {
structureErrors.push(
`${rulePath} front matter declares the key "${key}" twice; duplicate mapping keys are not supported`,
)
}
seenKeys.add(key)
blockIndent = null
reportedOrphanEntry = false
const value = trimAsciiWhitespace(rawValue)
if (value === '' || value.startsWith('#')) {
blockKey = key
continue
}
const { error } = parseFrontMatterScalar(value)
if (key === 'paths' && error === 'unsupported flow sequence or mapping') {
structureErrors.push(`${rulePath} front matter paths: must be a block sequence of "- glob" entries`)
} else if (error !== null) {
structureErrors.push(
`${rulePath} front matter has an ${error} on key "${key}", which this check cannot parse: ${line.trim()}`,
)
}
blockKey = null
}
return structureErrors
}
/**
* Parse the `paths:` block sequence out of an agent-rule file's YAML front matter.
*
* Fails closed on every shape it does not fully understand, and validates the complete front matter
* block first. Claude Code drops a rule file whose front matter does not parse, silently and with no
* error anywhere, so "cannot parse" has to mean "red check", never "no paths found".
*/
export function parseRuleFrontMatterPaths(ruleText, rulePath = CI_CONTROL_RULE_PATH) {
const frontMatterMatch = ruleText.match(/^---\r?\n([\s\S]*?)\r?\n---(\r?\n|$)/)
if (!frontMatterMatch) {
return {
paths: [],
errors: [`${rulePath} has no parseable YAML front matter (an unparseable rule file loads for nothing)`],
}
}
const lines = frontMatterMatch[1].split(/\r?\n/)
const structureErrors = validateFrontMatterStructure(lines, rulePath)
if (structureErrors.length > 0) {
return { paths: [], errors: structureErrors }
}
const keyIndex = lines.findIndex((line) => /^paths[ \t]*:/.test(line))
if (keyIndex === -1) {
return { paths: [], errors: [`${rulePath} front matter has no paths: key`] }
}
if (!/^paths[ \t]*:[ \t]*(#.*)?$/.test(lines[keyIndex])) {
return {
paths: [],
errors: [`${rulePath} front matter paths: must be a block sequence of "- glob" entries`],
}
}
const paths = []
const errors = []
for (let index = keyIndex + 1; index < lines.length; index += 1) {
const line = lines[index]
if (trimAsciiWhitespace(line) === '' || /^[ \t]*#/.test(line)) {
continue
}
if (!/^[ \t]/.test(line)) {
break
}
const itemMatch = line.match(/^ +-[ \t]+(.*?)[ \t]*$/)
if (!itemMatch) {
errors.push(`${rulePath} front matter paths: has an entry this check cannot parse: ${line.trim()}`)
continue
}
const { value, quoted } = parseFrontMatterScalar(itemMatch[1])
if (
value === null ||
value === '' ||
/^\s|\s$/u.test(value) ||
FORBIDDEN_SCALAR_CONTROL.test(value) ||
(!quoted && isYamlImplicitNonStringScalar(value))
) {
errors.push(`${rulePath} front matter paths: has an entry this check cannot parse: ${line.trim()}`)
continue
}
paths.push(value)
}
if (errors.length === 0 && paths.length === 0) {
errors.push(`${rulePath} front matter paths: declares no indented entries`)
}
return { paths, errors }
}
/**
* `ci/policy.v1.json` controlPaths is the authority; the rule file's `paths:` front matter is a
* mirror of it, and must be a superset (extras such as `.github/**` are deliberate, see the rule).
*/
export function collectControlPathMirrorErrors(
policyText,
ruleText,
{ policyPath = CI_POLICY_PATH, rulePath = CI_CONTROL_RULE_PATH } = {},
) {
const policyResult = parsePolicyControlPaths(policyText, policyPath)
const ruleResult = parseRuleFrontMatterPaths(ruleText, rulePath)
const mirrorErrors = [...policyResult.errors, ...ruleResult.errors]
if (mirrorErrors.length > 0) {
return mirrorErrors
}
const declared = new Set(ruleResult.paths)
const missing = policyResult.controlPaths.filter((controlPath) => !declared.has(controlPath))
if (missing.length > 0) {
mirrorErrors.push(
`${rulePath} front matter paths: is missing ${missing.length} control path(s) declared in ` +
`${policyPath} controlPaths: ${missing.join(', ')} ` +
`(add them to the rule in the same PR, or the rule stops loading for those paths)`,
)
}
return mirrorErrors
}
async function validateControlPathMirror() {
for (const path of [CI_POLICY_PATH, CI_CONTROL_RULE_PATH]) {
if (!(await fileExists(path))) {
errors.push(`Missing required control-path mirror input: ${path}`)
return
}
}
const [policyText, ruleText] = await Promise.all([
readFile(resolve(CI_POLICY_PATH), 'utf8'),
readFile(resolve(CI_CONTROL_RULE_PATH), 'utf8'),
])
errors.push(...collectControlPathMirrorErrors(policyText, ruleText))
}
async function main() {
for (const path of requiredDocs) {
if (!(await fileExists(path))) {
errors.push(`Missing required active document: ${path}`)
}
}
if (!(await fileExists('docs/INDEX.md'))) {
errors.push('Missing required docs index: docs/INDEX.md')
} else {
const indexText = await readFile(resolve('docs/INDEX.md'), 'utf8')
for (const path of requiredDocs) {
expectContains(indexText, path.replace('docs/', ''), 'docs/INDEX.md')
}
const hasArchiveLink = indexText.includes('archive/') || indexText.includes('docs/archive/')
if (!hasArchiveLink) {
errors.push('docs/INDEX.md must reference the archive directory')
}
}
const docsRequiringLastUpdated = [
'docs/STATUS.md',
'docs/GOLDEN_PRINCIPLES.md',
]
for (const path of docsRequiringLastUpdated) {
if (!(await fileExists(path))) {
continue
}
const text = await readFile(resolve(path), 'utf8')
const hasLastUpdatedLine = /^Last Updated:\s*\d{4}-\d{2}-\d{2}\s*$/m.test(text)
if (!hasLastUpdatedLine) {
errors.push(`${path} must contain a "Last Updated: YYYY-MM-DD" line`)
}
}
await validateControlPathMirror()
if (errors.length > 0) {
console.error('Docs governance check failed:')
for (const error of errors) {
console.error(`- ${error}`)
}
process.exit(1)
}
console.log('Docs governance check passed.')
}
if (process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.url)) {
main().catch((error) => {
console.error('Docs governance check crashed:', error)
process.exit(1)
})
}Exact test results
Summary
|
Summary
Testing
Complete Final File Contents
|
|
@codex please run the exact focused suite on test-only head
Report the exact failing assertions and counts. This commit intentionally adds resolver-exact RED cases before the implementation repair. Verify |
Focused Suite ReportVerified Results
Exact Failing Assertions
Repository Integrity Checks
Outstanding Human Actions
|
|
@codex please verify implementation head
Report exact test totals and command outputs. Do not commit or push. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dae421b1eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex please verify exact head
Report exact test counts and command outputs. Pay particular attention to rejection of all four lone surrogate boundary cases and acceptance of valid non-BMP surrogate pairs. Do not commit or push. |
Verification ReportVerified the requested exact head before and after execution: The working tree was clean both before and after. No files were modified, committed, or pushed, and no pull request was created. Command Results
OutcomeAll 91 of 91 tests passed. The focused coverage confirms rejection of all four lone-surrogate boundary cases while preserving valid non-BMP surrogate pairs. Every requested syntax, live-governance, documentation-link, and diff check also passed. Outstanding Human Actions
|
|
@codex review Please review exact documentation-final head |
|
@codex please verify exact final head
Report exact totals, outputs, and final changed-file inventory. Do not commit or push. |
Verification SummaryThe requested final head was confirmed unchanged before and after verification:
The focused suite confirms the documented control-character, surrogate, YAML resolver, quoted-scalar, and ordinary-string boundaries. scripts/check-docs-governance.hardening.test.mjsL15-L74 scripts/check-docs-governance.hardening.test.mjsL76-L150 The evidence note’s recorded 91/91 result agrees with this final-head run. docs/analysis/2026-09-12-frontmatter-scalar-validation.mdL39-L55 Exact Command Results
Final Changed-File Inventory
Totals: 4 changed files — 2 added, 2 modified, 0 deleted. Outstanding Human Actions
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2657c47458
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| export const CI_POLICY_PATH = 'ci/policy.v1.json' | ||
| export const CI_CONTROL_RULE_PATH = '.claude/rules/ci-control.md' | ||
| const FORBIDDEN_SCALAR_CONTROL = /[\u0000-\u001F\u007F-\u009F]/u |
There was a problem hiding this comment.
Reject non-printable YAML sentinel code points
When both controlPaths and a quoted rule path contain a literal U+FFFE or U+FFFF, collectControlPathMirrorErrors returns no errors because this character boundary omits both code points, while YAML scanners reject either character as non-printable and can therefore drop the rule frontmatter. Fresh exact-head reproduction shows this remains distinct from the resolved unpaired-surrogate case; include U+FFFE/U+FFFF in decoded-character validation and its boundary tests.
Useful? React with 👍 / 👎.
| if (/^ *#/.test(line)) { | ||
| continue |
Outcome
Replaces the contaminated history in #3067 with a bounded current-main change and closes two confirmed control-path mirror gaps:
Whole-scalar matching avoids false positives for ordinary paths such as
2026-09-17-notes.md,true/guide.md, and123/notes.md. The earlier scalar-boundary work remains intact: complete quoted-scalar parsing, an explicit supported-YAML subset, malformed/trailing-content rejection, quoted empty metadata handling, Unicode edge-whitespace rejection, and fail-closed policy/rule mirroring.Clean-history recovery and audit correction
The source PR has 138 commits and 117 changed files despite its intended validator-only scope. This branch started directly from current main
095e277f4825bfc6526d986a627d7eccc3e52511.A second audit found that the source branch did not actually contain the final control-range and implicit-scalar changes claimed by its automation summary. Those claims are not inherited here. The missing behavior was reproduced with focused regressions and implemented directly on this clean branch.
Final scope is four files:
scripts/check-docs-governance.mjsscripts/check-docs-governance.test.mjsscripts/check-docs-governance.hardening.test.mjsdocs/analysis/2026-09-12-frontmatter-scalar-validation.mdThe two original script blobs on current main were byte-identical to their versions at #3067's original base
54e4c0a86fb77eabba73b5d21557d6f8720571bd, so the recovery did not overwrite intervening work.Red/green verification
Node v22.16.0 focused evidence:
~was accepted as a string path;node --check scripts/check-docs-governance.mjspassed;844a91393eb94ac2565ab816305a6a774d63369ematches the locally verified file byte-for-byte.The hosted Docs Governance job executes the checker against the repository policy/rule pair but does not yet invoke the Node regression suite. #3005 separately owns that workflow wiring. This PR deliberately does not edit a control-plane workflow and does not present hosted script execution as equivalent to regression-suite execution.
Exact-head hosted CI and a fresh code review are required on
3562268ccbce594bab1fbcd9ed5acb0680a9d6f6before merge.Supersedes #3067 after this clean replacement is verified. Part of #3006.