fix(drill): the leg row is the whole lifecycle, not just the job
Some checks failed
ci / check (pull_request) Failing after 7s
ci / install (pull_request) Successful in 4s
ci / db-integration (pull_request) Successful in 4s
labels / labels (pull_request) Failing after 7s

codex, grok and kimi all landed on the same hole at e42ba31, and they are
right. The record row keyed only on fj_took, so install/status/remove failures
called `no` without gating it. A dispatched job only needs SOMETHING answering
runs-on: drill — and this leg removes locally, telling the operator to delete
the stale runner by hand, so a leftover drill-labeled runner from the previous
drill is the designed-for aftermath, not a contrived case. It answers the
fixture exactly, and the row then read "PASS — registered, took a job,
removed" for a drill whose install never happened.

The exit code was never wrong: every one of those failures also called `no`.
What was wrong is the row, and the row is what outlives the run in
drills/<v>.md. A row claiming a lifecycle that did not happen is exactly what
the gate exists to refuse — kimi's framing, and it is the right one.

forgejo_leg_row folds all five assertions, and the leg no longer dispatches
after install or status has failed: a foreign runner's success could only ever
be evidence about someone else's runner. The regression composes the real
functions over a payload carrying a newer successful run, the way the leg
composes them.

The GitHub leg has the identical shape and is byte-frozen by #129's contract;
left alone deliberately, and worth a sibling issue for triage.

Refs #129

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
cluade-reviewer-andresmgsl 2026-07-31 20:04:16 +00:00
parent e42ba31a09
commit 9afcdf858f
2 changed files with 107 additions and 17 deletions

View file

