From a4c54e26b18cf2360cb76074c882f7b695973a52 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 09:06:35 +0000 Subject: [PATCH] fix(ci): read workflows as KYAML as well as block YAML (S4 prerequisites) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Workflows move to KYAML (YAML-POLICY §3.4) once the standards#1025 gate is met. Three local checks read workflows line by line and give wrong answers on the KYAML that `yq -o kyaml` writes: - check-workflow-duplicate-keys.sh flagged every KYAML workflow. KYAML marks a sequence item with `{`, not `- `, so sibling steps looked like duplicates. A bare `{` now opens a fresh scope. `}`/`]` lines name no key, and a quoted scalar item ("echo: hi",) is no longer read as a key. - check-allow-fail-expiry.sh counted `continue-on-error: false,` as an allow-fail unit (trailing comma) and missed a quoted key. Both are handled. yq keeps the same-line `# allow-fail:` mark through the conversion. - test-issue-regressions.sh tested `^permissions:`. KYAML indents top-level keys inside the root `{`. Top-level `permissions` is now read with yq. The script refuses any yq but mikefarah v4: the `yq` on this container's PATH is the Python jq wrapper, which D293 rules out. New KYAML fixtures cover a duplicate `with`, a clean multi-step file, quoted scalar items, allow-fail units, top-level permissions, and a job-level `permissions` that must not count. Against the old checkers the suite fails, and with the new ones it passes. Every real workflow passes both as committed and as rendered by `yq -o kyaml`, with 35 clean files and 14 allow-fail units each. Not in this change: the governance-reusable pin bump. It needs `gh actions-lock` to regenerate actions.lock, which needs an authenticated GitHub token this session lacks. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019a95kk88TZ1iegsBK3kox3 --- scripts/ci/check-allow-fail-expiry.sh | 9 +- scripts/ci/check-workflow-duplicate-keys.sh | 19 ++- scripts/ci/test-issue-regressions.sh | 121 +++++++++++++++++++- 3 files changed, 141 insertions(+), 8 deletions(-) diff --git a/scripts/ci/check-allow-fail-expiry.sh b/scripts/ci/check-allow-fail-expiry.sh index 9af3bdf4..9d96b8ee 100755 --- a/scripts/ci/check-allow-fail-expiry.sh +++ b/scripts/ci/check-allow-fail-expiry.sh @@ -12,6 +12,11 @@ # the same literal the runner sees. It does not decide what happens at expiry. # A person does (ultraplan Phase 2.3, tracking issue #416). # +# Block YAML and KYAML (YAML-POLICY §3.4) are both read. In KYAML the key may +# be quoted and the value ends in a comma (`continue-on-error: false, # …`); +# `yq -o kyaml` keeps the trailing comment on the same line, so the mark +# survives the conversion. +# # Usage: check-allow-fail-expiry.sh [--today YYYY-MM-DD] [--warn-days N] [DIR ...] # # Exit codes: 0 all units owned and in date, 1 a unit is unowned, undated, @@ -67,8 +72,8 @@ for file in "${files[@]}"; do while IFS= read -r line || [ -n "$line" ]; do lineno=$((lineno + 1)) # Same shape as the old Python KEY pattern: the key must open the line. - [[ $line =~ ^[[:space:]]*continue-on-error:[[:space:]]*(.*)$ ]] || continue - value=$(printf '%s' "${BASH_REMATCH[1]}" | sed -E 's/[[:space:]]*(#.*)?$//') + [[ $line =~ ^[[:space:]]*\"?continue-on-error\"?:[[:space:]]*(.*)$ ]] || continue + value=$(printf '%s' "${BASH_REMATCH[1]}" | sed -E 's/[[:space:]]*(#.*)?$//; s/,$//') [ "$value" = "false" ] && continue units=$((units + 1)) diff --git a/scripts/ci/check-workflow-duplicate-keys.sh b/scripts/ci/check-workflow-duplicate-keys.sh index a527697e..de9e5005 100644 --- a/scripts/ci/check-workflow-duplicate-keys.sh +++ b/scripts/ci/check-workflow-duplicate-keys.sh @@ -39,6 +39,15 @@ # * LIST ITEMS — every step in a job legitimately repeats `name:`. # * DOCUMENT SEPARATORS — `---` starts a fresh sibling scope. # * QUOTED KEYS — `"on":` and `on:` are the same key. +# * KYAML FLOW ITEMS — KYAML (YAML-POLICY §3.4) writes a sequence of +# mappings as `{` lines with no `- ` marker, so a line that is only `{` +# opens a fresh scope for the keys under it, exactly as `- ` does. +# A quoted scalar item such as `"echo: hi",` is not a key, and `}`/`]` +# lines close a scope without naming one. +# +# KYAML is checked in the layout `yq -o kyaml` writes (one key per line), +# which `kyaml-format.sh --check` enforces. Duplicates inside a single-line +# flow mapping such as `{a: 1, a: 2}` are not seen; that layout is not KYAML. set -uo pipefail scan_one() { @@ -65,13 +74,19 @@ scan_one() { if (trimmed == "" || substr(trimmed, 1, 1) == "#") next if (trimmed == "---") { delete seen; next } + + # KYAML: a bare `{` is a mapping item of a sequence, a fresh scope. + if (trimmed == "{") { clear_from(ind + 1); next } + # KYAML: closing brackets end a scope and name no key. + c = substr(trimmed, 1, 1) + if (c == "}" || c == "]") next if (trimmed == "...") { delete seen; next } # a list item opens a fresh mapping scope if (substr(trimmed, 1, 2) == "- " || trimmed == "-") { clear_from(ind) rest = substr(trimmed, 3) - if (match(rest, /^("[^"]*"|\x27[^\x27]*\x27|[^ #][^:]*):( |$)/)) { + if (match(rest, /^("[^"]*"|\x27[^\x27]*\x27|[^ #"\x27][^:]*):( |$)/)) { key = substr(rest, RSTART, RLENGTH) sub(/:.*$/, "", key); gsub(/^["\x27]|["\x27]$/, "", key) seen[ind + 2, key] = 1 @@ -80,7 +95,7 @@ scan_one() { next } - if (!match(trimmed, /^("[^"]*"|\x27[^\x27]*\x27|[^ #][^:]*):( |$)/)) { + if (!match(trimmed, /^("[^"]*"|\x27[^\x27]*\x27|[^ #"\x27][^:]*):( |$)/)) { if (match(trimmed, /:[ ]*[|>][-+0-9]*$/)) block = ind next } diff --git a/scripts/ci/test-issue-regressions.sh b/scripts/ci/test-issue-regressions.sh index 94ca5ff0..b1433f7a 100644 --- a/scripts/ci/test-issue-regressions.sh +++ b/scripts/ci/test-issue-regressions.sh @@ -40,16 +40,129 @@ jobs: YAML bash scripts/ci/check-workflow-duplicate-keys.sh "$tmp/valid.yml" +# The same two cases in KYAML (YAML-POLICY §3.4), the form workflows move to. +# KYAML marks sequence items with `{`, not `- `, so sibling steps must not +# read as duplicates, while a repeated key inside one step still must. +cat > "$tmp/duplicate.kyaml.yml" <<'YAML' +{ + jobs: { + test: { + steps: [ + { + uses: "dtolnay/rust-toolchain@v1", + with: { + toolchain: "master", + }, + with: { + toolchain: "stable", + }, + }, + ], + }, + }, +} +YAML +if bash scripts/ci/check-workflow-duplicate-keys.sh "$tmp/duplicate.kyaml.yml" > "$tmp/duplicate.log"; then + echo 'FAIL: duplicate toolchain mapping accepted in KYAML' >&2 + exit 1 +fi +grep -q "duplicate key(s): 'with'" "$tmp/duplicate.log" +cat > "$tmp/valid.kyaml.yml" <<'YAML' +{ + jobs: { + first: { + steps: [ + { + name: "First", + run: "echo 'with: not a YAML key'\necho 'with: still shell text'\n", + }, + { + name: "Second", + run: "echo ok", + }, + "with: a quoted scalar item, not a key", + "with: a quoted scalar item, not a key", + ], + }, + }, +} +YAML +bash scripts/ci/check-workflow-duplicate-keys.sh "$tmp/valid.kyaml.yml" + +# Allow-fail units in KYAML: `false,` is not a unit, a quoted key is one. +mkdir "$tmp/allow-fail" +cat > "$tmp/allow-fail/units.yml" <<'YAML' +{ + jobs: { + a: { + continue-on-error: false, + }, + b: { + "continue-on-error": true, + }, + }, +} +YAML +if bash scripts/ci/check-allow-fail-expiry.sh --today 2026-10-14 "$tmp/allow-fail" > "$tmp/allow-fail.log"; then + echo 'FAIL: unmarked KYAML allow-fail unit accepted' >&2 + exit 1 +fi +grep -q 'allow-fail units checked: 1$' "$tmp/allow-fail.log" +grep -q 'units.yml:7: continue-on-error without' "$tmp/allow-fail.log" + # Managed workflows start with an actions-lock comment, not the SPDX header. # Inspect the comment preamble rather than assuming the licence is line one. -for f in .github/workflows/*.yml; do +# Top-level `permissions` is read with yq (YAML-POLICY Y-1): in KYAML it is +# indented inside the root `{`, so a `^permissions:` line test cannot see it, +# and an indented job-level key must not count. +# Only mikefarah yq v4 qualifies: a `yq` on PATH can be the Python jq wrapper, +# and no Python YAML parser may decide a check (YAML-POLICY, D293). +yq --version 2>/dev/null | grep -q 'mikefarah/yq' || { + echo 'FAIL: mikefarah yq v4 is required (the yq on PATH is missing or another tool)' >&2; exit 1; +} +check_workflow_preamble() { + local f=$1 has sed '/^[^#[:space:]]/q' "$f" | grep -q '^# SPDX-License-Identifier:' || { - echo "FAIL: $f missing SPDX header" >&2; exit 1; + echo "FAIL: $f missing SPDX header" >&2; return 1; + } + has=$(yq -r 'has("permissions")' "$f") || { + echo "FAIL: $f does not parse as YAML" >&2; return 1; } - grep -q '^permissions:' "$f" || { - echo "FAIL: $f missing top-level permissions" >&2; exit 1; + [ "$has" = true ] || { + echo "FAIL: $f missing top-level permissions" >&2; return 1; } +} +for f in .github/workflows/*.yml; do + check_workflow_preamble "$f" done +cat > "$tmp/preamble.kyaml.yml" <<'YAML' +# SPDX-License-Identifier: MPL-2.0 +{ + "on": "push", + permissions: { + contents: "read", + }, + jobs: {}, +} +YAML +check_workflow_preamble "$tmp/preamble.kyaml.yml" +cat > "$tmp/job-permissions.kyaml.yml" <<'YAML' +# SPDX-License-Identifier: MPL-2.0 +{ + "on": "push", + jobs: { + a: { + permissions: { + contents: "read", + }, + }, + }, +} +YAML +if check_workflow_preamble "$tmp/job-permissions.kyaml.yml" 2>/dev/null; then + echo 'FAIL: job-level permissions accepted as top-level' >&2 + exit 1 +fi # #240: deletion of legacy source must not leave a reusable exemption behind. if [[ -n $(git ls-files '*.res') ]]; then