From 72c44db9926ab9edfc3c6b504d755669414a6cbb Mon Sep 17 00:00:00 2001 From: Matthew Valancy Date: Wed, 11 Nov 2026 05:02:42 -0800 Subject: [PATCH 1/2] fix(installer): retry the network check before cancelling the install The pre-flight check made one 5 s request to github.com. With no terminal, one dropped request answered its Continue? prompt with no and cancelled the whole install; CI's installer test hit this on 2026-11-11 (job 130418062294, the stranded case). network_reachable now asks up to three times, 2 s apart (GRAPHDONE_NETWORK_TRIES / GRAPHDONE_NETWORK_PAUSE). installer-contract runs the real functions against a fake curl: two failures then success continues, three failures cancel, and TRIES=1 asks once. It fails on the previous install.sh. Co-Authored-By: Claude Opus 5.5 --- scripts/install.sh | 46 ++++++++++++++++---------- tests/toolchain/installer-contract.mjs | 42 +++++++++++++++++++++-- 2 files changed, 68 insertions(+), 20 deletions(-) diff --git a/scripts/install.sh b/scripts/install.sh index 8483a8f6..1f2ae043 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -437,27 +437,37 @@ check_disk_space() { fi } -# Check network connectivity -check_network() { +# Whether GitHub answers. One dropped request is not "no network": it asks up +# to GRAPHDONE_NETWORK_TRIES times (default 3), GRAPHDONE_NETWORK_PAUSE +# seconds apart (default 2), before saying it is unreachable. +network_reachable() { local test_url="https://github.com" + local tries="${GRAPHDONE_NETWORK_TRIES:-3}" + local pause="${GRAPHDONE_NETWORK_PAUSE:-2}" + local attempt=1 - if command -v curl >/dev/null 2>&1; then - if ! curl -sf --max-time 5 "$test_url" >/dev/null 2>&1; then - warn "Network connectivity test failed" - printf "${CYAN}ℹ${NC} This may cause download failures. Continue? ${GRAY}[y/N]${NC} " - read -r response || response="n" - if [ "$response" != "y" ] && [ "$response" != "Y" ]; then - error "Installation cancelled - network required" - fi + while :; do + if command -v curl >/dev/null 2>&1; then + curl -sf --max-time 5 "$test_url" >/dev/null 2>&1 && return 0 + elif command -v wget >/dev/null 2>&1; then + wget -q --timeout=5 --spider "$test_url" 2>/dev/null && return 0 + else + return 0 # nothing to test with; the downloads will say fi - elif command -v wget >/dev/null 2>&1; then - if ! wget -q --timeout=5 --spider "$test_url" 2>/dev/null; then - warn "Network connectivity test failed" - printf "${CYAN}ℹ${NC} This may cause download failures. Continue? ${GRAY}[y/N]${NC} " - read -r response || response="n" - if [ "$response" != "y" ] && [ "$response" != "Y" ]; then - error "Installation cancelled - network required" - fi + [ "$attempt" -ge "$tries" ] && return 1 + attempt=$((attempt + 1)) + sleep "$pause" + done +} + +# Check network connectivity +check_network() { + if ! network_reachable; then + warn "Network connectivity test failed" + printf "${CYAN}ℹ${NC} This may cause download failures. Continue? ${GRAY}[y/N]${NC} " + read -r response || response="n" + if [ "$response" != "y" ] && [ "$response" != "Y" ]; then + error "Installation cancelled - network required" fi fi } diff --git a/tests/toolchain/installer-contract.mjs b/tests/toolchain/installer-contract.mjs index ac9adc54..ba64f5f4 100644 --- a/tests/toolchain/installer-contract.mjs +++ b/tests/toolchain/installer-contract.mjs @@ -1,7 +1,8 @@ import assert from 'node:assert/strict'; import { spawnSync } from 'node:child_process'; -import { existsSync, readFileSync } from 'node:fs'; -import { dirname, resolve } from 'node:path'; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import test from 'node:test'; @@ -156,3 +157,40 @@ test('documented one-line install URLs serve the installer from main', () => { const wrong = listed.stdout.split('\n').filter(Boolean).filter((row) => !row.includes('/GraphDone-Core/main/scripts/install.sh')); assert.deepEqual(wrong, [], `Install URLs must point at /main/scripts/install.sh:\n${wrong.join('\n')}`); }); + +// The pre-flight network check once made a single 5 s request; on a run with +// no terminal one dropped request cancelled the whole install (CI saw it on +// 2026-11-11). It now asks up to three times before calling the network down. +function networkCheck(curlFailures, tries) { + const source = readFileSync(resolve(root, INSTALLER), 'utf8'); + const fn = (name) => source.match(new RegExp(`^${name}\\(\\) \\{\\n[\\s\\S]*?\\n\\}`, 'm'))?.[0]; + assert.ok(fn('network_reachable') && fn('check_network'), 'install.sh defines network_reachable and check_network'); + const dir = mkdtempSync(join(tmpdir(), 'gd-net-')); + try { + writeFileSync(join(dir, 'curl'), `#!/bin/sh\nn=$(cat "${dir}/calls" 2>/dev/null || echo 0); n=$((n + 1)); echo $n > "${dir}/calls"\n[ "$n" -gt ${curlFailures} ]\n`, { mode: 0o755 }); + const script = [ + 'warn() { echo "WARN $*"; }', 'error() { echo "ERROR $*"; exit 1; }', + fn('network_reachable'), fn('check_network'), 'check_network { + const flaky = networkCheck(2); + assert.match(flaky.out, /CONTINUED/, flaky.out); + assert.doesNotMatch(flaky.out, /WARN/); + assert.equal(flaky.calls, 3); + + const down = networkCheck(99); + assert.match(down.out, /ERROR Installation cancelled - network required/, down.out); + assert.equal(down.calls, 3); + + assert.equal(networkCheck(99, 1).calls, 1, 'GRAPHDONE_NETWORK_TRIES=1 asks once'); +}); From d4235e8f4e3372bcb4cc052972169b37796ea095 Mon Sep 17 00:00:00 2001 From: Matthew Valancy Date: Wed, 11 Nov 2026 05:14:20 -0800 Subject: [PATCH 2/2] fix(installer): answer Continue? from the terminal, one wget try, sane retry input Review of #287: - The Continue? prompt ran inside the background check_network job, whose stdin is empty, so it could never be answered. Only network_reachable runs in the background now; the prompt runs after it, in the foreground. - Prompts read the terminal (ask_continue), not stdin: under curl | sh stdin is the script itself. With no terminal the answer is no. The disk-space and macOS prompts use it too. - wget gets --tries=1 (its default of 20 tries could hold the install for minutes on a silently dropped route). - A non-number GRAPHDONE_NETWORK_TRIES/PAUSE falls back to the default instead of looping forever. - The dots keep moving while retries run, and a continued install says GitHub was not reachable instead of Connected. The contract now runs the functions under sh with only fakes on PATH: curl and wget paths, bad input, and the terminal-vs-stdin answer. 3 of its 8 tests fail on the previous commit and on dev. Co-Authored-By: Claude Opus 5.5 --- scripts/install.sh | 72 +++++++++++++++----------- tests/toolchain/installer-contract.mjs | 50 +++++++++++++----- 2 files changed, 81 insertions(+), 41 deletions(-) diff --git a/scripts/install.sh b/scripts/install.sh index 1f2ae043..36ede0a2 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -429,14 +429,22 @@ check_disk_space() { if [ "$available_gb" -lt "$required_gb" ]; then warn "Low disk space: ${available_gb}GB available (${required_gb}GB recommended)" printf "${CYAN}ℹ${NC} Continue anyway? ${GRAY}[y/N]${NC} " - read -r response || response="n" - if [ "$response" != "y" ] && [ "$response" != "Y" ]; then + if ! ask_continue; then error "Installation cancelled due to low disk space" fi fi fi } +# Ask "Continue? [y/N]" of the person at the terminal. The answer is read +# from the terminal, not stdin: under `curl ... | sh` stdin is this script. +# With no terminal (CI, a pipe) the answer is no. GRAPHDONE_TTY is for tests. +ask_continue() { + local answer="" + { read -r answer < "${GRAPHDONE_TTY:-/dev/tty}"; } 2>/dev/null || answer="" + [ "$answer" = "y" ] || [ "$answer" = "Y" ] +} + # Whether GitHub answers. One dropped request is not "no network": it asks up # to GRAPHDONE_NETWORK_TRIES times (default 3), GRAPHDONE_NETWORK_PAUSE # seconds apart (default 2), before saying it is unreachable. @@ -444,13 +452,16 @@ network_reachable() { local test_url="https://github.com" local tries="${GRAPHDONE_NETWORK_TRIES:-3}" local pause="${GRAPHDONE_NETWORK_PAUSE:-2}" + case "$tries" in ''|*[!0-9]*|0) tries=3 ;; esac + case "$pause" in ''|*[!0-9]*) pause=2 ;; esac local attempt=1 while :; do if command -v curl >/dev/null 2>&1; then curl -sf --max-time 5 "$test_url" >/dev/null 2>&1 && return 0 elif command -v wget >/dev/null 2>&1; then - wget -q --timeout=5 --spider "$test_url" 2>/dev/null && return 0 + # --tries=1: wget's own default is 20 tries, minutes on a dropped route + wget -q --tries=1 --timeout=5 --spider "$test_url" 2>/dev/null && return 0 else return 0 # nothing to test with; the downloads will say fi @@ -460,16 +471,19 @@ network_reachable() { done } +# GitHub did not answer: go on only if the person says so. +network_unreachable() { + warn "Network connectivity test failed" + printf "${CYAN}ℹ${NC} This may cause download failures. Continue? ${GRAY}[y/N]${NC} " + if ! ask_continue; then + printf "\n" + error "Installation cancelled - network required" + fi +} + # Check network connectivity check_network() { - if ! network_reachable; then - warn "Network connectivity test failed" - printf "${CYAN}ℹ${NC} This may cause download failures. Continue? ${GRAY}[y/N]${NC} " - read -r response || response="n" - if [ "$response" != "y" ] && [ "$response" != "Y" ]; then - error "Installation cancelled - network required" - fi - fi + network_reachable || network_unreachable } # ───────────────────────────────────────────────────────────────────────── @@ -3018,17 +3032,14 @@ install_graphdone() { printf "\n" printf "${TEAL}────────────────────────────────────${NC} ${CYAN}${BOLD}✈️ Pre-flight Checks${NC} ${TEAL}────────────────────────────────────────${NC}\n" - # Check network connectivity with 4-dot animation - check_network & + # Check network connectivity with 4-dot animation. Only the check runs in + # the background: a prompt there could never be answered. + network_reachable & network_pid=$! - for cycle in 1 2 3 4 5 6; do - # Check if network check is still running - if ! kill -0 $network_pid 2>/dev/null; then - break - fi - - # Build the dots display based on cycle + cycle=1 + while kill -0 $network_pid 2>/dev/null; do + # Build the dots display based on cycle (it repeats while retries run) dots_display="" if [ $cycle -ge 2 ]; then dots_display=" ${GRAY}●${NC}" @@ -3046,15 +3057,19 @@ install_graphdone() { printf "\r ${BLUE}◉${NC} ${GRAY}Checking network${NC}$dots_display" printf "\033[K" sleep 0.5 + cycle=$((cycle % 5 + 1)) done - wait $network_pid - - # Show all 4 dots completed - printf "\r ${BLUE}◉${NC} ${GRAY}Checking network${NC} ${GRAY}●${NC} ${BLUE}●${NC} ${CYAN}●${NC} ${GREEN}●${NC}" - sleep 0.3 - - printf "\r\033[K ${GREEN}✓${NC} ${GRAY}Network:${NC} ${BOLD}Connected${NC}\n" + if wait $network_pid; then + # Show all 4 dots completed + printf "\r ${BLUE}◉${NC} ${GRAY}Checking network${NC} ${GRAY}●${NC} ${BLUE}●${NC} ${CYAN}●${NC} ${GREEN}●${NC}" + sleep 0.3 + printf "\r\033[K ${GREEN}✓${NC} ${GRAY}Network:${NC} ${BOLD}Connected${NC}\n" + else + printf "\r\033[K" + network_unreachable + printf " ${YELLOW}⚠${NC} ${GRAY}Network:${NC} ${BOLD}GitHub not reachable, continuing${NC}\n" + fi # Test download speed with 4-dot animation local download_tmp="/tmp/graphdone_download_$$" @@ -3251,8 +3266,7 @@ install_graphdone() { printf " ${GRAY}Your version (${BOLD}${MACOS_VERSION}${NC}${GRAY}) may not be fully supported${NC}\n" printf "\n" printf " ${CYAN}ℹ${NC} Continue installation anyway? ${GRAY}[y/N]${NC} " - read -r response || response="n" - if [ "$response" != "y" ] && [ "$response" != "Y" ]; then + if ! ask_continue; then printf "\n" error "Installation cancelled - please upgrade to macOS 10.15 or later" fi diff --git a/tests/toolchain/installer-contract.mjs b/tests/toolchain/installer-contract.mjs index ba64f5f4..06dc10e3 100644 --- a/tests/toolchain/installer-contract.mjs +++ b/tests/toolchain/installer-contract.mjs @@ -1,6 +1,6 @@ import assert from 'node:assert/strict'; import { spawnSync } from 'node:child_process'; -import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -160,37 +160,63 @@ test('documented one-line install URLs serve the installer from main', () => { // The pre-flight network check once made a single 5 s request; on a run with // no terminal one dropped request cancelled the whole install (CI saw it on -// 2026-11-11). It now asks up to three times before calling the network down. -function networkCheck(curlFailures, tries) { +// 2026-11-11). It now asks up to three times before calling the network down, +// and asks the person at the terminal (never stdin, which is this script under +// `curl | sh`) whether to go on. Run under sh, as `curl ... | sh` runs it. +function networkCheck({ failures, tool = 'curl', tries, tty, stdin = '' }) { const source = readFileSync(resolve(root, INSTALLER), 'utf8'); const fn = (name) => source.match(new RegExp(`^${name}\\(\\) \\{\\n[\\s\\S]*?\\n\\}`, 'm'))?.[0]; - assert.ok(fn('network_reachable') && fn('check_network'), 'install.sh defines network_reachable and check_network'); + const names = ['ask_continue', 'network_reachable', 'network_unreachable', 'check_network']; + for (const name of names) assert.ok(fn(name), `install.sh defines ${name}`); const dir = mkdtempSync(join(tmpdir(), 'gd-net-')); try { - writeFileSync(join(dir, 'curl'), `#!/bin/sh\nn=$(cat "${dir}/calls" 2>/dev/null || echo 0); n=$((n + 1)); echo $n > "${dir}/calls"\n[ "$n" -gt ${curlFailures} ]\n`, { mode: 0o755 }); + // Only the fake tool and sleep are on PATH, so the real curl or wget is never reached. + writeFileSync(join(dir, tool), `#!/bin/sh\nn=$(/bin/cat "${dir}/calls" 2>/dev/null || echo 0); n=$((n + 1)); echo $n > "${dir}/calls"\necho "$*" > "${dir}/args"\n[ "$n" -gt ${failures} ]\n`, { mode: 0o755 }); + symlinkSync('/bin/sleep', join(dir, 'sleep')); + if (tty !== undefined) writeFileSync(join(dir, 'tty'), `${tty}\n`); const script = [ 'warn() { echo "WARN $*"; }', 'error() { echo "ERROR $*"; exit 1; }', - fn('network_reachable'), fn('check_network'), 'check_network (existsSync(join(dir, name)) ? readFileSync(join(dir, name), 'utf8') : ''); + return { out: run.stdout + run.stderr, calls: Number(read('calls') || 0), args: read('args') }; } finally { rmSync(dir, { recursive: true, force: true }); } } test('installer network check survives two dropped requests and gives up after three', () => { - const flaky = networkCheck(2); + const flaky = networkCheck({ failures: 2 }); assert.match(flaky.out, /CONTINUED/, flaky.out); assert.doesNotMatch(flaky.out, /WARN/); assert.equal(flaky.calls, 3); - const down = networkCheck(99); + const down = networkCheck({ failures: 99 }); assert.match(down.out, /ERROR Installation cancelled - network required/, down.out); assert.equal(down.calls, 3); - assert.equal(networkCheck(99, 1).calls, 1, 'GRAPHDONE_NETWORK_TRIES=1 asks once'); + assert.equal(networkCheck({ failures: 99, tries: 1 }).calls, 1, 'GRAPHDONE_NETWORK_TRIES=1 asks once'); + assert.equal(networkCheck({ failures: 99, tries: 'abc' }).calls, 3, 'a non-number falls back to 3 instead of looping forever'); +}); + +test('installer network check with only wget asks wget for one try each time', () => { + const flaky = networkCheck({ failures: 1, tool: 'wget' }); + assert.match(flaky.out, /CONTINUED/, flaky.out); + assert.equal(flaky.calls, 2); + assert.match(flaky.args, /--tries=1/, 'wget would otherwise retry 20 times on its own'); +}); + +test('installer asks the terminal, not stdin, whether to go on without GitHub', () => { + assert.match(networkCheck({ failures: 99, tty: 'y' }).out, /WARN Network connectivity test failed[\s\S]*CONTINUED/); + assert.match(networkCheck({ failures: 99, tty: 'n' }).out, /ERROR Installation cancelled/); + // Under `curl | sh` stdin is the script itself; a "y" there is not an answer. + assert.match(networkCheck({ failures: 99, stdin: 'y\n' }).out, /ERROR Installation cancelled/); });