From b60b3e1f2e2899d4ac62435e4987eada7d9340e4 Mon Sep 17 00:00:00 2001 From: Noah Gift Date: Wed, 16 Sep 2026 10:52:00 +0200 Subject: [PATCH 1/3] a roadmap edit without its fragment is refused (PMAT-3296, #3296) #3297 made docs/roadmaps/roadmap.yaml a GENERATED aggregate of docs/roadmaps/entries/ so that two PRs touching the roadmap touch two different files. Nothing enforced it. Measured on a clean worktree cut from origin/main, 2026-09-16: $ pmat work add "..." --github-issue 3294 M docs/roadmaps/roadmap.yaml <- the MONOLITH, 16 lines, no fragment $ python3 scripts/lib/roadmap_fragments.py aggregate --check ok roadmap.yaml == aggregate(1 fragment(s)), idempotent # rc=0 The existing aggregate check PASSES that edit and cannot do otherwise: `aggregate` takes roadmap.yaml as its own base, so an entry written straight into the base is a fixed point. It answers "is the aggregate consistent?", never "did this change come through the fragment path?". check_roadmap_fragment_required.sh asks the second question, over the base..head diff: 1. every top-level entry whose BYTES change in roadmap.yaml must have docs/roadmaps/entries/.yaml changing in the same diff; 2. whenever either side changes, head's roadmap.yaml must equal aggregate(head's entries/) -- IMPORTED from roadmap_fragments.py, never restated, so guard and generator cannot drift. A preamble-only change and a diff touching neither side still pass; an id that cannot be a filename has no fragment path at all and is refused with that stated (RMFR-OB-005). Retro-verdicts over the last 8 first-parent commits of origin/main: the one PR that used the fragment path (#3297) PASSES; the two fragment-less roadmap edits (#3348, 334646674) are refused. `pmat work add` writes the monolith, so the gate would block every future ticket. The remedy is real and is NAMED IN THE FAILURE MESSAGE: `roadmap_fragments.py adopt ` moves the entry into entries/.yaml (proved to parse alone and to carry the same mapping) and regenerates the aggregate, which also moves it to its sorted slot. This commit's own PMAT-3294 entry took that path: check_roadmap_sorted.sh PASS, check_roadmap_diff_additive.sh added=1 reserialised=0. Case table: 12 rows, hermetic fixture repos. Mutations: neutering rule 2 turns row 3 (drift) RED; comparing id SETS only turns rows 5, 6, 8 and 11 RED. bashrs lint --no-ignore --level error: 0 errors. Pmat-Ticket: PMAT-3296 Co-Authored-By: Claude Opus 5 (1M context) --- docs/roadmaps/entries/PMAT-3294.yaml | 16 + docs/roadmaps/roadmap.yaml | 16 + scripts/check_roadmap_fragment_required.sh | 456 +++++++++++++++++++++ scripts/lib/roadmap_fragments.py | 198 ++++++++- 4 files changed, 673 insertions(+), 13 deletions(-) create mode 100644 docs/roadmaps/entries/PMAT-3294.yaml create mode 100755 scripts/check_roadmap_fragment_required.sh diff --git a/docs/roadmaps/entries/PMAT-3294.yaml b/docs/roadmaps/entries/PMAT-3294.yaml new file mode 100644 index 0000000000..8dff170dfc --- /dev/null +++ b/docs/roadmaps/entries/PMAT-3294.yaml @@ -0,0 +1,16 @@ +- id: PMAT-3294 + github_issue: 3294 + item_type: task + title: a roadmap edit without its fragment is refused + status: planned + priority: medium + assigned_to: null + created: 2026-09-16T08:42:17Z + updated: 2026-09-16T08:42:17Z + spec: null + acceptance_criteria: [] + phases: [] + subtasks: [] + estimated_effort: null + labels: [] + notes: null diff --git a/docs/roadmaps/roadmap.yaml b/docs/roadmaps/roadmap.yaml index eb9b495901..0e7000f5c8 100644 --- a/docs/roadmaps/roadmap.yaml +++ b/docs/roadmaps/roadmap.yaml @@ -17914,6 +17914,22 @@ roadmap: notes: 'llvm-cov overhead scales with instrumented branches EXECUTED, which differs between the compress and decompress paths; the ratio had already been widened once to 0.25 for this and failed anyway' +- id: PMAT-3294 + github_issue: 3294 + item_type: task + title: a roadmap edit without its fragment is refused + status: planned + priority: medium + assigned_to: null + created: 2026-09-16T08:42:17Z + updated: 2026-09-16T08:42:17Z + spec: null + acceptance_criteria: [] + phases: [] + subtasks: [] + estimated_effort: null + labels: [] + notes: null - id: PMAT-3296 github_issue: 3296 item_type: task diff --git a/scripts/check_roadmap_fragment_required.sh b/scripts/check_roadmap_fragment_required.sh new file mode 100755 index 0000000000..97be7d1c29 --- /dev/null +++ b/scripts/check_roadmap_fragment_required.sh @@ -0,0 +1,456 @@ +#!/usr/bin/env bash +# check_roadmap_fragment_required.sh — a change to docs/roadmaps/roadmap.yaml +# must arrive WITH its docs/roadmaps/entries/.yaml fragment +# (PMAT-3296, #3296; five-whys #3294). +# +# WHY THIS EXISTS +# --------------- +# #3297 made roadmap.yaml a GENERATED aggregate of docs/roadmaps/entries/ so +# that two PRs touching the roadmap touch two different FILES: unique filename +# by construction => pull requests are pairwise disjoint on the roadmap, and the +# Amdahl serial fraction on the merge path stops being 1. Nothing enforced it. +# Measured on a clean worktree cut from origin/main, 2026-09-16: +# +# $ pmat work add "..." --github-issue 3294 +# M docs/roadmaps/roadmap.yaml <- the MONOLITH, 16 lines, no fragment +# $ python3 scripts/lib/roadmap_fragments.py aggregate --check +# ok roadmap.yaml == aggregate(1 fragment(s)), idempotent # rc=0 +# +# The aggregate check PASSES that edit, and cannot do otherwise: `aggregate` +# takes roadmap.yaml as its own base, so an entry written straight into the base +# is a fixed point. `--check` answers "is the aggregate consistent?", never "did +# this change come through the fragment path?". Without the second question the +# contention #3297 removed walks straight back in, one `pmat work add` at a +# time. (That same edit was also appended to the TAIL, which +# check_roadmap_sorted.sh refuses — two guards, two different defects, and only +# this one sees the missing fragment.) +# +# THE RULE, over the diff base..head: +# +# 1. FRAGMENT REQUIRED. For every top-level entry whose BYTES in roadmap.yaml +# differ between base and head (added, removed or changed), the same diff +# must change docs/roadmaps/entries/.yaml. A re-serialisation that +# changes no field still counts as changed — check_roadmap_diff_additive.sh +# already refuses that shape, so this gate must not be the one place it +# reads as "nothing happened". +# 2. THE AGGREGATE MUST BE REGENERATED. Whenever either side of the pair +# changes, head's roadmap.yaml must equal aggregate(head's entries/). +# That placement rule is NOT restated here: it is +# `roadmap_fragments.py aggregate --check`, the same function `make +# roadmap-aggregate` writes with, so the guard and the generator cannot +# drift. A fragment landed without `make roadmap-aggregate` is drift and is +# named as such. +# +# WHAT IS STILL ALLOWED: the aggregate changing AS AN AGGREGATE — every entry +# that moved has its fragment in the same diff and the file equals +# aggregate(fragments). Also a preamble-only change (the header above the first +# `- id:` is not entry content and has no fragment), and any diff that touches +# neither roadmap.yaml nor entries/. +# +# AN ID THAT CANNOT BE A FILENAME HAS NO FRAGMENT PATH AT ALL. 52 of the real +# entries are prose ids (one contains `origin/main`, a path separator). There is +# no entries/.yaml they could be accompanied by, so a change to one is +# refused with that stated — the scope boundary RMFR-OB-005 already declares, +# now enforced rather than described. +# +# `pmat work add` DOES NOT WRITE FRAGMENTS (measured above). That is a real +# conflict with this gate, so the remedy is named in the failure message rather +# than left for the next person to rediscover: +# +# pmat work add "" --github-issue <N> # writes the monolith +# python3 scripts/lib/roadmap_fragments.py adopt <ID> # -> entries/<ID>.yaml +# # + regenerates +# git add docs/roadmaps/entries/<ID>.yaml docs/roadmaps/roadmap.yaml +# +# bash scripts/check_roadmap_fragment_required.sh [<base-ref> [<head-ref>]] +# bash scripts/check_roadmap_fragment_required.sh --self-test +# +# Exit: 0 clean · 1 a violation · 2 this box cannot judge (never a silent pass). +# +# Refs: PMAT-3296, #3296, #3294, #3297 (the fragment mechanism), +# contracts/apr-roadmap-fragments-v1.yaml, scripts/check_roadmap_sorted.sh. + +set -uo pipefail + +PROG=${0##*/} +REPO_ROOT="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)" +PY_LIB="$REPO_ROOT/scripts/lib/roadmap_fragments.py" +ROADMAP_FILE="docs/roadmaps/roadmap.yaml" +ENTRIES_DIR="docs/roadmaps/entries" + +# shellcheck source=scripts/lib/resolve_base.sh +. "$REPO_ROOT/scripts/lib/resolve_base.sh" || exit 1 + +usage() { + printf 'usage: %s [<base-ref> [<head-ref>]] or %s --self-test\n' "$PROG" "$PROG" >&2 + printf ' a roadmap.yaml change must arrive with its docs/roadmaps/entries/<ID>.yaml fragment\n' >&2 + exit 2 +} + +# filename_safe ID -- the same shape roadmap_fragments.py's census uses: an id +# that cannot be a filename cannot be a fragment, so it has no write path. +filename_safe() { + case "$1" in + ''|*/*|*' '*) return 1 ;; + esac + [ "${#1}" -le 111 ] && [[ "$1" =~ ^[A-Za-z0-9][A-Za-z0-9._-]*$ ]] +} + +remedy() { + printf '\nREMEDY (PMAT-3296, #3296). roadmap.yaml is GENERATED from %s/:\n' "$ENTRIES_DIR" + printf ' * a NEW ticket — `pmat work add` writes the monolith and knows nothing about\n' + printf ' fragments (measured 2026-09-16), so take the two-step:\n' + printf ' pmat work add "<title>" --github-issue <N>\n' + printf ' python3 scripts/lib/roadmap_fragments.py adopt <ID>\n' + printf ' * an EXISTING entry — adopt it once, then edit %s/<ID>.yaml\n' "$ENTRIES_DIR" + printf ' (a fragment SUPERSEDES the base entry of the same id).\n' + printf ' * then regenerate and stage BOTH sides:\n' + printf ' make roadmap-aggregate\n' + printf ' git add %s/<ID>.yaml %s\n' "$ENTRIES_DIR" "$ROADMAP_FILE" + printf ' * an id that is not filename-safe (prose, or carrying a path separator) has NO\n' + printf ' fragment path: it stays in the base and is immutable (RMFR-OB-005).\n' +} + +# --------------------------------------------------------------------------- +# judge <repo> <base-ref> <head-ref> -> 0 clean · 1 violation · 2 cannot judge +# --------------------------------------------------------------------------- +judge() { + local repo=$1 base=$2 head=$3 + local td roadmap_changed=0 frag_paths names name verdict eid line + local violations=0 checked=0 out rc + + git -C "$repo" rev-parse --verify -q "$base^{commit}" >/dev/null || { + printf 'ENV %s: base ref %s is not a commit here — refusing to judge (never a pass)\n' "$PROG" "$base" >&2 + return 2 + } + git -C "$repo" rev-parse --verify -q "$head^{commit}" >/dev/null || { + printf 'ENV %s: head ref %s is not a commit here — refusing to judge (never a pass)\n' "$PROG" "$head" >&2 + return 2 + } + + td=$(mktemp -d "${TMPDIR:-/tmp}/rmfragreq.XXXXXX") || return 2 + + if [ -n "$(git -C "$repo" diff --name-only "$base" "$head" -- "$ROADMAP_FILE")" ]; then + roadmap_changed=1 + fi + frag_paths=$(git -C "$repo" diff --name-only "$base" "$head" -- "$ENTRIES_DIR") + + if [ "$roadmap_changed" = 0 ] && [ -z "$frag_paths" ]; then + rm -rf -- "${td:?}" + printf 'PASS %s: this diff touches neither %s nor %s/ — nothing to judge\n' \ + "$PROG" "$ROADMAP_FILE" "$ENTRIES_DIR" + return 0 + fi + + # The ids whose FRAGMENT this diff changes (added, edited or deleted). + : >"$td/frag_ids" + while IFS= read -r name; do + [ -n "$name" ] || continue + case "$name" in *.yaml) ;; *) continue ;; esac + name=${name##*/} + printf '%s\n' "${name%.yaml}" >>"$td/frag_ids" + done < <(printf '%s\n' "$frag_paths") + + # Materialise both roadmaps and head's fragment set. A side that does not + # exist at that ref is EMPTY, not an error: before the first fragment lands, + # the aggregate is just the base. + git -C "$repo" show "$base:$ROADMAP_FILE" >"$td/base.yaml" 2>/dev/null || : >"$td/base.yaml" + git -C "$repo" show "$head:$ROADMAP_FILE" >"$td/head.yaml" 2>/dev/null || : >"$td/head.yaml" + mkdir -p "$td/entries" + names=$(git -C "$repo" ls-tree -r --name-only "$head" -- "$ENTRIES_DIR" 2>/dev/null) || names="" + while IFS= read -r name; do + [ -n "$name" ] || continue + case "$name" in *.yaml) ;; *) continue ;; esac + if ! git -C "$repo" show "$head:$name" >"$td/entries/${name##*/}"; then + rm -rf -- "${td:?}" + printf 'ENV %s: cannot read %s at %s\n' "$PROG" "$name" "$head" >&2 + return 2 + fi + done < <(printf '%s\n' "$names") + + printf '=== %s: base=%s head=%s ===\n' "$PROG" "$base" "$head" + + # --- RULE 1: every changed entry carries its fragment in the same diff ---- + if ! out=$(python3 "$PY_LIB" changed --base "$td/base.yaml" --head "$td/head.yaml" 2>&1); then + rm -rf -- "${td:?}" + printf 'ENV %s: the entry reader could not compare the two roadmaps:\n%s\n' "$PROG" "$out" >&2 + return 2 + fi + while IFS=$'\t' read -r verdict eid; do + [ -n "$verdict" ] || continue + if [ "$verdict" = PREAMBLE ]; then + printf 'ok the header changed (not entry content, no fragment exists for it)\n' + continue + fi + checked=$((checked + 1)) + if grep -Fxq -- "$eid" "$td/frag_ids"; then + printf 'ok %-8s %s — %s/%s.yaml changes in the same diff\n' "$verdict" "$eid" "$ENTRIES_DIR" "$eid" + continue + fi + violations=$((violations + 1)) + if filename_safe "$eid"; then + printf 'FAIL %-8s %s in %s with NO change to %s/%s.yaml — a roadmap edit without its fragment\n' \ + "$verdict" "$eid" "$ROADMAP_FILE" "$ENTRIES_DIR" "$eid" + else + printf 'FAIL %-8s %s in %s — that id cannot be a filename, so it has NO fragment path: it lives in the base and is immutable (RMFR-OB-005)\n' \ + "$verdict" "$eid" "$ROADMAP_FILE" + fi + done < <(printf '%s\n' "$out") + + # --- RULE 2: the aggregate at head is REGENERATED, not drifting ---------- + out=$(python3 "$PY_LIB" aggregate --check --roadmap "$td/head.yaml" --entries "$td/entries" 2>&1) + rc=$? + rm -rf -- "${td:?}" + if [ "$rc" = 0 ]; then + printf 'ok %s == aggregate(%s/) at head\n' "$ROADMAP_FILE" "$ENTRIES_DIR" + else + violations=$((violations + 1)) + printf 'FAIL DRIFT: at head, %s is NOT aggregate(%s/) — a fragment changed and the aggregate was not regenerated (run `make roadmap-aggregate`)\n' \ + "$ROADMAP_FILE" "$ENTRIES_DIR" + while IFS= read -r line; do + [ -n "$line" ] && printf ' %s\n' "$line" + done < <(printf '%s\n' "$out") + fi + + if [ "$violations" != 0 ]; then + printf '%s: %s violation(s) over %s changed entry/entries\n' "$PROG" "$violations" "$checked" + remedy + return 1 + fi + printf 'PASS %s: %s changed entry/entries, each with its fragment; the aggregate is regenerated\n' "$PROG" "$checked" + return 0 +} + +# --------------------------------------------------------------------------- +# --self-test: hermetic fixture repos under mktemp -d. No file in this repo is +# read as data and none is written. +# --------------------------------------------------------------------------- +BASE_ROADMAP="roadmap_version: '1.0' +github_enabled: true +roadmap: +- id: PMAT-100 + title: first + status: planned +- id: PMAT-300 + title: third + status: planned +- id: Push completed work to origin/main (5 commits) + title: a real prose id, with a path separator in it + status: planned +" + +# mkrepo DIR -- a fixture repo whose HEAD is BASE_ROADMAP plus one fragment for +# PMAT-100, i.e. the post-#3297 shape. +mkrepo() { + local d=$1 + mkdir -p "$d/$ENTRIES_DIR" || return 2 + git -C "$d" init -q --template="$TEMPLATE" || return 2 + git -C "$d" config user.email test@example.com + git -C "$d" config user.name test + printf '%s' "$BASE_ROADMAP" >"$d/$ROADMAP_FILE" + printf -- '- id: PMAT-100\n title: first\n status: planned\n' >"$d/$ENTRIES_DIR/PMAT-100.yaml" + printf 'placeholder\n' >"$d/docs/roadmaps/README.md" + git -C "$d" add -A + git -C "$d" commit -qm base +} + +self_test() { + local td n=0 red=0 TEMPLATE + td=$(mktemp -d "${TMPDIR:-/tmp}/rmfragreq-selftest.XXXXXX") || return 2 + cleanup() { + local victim=${td:-} + case "$victim" in + *rmfragreq-selftest.*) if [ -n "$victim" ] && [ "$victim" != "/" ]; then rm -rf -- "$victim"; fi ;; + *) return 0 ;; + esac + } + trap cleanup RETURN + TEMPLATE="$td/.empty-template" + mkdir -p "$TEMPLATE" + export GIT_TERMINAL_PROMPT=0 + + # row NAME WANT_RC MUST_MATCH BUILDER -- BUILDER edits a fresh fixture repo + # and commits; the guard then judges HEAD~1..HEAD inside it. + row() { + local label=$1 want=$2 pat=$3 builder=$4 d out rc + n=$((n + 1)) + d="$td/r$n" + if ! mkrepo "$d" >/dev/null 2>&1; then + printf 'FAIL row %-2s %s: fixture repo could not be built\n' "$n" "$label" + red=$((red + 1)) + return + fi + "$builder" "$d" || { printf 'FAIL row %-2s %s: builder failed\n' "$n" "$label"; red=$((red + 1)); return; } + out=$(judge "$d" HEAD~1 HEAD 2>&1) + rc=$? + if [ "$rc" != "$want" ] || ! printf '%s' "$out" | grep -qF -- "$pat"; then + printf 'FAIL row %-2s rc=%s (wanted %s, must contain: %s) %s\n' "$n" "$rc" "$want" "$pat" "$label" + printf '%s\n' "$out" | sed 's/^/ /' + red=$((red + 1)) + return + fi + printf 'ok row %-2s rc=%s %s\n' "$n" "$rc" "$label" + } + + commit_all() { git -C "$1" add -A && git -C "$1" commit -qm head; } + + # ---- builders ------------------------------------------------------- + # THE MEASURED SHAPE: `pmat work add` appends an entry to the monolith and + # writes no fragment. + b_monolith_only() { + printf -- '- id: PMAT-200\n title: added by pmat work add\n status: planned\n' >>"$1/$ROADMAP_FILE" + commit_all "$1" + } + # The correct shape: the fragment, and the aggregate regenerated from it. + b_fragment_and_aggregate() { + printf -- '- id: PMAT-200\n title: added the fragment way\n status: planned\n' >"$1/$ENTRIES_DIR/PMAT-200.yaml" + python3 "$PY_LIB" aggregate --write --roadmap "$1/$ROADMAP_FILE" >/dev/null 2>&1 || return 1 + commit_all "$1" + } + # A fragment with no `make roadmap-aggregate`. + b_fragment_no_regen() { + printf -- '- id: PMAT-200\n title: fragment only\n status: planned\n' >"$1/$ENTRIES_DIR/PMAT-200.yaml" + commit_all "$1" + } + b_unrelated() { + printf 'a docs-only change\n' >>"$1/docs/roadmaps/README.md" + commit_all "$1" + } + # Re-serialisation: the bytes of every entry change, no field does. + b_reserialised() { + python3 - "$1/$ROADMAP_FILE" <<'PY' || return 1 +import sys +p = sys.argv[1] +t = open(p, encoding="utf-8").read() +t = t.replace(" title: third\n", " title: 'third'\n") +open(p, "w", encoding="utf-8").write(t) +PY + commit_all "$1" + } + # Lifecycle edit straight into the base, no fragment. + b_lifecycle_in_base() { + python3 - "$1/$ROADMAP_FILE" <<'PY' || return 1 +import sys +p = sys.argv[1] +t = open(p, encoding="utf-8").read() +t = t.replace("- id: PMAT-300\n title: third\n status: planned\n", + "- id: PMAT-300\n title: third\n status: completed\n") +open(p, "w", encoding="utf-8").write(t) +PY + commit_all "$1" + } + b_deleted_entry() { + python3 - "$1/$ROADMAP_FILE" <<'PY' || return 1 +import sys +p = sys.argv[1] +t = open(p, encoding="utf-8").read() +t = t.replace("- id: PMAT-300\n title: third\n status: planned\n", "") +open(p, "w", encoding="utf-8").write(t) +PY + commit_all "$1" + } + # A prose id that cannot be a filename. + b_prose_id_edit() { + python3 - "$1/$ROADMAP_FILE" <<'PY' || return 1 +import sys +p = sys.argv[1] +t = open(p, encoding="utf-8").read() +open(p, "w", encoding="utf-8").write(t.replace( + " title: a real prose id, with a path separator in it\n", + " title: an edit to a prose-id entry\n")) +PY + commit_all "$1" + } + # Preamble-only: the header above the first entry. + b_preamble_only() { + python3 - "$1/$ROADMAP_FILE" <<'PY' || return 1 +import sys +p = sys.argv[1] +t = open(p, encoding="utf-8").read() +open(p, "w", encoding="utf-8").write(t.replace("github_enabled: true\n", "github_enabled: false\n")) +PY + commit_all "$1" + } + # A fragment for a DIFFERENT ticket does not license this entry. + b_wrong_fragment() { + printf -- '- id: PMAT-100\n title: first\n status: planned\n' >"$1/$ENTRIES_DIR/PMAT-100.yaml" + printf -- '- id: PMAT-200\n title: unlicensed\n status: planned\n' >>"$1/$ROADMAP_FILE" + commit_all "$1" + } + # Supersession: edit an ADOPTED entry through its fragment, regenerate. + b_supersede() { + printf -- '- id: PMAT-100\n title: first\n status: completed\n' >"$1/$ENTRIES_DIR/PMAT-100.yaml" + python3 "$PY_LIB" aggregate --write --roadmap "$1/$ROADMAP_FILE" >/dev/null 2>&1 || return 1 + commit_all "$1" + } + + # ---- the case table -------------------------------------------------- + row 'MEASURED SHAPE: entry added to roadmap.yaml only (what pmat work add writes) -> REFUSE' \ + 1 'ADDED PMAT-200 in docs/roadmaps/roadmap.yaml with NO change' b_monolith_only + row 'fragment + regenerated aggregate, in sync -> PASS' \ + 0 'ok ADDED PMAT-200 — docs/roadmaps/entries/PMAT-200.yaml changes in the same diff' b_fragment_and_aggregate + row 'fragment added, aggregate NOT regenerated -> REFUSE, naming the drift' \ + 1 'FAIL DRIFT' b_fragment_no_regen + row 'a docs-only diff touching neither side -> PASS (no false positive)' \ + 0 'nothing to judge' b_unrelated + row 'the aggregate RE-SERIALISED with no content change -> REFUSE' \ + 1 'CHANGED PMAT-300' b_reserialised + row 'a lifecycle edit written straight into the base -> REFUSE (adopt it first)' \ + 1 'CHANGED PMAT-300' b_lifecycle_in_base + row 'an entry DELETED from the monolith with no fragment change -> REFUSE' \ + 1 'REMOVED PMAT-300' b_deleted_entry + row 'an id that cannot be a filename -> REFUSE, naming the absent write path' \ + 1 'NO fragment path' b_prose_id_edit + row 'a preamble-only change -> PASS (the header is not entry content)' \ + 0 'ok the header changed (not entry content, no fragment exists for it)' b_preamble_only + row "another ticket's fragment does not license this entry -> REFUSE" \ + 1 'ADDED PMAT-200 in docs/roadmaps/roadmap.yaml with NO change' b_wrong_fragment + row 'supersession through the fragment, regenerated -> PASS' \ + 0 'ok CHANGED PMAT-100 — docs/roadmaps/entries/PMAT-100.yaml changes in the same diff' b_supersede + + # An unresolvable ref is ENV (rc 2), never a pass. + n=$((n + 1)) + local out rc + mkrepo "$td/r$n" >/dev/null 2>&1 + out=$(judge "$td/r$n" deadbeefdeadbeefdeadbeefdeadbeefdeadbeef HEAD 2>&1) + rc=$? + if [ "$rc" = 2 ] && printf '%s' "$out" | grep -qF 'refusing to judge'; then + printf 'ok row %-2s rc=2 an unresolvable base ref is ENV, never a pass\n' "$n" + else + printf 'FAIL row %-2s rc=%s (wanted 2) an unresolvable base ref is ENV, never a pass\n' "$n" "$rc" + printf '%s\n' "$out" | sed 's/^/ /' + red=$((red + 1)) + fi + + printf '%s/%s rows, %s failed\n' "$((n - red))" "$n" "$red" + [ "$red" = 0 ] +} + +# --------------------------------------------------------------------------- +case "${1:-}" in + --self-test|--selftest) self_test; exit $? ;; + --help|-h) usage ;; + --*) usage ;; +esac + +if ! git -C "$REPO_ROOT" rev-parse --verify -q origin/main >/dev/null; then + printf '%s: origin/main is not resolvable here (no such remote-tracking ref).\n' "$PROG" >&2 + printf ' An environment gap, not a roadmap defect. Fetch it: git -C %s fetch origin main\n' "$REPO_ROOT" >&2 + exit 2 +fi + +HEAD_REF="${2:-HEAD}" +if [ -n "${1:-}" ]; then + BASE_REF="$1"; BASE_HOW="argument" +else + if ! resolve_base "$HEAD_REF"; then exit 2; fi +fi + +if [ "$(git -C "$REPO_ROOT" rev-parse "$BASE_REF^{commit}")" = "$(git -C "$REPO_ROOT" rev-parse "$HEAD_REF^{commit}")" ]; then + printf 'PASS base and head are the same commit: there is no diff to judge here\n' + exit 0 +fi + +printf '=== base=%s (%s) head=%s ===\n' "$BASE_REF" "$BASE_HOW" "$HEAD_REF" +judge "$REPO_ROOT" "$BASE_REF" "$HEAD_REF" +exit $? diff --git a/scripts/lib/roadmap_fragments.py b/scripts/lib/roadmap_fragments.py index 4125fab9a8..c9fa1ff535 100644 --- a/scripts/lib/roadmap_fragments.py +++ b/scripts/lib/roadmap_fragments.py @@ -212,6 +212,101 @@ def _write_fragments(frags, preamble, entries_dir): +# ------------------------------------------------------------------- changed + +def entry_map(text): + """-> (preamble, {id: block bytes}). Duplicate ids CONCATENATE rather than + overwrite: uniqueness is check_roadmap_ids_unique.sh's rule, and a reader + that silently dropped one side of a duplicate would compare the wrong + bytes.""" + pre, entries = split_entries(text) + out = {} + for eid, block in entries: + out[eid] = out.get(eid, "") + block + return pre, out + + +def changed_entries(base_text, head_text): + """-> (preamble_changed, [(verdict, id)]) over BYTE-EXACT entry blocks. + + A re-serialisation that changes no field reads as CHANGED here, and that is + deliberate: check_roadmap_diff_additive.sh already refuses that shape + ("bytes differ, no field actually changed"), so the fragment gate must not + be the one place it reads as "nothing happened".""" + bpre, b = entry_map(base_text) + hpre, h = entry_map(head_text) + verdicts = [("ADDED", e) if e not in b else ("CHANGED", e) + for e in h if e not in b or h[e] != b[e]] + verdicts += [("REMOVED", e) for e in b if e not in h] + verdicts.sort(key=lambda v: (v[1], v[0])) + return bpre != hpre, verdicts + + +# --------------------------------------------------------------------- adopt + +def _refuse_borrowed_anchor(eid, block, text): + """An entry that DEFINES an anchor other entries alias cannot be adopted on + its own: self_contain() strips the definition, and every `*idNNN` left in + the base would then be unresolvable YAML. Refuse rather than write a + roadmap.yaml that does not parse.""" + defined = set(ANCHOR_DEF_RE.findall(block)) + if not defined: + return + rest = text.replace(block, "", 1) + borrowed = sorted(d for d in defined if ("*" + d) in rest) + if borrowed: + raise ValueError( + "%s defines anchor(s) %s that other entries alias; adopting it alone " + "would leave them unresolvable. Split the whole file instead." + % (eid, ", ".join(borrowed))) + + +def adopt(ids, roadmap_path=ROADMAP, entries_dir=ENTRIES): + """Move entries out of the aggregate's BASE and into docs/roadmaps/entries/. + + This exists because `pmat work add` writes the monolith and knows nothing + about fragments (MEASURED 2026-09-16 on a clean worktree: one 16-line entry + appended to docs/roadmaps/roadmap.yaml and nothing else -- appended to the + TAIL, which check_roadmap_sorted.sh also refuses). Rather than block the + ticket path, the gate names a two-step: + + pmat work add "..." --github-issue N + python3 scripts/lib/roadmap_fragments.py adopt <ID> + + The second step writes the fragment and regenerates the aggregate, which + also moves the entry to its sorted slot. Every fragment is proved to parse + ALONE and to carry the mapping the monolith carried before anything is + written -- the same proof split() applies.""" + import yaml + with open(roadmap_path, encoding="utf-8") as fh: + text = fh.read() + _preamble, entries = split_entries(text) + anchors = collect_anchors(text) + items = yaml.safe_load(text)["roadmap"] + if len(items) != len(entries): + raise ValueError("byte split (%d) disagrees with the parse (%d)" + % (len(entries), len(items))) + at = {} + for i, (eid, _) in enumerate(entries): + at.setdefault(eid, []).append(i) + os.makedirs(entries_dir, exist_ok=True) + written = [] + for eid in ids: + where = at.get(eid, []) + if not where: + raise ValueError("%s is not a top-level entry of %s" % (eid, roadmap_path)) + if len(where) > 1: + raise ValueError("%s appears %d times in %s -- fix the duplicate first " + "(check_roadmap_ids_unique.sh)" % (eid, len(where), roadmap_path)) + block = entries[where[0]][1] + _refuse_borrowed_anchor(eid, block, text) + frag = _proved_fragment(eid, block, items[where[0]], anchors) + with open(fragment_path(eid, entries_dir), "w", encoding="utf-8") as fh: + fh.write(frag) + written.append(eid) + return written + + # ---------------------------------------------------------------- self-test BASE = ( @@ -349,26 +444,106 @@ def _check(base, out, frags): return 0 -def _emit(a, base, out, frags): +def _emit(a, base, out, frags, roadmap): """The three terminal arms of `aggregate`: verify, write, or print.""" if a.check: return _check(base, out, frags) if a.write: - with open(ROADMAP, "w", encoding="utf-8") as fh: + with open(roadmap, "w", encoding="utf-8") as fh: fh.write(out) sys.stderr.write("aggregate: %d base + %d fragment(s) -> %s\n" - % (len(split_entries(base)[1]), len(frags), ROADMAP)) + % (len(split_entries(base)[1]), len(frags), roadmap)) return 0 sys.stdout.write(out) return 0 +def _paths(a): + """(roadmap, entries). --roadmap moves BOTH by default: a caller judging a + tree that is not this checkout (check_roadmap_fragment_required.sh extracts + base and head into a scratch dir) must never silently read this repo's live + docs/roadmaps/entries/ as the other tree's fragments.""" + roadmap = a.roadmap or ROADMAP + if a.entries: + return roadmap, a.entries + if a.roadmap: + return roadmap, os.path.join(os.path.dirname(os.path.abspath(roadmap)), "entries") + return roadmap, ENTRIES + + +def _read(path, what): + try: + with open(path, encoding="utf-8") as fh: + return fh.read() + except OSError as e: + sys.stderr.write("FAIL %s is unreadable (%s) -- this box cannot judge\n" % (what, e)) + return None + + +def _cmd_aggregate(a, roadmap, entries_dir): + base = _read(roadmap, roadmap) + if base is None: + return 2 + frags = read_fragments(entries_dir) + try: + out = aggregate(base, frags) + except ValueError as e: + sys.stderr.write("FAIL %s\n" % e) + return 1 + return _emit(a, base, out, frags, roadmap) + + +def _cmd_changed(a): + """`changed --base A --head B` -> one `VERDICT<TAB>ID` line per entry whose + BYTES differ, plus `PREAMBLE` if the header did. The consumer decides + policy; this only reports. rc 2 (never 0) if either side is unreadable.""" + if not a.base or not a.head: + sys.stderr.write("changed: --base and --head are both required\n") + return 2 + base = _read(a.base, a.base) + head = _read(a.head, a.head) + if base is None or head is None: + return 2 + pre_changed, verdicts = changed_entries(base, head) + if pre_changed: + sys.stdout.write("PREAMBLE\t(the header outside every entry)\n") + for verdict, eid in verdicts: + sys.stdout.write("%s\t%s\n" % (verdict, eid)) + return 0 + + +def _cmd_adopt(a, roadmap, entries_dir): + if not a.ids: + sys.stderr.write("adopt: name at least one entry id\n") + return 2 + try: + written = adopt(a.ids, roadmap, entries_dir) + base = _read(roadmap, roadmap) + if base is None: + return 2 + frags = read_fragments(entries_dir) + out = aggregate(base, frags) + except (ValueError, KeyError) as e: + sys.stderr.write("FAIL %s\n" % e) + return 1 + with open(roadmap, "w", encoding="utf-8") as fh: + fh.write(out) + sys.stderr.write("adopt: %s -> %s/ ; %s regenerated from %d fragment(s)\n" + % (", ".join(written), entries_dir, roadmap, len(frags))) + return 0 + + def _parser(): ap = argparse.ArgumentParser(prog="roadmap_fragments.py") - ap.add_argument("cmd", nargs="?", choices=["aggregate"]) + ap.add_argument("cmd", nargs="?", choices=["aggregate", "changed", "adopt"]) + ap.add_argument("ids", nargs="*", help="adopt: the entry id(s) to move into entries/") ap.add_argument("--write", action="store_true") ap.add_argument("--check", action="store_true", help="fail if roadmap.yaml is not what the aggregator produces") + ap.add_argument("--roadmap", help="judge this roadmap.yaml instead of the repo's") + ap.add_argument("--entries", help="read fragments from here instead of docs/roadmaps/entries/") + ap.add_argument("--base", help="changed: the roadmap.yaml to compare FROM") + ap.add_argument("--head", help="changed: the roadmap.yaml to compare TO") ap.add_argument("--selftest", action="store_true") return ap @@ -378,18 +553,15 @@ def main(argv=None): a = ap.parse_args(argv) if a.selftest: return selftest() + roadmap, entries_dir = _paths(a) + if a.cmd == "changed": + return _cmd_changed(a) + if a.cmd == "adopt": + return _cmd_adopt(a, roadmap, entries_dir) if a.cmd != "aggregate": ap.print_usage(sys.stderr) return 2 - with open(ROADMAP, encoding="utf-8") as fh: - base = fh.read() - frags = read_fragments() - try: - out = aggregate(base, frags) - except ValueError as e: - sys.stderr.write("FAIL %s\n" % e) - return 1 - return _emit(a, base, out, frags) + return _cmd_aggregate(a, roadmap, entries_dir) if __name__ == "__main__": From 0df057c3b0201cf1f007ddad1ce149faafbb978d Mon Sep 17 00:00:00 2001 From: Noah Gift <noah.gift@gmail.com> Date: Wed, 16 Sep 2026 11:02:26 +0200 Subject: [PATCH 2/3] =?UTF-8?q?push=20shape=20is=20not=20judged:=20the=20g?= =?UTF-8?q?uard=20declines,=20out=20loud=20(PMAT-3296,=20=C2=A78)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A differential guard run bare on main is handed HEAD^1 as its base, because resolve_base's push-shape arm exists to avoid judging a commit against itself. For THIS guard that base is wrong: HEAD^1..HEAD is the PREVIOUS merge's diff, not this change. Measured before the fix: 2 of the last 8 first-parent commits of origin/main (#3348, 334646674) are fragment-less, so a bare run on main refused changes that landed before the guard existed and would have red-lined guard_tree from the moment it merged. That is re-litigating history, not measuring this change. In push shape the guard now reports a named SKIP and exits 0, and says what it did NOT check: the shape detected, the base it would have used, and the roadmap.yaml <-> entries/ pairing it therefore left ungraded -- which the change's own pull_request / merge_group run grades against a real merge-base. PR shape is untouched. The decision is read from BASE_HOW, the one place resolve_base makes it, so this cannot drift from it. Two case-table rows, over the real DISPATCH rather than judge() -- the fixture carries its own copy of the guard and the libraries it sources, so $REPO_ROOT is the fixture and origin/main is whatever the row points at. Same commit, same fragment-less content, two verdicts: row 13 origin/main IS this commit -> SKIP, exit 0 (the row that would have red-lined main) row 14 origin/main is its parent -> REFUSE, exit 1 14/14 rows, 0 failed. bashrs lint --no-ignore --level error: 0 errors. RETRO over the last 8 first-parent commits, after the fix: 8/8 exit 0 in push shape (none refused), while PR shape still refuses the same 2. Note 7 of those 8 exit 0 via base==head rather than via SKIP: resolve_base's "behind the tip" arm tests `git rev-list --first-parent | grep -qx`, and grep -q's early exit SIGPIPEs rev-list (rc 141 under the `set -o pipefail` every caller sets), so only the tip itself -- which short-circuits on string equality -- reaches the push-shape arm. Both routes exit 0, so the §8 requirement holds either way; the SIGPIPE is a pre-existing defect in scripts/lib/resolve_base.sh (shared with check_roadmap_diff_additive.sh), OUT OF SCOPE here and reported rather than fixed. Pmat-Ticket: PMAT-3296 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- scripts/check_roadmap_fragment_required.sh | 79 ++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/scripts/check_roadmap_fragment_required.sh b/scripts/check_roadmap_fragment_required.sh index 97be7d1c29..597bf7cc0f 100755 --- a/scripts/check_roadmap_fragment_required.sh +++ b/scripts/check_roadmap_fragment_required.sh @@ -62,7 +62,23 @@ # # + regenerates # git add docs/roadmaps/entries/<ID>.yaml docs/roadmaps/roadmap.yaml # +# PUSH SHAPE IS NOT JUDGED (§8 decision, 2026-09-16). resolve_base names +# HEAD^1 as the base when HEAD is on the origin/main first-parent line, because +# for most guards a commit judged against ITSELF is the vacuous pass to avoid. +# For a DIFFERENTIAL guard that is the wrong base: HEAD^1..HEAD is the PREVIOUS +# merge's diff, not this change. A guard added today would then grade commits +# that landed before it existed -- measured here: 2 of the last 8 first-parent +# commits of origin/main (#3348, 334646674) are fragment-less, so a bare run on +# main would have red-lined guard_tree from the moment this merged. It is not +# re-litigating history. In push shape the guard reports a named SKIP, prints +# the shape and the base it WOULD have used and what it therefore did not +# check, and exits 0. The change is graded on its pull_request / merge_group +# run, where the base is a real merge-base. PR shape is unchanged. +# # bash scripts/check_roadmap_fragment_required.sh [<base-ref> [<head-ref>]] +# bash scripts/check_roadmap_fragment_required.sh '' <head-ref> # resolve the +# base for an arbitrary head (how the retro and the case table reach the +# push-shape arm at a commit that is not the checkout's HEAD) # bash scripts/check_roadmap_fragment_required.sh --self-test # # Exit: 0 clean · 1 a violation · 2 this box cannot judge (never a silent pass). @@ -84,6 +100,7 @@ ENTRIES_DIR="docs/roadmaps/entries" usage() { printf 'usage: %s [<base-ref> [<head-ref>]] or %s --self-test\n' "$PROG" "$PROG" >&2 printf ' a roadmap.yaml change must arrive with its docs/roadmaps/entries/<ID>.yaml fragment\n' >&2 + printf " an empty <base-ref> ('') resolves the base for <head-ref>; push shape SKIPs, never grades\n" >&2 exit 2 } @@ -422,6 +439,51 @@ PY red=$((red + 1)) fi + # ---- the push-shape pair (§8). These exercise the real DISPATCH -- + # resolve_base plus the skip -- not judge(), so the fixture carries its own + # copy of the guard and the libraries it sources: $REPO_ROOT is then the + # fixture, and origin/main is whatever the row points it at. Same commit, + # same fragment-less content, two verdicts: the guard refuses the change + # when it can SEE the change, and declines to judge when it cannot. + dispatch_row() { + local label=$1 want=$2 pat=$3 target=$4 d out rc f + n=$((n + 1)) + d="$td/r$n" + if ! mkrepo "$d" >/dev/null 2>&1 || ! b_monolith_only "$d" >/dev/null 2>&1; then + printf 'FAIL row %-2s %s: fixture could not be built\n' "$n" "$label" + red=$((red + 1)) + return + fi + mkdir -p "$d/scripts/lib" + if ! cp "$REPO_ROOT/scripts/$PROG" "$d/scripts/$PROG"; then + printf 'FAIL row %-2s %s: the guard could not be copied into the fixture\n' "$n" "$label" + red=$((red + 1)) + return + fi + for f in resolve_base.sh roadmap_fragments.py roadmap_diff.py roadmap_merge.py; do + if ! cp "$REPO_ROOT/scripts/lib/$f" "$d/scripts/lib/$f"; then + printf 'FAIL row %-2s %s: scripts/lib/%s could not be copied into the fixture\n' "$n" "$label" "$f" + red=$((red + 1)) + return + fi + done + git -C "$d" update-ref refs/remotes/origin/main "$(git -C "$d" rev-parse "$target")" + out=$(bash "$d/scripts/$PROG" 2>&1) + rc=$? + if [ "$rc" != "$want" ] || ! printf '%s' "$out" | grep -qF -- "$pat"; then + printf 'FAIL row %-2s rc=%s (wanted %s, must contain: %s) %s\n' "$n" "$rc" "$want" "$pat" "$label" + printf '%s\n' "$out" | sed 's/^/ /' + red=$((red + 1)) + return + fi + printf 'ok row %-2s rc=%s %s\n' "$n" "$rc" "$label" + } + + dispatch_row 'PUSH shape (origin/main IS this commit), last merge fragment-less -> SKIP, exit 0 (the row that would have red-lined main)' \ + 0 'SKIP push-shape' HEAD + dispatch_row 'PR shape (origin/main is its parent), SAME fragment-less content -> REFUSE, exit 1' \ + 1 'FAIL ADDED PMAT-200' HEAD~1 + printf '%s/%s rows, %s failed\n' "$((n - red))" "$n" "$red" [ "$red" = 0 ] } @@ -446,6 +508,23 @@ else if ! resolve_base "$HEAD_REF"; then exit 2; fi fi +# PUSH SHAPE: decline to judge, out loud. `resolve_base` says so in BASE_HOW -- +# the one place that decision is made, so this cannot drift from it. +case "$BASE_HOW" in + *"push shape"*) + printf 'SKIP push-shape: %s (%s) is on the origin/main first-parent line, so the only nameable base is %s.\n' \ + "$HEAD_REF" "$(git -C "$REPO_ROOT" rev-parse --short "$HEAD_REF^{commit}")" "$BASE_REF" + printf ' base would have been: %s\n' "$BASE_HOW" + printf ' Grading it would judge the PREVIOUS merge, not this change: a differential guard added today would\n' + printf ' refuse commits that landed before it existed (2 of the last 8 first-parent commits of origin/main are\n' + printf ' fragment-less) and red-line main from the moment it merges.\n' + printf ' NOT CHECKED HERE: the %s <-> %s/ pairing, and the aggregate, over %s..%s. Both are\n' \ + "$ROADMAP_FILE" "$ENTRIES_DIR" "$BASE_REF" "$HEAD_REF" + printf ' graded on the pull_request / merge_group run of that change, where the base is a real merge-base.\n' + exit 0 + ;; +esac + if [ "$(git -C "$REPO_ROOT" rev-parse "$BASE_REF^{commit}")" = "$(git -C "$REPO_ROOT" rev-parse "$HEAD_REF^{commit}")" ]; then printf 'PASS base and head are the same commit: there is no diff to judge here\n' exit 0 From 1d132820f30c051b4634ea61ddde382020a40835 Mon Sep 17 00:00:00 2001 From: Noah Gift <noah.gift@gmail.com> Date: Wed, 16 Sep 2026 11:45:57 +0200 Subject: [PATCH 3/3] fix(guard): the new guard's own case table piped into grep -q MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check_no_pipe_into_grep_q refuses it, and correctly: under pipefail the producer's SIGPIPE is what the pipeline reports, not grep's verdict — so a case-table row could pass on a death rather than on a match. The three sites read a here-string now. No row's meaning changes. Pmat-Ticket: PMAT-3296 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- scripts/check_roadmap_fragment_required.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/check_roadmap_fragment_required.sh b/scripts/check_roadmap_fragment_required.sh index 597bf7cc0f..5a428523f6 100755 --- a/scripts/check_roadmap_fragment_required.sh +++ b/scripts/check_roadmap_fragment_required.sh @@ -300,7 +300,7 @@ self_test() { "$builder" "$d" || { printf 'FAIL row %-2s %s: builder failed\n' "$n" "$label"; red=$((red + 1)); return; } out=$(judge "$d" HEAD~1 HEAD 2>&1) rc=$? - if [ "$rc" != "$want" ] || ! printf '%s' "$out" | grep -qF -- "$pat"; then + if [ "$rc" != "$want" ] || ! grep -qF -- "$pat" <<<"$out"; then printf 'FAIL row %-2s rc=%s (wanted %s, must contain: %s) %s\n' "$n" "$rc" "$want" "$pat" "$label" printf '%s\n' "$out" | sed 's/^/ /' red=$((red + 1)) @@ -431,7 +431,7 @@ PY mkrepo "$td/r$n" >/dev/null 2>&1 out=$(judge "$td/r$n" deadbeefdeadbeefdeadbeefdeadbeefdeadbeef HEAD 2>&1) rc=$? - if [ "$rc" = 2 ] && printf '%s' "$out" | grep -qF 'refusing to judge'; then + if [ "$rc" = 2 ] && grep -qF 'refusing to judge' <<<"$out"; then printf 'ok row %-2s rc=2 an unresolvable base ref is ENV, never a pass\n' "$n" else printf 'FAIL row %-2s rc=%s (wanted 2) an unresolvable base ref is ENV, never a pass\n' "$n" "$rc" @@ -470,7 +470,7 @@ PY git -C "$d" update-ref refs/remotes/origin/main "$(git -C "$d" rev-parse "$target")" out=$(bash "$d/scripts/$PROG" 2>&1) rc=$? - if [ "$rc" != "$want" ] || ! printf '%s' "$out" | grep -qF -- "$pat"; then + if [ "$rc" != "$want" ] || ! grep -qF -- "$pat" <<<"$out"; then printf 'FAIL row %-2s rc=%s (wanted %s, must contain: %s) %s\n' "$n" "$rc" "$want" "$pat" "$label" printf '%s\n' "$out" | sed 's/^/ /' red=$((red + 1))