diff --git a/.claude/scripts/cockpit.sh b/.claude/scripts/cockpit.sh index 72f9be6..e242845 100755 --- a/.claude/scripts/cockpit.sh +++ b/.claude/scripts/cockpit.sh @@ -22,6 +22,11 @@ # rows); an unfinished task silent for >COCKPIT_STALE_AFTER_SECONDS (default # 2h) is badged "stale" with muted rows instead of reading as active work. # +# Issue #173 adds a priority chip to each issue row when the owner has set a +# `priority:critical|high|medium|low` label (see priorityOf() below) — the +# same label set loop-census.sh's ADVANCE ordering uses. Unprioritized issues +# render with no chip. +# # Usage: # cockpit.sh [--fixtures ] [output-path] # cockpit.sh --parse-blocking @@ -455,6 +460,27 @@ function hasLabel(obj, name) { return (obj.labels || []).some((l) => l && l.name === name); } +// ---- Priority chip (issue #173) -------------------------------------------- +// The owner sets AT MOST one `priority:critical|high|medium|low` label per +// issue in the GitHub UI (loop-census.sh's ordering feature, same label +// set). priorityOf() surfaces that label for renderIssues() below, reusing +// the SAME .badge classes the rest of the dashboard already uses rather than +// inventing new ones: critical=bad (red), high=warn (amber), medium/low= +// muted (grey). Returns null (no chip) when the issue carries none of the +// four labels — an unprioritized issue stays visually quiet, not noisy. +const PRIORITY_LEVELS = [ + { name: "critical", cls: "bad" }, + { name: "high", cls: "warn" }, + { name: "medium", cls: "muted" }, + { name: "low", cls: "muted" }, +]; +function priorityOf(issue) { + for (const level of PRIORITY_LEVELS) { + if (hasLabel(issue, "priority:" + level.name)) return level; + } + return null; +} + // ---- Live worker progress section (issue #52, grouped by task in #92) ----- // Derive the CURRENT state per worker keyed by (role, task): keep the LATEST // event (by file order, i.e. append order) per key. No event log, or an @@ -870,6 +896,8 @@ function renderIssues() { for (const issue of list) { const edges = parseBlocking(issue.body || ""); html += `
  • #${issue.number} ${esc(issue.title)}`; + const priority = priorityOf(issue); + if (priority) html += ` priority: ${esc(priority.name)}`; const rel = []; if (edges.blockedBy.length) rel.push(`Blocked by ${refList(edges.blockedBy)}`); if (edges.blocks.length) rel.push(`Blocks ${refList(edges.blocks)}`); diff --git a/.claude/scripts/cockpit.test.sh b/.claude/scripts/cockpit.test.sh index cacd14a..d1a0cf0 100755 --- a/.claude/scripts/cockpit.test.sh +++ b/.claude/scripts/cockpit.test.sh @@ -105,7 +105,7 @@ check "parser finds only 'blocks' edge, no false blockedBy" node -e ' mkdir -p "$work/fixtures" cat > "$work/fixtures/issues.json" <<'EOF' [ - {"number":100,"title":"Issue A ","url":"https://example.com/100","labels":[{"name":"module:harness"}],"body":"Blocked by #101, #102\nBlocks #103\n- [ ] #104 subtask\n- [x] #105 done subtask"}, + {"number":100,"title":"Issue A ","url":"https://example.com/100","labels":[{"name":"module:harness"},{"name":"priority:critical"}],"body":"Blocked by #101, #102\nBlocks #103\n- [ ] #104 subtask\n- [x] #105 done subtask"}, {"number":101,"title":"Issue B","url":"https://example.com/101","labels":[{"name":"module:docs"}],"body":""}, {"number":103,"title":"Issue C","url":"https://example.com/103","labels":[],"body":""} ] @@ -166,6 +166,13 @@ check "blocked-by relationship rendered, linked to known issue #101" grep -qF 'B check "blocks relationship rendered, linked to known issue #103" grep -qF 'Blocks #103' "$html" check "subtasks (task-list refs) rendered" grep -qF 'Subtasks #104, #105' "$html" +# Priority chip (issue #173): issue 100 carries priority:critical -> renders +# a "bad"-classed chip on its row. Issue 101/103 carry no priority:* label -> +# no chip at all (unprioritized issues stay visually quiet). +check "prioritized issue #100 renders a priority:critical chip (badge bad)" grep -qF 'priority: critical' "$html" +check "only the one prioritized issue renders a priority chip (101/103 render none)" bash -c \ + '[ "$(grep -o "badge [a-z]*\">priority:" "$1" | wc -l)" -eq 1 ]' _ "$html" + # PRs with review + CI status. check "PRs section present" grep -q '
    branch= title= # in_flight=<n> one line PER planned issue that has a # feat/issue-n-* branch (local or remote) but NO @@ -46,9 +48,11 @@ # (issue #97). Only the first open blocker per # candidate is reported — one is enough to # explain the skip; a candidate may have more. -# advance_ready=<n|none> lowest-numbered planned issue with no branch, -# only when open_prs=0 (the ADVANCE precondition) -# AND not blocked by an open "Blocked by #N" edge +# advance_ready=<n|none> highest-priority, then lowest-numbered, planned +# issue with no branch, only when open_prs=0 (the +# ADVANCE precondition) AND not blocked by an +# open "Blocked by #N" edge — see PRIORITY +# ORDERING below (issue #173) # plan_wait=<n> one line per candidate that would otherwise be # advance_ready but is awaiting owner review of a # posted plan (labelled `plan-review`, no @@ -101,6 +105,19 @@ # `tail -1` — reported "No actionable activity" while ADVANCE work sat ready. # A tick may claim "No actionable activity" ONLY when this census prints zeros. # +# --- PRIORITY ORDERING (issue #173) ----------------------------------------- +# The planned-candidate TSV is ordered by (priority rank, issue number) before +# advance_ready/detail/blocking/in_flight ever iterate it. Priority rank comes +# straight from the labels already fetched (2nd TSV field) — critical=0, +# high=1, medium=2, low=3, and NO priority:* label ranks LAST (4). Issue +# number is the tiebreaker within a rank (and the sole ordering when every +# candidate is unlabeled), so a label-free backlog sorts identically to the +# pre-#173 `sort -n` and every downstream line (issue=/in_flight=/blocked=/ +# advance_ready=) is byte-identical to before this feature. Priority is +# PREFERENCE only: the blocking-graph gate below still overrides it — a +# blocked candidate is skipped regardless of how high its priority is; edges +# are semantics, priority is just iteration order among what's unblocked. +# # --- BLOCKING-GRAPH GATE (issue #97) ---------------------------------------- # advance_ready additionally skips any otherwise-eligible candidate (branch= # none, open_prs=0, no active driver) whose body contains an explicit @@ -352,9 +369,27 @@ echo "comment_fix_prs=$comment_fix_prs" rebase_prs=$(bash "$script_dir/pr-rebase.sh" "$repo" | grep -c . || true) echo "rebase_prs=$rebase_prs" -# Open `planned` issues carrying any of the adapter's module labels, ascending. +# Open `planned` issues carrying any of the adapter's module labels, ordered +# by (priority rank, issue number) — see PRIORITY ORDERING above (issue +# #173). Rank is derived from the labels field already in the TSV (no extra +# gh call): prepend a rank column, sort numerically on (rank, number), then +# strip the rank column back off so the downstream `while IFS=$'\t' read -r +# num labels title` loop is unchanged. Titles are the LAST TSV field (may +# contain spaces) and are left untouched by this transform. planned=$(gh issue list -R "$repo" --state open --label planned --json number,title,labels \ - --jq '.[] | [.number, ([.labels[].name]|join(",")), .title] | @tsv' | sort -n) + --jq '.[] | [.number, ([.labels[].name]|join(",")), .title] | @tsv' \ + | awk -F'\t' 'BEGIN { OFS = "\t" } + { + labels = $2 + rank = 4 + if (labels ~ /(^|,)priority:critical(,|$)/) rank = 0 + else if (labels ~ /(^|,)priority:high(,|$)/) rank = 1 + else if (labels ~ /(^|,)priority:medium(,|$)/) rank = 2 + else if (labels ~ /(^|,)priority:low(,|$)/) rank = 3 + print rank OFS $0 + }' \ + | sort -t $'\t' -k1,1n -k2,2n \ + | cut -f2-) planned_count=0 advance_ready="none" @@ -456,8 +491,9 @@ while IFS=$'\t' read -r num labels title; do fi fi - # fallback_ready: lowest-numbered otherwise-eligible candidate, IGNORING the - # blocking-graph gate — used only if the gate leaves advance_ready="none". + # fallback_ready: first otherwise-eligible candidate in (priority, number) + # order, IGNORING the blocking-graph gate — used only if the gate leaves + # advance_ready="none". if [ "$eligible" -eq 1 ] && [ "$fallback_ready" = "none" ]; then fallback_ready="$num" fallback_plan_state="$plan_state" diff --git a/.claude/scripts/loop-census.test.sh b/.claude/scripts/loop-census.test.sh index 67d32a2..a56e93c 100644 --- a/.claude/scripts/loop-census.test.sh +++ b/.claude/scripts/loop-census.test.sh @@ -637,6 +637,196 @@ check "(d) task-list ref alone does not gate — advance_ready=40" bash -c \ check "(d) no blocked= line emitted for a task-list-only reference" bash -c \ '! printf "%s\n" "$1" | grep -q "^blocked="' _ "$outTasklist" +# --------------------------------------------------------------------------- +# Priority-label ordering (issue #173): candidates are ordered by (priority +# rank, then issue number) BEFORE advance_ready/detail/blocking iterate them — +# critical=0, high=1, medium=2, low=3, unlabeled=4 (last), number is the +# tiebreaker. Three fixtures, one per acceptance scenario. None of these need +# cockpit.sh: every candidate's stubbed issue body is empty, so the +# --parse-blocking shell-out never fires except in the (c) blocked fixture +# below, which copies cockpit.sh in exactly like the blocking-graph fixtures +# above. +# --------------------------------------------------------------------------- +build_plain_fixture() { + # $1 = dir. Same shape as build_guard_fixture above (no branches, no open + # PRs) but factored out so the priority fixtures below can each supply + # their own bot-gh.sh issue-list body. + local dir="$1" + local scripts="$dir/.claude/scripts" + mkdir -p "$scripts" + cp "$census_src" "$scripts/loop-census.sh" + cp "$resolve_roots_src" "$scripts/resolve-roots.sh" + cat > "$dir/.claude/gates.json" <<'EOF' +{ + "modules": [{ "name": "test", "path": ".", "description": "", "owner": "" }], + "merge": { "baseBranch": "main" } +} +EOF + for stub in pr-feedback pr-ci-fix pr-comment-fix pr-rebase; do + cat > "$scripts/$stub.sh" <<'EOF' +#!/usr/bin/env bash +exit 0 +EOF + done + chmod +x "$scripts"/*.sh + git -C "$dir" init -q -b main + git -C "$dir" -c user.email=t@e.st -c user.name=t commit -q --allow-empty -m init +} + +# --- (a) priority beats number: issue 50 (priority:critical) must become +# advance_ready over issue 3 (unlabeled, lower number) -- pure number order +# would pick 3 first. --- +dirPrioNum="$work/prio-beats-number" +build_plain_fixture "$dirPrioNum" +cat > "$dirPrioNum/.claude/scripts/bot-gh.sh" <<'EOF' +#!/usr/bin/env bash +case "$1" in + repo) echo "acme/repo" ;; + pr) + if printf '%s\n' "$*" | grep -q 'headRefName'; then + : # no open PRs + else + echo 0 + fi + ;; + issue) + case "$2" in + list) + if printf '%s\n' "$*" | grep -q -- '--label'; then + printf '3\tplanned,module:test\tLow-priority-in-number-order candidate\n' + printf '50\tplanned,module:test,priority:critical\tHigh-priority higher-numbered candidate\n' + else + printf '3\n50\n' + fi + ;; + view) echo '{"body":""}' ;; + *) echo "unhandled issue subcmd: $*" >&2; exit 1 ;; + esac + ;; + *) echo "fake-bot-gh.sh: unhandled args: $*" >&2; exit 1 ;; +esac +EOF +chmod +x "$dirPrioNum/.claude/scripts/bot-gh.sh" +outPrioNum="$(env -u GATES_FILE bash "$dirPrioNum/.claude/scripts/loop-census.sh" "acme/repo")" + +check "(a) priority-critical issue 50 becomes advance_ready over lower-numbered unlabeled issue 3" bash -c \ + 'printf "%s\n" "$1" | grep -qx "advance_ready=50"' _ "$outPrioNum" +check "(a) issue 50 (critical) is iterated/detailed BEFORE issue 3 (unlabeled)" bash -c ' + printf "%s\n" "$1" | grep -n "^issue=" | sort -t: -k1,1n | head -1 | grep -q "issue=50 " +' _ "$outPrioNum" + +# --- (b) unlabeled-last: issue 5 (unlabeled, LOWER number) and issue 6 +# (priority:low, HIGHER number) are BOTH otherwise-eligible (no branch, no +# open PR, not blocked) -- priority:low still outranks unlabeled despite +# 6 > 5, so issue 6 must be iterated first and win advance_ready. A plain +# `sort -n` revert would pick issue 5 first instead, so this genuinely +# discriminates (unlike a shape where the lower-numbered candidate is made +# ineligible, which would pass either way). --- +dirUnlabeledLast="$work/unlabeled-last" +build_plain_fixture "$dirUnlabeledLast" +cat > "$dirUnlabeledLast/.claude/scripts/bot-gh.sh" <<'EOF' +#!/usr/bin/env bash +case "$1" in + repo) echo "acme/repo" ;; + pr) + if printf '%s\n' "$*" | grep -q 'headRefName'; then + : # no open PRs + else + echo 0 + fi + ;; + issue) + case "$2" in + list) + if printf '%s\n' "$*" | grep -q -- '--label'; then + printf '5\tplanned,module:test\tUnlabeled lower-numbered candidate\n' + printf '6\tplanned,module:test,priority:low\tLabeled higher-numbered candidate\n' + else + printf '5\n6\n' + fi + ;; + view) echo '{"body":""}' ;; + *) echo "unhandled issue subcmd: $*" >&2; exit 1 ;; + esac + ;; + *) echo "fake-bot-gh.sh: unhandled args: $*" >&2; exit 1 ;; +esac +EOF +chmod +x "$dirUnlabeledLast/.claude/scripts/bot-gh.sh" +# Neither issue has a branch, an open PR, or a blocker -- both are fully +# eligible, so the only thing that can decide the outcome is priority order. +outUnlabeledLast="$(env -u GATES_FILE bash "$dirUnlabeledLast/.claude/scripts/loop-census.sh" "acme/repo")" + +check "(b) issue 6 (priority:low) is iterated before issue 5 (unlabeled)" bash -c ' + printf "%s\n" "$1" | grep -n "^issue=" | sort -t: -k1,1n | head -1 | grep -q "issue=6 " +' _ "$outUnlabeledLast" +check "(b) priority:low issue 6 becomes advance_ready over lower-numbered unlabeled issue 5" bash -c \ + 'printf "%s\n" "$1" | grep -qx "advance_ready=6"' _ "$outUnlabeledLast" +check "(b) unlabeled issue 5 (otherwise eligible) is NOT advance_ready" bash -c \ + '! printf "%s\n" "$1" | grep -qx "advance_ready=5"' _ "$outUnlabeledLast" +check "(b) unlabeled issue 5 is not in_flight either (genuinely eligible, just outranked)" bash -c \ + '! printf "%s\n" "$1" | grep -qx "in_flight=5"' _ "$outUnlabeledLast" + +# --- (c) blocked-high-priority skipped for unblocked-lower: issue 70 +# (priority:high, HIGHER number) is "Blocked by #99" (still OPEN) -> skipped, +# emits blocked=70 by=99; issue 8 (unlabeled, unblocked, LOWER number) +# becomes advance_ready instead, even though 70 outranks it on priority. +# Because 70 > 8, priority order and plain numeric order disagree on +# iteration order (priority puts 70 first; `sort -n` would put 8 first), +# so this genuinely discriminates a `sort -n` revert. Reuses the real +# cockpit.sh --parse-blocking seam, exactly like the blocking-graph fixtures +# above (scaffold_blocking_fixture copies it in verbatim). --- +dirBlockedPrio="$work/blocked-priority" +scaffold_blocking_fixture "$dirBlockedPrio" +cat > "$dirBlockedPrio/.claude/scripts/bot-gh.sh" <<'EOF' +#!/usr/bin/env bash +case "$1" in + repo) echo "acme/repo" ;; + pr) + if printf '%s\n' "$*" | grep -q 'headRefName'; then + : # no open PRs + else + echo 0 + fi + ;; + issue) + case "$2" in + list) + if printf '%s\n' "$*" | grep -q -- '--label'; then + printf '70\tplanned,module:test,priority:high\tBlocked high-priority candidate\n' + printf '8\tplanned,module:test\tUnblocked lower-priority candidate\n' + else + # all-open-issue-numbers fetch: 99 (the blocker) is still OPEN. + printf '70\n8\n99\n' + fi + ;; + view) + case "$3" in + 70) echo '{"body":"Blocked by #99"}' ;; + 8) echo '{"body":""}' ;; + *) echo '{"body":""}' ;; + esac + ;; + *) echo "unhandled issue subcmd: $*" >&2; exit 1 ;; + esac + ;; + *) echo "fake-bot-gh.sh: unhandled args: $*" >&2; exit 1 ;; +esac +EOF +chmod +x "$dirBlockedPrio/.claude/scripts/bot-gh.sh" +errBlockedPrio="$work/blocked-priority.stderr" +outBlockedPrio="$(run_blocking_fixture "$dirBlockedPrio" "$errBlockedPrio")" + +check "(c) issue 70 (priority:high, iterated first) is iterated before issue 8 (unlabeled)" bash -c ' + printf "%s\n" "$1" | grep -n "^issue=" | sort -t: -k1,1n | head -1 | grep -q "issue=70 " +' _ "$outBlockedPrio" +check "(c) blocked=70 by=99 census line emitted (higher-priority candidate skipped)" bash -c \ + 'printf "%s\n" "$1" | grep -qx "blocked=70 by=99"' _ "$outBlockedPrio" +check "(c) issue 70 (blocked) is NOT advance_ready despite outranking issue 8 on priority" bash -c \ + '! printf "%s\n" "$1" | grep -qx "advance_ready=70"' _ "$outBlockedPrio" +check "(c) advance_ready instead picks the unblocked lower-priority issue 8" bash -c \ + 'printf "%s\n" "$1" | grep -qx "advance_ready=8"' _ "$outBlockedPrio" + # --------------------------------------------------------------------------- # Stall detection (issue #98): loop-census.sh must emit `stalled=N age_min=M` # for an in_flight issue whose newest events.jsonl activity (task field either diff --git a/docs/USAGE.md b/docs/USAGE.md index 1c534c7..5ca1906 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -353,16 +353,25 @@ decide what the loop actually touches: - **`module:<name>`** — routing, not approval. It maps issue → module → the worker's `path` boundary (`gates.json.modules[]`). No module ⇒ no boundary ⇒ nothing safe to hand a worker, `planned` or not. - The ADVANCE step picks the **lowest-numbered open issue labelled `planned` + `module:*`** with no - existing `feat/issue-<n>-*` branch — one at a time, and only when there are zero open PRs. Tracking - issues (plans split into `Blocked by` sub-issue chains) stay `backlog` forever so the loop works the - chain, never the tracker. **"Blocked by #N" in an issue body is load-bearing for this selection, not - merely cosmetic for the cockpit graph** (`loop-census.sh`, issue #97): the census skips a `planned` - candidate while any issue it declares "Blocked by" is still open, emits a `blocked=<n> by=<N>` line - explaining the skip, and picks the next unblocked lowest-numbered candidate instead — a blocker - closing makes the skipped issue eligible again on the very next tick, no extra bookkeeping required. - A "Blocked by" cycle falls back to the lowest-numbered candidate rather than wedging the loop. Only - the explicit "Blocked by" phrase gates; task-list/parent-child refs (`- [ ] #N`) do not. + The ADVANCE step picks the **highest-priority, then lowest-numbered, open issue labelled + `planned` + `module:*`** with no existing `feat/issue-<n>-*` branch — one at a time, and only when + there are zero open PRs. Tracking issues (plans split into `Blocked by` sub-issue chains) stay + `backlog` forever so the loop works the chain, never the tracker. **"Blocked by #N" in an issue body + is load-bearing for this selection, not merely cosmetic for the cockpit graph** (`loop-census.sh`, + issue #97): the census skips a `planned` candidate while any issue it declares "Blocked by" is still + open, emits a `blocked=<n> by=<N>` line explaining the skip, and picks the next unblocked candidate + instead — a blocker closing makes the skipped issue eligible again on the very next tick, no extra + bookkeeping required. A "Blocked by" cycle falls back to the lowest-numbered candidate rather than + wedging the loop. Only the explicit "Blocked by" phrase gates; task-list/parent-child refs (`- [ ] #N`) + do not. + - **Priority labels (`priority:critical|high|medium|low`, issue #173).** The owner sets one of these + four labels in the GitHub UI on any `planned` issue; nothing else about the workflow changes. The + census orders candidates by (priority rank, then issue number): `critical` < `high` < `medium` < + `low` < unlabeled, with issue number as the tiebreaker within a rank — so a label-free backlog + behaves exactly as before (lowest-numbered first). The blocking-graph gate above still overrides + priority: a blocked candidate is skipped regardless of how high its priority is — priority is + preference, "Blocked by" edges are semantics. The cockpit shows a priority chip on each prioritized + issue row. - **Owner-approval merge gate.** Workers author PRs as the **bot** (`bot-gh.sh`); the MERGE step (above) only merges PRs the repo **owner** has Approved on GitHub that are CI-green and mergeable. It never approves on the owner's behalf.