@ -184,6 +184,35 @@ EOF
esac esac
} }
# forgejo_leg_row <install_ok> <status_ok> <took> <remove_ok> <absent_ok>
# The record row for the Forgejo runner leg. PASS requires the WHOLE lifecycle,
# not just the take-a-job outcome.
#
# Keying the row on <took> alone let it read "PASS — registered, took a job,
# removed" when install had failed, because the dispatched job only needs
# SOMETHING answering runs-on: drill — and this leg removes locally, telling the
# operator to delete the stale runner by hand, so a leftover drill-labeled
# runner from the previous drill is the designed-for aftermath rather than a
# contrived case (codex/grok/kimi on !130). drills/<v>.md is the release's
# durable evidence; a row claiming a lifecycle that did not happen is exactly
# what the gate exists to refuse.
#
# The drill's exit code was never wrong here — every one of those failures also
# called `no`. What was wrong is the row, and the row is what outlives the run.
forgejo_leg_row() {
local install_ok="$1" status_ok="$2" took="$3" remove_ok="$4" absent_ok="$5"
if [ "$install_ok" != 1 ] || [ "$status_ok" != 1 ] \
|| [ "$remove_ok" != 1 ] || [ "$absent_ok" != 1 ]; then
echo "FAIL — see Failed below"
return 0
fi
case "$took" in
success) echo "PASS — registered, took a job, removed (stale row needs deleting by hand)" ;;
none) echo "PARTIAL — registered and removed; took a job: not attempted (no FORGEJO_API_TOKEN)" ;;
*) echo "FAIL — see Failed below" ;;
esac
}
# forgejo_max_task_id <tasks-json> — the highest numeric task id in the payload, # forgejo_max_task_id <tasks-json> — the highest numeric task id in the payload,
# empty when there is none. This is the PRE-DISPATCH baseline, and it must fold # empty when there is none. This is the PRE-DISPATCH baseline, and it must fold
# max exactly as forgejo_run_verdict does: taking the first id instead names an # max exactly as forgejo_run_verdict does: taking the first id instead names an
@ -851,21 +880,31 @@ else
leg "forgejo runner lifecycle ($FJ_RUNNER_REPO)" "SKIPPED — no registration token source" leg "forgejo runner lifecycle ($FJ_RUNNER_REPO)" "SKIPPED — no registration token source"
else else
FJ_NAME="drill-$(hostname)-$$" FJ_NAME="drill-$(hostname)-$$"
fj_install_ok=0 fj_status_ok=0 fj_remove_ok=0 fj_absent_ok=0
# The label MUST carry a docker:// image: forgejo-runner runs jobs in # The label MUST carry a docker:// image: forgejo-runner runs jobs in
# containers, and a bare label leaves runs-on matched but unrunnable. # containers, and a bare label leaves runs-on matched but unrunnable.
if FORGEJO_RUNNER_TOKEN="$fj_reg" run_logged /tmp/drill-forgejo-runner-install.log \ if FORGEJO_RUNNER_TOKEN="$fj_reg" run_logged /tmp/drill-forgejo-runner-install.log \
rig forgejo-runner install --instance "$FJ_INSTANCE" --name "$FJ_NAME" \ rig forgejo-runner install --instance "$FJ_INSTANCE" --name "$FJ_NAME" \
--labels 'drill:docker://node:22-bookworm'; then --labels 'drill:docker://node:22-bookworm'; then
ok "rig forgejo-runner install --instance $FJ_INSTANCE exited 0 (registered as $FJ_NAME)" ok "rig forgejo-runner install --instance $FJ_INSTANCE exited 0 (registered as $FJ_NAME)"
fj_install_ok=1
else else
no "forgejo-runner install FAILED — tail: $(tail -3 /tmp/drill-forgejo-runner-install.log | tr '\n' ' ')" no "forgejo-runner install FAILED — tail: $(tail -3 /tmp/drill-forgejo-runner-install.log | tr '\n' ' ')"
fi fi
rig forgejo-runner status 2>/dev/null | grep -qF "${FJ_INSTANCE%/}" \ if rig forgejo-runner status 2>/dev/null | grep -qF "${FJ_INSTANCE%/}"; then
&& ok "forgejo-runner status names the instance: $FJ_INSTANCE" \ ok "forgejo-runner status names the instance: $FJ_INSTANCE"; fj_status_ok=1
|| no "forgejo-runner status does not name ${FJ_INSTANCE}" else
no "forgejo-runner status does not name ${FJ_INSTANCE}"
fi
fj_took=none fj_took=none
if [ -n "${FORGEJO_API_TOKEN:-}" ]; then # Do NOT dispatch once install or status has failed. The job would be taken
# by whatever else answers runs-on: drill — a stale runner this leg's own
# hand-delete caveat leaves behind — and its success would be evidence about
# someone else's runner (codex/grok/kimi, !130).
if [ "$fj_install_ok" != 1 ] || [ "$fj_status_ok" != 1 ]; then
skip "took a job: not attempted — install or status failed, and a foreign runner answering 'drill' could only manufacture a false pass"
elif [ -n "${FORGEJO_API_TOKEN:-}" ]; then
fj_api="${FJ_INSTANCE%/}/api/v1/repos/${FJ_RUNNER_REPO}" fj_api="${FJ_INSTANCE%/}/api/v1/repos/${FJ_RUNNER_REPO}"
# Read the newest ASSIGNED task id BEFORE dispatching, same guard as the # Read the newest ASSIGNED task id BEFORE dispatching, same guard as the
# GitHub leg: an already-completed run must never be read as ours. # GitHub leg: an already-completed run must never be read as ours.
@ -908,19 +947,21 @@ else
fi fi
# No removal token exists on this forge — remove is local by design. # No removal token exists on this forge — remove is local by design.
rig forgejo-runner remove >/dev/null 2>&1 \ if rig forgejo-runner remove >/dev/null 2>&1; then
&& note "forgejo-runner removed locally — Forgejo has no deregistration endpoint, so DELETE the stale '$FJ_NAME' row under $FJ_RUNNER_REPO > Settings > Actions > Runners by hand" \ note "forgejo-runner removed locally — Forgejo has no deregistration endpoint, so DELETE the stale '$FJ_NAME' row under $FJ_RUNNER_REPO > Settings > Actions > Runners by hand"
|| no "forgejo-runner remove FAILED" fj_remove_ok=1
rig forgejo-runner status >/dev/null 2>&1 \ else
&& no "forgejo-runner status still answers after remove — the removal did not take" \ no "forgejo-runner remove FAILED"
|| ok "forgejo-runner status confirms: nothing registered" fi
if rig forgejo-runner status >/dev/null 2>&1; then
no "forgejo-runner status still answers after remove — the removal did not take"
else
ok "forgejo-runner status confirms: nothing registered"; fj_absent_ok=1
fi
leg "forgejo runner lifecycle ($FJ_RUNNER_REPO)" \ leg "forgejo runner lifecycle ($FJ_RUNNER_REPO)" \
"$(case "$fj_took" in "$(forgejo_leg_row "$fj_install_ok" "$fj_status_ok" "$fj_took" \
success) echo "PASS — registered, took a job, removed (stale row needs deleting by hand)" ;; "$fj_remove_ok" "$fj_absent_ok")"
none) echo "PARTIAL — registered and removed; took a job: not attempted (no FORGEJO_API_TOKEN)" ;;
*) echo "FAIL — see Failed below" ;;
esac)"
fi fi
fi fi

View file

