From 8335d15b9caff43ce8c0d68e1c1db4c5cabdefd7 Mon Sep 17 00:00:00 2001 From: ArchonVII Date: Sat, 18 Jul 2026 18:36:39 -0500 Subject: [PATCH 1/2] fix(gate): accept plural required check maps (#116) --- .../unreleased/116-plural-required-gates.md | 3 + .github/workflows/repo-required-gate.yml | 13 +- docs/repo-update-log.md | 9 + scripts/validate-check-map.mjs | 439 ++++++++++++++++++ scripts/validate-check-map.test.mjs | 348 ++++++++++++++ scripts/workflow-structure.test.mjs | 20 + 6 files changed, 828 insertions(+), 4 deletions(-) create mode 100644 .changelog/unreleased/116-plural-required-gates.md create mode 100644 scripts/validate-check-map.mjs create mode 100644 scripts/validate-check-map.test.mjs diff --git a/.changelog/unreleased/116-plural-required-gates.md b/.changelog/unreleased/116-plural-required-gates.md new file mode 100644 index 0000000..5b6e3de --- /dev/null +++ b/.changelog/unreleased/116-plural-required-gates.md @@ -0,0 +1,3 @@ +### Fixed + +- Accept canonical plural `required_gates` check maps in the reusable policy-validation lane while preserving legacy singular `required_gate` compatibility. Every declared gate must contain a unique, non-empty string `check_name`, and ambiguous, empty, duplicate, incorrectly indented, or malformed declarations fail closed. diff --git a/.github/workflows/repo-required-gate.yml b/.github/workflows/repo-required-gate.yml index e6ab378..c8642f2 100644 --- a/.github/workflows/repo-required-gate.yml +++ b/.github/workflows/repo-required-gate.yml @@ -621,14 +621,19 @@ jobs: permissions: contents: read steps: - - uses: actions/checkout@v7 + - name: Check out consumer repository + uses: actions/checkout@v7 + - name: Check out github-workflows for check-map validator + uses: actions/checkout@v7 + with: + repository: ArchonVII/github-workflows + ref: ${{ inputs.workflow-library-ref }} + path: __github-workflows__ - name: Validate check map shell: bash run: | if [ -f .agent/check-map.yml ]; then - grep -Eq '^version: [0-9]+' .agent/check-map.yml - grep -Eq '^required_gate:' .agent/check-map.yml - grep -Eq '^[[:space:]]+check_name: repo-required-gate / decision' .agent/check-map.yml + node __github-workflows__/scripts/validate-check-map.mjs .agent/check-map.yml fi dependency-review: diff --git a/docs/repo-update-log.md b/docs/repo-update-log.md index 940897c..0f7a249 100644 --- a/docs/repo-update-log.md +++ b/docs/repo-update-log.md @@ -15,6 +15,15 @@ This log records agent-visible repository changes that should be easy to audit l - **Propagation:** none | pending | completed ``` +## 2026-07-18 - Plural required-gate check-map validation + +- **Issue/PR:** #116 / (pending) +- **Branch:** agent/codex/116-plural-required-gates +- **Changed paths:** .github/workflows/repo-required-gate.yml, scripts/validate-check-map.mjs, scripts/validate-check-map.test.mjs, scripts/workflow-structure.test.mjs, .changelog/unreleased/116-plural-required-gates.md, docs/repo-update-log.md +- **What changed:** Replaced the policy lane's singular-only grep with a zero-dependency check-map validator loaded from the provider checkout at `inputs.workflow-library-ref`. The validator accepts canonical plural gate lists and the legacy singular mapping, allows custom check names, and fails closed on missing, empty, ambiguous, duplicate, incorrectly indented, non-string, or malformed declarations. +- **Verification:** Initial TDD RED: `npx vitest run scripts/validate-check-map.test.mjs scripts/workflow-structure.test.mjs` reported one failed suite for the missing helper and one failed structural assertion (23 tests passed). Review-repair RED runs then reported 13 failed / 47 passed for separator, indentation, duplicate-name, and non-string scalar gaps, followed by 16 failed / 66 passed for strict scalar/list subset parity. Final GREEN: the focused command reported 2 files / 82 tests passed. Full `npm test` reported 10 files / 240 tests passed. `node --check` on all touched `.mjs` files, `node scripts/validate-check-map.mjs .agent/check-map.yml`, `C:\Users\josep\go\bin\actionlint.exe .github\workflows\repo-required-gate.yml`, and `git diff --check` exited 0; the diff check emitted only working-tree LF-to-CRLF warnings. +- **Propagation:** pending owner-gated `v1` tag movement after merge; consumers pinned to `@v1` receive the validator on their next required-gate run. + ## 2026-07-09 - Anomaly-triage documented metadata parsing - **Issue/PR:** #110 / #111 diff --git a/scripts/validate-check-map.mjs b/scripts/validate-check-map.mjs new file mode 100644 index 0000000..057e3ba --- /dev/null +++ b/scripts/validate-check-map.mjs @@ -0,0 +1,439 @@ +#!/usr/bin/env node + +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { pathToFileURL } from 'node:url'; + +// YAML block mapping separators require whitespace or end-of-line after `:`. +// Keep the accepted surface deliberately small instead of approximating YAML +// with a pattern that also accepts `key:value` as a mapping. +const TOP_LEVEL_KEY = /^([A-Za-z_][A-Za-z0-9_.-]*) *:(?: +(.*))?$/; +const MAPPING_KEY = /^([A-Za-z_][A-Za-z0-9_.-]*) *:(?: +(.*))?$/; +const YAML_NON_STRING_SCALAR = /^(?:null|~|true|false)$/i; +const YAML_NUMBER_SCALAR = /^[+-]?(?:[0-9][0-9_]*(?:\.[0-9_]*)?(?:e[+-]?[0-9_]+)?|0b[01_]+|0o[0-7_]+|0x[0-9a-f_]+|\.[0-9_]+(?:e[+-]?[0-9_]+)?|\.(?:inf|nan))$/i; +const YAML_PLAIN_CONTROL = /[\u0000-\u001f\u007f-\u009f]/; +const YAML_PLAIN_FORBIDDEN_LEADING = /^(?:[-?:](?:\s|$)|[\[\]{},#&*!|>'"%@`])/u; + +const indentation = (line) => line.match(/^ */)[0].length; +const isIgnoredLine = (line) => { + const trimmed = line.trim(); + return trimmed === '' || trimmed.startsWith('#'); +}; + +const stripInlineComment = (value) => { + let quote = null; + let escaped = false; + + for (let index = 0; index < value.length; index += 1) { + const character = value[index]; + + if (quote === '"') { + if (escaped) { + escaped = false; + } else if (character === '\\') { + escaped = true; + } else if (character === quote) { + quote = null; + } + continue; + } + + if (quote === "'") { + if (character === quote) { + if (value[index + 1] === quote) { + index += 1; + } else { + quote = null; + } + } + continue; + } + + if (character === '"' || character === "'") { + quote = character; + continue; + } + + if (character === '#' && (index === 0 || /\s/.test(value[index - 1]))) { + return { value: value.slice(0, index).trim(), unterminatedQuote: false }; + } + } + + return { value: value.trim(), unterminatedQuote: quote !== null }; +}; + +const parseQuotedString = (value, quote, location, fieldName, errors) => { + let decoded = ''; + for (let index = 1; index < value.length; index += 1) { + const character = value[index]; + if (/[\u0000-\u001f\u007f]/.test(character)) { + errors.push(`${location} ${fieldName} contains a control character.`); + return null; + } + + if (quote === "'" && character === "'" && value[index + 1] === "'") { + decoded += "'"; + index += 1; + continue; + } + + if (quote === '"' && character === '\\') { + const escaped = value[index + 1]; + if (escaped !== '"' && escaped !== '\\') { + errors.push(`${location} ${fieldName} uses an unsupported quoted escape.`); + return null; + } + decoded += escaped; + index += 1; + continue; + } + + if (character === quote) { + if (index !== value.length - 1) { + errors.push(`${location} has a malformed quoted ${fieldName}.`); + return null; + } + if (decoded.trim() === '') { + errors.push(`${location} must have a non-empty ${fieldName}.`); + return null; + } + return decoded; + } + decoded += character; + } + + errors.push(`${location} has a malformed quoted ${fieldName}.`); + return null; +}; + +const parseStringScalar = (rawValue, location, fieldName, errors) => { + const stripped = stripInlineComment(rawValue); + if (stripped.unterminatedQuote) { + errors.push(`${location} has an unterminated quoted ${fieldName}.`); + return null; + } + + const value = stripped.value; + if (value === '') { + errors.push(`${location} must have a non-empty ${fieldName}.`); + return null; + } + + if (value.startsWith("'") || value.startsWith('"')) { + return parseQuotedString(value, value[0], location, fieldName, errors); + } + + if (YAML_PLAIN_CONTROL.test(value)) { + errors.push(`${location} ${fieldName} contains a control character.`); + return null; + } + if (YAML_NON_STRING_SCALAR.test(value) || YAML_NUMBER_SCALAR.test(value)) { + errors.push(`${location} ${fieldName} must be a string; quote YAML boolean and numeric values.`); + return null; + } + if (YAML_PLAIN_FORBIDDEN_LEADING.test(value) || /:(?:\s|$)/.test(value)) { + errors.push(`${location} ${fieldName} uses unsupported plain YAML syntax; quote the string.`); + return null; + } + + return value; +}; + +const parseMappingPair = (text, location, errors) => { + const match = text.match(MAPPING_KEY); + if (!match) { + errors.push(`${location} must be a YAML mapping field.`); + return null; + } + return { key: match[1], rawValue: match[2] ?? '' }; +}; + +const addField = (fields, pair, location, errors) => { + if (!pair) return; + if (fields.has(pair.key)) { + errors.push(`${location} has duplicate ${pair.key} fields.`); + return; + } + fields.set(pair.key, pair.rawValue); +}; + +const blockLines = (section) => section.body.filter(({ raw }) => !isIgnoredLine(raw)); + +const validateSingularGate = (section, errors) => { + const inline = stripInlineComment(section.rawValue); + if (inline.unterminatedQuote || inline.value !== '') { + errors.push(`required_gate at line ${section.line} must be a block mapping.`); + return []; + } + + const content = blockLines(section); + if (content.length === 0) { + errors.push(`required_gate at line ${section.line} must not be empty.`); + return []; + } + + const directIndent = Math.min(...content.map(({ raw }) => indentation(raw))); + const fields = new Map(); + for (const line of content) { + if (indentation(line.raw) !== directIndent) { + errors.push(`required_gate line ${line.line} has unexpected nested indentation.`); + continue; + } + addField( + fields, + parseMappingPair(line.raw.slice(directIndent), `required_gate line ${line.line}`, errors), + `required_gate at line ${section.line}`, + errors, + ); + } + + if (!fields.has('check_name')) { + errors.push(`required_gate at line ${section.line} requires a direct check_name field.`); + return []; + } + + const parsedFields = new Map(); + for (const [fieldName, rawValue] of fields) { + const value = parseStringScalar( + rawValue, + `required_gate at line ${section.line}`, + fieldName, + errors, + ); + if (value !== null) parsedFields.set(fieldName, value); + } + const name = parsedFields.get('check_name') ?? null; + return name === null ? [] : [name]; +}; + +const parsePluralEntry = (entry, listIndent, errors) => { + const location = `required_gates entry at line ${entry.line}`; + const fields = new Map(); + const inline = stripInlineComment(entry.inline); + + if (inline.unterminatedQuote) { + errors.push(`${location} has an unterminated quote.`); + } else if (inline.value !== '') { + addField(fields, parseMappingPair(inline.value, location, errors), location, errors); + } + + let directIndent = entry.inline.trim() === '' || entry.inline.trim().startsWith('#') + ? null + : listIndent + 1 + entry.separator.length; + for (const line of entry.body) { + if (isIgnoredLine(line.raw)) continue; + directIndent ??= indentation(line.raw); + if (indentation(line.raw) !== directIndent) { + errors.push(`${location}, line ${line.line} has unexpected nested indentation.`); + continue; + } + addField( + fields, + parseMappingPair(line.raw.slice(directIndent), `${location}, line ${line.line}`, errors), + location, + errors, + ); + } + + if (fields.size === 0) { + errors.push(`${location} must not be empty and must be a mapping.`); + return null; + } + if (!fields.has('check_name')) { + errors.push(`${location} requires a direct check_name field.`); + return null; + } + + const parsedFields = new Map(); + for (const [fieldName, rawValue] of fields) { + const value = parseStringScalar(rawValue, location, fieldName, errors); + if (value !== null) parsedFields.set(fieldName, value); + } + return parsedFields.get('check_name') ?? null; +}; + +const validatePluralGates = (section, errors) => { + const inline = stripInlineComment(section.rawValue); + if (inline.unterminatedQuote) { + errors.push(`required_gates at line ${section.line} has an unterminated quote.`); + return []; + } + if (inline.value === '[]') { + errors.push(`required_gates at line ${section.line} must contain at least one entry; the list is empty.`); + return []; + } + if (inline.value !== '') { + errors.push(`required_gates at line ${section.line} must be a block list.`); + return []; + } + + const content = blockLines(section); + if (content.length === 0) { + errors.push(`required_gates at line ${section.line} must contain at least one entry; the list is empty.`); + return []; + } + + const listIndent = Math.min(...content.map(({ raw }) => indentation(raw))); + const entries = []; + let current = null; + + for (const line of content) { + const indent = indentation(line.raw); + if (indent === listIndent) { + const match = line.raw.slice(listIndent).match(/^-([ ]*)(.*)$/); + if (!match) { + errors.push(`required_gates line ${line.line} must start a list entry with '-'.`); + current = null; + continue; + } + if (match[1] === '' && match[2] !== '') { + errors.push(`required_gates line ${line.line} requires whitespace after '-'.`); + current = null; + continue; + } + current = { line: line.line, separator: match[1], inline: match[2], body: [] }; + entries.push(current); + continue; + } + + if (!current) { + errors.push(`required_gates line ${line.line} appears before a valid list entry.`); + continue; + } + current.body.push(line); + } + + if (entries.length === 0) { + errors.push(`required_gates at line ${section.line} must contain at least one list entry.`); + return []; + } + + const names = []; + const seenNames = new Set(); + for (const entry of entries) { + const name = parsePluralEntry(entry, listIndent, errors); + if (name === null) continue; + if (seenNames.has(name)) { + errors.push(`required_gates has duplicate check_name ${JSON.stringify(name)}.`); + continue; + } + seenNames.add(name); + names.push(name); + } + return names; +}; + +export const validateCheckMap = (source) => { + const errors = []; + if (typeof source !== 'string') { + return { + ok: false, + errors: ['check map contents must be a string.'], + version: null, + requiredCheckNames: [], + }; + } + + const lines = source.replace(/^\uFEFF/, '').split(/\r?\n/).map((raw, index) => ({ + raw, + line: index + 1, + })); + + for (const line of lines) { + if (line.raw.includes('\t')) { + errors.push(`line ${line.line} contains a tab; check-map indentation must use spaces.`); + } + } + + const sections = new Map(); + let current = null; + for (const line of lines) { + if (isIgnoredLine(line.raw)) continue; + + if (indentation(line.raw) === 0) { + const match = line.raw.match(TOP_LEVEL_KEY); + if (!match) { + errors.push(`line ${line.line} is not a valid top-level declaration.`); + current = null; + continue; + } + + const section = { key: match[1], rawValue: match[2] ?? '', line: line.line, body: [] }; + if (sections.has(section.key)) { + errors.push(`duplicate top-level ${section.key} declaration at line ${line.line}.`); + } else { + sections.set(section.key, section); + } + current = section; + continue; + } + + if (!current) { + errors.push(`line ${line.line} is indented without a top-level declaration.`); + continue; + } + current.body.push(line); + } + + let version = null; + const versionSection = sections.get('version'); + if (!versionSection) { + errors.push('check map requires a top-level version declaration.'); + } else { + const parsed = stripInlineComment(versionSection.rawValue); + if (parsed.unterminatedQuote || !/^\d+$/.test(parsed.value)) { + errors.push(`version at line ${versionSection.line} must be a non-negative integer.`); + } else if (blockLines(versionSection).length > 0) { + errors.push(`version at line ${versionSection.line} must be a scalar integer.`); + } else { + version = Number(parsed.value); + } + } + + const singular = sections.get('required_gate'); + const plural = sections.get('required_gates'); + let requiredCheckNames = []; + if (singular && plural) { + errors.push('check map cannot declare both required_gate and required_gates.'); + } else if (plural) { + requiredCheckNames = validatePluralGates(plural, errors); + } else if (singular) { + requiredCheckNames = validateSingularGate(singular, errors); + } else { + errors.push('check map requires a top-level required_gate or required_gates declaration.'); + } + + return { + ok: errors.length === 0, + errors, + version, + requiredCheckNames, + }; +}; + +const runCli = () => { + const checkMapPath = resolve(process.argv[2] || '.agent/check-map.yml'); + let source; + try { + source = readFileSync(checkMapPath, 'utf8'); + } catch (error) { + console.error(`check-map validation failed: could not read ${checkMapPath}: ${error.message}`); + process.exitCode = 1; + return; + } + + const result = validateCheckMap(source); + if (!result.ok) { + console.error(`check-map validation failed for ${checkMapPath}:`); + for (const error of result.errors) console.error(`- ${error}`); + process.exitCode = 1; + return; + } + + console.log( + `check-map validation passed: version ${result.version}; required checks: ${result.requiredCheckNames.join(', ')}`, + ); +}; + +const invokedPath = process.argv[1] ? pathToFileURL(resolve(process.argv[1])).href : ''; +if (invokedPath === import.meta.url) runCli(); diff --git a/scripts/validate-check-map.test.mjs b/scripts/validate-check-map.test.mjs new file mode 100644 index 0000000..de6a5d4 --- /dev/null +++ b/scripts/validate-check-map.test.mjs @@ -0,0 +1,348 @@ +import { describe, expect, it } from 'vitest'; + +import { validateCheckMap } from './validate-check-map.mjs'; + +const expectValid = (source, expectedNames) => { + const result = validateCheckMap(source); + + expect(result.errors).toEqual([]); + expect(result.ok).toBe(true); + expect(result.requiredCheckNames).toEqual(expectedNames); +}; + +const expectInvalid = (source, errorPattern) => { + const result = validateCheckMap(source); + + expect(result.ok).toBe(false); + expect(result.errors.join('\n')).toMatch(errorPattern); +}; + +describe('check-map required-gate validation', () => { + it('accepts a canonical plural list with multiple custom check names', () => { + expectValid( + `version: 2 # schema version + +required_gates: + - check_name: "repo-required-gate / decision" # stable gate + workflow: .github/workflows/repo-required-gate.yml + - check_name: custom security / decision + workflow: .github/workflows/security.yml + +paths: + policy: + requires: [policy-validation] +`, + ['repo-required-gate / decision', 'custom security / decision'], + ); + }); + + it('keeps the legacy singular mapping compatible', () => { + expectValid( + `version: 1 +required_gate: + check_name: 'legacy # decision' + workflow: .github/workflows/repo-required-gate.yml +`, + ['legacy # decision'], + ); + }); + + it('accepts quoted names that would otherwise be YAML booleans or numbers', () => { + expectValid( + `version: 1 +required_gates: + - check_name: "true" + - check_name: '123' +`, + ['true', '123'], + ); + }); + + it('accepts a plural mapping whose item marker is on its own line', () => { + expectValid( + `version: 1 +required_gates: + - + check_name: standalone dash / decision +`, + ['standalone dash / decision'], + ); + }); + + it.each([ + ['missing', 'required_gate:\n check_name: gate / decision\n'], + ['empty', 'version:\nrequired_gate:\n check_name: gate / decision\n'], + ['non-integer', 'version: latest\nrequired_gate:\n check_name: gate / decision\n'], + ['duplicate', 'version: 1\nversion: 2\nrequired_gate:\n check_name: gate / decision\n'], + ])('rejects a %s version declaration', (_label, source) => { + expectInvalid(source, /version/i); + }); + + it('rejects a missing required-gate declaration', () => { + expectInvalid('version: 1\npaths: {}\n', /required_gate|required_gates/i); + }); + + it('rejects a version mapping separator without required whitespace', () => { + expectInvalid( + `version:1 +required_gate: + check_name: gate / decision +`, + /top-level|version/i, + ); + }); + + it.each([ + [ + 'singular', + `version: 1 +required_gate: + check_name:gate / decision +`, + ], + [ + 'plural', + `version: 1 +required_gates: + - check_name:gate / decision +`, + ], + ])('rejects a %s check_name separator without required whitespace', (_shape, source) => { + expectInvalid(source, /mapping|check_name/i); + }); + + it('rejects ambiguous singular and plural declarations', () => { + expectInvalid( + `version: 1 +required_gate: + check_name: legacy / decision +required_gates: + - check_name: canonical / decision +`, + /both|required_gate.*required_gates/i, + ); + }); + + it.each([ + ['inline empty list', 'version: 1\nrequired_gates: []\n'], + ['empty block', 'version: 1\nrequired_gates:\npaths: {}\n'], + ])('rejects an %s plural declaration', (_label, source) => { + expectInvalid(source, /required_gates.*empty|at least one/i); + }); + + it.each([ + [ + 'entry without check_name', + `version: 1 +required_gates: + - workflow: .github/workflows/repo-required-gate.yml +`, + ], + [ + 'entry with an empty check_name', + `version: 1 +required_gates: + - check_name: " " +`, + ], + [ + 'empty list item', + `version: 1 +required_gates: + - +`, + ], + [ + 'scalar list item', + `version: 1 +required_gates: + - custom gate / decision +`, + ], + ])('rejects a plural %s', (_label, source) => { + expectInvalid(source, /required_gates|check_name|mapping|empty/i); + }); + + it('does not let a check_name outside the gate declaration mask a malformed entry', () => { + expectInvalid( + `version: 1 +required_gates: + - workflow: .github/workflows/repo-required-gate.yml +metadata: + check_name: valid but unrelated / decision +`, + /check_name/i, + ); + }); + + it('does not let a nested check_name mask a missing direct entry field', () => { + expectInvalid( + `version: 1 +required_gates: + - metadata: + check_name: nested / decision +`, + /check_name/i, + ); + }); + + it.each([ + [ + 'singular', + `version: 1 +required_gate: + check_name: gate / decision + unexpected: nested content +`, + ], + [ + 'plural', + `version: 1 +required_gates: + - check_name: gate / decision + unexpected: nested content +`, + ], + ])('rejects unexpected over-indented content in a %s gate block', (_shape, source) => { + expectInvalid(source, /indent|nested|unexpected/i); + }); + + it('rejects duplicate plural check names after decoding quoted scalars', () => { + expectInvalid( + `version: 1 +required_gates: + - check_name: duplicate / decision + - check_name: "duplicate / decision" +`, + /duplicate.*check_name/i, + ); + }); + + it.each([ + 'true', + 'FALSE', + '123', + '-1.5', + '1.', + '6.02e23', + '01', + '+01', + '01.5', + '0b10', + '0o17', + '0x2A', + '.nan', + ])( + 'rejects the YAML non-string bare scalar %s as a check name', + (value) => { + expectInvalid( + `version: 1 +required_gate: + check_name: ${value} +`, + /string|check_name/i, + ); + }, + ); + + it.each([ + ['colon followed by whitespace', 'gate: broken'], + ['terminal colon', 'gate:'], + ['sequence indicator', '- gate'], + ['mapping-key indicator', '? gate'], + ['mapping-value indicator', ': gate'], + ['flow-sequence close', ']gate'], + ['flow-mapping close', '}gate'], + ['flow separator', ',gate'], + ['directive indicator', '%gate'], + ['reserved at indicator', '@gate'], + ['reserved backtick indicator', '`gate'], + ['control character', 'gate\u0007suffix'], + ])('rejects unsupported plain YAML syntax: %s', (_label, value) => { + expectInvalid( + `version: 1 +required_gate: + check_name: ${value} +`, + /string|check_name|mapping/i, + ); + }); + + it.each(['"line\\nbreak"', '"unicode \\u0041"']) ( + 'rejects unsupported quoted YAML escape syntax in %s', + (value) => { + expectInvalid( + `version: 1 +required_gate: + check_name: ${value} +`, + /quoted|check_name|string/i, + ); + }, + ); + + it.each(['-check_name: hidden', '-# hidden comment']) ( + 'rejects a plural list item without whitespace after the dash: %s', + (item) => { + expectInvalid( + `version: 1 +required_gates: + ${item} + check_name: visible / decision +`, + /list|separator|required_gates/i, + ); + }, + ); + + it.each([ + [ + 'required_gate', + `version: 1 +required_gate: + check_name: first / decision +required_gate: + check_name: second / decision +`, + ], + [ + 'required_gates', + `version: 1 +required_gates: + - check_name: first / decision +required_gates: + - check_name: second / decision +`, + ], + ])('rejects duplicate top-level %s declarations', (_key, source) => { + expectInvalid(source, /duplicate/i); + }); + + it('rejects malformed top-level declarations', () => { + expectInvalid( + `version: 1 +required_gates + - check_name: gate / decision +`, + /top-level|required_gates/i, + ); + }); + + it('rejects tab indentation instead of interpreting a different structure', () => { + expectInvalid( + 'version: 1\nrequired_gates:\n\t- check_name: gate / decision\n', + /tab/i, + ); + }); + + it('rejects duplicate check_name fields within one gate entry', () => { + expectInvalid( + `version: 1 +required_gates: + - check_name: first / decision + check_name: second / decision +`, + /duplicate.*check_name/i, + ); + }); +}); diff --git a/scripts/workflow-structure.test.mjs b/scripts/workflow-structure.test.mjs index fe552bd..7cd2d05 100644 --- a/scripts/workflow-structure.test.mjs +++ b/scripts/workflow-structure.test.mjs @@ -113,6 +113,26 @@ describe('repo-required-gate workflow node delegation', () => { }); }); +describe('repo-required-gate check-map policy validation (#116)', () => { + it('validates the consumer map with the caller-aligned provider helper', () => { + const body = readWorkflow('repo-required-gate'); + const block = workflowJobBlock(body, 'policy-validation'); + + expect(block).toContain('uses: actions/checkout@v7'); + expect(block).toContain('repository: ArchonVII/github-workflows'); + expect(block).toContain('ref: ${{ inputs.workflow-library-ref }}'); + expect(block).toContain('path: __github-workflows__'); + expect(block).toContain( + 'node __github-workflows__/scripts/validate-check-map.mjs .agent/check-map.yml', + ); + expect(block.indexOf('name: Check out consumer repository')).toBeLessThan( + block.indexOf('name: Check out github-workflows for check-map validator'), + ); + expect(block).not.toContain("grep -Eq '^required_gate:'"); + expect(block).not.toContain('check_name: repo-required-gate / decision'); + }); +}); + describe('repo-required-gate docs-gate lane (#104)', () => { it('declares the opt-in docs-system input defaulting to false', () => { const body = readWorkflow('repo-required-gate'); From 197d9c9d9ea696ccd7b433eaaf513a0d2915a102 Mon Sep 17 00:00:00 2001 From: ArchonVII Date: Sat, 18 Jul 2026 19:44:05 -0500 Subject: [PATCH 2/2] fix(gate): accept embedded quotes in plain names (#116) --- .changelog/unreleased/116-plural-required-gates.md | 2 +- docs/repo-update-log.md | 4 ++-- scripts/validate-check-map.mjs | 8 +++++++- scripts/validate-check-map.test.mjs | 13 +++++++++++++ 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/.changelog/unreleased/116-plural-required-gates.md b/.changelog/unreleased/116-plural-required-gates.md index 5b6e3de..fdf40cc 100644 --- a/.changelog/unreleased/116-plural-required-gates.md +++ b/.changelog/unreleased/116-plural-required-gates.md @@ -1,3 +1,3 @@ ### Fixed -- Accept canonical plural `required_gates` check maps in the reusable policy-validation lane while preserving legacy singular `required_gate` compatibility. Every declared gate must contain a unique, non-empty string `check_name`, and ambiguous, empty, duplicate, incorrectly indented, or malformed declarations fail closed. +- Accept canonical plural `required_gates` check maps in the reusable policy-validation lane while preserving legacy singular `required_gate` compatibility. Every declared gate must contain a unique, non-empty string `check_name`; valid plain names may contain embedded quote characters, while ambiguous, empty, duplicate, incorrectly indented, or malformed declarations fail closed. diff --git a/docs/repo-update-log.md b/docs/repo-update-log.md index 0f7a249..d873705 100644 --- a/docs/repo-update-log.md +++ b/docs/repo-update-log.md @@ -20,8 +20,8 @@ This log records agent-visible repository changes that should be easy to audit l - **Issue/PR:** #116 / (pending) - **Branch:** agent/codex/116-plural-required-gates - **Changed paths:** .github/workflows/repo-required-gate.yml, scripts/validate-check-map.mjs, scripts/validate-check-map.test.mjs, scripts/workflow-structure.test.mjs, .changelog/unreleased/116-plural-required-gates.md, docs/repo-update-log.md -- **What changed:** Replaced the policy lane's singular-only grep with a zero-dependency check-map validator loaded from the provider checkout at `inputs.workflow-library-ref`. The validator accepts canonical plural gate lists and the legacy singular mapping, allows custom check names, and fails closed on missing, empty, ambiguous, duplicate, incorrectly indented, non-string, or malformed declarations. -- **Verification:** Initial TDD RED: `npx vitest run scripts/validate-check-map.test.mjs scripts/workflow-structure.test.mjs` reported one failed suite for the missing helper and one failed structural assertion (23 tests passed). Review-repair RED runs then reported 13 failed / 47 passed for separator, indentation, duplicate-name, and non-string scalar gaps, followed by 16 failed / 66 passed for strict scalar/list subset parity. Final GREEN: the focused command reported 2 files / 82 tests passed. Full `npm test` reported 10 files / 240 tests passed. `node --check` on all touched `.mjs` files, `node scripts/validate-check-map.mjs .agent/check-map.yml`, `C:\Users\josep\go\bin\actionlint.exe .github\workflows\repo-required-gate.yml`, and `git diff --check` exited 0; the diff check emitted only working-tree LF-to-CRLF warnings. +- **What changed:** Replaced the policy lane's singular-only grep with a zero-dependency check-map validator loaded from the provider checkout at `inputs.workflow-library-ref`. The validator accepts canonical plural gate lists and the legacy singular mapping, allows custom check names (including valid plain names with embedded quote characters), and fails closed on missing, empty, ambiguous, duplicate, incorrectly indented, non-string, or malformed declarations. +- **Verification:** Initial TDD RED: `npx vitest run scripts/validate-check-map.test.mjs scripts/workflow-structure.test.mjs` reported one failed suite for the missing helper and one failed structural assertion (23 tests passed). Review-repair RED runs then reported 13 failed / 47 passed for separator, indentation, duplicate-name, and non-string scalar gaps, followed by 16 failed / 66 passed for strict scalar/list subset parity. The embedded-quote review regression reported 4 failed / 58 passed before production changes. Final GREEN: the focused command reported 2 files / 86 tests passed. Full `npm test` reported 10 files / 244 tests passed. `node --check` on all touched `.mjs` files, validator CLI runs against this repository's legacy singular map and repo-template PR #187's plural map, `C:\Users\josep\go\bin\actionlint.exe .github\workflows\repo-required-gate.yml`, and `git diff --check` exited 0. - **Propagation:** pending owner-gated `v1` tag movement after merge; consumers pinned to `@v1` receive the validator on their next required-gate run. ## 2026-07-09 - Anomaly-triage documented metadata parsing diff --git a/scripts/validate-check-map.mjs b/scripts/validate-check-map.mjs index 057e3ba..d8c6172 100644 --- a/scripts/validate-check-map.mjs +++ b/scripts/validate-check-map.mjs @@ -20,6 +20,12 @@ const isIgnoredLine = (line) => { return trimmed === '' || trimmed.startsWith('#'); }; +const isQuotedScalarStart = (value, index) => { + const prefix = value.slice(0, index); + return prefix.trim() === '' + || /^\s*[A-Za-z_][A-Za-z0-9_.-]*\s*:\s*$/.test(prefix); +}; + const stripInlineComment = (value) => { let quote = null; let escaped = false; @@ -49,7 +55,7 @@ const stripInlineComment = (value) => { continue; } - if (character === '"' || character === "'") { + if ((character === '"' || character === "'") && isQuotedScalarStart(value, index)) { quote = character; continue; } diff --git a/scripts/validate-check-map.test.mjs b/scripts/validate-check-map.test.mjs index de6a5d4..b274cd3 100644 --- a/scripts/validate-check-map.test.mjs +++ b/scripts/validate-check-map.test.mjs @@ -58,6 +58,19 @@ required_gates: ); }); + it.each([ + ['plural', "team's / gate"], + ['plural', '6" screen / gate'], + ['legacy', "team's / gate"], + ['legacy', '6" screen / gate'], + ])('accepts embedded quote characters in %s plain names: %s', (shape, name) => { + const declaration = shape === 'plural' + ? `required_gates:\n - check_name: ${name}` + : `required_gate:\n check_name: ${name}`; + + expectValid(`version: 1\n${declaration}\n`, [name]); + }); + it('accepts a plural mapping whose item marker is on its own line', () => { expectValid( `version: 1