From 25ee5f3d30f4ab44a1aa5c1ebd072f7262f0ae42 Mon Sep 17 00:00:00 2001 From: Bellman <54757707+Yeachan-Heo@users.noreply.github.com> Date: Tue, 26 May 2026 08:49:23 +0900 Subject: [PATCH] Prevent helper-era ROADMAP id collisions before review (#3115) Add a lightweight ROADMAP duplicate-id guard and wire it into the low-risk docs/pre-push paths so optimistic append collisions introduced after the next-id helper are caught before merge. Constraint: Current ROADMAP contains legacy numbered lists and pre-helper duplicate low ids, so the default guard checks helper-era ids >=723 while preserving --min-id 1 for a future strict audit. Rejected: Fail CI on every numeric duplicate in the whole historical ROADMAP | current main would fail before this PR because old prose/list numbering is already duplicated. Confidence: high Scope-risk: narrow Directive: Keep roadmap-next-id.sh paired with roadmap-check-ids.sh when changing ROADMAP append workflows. Tested: bash -n scripts/roadmap-check-ids.sh scripts/roadmap-next-id.sh .github/hooks/pre-push; scripts/roadmap-check-ids.sh; temp ROADMAP copy with duplicate 723 failed nonzero and listed id 723; SKIP_CLAW_PRE_PUSH_BUILD=1 .github/hooks/pre-push; git diff --check; python3 .github/scripts/check_doc_source_of_truth.py; python3 .github/scripts/check_release_readiness.py Not-tested: full cargo workspace build/test because this is docs/scripts-only and the local pre-push cargo build was smoke-tested with its documented skip path. --- .github/hooks/pre-push | 11 ++++-- .github/workflows/rust-ci.yml | 4 ++ CONTRIBUTING.md | 27 +++++++++++-- ROADMAP.md | 2 + scripts/roadmap-check-ids.sh | 72 +++++++++++++++++++++++++++++++++++ scripts/roadmap-next-id.sh | 5 ++- 6 files changed, 113 insertions(+), 8 deletions(-) create mode 100755 scripts/roadmap-check-ids.sh diff --git a/.github/hooks/pre-push b/.github/hooks/pre-push index c3644a31..de2b9259 100755 --- a/.github/hooks/pre-push +++ b/.github/hooks/pre-push @@ -8,14 +8,19 @@ # caught before pushing to main or PR branches. set -euo pipefail +repo_root="$(git rev-parse --show-toplevel 2>/dev/null)" +cd "$repo_root" + +if [[ -x scripts/roadmap-check-ids.sh ]]; then + echo "pre-push: scripts/roadmap-check-ids.sh" >&2 + scripts/roadmap-check-ids.sh +fi + if [[ "${SKIP_CLAW_PRE_PUSH_BUILD:-}" == "1" ]]; then echo "pre-push: SKIP_CLAW_PRE_PUSH_BUILD=1 set; skipping cargo workspace build" >&2 exit 0 fi -repo_root="$(git rev-parse --show-toplevel 2>/dev/null)" -cd "$repo_root" - if [[ ! -f rust/Cargo.toml ]]; then echo "pre-push: rust/Cargo.toml not found; skipping cargo workspace build" >&2 exit 0 diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index a316aec7..b12c5e5c 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -21,6 +21,7 @@ on: - PARITY.md - PHILOSOPHY.md - ROADMAP.md + - scripts/roadmap-*.sh - docs/** - rust/** pull_request: @@ -41,6 +42,7 @@ on: - PARITY.md - PHILOSOPHY.md - ROADMAP.md + - scripts/roadmap-*.sh - docs/** - rust/** workflow_dispatch: @@ -72,6 +74,8 @@ jobs: run: python .github/scripts/check_doc_source_of_truth.py - name: Check release policy docs and local links run: python .github/scripts/check_release_readiness.py + - name: Check ROADMAP ids + run: scripts/roadmap-check-ids.sh fmt: name: cargo fmt diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0f191be2..bb2c4f72 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -43,9 +43,30 @@ git config core.hooksPath .github/hooks This sets the repo's Git hook directory to `.github/hooks`; if you already use a custom `core.hooksPath`, copy or chain `.github/hooks/pre-push` instead. The hook -runs `cargo build --manifest-path rust/Cargo.toml --workspace --locked` from the -repository root. If you must bypass it for a non-code/docs-only push, set -`SKIP_CLAW_PRE_PUSH_BUILD=1`; the hook prints when that escape hatch is used. +runs the ROADMAP id guard, then runs +`cargo build --manifest-path rust/Cargo.toml --workspace --locked` from the +repository root. If you must bypass the cargo build for a docs-only push, set +`SKIP_CLAW_PRE_PUSH_BUILD=1`; the hook still runs the ROADMAP guard and prints +when the cargo-build escape hatch is used. + +## ROADMAP id allocation + +Before appending a new numeric ROADMAP entry, pull/rebase onto the latest +`main`, allocate the id from the file you are about to edit, and run the duplicate +id guard before pushing: + +```bash +git pull --rebase +NEXT=$(scripts/roadmap-next-id.sh) +# append "${NEXT}. **...**" to ROADMAP.md +scripts/roadmap-check-ids.sh +``` + +The duplicate guard currently checks helper-era ids (`>=723`) by default so it +catches new optimistic-append collisions without failing on legacy numbered lists +already present in the historical roadmap. Use `scripts/roadmap-check-ids.sh +--min-id 1` for a strict whole-file audit after those legacy collisions are +cleaned up. ## Checks before opening a pull request diff --git a/ROADMAP.md b/ROADMAP.md index e68f0ac1..f44ea0a8 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -7613,3 +7613,5 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed) 722. **ROADMAP #721 re-entry after rebase conflict: `claw config mcp|sandbox|permissions|skills|agents` returned `unsupported_config_section` — code fix is in `6e44da10`** (main.rs changes preserved through rebase, only ROADMAP.md was conflict-resolved to Gaebal's version). Both text and JSON config section handlers now support `mcp`, `sandbox`, `permissions`, `skills`, `agents`; error envelope includes `supported_sections:[]`. Source: Jobdori dogfood on `02d1f6a0`, 2026-05-26. 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] diff --git a/scripts/roadmap-check-ids.sh b/scripts/roadmap-check-ids.sh new file mode 100755 index 00000000..e2cdd5aa --- /dev/null +++ b/scripts/roadmap-check-ids.sh @@ -0,0 +1,72 @@ +#!/usr/bin/env bash +# roadmap-check-ids.sh — fail when helper-era ROADMAP item ids are duplicated. +# Usage: scripts/roadmap-check-ids.sh [--min-id N] [path/to/ROADMAP.md] +# +# By default this validates ids >= 723, the point where ROADMAP appends started +# using scripts/roadmap-next-id.sh. Earlier ROADMAP content contains historical +# numbered lists and already-landed duplicate low ids, so the default guard is +# intentionally scoped to new helper-era append collisions. Use --min-id 1 for a +# strict whole-file audit after legacy numbering is cleaned up. +set -euo pipefail + +MIN_ID=723 +ROADMAP="ROADMAP.md" + +while [[ $# -gt 0 ]]; do + case "$1" in + --min-id) + if [[ $# -lt 2 || ! "$2" =~ ^[0-9]+$ ]]; then + echo "error: --min-id requires a non-negative integer" >&2 + exit 2 + fi + MIN_ID="$2" + shift 2 + ;; + --help|-h) + sed -n '2,9p' "$0" | sed 's/^# //; s/^#//' + exit 0 + ;; + --*) + echo "error: unknown option: $1" >&2 + exit 2 + ;; + *) + ROADMAP="$1" + shift + ;; + esac +done + +if [[ ! -f "$ROADMAP" ]]; then + echo "error: ROADMAP not found at $ROADMAP" >&2 + exit 1 +fi + +awk -v min_id="$MIN_ID" -v path="$ROADMAP" ' + /^[0-9]+\./ { + id = $0 + sub(/\..*/, "", id) + id += 0 + if (id >= min_id) { + count[id]++ + lines[id] = lines[id] (lines[id] ? ", " : "") FNR + } + } + END { + for (id in count) { + if (count[id] > 1) { + duplicate_count++ + duplicate_ids[duplicate_count] = id + } + } + if (duplicate_count) { + print "error: duplicate ROADMAP numeric id(s) in " path " (min id " min_id "):" > "/dev/stderr" + for (i = 1; i <= duplicate_count; i++) { + id = duplicate_ids[i] + print " - " id " at line(s) " lines[id] > "/dev/stderr" + } + exit 1 + } + print "roadmap id check passed: no duplicate ids >= " min_id " in " path + } +' "$ROADMAP" diff --git a/scripts/roadmap-next-id.sh b/scripts/roadmap-next-id.sh index 07f139bb..4505aa69 100755 --- a/scripts/roadmap-next-id.sh +++ b/scripts/roadmap-next-id.sh @@ -12,8 +12,9 @@ # # 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 and resolve -# any append collision at git-push time. +# 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}"