@ -50,10 +50,10 @@ trap 'rm -rf "$WORK"' EXIT
# --- the functions under test, extracted ------------------------------------- # --- the functions under test, extracted -------------------------------------
FNS="$WORK/drill-fns.sh" FNS="$WORK/drill-fns.sh"
for fn in tree_of assert_installed_from classify_leg capture_state emit_record forgejo_run_verdict forgejo_token_verdict forgejo_max_task_id; do for fn in tree_of assert_installed_from classify_leg capture_state emit_record forgejo_run_verdict forgejo_token_verdict forgejo_max_task_id forgejo_leg_row; do
awk "/^${fn}\(\) \{/,/^\}/" "$ROOT/drill/drill.sh" >> "$FNS" awk "/^${fn}\(\) \{/,/^\}/" "$ROOT/drill/drill.sh" >> "$FNS"
done done
for fn in tree_of assert_installed_from classify_leg capture_state emit_record forgejo_run_verdict forgejo_token_verdict forgejo_max_task_id; do for fn in tree_of assert_installed_from classify_leg capture_state emit_record forgejo_run_verdict forgejo_token_verdict forgejo_max_task_id forgejo_leg_row; do
check "extraction guards the awk: ${fn}() landed" 0 "${fn}() {" grep -F "${fn}() {" "$FNS" check "extraction guards the awk: ${fn}() landed" 0 "${fn}() {" grep -F "${fn}() {" "$FNS"
done done
# shellcheck source=/dev/null # shellcheck source=/dev/null
@ -298,6 +298,55 @@ check "no new run after dispatch: max-id baseline keeps it PENDING (false-PASS g
check "…and the same composition on a pretty payload" 0 "pending" \ check "…and the same composition on a pretty payload" 0 "pending" \
verdict_after_no_new_run "$FJ/pretty.json" verdict_after_no_new_run "$FJ/pretty.json"
# =============================================================================
# forgejo_leg_row — the row is the WHOLE lifecycle, not just the job
# =============================================================================
# codex/grok/kimi on !130: keying the record row on the take-a-job outcome alone
# lets it read "PASS — registered, took a job, removed" when install failed, so
# long as SOMETHING answered runs-on: drill. That is not contrived — this leg
# removes locally and tells the operator to delete the stale runner by hand, so
# a leftover drill-labeled runner from the previous drill is the DESIGNED-FOR
# aftermath, and it answers the fixture exactly.
#
# drills/<v>.md is the release's durable evidence. A row claiming a lifecycle
# that did not happen is precisely what the gate exists to refuse, so PASS
# requires every assertion, not just the interesting one.
check "leg row: everything succeeded is the only PASS" 0 "PASS" \
forgejo_leg_row 1 1 success 1 1
check "leg row: install failed cannot PASS, even when a foreign runner took the job" 0 "FAIL" \
forgejo_leg_row 0 1 success 1 1
check "leg row: status failed cannot PASS either" 0 "FAIL" \
forgejo_leg_row 1 0 success 1 1
check "leg row: remove failed cannot PASS" 0 "FAIL" \
forgejo_leg_row 1 1 success 0 1
check "leg row: a runner still registered after remove cannot PASS" 0 "FAIL" \
forgejo_leg_row 1 1 success 1 0
check "leg row: no dispatch attempted, everything else clean, is PARTIAL" 0 "PARTIAL" \
forgejo_leg_row 1 1 none 1 1
check "leg row: a job that was never taken is a FAIL" 0 "FAIL" \
forgejo_leg_row 1 1 timeout 1 1
check "leg row: PARTIAL requires a clean lifecycle too" 0 "FAIL" \
forgejo_leg_row 0 1 none 1 1
# The end-to-end shape codex/grok/kimi asked for, composed the way the leg
# composes it: a tasks payload carrying a NEWER successful run (as a foreign
# drill-labeled runner would produce) must still not yield a PASS row when the
# drill's own install failed. This is the exact false-evidence case.
row_after_failed_install() {
forgejo_leg_row 0 1 "$(forgejo_run_verdict "$(forgejo_max_task_id "$1")" "$1")" 1 1
}
printf '%s' '{"workflow_runs":[{"id":24,"status":"success"},{"id":99,"status":"success"}],"total_count":2}' \
> "$FJ/foreign-runner-took-it.json"
check "install failed + a newer successful run in the payload is still FAIL, never PASS" 0 "FAIL" \
row_after_failed_install "$FJ/foreign-runner-took-it.json"
# …and the same payload with a clean lifecycle is the PASS, so the check above
# is discriminating rather than always-FAIL.
row_after_clean_install() {
forgejo_leg_row 1 1 "$(forgejo_run_verdict "" "$1")" 1 1
}
check "…while the same payload with a clean lifecycle does PASS" 0 "PASS" \
row_after_clean_install "$FJ/foreign-runner-took-it.json"
# ============================================================================= # =============================================================================
# forgejo_token_verdict — a configured leg that cannot mint must FAIL, not SKIP # forgejo_token_verdict — a configured leg that cannot mint must FAIL, not SKIP
# ============================================================================= # =============================================================================