From a9627ad74d01414600a54b4cf53ac2065b35f0bd Mon Sep 17 00:00:00 2001 From: verlyn13 Date: Sat, 1 Aug 2026 16:12:25 -0700 Subject: [PATCH] fix(system-update): report Claude Code update outcomes truthfully MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit step_claude_code branched on the exit status of `claude update`, guided by a comment asserting it "exits nonzero when already at latest". It does not: it exits 0 for updated, already-latest, and failed alike. Both arms also returned 0, so the step was structurally incapable of reporting a failed update. Across 111 run logs the step printed "claude: successfully updated" 72 times against 3 real version changes, and summarize_step keyed its console detail off that same string — so an untouched install rendered as "updated to 2.1.220", and a genuine failure would have rendered as a green check. Decide the outcome from what the updater reported instead of its exit status, and emit a `claude-update-failed:` marker with a nonzero return when it reports neither an update nor an up-to-date version. - is_network_failure: add `failed to fetch version from`, the signature of claude's release-feed failure. No existing alternative matched it, so a transient fetch error would otherwise land in the hard fail lane instead of the retryable one. - summarize_step: report "update did not apply" on failure. - collect_notices: claim the failure so it stops reaching the residual catch-all, which could only describe it as an unexplained warning. The action item is suppressed when the shared network notice already covers it. - tests: 9 assertions over a stub that reproduces the exit-0-on-failure behavior, covering updated / up-to-date / retryable / hard-fail, plus a guard that a clean run stays silent. Default-mode behavior is unchanged: only --strict aborts on a failed step, and a network-class failure still counts as a warning, so the process exit contract holds. A genuine non-network update failure now exits 1 where it previously reported success. --- scripts/system-update.sh | 62 ++++++++++++++++++--- tests/system-update/run.sh | 110 +++++++++++++++++++++++++++++++++++++ 2 files changed, 163 insertions(+), 9 deletions(-) diff --git a/scripts/system-update.sh b/scripts/system-update.sh index 01a2c36..c1c1ce1 100755 --- a/scripts/system-update.sh +++ b/scripts/system-update.sh @@ -303,7 +303,7 @@ su_status_row() { # retryable "network" class. is_network_failure() { local file="$1" - grep -qiE '(npm error code (etimedout|econnreset|econnrefused|eai_again|enotfound|ehostunreach|enetunreach|err_socket_timeout)|npm error network|network timeout|socket timeout|request to .* failed|fetch failed|proxy connection|tls connection|403 forbidden.*(security policy|do not have access)|tunnel error|error sending request for url|\(transient\)|failed to download|could not resolve host|curl: \(([67]|28|35|52|56)\)|temporary failure in name resolution|could not fetch url|connection (timed out|refused)|network is unreachable|name or service not known|ssl(error| handshake)|proxyerror|50[234] (bad gateway|service unavailable|gateway))' "$file" 2>/dev/null + grep -qiE '(npm error code (etimedout|econnreset|econnrefused|eai_again|enotfound|ehostunreach|enetunreach|err_socket_timeout)|npm error network|network timeout|socket timeout|request to .* failed|fetch failed|failed to fetch version from|proxy connection|tls connection|403 forbidden.*(security policy|do not have access)|tunnel error|error sending request for url|\(transient\)|failed to download|could not resolve host|curl: \(([67]|28|35|52|56)\)|temporary failure in name resolution|could not fetch url|connection (timed out|refused)|network is unreachable|name or service not known|ssl(error| handshake)|proxyerror|50[234] (bad gateway|service unavailable|gateway))' "$file" 2>/dev/null } # Failures that require an operator-owned workstation action rather than a @@ -1470,11 +1470,16 @@ summarize_step() { if [[ $rc -eq 0 ]]; then local ver ver="$(grep -oE '[0-9]+\.[0-9]+\.[0-9]+' "$tmp" | tail -1)" + # step_claude_code now prints "successfully updated" only when the + # updater reported an actual version change, so this no longer claims + # an update on every run. if grep -q "successfully updated" "$tmp" 2>/dev/null; then detail="updated${ver:+ to $ver}" else detail="up to date${ver:+ ($ver)}" fi + elif grep -q '^claude-update-failed: ' "$tmp" 2>/dev/null; then + detail="update did not apply" fi ;; @@ -1634,6 +1639,21 @@ collect_notices() { add_notice "AI CLI channels: ${spec} platform binary could not be repaired (targeted install and clean reinstall both failed) — likely an upstream/registry/network issue; recheck the channel and rerun 'system-update --only ai-cli-channels'" done < <(grep '^ai-cli-channel-heal-failed:' "$tmp" 2>/dev/null || true) ;; + "Claude Code") + # Claimed explicitly so a failed update stops reaching the residual + # catch-all, which could only describe it as an unexplained warning. The + # updater exits 0 on failure, so the marker is the only evidence. + local claude_fail + claude_fail="$(grep -m1 '^claude-update-failed: ' "$tmp" 2>/dev/null || true)" + if [[ -n "$claude_fail" ]]; then + add_notice "Claude Code: update did not apply — ${claude_fail#claude-update-failed: }" + # A retryable fetch/proxy error already carries the shared network + # notice; a second, differently-worded action would misdescribe it. + if ! is_network_failure "$tmp"; then + add_action_once "Claude Code: rerun 'system-update --only claude-code'; if it persists, run 'claude doctor' and inspect ~/.claude/.last-update-result.json" + fi + fi + ;; "mise runtimes") if grep -qiE 'failed to update .* repo' "$tmp" 2>/dev/null || mise_runtime_has_incomplete_discovery "$tmp"; then add_notice "mise runtimes: remote version discovery was incomplete; successful upgrades may not cover every configured runtime" @@ -2976,15 +2996,39 @@ step_claude_code() { if ! have claude; then echo "claude not found, skipping"; return 0; fi if [[ "$MODE" == "check" ]]; then claude --version - else - if claude update; then - echo "claude: successfully updated" - else - # claude update exits nonzero when already at latest — not a real failure - echo "claude: already at latest" - claude --version - fi + return 0 + fi + + local out rc reason + out="$(claude update 2>&1)" + rc=$? + printf '%s\n' "$out" + + # `claude update` exits 0 for "updated", "already at latest", and for a failed + # update alike, so the exit status cannot tell them apart. The previous + # implementation branched on it anyway and returned 0 from both arms, which + # made a failed update structurally unreportable and printed "successfully + # updated" on every run. Decide on what the updater reported instead. + if grep -qE '^Successfully updated from .* to version ' <<<"$out"; then + echo "claude: successfully updated" + return 0 + fi + if grep -qiE 'is up to date' <<<"$out"; then + echo "claude: already at latest" + return 0 + fi + + # Prefer the updater's own error line; fall back to the last non-empty line so + # the marker can never render empty. + reason="$(grep -m1 -E '^[A-Za-z]*Error: ' <<<"$out")" + if [[ -z "$reason" ]]; then + reason="$(grep -v '^[[:space:]]*$' <<<"$out" | tail -1)" + fi + echo "claude-update-failed: ${reason:-claude update reported neither an update nor an up-to-date version}" + if [[ "$rc" -eq 0 ]]; then + return 1 fi + return "$rc" } step_gh_extensions() { diff --git a/tests/system-update/run.sh b/tests/system-update/run.sh index 1a45ed0..2f9bad6 100755 --- a/tests/system-update/run.sh +++ b/tests/system-update/run.sh @@ -1623,6 +1623,116 @@ else not_ok "a clean Android Studio check emitted spurious operator output" fi +# --- Claude Code update lane ------------------------------------------------- +# `claude update` exits 0 for "updated", "already at latest", and for a failed +# update alike. The stub reproduces that exit-0-on-failure behavior, because it +# is the whole reason the outcome has to be read from the reported text: the +# historic bug reported "successfully updated" on every run and could never +# surface a failure at all. +claude_stub_dir="$TMP_ROOT/claude-stub" +mkdir -p "$claude_stub_dir" +cat >"$claude_stub_dir/claude" <<'EOF' +#!/usr/bin/env bash +case "${CLAUDE_STUB_CASE:-uptodate}" in + updated) + echo "Current version: 2.1.218" + echo "Successfully updated from 2.1.218 to version 2.1.219" + ;; + uptodate) + echo "Current version: 2.1.220" + echo "Claude Code is up to date (2.1.220)" + ;; + fetchfail) + echo "Current version: 2.1.220" + echo "Error: Failed to install native update" + echo "TelemetrySafeError: Failed to fetch version from https://downloads.claude.ai/claude-code-releases/latest after 3 attempt(s): canceled" + echo 'Try running "claude doctor" for diagnostics' + ;; + otherfail) + echo "Current version: 2.1.220" + echo "Error: could not replace the installed binary" + ;; +esac +exit 0 +EOF +chmod +x "$claude_stub_dir/claude" +PATH="$claude_stub_dir:$PATH" +export PATH +# shellcheck disable=SC2034 # consumed by sourced step_claude_code +MODE=update +export CLAUDE_STUB_CASE + +run_claude_step() { + CLAUDE_STUB_CASE="$1" + step_claude_code >"$2" 2>&1 +} + +claude_updated_log="$TMP_ROOT/claude-updated.log" +run_claude_step updated "$claude_updated_log" +claude_updated_rc=$? +if [[ "$claude_updated_rc" -eq 0 ]] && grep -q '^claude: successfully updated$' "$claude_updated_log"; then + ok "Claude Code: a real version change reports an update" +else + not_ok "Claude Code: a real version change was not reported as an update" +fi +if [[ "$(summarize_step "Claude Code" "$claude_updated_log" 0)" == "updated to 2.1.219" ]]; then + ok "Claude Code: updated row names the new version" +else + not_ok "Claude Code: updated row did not name the new version" +fi + +# The regression that started this: an up-to-date run must not claim an update. +claude_uptodate_log="$TMP_ROOT/claude-uptodate.log" +run_claude_step uptodate "$claude_uptodate_log" +claude_uptodate_rc=$? +if [[ "$claude_uptodate_rc" -eq 0 ]] && ! grep -q 'successfully updated' "$claude_uptodate_log"; then + ok "Claude Code: an up-to-date run does not claim an update" +else + not_ok "Claude Code: an up-to-date run still claims an update" +fi +if [[ "$(summarize_step "Claude Code" "$claude_uptodate_log" 0)" == "up to date (2.1.220)" ]]; then + ok "Claude Code: up-to-date row reports the installed version" +else + not_ok "Claude Code: up-to-date row misreported the installed version" +fi + +# A failed update must exit nonzero even though `claude update` exited 0. +claude_fetchfail_log="$TMP_ROOT/claude-fetchfail.log" +run_claude_step fetchfail "$claude_fetchfail_log" +claude_fetchfail_rc=$? +if [[ "$claude_fetchfail_rc" -ne 0 ]] && grep -q '^claude-update-failed: ' "$claude_fetchfail_log"; then + ok "Claude Code: a failed update exits nonzero despite claude update exiting 0" +else + not_ok "Claude Code: a failed update was swallowed as success" +fi +assert_status network "Claude Code" "$claude_fetchfail_rc" "$claude_fetchfail_log" + +# A failure with no network signature stays in the ordinary fail lane and owes +# the operator a notice plus an action. +claude_otherfail_log="$TMP_ROOT/claude-otherfail.log" +run_claude_step otherfail "$claude_otherfail_log" +claude_otherfail_rc=$? +assert_status fail "Claude Code" "$claude_otherfail_rc" "$claude_otherfail_log" +NOTICES=() +ACTION_ITEMS=() +collect_notices "Claude Code" "$claude_otherfail_log" "$claude_otherfail_rc" +if [[ "${#NOTICES[@]}" -gt 0 && "${#ACTION_ITEMS[@]}" -gt 0 ]]; then + ok "Claude Code: a failed update carries a notice and an action" +else + not_ok "Claude Code: a failed update rendered with no operator output" +fi + +# The up-to-date fixture must stay silent, or the assertion above would pass for +# a check that simply always complains. +NOTICES=() +ACTION_ITEMS=() +collect_notices "Claude Code" "$claude_uptodate_log" 0 +if [[ "${#NOTICES[@]}" -eq 0 && "${#ACTION_ITEMS[@]}" -eq 0 ]]; then + ok "Claude Code: a clean run stays silent" +else + not_ok "Claude Code: a clean run emitted spurious operator output" +fi + printf '%s\n' '----' printf '%d passed, %d failed\n' "$pass" "$fail" [[ "$fail" -eq 0 ]]