fix(drill): the pre-dispatch baseline folds max too, and the suite is hermetic

Round 2 of !130. Both blockers were mine, and one was the same mistake I had
just fixed one layer up.

1. fj_pre still took `head -n1`. The verdict folds max over every id, but the
   PRE-DISPATCH snapshot did not, so an oldest-first payload named an old run
   as the baseline — and a later poll finding the same body then read the
   PREVIOUS drill's run as this dispatch's result. That is a false PASS on the
   take-a-job assertion, strictly worse than the false FAIL entry[0] caused
   inside the verdict. Both sides now share forgejo_max_task_id, and a test
   composes them the way the leg does so the pair cannot drift apart again.

2. test/drill.sh copied its pretty-printed fixture from /tmp/fjfix — a scratch
   path that existed only on the box the fix was built on. Everywhere else the
   cp failed, the guard returned pending, and the suite was 65/66. The claimed
   66/66 was true on one machine. The fixture is written inline like every
   other one; verified by deleting the scratch dir and running the suite from
   a clean tree under env -i.

Refs #129

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

View file

@ -184,6 +184,28 @@ EOF
esac
}
# 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
# max exactly as forgejo_run_verdict does: taking the first id instead names an
# OLD run as the baseline whenever the payload is not newest-first (the order is
# undocumented). A later poll that finds the same body then reads the PREVIOUS
# drill's run as this dispatch's result — a false PASS on the take-a-job
# assertion, which is worse than the false FAIL the same mistake caused inside
# the verdict (grok/kimi, !130). The GitHub leg is safe from this only because
# `gh run list --limit 1` contracts newest-first; this API contracts nothing.
forgejo_max_task_id() {
local file="$1" id best=""
[ -r "$file" ] || return 0
while IFS= read -r id; do
[ -n "$id" ] || continue
if [ -z "$best" ] || [ "$id" -gt "$best" ] 2>/dev/null; then best="$id"; fi
done <<EOF
$(tr -d '\n' < "$file" 2>/dev/null \
| grep -o '"id"[[:space:]]*:[[:space:]]*[0-9][0-9]*' | sed 's/.*:[[:space:]]*//')
EOF
printf '%s\n' "$best"
}
# forgejo_token_verdict <resolved_token> <api_token> — 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
@ -850,8 +872,7 @@ else
fj_pre_body="$(curl -fsSL -H "Authorization: token ${FORGEJO_API_TOKEN}" \
"$fj_api/actions/tasks" 2>/dev/null || echo '{}')"
printf '%s' "$fj_pre_body" > /tmp/drill-forgejo-pre.json
fj_pre="$(grep -o '"id"[[:space:]]*:[[:space:]]*[0-9][0-9]*' /tmp/drill-forgejo-pre.json \
2>/dev/null | head -n1 | sed 's/.*:[[:space:]]*//')"
fj_pre="$(forgejo_max_task_id /tmp/drill-forgejo-pre.json)"
if curl -fsSL -o /dev/null -X POST -H "Authorization: token ${FORGEJO_API_TOKEN}" \
-H "Content-Type: application/json" -d "{\"ref\":\"${FJ_REF}\"}" \
"$fj_api/actions/workflows/${RUNNER_WORKFLOW}/dispatches" 2>/dev/null; then

View file

@ -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 forgejo_token_verdict; 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; 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 forgejo_token_verdict; 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; do
check "extraction guards the awk: ${fn}() landed" 0 "${fn}() {" grep -F "${fn}() {" "$FNS"
done
# shellcheck source=/dev/null
@ -254,7 +254,15 @@ printf '%s' '{"workflow_runs":[{"id":25,"status":"running"},{"id":26,"status":"s
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"
# Written HERE, like every other fixture: a suite that copies from a scratch
# path passes only on the box that built it (grok/kimi, !130 round 2).
printf '%s\n' '{
"workflow_runs": [
{"id": 24, "status": "success"},
{"id": 25, "name": "drill", "status": "success"}
],
"total_count": 2
}' > "$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" \
@ -266,6 +274,30 @@ check "verdict: every entry at or below pre is stale — PENDING" 0 "pending" \
check "verdict: a pretty-printed payload parses too" 0 "success" \
forgejo_run_verdict "24" "$FJ/pretty.json"
# The PRE-DISPATCH snapshot has the same multi-entry hazard as the verdict, and
# getting it wrong is worse: a `head -n1` pre-id on an oldest-first payload
# names an OLD run as the baseline, so a later poll that finds the same body
# reports the PREVIOUS drill's run as ours — a false PASS on the take-a-job
# assertion, where the entry[0] bug only produced a false failure (grok, !130).
# Both sides must fold max over every id, which is why they share one function.
check "max id: oldest-first payload yields the NEWEST id, not the first" 0 "25" \
forgejo_max_task_id "$FJ/oldest-first.json"
check "max id: newest-first payload yields the same answer" 0 "25" \
forgejo_max_task_id "$FJ/newest-first.json"
check "max id: an empty payload has no id at all" 0 "" \
forgejo_max_task_id "$FJ/empty.json"
check "max id: a pretty-printed payload folds too" 0 "25" \
forgejo_max_task_id "$FJ/pretty.json"
# The false PASS, pinned end to end: snapshot the oldest-first body, dispatch,
# the runner never takes it so the body is unchanged — the verdict must stay
# pending. With head -n1 this returned success.
# The two halves composed exactly as the leg composes them.
verdict_after_no_new_run() { forgejo_run_verdict "$(forgejo_max_task_id "$1")" "$1"; }
check "no new run after dispatch: max-id baseline keeps it PENDING (false-PASS guard)" 0 "pending" \
verdict_after_no_new_run "$FJ/oldest-first.json"
check "…and the same composition on a pretty payload" 0 "pending" \
verdict_after_no_new_run "$FJ/pretty.json"
# =============================================================================
# forgejo_token_verdict — a configured leg that cannot mint must FAIL, not SKIP
# =============================================================================