From e364e0a3d1d6ed9c6fb16313b3e500bd9e673b07 Mon Sep 17 00:00:00 2001 From: Paul Gebheim <86010+pgebheim@users.noreply.github.com> Date: Sun, 9 Aug 2026 06:04:40 +0000 Subject: [PATCH] fix(tracker): scan candidates independent of --limit; validate --status column (#66, #67) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit verb_select pulled both candidate sets — the label queries and the board column — with the result --limit, then intersected. So --limit meant "consider N candidates", not "return N results", and `next` (hardcoded --limit 1) fetched one issue per shape label and one board item, whose intersection is empty unless that single board item happens to be the dispatchable one. Real boards returned [] with dispatchable work sitting in Todo. (#66) Decouple the two: pull SCAN_DEPTH (default 500, RIG_TRACKER_SCAN-overridable) rows from each source, intersect, THEN return the first . --limit is now purely the result count. Separately, `select --status ` used the status only as a client-side jq equality filter, so a column name that isn't on the board (a typo, or drift after someone renames a Projects column) returned [] with exit 0 — indistinguishable from "no work in that column". Validate the name against the field's real options first and die otherwise, the same resolution set-status already uses for writes. A valid-but-empty column still returns [] / exit 0. (#67) Tests: the mock gh now honors --limit and answers project field-list, and the board returns a non-Todo item first so the truncation is observable. Added a #66 regression (next finds the dispatchable item though it isn't first) and #67 cases (unknown column errors; valid-but-empty column is a clean []). Verified these 3 assertions fail on the pre-fix script and pass after. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0157AoJ6PVpKyt5V8GCVrodf --- scripts/rig-tracker.sh | 35 +++++++++++++++++++++-- scripts/rig-tracker.test.ts | 55 ++++++++++++++++++++++++++++++++----- 2 files changed, 81 insertions(+), 9 deletions(-) diff --git a/scripts/rig-tracker.sh b/scripts/rig-tracker.sh index 9bca36a..2ffaebd 100755 --- a/scripts/rig-tracker.sh +++ b/scripts/rig-tracker.sh @@ -57,6 +57,12 @@ set -euo pipefail GH="${RIG_TRACKER_GH:-gh}" JQ="${RIG_TRACKER_JQ:-jq}" +# Candidate scan depth: how many rows to pull from each source (label queries and +# the board column) BEFORE intersecting — deliberately independent of the result +# --limit, so a small --limit (e.g. next's 1) can't truncate a source below the +# overlap (see #66). Overridable for huge boards / tests. +SCAN_DEPTH="${RIG_TRACKER_SCAN:-500}" + die() { echo "rig-tracker: $*" >&2; exit 1; } warn() { echo "rig-tracker: $*" >&2; } @@ -140,6 +146,23 @@ gh_issue_numbers_by_status() { '.items[] | select((.[$k] // "") == $s) | .content.number // empty' } +# Valid option names of the board's Status field (one per line). Same source +# set-status resolves option ids from, so read validation matches write behavior. +gh_status_option_names() { + require_board + "$GH" project field-list "$B_NUM" --owner "$B_OWNER" --format json \ + | "$JQ" -r --arg f "$B_FIELD" '.fields[] | select(.name==$f) | .options[]?.name' +} + +# Die unless is an actual column (Status option) on the board. Prevents a +# typo'd / renamed column from silently reading as "no work here" (see #67). +assert_valid_status() { + local status="$1" names + names="$(gh_status_option_names)" + printf '%s\n' "$names" | grep -qxF -- "$status" && return 0 + die "status '$status' is not a column on Project $B_OWNER/#$B_NUM (field '$B_FIELD'); valid: $(printf '%s' "$names" | tr '\n' ' ')" +} + # ---- verbs ------------------------------------------------------------------ verb_select() { local status="" dispatchable=0 limit=50; local -a labels=() @@ -167,15 +190,23 @@ verb_select() { [ -n "$status" ] || die "--dispatchable needs tracker.board.statusOptions.todo set" fi + # Validate the board column up front (#67): an unknown/typo'd status must fail + # loudly rather than silently intersect to [] (which reads as "no work here"). + [ -n "$status" ] && assert_valid_status "$status" + + # Pull SCAN_DEPTH candidates from each source, THEN return the first + # matches — never let --limit shrink a source below the overlap (see #66). + local scan=$(( limit > SCAN_DEPTH ? limit : SCAN_DEPTH )) + # Candidate set by labels (carries labels[]). Empty labels = all open issues. - local by_labels; by_labels="$(gh_issues_by_labels "$limit" "${labels[@]}")" + local by_labels; by_labels="$(gh_issues_by_labels "$scan" "${labels[@]}")" if [ -z "$status" ]; then echo "$by_labels" | "$JQ" -c ".[:$limit]"; return fi # Intersect with the board's column, and stamp the status. - local nums; nums="$(gh_issue_numbers_by_status "$status" "$limit" | "$JQ" -R . | "$JQ" -sc 'map(tonumber)')" + local nums; nums="$(gh_issue_numbers_by_status "$status" "$scan" | "$JQ" -R . | "$JQ" -sc 'map(tonumber)')" echo "$by_labels" | "$JQ" -c --argjson nums "$nums" --arg st "$status" \ '[ .[] | select(.number as $n | $nums | index($n)) | .status = $st ] | .[:'"$limit"']' } diff --git a/scripts/rig-tracker.test.ts b/scripts/rig-tracker.test.ts index 0775262..824a76b 100644 --- a/scripts/rig-tracker.test.ts +++ b/scripts/rig-tracker.test.ts @@ -12,24 +12,39 @@ const SCRIPT = join(import.meta.dir, "rig-tracker.sh"); const dir = mkdtempSync(join(tmpdir(), "rig-tracker-")); afterAll(() => rmSync(dir, { recursive: true, force: true })); -// A single mock `gh`: canned issue-list (honors --label), project item-list, and -// pr view/edit (body comes from $MOCK_PR_BODY). +// A single mock `gh`: canned issue-list (honors --label AND --limit), project +// item-list (honors --limit), project field-list (Status options, for status +// validation), and pr view/edit (body comes from $MOCK_PR_BODY). +// +// The board deliberately returns a NON-Todo item first, so a naive candidate +// fetch that truncates by the result --limit (the #66 bug: next uses --limit 1) +// drops the dispatchable item out before the column intersection. const mockGh = join(dir, "gh"); writeFileSync( mockGh, `#!/usr/bin/env bash if [ "$1" = "issue" ] && [ "$2" = "list" ]; then - lbl=""; for ((i=1;i<=$#;i++)); do [ "\${!i}" = "--label" ] && { j=$((i+1)); lbl="\${!j}"; }; done + lbl=""; lim=1000 + for ((i=1;i<=$#;i++)); do + [ "\${!i}" = "--label" ] && { j=$((i+1)); lbl="\${!j}"; } + [ "\${!i}" = "--limit" ] && { j=$((i+1)); lim="\${!j}"; } + done all='[{"number":10,"title":"Epic A","url":"u10","labels":[{"name":"epic"}],"state":"OPEN"}, {"number":11,"title":"Sprint B","url":"u11","labels":[{"name":"sprint"}],"state":"OPEN"}, {"number":12,"title":"Plain C","url":"u12","labels":[],"state":"OPEN"}]' - if [ -n "$lbl" ]; then echo "$all" | jq -c --arg l "$lbl" '[.[]|select(.labels[]?.name==$l)]'; else echo "$all"; fi + if [ -n "$lbl" ]; then echo "$all" | jq -c --arg l "$lbl" --argjson n "$lim" '[.[]|select(.labels[]?.name==$l)][:$n]'; + else echo "$all" | jq -c --argjson n "$lim" '.[:$n]'; fi exit 0 fi if [ "$1" = "project" ] && [ "$2" = "item-list" ]; then - echo '{"items":[{"id":"i10","content":{"number":10},"status":"Todo"}, - {"id":"i11","content":{"number":11},"status":"Done"}, - {"id":"i12","content":{"number":12},"status":"Todo"}]}' + lim=1000; for ((i=1;i<=$#;i++)); do [ "\${!i}" = "--limit" ] && { j=$((i+1)); lim="\${!j}"; }; done + echo '{"items":[{"id":"i11","content":{"number":11},"status":"Done"}, + {"id":"i10","content":{"number":10},"status":"Todo"}, + {"id":"i12","content":{"number":12},"status":"Todo"}]}' | jq -c --argjson n "$lim" '{items: (.items[:$n])}' + exit 0 +fi +if [ "$1" = "project" ] && [ "$2" = "field-list" ]; then + echo '{"fields":[{"name":"Status","options":[{"name":"Todo","id":"o1"},{"name":"In Progress","id":"o2"},{"name":"In Review","id":"o3"},{"name":"Done","id":"o4"}]}]}' exit 0 fi if [ "$1" = "pr" ] && [ "$2" = "view" ]; then echo "\${MOCK_PR_BODY:-a body}"; exit 0; fi @@ -108,6 +123,32 @@ describe("rig-tracker select", () => { const r = run(config(GH), ["next"]); expect(JSON.parse(r.out)).toHaveLength(1); }); + + it("next finds the dispatchable item even when it isn't first on the board (#66)", () => { + // Board yields a Done item first; the result --limit of 1 must NOT truncate + // the board/label candidates before the Todo∩shape intersection. + const r = run(config(GH), ["next"]); + expect(r.code).toBe(0); + const items = JSON.parse(r.out); + expect(items).toHaveLength(1); + expect(items[0].number).toBe(10); + expect(items[0].status).toBe("Todo"); + }); +}); + +describe("rig-tracker select --status validation (#67)", () => { + it("a column that doesn't exist on the board errors (not a silent [])", () => { + const r = run(config(GH), ["select", "--status", "Nope"]); + expect(r.code).not.toBe(0); + expect(r.err).toContain("not a column"); + }); + + it("a valid but empty column returns [] with exit 0 (distinct from a typo)", () => { + // "In Review" is a real board option with no items in it. + const r = run(config(GH), ["select", "--status", "In Review"]); + expect(r.code).toBe(0); + expect(JSON.parse(r.out)).toEqual([]); + }); }); describe("rig-tracker link-pr", () => {