mirror of
https://github.com/instructkr/claude-code.git
synced 2026-05-27 16:06:44 +00:00
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.
This commit is contained in:
11
.github/hooks/pre-push
vendored
11
.github/hooks/pre-push
vendored
@@ -8,14 +8,19 @@
|
|||||||
# caught before pushing to main or PR branches.
|
# caught before pushing to main or PR branches.
|
||||||
set -euo pipefail
|
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
|
if [[ "${SKIP_CLAW_PRE_PUSH_BUILD:-}" == "1" ]]; then
|
||||||
echo "pre-push: SKIP_CLAW_PRE_PUSH_BUILD=1 set; skipping cargo workspace build" >&2
|
echo "pre-push: SKIP_CLAW_PRE_PUSH_BUILD=1 set; skipping cargo workspace build" >&2
|
||||||
exit 0
|
exit 0
|
||||||
fi
|
fi
|
||||||
|
|
||||||
repo_root="$(git rev-parse --show-toplevel 2>/dev/null)"
|
|
||||||
cd "$repo_root"
|
|
||||||
|
|
||||||
if [[ ! -f rust/Cargo.toml ]]; then
|
if [[ ! -f rust/Cargo.toml ]]; then
|
||||||
echo "pre-push: rust/Cargo.toml not found; skipping cargo workspace build" >&2
|
echo "pre-push: rust/Cargo.toml not found; skipping cargo workspace build" >&2
|
||||||
exit 0
|
exit 0
|
||||||
|
|||||||
4
.github/workflows/rust-ci.yml
vendored
4
.github/workflows/rust-ci.yml
vendored
@@ -21,6 +21,7 @@ on:
|
|||||||
- PARITY.md
|
- PARITY.md
|
||||||
- PHILOSOPHY.md
|
- PHILOSOPHY.md
|
||||||
- ROADMAP.md
|
- ROADMAP.md
|
||||||
|
- scripts/roadmap-*.sh
|
||||||
- docs/**
|
- docs/**
|
||||||
- rust/**
|
- rust/**
|
||||||
pull_request:
|
pull_request:
|
||||||
@@ -41,6 +42,7 @@ on:
|
|||||||
- PARITY.md
|
- PARITY.md
|
||||||
- PHILOSOPHY.md
|
- PHILOSOPHY.md
|
||||||
- ROADMAP.md
|
- ROADMAP.md
|
||||||
|
- scripts/roadmap-*.sh
|
||||||
- docs/**
|
- docs/**
|
||||||
- rust/**
|
- rust/**
|
||||||
workflow_dispatch:
|
workflow_dispatch:
|
||||||
@@ -72,6 +74,8 @@ jobs:
|
|||||||
run: python .github/scripts/check_doc_source_of_truth.py
|
run: python .github/scripts/check_doc_source_of_truth.py
|
||||||
- name: Check release policy docs and local links
|
- name: Check release policy docs and local links
|
||||||
run: python .github/scripts/check_release_readiness.py
|
run: python .github/scripts/check_release_readiness.py
|
||||||
|
- name: Check ROADMAP ids
|
||||||
|
run: scripts/roadmap-check-ids.sh
|
||||||
|
|
||||||
fmt:
|
fmt:
|
||||||
name: cargo fmt
|
name: cargo fmt
|
||||||
|
|||||||
@@ -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
|
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
|
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
|
runs the ROADMAP id guard, then runs
|
||||||
repository root. If you must bypass it for a non-code/docs-only push, set
|
`cargo build --manifest-path rust/Cargo.toml --workspace --locked` from the
|
||||||
`SKIP_CLAW_PRE_PUSH_BUILD=1`; the hook prints when that escape hatch is used.
|
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
|
## Checks before opening a pull request
|
||||||
|
|
||||||
|
|||||||
@@ -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.
|
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 <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.
|
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]
|
||||||
|
|||||||
72
scripts/roadmap-check-ids.sh
Executable file
72
scripts/roadmap-check-ids.sh
Executable file
@@ -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"
|
||||||
@@ -12,8 +12,9 @@
|
|||||||
#
|
#
|
||||||
# The script reads the highest numeric id prefix from ROADMAP.md and
|
# The script reads the highest numeric id prefix from ROADMAP.md and
|
||||||
# prints highest+1. It does not lock the file; callers working in
|
# prints highest+1. It does not lock the file; callers working in
|
||||||
# parallel should git-pull immediately before appending and resolve
|
# parallel should git-pull immediately before appending, run
|
||||||
# any append collision at git-push time.
|
# scripts/roadmap-check-ids.sh before push, and resolve any append
|
||||||
|
# collision at git-push time.
|
||||||
set -euo pipefail
|
set -euo pipefail
|
||||||
|
|
||||||
ROADMAP="${1:-ROADMAP.md}"
|
ROADMAP="${1:-ROADMAP.md}"
|
||||||
|
|||||||
Reference in New Issue
Block a user