From 1daf7004d7a0a0f47666b07c2bc086a563a9551e Mon Sep 17 00:00:00 2001 From: cluade-reviewer-andresmgsl Date: Fri, 31 Jul 2026 00:15:02 +0000 Subject: [PATCH 1/2] fix(forgejo-runner): 'active' is not proof the runner is fetching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit status reports `systemctl is-active` as the service line, and that is the strongest health signal it has — the command reads only on-disk config, by contract. But a poller can go quiet while the process stays up: measured 2026-07-30 while probing for #129, a daemon logged "[poller] launched" and never fetched a job dispatched four minutes later, while a daemon started fresh claimed that same queued task in one second. Both times it read as a label-mapping bug on the forge, which is the wrong place to look. Says so where an operator already looks when nothing is obviously wrong, and names the remedy. log, not warn: an idle-but-healthy runner is silent in exactly the same way a stalled one is, so there is no signal separating them and a warning on every run would be crying wolf. No network call and no token read — the header contract, and test/cli.sh's existing guard, both stand. Refs #133 Co-Authored-By: Claude Opus 5 (1M context) --- changelog.d/133.md | 3 +++ commands/forgejo-runner-status.sh | 17 +++++++++++++++++ test/cli.sh | 20 ++++++++++++++++++++ 3 files changed, 40 insertions(+) create mode 100644 changelog.d/133.md diff --git a/changelog.d/133.md b/changelog.d/133.md new file mode 100644 index 0000000..fc4f074 --- /dev/null +++ b/changelog.d/133.md @@ -0,0 +1,3 @@ +### Fixed + +- `rig forgejo-runner status` no longer lets a service's `(active)` stand as proof the runner is fetching jobs (#133) diff --git a/commands/forgejo-runner-status.sh b/commands/forgejo-runner-status.sh index f201e9f..146d1bd 100755 --- a/commands/forgejo-runner-status.sh +++ b/commands/forgejo-runner-status.sh @@ -80,6 +80,23 @@ log "name: ${RUNNER_NAME:-unknown}" log "labels: ${LABELS}" log "dir: ${RUNNER_DIR}" log "service: ${SERVICE}" +# `active` is the strongest health signal this command has, and it proves only +# that a process exists — not that the runner is still asking Forgejo for work. +# A poller can go quiet while the daemon stays up: measured 2026-07-30 (#129, +# #133), a daemon logged "[poller] launched" and never fetched a job dispatched +# four minutes later, while a daemon started fresh claimed that same queued +# task in one second. Both times it read as a label-mapping bug on the forge. +# +# log, not warn: nothing has been DETECTED here. An idle runner with no queued +# jobs is silent in exactly the same way a stalled one is, so there is no +# signal separating them — a warning on every status run would be crying wolf, +# and warn in this file means a drift actually measured (the .runner mode +# below). Saying what the signal does not cover is the honest middle. +if [ "${STATE:-}" = active ]; then + log " note: 'active' is not proof the runner is fetching jobs — only that the process is up." + log " If a job stays queued and its run page says it never started, run" + log " 'systemctl restart forgejo-runner' and re-read before suspecting the labels." +fi # status is the only command an operator runs when nothing is obviously wrong, # which makes it the right place to notice a mode that drifted. It reports and diff --git a/test/cli.sh b/test/cli.sh index 4846778..4cf5bcd 100644 --- a/test/cli.sh +++ b/test/cli.sh @@ -3407,6 +3407,26 @@ check "forgejo-runner: install converges the mode on EVERY run, not only at regi fr_secure_every_run check "forgejo-runner: status warns on a drifted mode" 0 "FORGEJO_RUNNER_FILE_MODE" \ grep -o "FORGEJO_RUNNER_FILE_MODE" "$ROOT/commands/forgejo-runner-status.sh" +# #133: `active` is the strongest signal this command has, and it proves only +# that a process exists. A poller can go quiet while the process stays up — +# measured for #129: a daemon logged "[poller] launched" and never fetched a +# job dispatched four minutes later, while a fresh daemon claimed the same +# queued task in one second. status is where an operator looks when nothing +# is obviously wrong, so it says so there. +FJS="$ROOT/commands/forgejo-runner-status.sh" +check "forgejo-runner: status says 'active' is not proof the runner is fetching" 0 "not proof" \ + grep -o "not proof" "$FJS" +check "forgejo-runner: …and names the remedy, so the line is actionable" 0 "restart" \ + grep -oi "systemctl restart forgejo-runner" "$FJS" +# It must NOT be a warning: nothing has been detected. An idle-but-healthy +# runner logs nothing either, so there is no signal that separates it from a +# stalled one — a WARNING on every status run would be crying wolf, and this +# file reserves warn for drift it has actually measured (the .runner mode). +check "forgejo-runner: the liveness note is informational, never a WARNING" 1 "" \ + grep -nE 'warn ".*not proof' "$FJS" +# The header contract at :25 — no token, no network call — survives this. +check "forgejo-runner: status still makes no network call" 1 "" \ + grep -nE '^[^#]*(curl|wget) ' "$FJS" rm -rf "$FRW" # --- the checksum gate, DRIVEN not grepped (review !110) -------------------- From 848e3f42ef063941f3d3dfcf858aa5941dcca180 Mon Sep 17 00:00:00 2001 From: cluade-reviewer-andresmgsl Date: Fri, 31 Jul 2026 20:11:53 +0000 Subject: [PATCH 2/2] test(forgejo-runner): drive the liveness note's state boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit codex and kimi are right: the four checks proved the LINES EXIST and nothing proved they fire only when the unit is active. kimi deleted the state guard and the suite stayed 790/790 green, so the boundary #133 cares about most — no misleading liveness note on an absent or inactive unit — was unprotected. #133's own test plan says driven, not grepped where behaviour can be executed, and this was the line it crossed. The note is now a function, which is what makes the boundary executable, and the suite drives it on active (note present, remedy named), inactive (empty) and unset (empty). Removing the guard now fails two checks instead of none. The no-warn and no-network guards stay greps: those properties are source-level by nature, as kimi noted. Two things I got wrong on the way, both caught by running it rather than reading it: the function was defined below its call site (shellcheck SC2218), and the block reused $WORK, which is rm -rf'd at :3206 long before it — so it now takes its own scratch dir like the file's other fixtures. Refs #133 Co-Authored-By: Claude Opus 5 (1M context) --- commands/forgejo-runner-status.sh | 44 +++++++++++++++++++------------ test/cli.sh | 21 +++++++++++++++ 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/commands/forgejo-runner-status.sh b/commands/forgejo-runner-status.sh index 146d1bd..6119d47 100755 --- a/commands/forgejo-runner-status.sh +++ b/commands/forgejo-runner-status.sh @@ -11,6 +11,31 @@ log() { printf 'rig-forgejo-runner: %s\n' "$*"; } warn() { printf 'rig-forgejo-runner: WARNING: %s\n' "$*" >&2; } die() { printf 'rig-forgejo-runner: ERROR: %s\n' "$1" >&2; exit "${2:-1}"; } +# forgejo_runner_liveness_note — what `active` does not cover. +# +# `active` is the strongest health signal this command has, and it proves only +# that a process exists — not that the runner is still asking Forgejo for work. +# A poller can go quiet while the daemon stays up: measured 2026-07-30 (#129, +# #133), a daemon logged "[poller] launched" and never fetched a job dispatched +# four minutes later, while a daemon started fresh claimed that same queued task +# in one second. Both times it read as a label-mapping bug on the forge, which +# is the wrong place to look. +# +# A FUNCTION rather than an inline `if`, because the state boundary is the part +# worth pinning: an absent or inactive unit must say nothing, and a grep over +# the source cannot tell the difference (codex/kimi, !134). +# +# log, not warn: nothing has been DETECTED here. An idle runner with no queued +# jobs is silent in exactly the same way a stalled one is, so there is no signal +# separating them — a warning on every status run would be crying wolf, and warn +# in this file means a drift actually measured (the .runner mode below). +forgejo_runner_liveness_note() { + [ "${1:-}" = active ] || return 0 + log " note: 'active' is not proof the runner is fetching jobs — only that the process is up." + log " If a job stays queued and its run page says it never started, run" + log " 'systemctl restart forgejo-runner' and re-read before suspecting the labels." +} + usage() { cat <<'EOF' usage: rig forgejo-runner status [--user ] @@ -80,23 +105,8 @@ log "name: ${RUNNER_NAME:-unknown}" log "labels: ${LABELS}" log "dir: ${RUNNER_DIR}" log "service: ${SERVICE}" -# `active` is the strongest health signal this command has, and it proves only -# that a process exists — not that the runner is still asking Forgejo for work. -# A poller can go quiet while the daemon stays up: measured 2026-07-30 (#129, -# #133), a daemon logged "[poller] launched" and never fetched a job dispatched -# four minutes later, while a daemon started fresh claimed that same queued -# task in one second. Both times it read as a label-mapping bug on the forge. -# -# log, not warn: nothing has been DETECTED here. An idle runner with no queued -# jobs is silent in exactly the same way a stalled one is, so there is no -# signal separating them — a warning on every status run would be crying wolf, -# and warn in this file means a drift actually measured (the .runner mode -# below). Saying what the signal does not cover is the honest middle. -if [ "${STATE:-}" = active ]; then - log " note: 'active' is not proof the runner is fetching jobs — only that the process is up." - log " If a job stays queued and its run page says it never started, run" - log " 'systemctl restart forgejo-runner' and re-read before suspecting the labels." -fi +forgejo_runner_liveness_note "${STATE:-}" + # status is the only command an operator runs when nothing is obviously wrong, # which makes it the right place to notice a mode that drifted. It reports and diff --git a/test/cli.sh b/test/cli.sh index 4cf5bcd..6a0b4e7 100644 --- a/test/cli.sh +++ b/test/cli.sh @@ -3424,6 +3424,27 @@ check "forgejo-runner: …and names the remedy, so the line is actionable" 0 "re # file reserves warn for drift it has actually measured (the .runner mode). check "forgejo-runner: the liveness note is informational, never a WARNING" 1 "" \ grep -nE 'warn ".*not proof' "$FJS" + +# codex/kimi on !134: the three greps above prove the LINES EXIST; nothing +# proved they fire only when the unit is active. Deleting the state guard left +# the suite 790/790 green, so the acceptance boundary #133 cares about most — +# no misleading liveness note on an absent or inactive unit — was unprotected. +# Drive the decision instead, extracted the way test/drill.sh extracts its own. +# Its own scratch dir: $WORK is rm -rf'd at :3206, well before this block. +FJS_DIR="$(mktemp -d)" +FJSW="$FJS_DIR/fjs-note.sh" +{ printf '%s\n' 'log() { printf "rig-forgejo-runner: %s\\n" "$*"; }' + awk '/^forgejo_runner_liveness_note\(\) \{/,/^\}/' "$FJS" +} > "$FJSW" +check "forgejo-runner: liveness note extracted (guards the awk)" 0 "forgejo_runner_liveness_note() {" \ + cat "$FJSW" +note_for() { bash -c '. "$1"; forgejo_runner_liveness_note "$2"' _ "$FJSW" "$1"; } +check "liveness note: an ACTIVE unit is told what active does not prove" 0 "not proof" note_for active +check "liveness note: …and is given the remedy" 0 "systemctl restart forgejo-runner" note_for active +note_is_empty() { [ -z "$(note_for "$1")" ]; } +check "liveness note: an INACTIVE unit gets nothing" 0 "" note_is_empty inactive +check "liveness note: an ABSENT unit (empty state) gets nothing" 0 "" note_is_empty "" +rm -rf "$FJS_DIR" # The header contract at :25 — no token, no network call — survives this. check "forgejo-runner: status still makes no network call" 1 "" \ grep -nE '^[^#]*(curl|wget) ' "$FJS"