diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index b12c5e5c..e41d95cb 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -22,6 +22,7 @@ on: - PHILOSOPHY.md - ROADMAP.md - scripts/roadmap-*.sh + - tests/test_roadmap_helpers.py - docs/** - rust/** pull_request: @@ -43,6 +44,7 @@ on: - PHILOSOPHY.md - ROADMAP.md - scripts/roadmap-*.sh + - tests/test_roadmap_helpers.py - docs/** - rust/** workflow_dispatch: @@ -76,6 +78,8 @@ jobs: run: python .github/scripts/check_release_readiness.py - name: Check ROADMAP ids run: scripts/roadmap-check-ids.sh + - name: Check ROADMAP helper behavior + run: python -m unittest discover -s tests -p test_roadmap_helpers.py fmt: name: cargo fmt diff --git a/ROADMAP.md b/ROADMAP.md index f44ea0a8..89ff2be3 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -7615,3 +7615,5 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed) 723. **Concurrent dogfood claws allocate ROADMAP ids manually and collide — same id reused by two contributors simultaneously, causing PR ROADMAP.md conflicts and lost entries** — observed live 2026-05-26 during Jobdori+Gaebal parallel dogfood session: Gaebal filed stale-local-probe as #719; Jobdori landed `plugins list ` as #719 on main first; Gaebal shifted to #720; Jobdori landed `claw help ` as #720; stale-local-probe eventually landed as #721 after two forced rebase cycles. The ROADMAP append workflow has no reservation or conflict-aware id allocation. **Required fix shape:** (a) add `scripts/roadmap-next-id.sh` that reads the highest id from ROADMAP.md and prints `highest+1` — claws should call this immediately before appending any new entry; (b) document in CONTRIBUTING.md that id allocation is optimistic-append: call `roadmap-next-id.sh` immediately before the append, git-pull first, resolve collisions at push time by re-numbering the appended entry; (c) long-term: a GitHub Action that validates no duplicate ROADMAP ids on PR would catch this before merge. Added `scripts/roadmap-next-id.sh` (this commit). Source: Gaebal Gajae live observation, 2026-05-26. 724. **DONE — ROADMAP duplicate-id validation guard for helper-era append collisions** — follow-up to #723 after dogfood showed `scripts/roadmap-next-id.sh` still printed 724 and exited 0 when a temp ROADMAP copy already contained a second `723. ...` line. This PR closes the gap for new optimistic-append collisions by adding `scripts/roadmap-check-ids.sh`, wiring it into docs CI and the local pre-push hook, documenting the pre-push command in CONTRIBUTING, and mentioning the guard from `roadmap-next-id.sh`. The guard defaults to ids >=723 so current historical roadmap content and old numbered lists do not block docs-only PRs; `--min-id 1` is available for a strict whole-file audit once legacy collisions are cleaned up. **Verification:** `scripts/roadmap-check-ids.sh` passes on current ROADMAP; a temp copy with an appended duplicate `723.` fails nonzero and lists duplicate id 723 with line numbers. Source: Jobdori dogfood follow-up on origin/main `922c2398`, 2026-05-25. [SCOPE: docs/scripts] + +725. **DONE — roadmap-next-id helper now fails closed on helper-era duplicate ids before printing a next id** — follow-up to #724 after dogfood on origin/main 25ee5f3d showed `scripts/roadmap-next-id.sh` could print `1000` and exit 0 when a temp ROADMAP copy already contained two `999.` helper-era entries. This PR makes `roadmap-next-id.sh` resolve `roadmap-check-ids.sh` by its own script directory, run the checker with default helper-era min-id semantics before computing `highest+1`, keep stdout reserved for the single next id on success, and fail closed with a useful error if the checker is unavailable. Added focused pytest coverage for clean next-id output, duplicate fail-fast behavior, and missing-checker fail-closed behavior. **Verification:** `scripts/roadmap-next-id.sh ROADMAP.md` prints `725`; `scripts/roadmap-check-ids.sh ROADMAP.md` passes; a temp ROADMAP with duplicate `999.` exits nonzero and lists duplicate id 999 without printing a next id; `bash -n scripts/roadmap-next-id.sh scripts/roadmap-check-ids.sh` passes; `python -m pytest tests/test_roadmap_helpers.py -q` passes. Source: Jobdori dogfood follow-up on origin/main 25ee5f3d. [SCOPE: docs/scripts] diff --git a/scripts/roadmap-next-id.sh b/scripts/roadmap-next-id.sh index 4505aa69..3db4c49d 100755 --- a/scripts/roadmap-next-id.sh +++ b/scripts/roadmap-next-id.sh @@ -10,24 +10,47 @@ # ${NEXT}. **...description...** # EOF # -# The script reads the highest numeric id prefix from ROADMAP.md and -# prints highest+1. It does not lock the file; callers working in -# parallel should git-pull immediately before appending, run -# scripts/roadmap-check-ids.sh before push, and resolve any append -# collision at git-push time. +# The script first validates helper-era ids with roadmap-check-ids.sh, then +# reads the highest numeric id prefix from ROADMAP.md and prints highest+1. It +# does not lock the file; callers working in parallel should git-pull +# immediately before appending, run scripts/roadmap-check-ids.sh before push, +# and resolve any append collision at git-push time. set -euo pipefail ROADMAP="${1:-ROADMAP.md}" +SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P)" +CHECKER="$SCRIPT_DIR/roadmap-check-ids.sh" if [[ ! -f "$ROADMAP" ]]; then echo "error: ROADMAP not found at $ROADMAP" >&2 exit 1 fi -# Find the highest leading integer from lines that start with a number + '.'. -highest=$(grep -E '^[0-9]+\.' "$ROADMAP" | grep -Eo '^[0-9]+' | sort -n | tail -1) +if [[ ! -f "$CHECKER" || ! -r "$CHECKER" ]]; then + echo "error: required ROADMAP id checker not found or not readable at $CHECKER" >&2 + echo "error: refusing to print a next id without duplicate-id validation" >&2 + exit 1 +fi -if [[ -z "$highest" ]]; then +if ! checker_output="$(bash "$CHECKER" "$ROADMAP" 2>&1)"; then + printf '%s\n' "$checker_output" >&2 + exit 1 +fi + +# Find the highest leading integer from lines that start with a number + '.'. +highest=$(awk ' + /^[0-9]+\./ { + id = $0 + sub(/\..*/, "", id) + id += 0 + if (id > highest) { + highest = id + } + } + END { print highest + 0 } +' "$ROADMAP") + +if [[ "$highest" -eq 0 ]]; then echo 1 else echo $(( highest + 1 )) diff --git a/tests/test_roadmap_helpers.py b/tests/test_roadmap_helpers.py new file mode 100644 index 00000000..0169f88c --- /dev/null +++ b/tests/test_roadmap_helpers.py @@ -0,0 +1,67 @@ +from __future__ import annotations + +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parents[1] +NEXT_ID = REPO_ROOT / 'scripts' / 'roadmap-next-id.sh' + + +def run_next_id(roadmap: Path, script: Path = NEXT_ID) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ['bash', str(script), str(roadmap)], + cwd=REPO_ROOT, + capture_output=True, + text=True, + check=False, + ) + + +class RoadmapHelperTests(unittest.TestCase): + def test_roadmap_next_id_prints_only_next_id_after_duplicate_check(self) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + roadmap = Path(temp_dir) / 'ROADMAP.md' + roadmap.write_text('721. old\n723. helper era\n724. guard\n') + + result = run_next_id(roadmap) + + self.assertEqual(0, result.returncode) + self.assertEqual('725\n', result.stdout) + self.assertEqual('', result.stderr) + + def test_roadmap_next_id_fails_fast_on_helper_era_duplicate(self) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + roadmap = Path(temp_dir) / 'ROADMAP.md' + roadmap.write_text('722. legacy\n999. first\n999. duplicate\n') + + result = run_next_id(roadmap) + + self.assertNotEqual(0, result.returncode) + self.assertEqual('', result.stdout) + self.assertIn('duplicate ROADMAP numeric id(s)', result.stderr) + self.assertIn('999', result.stderr) + self.assertNotIn('1000', result.stdout) + + def test_roadmap_next_id_fails_closed_when_checker_is_unavailable(self) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + script_dir = Path(temp_dir) / 'scripts' + script_dir.mkdir() + copied_next_id = script_dir / 'roadmap-next-id.sh' + shutil.copy2(NEXT_ID, copied_next_id) + roadmap = Path(temp_dir) / 'ROADMAP.md' + roadmap.write_text('724. guard\n') + + result = run_next_id(roadmap, copied_next_id) + + self.assertNotEqual(0, result.returncode) + self.assertEqual('', result.stdout) + self.assertIn('required ROADMAP id checker not found or not readable', result.stderr) + self.assertIn('refusing to print a next id', result.stderr) + + +if __name__ == '__main__': + unittest.main()