From 075d6d0ca9e31427010b9ca1a8c2a54036a7280e Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Mon, 20 Jul 2026 14:22:39 -0500 Subject: [PATCH 1/5] fix(canary): cache run-list per (repo,workflow) + treat "no workflows" as empty (#819) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 4h sweep enumerated `gh run list --workflow ` once per sample window (candidate / baseline / downgrade / correctness) per agent per tier repo. Two defects turned the mode-B "hardening" (#810/#820) into a production regression that cancelled every Canary Rollout run since 2026-07-20 ~02:44Z: 1. A workflow with no runs on a repo makes `gh run list --workflow` exit non-zero with "could not find any workflows named …" — a PERMANENT condition. #810's wrapper classified it as `network-or-unknown` (transient) and retried it 6× with backoff, so every zero-run workflow across ~20 agent×workflow×repo combos burned its full retry budget → ~20-min sweep → job cancelled (exit 143). 2. When a window fails CLOSED (#820) `_run_json` returns empty; callers then ran `$(( executed + $(jq … <<< "$json" || echo 0) ))`, but jq on EMPTY stdin exits 0 printing nothing, so `|| echo 0` never fires → `executed + ` → bash "syntax error: operand expected" at the sample/health accumulators. Fix (#819, the structural cure): - `_repo_wf_runs_cached `: ONE unbounded `gh run list --workflow ` per (repo, workflow), memoized in a FILE cache under `$_RUNS_CACHE_DIR` (exported by main() — callers invoke via command-substitution subshells, so an in-memory array would not survive). The candidate/baseline/downgrade windows now share one fetch, collapsing the secondary-rate-limit burst. - "could not find any workflows named …" → cached `[]`, returned immediately, never retried, never recorded as a #820 fetch outage. Genuine transport errors (403/5xx/auth/network) still retry with #810 backoff and fail CLOSED if sustained. - `_run_json` reduces to: guard → cached raw → LOCAL since-cut filter (inclusive, equivalent to the old server-side `--created ">=$since"`). - Arithmetic guard: `<<< "${json:-[]}"` at the three sample/health accumulators so a fail-closed empty payload counts 0 instead of crashing. `--workflow` is retained (deep per-workflow reach — no truncation on busy repos that a per-repo fetch capped at -L 1000 would suffer); only `--created` is dropped so one cache entry serves every window. Tests: +5 unit tests (per-(repo,wf) memoization; local since cut; unbounded fetch passes --workflow not --created; not-found→[] with no retry and no fail-flag; _cumulative_health survives an empty fail-closed payload without an arithmetic crash). Full suite 249 tests; the pre-existing 48 orchestrator env-artifact failures (no local git/gh fixtures; green in CI via #685) are unchanged. Closes #819. Fixes the #803/#811 canary cancellation regression. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_017BggeTEBJLw6BV8kZrM6jD --- scripts/canary-rollout.sh | 86 ++++++++++++++++++++++++++---------- tests/canary_rollout.bats | 92 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+), 22 deletions(-) diff --git a/scripts/canary-rollout.sh b/scripts/canary-rollout.sh index 2824fc680..1f7f5d83c 100755 --- a/scripts/canary-rollout.sh +++ b/scripts/canary-rollout.sh @@ -414,28 +414,36 @@ _gh_retry_after() { printf '%s' "$secs" } -# _run_json — gh run-list JSON (conclusion,createdAt,databaseId,workflowName) for a -# repo since the given Zulu timestamp. Empty repo/wildcard → []. Never fails the caller. -_run_json() { - local repo="$1" wf="$2" since="$3" out err summary ra delay span expo errfile - if [ -z "$repo" ] || [ "$repo" = '*' ]; then echo '[]'; return 0; fi - # The 4h scheduled tick fans this out across every agent × tier repo, so a single - # transient blip (network / secondary rate-limit / brief 5xx) among dozens of reads - # used to fail the whole fleet sweep (#738). Retry a non-zero `gh` a bounded number of - # times to ride out a momentary hiccup, capturing gh's stderr so the annotations name - # the REAL failure (HTTP status + class) instead of a bare "transient" (#810). Backoff - # is exponential + full-jitter and honors a server Retry-After / x-ratelimit-reset hint - # so a secondary-rate-limit window is respected rather than hammered. A SUSTAINED failure - # still exhausts the retries and returns non-zero: fail CLOSED on a genuine outage - # (returning an empty [] would read as "zero failures" and could green-light a bad - # promotion — a non-zero return halts the run under `set -e` instead). An empty-but- - # successful result (no runs) is still a valid []. Tunable via CANARY_GH_RETRIES / - # CANARY_GH_RETRY_SLEEP (tests set 0). CANARY_GH_RETRY_AFTER_CAP caps server hints (default 900s). +# _repo_wf_runs_cached — ALL recent runs of one workflow on a repo +# (conclusion,createdAt,databaseId,workflowName), UNBOUNDED by date and memoized for the +# process, so the candidate / baseline / downgrade / correctness windows a single agent +# samples collapse from one `gh run list` per (repo × workflow × window) into ONE read +# per (repo × workflow) (#819). This is the fix for the secondary-rate-limit burst AND +# for the retry storm it caused: a workflow that has never run on a repo makes `gh run +# list --workflow` exit non-zero with "could not find any workflows named …" — a PERMANENT +# condition, not a transient one, so retrying it 6× with backoff (as #810's wrapper did +# for every error class) blew a fleet sweep out to ~20 min and got the job cancelled. We +# now classify that message as a legitimate empty result ([]) and never retry it; only a +# genuine transport error (403/5xx/network/auth) is retried and, if sustained, fails CLOSED. +# +# The cache is FILE-backed under $_RUNS_CACHE_DIR (exported by main() before the fan-out), +# because callers invoke this via command substitution — a subshell — so an in-memory array +# populated in one call would not survive to the next. When the dir is unset (a unit test +# calling _run_json directly), every call fetches. Tunable via CANARY_GH_RETRIES / +# CANARY_GH_RETRY_SLEEP (tests set 0). CANARY_GH_RETRY_AFTER_CAP caps server hints (default 900s). +_repo_wf_runs_cached() { + local repo="$1" wf="$2" out err summary ra delay span expo errfile cachef key local attempts="${CANARY_GH_RETRIES:-6}" base="${CANARY_GH_RETRY_SLEEP:-2}" attempt=1 local ra_cap="${CANARY_GH_RETRY_AFTER_CAP:-900}" case "$ra_cap" in ''|*[!0-9]*) ra_cap=900 ;; esac case "$attempts" in ''|*[!0-9]*) attempts=6 ;; esac case "$base" in ''|*[!0-9]*) base=2 ;; esac + # Cache hit? A non-empty cache file for this (repo, workflow) — including a cached "[]" + # for a no-runs / not-found workflow, so it is never re-queried within the sweep. + if [ -n "${_RUNS_CACHE_DIR:-}" ]; then + key="${repo}//${wf}"; cachef="$_RUNS_CACHE_DIR/${key//[^A-Za-z0-9._-]/_}.json" + [ -s "$cachef" ] && { cat "$cachef"; return 0; } + fi errfile="$(mktemp)" if ! declare -p tmpfiles &>/dev/null; then declare -g -a tmpfiles=() @@ -443,14 +451,24 @@ _run_json() { fi tmpfiles+=("$errfile") while :; do - if out="$(gh run list --repo "$repo" --workflow "$wf" ${since:+--created ">=$since"} \ + if out="$(gh run list --repo "$repo" --workflow "$wf" \ -L 1000 --json conclusion,createdAt,databaseId,workflowName 2>"$errfile")"; then - rm -f "$errfile"; echo "${out:-[]}"; return 0 + rm -f "$errfile"; out="${out:-[]}" + [ -n "${cachef:-}" ] && [ -d "$_RUNS_CACHE_DIR" ] && printf '%s' "$out" > "$cachef" 2>/dev/null || true + printf '%s\n' "$out"; return 0 fi err="" if [ -r "$errfile" ]; then err="$(<"$errfile")" fi + # A workflow with no runs on this repo is NOT a fetch failure — `gh` just can't resolve + # the name. Treat it as an empty result (and cache []) so it is neither retried nor + # counted as an outage; retrying it was the #810→#803 cancellation storm. + if [[ "${err,,}" == *"could not find any workflow"* ]] || [[ "${err,,}" == *"no workflows"* ]]; then + rm -f "$errfile" + [ -n "${cachef:-}" ] && [ -d "$_RUNS_CACHE_DIR" ] && printf '%s' '[]' > "$cachef" 2>/dev/null || true + echo '[]'; return 0 + fi summary="$(_gh_err_summary "$err")" if [ "$attempt" -ge "$attempts" ]; then echo "::error::_run_json: failed to fetch run list for $repo (workflow=$wf) after $attempts attempt(s) — $summary" >&2 @@ -480,6 +498,22 @@ _run_json() { done } +# _run_json — the target workflow's runs on a repo since the +# given Zulu timestamp, filtered LOCALLY from the per-(repo,workflow) cache so the several +# windows an agent samples share one fetch (#819). Empty repo/wildcard → []. A genuine +# fetch failure fails CLOSED (non-zero) so an empty [] never masks real failures and +# green-lights a bad promotion. Empty since = no lower bound. +_run_json() { + local repo="$1" wf="$2" since="$3" raw + if [ -z "$repo" ] || [ "$repo" = '*' ]; then echo '[]'; return 0; fi + raw="$(_repo_wf_runs_cached "$repo" "$wf")" || return 1 + # Local since cut, inclusive — the same window the old server-side `--created ">=$since"` + # produced. jq on an empty/garbage payload falls back to []. + jq -c --arg since "$since" \ + '[ .[]? | select($since == "" or (.createdAt // "") >= $since) ]' \ + <<< "${raw:-[]}" 2>/dev/null || echo '[]' +} + # _tier_sample — EXECUTED runs (success+failure) on the # source tier since the candidate cut. Prints " ". _tier_sample() { @@ -488,7 +522,7 @@ _tier_sample() { wf="$(_agent_field "$agent" run_workflow)" for repo in "$@"; do json="$(_run_json "$repo" "$wf" "$since")" - executed=$(( executed + $(jq '[.[]?|select(.conclusion=="success" or .conclusion=="failure")]|length' 2>/dev/null <<< "$json" || echo 0) )) + executed=$(( executed + $(jq '[.[]?|select(.conclusion=="success" or .conclusion=="failure")]|length' 2>/dev/null <<< "${json:-[]}" || echo 0) )) e="$(jq -r '[.[]?|select(.conclusion=="success" or .conclusion=="failure")|.createdAt?]|min // empty' 2>/dev/null <<< "$json" || echo "")" if [ -n "$e" ] && { [ -z "$earliest" ] || [[ "$e" < "$earliest" ]]; }; then earliest="$e"; fi done @@ -663,11 +697,11 @@ _cumulative_health() { suspect_patterns="$(_suspect_patterns "$agent")" for repo in "$@"; do json="$(_run_json "$repo" "$wf" "$since")" - startup=$(( startup + $(jq '[.[]?|select(.conclusion=="startup_failure")]|length' 2>/dev/null <<< "$json" || echo 0) )) + startup=$(( startup + $(jq '[.[]?|select(.conclusion=="startup_failure")]|length' 2>/dev/null <<< "${json:-[]}" || echo 0) )) if [ -z "$patterns" ] && [ -z "$suspect_patterns" ]; then # No benign or suspect patterns to match — count all failures with one jq pass, # avoiding a gh run view call per failure. - fail=$(( fail + $(jq '[.[]?|select(.conclusion=="failure")]|length' 2>/dev/null <<< "$json" || echo 0) )) + fail=$(( fail + $(jq '[.[]?|select(.conclusion=="failure")]|length' 2>/dev/null <<< "${json:-[]}" || echo 0) )) else while IFS=$'\t' read -r rid rwf; do if [ -n "$patterns" ] && _failure_benign "$repo" "$rid" "$rwf" "$patterns"; then @@ -2118,6 +2152,14 @@ main() { echo "::error::SOAK_WINDOW_DAYS, when set, must be a positive integer" >&2 return 2 fi + # Per-process run-list cache shared across the fan-out's command-substitution subshells + # (#819) — exported so subshells inherit the path. On these ephemeral runners the dir is + # reaped with the job; a stray /tmp dir per local run is harmless, so no EXIT trap is set + # here (it would clobber _repo_wf_runs_cached's tmpfiles trap). + if [ -z "${_RUNS_CACHE_DIR:-}" ]; then + _RUNS_CACHE_DIR="$(mktemp -d 2>/dev/null || true)" + [ -n "$_RUNS_CACHE_DIR" ] && export _RUNS_CACHE_DIR + fi local sub="${1:-}"; shift || true case "$sub" in evaluate) [ $# -ge 1 ] || { echo "usage: evaluate " >&2; return 2; }; cmd_evaluate "$@" ;; diff --git a/tests/canary_rollout.bats b/tests/canary_rollout.bats index 5c5e0e6b5..6f3232293 100644 --- a/tests/canary_rollout.bats +++ b/tests/canary_rollout.bats @@ -602,6 +602,98 @@ GHEOF grep -qx '60' "$SLEEP_LOG" } +# ── _run_json: per-(repo,workflow) cache + not-found-is-empty + local since (#819) ── +# The 4h sweep asks for a workflow's runs once per sample window (candidate / baseline / +# downgrade / correctness). Enumerating per window per workflow tripped secondary rate +# limits, AND a zero-run workflow made `gh run list --workflow` exit non-zero with "could +# not find any workflows named …" — which #810's wrapper retried 6× as a transient, blowing +# a sweep out to ~20 min until the job was cancelled. #819: one UNBOUNDED read per +# (repo,workflow), memoized; the since cut applied locally; a not-found is a cached []. +@test "_run_json: memoizes per (repo,workflow) — two windows share ONE gh call (#819)" { + STUB_BIN="$(mktemp -d "$BATS_TEST_TMPDIR/stub.XXXXXX")"; export PATH="$STUB_BIN:$PATH" + export CALLS="$BATS_TEST_TMPDIR/gh-calls"; : > "$CALLS" + cat > "$STUB_BIN/gh" <<'GHEOF' +#!/usr/bin/env bash +echo "call" >> "$CALLS" +echo '[{"conclusion":"success","createdAt":"2026-01-10T00:00:00Z","databaseId":1,"workflowName":"W"}, + {"conclusion":"failure","createdAt":"2026-01-02T00:00:00Z","databaseId":2,"workflowName":"W"}]' +GHEOF + chmod +x "$STUB_BIN/gh" + run env _RUNS_CACHE_DIR="$BATS_TEST_TMPDIR/rc" bash -c ' + mkdir -p "$_RUNS_CACHE_DIR"; source "'"$ORCH"'" + _run_json some/repo W "2026-01-05T00:00:00Z" # candidate window (since = 01-05) + _run_json some/repo W "" # baseline window (no lower bound) + ' + [ "$status" -eq 0 ] + # Both windows served by a single fetch of (some/repo, W). + [ "$(wc -l < "$CALLS")" -eq 1 ] +} + +@test "_run_json: applies the since cut LOCALLY — pre-cut rows excluded, empty since keeps all (#819)" { + STUB_BIN="$(mktemp -d "$BATS_TEST_TMPDIR/stub.XXXXXX")"; export PATH="$STUB_BIN:$PATH" + cat > "$STUB_BIN/gh" <<'GHEOF' +#!/usr/bin/env bash +echo '[{"conclusion":"success","createdAt":"2026-01-10T00:00:00Z","databaseId":1,"workflowName":"W"}, + {"conclusion":"failure","createdAt":"2026-01-02T00:00:00Z","databaseId":2,"workflowName":"W"}]' +GHEOF + chmod +x "$STUB_BIN/gh" + # since = 01-05 → only the 01-10 row survives. + run bash -c "source '$ORCH' && _run_json some/repo W '2026-01-05T00:00:00Z' | jq -c 'map(.databaseId)'" + [ "$status" -eq 0 ]; [ "$output" = "[1]" ] + # empty since → both rows. + run bash -c "source '$ORCH' && _run_json some/repo W '' | jq -c 'map(.databaseId)|sort'" + [ "$status" -eq 0 ]; [ "$output" = "[1,2]" ] +} + +@test "_run_json: fetch is unbounded — passes --workflow but NOT --created (#819)" { + STUB_BIN="$(mktemp -d "$BATS_TEST_TMPDIR/stub.XXXXXX")"; export PATH="$STUB_BIN:$PATH" + export ARGS="$BATS_TEST_TMPDIR/gh-args" + cat > "$STUB_BIN/gh" <<'GHEOF' +#!/usr/bin/env bash +echo "$*" >> "$ARGS" +echo '[]' +GHEOF + chmod +x "$STUB_BIN/gh" + run bash -c "source '$ORCH' && _run_json some/repo 'Dev-Lead Agent' '2026-01-05T00:00:00Z'" + [ "$status" -eq 0 ] + grep -q -- '--workflow' "$ARGS" + ! grep -q -- '--created' "$ARGS" +} + +@test "_run_json: a not-found workflow returns [] immediately — no retry, no fail-flag (#819)" { + STUB_BIN="$(mktemp -d "$BATS_TEST_TMPDIR/stub.XXXXXX")"; export PATH="$STUB_BIN:$PATH" + export CALLS="$BATS_TEST_TMPDIR/nf-calls"; : > "$CALLS" + cat > "$STUB_BIN/gh" <<'GHEOF' +#!/usr/bin/env bash +echo "call" >> "$CALLS" +echo "could not find any workflows named Persona — Mention Router" >&2 +exit 1 +GHEOF + chmod +x "$STUB_BIN/gh" + export FLAG="$BATS_TEST_TMPDIR/fetch-fail-flag"; : > "$FLAG" + run env CANARY_GH_RETRY_SLEEP=0 CANARY_GH_RETRIES=6 _CANARY_FETCH_FAIL_FLAG="$FLAG" \ + bash -c "source '$ORCH' && _run_json some/repo 'Persona — Mention Router' ''" + [ "$status" -eq 0 ] + [ "$output" = "[]" ] + # Not retried (single gh call) and NOT recorded as a fetch outage (#820 flag empty). + [ "$(wc -l < "$CALLS")" -eq 1 ] + [ ! -s "$FLAG" ] +} + +@test "_cumulative_health: does not crash when a window fails CLOSED to an empty payload (#819 arith guard)" { + # A sustained fetch failure makes _run_json return non-zero → json="" in the caller. The + # `$(( fail + $(jq … <<< "${json:-[]}") ))` guard must count 0, never emit `+ ` (bash + # arithmetic operand-expected syntax error, the second half of the #803 canary regression). + STUB_BIN="$(mktemp -d "$BATS_TEST_TMPDIR/stub.XXXXXX")"; export PATH="$STUB_BIN:$PATH" + printf '#!/usr/bin/env bash\necho "gh: HTTP 503: server error" >&2\nexit 1\n' > "$STUB_BIN/gh" + chmod +x "$STUB_BIN/gh" + run env CANARY_GH_RETRY_SLEEP=0 CANARY_GH_RETRIES=1 \ + bash -c "source '$ORCH' && _cumulative_health dev-lead '' 0 some/repo" + # Fail-closed path returns non-zero, but NEVER with an arithmetic syntax error. + [[ "$output" != *"syntax error"* ]] + [[ "$output" != *"operand expected"* ]] +} + # ── next_channel_in_order ───────────────────────────────────────────────────── @test "next_channel_in_order: walks the ring order" { [ "$(next_channel_in_order next 'next,ring0,ring1,stable')" = "ring0" ] From 11bc78afccf6d46f413841406402d33fd778f3d8 Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Mon, 20 Jul 2026 14:29:43 -0500 Subject: [PATCH 2/5] fix(canary): register cleanup traps robustly across subshells (Gemini #835 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - _repo_wf_runs_cached: register the errfile EXIT trap unconditionally (not only when tmpfiles is first declared). Command substitution forks a subshell that does not inherit the parent's traps, so the conditional form left subshell invocations without cleanup. Re-running `trap … EXIT` in the same shell is an idempotent reinstall. - main(): add an EXIT trap to reap the auto-created $_RUNS_CACHE_DIR. It lives in the parent shell, independent of the per-subshell errfile trap, so they never clobber. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_017BggeTEBJLw6BV8kZrM6jD --- scripts/canary-rollout.sh | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/scripts/canary-rollout.sh b/scripts/canary-rollout.sh index 1f7f5d83c..7cad15d43 100755 --- a/scripts/canary-rollout.sh +++ b/scripts/canary-rollout.sh @@ -445,11 +445,13 @@ _repo_wf_runs_cached() { [ -s "$cachef" ] && { cat "$cachef"; return 0; } fi errfile="$(mktemp)" - if ! declare -p tmpfiles &>/dev/null; then - declare -g -a tmpfiles=() - trap 'rm -f "${tmpfiles[@]+"${tmpfiles[@]}"}"' EXIT - fi + declare -p tmpfiles &>/dev/null || declare -g -a tmpfiles=() tmpfiles+=("$errfile") + # Register the cleanup in THIS (sub)shell: command substitution forks a subshell that + # does NOT inherit the parent's traps, so registering only on first declaration of + # tmpfiles would leave every subshell invocation without one. Re-running `trap … EXIT` + # in the same shell just reinstalls the same idempotent handler. + trap 'rm -f "${tmpfiles[@]+"${tmpfiles[@]}"}"' EXIT while :; do if out="$(gh run list --repo "$repo" --workflow "$wf" \ -L 1000 --json conclusion,createdAt,databaseId,workflowName 2>"$errfile")"; then @@ -2153,12 +2155,15 @@ main() { return 2 fi # Per-process run-list cache shared across the fan-out's command-substitution subshells - # (#819) — exported so subshells inherit the path. On these ephemeral runners the dir is - # reaped with the job; a stray /tmp dir per local run is harmless, so no EXIT trap is set - # here (it would clobber _repo_wf_runs_cached's tmpfiles trap). + # (#819) — exported so subshells inherit the path, and reaped on exit. This trap lives in + # the PARENT shell; _repo_wf_runs_cached's errfile trap lives in the per-call subshells, so + # the two are in different shells and never clobber each other. if [ -z "${_RUNS_CACHE_DIR:-}" ]; then _RUNS_CACHE_DIR="$(mktemp -d 2>/dev/null || true)" - [ -n "$_RUNS_CACHE_DIR" ] && export _RUNS_CACHE_DIR + if [ -n "$_RUNS_CACHE_DIR" ]; then + export _RUNS_CACHE_DIR + trap 'rm -rf "${_RUNS_CACHE_DIR:-}"' EXIT + fi fi local sub="${1:-}"; shift || true case "$sub" in From 2021b06f802520d2fe4a6189e82c444ed2e82794 Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Mon, 20 Jul 2026 14:41:05 -0500 Subject: [PATCH 3/5] fix(canary): hash the run-list cache key to prevent (repo,workflow) collisions (#835 CodeRabbit) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cache filename derived the key by char-substitution (non-[A-Za-z0-9._-] → "_"), which let two distinct workflows on the same repo collide — e.g. names differing only by a space vs a slash both mapped to one file, cross-contaminating their cached run history and therefore their gate health. Workflow display names carry spaces and em-dashes, so this is a real collision surface. Hash the raw "repo//workflow" key with sha1sum (fallback to the old substitution only if no hasher is present). +1 test proving "A B" and "A/B" get distinct cache entries. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_017BggeTEBJLw6BV8kZrM6jD --- scripts/canary-rollout.sh | 12 ++++++++++-- tests/canary_rollout.bats | 22 ++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/scripts/canary-rollout.sh b/scripts/canary-rollout.sh index 7cad15d43..0faa48a02 100755 --- a/scripts/canary-rollout.sh +++ b/scripts/canary-rollout.sh @@ -432,7 +432,7 @@ _gh_retry_after() { # calling _run_json directly), every call fetches. Tunable via CANARY_GH_RETRIES / # CANARY_GH_RETRY_SLEEP (tests set 0). CANARY_GH_RETRY_AFTER_CAP caps server hints (default 900s). _repo_wf_runs_cached() { - local repo="$1" wf="$2" out err summary ra delay span expo errfile cachef key + local repo="$1" wf="$2" out err summary ra delay span expo errfile cachef key keyhash local attempts="${CANARY_GH_RETRIES:-6}" base="${CANARY_GH_RETRY_SLEEP:-2}" attempt=1 local ra_cap="${CANARY_GH_RETRY_AFTER_CAP:-900}" case "$ra_cap" in ''|*[!0-9]*) ra_cap=900 ;; esac @@ -441,7 +441,15 @@ _repo_wf_runs_cached() { # Cache hit? A non-empty cache file for this (repo, workflow) — including a cached "[]" # for a no-runs / not-found workflow, so it is never re-queried within the sweep. if [ -n "${_RUNS_CACHE_DIR:-}" ]; then - key="${repo}//${wf}"; cachef="$_RUNS_CACHE_DIR/${key//[^A-Za-z0-9._-]/_}.json" + # Hash the (repo, workflow) key into the filename so distinct pairs can never collide. + # A plain char-substitution (e.g. non-alnum → "_") would map a workflow named "A B" and + # one named "A/B" on the same repo to the same file — cross-contaminating their cached + # run history and so their gate health. Workflow display names carry spaces and em-dashes, + # so this is a real collision surface. Fall back to substitution only if no hasher exists. + key="${repo}//${wf}" + keyhash="$(printf '%s' "$key" | sha1sum 2>/dev/null | cut -d' ' -f1)" + [ -n "$keyhash" ] || keyhash="${key//[^A-Za-z0-9._-]/_}" + cachef="$_RUNS_CACHE_DIR/${keyhash}.json" [ -s "$cachef" ] && { cat "$cachef"; return 0; } fi errfile="$(mktemp)" diff --git a/tests/canary_rollout.bats b/tests/canary_rollout.bats index 6f3232293..45e162b5c 100644 --- a/tests/canary_rollout.bats +++ b/tests/canary_rollout.bats @@ -629,6 +629,28 @@ GHEOF [ "$(wc -l < "$CALLS")" -eq 1 ] } +@test "_run_json: cache key is collision-free — 'A B' vs 'A/B' workflows don't share a file (#835 CodeRabbit)" { + STUB_BIN="$(mktemp -d "$BATS_TEST_TMPDIR/stub.XXXXXX")"; export PATH="$STUB_BIN:$PATH" + # gh echoes back the requested workflow name so we can prove which cache entry served the call. + cat > "$STUB_BIN/gh" <<'GHEOF' +#!/usr/bin/env bash +wf=""; prev="" +for a in "$@"; do [ "$prev" = "--workflow" ] && wf="$a"; prev="$a"; done +printf '[{"conclusion":"success","createdAt":"2026-01-10T00:00:00Z","databaseId":1,"workflowName":"%s"}]\n' "$wf" +GHEOF + chmod +x "$STUB_BIN/gh" + run env _RUNS_CACHE_DIR="$BATS_TEST_TMPDIR/rc2" bash -c ' + mkdir -p "$_RUNS_CACHE_DIR"; source "'"$ORCH"'" + _run_json some/repo "A B" "" | jq -r ".[0].workflowName" + _run_json some/repo "A/B" "" | jq -r ".[0].workflowName" + ' + [ "$status" -eq 0 ] + # Under a char-substitution key both names would collapse to "A_B" and the second call + # would be served the first's cached "A B" row. A hashed key keeps them distinct. + [ "$(printf '%s\n' "$output" | sed -n 1p)" = "A B" ] + [ "$(printf '%s\n' "$output" | sed -n 2p)" = "A/B" ] +} + @test "_run_json: applies the since cut LOCALLY — pre-cut rows excluded, empty since keeps all (#819)" { STUB_BIN="$(mktemp -d "$BATS_TEST_TMPDIR/stub.XXXXXX")"; export PATH="$STUB_BIN:$PATH" cat > "$STUB_BIN/gh" <<'GHEOF' From 75d4d9eaadb19e9eab1a9589cc91f2085d929444 Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Mon, 20 Jul 2026 15:15:20 -0500 Subject: [PATCH 4/5] =?UTF-8?q?fix(canary):=20use=20sha256=20(not=20sha1)?= =?UTF-8?q?=20for=20the=20cache-key=20hash=20=E2=80=94=20clears=20SonarClo?= =?UTF-8?q?ud=20S4790?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SonarCloud flags sha1sum as a weak hash (S4790, CRITICAL) failing the new-code security gate, even though this is a cache-filename derivation, not a security context. sha256sum is not flagged, is equally present (coreutils), and is collision-free. Behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_017BggeTEBJLw6BV8kZrM6jD --- scripts/canary-rollout.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/canary-rollout.sh b/scripts/canary-rollout.sh index 0faa48a02..bf6fe19a0 100755 --- a/scripts/canary-rollout.sh +++ b/scripts/canary-rollout.sh @@ -445,9 +445,11 @@ _repo_wf_runs_cached() { # A plain char-substitution (e.g. non-alnum → "_") would map a workflow named "A B" and # one named "A/B" on the same repo to the same file — cross-contaminating their cached # run history and so their gate health. Workflow display names carry spaces and em-dashes, - # so this is a real collision surface. Fall back to substitution only if no hasher exists. + # so this is a real collision surface. sha256 (not sha1/md5 — those trip weak-hash linters + # and are collision-broken) keeps the mapping injective; fall back to substitution only if + # no hasher exists. This is a filename derivation, not a security context. key="${repo}//${wf}" - keyhash="$(printf '%s' "$key" | sha1sum 2>/dev/null | cut -d' ' -f1)" + keyhash="$(printf '%s' "$key" | sha256sum 2>/dev/null | cut -d' ' -f1)" [ -n "$keyhash" ] || keyhash="${key//[^A-Za-z0-9._-]/_}" cachef="$_RUNS_CACHE_DIR/${keyhash}.json" [ -s "$cachef" ] && { cat "$cachef"; return 0; } From eeb7c38619c661111e4108116c9cfbcea3977324 Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Mon, 20 Jul 2026 20:00:51 -0500 Subject: [PATCH 5/5] fix(canary): add shasum -a 256 fallback for macOS runners (#835 CodeRabbit) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sha256sum is coreutils (Linux); macOS runners ship `shasum -a 256` instead. Try sha256sum, then shasum, before the substitution fallback — so the cache key stays a real hash (no collision surface) on any runner. This workflow runs on ubuntu-latest, but the fallback is trivial and portability-correct. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_017BggeTEBJLw6BV8kZrM6jD --- scripts/canary-rollout.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/canary-rollout.sh b/scripts/canary-rollout.sh index bf6fe19a0..849e7589b 100755 --- a/scripts/canary-rollout.sh +++ b/scripts/canary-rollout.sh @@ -449,7 +449,9 @@ _repo_wf_runs_cached() { # and are collision-broken) keeps the mapping injective; fall back to substitution only if # no hasher exists. This is a filename derivation, not a security context. key="${repo}//${wf}" - keyhash="$(printf '%s' "$key" | sha256sum 2>/dev/null | cut -d' ' -f1)" + # sha256sum on Linux runners, shasum -a 256 on macOS; substitution only if neither + # exists (and then, at worst, the pre-existing collision surface — never a crash). + keyhash="$(printf '%s' "$key" | { sha256sum 2>/dev/null || shasum -a 256 2>/dev/null; } | cut -d' ' -f1)" [ -n "$keyhash" ] || keyhash="${key//[^A-Za-z0-9._-]/_}" cachef="$_RUNS_CACHE_DIR/${keyhash}.json" [ -s "$cachef" ] && { cat "$cachef"; return 0; }