From 01dd8756000957b61edc26bef354b048162d0080 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 23 Sep 2026 23:11:10 -0700 Subject: [PATCH 1/3] test: fail when the cmux-tui resolver retries a missing manifest A 404 on a cmux-tui client manifest is definitive, but the resolver's curl --retry-all-errors retries it five times, 3 s apart. Each unpublished candidate costs 15 s in the test and in every release and nightly fallback. Bound that walk, and prove a transient probe failure is still retried. Co-Authored-By: Claude Opus 5.5 --- .../test_ci_resolve_cmux_tui_client_commit.sh | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tests/test_ci_resolve_cmux_tui_client_commit.sh b/tests/test_ci_resolve_cmux_tui_client_commit.sh index 5777ca74c809..675aee0090bf 100755 --- a/tests/test_ci_resolve_cmux_tui_client_commit.sh +++ b/tests/test_ci_resolve_cmux_tui_client_commit.sh @@ -146,4 +146,55 @@ if ! grep -q 'retrying' "$TMP/flaky.err"; then exit 1 fi +# A missing manifest is definitive: the walk past it must not sleep through retries. +# curl --retry-all-errors retried every 404 five times, 3 s apart, so each unpublished +# candidate cost 15 s here and in every release and nightly fallback. +rm "$STORE/$C3/manifest.json" +started=$SECONDS +got="$(cd "$TMP/full" && "$RESOLVER" --max-fallback 3 2>/dev/null)" +elapsed=$((SECONDS - started)) +if [[ "$got" != "$C1" ]]; then + echo "FAIL: fallback past a missing manifest must resolve $C1, got '$got'" + exit 1 +fi +if [[ $elapsed -gt 5 ]]; then + echo "FAIL: skipping a missing manifest took ${elapsed}s; a 404 must not be retried" + exit 1 +fi + +# A transient probe failure (here, DNS) is still retried, so it does not read as a +# missing manifest. The shim fails its first two calls, then runs the real curl. +REAL_CURL="$(command -v curl)" +mkdir -p "$TMP/shim" +cat >"$TMP/shim/curl" <"$TMP/curl-failures-left" + echo "curl: (6) Could not resolve host: files.cmux.com" >&2 + exit 6 +fi +exec "$REAL_CURL" "\$@" +EOF +chmod +x "$TMP/shim/curl" +printf '{"commit":"%s"}\n' "$C3" >"$STORE/$C3/manifest.json" +echo 2 >"$TMP/curl-failures-left" +got="$(cd "$TMP/full" && PATH="$TMP/shim:$PATH" CMUX_TUI_CLIENT_PROBE_RETRY_SECONDS=0 "$RESOLVER" 2>/dev/null)" +if [[ "$got" != "$C3" ]]; then + echo "FAIL: a transient probe failure must be retried and resolve $C3, got '$got'" + exit 1 +fi +if [[ "$(cat "$TMP/curl-failures-left")" != 0 ]]; then + echo "FAIL: the curl shim was not exercised (test setup)" + exit 1 +fi +for bad in x -1; do + rc=0 + (cd "$TMP/full" && CMUX_TUI_CLIENT_PROBE_RETRY_SECONDS="$bad" "$RESOLVER" >/dev/null 2>&1) || rc=$? + if [[ $rc != 64 ]]; then + echo "FAIL: CMUX_TUI_CLIENT_PROBE_RETRY_SECONDS='$bad' must exit 64 (usage error), got $rc" + exit 1 + fi +done + echo "PASS: resolve-cmux-tui-client-commit picks the newest published cmux-tui commit, shallow or not" From 6e9eb5b7e5bd6519dc39333e9942a9f63ec36832 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 23 Sep 2026 23:11:30 -0700 Subject: [PATCH 2/3] ci: stop retrying a missing cmux-tui manifest The manifest probe used curl --retry-all-errors, which cannot tell a 404 from a network failure, so every unpublished candidate slept through five 3 s retries. Probe with an explicit loop instead: a 404 or 410 (or a missing file:// path) moves on at once, while DNS, connection and 5xx failures keep the bounded retry. Statuses below 400 still count as found, as they did under curl -f. The tui-resolution guard test drops from 61 s to under 1 s locally. Co-Authored-By: Claude Opus 5.5 --- scripts/ci/resolve-cmux-tui-client-commit.sh | 46 +++++++++++++++++--- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/scripts/ci/resolve-cmux-tui-client-commit.sh b/scripts/ci/resolve-cmux-tui-client-commit.sh index a13512a78cfa..331f6def8551 100755 --- a/scripts/ci/resolve-cmux-tui-client-commit.sh +++ b/scripts/ci/resolve-cmux-tui-client-commit.sh @@ -22,7 +22,9 @@ # Env: CMUX_TUI_CLIENT_MANIFEST_BASE (default https://files.cmux.com/cmux-tui), # CMUX_TUI_CLIENT_REMOTE (default origin; where a shallow clone deepens from), # CMUX_TUI_CLIENT_FETCH_ATTEMPTS (default 5; tries per deepen before giving up), -# CMUX_TUI_CLIENT_FETCH_RETRY_SECONDS (default 2; first backoff, doubles per try). +# CMUX_TUI_CLIENT_FETCH_RETRY_SECONDS (default 2; first backoff, doubles per try), +# CMUX_TUI_CLIENT_PROBE_RETRY_SECONDS (default 3; wait between manifest probes after +# a transient failure; a 404 never waits). # The chosen 40-hex commit is the only stdout line; diagnostics go to stderr. set -euo pipefail @@ -33,7 +35,7 @@ MAX_FALLBACK=0 HEAD_REV=HEAD log() { echo "resolve-cmux-tui-client-commit: $*" >&2; } -usage() { sed -n '2,24p' "$0"; } +usage() { sed -n '2,28p' "$0"; } while [[ $# -gt 0 ]]; do case "$1" in @@ -51,15 +53,20 @@ esac MAX_FALLBACK=$((10#$MAX_FALLBACK)) FETCH_ATTEMPTS="${CMUX_TUI_CLIENT_FETCH_ATTEMPTS:-5}" FETCH_RETRY_SECONDS="${CMUX_TUI_CLIENT_FETCH_RETRY_SECONDS:-2}" +PROBE_RETRY_SECONDS="${CMUX_TUI_CLIENT_PROBE_RETRY_SECONDS:-3}" case "$FETCH_ATTEMPTS" in ''|*[!0-9]*) echo "error: CMUX_TUI_CLIENT_FETCH_ATTEMPTS must be a positive integer" >&2; exit 64 ;; esac case "$FETCH_RETRY_SECONDS" in ''|*[!0-9]*) echo "error: CMUX_TUI_CLIENT_FETCH_RETRY_SECONDS must be a non-negative integer" >&2; exit 64 ;; esac +case "$PROBE_RETRY_SECONDS" in + ''|*[!0-9]*) echo "error: CMUX_TUI_CLIENT_PROBE_RETRY_SECONDS must be a non-negative integer" >&2; exit 64 ;; +esac # Normalize before the positive check so an all-zero spelling (00) is rejected too. FETCH_ATTEMPTS=$((10#$FETCH_ATTEMPTS)) FETCH_RETRY_SECONDS=$((10#$FETCH_RETRY_SECONDS)) +PROBE_RETRY_SECONDS=$((10#$PROBE_RETRY_SECONDS)) if [[ $FETCH_ATTEMPTS -lt 1 ]]; then echo "error: CMUX_TUI_CLIENT_FETCH_ATTEMPTS must be a positive integer" >&2 exit 64 @@ -133,16 +140,41 @@ if [[ ${#CANDIDATES[@]} -eq 0 ]]; then exit 1 fi +# Succeeds when the manifest exists. A definitive miss (HTTP 404/410, or a missing +# file:// path, curl exit 37) returns at once. Anything else (DNS, connection reset, +# 5xx, 429) is retried with bounded delay so a network hiccup on the release runner +# does not read as a missing manifest. curl --retry-all-errors cannot tell the two +# apart and slept through five retries on every genuine 404. +PROBE_ATTEMPTS=6 +probe_manifest() { + local url="$1" attempt code rc + for ((attempt = 1; attempt <= PROBE_ATTEMPTS; attempt++)); do + rc=0 + code="$(curl --proto '=https,file' --tlsv1.2 -sS -o /dev/null -w '%{http_code}' "$url" 2>/dev/null)" || rc=$? + if [[ $rc -eq 0 ]]; then + # Like curl -f, anything below 400 is found; file:// reports 000 on success. + case "$code" in + 404|410) return 1 ;; + [45]??) ;; + *) return 0 ;; + esac + elif [[ $rc -eq 37 ]]; then + return 1 + fi + if [[ $attempt -lt $PROBE_ATTEMPTS ]]; then sleep "$PROBE_RETRY_SECONDS"; fi + done + log "manifest probe still failing after $PROBE_ATTEMPTS attempts (curl exit $rc, HTTP $code): $url" + return 1 +} + chosen="" skipped=0 for ((i = 0; i < ${#CANDIDATES[@]}; i++)); do sha="${CANDIDATES[$i]}" url="$BASE/$sha/manifest.json" - # One probe per candidate: a 404 moves on to the next candidate (or fails exact - # mode). curl retries resolution and connection blips itself, bounded, so a DNS - # hiccup on the release runner does not read as a missing manifest. - if curl --proto '=https,file' --tlsv1.2 -fsS -o /dev/null \ - --retry 5 --retry-delay 3 --retry-all-errors --retry-connrefused "$url" 2>/dev/null; then + # One probe per candidate: a missing manifest moves on to the next candidate (or + # fails exact mode). + if probe_manifest "$url"; then chosen="$sha" break fi From 0d15abcc04ac33c1fc807e1bebfbf530e8896fc6 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 23 Sep 2026 23:56:46 -0700 Subject: [PATCH 3/3] test: cover the resolver's HTTP miss and retry branches The curl shim now replays scripted outcomes, so the test reaches the HTTP branches a file:// store never produces: DNS, 503 and 429 are retried until the published manifest is found, and 404 and 410 skip the candidate at once. A resolver that retried 404, or gave up on a 5xx, now fails with a message instead of exiting silently under set -e. The wall-clock bound for the 404 walk is 10 s, still under the old 15 s cost. Co-Authored-By: Claude Opus 5.5 --- .../test_ci_resolve_cmux_tui_client_commit.sh | 58 ++++++++++++++----- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/tests/test_ci_resolve_cmux_tui_client_commit.sh b/tests/test_ci_resolve_cmux_tui_client_commit.sh index 675aee0090bf..382572516c24 100755 --- a/tests/test_ci_resolve_cmux_tui_client_commit.sh +++ b/tests/test_ci_resolve_cmux_tui_client_commit.sh @@ -157,37 +157,63 @@ if [[ "$got" != "$C1" ]]; then echo "FAIL: fallback past a missing manifest must resolve $C1, got '$got'" exit 1 fi -if [[ $elapsed -gt 5 ]]; then +# The old behaviour took at least 15 s; the bound leaves room for a loaded runner. +if [[ $elapsed -gt 10 ]]; then echo "FAIL: skipping a missing manifest took ${elapsed}s; a 404 must not be retried" exit 1 fi -# A transient probe failure (here, DNS) is still retried, so it does not read as a -# missing manifest. The shim fails its first two calls, then runs the real curl. +# The curl shim replays one scripted outcome per call from $TMP/curl-script, then runs +# the real curl once the script is empty: `dns` fails like a DNS blip (exit 6), and a +# status code answers like an HTTP server (curl exit 0, that code on stdout). REAL_CURL="$(command -v curl)" mkdir -p "$TMP/shim" -cat >"$TMP/shim/curl" <"$TMP/shim/curl" <"$TMP/curl-failures-left" +script="$TMP/curl-script" +step="\$(head -n 1 "\$script")" +if [[ -z "\$step" ]]; then exec "$REAL_CURL" "\$@"; fi +tail -n +2 "\$script" >"\$script.next" +mv "\$script.next" "\$script" +if [[ "\$step" == dns ]]; then echo "curl: (6) Could not resolve host: files.cmux.com" >&2 exit 6 fi -exec "$REAL_CURL" "\$@" -EOF +printf '%s' "\$step" +SHIM chmod +x "$TMP/shim/curl" printf '{"commit":"%s"}\n' "$C3" >"$STORE/$C3/manifest.json" -echo 2 >"$TMP/curl-failures-left" -got="$(cd "$TMP/full" && PATH="$TMP/shim:$PATH" CMUX_TUI_CLIENT_PROBE_RETRY_SECONDS=0 "$RESOLVER" 2>/dev/null)" +resolve_with_curl_script() { + printf '%s\n' "$@" >"$TMP/curl-script" + (cd "$TMP/full" && PATH="$TMP/shim:$PATH" CMUX_TUI_CLIENT_PROBE_RETRY_SECONDS=0 "$RESOLVER" --max-fallback 3 2>/dev/null) +} +expect_curl_script_drained() { + if [[ -s "$TMP/curl-script" ]]; then + echo "FAIL: the curl shim did not consume its script (test setup): $(tr '\n' ' ' <"$TMP/curl-script")" + exit 1 + fi +} + +# Transient failures (DNS, 5xx, 429) are retried, so they do not read as a missing +# manifest: C3 is published and must still win. +got="$(resolve_with_curl_script dns 503 429 || true)" +expect_curl_script_drained if [[ "$got" != "$C3" ]]; then - echo "FAIL: a transient probe failure must be retried and resolve $C3, got '$got'" - exit 1 -fi -if [[ "$(cat "$TMP/curl-failures-left")" != 0 ]]; then - echo "FAIL: the curl shim was not exercised (test setup)" + echo "FAIL: transient probe failures must be retried and resolve $C3, got '$got'" exit 1 fi + +# HTTP 404 and 410 are definitive: the resolver moves past C3 at once, although a +# retry would have found it, and falls back to C1. +for missing in 404 410; do + got="$(resolve_with_curl_script "$missing" || true)" + expect_curl_script_drained + if [[ "$got" != "$C1" ]]; then + echo "FAIL: HTTP $missing must skip $C3 without a retry and fall back to $C1, got '$got'" + exit 1 + fi +done + for bad in x -1; do rc=0 (cd "$TMP/full" && CMUX_TUI_CLIENT_PROBE_RETRY_SECONDS="$bad" "$RESOLVER" >/dev/null 2>&1) || rc=$?