From 9c0e508b76957882f9d83b5d6b0d00cf165670c3 Mon Sep 17 00:00:00 2001 From: cluade-reviewer-andresmgsl Date: Fri, 31 Jul 2026 00:13:59 +0000 Subject: [PATCH] fix(drill): read every task, and fail a configured leg that cannot mint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses grok's and kimi's REQUEST_CHANGES on !130. 1. forgejo_run_verdict read only the FIRST entry of actions/tasks. That payload accumulates, so the moment a repo is drilled twice our run shares it with older ones — and nothing documents the sort order. A green job then reports as a timeout: a false FAILURE on the gate this leg exists to provide. It now inspects every entry and lets the newest id above pre_id decide. Newlines are stripped first, so a pretty-printed payload parses like a compact one. 2. A mint that yielded nothing degraded to SKIPPED "no registration token source" — violating #129's own acceptance ("token source present but the instance is unreachable -> the leg FAILS; it must not skip and must not pass") and sending the operator to check an env var they had already set. forgejo_token_verdict separates absent inputs from a configured leg that could not mint; only the former skips. 3. The pre---yes confirm block still announced a GitHub runner alone. Refs #129 Co-Authored-By: Claude Opus 5 (1M context) --- drill/drill.sh | 71 ++++++++++++++++++++++++++++++++++++++------------ test/drill.sh | 46 ++++++++++++++++++++++++++++++-- 2 files changed, 98 insertions(+), 19 deletions(-) diff --git a/drill/drill.sh b/drill/drill.sh index 2e7412b..06d1b7f 100644 --- a/drill/drill.sh +++ b/drill/drill.sh @@ -143,29 +143,61 @@ leg() { LEG_NAMES+=("$1"); LEG_RESULTS+=("$2"); } # non-terminal status is pending, not failed; grading a running job as a # failure would make the leg flaky inside its own watch window. # +# EVERY entry is inspected, and the NEWEST id above pre_id decides — never +# entry[0]. actions/tasks accumulates, so the moment a repo is drilled twice +# our run shares the payload with older ones, and nothing documents the sort +# order. Reading the first entry made a green job report as a timeout, a false +# FAILURE on the gate this leg exists to provide (grok/kimi on !130). +# # grep-and-sed, not jq: a throwaway drill machine has neither jq nor an # authenticated forge CLI, the same constraint json_field() carries in # commands/lib/runner-config.sh. `id` is a bare number, which json_field's -# quoted-value shape cannot read, so this reads both forms itself. +# quoted-value shape cannot read, so this reads both forms itself. Newlines are +# stripped first so a pretty-printed payload parses identically to a compact +# one — the instance documents neither. forgejo_run_verdict() { - # `run_status`, not `status`: the latter is read-only in zsh, and while this - # file is bash, the function is awk-extracted and sourced by other shells in - # test harnesses — where the assignment fails silently and grades every run - # as pending. - local pre="$1" file="$2" id run_status - id="$(grep -o '"id"[[:space:]]*:[[:space:]]*[0-9][0-9]*' "$file" 2>/dev/null \ - | head -n1 | sed 's/.*:[[:space:]]*//')" - [ -n "$id" ] || { echo pending; return 0; } - [ "$id" != "$pre" ] || { echo pending; return 0; } - run_status="$(grep -o '"status"[[:space:]]*:[[:space:]]*"[^"]*"' "$file" 2>/dev/null \ - | head -n1 | sed 's/.*:[[:space:]]*"//; s/"$//')" - case "$run_status" in - success) echo success ;; + local pre="$1" file="$2" obj id best_id="" best_st="" + [ -r "$file" ] || { echo pending; return 0; } + while IFS= read -r obj; do + [ -n "$obj" ] || continue + id="$(printf '%s' "$obj" | grep -o '"id"[[:space:]]*:[[:space:]]*[0-9][0-9]*' \ + | head -n1 | sed 's/.*:[[:space:]]*//')" + [ -n "$id" ] || continue + # Strictly newer than the pre-dispatch id. Equal is the run that was + # already there; lower is older still. + if [ -n "$pre" ]; then + [ "$id" -gt "$pre" ] 2>/dev/null || continue + fi + if [ -z "$best_id" ] || [ "$id" -gt "$best_id" ] 2>/dev/null; then + best_id="$id" + best_st="$(printf '%s' "$obj" | grep -o '"status"[[:space:]]*:[[:space:]]*"[^"]*"' \ + | head -n1 | sed 's/.*:[[:space:]]*"//; s/"$//')" + fi + done </dev/null | grep -o '{[^{}]*}') +EOF + [ -n "$best_id" ] || { echo pending; return 0; } + case "$best_st" in + success) echo success ;; failure | cancelled | skipped | timedout) echo failed ;; - *) echo pending ;; + *) echo pending ;; esac } +# forgejo_token_verdict — ok | mint-failed | no-source. +# #129's acceptance: "Token source present but the instance is unreachable -> +# the leg FAILS; it must not skip and must not pass". A mint that yields +# nothing — unreachable instance, under-scoped token, wrong repo — is a +# CONFIGURED leg failing, and reporting it as "no token source" both writes +# SKIPPED where the record owes a FAIL and sends the operator to check an env +# var they already set. Absent inputs are the only honest skip. +forgejo_token_verdict() { + if [ -n "$1" ]; then echo ok + elif [ -n "$2" ]; then echo mint-failed + else echo no-source + fi +} + # run_logged — run a long command with its narration in a file # and a dot every 5s on the terminal: a silent multi-minute apt/install run is @@ -411,7 +443,7 @@ This will, ON THIS HOST ($(hostname)): · run 'rig bootstrap $ROLE --users $USERS_FILE' — sshd hardening, hostname change, tailnet join, box ($BOXREPO@$BOXREF) + its Incus stack — TWICE (the second run is the idempotence assertion) - · install Coolify${COOLIFY_VERSION:+ $COOLIFY_VERSION} and a GitHub runner${RUNNER_REPO:+ against $RUNNER_REPO} + · install Coolify${COOLIFY_VERSION:+ $COOLIFY_VERSION}, a GitHub runner${RUNNER_REPO:+ against $RUNNER_REPO} and a Forgejo runner${FJ_INSTANCE:+ against $FJ_INSTANCE}${FJ_RUNNER_REPO:+ ($FJ_RUNNER_REPO)} Only do this on a THROWAWAY machine you can format. EOF [ -t 0 ] || { echo "drill: no TTY to confirm on — pass --yes if you mean it." >&2; exit 2; } @@ -787,7 +819,12 @@ else | grep -o '"token"[[:space:]]*:[[:space:]]*"[^"]*"' | head -n1 \ | sed 's/.*:[[:space:]]*"//; s/"$//')" fi - if [ -z "$fj_reg" ]; then + fj_tok_verdict="$(forgejo_token_verdict "$fj_reg" "${FORGEJO_API_TOKEN:-}")" + if [ "$fj_tok_verdict" = mint-failed ]; then + # Configured, and it did not work. Never a skip: see forgejo_token_verdict. + no "registration-token mint FAILED against ${FJ_INSTANCE} — is it reachable, and does FORGEJO_API_TOKEN own ${FJ_RUNNER_REPO}? (the token is never printed)" + leg "forgejo runner lifecycle ($FJ_RUNNER_REPO)" "FAIL — registration-token mint failed" + elif [ "$fj_tok_verdict" = no-source ]; then skip "forgejo runner lifecycle: no FORGEJO_RUNNER_TOKEN and no FORGEJO_API_TOKEN to mint one — the leg did not run" leg "forgejo runner lifecycle ($FJ_RUNNER_REPO)" "SKIPPED — no registration token source" else diff --git a/test/drill.sh b/test/drill.sh index 04b8f38..5dd965b 100644 --- a/test/drill.sh +++ b/test/drill.sh @@ -50,10 +50,10 @@ trap 'rm -rf "$WORK"' EXIT # --- the functions under test, extracted ------------------------------------- FNS="$WORK/drill-fns.sh" -for fn in tree_of assert_installed_from classify_leg capture_state emit_record forgejo_run_verdict; do +for fn in tree_of assert_installed_from classify_leg capture_state emit_record forgejo_run_verdict forgejo_token_verdict; do awk "/^${fn}\(\) \{/,/^\}/" "$ROOT/drill/drill.sh" >> "$FNS" done -for fn in tree_of assert_installed_from classify_leg capture_state emit_record forgejo_run_verdict; do +for fn in tree_of assert_installed_from classify_leg capture_state emit_record forgejo_run_verdict forgejo_token_verdict; do check "extraction guards the awk: ${fn}() landed" 0 "${fn}() {" grep -F "${fn}() {" "$FNS" done # shellcheck source=/dev/null @@ -243,6 +243,48 @@ printf '%s' '{"workflow_runs":[{"id":25,"status":"running","run_number":1}],"tot check "verdict: an assigned-but-running task is PENDING, not FAILED" 0 "pending" \ forgejo_run_verdict "24" "$FJ/new-running.json" +# grok/kimi on !130: the reader must not stop at the FIRST run. actions/tasks +# accumulates — the moment a repo is drilled twice, our run shares the payload +# with older ones, and nothing documents the sort order. Reading entry[0] makes +# a green job read as a timeout, which is a FALSE FAILURE on the very gate this +# leg exists to provide. +printf '%s' '{"workflow_runs":[{"id":24,"status":"success"},{"id":25,"status":"success"}],"total_count":2}' > "$FJ/oldest-first.json" +printf '%s' '{"workflow_runs":[{"id":25,"status":"success"},{"id":24,"status":"success"}],"total_count":2}' > "$FJ/newest-first.json" +printf '%s' '{"workflow_runs":[{"id":25,"status":"running"},{"id":26,"status":"success"}],"total_count":2}' > "$FJ/ours-not-first.json" +printf '%s' '{"workflow_runs":[{"id":23,"status":"success"},{"id":24,"status":"failure"}],"total_count":2}' > "$FJ/all-stale.json" +# Pretty-printed: the instance may or may not compact its JSON, and a parser +# that silently depends on one-line objects is a latent failure (kimi, !130). +cp /tmp/fjfix/pretty.json "$FJ/pretty.json" +check "verdict: ours is LAST in the payload — order must not decide" 0 "success" \ + forgejo_run_verdict "24" "$FJ/oldest-first.json" +check "verdict: ours is FIRST in the payload — same answer" 0 "success" \ + forgejo_run_verdict "24" "$FJ/newest-first.json" +check "verdict: a stale RUNNING entry ahead of ours does not mask it" 0 "success" \ + forgejo_run_verdict "24" "$FJ/ours-not-first.json" +check "verdict: every entry at or below pre is stale — PENDING" 0 "pending" \ + forgejo_run_verdict "24" "$FJ/all-stale.json" +check "verdict: a pretty-printed payload parses too" 0 "success" \ + forgejo_run_verdict "24" "$FJ/pretty.json" + +# ============================================================================= +# forgejo_token_verdict — a configured leg that cannot mint must FAIL, not SKIP +# ============================================================================= +# #129's own acceptance: "Token source present but the instance is unreachable +# -> the leg FAILS; it must not skip and must not pass". A mint that returns +# nothing because the instance is unreachable, the token is under-scoped or the +# repo name is wrong is a CONFIGURED leg failing — reporting "no token source" +# sends the operator to check an env var they already set, and writes SKIPPED +# where the record owes a FAIL. That is the UNREADABLE-vs-NONE shape +# drills/README.md names. +check "token: a resolved registration token is ok" 0 "ok" \ + forgejo_token_verdict "reg-tok" "" +check "token: an explicit token wins even with no API token" 0 "ok" \ + forgejo_token_verdict "reg-tok" "" +check "token: no token at all and no API token is a genuine SKIP" 0 "no-source" \ + forgejo_token_verdict "" "" +check "token: API token offered but mint produced nothing is a FAILURE" 0 "mint-failed" \ + forgejo_token_verdict "" "api-tok" + # The anti-false-positive guard, stated as its own case: an OLD completed run # with the SAME id as pre_id must never be read as this dispatch's result. check "verdict: a pre-existing success with the pre-id is NOT our run" 0 "pending" \