Compare commits

...

2 Commits

Author SHA1 Message Date
Yeachan-Heo dd12e49f2f Protect ROADMAP helper missing-path behavior
Add the missing explicit-path regression so operators keep the current fail-fast contract covered, and record the completed ROADMAP follow-up.

Constraint: scope limited to ROADMAP helper regression coverage plus the DONE roadmap entry; tests/__init__.py is included so the requested python -m unittest tests.test_roadmap_helpers invocation resolves locally instead of an external tests package.

Rejected: changing roadmap-next-id.sh behavior | dogfood showed the script already returns the desired nonzero empty-stdout error.

Confidence: high

Scope-risk: narrow

Directive: Keep stdout reserved for the single next id on successful roadmap-next-id.sh runs.

Tested: python -m unittest tests.test_roadmap_helpers -q; scripts/roadmap-check-ids.sh ROADMAP.md; scripts/roadmap-next-id.sh ROADMAP.md; git diff --check -- ROADMAP.md tests/test_roadmap_helpers.py tests/__init__.py
2026-05-26 00:33:06 +00:00
Bellman 49d5b3fcdc Prevent poisoned ROADMAP ids before allocation (#3116)
Constraint: roadmap-next-id.sh must preserve single-id stdout on success while failing closed if duplicate validation cannot run.
Rejected: Relying only on CI/pre-push duplicate checks | the helper is used immediately before appending and must not certify an already-poisoned file.
Confidence: high
Scope-risk: narrow
Directive: Keep roadmap-next-id.sh stdout machine-clean; route validation failures and checker availability errors to stderr, and keep focused helper behavior coverage in the docs/ROADMAP CI path.
Tested: scripts/roadmap-next-id.sh ROADMAP.md printed 725 before appending #725 and 726 after; temp ROADMAP with duplicate 999 exited nonzero and listed duplicate id; scripts/roadmap-check-ids.sh ROADMAP.md; bash -n scripts/roadmap-next-id.sh scripts/roadmap-check-ids.sh; python -m unittest discover -s tests -p test_roadmap_helpers.py; python -m pytest tests/test_roadmap_helpers.py -q; SKIP_CLAW_PRE_PUSH_BUILD=1 bash .github/hooks/pre-push
Not-tested: full cargo workspace build, unchanged docs/script-only path
2026-05-26 09:10:02 +09:00
5 changed files with 117 additions and 8 deletions
+4
View File
@@ -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
+4
View File
@@ -7615,3 +7615,7 @@ 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 <filter>` as #719 on main first; Gaebal shifted to #720; Jobdori landed `claw help <topic>` 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]
726. **DONE — roadmap-next-id helper missing explicit ROADMAP path behavior is regression-tested** — follow-up to #725 after dogfood on origin/main 49d5b3fc showed the helper already failed correctly for `scripts/roadmap-next-id.sh /tmp/nonexistent-roadmap` but tests only covered clean next-id output, duplicate fail-fast behavior, and missing-checker fail-closed behavior. This PR adds focused unittest coverage proving a missing explicit ROADMAP path exits nonzero, keeps stdout empty, and reports both `ROADMAP not found` and the requested path on stderr. **Verification:** `python -m unittest tests.test_roadmap_helpers -q`; `scripts/roadmap-check-ids.sh ROADMAP.md`; `scripts/roadmap-next-id.sh ROADMAP.md`. Source: Jobdori dogfood follow-up on origin/main 49d5b3fc. [SCOPE: docs/scripts]
+31 -8
View File
@@ -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 ))
View File
+78
View File
@@ -0,0 +1,78 @@
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_when_explicit_roadmap_path_is_missing(self) -> None:
with tempfile.TemporaryDirectory() as temp_dir:
roadmap = Path(temp_dir) / 'missing-ROADMAP.md'
result = run_next_id(roadmap)
self.assertNotEqual(0, result.returncode)
self.assertEqual('', result.stdout)
self.assertIn('ROADMAP not found', result.stderr)
self.assertIn(str(roadmap), result.stderr)
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()