diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index da26001355..2cfef1cf98 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -204,11 +204,17 @@ jobs: setsid --wait bash scripts/check_no_claim_literals.sh --selftest setsid --wait bash scripts/check_hardcoded_paths.sh --self-test setsid --wait bash scripts/check_pr_review_wiring.sh --self-test + setsid --wait bash scripts/ci/fence_workflow_commands.sh --self-test + setsid --wait bash scripts/ci/fence_workflow_commands.sh + setsid --wait bash scripts/ci/fetch_p6.sh --self-test + setsid --wait bash scripts/ci/fetch_p6.sh --check-wiring # The driver decides what every section's if:/matrix/needs means and whether # the vendored workflow is the upstream one; its case table must go RED on # each planted defect before it is trusted to report a section green. + # #4936: fenced, so the planted rows' ::error:: lines ("external job + # 'workspace-test': not completed within 12000s") are not job annotations. - name: fat_driver case table must still turn RED - run: python3 scripts/ci/fat_driver.py self-test + run: bash scripts/ci/fence_workflow_commands.sh -- python3 scripts/ci/fat_driver.py self-test # PMAT-4510: every secrets. a section reads must be plumbed above as # FAT_SECRET_ under its full name; #4441 cut one short and the # receipt signer ran with an empty key on every PR. Text-only, no build. @@ -540,7 +546,7 @@ jobs: with: fetch-depth: 0 - name: fat_driver case table must still turn RED - run: python3 scripts/ci/fat_driver.py self-test + run: bash scripts/ci/fence_workflow_commands.sh -- python3 scripts/ci/fat_driver.py self-test # `?` for the brackets and the comma: --sections splits on commas. - name: Sections id: run diff --git a/ci/sections.yml b/ci/sections.yml index 47ab8afce2..5894a9bfc4 100644 --- a/ci/sections.yml +++ b/ci/sections.yml @@ -370,8 +370,8 @@ jobs: # warning below says so instead of the run silently paying the hour. case "${GITHUB_EVENT_NAME}" in merge_group|push) - git fetch --no-tags --depth=2 origin "${GITHUB_SHA}" 2>/dev/null \ - || git fetch --no-tags --deepen=1 origin 2>/dev/null \ + bash scripts/ci/fetch_p6.sh --no-tags --depth=2 origin "${GITHUB_SHA}" 2>/dev/null \ + || bash scripts/ci/fetch_p6.sh --no-tags --deepen=1 origin 2>/dev/null \ || true if git rev-parse -q --verify 'HEAD^1' >/dev/null 2>&1; then printf 'first parent available: %s\n' "$(git rev-parse HEAD^1)" @@ -382,7 +382,7 @@ jobs: esac case "${GITHUB_EVENT_NAME}" in pull_request) - git fetch --no-tags --depth=1 origin "+refs/heads/${GITHUB_BASE_REF}:refs/remotes/origin/${GITHUB_BASE_REF}" + bash scripts/ci/fetch_p6.sh --no-tags --depth=1 origin "+refs/heads/${GITHUB_BASE_REF}:refs/remotes/origin/${GITHUB_BASE_REF}" # --no-renames (#3664): a rename must list BOTH paths, or a source # moved into docs/ reads as docs-only and runs zero tests. git diff --no-renames --name-only "origin/${GITHUB_BASE_REF}" HEAD > "$RUNNER_TEMP/touched.txt" @@ -393,7 +393,7 @@ jobs: pr=$(printf '%s' "${GITHUB_REF_NAME}" | sed -n 's|^gh-readonly-queue/[^/]*/pr-\([0-9]*\)-.*|\1|p') if [ -n "$pr" ]; then head=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${pr}" --jq .head.sha) - git fetch --no-tags --depth=1 origin "$head" + bash scripts/ci/fetch_p6.sh --no-tags --depth=1 origin "$head" concl=$(gh api "repos/${GITHUB_REPOSITORY}/commits/${head}/check-runs?check_name=workspace-test&per_page=20" --jq '[.check_runs[]|select(.status=="completed")]|sort_by(.completed_at)|last|.conclusion // "none"') args+=(--pr-head "$head" --pr-head-conclusion "${concl:-none}") fi @@ -439,7 +439,7 @@ jobs: merge_group) base="${MG_BASE_SHA:-}" ;; push) base="${PUSH_BEFORE:-}" ;; workflow_dispatch) - git fetch --no-tags --depth=1 origin +refs/heads/main:refs/remotes/origin/main + bash scripts/ci/fetch_p6.sh --no-tags --depth=1 origin +refs/heads/main:refs/remotes/origin/main printf 'refinement base: origin/main tip %s (workflow_dispatch has no event base)\n' "$(git rev-parse origin/main)" exit 0 ;; @@ -449,7 +449,7 @@ jobs: printf '::error::refinement base: %s event carries no base sha (got "%s")\n' "${GITHUB_EVENT_NAME}" "$base" exit 1 fi - git fetch --no-tags --depth=1 origin "$base" + bash scripts/ci/fetch_p6.sh --no-tags --depth=1 origin "$base" git update-ref refs/remotes/origin/main "$base" printf 'refinement base: %s base %s pinned as origin/main\n' "${GITHUB_EVENT_NAME}" "$(git rev-parse 'origin/main^{commit}')" - name: Workspace lib tests (25,300+) @@ -3051,7 +3051,7 @@ jobs: # PR's diff, which is the safe direction for a selector — it can only # run the GPU jobs when they were not needed, never skip them when # they were. - git fetch --no-tags --depth=1 origin \ + bash scripts/ci/fetch_p6.sh --no-tags --depth=1 origin \ "+refs/heads/${GITHUB_BASE_REF}:refs/remotes/origin/${GITHUB_BASE_REF}" git diff --name-only "origin/${GITHUB_BASE_REF}" HEAD > "$RUNNER_TEMP/touched.txt" printf 'touched (tip vs tip): %s path(s)\n' \ diff --git a/docs/roadmaps/entries/PMAT-4936.yaml b/docs/roadmaps/entries/PMAT-4936.yaml new file mode 100644 index 0000000000..564c1640c5 --- /dev/null +++ b/docs/roadmaps/entries/PMAT-4936.yaml @@ -0,0 +1,18 @@ +- id: PMAT-4936 + github_issue: 4936 + item_type: task + title: 'x86-main: fence the fat_driver case-table annotations; section git fetch reads up to 3 times (P6)' + status: inprogress + priority: high + assigned_to: null + created: 2026-10-08T14:17:11Z + updated: 2026-10-08T14:17:16.352731207+00:00 + spec: null + acceptance_criteria: [] + phases: [] + subtasks: [] + estimated_effort: null + labels: + - kind:code + notes: null + epic: 3998 diff --git a/docs/roadmaps/roadmap.yaml b/docs/roadmaps/roadmap.yaml index 30bf23ab3e..a0e35c19e6 100644 --- a/docs/roadmaps/roadmap.yaml +++ b/docs/roadmaps/roadmap.yaml @@ -24382,6 +24382,24 @@ Not done, pending the ruling: moving the 31 fixture files to an obviously synthe - kind:code notes: null epic: 3998 +- id: PMAT-4936 + github_issue: 4936 + item_type: task + title: 'x86-main: fence the fat_driver case-table annotations; section git fetch reads up to 3 times (P6)' + status: inprogress + priority: high + assigned_to: null + created: 2026-10-08T14:17:11Z + updated: 2026-10-08T14:17:16.352731207+00:00 + spec: null + acceptance_criteria: [] + phases: [] + subtasks: [] + estimated_effort: null + labels: + - kind:code + notes: null + epic: 3998 - id: PMAT-4948 github_issue: 4948 item_type: task diff --git a/scripts/check_roadmap_diff_additive.sh b/scripts/check_roadmap_diff_additive.sh index 67c03af079..0434184309 100755 --- a/scripts/check_roadmap_diff_additive.sh +++ b/scripts/check_roadmap_diff_additive.sh @@ -419,6 +419,50 @@ EOF *"is not a merge commit nor a commit on the origin/main tip"*) printf 'ok row %-2s stacked entry with deepening disabled (mutation): refused by name\n' "$row" ;; *) printf 'FAIL row %-2s stacked entry with deepening disabled: refused for the wrong reason (rc=%s): %s\n' "$row" "$rc7" "$err7"; fails=1 ;; esac + # #4936 (P6): the same stacked entry with the deepen's first read dead (curl 92). fetch_p6.sh reads the + # deepen again and the base still resolves. A planted git logs every fetch read; both must be the + # deepen, so a deepen that is one bare read (it dies, and the by-sha fetch after it answers) is RED. + # It runs from a cwd outside REPO_ROOT (the shim's own dir), as check_ont_ratchet.sh may (it never cds), + # and git finds no repository above that cwd: a deepen read that lost its -C "$REPO_ROOT" fails, the row is + # RED. The deepen answers on its second read, so the by-sha read never runs here; the next row covers it. + row=$((row + 1)) + rm -rf "${Q:?}.clone"; git clone -q --depth=1 -b main "file://$Q" "$Q.clone" 2>/dev/null; git -C "$Q.clone" fetch -q --depth=1 origin '+queue-base:refs/remotes/origin/main' 2>/dev/null + S="$TD/p6-shim"; rm -rf "${S:?}"; mkdir -p "$S" + printf '%s\n' '#!/usr/bin/env bash' \ + 'case " $* " in *" fetch "*)' \ + ' printf "%s\n" "$*" >> "$P6_SHIM_LOG"' \ + ' if [ "$(wc -l < "$P6_SHIM_LOG")" -eq 1 ]; then' \ + " printf '%s\\n' 'error: RPC failed; curl 92 HTTP/2 stream 5 was not closed cleanly: CANCEL (err 8)' 'fatal: early EOF' >&2; exit 128" \ + ' fi ;;' \ + 'esac' \ + 'exec "$P6_SHIM_REAL" "$@"' > "$S/git" + printf '%s\n' '#!/usr/bin/env bash' 'exit 0' > "$S/sleep"; chmod +x "$S/git" "$S/sleep"; : > "$S/reads" + got8=$( PATH="$S:$PATH" P6_SHIM_REAL="$(command -v git)" P6_SHIM_LOG="$S/reads" GITHUB_EVENT_NAME=merge_group GIT_CEILING_DIRECTORIES="$TD" bash -c 'cd "$2" || exit 2; if git rev-parse --git-dir >/dev/null 2>&1; then printf "cwd is a repo|"; else printf "no repo|"; fi; . "$0" --lib-only; REPO_ROOT="$1"; resolve_base HEAD && printf "%s|%s" "$BASE_REF" "$BASE_HOW"' "$SELF" "$Q.clone" "$S" 2>/dev/null ) || true + reads8="$(grep -c -e '--deepen=1' "$S/reads") of $(wc -l < "$S/reads" | tr -d ' ')" + case "$reads8|$got8" in "2 of 2|no repo|$want6|single parent (stacked merge_group entry"*) printf 'ok row %-2s stacked entry, the deepen read dead once (curl 92), from a cwd that is no repo: read again under P6 in REPO_ROOT, base = the previous entry squash\n' "$row" ;; + *) printf 'FAIL row %-2s stacked entry, deepen read dead once: wanted 2 of 2 deepen reads, a cwd that is no repo and base %s; got %s deepen reads, %s\n' "$row" "$want6" "$reads8" "$got8"; fails=1 ;; esac + # #4936 (P6): the same entry with all 3 deepen reads dead, then the by-sha read dead once. P6 reads the by-sha + # fetch again and the base still resolves. Same cwd and ceiling as the row above: a by-sha read that lost its + # -C "$REPO_ROOT" runs in no repo, and one that skips P6 is never read again; either way the row is RED. + # Wanted: 3 deepen reads, then 2 by-sha reads. + row=$((row + 1)) + rm -rf "${Q:?}.clone"; git clone -q --depth=1 -b main "file://$Q" "$Q.clone" 2>/dev/null; git -C "$Q.clone" fetch -q --depth=1 origin '+queue-base:refs/remotes/origin/main' 2>/dev/null + printf '%s\n' '#!/usr/bin/env bash' \ + 'case " $* " in *" fetch "*)' \ + ' printf "%s\n" "$*" >> "$P6_SHIM_LOG"' \ + ' dead=; case " $* " in *" --deepen=1 "*) dead=1 ;; *) [ "$(grep -vc -e --deepen=1 "$P6_SHIM_LOG")" -eq 1 ] && dead=1 ;; esac' \ + ' if [ -n "$dead" ]; then' \ + " printf '%s\\n' 'error: RPC failed; curl 92 HTTP/2 stream 5 was not closed cleanly: CANCEL (err 8)' 'fatal: early EOF' >&2; exit 128" \ + ' fi ;;' \ + 'esac' \ + 'exec "$P6_SHIM_REAL" "$@"' > "$S/git" + : > "$S/reads" + got9=$( PATH="$S:$PATH" P6_SHIM_REAL="$(command -v git)" P6_SHIM_LOG="$S/reads" GITHUB_EVENT_NAME=merge_group GIT_CEILING_DIRECTORIES="$TD" bash -c 'cd "$2" || exit 2; if git rev-parse --git-dir >/dev/null 2>&1; then printf "cwd is a repo|"; else printf "no repo|"; fi; . "$0" --lib-only; REPO_ROOT="$1"; resolve_base HEAD && printf "%s|%s" "$BASE_REF" "$BASE_HOW"' "$SELF" "$Q.clone" "$S" 2>/dev/null ) || true + reads9="$(grep -c -e '--deepen=1' "$S/reads") of $(wc -l < "$S/reads" | tr -d ' ')" + case "$reads9|$got9" in "3 of 5|no repo|$want6|single parent (stacked merge_group entry"*) printf 'ok row %-2s stacked entry, all 3 deepen reads dead and the by-sha read dead once, from a cwd that is no repo: by-sha read again under P6 in REPO_ROOT, base = the previous entry squash\n' "$row" ;; + *) printf 'FAIL row %-2s stacked entry, all 3 deepen reads dead and the by-sha read dead once: wanted 3 of 5 deepen reads, a cwd that is no repo and base %s; got %s deepen reads, %s\n' "$row" "$want6" "$reads9" "$got9"; fails=1 ;; esac + rm -rf "${S:?}" + rm -rf "${Q:?}" "${Q:?}.clone" # Rows 19-22: --staged, the pre-commit entry point (B4, #3047). A scratch diff --git a/scripts/ci/fence_workflow_commands.sh b/scripts/ci/fence_workflow_commands.sh new file mode 100644 index 0000000000..211461c6c7 --- /dev/null +++ b/scripts/ci/fence_workflow_commands.sh @@ -0,0 +1,284 @@ +#!/usr/bin/env bash +# fence_workflow_commands.sh -- run a case table with GitHub's workflow-command +# processing stopped for its output, so a planted row cannot raise a live +# annotation on the job (#4936). +# +# WHY THIS EXISTS +# --------------- +# `fat_driver.py self-test` drives the real code paths with planted failures, +# and those paths print real `::error::` workflow commands: "section a: +# failure", "section a: cancelled" and "external job 'workspace-test': not +# completed within 12000s". The table passed 60/60, yet every x86-main and +# workspace-test shard job carried those three as error annotations about a +# minute in. x86-main runs no --external-job at all, so its 12000s line could +# only ever come from the table; when the job went red half an hour later on +# another section, the annotation was read as the cause (#4813, #4913). +# +# The fence is GitHub's own `::stop-commands::` ... `::::`. The +# token is drawn fresh per call and never exported, so the child cannot end +# the fence early. The child's stderr is merged into stdout so every line +# crosses one pipe in order and none lands after the resume line. The verdict +# is untouched: the child's exit status is the step's, and a failing table gets +# one live ::error:: naming it, after the fence. +# +# Only case tables are fenced. `fat_driver.py run` and `wait` stay unfenced: +# their deadline ::error:: is the real one, and the wiring check refuses a +# fenced one. +# +# bash scripts/ci/fence_workflow_commands.sh -- python3 scripts/ci/fat_driver.py self-test +# bash scripts/ci/fence_workflow_commands.sh # wiring check of ci.yml +# bash scripts/ci/fence_workflow_commands.sh --self-test +# bash scripts/ci/fence_workflow_commands.sh --live # live annotation count +set -uo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +WORKFLOW_REL=".github/workflows/ci.yml" +FENCE_REL="scripts/ci/fence_workflow_commands.sh" + +# fenced -> the child's output between a stop line and its resume +# line. rc = the child's rc; 2 when no command is given or no token is drawn. +fenced() { + if [ "$#" -eq 0 ]; then + printf '::error::%s: no command to fence\n' "$FENCE_REL" + return 2 + fi + local tok rc + tok="$(od -An -N16 -tx1 /dev/urandom | tr -d ' \n')" + if [ "${#tok}" -ne 32 ]; then + printf '::error::%s: no stop-commands token drawn\n' "$FENCE_REL" + return 2 + fi + printf '::stop-commands::%s\n' "$tok" + "$@" 2>&1 + rc=$? + # The resume line must start a line. A child whose last line has no newline + # would get it glued on, the fence would never close, and the failure + # ::error:: below would be swallowed too. One blank line is the cost. + printf '\n::%s::\n' "$tok" + if [ "$rc" -ne 0 ]; then + printf '::error::case table failed (rc=%s): %s\n' "$rc" "$*" + fi + return "$rc" +} + +# live_commands -> how many annotation commands the runner would act on, +# outside every stop..resume region. Each rule errs on the high side, so a 0 +# here is a 0 on the runner and a 1 is at most 1: +# - Lines split on \r as well as \n, as the runner splits them. The runner +# acts on at most one command per line, so a line counts once. +# - An annotation is ::error, ::warning or ::notice, or the legacy ##[error, +# ##[warning or ##[notice, anywhere in a line and in any case. The runner +# wants it first after any whitespace. +# - A region starts only at the fence's own stop line: column 0, lower case, +# a 32-hex token and nothing after it. +# - A region ends at the first line holding :: or ##[ in any +# case. That line is never live: it is the resume, or the runner is still +# stopped. +# - Doubt: any other line naming stop-commands may stop the runner, and a +# region that ends on a line other than :::: may not resume it. +# After either, no region starts again, so every later annotation counts. +# Problem matchers are not workflow commands: the fence does not stop them, +# and this does not count them. +live_commands() { + LC_ALL=C awk ' + function one(s, lc, t) { + lc = tolower(s) + if (stop != "") { + if (index(lc, "::" stop) == 0 && index(lc, "##[" stop) == 0) return + if (lc != "::" stop "::") doubt = 1 + stop = ""; return + } + if (index(lc, "stop-commands")) { + t = substr(s, 18) + if (!doubt && substr(s, 1, 17) == "::stop-commands::" && length(t) == 32 && t !~ /[^0-9a-f]/) { stop = t; return } + doubt = 1 + } + if (lc ~ /(::|##[[])(error|warning|notice)/) n++ + } + { k = split($0, part, "\r"); for (i = 1; i <= k; i++) one(part[i]) } + END { print n + 0 }' "$1" +} + +# check_wiring -> one FAIL line per defect. rc 0 = every +# `fat_driver.py self-test` line is exactly the fenced command, alone after an +# optional `run:` (so nothing on the line masks its rc), and no +# `fat_driver.py run` or `wait` is fenced; 1 = a defect; 2 = unreadable, or no +# self-test step found. +check_wiring() { + [ -r "$1" ] || return 2 + local bad=0 seen=0 line cmd + while IFS= read -r line; do + seen=$((seen + 1)) + cmd="$(printf '%s' "$line" | sed -E 's/^[[:space:]]*(-[[:space:]]+)?(run:[[:space:]]+)?//; s/[[:space:]]+$//')" + case "$cmd" in + "bash $FENCE_REL -- python3 scripts/ci/fat_driver.py self-test") ;; + *) + printf 'FAIL: %s runs the fat_driver case table other than as exactly `bash %s -- python3 scripts/ci/fat_driver.py self-test` -- unfenced, its planted rows raise live ::error:: annotations; with more on the line, its rc can be masked: %s\n' \ + "$WORKFLOW_REL" "$FENCE_REL" "$(printf '%s' "$line" | sed -E 's/^[[:space:]]+//')" + bad=1 ;; + esac + done < <(grep -E 'fat_driver\.py[[:space:]]+self-test' "$1") + while IFS= read -r line; do + printf 'FAIL: %s fences a real fat_driver run -- its deadline ::error:: must stay live: %s\n' \ + "$WORKFLOW_REL" "$(printf '%s' "$line" | sed -E 's/^[[:space:]]+//')" + bad=1 + done < <(grep -E "$FENCE_REL.*fat_driver\\.py[[:space:]]+(run|wait)" "$1") + [ "$seen" -gt 0 ] || return 2 + return "$bad" +} + +self_test() { + printf '=== case table: fence_workflow_commands.sh ===\n' + local tmp fails=0 rc t1 t2 t3 tk tk2 got i f good + tmp="$(mktemp -d)" + trap 'rm -rf "${tmp:?}"' RETURN + row() { # row